diff --git a/.golangci.yml b/.golangci.yml index 12d08ec69f1..36483d72a03 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -417,7 +417,6 @@ linters: - UpdateRuleParameters.UpdateAllowsFetchAndMerge # TODO: Rules - UploadOptions.Label # TODO: Repositories - UploadOptions.Name # TODO: Repositories - - UserListOptions.Since # TODO: Users exclusions: rules: - linters: diff --git a/github/examples_test.go b/github/examples_test.go index d65e4259b7d..989ba8ef86a 100644 --- a/github/examples_test.go +++ b/github/examples_test.go @@ -105,7 +105,7 @@ func ExampleUsersService_ListAll() { if len(users) == 0 { break } - opts.Since = *users[len(users)-1].ID + opts.Since = users[len(users)-1].ID // Process users... } } diff --git a/github/github-accessors.go b/github/github-accessors.go index 078bc7b7b40..27c88d51c39 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -31454,6 +31454,22 @@ func (u *UserLDAPMapping) GetURL() string { return *u.URL } +// GetPerPage returns the PerPage field if it's non-nil, zero value otherwise. +func (u *UserListOptions) GetPerPage() int { + if u == nil || u.PerPage == nil { + return 0 + } + return *u.PerPage +} + +// GetSince returns the Since field if it's non-nil, zero value otherwise. +func (u *UserListOptions) GetSince() int64 { + if u == nil || u.Since == nil { + return 0 + } + return *u.Since +} + // GetCreatedAt returns the CreatedAt field if it's non-nil, zero value otherwise. func (u *UserMigration) GetCreatedAt() string { if u == nil || u.CreatedAt == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 93efd460073..835f310e0b7 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -40617,6 +40617,28 @@ func TestUserLDAPMapping_GetURL(tt *testing.T) { u.GetURL() } +func TestUserListOptions_GetPerPage(tt *testing.T) { + tt.Parallel() + var zeroValue int + u := &UserListOptions{PerPage: &zeroValue} + u.GetPerPage() + u = &UserListOptions{} + u.GetPerPage() + u = nil + u.GetPerPage() +} + +func TestUserListOptions_GetSince(tt *testing.T) { + tt.Parallel() + var zeroValue int64 + u := &UserListOptions{Since: &zeroValue} + u.GetSince() + u = &UserListOptions{} + u.GetSince() + u = nil + u.GetSince() +} + func TestUserMigration_GetCreatedAt(tt *testing.T) { tt.Parallel() var zeroValue string diff --git a/github/github-iterators.go b/github/github-iterators.go index 6183e71f904..691b5ee10ce 100644 --- a/github/github-iterators.go +++ b/github/github-iterators.go @@ -4179,37 +4179,6 @@ func (s *TeamsService) ListUserTeamsIter(ctx context.Context, opts *ListOptions) } } -// ListAllIter returns an iterator that paginates through all results of ListAll. -func (s *UsersService) ListAllIter(ctx context.Context, opts *UserListOptions) iter.Seq2[*User, error] { - return func(yield func(*User, error) bool) { - // Create a copy of opts to avoid mutating the caller's struct - if opts == nil { - opts = &UserListOptions{} - } else { - opts = Ptr(*opts) - } - - for { - items, resp, err := s.ListAll(ctx, opts) - if err != nil { - yield(nil, err) - return - } - - for _, item := range items { - if !yield(item, nil) { - return - } - } - - if resp.NextPage == 0 { - break - } - opts.ListOptions.Page = resp.NextPage - } - } -} - // ListBlockedUsersIter returns an iterator that paginates through all results of ListBlockedUsers. func (s *UsersService) ListBlockedUsersIter(ctx context.Context, opts *ListOptions) iter.Seq2[*User, error] { return func(yield func(*User, error) bool) { diff --git a/github/github-iterators_test.go b/github/github-iterators_test.go index c7735e6e17d..fa5197ccf22 100644 --- a/github/github-iterators_test.go +++ b/github/github-iterators_test.go @@ -9663,78 +9663,6 @@ func TestTeamsService_ListUserTeamsIter(t *testing.T) { } } -func TestUsersService_ListAllIter(t *testing.T) { - t.Parallel() - client, mux, _ := setup(t) - var callNum int - mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - callNum++ - switch callNum { - case 1: - w.Header().Set("Link", `; rel="next"`) - fmt.Fprint(w, `[{},{},{}]`) - case 2: - fmt.Fprint(w, `[{},{},{},{}]`) - case 3: - fmt.Fprint(w, `[{},{}]`) - case 4: - w.WriteHeader(http.StatusNotFound) - case 5: - fmt.Fprint(w, `[{},{}]`) - } - }) - - iter := client.Users.ListAllIter(t.Context(), nil) - var gotItems int - for _, err := range iter { - gotItems++ - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - } - if want := 7; gotItems != want { - t.Errorf("client.Users.ListAllIter call 1 got %v items; want %v", gotItems, want) - } - - opts := &UserListOptions{} - iter = client.Users.ListAllIter(t.Context(), opts) - gotItems = 0 - for _, err := range iter { - gotItems++ - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - } - if want := 2; gotItems != want { - t.Errorf("client.Users.ListAllIter call 2 got %v items; want %v", gotItems, want) - } - - iter = client.Users.ListAllIter(t.Context(), nil) - gotItems = 0 - for _, err := range iter { - gotItems++ - if err == nil { - t.Error("expected error; got nil") - } - } - if gotItems != 1 { - t.Errorf("client.Users.ListAllIter call 3 got %v items; want 1 (an error)", gotItems) - } - - iter = client.Users.ListAllIter(t.Context(), nil) - gotItems = 0 - iter(func(item *User, err error) bool { - gotItems++ - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - return false - }) - if gotItems != 1 { - t.Errorf("client.Users.ListAllIter call 4 got %v items; want 1 (an error)", gotItems) - } -} - func TestUsersService_ListBlockedUsersIter(t *testing.T) { t.Parallel() client, mux, _ := setup(t) diff --git a/github/users.go b/github/users.go index 542e7da7026..d9591da8ae4 100644 --- a/github/users.go +++ b/github/users.go @@ -208,13 +208,9 @@ func (s *UsersService) GetHovercard(ctx context.Context, user string, opts *Hove // UserListOptions specifies optional parameters to the UsersService.ListAll // method. type UserListOptions struct { - // ID of the last user seen - Since int64 `url:"since,omitempty"` - - // Note: Pagination is powered exclusively by the Since parameter, - // ListOptions.Page has no effect. - // ListOptions.PerPage controls an undocumented GitHub API parameter. - ListOptions + // A user ID. Only return users with an ID greater than this ID. + Since *int64 `url:"since,omitempty"` + PerPage *int `url:"per_page,omitempty"` } // ListAll lists all GitHub users. diff --git a/github/users_test.go b/github/users_test.go index a9523b2f30b..23a00bb7e06 100644 --- a/github/users_test.go +++ b/github/users_test.go @@ -326,11 +326,11 @@ func TestUsersService_ListAll(t *testing.T) { mux.HandleFunc("/users", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testFormValues(t, r, values{"since": "1", "page": "2"}) + testFormValues(t, r, values{"since": "1", "per_page": "30"}) fmt.Fprint(w, `[{"id":2}]`) }) - opt := &UserListOptions{1, ListOptions{Page: 2}} + opt := &UserListOptions{Since: Ptr(int64(1)), PerPage: Ptr(30)} ctx := t.Context() users, _, err := client.Users.ListAll(ctx, opt) if err != nil { @@ -498,17 +498,13 @@ func TestUserListOptions_Marshal(t *testing.T) { testJSONMarshal(t, &UserListOptions{}, "{}") u := &UserListOptions{ - Since: int64(1900), - ListOptions: ListOptions{ - Page: int(1), - PerPage: int(10), - }, + Since: Ptr(int64(1900)), + PerPage: Ptr(30), } want := `{ "since" : 1900, - "page": 1, - "perPage": 10 + "perPage": 30 }` testJSONMarshal(t, u, want)