From e5c21cbe7abef4ce651172eadf3c53e4b43972d9 Mon Sep 17 00:00:00 2001 From: Crinu Dimitriu Date: Thu, 24 Apr 2025 12:58:19 +0300 Subject: [PATCH 1/3] Add support for pagination options in rules API methods Updated `GetRulesForBranch`, `GetAllRulesets`, and `GetAllRepositoryRulesets` methods to accept optional pagination parameters (`ListOptions`). Enhanced test cases to validate the use of these parameters in API requests. --- github/orgs_rules.go | 7 ++++++- github/orgs_rules_test.go | 9 +++++++-- github/repos_rules.go | 27 +++++++++++++++++++++++---- github/repos_rules_test.go | 25 +++++++++++++++++++++---- 4 files changed, 57 insertions(+), 11 deletions(-) diff --git a/github/orgs_rules.go b/github/orgs_rules.go index 8cb2e5d1f92..357eb8ce7cf 100644 --- a/github/orgs_rules.go +++ b/github/orgs_rules.go @@ -15,9 +15,14 @@ import ( // GitHub API docs: https://docs.github.com/rest/orgs/rules#get-all-organization-repository-rulesets // //meta:operation GET /orgs/{org}/rulesets -func (s *OrganizationsService) GetAllRepositoryRulesets(ctx context.Context, org string) ([]*RepositoryRuleset, *Response, error) { +func (s *OrganizationsService) GetAllRepositoryRulesets(ctx context.Context, org string, opts *ListOptions) ([]*RepositoryRuleset, *Response, error) { u := fmt.Sprintf("orgs/%v/rulesets", org) + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } + req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err diff --git a/github/orgs_rules_test.go b/github/orgs_rules_test.go index 37c98d5bcf4..ac1e9e61fb4 100644 --- a/github/orgs_rules_test.go +++ b/github/orgs_rules_test.go @@ -20,6 +20,10 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { mux.HandleFunc("/orgs/o/rulesets", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") + testFormValues(t, r, values{ + "page": "2", + "per_page": "35", + }) fmt.Fprint(w, `[{ "id": 21, "name": "test ruleset", @@ -37,8 +41,9 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { }]`) }) + opts := &ListOptions{Page: 2, PerPage: 35} ctx := context.Background() - rulesets, _, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o") + rulesets, _, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", opts) if err != nil { t.Errorf("Organizations.GetAllRepositoryRulesets returned error: %v", err) } @@ -62,7 +67,7 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { const methodName = "GetAllRepositoryRulesets" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o") + got, resp, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", opts) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } diff --git a/github/repos_rules.go b/github/repos_rules.go index d38e35cdd77..038cefd71a4 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -38,9 +38,14 @@ type rulesetClearBypassActors struct { // GitHub API docs: https://docs.github.com/rest/repos/rules#get-rules-for-a-branch // //meta:operation GET /repos/{owner}/{repo}/rules/branches/{branch} -func (s *RepositoriesService) GetRulesForBranch(ctx context.Context, owner, repo, branch string) (*BranchRules, *Response, error) { +func (s *RepositoriesService) GetRulesForBranch(ctx context.Context, owner, repo, branch string, opts *ListOptions) (*BranchRules, *Response, error) { u := fmt.Sprintf("repos/%v/%v/rules/branches/%v", owner, repo, branch) + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } + req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err @@ -55,14 +60,28 @@ func (s *RepositoriesService) GetRulesForBranch(ctx context.Context, owner, repo return rules, resp, nil } +// RepositoryListRulesetsOptions specifies optional parameters to the +// RepositoriesService.GetAllRulesets method. +type RepositoryListRulesetsOptions struct { + // IncludesParents indicates whether to include rulesets configured at the organization or enterprise level that apply to the repository. + IncludesParents *bool `url:"includes_parents,omitempty"` + ListOptions +} + // GetAllRulesets gets all the repository rulesets for the specified repository. -// If includesParents is true, rulesets configured at the organization or enterprise level that apply to the repository will be returned. +// By default, this endpoint will include rulesets configured at the organization or enterprise level that apply to the repository. +// To exclude those rulesets, set the `RepositoryListRulesetsOptions.IncludesParents` parameter to `false`. // // GitHub API docs: https://docs.github.com/rest/repos/rules#get-all-repository-rulesets // //meta:operation GET /repos/{owner}/{repo}/rulesets -func (s *RepositoriesService) GetAllRulesets(ctx context.Context, owner, repo string, includesParents bool) ([]*RepositoryRuleset, *Response, error) { - u := fmt.Sprintf("repos/%v/%v/rulesets?includes_parents=%v", owner, repo, includesParents) +func (s *RepositoriesService) GetAllRulesets(ctx context.Context, owner, repo string, opts *RepositoryListRulesetsOptions) ([]*RepositoryRuleset, *Response, error) { + u := fmt.Sprintf("repos/%v/%v/rulesets", owner, repo) + + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } req, err := s.client.NewRequest("GET", u, nil) if err != nil { diff --git a/github/repos_rules_test.go b/github/repos_rules_test.go index bb43d532a55..6f28ae08c4a 100644 --- a/github/repos_rules_test.go +++ b/github/repos_rules_test.go @@ -20,6 +20,10 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { mux.HandleFunc("/repos/o/repo/rules/branches/branch", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") + testFormValues(t, r, values{ + "page": "2", + "per_page": "35", + }) fmt.Fprint(w, `[ { "ruleset_id": 42069, @@ -39,8 +43,9 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { ]`) }) + opts := &ListOptions{Page: 2, PerPage: 35} ctx := context.Background() - rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch") + rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", opts) if err != nil { t.Errorf("Repositories.GetRulesForBranch returned error: %v", err) } @@ -57,7 +62,7 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { const methodName = "GetRulesForBranch" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch") + got, resp, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", opts) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } @@ -71,6 +76,11 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { mux.HandleFunc("/repos/o/repo/rulesets", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") + testFormValues(t, r, values{ + "includes_parents": "false", + "page": "2", + "per_page": "35", + }) fmt.Fprintf(w, `[ { "id": 42, @@ -93,8 +103,15 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { ]`, referenceTimeStr) }) + opts := &RepositoryListRulesetsOptions{ + IncludesParents: Ptr(false), + ListOptions: ListOptions{ + Page: 2, + PerPage: 35, + }, + } ctx := context.Background() - ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", false) + ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", opts) if err != nil { t.Errorf("Repositories.GetAllRulesets returned error: %v", err) } @@ -126,7 +143,7 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { const methodName = "GetAllRulesets" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", false) + got, resp, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", opts) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } From 9b0a5b84ee8d4fe0113d21ecbc58372ba65c1e58 Mon Sep 17 00:00:00 2001 From: Crinu Dimitriu Date: Fri, 25 Apr 2025 07:17:28 +0300 Subject: [PATCH 2/3] run generate.sh and fix test coverage --- github/github-accessors.go | 8 ++++++++ github/github-accessors_test.go | 11 +++++++++++ github/orgs_rules_test.go | 4 ++++ github/repos_rules_test.go | 8 ++++++++ 4 files changed, 31 insertions(+) diff --git a/github/github-accessors.go b/github/github-accessors.go index 8530cd914bc..b2f8b5f0f53 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -22782,6 +22782,14 @@ func (r *RepositoryLicense) GetURL() string { return *r.URL } +// GetIncludesParents returns the IncludesParents field if it's non-nil, zero value otherwise. +func (r *RepositoryListRulesetsOptions) GetIncludesParents() bool { + if r == nil || r.IncludesParents == nil { + return false + } + return *r.IncludesParents +} + // GetBase returns the Base field if it's non-nil, zero value otherwise. func (r *RepositoryMergeRequest) GetBase() string { if r == nil || r.Base == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 8b85e063f4d..a6a75ae0f10 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -29383,6 +29383,17 @@ func TestRepositoryLicense_GetURL(tt *testing.T) { r.GetURL() } +func TestRepositoryListRulesetsOptions_GetIncludesParents(tt *testing.T) { + tt.Parallel() + var zeroValue bool + r := &RepositoryListRulesetsOptions{IncludesParents: &zeroValue} + r.GetIncludesParents() + r = &RepositoryListRulesetsOptions{} + r.GetIncludesParents() + r = nil + r.GetIncludesParents() +} + func TestRepositoryMergeRequest_GetBase(tt *testing.T) { tt.Parallel() var zeroValue string diff --git a/github/orgs_rules_test.go b/github/orgs_rules_test.go index ac1e9e61fb4..5323549e9a1 100644 --- a/github/orgs_rules_test.go +++ b/github/orgs_rules_test.go @@ -65,6 +65,10 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { } const methodName = "GetAllRepositoryRulesets" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Organizations.GetAllRepositoryRulesets(ctx, "\n", opts) + return err + }) testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { got, resp, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", opts) diff --git a/github/repos_rules_test.go b/github/repos_rules_test.go index 6f28ae08c4a..04890d513f6 100644 --- a/github/repos_rules_test.go +++ b/github/repos_rules_test.go @@ -60,6 +60,10 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { } const methodName = "GetRulesForBranch" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.GetRulesForBranch(ctx, "\n", "\n", "\n", opts) + return err + }) testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { got, resp, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", opts) @@ -141,6 +145,10 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { } const methodName = "GetAllRulesets" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.GetAllRulesets(ctx, "\n", "\n", opts) + return err + }) testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { got, resp, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", opts) From 3d1050a1040e0df18974698f60aadd20a0a05a2f Mon Sep 17 00:00:00 2001 From: Crinu Dimitriu Date: Fri, 25 Apr 2025 19:48:04 +0300 Subject: [PATCH 3/3] separate tests with options --- github/orgs_rules_test.go | 46 +++++++++++++-- github/repos_rules_test.go | 114 ++++++++++++++++++++++++++++++------- 2 files changed, 135 insertions(+), 25 deletions(-) diff --git a/github/orgs_rules_test.go b/github/orgs_rules_test.go index 5323549e9a1..b21ff93b151 100644 --- a/github/orgs_rules_test.go +++ b/github/orgs_rules_test.go @@ -20,10 +20,6 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { mux.HandleFunc("/orgs/o/rulesets", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testFormValues(t, r, values{ - "page": "2", - "per_page": "35", - }) fmt.Fprint(w, `[{ "id": 21, "name": "test ruleset", @@ -41,9 +37,8 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { }]`) }) - opts := &ListOptions{Page: 2, PerPage: 35} ctx := context.Background() - rulesets, _, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", opts) + rulesets, _, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", nil) if err != nil { t.Errorf("Organizations.GetAllRepositoryRulesets returned error: %v", err) } @@ -64,6 +59,45 @@ func TestOrganizationsService_GetAllRepositoryRulesets(t *testing.T) { t.Errorf("Organizations.GetAllRepositoryRulesets returned %+v, want %+v", rulesets, want) } + const methodName = "GetAllRepositoryRulesets" + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", nil) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestOrganizationsService_GetAllRepositoryRulesets_ListOptions(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/orgs/o/rulesets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testFormValues(t, r, values{ + "page": "2", + "per_page": "35", + }) + fmt.Fprint(w, `[{ + "id": 21 + }]`) + }) + + opts := &ListOptions{Page: 2, PerPage: 35} + ctx := context.Background() + rulesets, _, err := client.Organizations.GetAllRepositoryRulesets(ctx, "o", opts) + if err != nil { + t.Errorf("Organizations.GetAllRepositoryRulesets returned error: %v", err) + } + + want := []*RepositoryRuleset{{ + ID: Ptr(int64(21)), + }} + if !cmp.Equal(rulesets, want) { + t.Errorf("Organizations.GetAllRepositoryRulesets returned %+v, want %+v", rulesets, want) + } + const methodName = "GetAllRepositoryRulesets" testBadOptions(t, methodName, func() (err error) { _, _, err = client.Organizations.GetAllRepositoryRulesets(ctx, "\n", opts) diff --git a/github/repos_rules_test.go b/github/repos_rules_test.go index 04890d513f6..24beb654199 100644 --- a/github/repos_rules_test.go +++ b/github/repos_rules_test.go @@ -20,10 +20,6 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { mux.HandleFunc("/repos/o/repo/rules/branches/branch", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testFormValues(t, r, values{ - "page": "2", - "per_page": "35", - }) fmt.Fprint(w, `[ { "ruleset_id": 42069, @@ -43,9 +39,8 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { ]`) }) - opts := &ListOptions{Page: 2, PerPage: 35} ctx := context.Background() - rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", opts) + rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", nil) if err != nil { t.Errorf("Repositories.GetRulesForBranch returned error: %v", err) } @@ -59,6 +54,49 @@ func TestRepositoriesService_GetRulesForBranch(t *testing.T) { t.Errorf("Repositories.GetRulesForBranch returned %+v, want %+v", rules, want) } + const methodName = "GetRulesForBranch" + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", nil) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestRepositoriesService_GetRulesForBranch_ListOptions(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/repos/o/repo/rules/branches/branch", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testFormValues(t, r, values{ + "page": "2", + "per_page": "35", + }) + fmt.Fprint(w, `[ + { + "ruleset_id": 42069, + "type": "creation" + } + ]`) + }) + + opts := &ListOptions{Page: 2, PerPage: 35} + ctx := context.Background() + rules, _, err := client.Repositories.GetRulesForBranch(ctx, "o", "repo", "branch", opts) + if err != nil { + t.Errorf("Repositories.GetRulesForBranch returned error: %v", err) + } + + want := &BranchRules{ + Creation: []*BranchRuleMetadata{{RulesetID: 42069}}, + } + + if !cmp.Equal(rules, want) { + t.Errorf("Repositories.GetRulesForBranch returned %+v, want %+v", rules, want) + } + const methodName = "GetRulesForBranch" testBadOptions(t, methodName, func() (err error) { _, _, err = client.Repositories.GetRulesForBranch(ctx, "\n", "\n", "\n", opts) @@ -80,11 +118,6 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { mux.HandleFunc("/repos/o/repo/rulesets", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testFormValues(t, r, values{ - "includes_parents": "false", - "page": "2", - "per_page": "35", - }) fmt.Fprintf(w, `[ { "id": 42, @@ -107,15 +140,8 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { ]`, referenceTimeStr) }) - opts := &RepositoryListRulesetsOptions{ - IncludesParents: Ptr(false), - ListOptions: ListOptions{ - Page: 2, - PerPage: 35, - }, - } ctx := context.Background() - ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", opts) + ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", nil) if err != nil { t.Errorf("Repositories.GetAllRulesets returned error: %v", err) } @@ -144,6 +170,56 @@ func TestRepositoriesService_GetAllRulesets(t *testing.T) { t.Errorf("Repositories.GetAllRulesets returned %+v, want %+v", ruleSet, want) } + const methodName = "GetAllRulesets" + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", nil) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + +func TestRepositoriesService_GetAllRulesets_ListOptions(t *testing.T) { + t.Parallel() + client, mux, _ := setup(t) + + mux.HandleFunc("/repos/o/repo/rulesets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "GET") + testFormValues(t, r, values{ + "includes_parents": "false", + "page": "2", + "per_page": "35", + }) + fmt.Fprint(w, `[ + { + "id": 42 + } + ]`) + }) + + opts := &RepositoryListRulesetsOptions{ + IncludesParents: Ptr(false), + ListOptions: ListOptions{ + Page: 2, + PerPage: 35, + }, + } + ctx := context.Background() + ruleSet, _, err := client.Repositories.GetAllRulesets(ctx, "o", "repo", opts) + if err != nil { + t.Errorf("Repositories.GetAllRulesets returned error: %v", err) + } + + want := []*RepositoryRuleset{ + { + ID: Ptr(int64(42)), + }, + } + if !cmp.Equal(ruleSet, want) { + t.Errorf("Repositories.GetAllRulesets returned %+v, want %+v", ruleSet, want) + } + const methodName = "GetAllRulesets" testBadOptions(t, methodName, func() (err error) { _, _, err = client.Repositories.GetAllRulesets(ctx, "\n", "\n", opts)