From 78d5ca8c811bd303eafe205b61bbc4ee5d9adc5b Mon Sep 17 00:00:00 2001 From: Yurii Soldak Date: Mon, 3 Jul 2023 12:25:32 +0200 Subject: [PATCH 1/2] Support repository_id in org ruleset conditions --- github/github-accessors.go | 12 +- github/github-accessors_test.go | 13 +- github/orgs_rules_test.go | 322 +++++++++++++++++++++++++++++++- github/repos_rules.go | 15 +- 4 files changed, 347 insertions(+), 15 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 41cd582c1a1..0333f604ba6 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -18942,8 +18942,16 @@ func (r *RulesetConditions) GetRefName() *RulesetRefConditionParameters { return r.RefName } +// GetRepositoryId returns the RepositoryId field. +func (r *RulesetConditions) GetRepositoryId() *RulesetRepositoryIdsConditionParameters { + if r == nil { + return nil + } + return r.RepositoryId +} + // GetRepositoryName returns the RepositoryName field. -func (r *RulesetConditions) GetRepositoryName() *RulesetRepositoryConditionParameters { +func (r *RulesetConditions) GetRepositoryName() *RulesetRepositoryNamesConditionParameters { if r == nil { return nil } @@ -18967,7 +18975,7 @@ func (r *RulesetLinks) GetSelf() *RulesetLink { } // GetProtected returns the Protected field if it's non-nil, zero value otherwise. -func (r *RulesetRepositoryConditionParameters) GetProtected() bool { +func (r *RulesetRepositoryNamesConditionParameters) GetProtected() bool { if r == nil || r.Protected == nil { return false } diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 993a12484b9..60c8610bdee 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -22094,6 +22094,13 @@ func TestRulesetConditions_GetRefName(tt *testing.T) { r.GetRefName() } +func TestRulesetConditions_GetRepositoryId(tt *testing.T) { + r := &RulesetConditions{} + r.GetRepositoryId() + r = nil + r.GetRepositoryId() +} + func TestRulesetConditions_GetRepositoryName(tt *testing.T) { r := &RulesetConditions{} r.GetRepositoryName() @@ -22118,11 +22125,11 @@ func TestRulesetLinks_GetSelf(tt *testing.T) { r.GetSelf() } -func TestRulesetRepositoryConditionParameters_GetProtected(tt *testing.T) { +func TestRulesetRepositoryNamesConditionParameters_GetProtected(tt *testing.T) { var zeroValue bool - r := &RulesetRepositoryConditionParameters{Protected: &zeroValue} + r := &RulesetRepositoryNamesConditionParameters{Protected: &zeroValue} r.GetProtected() - r = &RulesetRepositoryConditionParameters{} + r = &RulesetRepositoryNamesConditionParameters{} r.GetProtected() r = nil r.GetProtected() diff --git a/github/orgs_rules_test.go b/github/orgs_rules_test.go index 7719e142fb5..f7c0f262f84 100644 --- a/github/orgs_rules_test.go +++ b/github/orgs_rules_test.go @@ -71,7 +71,7 @@ func TestOrganizationsService_GetAllOrganizationRulesets(t *testing.T) { }) } -func TestOrganizationsService_CreateOrganizationRuleset(t *testing.T) { +func TestOrganizationsService_CreateOrganizationRulesetRepoNames(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -227,7 +227,7 @@ func TestOrganizationsService_CreateOrganizationRuleset(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryName: &RulesetRepositoryConditionParameters{ + RepositoryName: &RulesetRepositoryNamesConditionParameters{ Include: []string{"important_repository", "another_important_repository"}, Exclude: []string{"unimportant_repository"}, Protected: Bool(true), @@ -313,7 +313,7 @@ func TestOrganizationsService_CreateOrganizationRuleset(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryName: &RulesetRepositoryConditionParameters{ + RepositoryName: &RulesetRepositoryNamesConditionParameters{ Include: []string{"important_repository", "another_important_repository"}, Exclude: []string{"unimportant_repository"}, Protected: Bool(true), @@ -392,6 +392,316 @@ func TestOrganizationsService_CreateOrganizationRuleset(t *testing.T) { }) } +func TestOrganizationsService_CreateOrganizationRulesetRepoIds(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + mux.HandleFunc("/orgs/o/rulesets", func(w http.ResponseWriter, r *http.Request) { + testMethod(t, r, "POST") + fmt.Fprint(w, `{ + "id": 21, + "name": "ruleset", + "target": "branch", + "source_type": "Organization", + "source": "o", + "enforcement": "active", + "bypass_actors": [ + { + "actor_id": 234, + "actor_type": "Team" + } + ], + "conditions": { + "ref_name": { + "include": [ + "refs/heads/main", + "refs/heads/master" + ], + "exclude": [ + "refs/heads/dev*" + ] + }, + "repository_id": { + "repository_ids": [ 123, 456 ] + } + }, + "rules": [ + { + "type": "creation" + }, + { + "type": "update", + "parameters": { + "update_allows_fetch_and_merge": true + } + }, + { + "type": "deletion" + }, + { + "type": "required_linear_history" + }, + { + "type": "required_deployments", + "parameters": { + "required_deployment_environments": ["test"] + } + }, + { + "type": "required_signatures" + }, + { + "type": "pull_request", + "parameters": { + "dismiss_stale_reviews_on_push": true, + "require_code_owner_review": true, + "require_last_push_approval": true, + "required_approving_review_count": 1, + "required_review_thread_resolution": true + } + }, + { + "type": "required_status_checks", + "parameters": { + "required_status_checks": [ + { + "context": "test", + "integration_id": 1 + } + ], + "strict_required_status_checks_policy": true + } + }, + { + "type": "non_fast_forward" + }, + { + "type": "commit_message_pattern", + "parameters": { + "name": "avoid test commits", + "negate": true, + "operator": "starts_with", + "pattern": "[test]" + } + }, + { + "type": "commit_author_email_pattern", + "parameters": { + "operator": "contains", + "pattern": "github" + } + }, + { + "type": "committer_email_pattern", + "parameters": { + "name": "avoid commit emails", + "negate": true, + "operator": "ends_with", + "pattern": "abc" + } + }, + { + "type": "branch_name_pattern", + "parameters": { + "name": "avoid branch names", + "negate": true, + "operator": "regex", + "pattern": "github$" + } + }, + { + "type": "tag_name_pattern", + "parameters": { + "name": "avoid tag names", + "negate": true, + "operator": "contains", + "pattern": "github" + } + } + ] + }`) + }) + + ctx := context.Background() + ruleset, _, err := client.Organizations.CreateOrganizationRuleset(ctx, "o", &Ruleset{ + ID: 21, + Name: "ruleset", + Target: String("branch"), + SourceType: String("Organization"), + Source: "o", + Enforcement: "active", + BypassActors: []*BypassActor{ + { + ActorID: Int64(234), + ActorType: String("Team"), + }, + }, + Conditions: &RulesetConditions{ + RefName: &RulesetRefConditionParameters{ + Include: []string{"refs/heads/main", "refs/heads/master"}, + Exclude: []string{"refs/heads/dev*"}, + }, + RepositoryId: &RulesetRepositoryIdsConditionParameters{ + RepositoryIds: []int64{123, 456}, + }, + }, + Rules: []*RepositoryRule{ + NewCreationRule(), + NewUpdateRule(&UpdateAllowsFetchAndMergeRuleParameters{ + UpdateAllowsFetchAndMerge: true, + }), + NewDeletionRule(), + NewRequiredLinearHistoryRule(), + NewRequiredDeploymentsRule(&RequiredDeploymentEnvironmentsRuleParameters{ + RequiredDeploymentEnvironments: []string{"test"}, + }), + NewRequiredSignaturesRule(), + NewPullRequestRule(&PullRequestRuleParameters{ + RequireCodeOwnerReview: true, + RequireLastPushApproval: true, + RequiredApprovingReviewCount: 1, + RequiredReviewThreadResolution: true, + DismissStaleReviewsOnPush: true, + }), + NewRequiredStatusChecksRule(&RequiredStatusChecksRuleParameters{ + RequiredStatusChecks: []RuleRequiredStatusChecks{ + { + Context: "test", + IntegrationID: Int64(1), + }, + }, + StrictRequiredStatusChecksPolicy: true, + }), + NewNonFastForwardRule(), + NewCommitMessagePatternRule(&RulePatternParameters{ + Name: String("avoid test commits"), + Negate: Bool(true), + Operator: "starts_with", + Pattern: "[test]", + }), + NewCommitAuthorEmailPatternRule(&RulePatternParameters{ + Operator: "contains", + Pattern: "github", + }), + NewCommitterEmailPatternRule(&RulePatternParameters{ + Name: String("avoid commit emails"), + Negate: Bool(true), + Operator: "ends_with", + Pattern: "abc", + }), + NewBranchNamePatternRule(&RulePatternParameters{ + Name: String("avoid branch names"), + Negate: Bool(true), + Operator: "regex", + Pattern: "github$", + }), + NewTagNamePatternRule(&RulePatternParameters{ + Name: String("avoid tag names"), + Negate: Bool(true), + Operator: "contains", + Pattern: "github", + }), + }, + }) + if err != nil { + t.Errorf("Organizations.CreateOrganizationRuleset returned error: %v", err) + } + + want := &Ruleset{ + ID: 21, + Name: "ruleset", + Target: String("branch"), + SourceType: String("Organization"), + Source: "o", + Enforcement: "active", + BypassActors: []*BypassActor{ + { + ActorID: Int64(234), + ActorType: String("Team"), + }, + }, + Conditions: &RulesetConditions{ + RefName: &RulesetRefConditionParameters{ + Include: []string{"refs/heads/main", "refs/heads/master"}, + Exclude: []string{"refs/heads/dev*"}, + }, + RepositoryId: &RulesetRepositoryIdsConditionParameters{ + RepositoryIds: []int64{123, 456}, + }, + }, + Rules: []*RepositoryRule{ + NewCreationRule(), + NewUpdateRule(&UpdateAllowsFetchAndMergeRuleParameters{ + UpdateAllowsFetchAndMerge: true, + }), + NewDeletionRule(), + NewRequiredLinearHistoryRule(), + NewRequiredDeploymentsRule(&RequiredDeploymentEnvironmentsRuleParameters{ + RequiredDeploymentEnvironments: []string{"test"}, + }), + NewRequiredSignaturesRule(), + NewPullRequestRule(&PullRequestRuleParameters{ + RequireCodeOwnerReview: true, + RequireLastPushApproval: true, + RequiredApprovingReviewCount: 1, + RequiredReviewThreadResolution: true, + DismissStaleReviewsOnPush: true, + }), + NewRequiredStatusChecksRule(&RequiredStatusChecksRuleParameters{ + RequiredStatusChecks: []RuleRequiredStatusChecks{ + { + Context: "test", + IntegrationID: Int64(1), + }, + }, + StrictRequiredStatusChecksPolicy: true, + }), + NewNonFastForwardRule(), + NewCommitMessagePatternRule(&RulePatternParameters{ + Name: String("avoid test commits"), + Negate: Bool(true), + Operator: "starts_with", + Pattern: "[test]", + }), + NewCommitAuthorEmailPatternRule(&RulePatternParameters{ + Operator: "contains", + Pattern: "github", + }), + NewCommitterEmailPatternRule(&RulePatternParameters{ + Name: String("avoid commit emails"), + Negate: Bool(true), + Operator: "ends_with", + Pattern: "abc", + }), + NewBranchNamePatternRule(&RulePatternParameters{ + Name: String("avoid branch names"), + Negate: Bool(true), + Operator: "regex", + Pattern: "github$", + }), + NewTagNamePatternRule(&RulePatternParameters{ + Name: String("avoid tag names"), + Negate: Bool(true), + Operator: "contains", + Pattern: "github", + }), + }, + } + if !cmp.Equal(ruleset, want) { + t.Errorf("Organizations.CreateOrganizationRuleset returned %+v, want %+v", ruleset, want) + } + + const methodName = "CreateOrganizationRuleset" + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Organizations.CreateOrganizationRuleset(ctx, "o", nil) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) +} + func TestOrganizationsService_GetOrganizationRuleset(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -464,7 +774,7 @@ func TestOrganizationsService_GetOrganizationRuleset(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryName: &RulesetRepositoryConditionParameters{ + RepositoryName: &RulesetRepositoryNamesConditionParameters{ Include: []string{"important_repository", "another_important_repository"}, Exclude: []string{"unimportant_repository"}, Protected: Bool(true), @@ -549,7 +859,7 @@ func TestOrganizationsService_UpdateOrganizationRuleset(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryName: &RulesetRepositoryConditionParameters{ + RepositoryName: &RulesetRepositoryNamesConditionParameters{ Include: []string{"important_repository", "another_important_repository"}, Exclude: []string{"unimportant_repository"}, Protected: Bool(true), @@ -581,7 +891,7 @@ func TestOrganizationsService_UpdateOrganizationRuleset(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryName: &RulesetRepositoryConditionParameters{ + RepositoryName: &RulesetRepositoryNamesConditionParameters{ Include: []string{"important_repository", "another_important_repository"}, Exclude: []string{"unimportant_repository"}, Protected: Bool(true), diff --git a/github/repos_rules.go b/github/repos_rules.go index 9299d3e7f3d..67d1cf0623c 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -34,17 +34,24 @@ type RulesetRefConditionParameters struct { Exclude []string `json:"exclude"` } -// RulesetRepositoryConditionParameters represents the conditions object for repository_names. -type RulesetRepositoryConditionParameters struct { +// RulesetRepositoryNamesConditionParameters represents the conditions object for repository_names. +type RulesetRepositoryNamesConditionParameters struct { Include []string `json:"include,omitempty"` Exclude []string `json:"exclude,omitempty"` Protected *bool `json:"protected,omitempty"` } +// RulesetRepositoryIdsConditionParameters represents the conditions object for repository_ids. +type RulesetRepositoryIdsConditionParameters struct { + RepositoryIds []int64 `json:"repository_ids,omitempty"` +} + // RulesetCondition represents the conditions object in a ruleset. +// Set either RepositoryName or RepositoryId, not both type RulesetConditions struct { - RefName *RulesetRefConditionParameters `json:"ref_name,omitempty"` - RepositoryName *RulesetRepositoryConditionParameters `json:"repository_name,omitempty"` + RefName *RulesetRefConditionParameters `json:"ref_name,omitempty"` + RepositoryName *RulesetRepositoryNamesConditionParameters `json:"repository_name,omitempty"` + RepositoryId *RulesetRepositoryIdsConditionParameters `json:"repository_id,omitempty"` } // RulePatternParameters represents the rule pattern parameters. From 3881d6f14d1712ea758937fa24009c0c64efdaf9 Mon Sep 17 00:00:00 2001 From: Yurii Soldak Date: Tue, 4 Jul 2023 20:14:42 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- github/github-accessors.go | 6 +++--- github/github-accessors_test.go | 6 +++--- github/orgs_rules_test.go | 12 ++++++------ github/repos_rules.go | 10 +++++----- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index 0333f604ba6..b420d53fe7d 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -18942,12 +18942,12 @@ func (r *RulesetConditions) GetRefName() *RulesetRefConditionParameters { return r.RefName } -// GetRepositoryId returns the RepositoryId field. -func (r *RulesetConditions) GetRepositoryId() *RulesetRepositoryIdsConditionParameters { +// GetRepositoryID returns the RepositoryID field. +func (r *RulesetConditions) GetRepositoryID() *RulesetRepositoryIDsConditionParameters { if r == nil { return nil } - return r.RepositoryId + return r.RepositoryID } // GetRepositoryName returns the RepositoryName field. diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 60c8610bdee..859ac1861e3 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -22094,11 +22094,11 @@ func TestRulesetConditions_GetRefName(tt *testing.T) { r.GetRefName() } -func TestRulesetConditions_GetRepositoryId(tt *testing.T) { +func TestRulesetConditions_GetRepositoryID(tt *testing.T) { r := &RulesetConditions{} - r.GetRepositoryId() + r.GetRepositoryID() r = nil - r.GetRepositoryId() + r.GetRepositoryID() } func TestRulesetConditions_GetRepositoryName(tt *testing.T) { diff --git a/github/orgs_rules_test.go b/github/orgs_rules_test.go index f7c0f262f84..48c6c5e2bd1 100644 --- a/github/orgs_rules_test.go +++ b/github/orgs_rules_test.go @@ -71,7 +71,7 @@ func TestOrganizationsService_GetAllOrganizationRulesets(t *testing.T) { }) } -func TestOrganizationsService_CreateOrganizationRulesetRepoNames(t *testing.T) { +func TestOrganizationsService_CreateOrganizationRuleset_RepoNames(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -392,7 +392,7 @@ func TestOrganizationsService_CreateOrganizationRulesetRepoNames(t *testing.T) { }) } -func TestOrganizationsService_CreateOrganizationRulesetRepoIds(t *testing.T) { +func TestOrganizationsService_CreateOrganizationRuleset_RepoIDs(t *testing.T) { client, mux, _, teardown := setup() defer teardown() @@ -541,8 +541,8 @@ func TestOrganizationsService_CreateOrganizationRulesetRepoIds(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryId: &RulesetRepositoryIdsConditionParameters{ - RepositoryIds: []int64{123, 456}, + RepositoryID: &RulesetRepositoryIDsConditionParameters{ + RepositoryIDs: []int64{123, 456}, }, }, Rules: []*RepositoryRule{ @@ -625,8 +625,8 @@ func TestOrganizationsService_CreateOrganizationRulesetRepoIds(t *testing.T) { Include: []string{"refs/heads/main", "refs/heads/master"}, Exclude: []string{"refs/heads/dev*"}, }, - RepositoryId: &RulesetRepositoryIdsConditionParameters{ - RepositoryIds: []int64{123, 456}, + RepositoryID: &RulesetRepositoryIDsConditionParameters{ + RepositoryIDs: []int64{123, 456}, }, }, Rules: []*RepositoryRule{ diff --git a/github/repos_rules.go b/github/repos_rules.go index 67d1cf0623c..ee9c82e7145 100644 --- a/github/repos_rules.go +++ b/github/repos_rules.go @@ -41,17 +41,17 @@ type RulesetRepositoryNamesConditionParameters struct { Protected *bool `json:"protected,omitempty"` } -// RulesetRepositoryIdsConditionParameters represents the conditions object for repository_ids. -type RulesetRepositoryIdsConditionParameters struct { - RepositoryIds []int64 `json:"repository_ids,omitempty"` +// RulesetRepositoryIDsConditionParameters represents the conditions object for repository_ids. +type RulesetRepositoryIDsConditionParameters struct { + RepositoryIDs []int64 `json:"repository_ids,omitempty"` } // RulesetCondition represents the conditions object in a ruleset. -// Set either RepositoryName or RepositoryId, not both +// Set either RepositoryName or RepositoryID, not both. type RulesetConditions struct { RefName *RulesetRefConditionParameters `json:"ref_name,omitempty"` RepositoryName *RulesetRepositoryNamesConditionParameters `json:"repository_name,omitempty"` - RepositoryId *RulesetRepositoryIdsConditionParameters `json:"repository_id,omitempty"` + RepositoryID *RulesetRepositoryIDsConditionParameters `json:"repository_id,omitempty"` } // RulePatternParameters represents the rule pattern parameters.