From b485d8a3c138eeae6a395373fa9ccec2960e0f54 Mon Sep 17 00:00:00 2001 From: Travis Tomsu Date: Wed, 10 Jan 2024 09:48:25 -0500 Subject: [PATCH 1/3] Add suspended as option to AdminService.CreateUser() --- github/admin_users.go | 12 +++++++----- github/admin_users_test.go | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/github/admin_users.go b/github/admin_users.go index 0d3fe9fafc..be0f225fcb 100644 --- a/github/admin_users.go +++ b/github/admin_users.go @@ -13,8 +13,9 @@ import ( // createUserRequest is a subset of User and is used internally // by CreateUser to pass only the known fields for the endpoint. type createUserRequest struct { - Login *string `json:"login,omitempty"` - Email *string `json:"email,omitempty"` + Login *string `json:"login,omitempty"` + Email *string `json:"email,omitempty"` + Suspended *bool `json:"suspended,omitempty"` } // CreateUser creates a new user in GitHub Enterprise. @@ -22,12 +23,13 @@ type createUserRequest struct { // GitHub API docs: https://docs.github.com/enterprise-server@3.11/rest/enterprise-admin/users#create-a-user // //meta:operation POST /admin/users -func (s *AdminService) CreateUser(ctx context.Context, login, email string) (*User, *Response, error) { +func (s *AdminService) CreateUser(ctx context.Context, login, email string, suspended bool) (*User, *Response, error) { u := "admin/users" userReq := &createUserRequest{ - Login: &login, - Email: &email, + Login: &login, + Email: &email, + Suspended: &suspended, } req, err := s.client.NewRequest("POST", u, userReq) diff --git a/github/admin_users_test.go b/github/admin_users_test.go index 2dac611709..a61a06522d 100644 --- a/github/admin_users_test.go +++ b/github/admin_users_test.go @@ -25,7 +25,7 @@ func TestAdminUsers_Create(t *testing.T) { assertNilError(t, json.NewDecoder(r.Body).Decode(v)) testMethod(t, r, "POST") - want := &createUserRequest{Login: String("github"), Email: String("email@domain.com")} + want := &createUserRequest{Login: String("github"), Email: String("email@domain.com"), Suspended: Bool(false)} if !cmp.Equal(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } @@ -34,7 +34,7 @@ func TestAdminUsers_Create(t *testing.T) { }) ctx := context.Background() - org, _, err := client.Admin.CreateUser(ctx, "github", "email@domain.com") + org, _, err := client.Admin.CreateUser(ctx, "github", "email@domain.com", false) if err != nil { t.Errorf("Admin.CreateUser returned error: %v", err) } @@ -46,7 +46,7 @@ func TestAdminUsers_Create(t *testing.T) { const methodName = "CreateUser" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Admin.CreateUser(ctx, "github", "email@domain.com") + got, resp, err := client.Admin.CreateUser(ctx, "github", "email@domain.com", false) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } From 3e623b843baa6e364f7de792c609cad6a1585a9c Mon Sep 17 00:00:00 2001 From: Travis Tomsu Date: Tue, 23 Jan 2024 09:08:19 -0500 Subject: [PATCH 2/3] Pass the (now public struct) request body directly --- github/admin_users.go | 19 ++++++++----------- github/admin_users_test.go | 20 ++++++++++++++------ github/github-accessors.go | 24 ++++++++++++++++++++++++ github/github-accessors_test.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 17 deletions(-) diff --git a/github/admin_users.go b/github/admin_users.go index be0f225fcb..746d5fe225 100644 --- a/github/admin_users.go +++ b/github/admin_users.go @@ -10,28 +10,25 @@ import ( "fmt" ) -// createUserRequest is a subset of User and is used internally -// by CreateUser to pass only the known fields for the endpoint. -type createUserRequest struct { - Login *string `json:"login,omitempty"` +// CreateUserRequest represents the fields sent to the `CreateUser` endpoint. +// Note that `Login` is a required field. +type CreateUserRequest struct { + Login *string `json:"login"` Email *string `json:"email,omitempty"` Suspended *bool `json:"suspended,omitempty"` } +type CreateUserOptions struct { +} + // CreateUser creates a new user in GitHub Enterprise. // // GitHub API docs: https://docs.github.com/enterprise-server@3.11/rest/enterprise-admin/users#create-a-user // //meta:operation POST /admin/users -func (s *AdminService) CreateUser(ctx context.Context, login, email string, suspended bool) (*User, *Response, error) { +func (s *AdminService) CreateUser(ctx context.Context, userReq *CreateUserRequest) (*User, *Response, error) { u := "admin/users" - userReq := &createUserRequest{ - Login: &login, - Email: &email, - Suspended: &suspended, - } - req, err := s.client.NewRequest("POST", u, userReq) if err != nil { return nil, nil, err diff --git a/github/admin_users_test.go b/github/admin_users_test.go index a61a06522d..8c82792f65 100644 --- a/github/admin_users_test.go +++ b/github/admin_users_test.go @@ -21,11 +21,11 @@ func TestAdminUsers_Create(t *testing.T) { defer teardown() mux.HandleFunc("/admin/users", func(w http.ResponseWriter, r *http.Request) { - v := new(createUserRequest) + v := new(CreateUserRequest) assertNilError(t, json.NewDecoder(r.Body).Decode(v)) testMethod(t, r, "POST") - want := &createUserRequest{Login: String("github"), Email: String("email@domain.com"), Suspended: Bool(false)} + want := &CreateUserRequest{Login: String("github"), Email: String("email@domain.com"), Suspended: Bool(false)} if !cmp.Equal(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } @@ -34,7 +34,11 @@ func TestAdminUsers_Create(t *testing.T) { }) ctx := context.Background() - org, _, err := client.Admin.CreateUser(ctx, "github", "email@domain.com", false) + org, _, err := client.Admin.CreateUser(ctx, &CreateUserRequest{ + Login: String("github"), + Email: String("email@domain.com"), + Suspended: Bool(false), + }) if err != nil { t.Errorf("Admin.CreateUser returned error: %v", err) } @@ -46,7 +50,11 @@ func TestAdminUsers_Create(t *testing.T) { const methodName = "CreateUser" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Admin.CreateUser(ctx, "github", "email@domain.com", false) + got, resp, err := client.Admin.CreateUser(ctx, &CreateUserRequest{ + Login: String("github"), + Email: String("email@domain.com"), + Suspended: Bool(false), + }) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } @@ -177,9 +185,9 @@ func TestUserImpersonation_Delete(t *testing.T) { } func TestCreateUserRequest_Marshal(t *testing.T) { - testJSONMarshal(t, &createUserRequest{}, "{}") + testJSONMarshal(t, &CreateUserRequest{}, "{}") - u := &createUserRequest{ + u := &CreateUserRequest{ Login: String("l"), Email: String("e"), } diff --git a/github/github-accessors.go b/github/github-accessors.go index 5a763df1e2..24da99c211 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -4478,6 +4478,30 @@ func (c *CreateUserProjectOptions) GetBody() string { return *c.Body } +// GetEmail returns the Email field if it's non-nil, zero value otherwise. +func (c *CreateUserRequest) GetEmail() string { + if c == nil || c.Email == nil { + return "" + } + return *c.Email +} + +// GetLogin returns the Login field if it's non-nil, zero value otherwise. +func (c *CreateUserRequest) GetLogin() string { + if c == nil || c.Login == nil { + return "" + } + return *c.Login +} + +// GetSuspended returns the Suspended field if it's non-nil, zero value otherwise. +func (c *CreateUserRequest) GetSuspended() bool { + if c == nil || c.Suspended == nil { + return false + } + return *c.Suspended +} + // GetCreated returns the Created field if it's non-nil, zero value otherwise. func (c *CreationInfo) GetCreated() Timestamp { if c == nil || c.Created == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 5c291306ab..a75476ea2f 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -5262,6 +5262,36 @@ func TestCreateUserProjectOptions_GetBody(tt *testing.T) { c.GetBody() } +func TestCreateUserRequest_GetEmail(tt *testing.T) { + var zeroValue string + c := &CreateUserRequest{Email: &zeroValue} + c.GetEmail() + c = &CreateUserRequest{} + c.GetEmail() + c = nil + c.GetEmail() +} + +func TestCreateUserRequest_GetLogin(tt *testing.T) { + var zeroValue string + c := &CreateUserRequest{Login: &zeroValue} + c.GetLogin() + c = &CreateUserRequest{} + c.GetLogin() + c = nil + c.GetLogin() +} + +func TestCreateUserRequest_GetSuspended(tt *testing.T) { + var zeroValue bool + c := &CreateUserRequest{Suspended: &zeroValue} + c.GetSuspended() + c = &CreateUserRequest{} + c.GetSuspended() + c = nil + c.GetSuspended() +} + func TestCreationInfo_GetCreated(tt *testing.T) { var zeroValue Timestamp c := &CreationInfo{Created: &zeroValue} From db9c8bc1a2a1bed4db5adc4274d684ade41cb75a Mon Sep 17 00:00:00 2001 From: Travis Tomsu Date: Tue, 23 Jan 2024 09:54:40 -0500 Subject: [PATCH 3/3] Review comments --- github/admin_users.go | 7 ++----- github/admin_users_test.go | 12 ++++++------ github/github-accessors.go | 8 -------- github/github-accessors_test.go | 10 ---------- 4 files changed, 8 insertions(+), 29 deletions(-) diff --git a/github/admin_users.go b/github/admin_users.go index 746d5fe225..e134ff34db 100644 --- a/github/admin_users.go +++ b/github/admin_users.go @@ -13,20 +13,17 @@ import ( // CreateUserRequest represents the fields sent to the `CreateUser` endpoint. // Note that `Login` is a required field. type CreateUserRequest struct { - Login *string `json:"login"` + Login string `json:"login"` Email *string `json:"email,omitempty"` Suspended *bool `json:"suspended,omitempty"` } -type CreateUserOptions struct { -} - // CreateUser creates a new user in GitHub Enterprise. // // GitHub API docs: https://docs.github.com/enterprise-server@3.11/rest/enterprise-admin/users#create-a-user // //meta:operation POST /admin/users -func (s *AdminService) CreateUser(ctx context.Context, userReq *CreateUserRequest) (*User, *Response, error) { +func (s *AdminService) CreateUser(ctx context.Context, userReq CreateUserRequest) (*User, *Response, error) { u := "admin/users" req, err := s.client.NewRequest("POST", u, userReq) diff --git a/github/admin_users_test.go b/github/admin_users_test.go index 8c82792f65..d29a7e672f 100644 --- a/github/admin_users_test.go +++ b/github/admin_users_test.go @@ -25,7 +25,7 @@ func TestAdminUsers_Create(t *testing.T) { assertNilError(t, json.NewDecoder(r.Body).Decode(v)) testMethod(t, r, "POST") - want := &CreateUserRequest{Login: String("github"), Email: String("email@domain.com"), Suspended: Bool(false)} + want := &CreateUserRequest{Login: "github", Email: String("email@domain.com"), Suspended: Bool(false)} if !cmp.Equal(v, want) { t.Errorf("Request body = %+v, want %+v", v, want) } @@ -34,8 +34,8 @@ func TestAdminUsers_Create(t *testing.T) { }) ctx := context.Background() - org, _, err := client.Admin.CreateUser(ctx, &CreateUserRequest{ - Login: String("github"), + org, _, err := client.Admin.CreateUser(ctx, CreateUserRequest{ + Login: "github", Email: String("email@domain.com"), Suspended: Bool(false), }) @@ -50,8 +50,8 @@ func TestAdminUsers_Create(t *testing.T) { const methodName = "CreateUser" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Admin.CreateUser(ctx, &CreateUserRequest{ - Login: String("github"), + got, resp, err := client.Admin.CreateUser(ctx, CreateUserRequest{ + Login: "github", Email: String("email@domain.com"), Suspended: Bool(false), }) @@ -188,7 +188,7 @@ func TestCreateUserRequest_Marshal(t *testing.T) { testJSONMarshal(t, &CreateUserRequest{}, "{}") u := &CreateUserRequest{ - Login: String("l"), + Login: "l", Email: String("e"), } diff --git a/github/github-accessors.go b/github/github-accessors.go index 24da99c211..3b1a662966 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -4486,14 +4486,6 @@ func (c *CreateUserRequest) GetEmail() string { return *c.Email } -// GetLogin returns the Login field if it's non-nil, zero value otherwise. -func (c *CreateUserRequest) GetLogin() string { - if c == nil || c.Login == nil { - return "" - } - return *c.Login -} - // GetSuspended returns the Suspended field if it's non-nil, zero value otherwise. func (c *CreateUserRequest) GetSuspended() bool { if c == nil || c.Suspended == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index a75476ea2f..f4c71742ef 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -5272,16 +5272,6 @@ func TestCreateUserRequest_GetEmail(tt *testing.T) { c.GetEmail() } -func TestCreateUserRequest_GetLogin(tt *testing.T) { - var zeroValue string - c := &CreateUserRequest{Login: &zeroValue} - c.GetLogin() - c = &CreateUserRequest{} - c.GetLogin() - c = nil - c.GetLogin() -} - func TestCreateUserRequest_GetSuspended(tt *testing.T) { var zeroValue bool c := &CreateUserRequest{Suspended: &zeroValue}