diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index d94f7a1d467..aff7a5fe188 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "slices" "strings" "time" @@ -35,6 +36,11 @@ const ( allowSquashMerge = "Allow Squash Merging" allowRebaseMerge = "Allow Rebase Merging" + squashMsgDefault = "default" + squashMsgPRTitle = "pr-title" + squashMsgPRTitleCommits = "pr-title-commits" + squashMsgPRTitleDescription = "pr-title-description" + optionAllowForking = "Allow Forking" optionDefaultBranchName = "Default Branch Name" optionDescription = "Description" @@ -42,11 +48,13 @@ const ( optionIssues = "Issues" optionMergeOptions = "Merge Options" optionProjects = "Projects" - optionDiscussions = "Discussions" optionTemplateRepo = "Template Repository" optionTopics = "Topics" optionVisibility = "Visibility" optionWikis = "Wikis" + + // TODO: GitHub Enterprise Server does not support has_discussions yet + // optionDiscussions = "Discussions" ) type EditOptions struct { @@ -69,24 +77,27 @@ type EditRepositoryInput struct { enableAdvancedSecurity *bool enableSecretScanning *bool enableSecretScanningPushProtection *bool - - AllowForking *bool `json:"allow_forking,omitempty"` - AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"` - DefaultBranch *string `json:"default_branch,omitempty"` - DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"` - Description *string `json:"description,omitempty"` - EnableAutoMerge *bool `json:"allow_auto_merge,omitempty"` - EnableIssues *bool `json:"has_issues,omitempty"` - EnableMergeCommit *bool `json:"allow_merge_commit,omitempty"` - EnableProjects *bool `json:"has_projects,omitempty"` - EnableDiscussions *bool `json:"has_discussions,omitempty"` - EnableRebaseMerge *bool `json:"allow_rebase_merge,omitempty"` - EnableSquashMerge *bool `json:"allow_squash_merge,omitempty"` - EnableWiki *bool `json:"has_wiki,omitempty"` - Homepage *string `json:"homepage,omitempty"` - IsTemplate *bool `json:"is_template,omitempty"` - SecurityAndAnalysis *SecurityAndAnalysisInput `json:"security_and_analysis,omitempty"` - Visibility *string `json:"visibility,omitempty"` + squashMergeCommitMsg *string + + AllowForking *bool `json:"allow_forking,omitempty"` + AllowUpdateBranch *bool `json:"allow_update_branch,omitempty"` + DefaultBranch *string `json:"default_branch,omitempty"` + DeleteBranchOnMerge *bool `json:"delete_branch_on_merge,omitempty"` + Description *string `json:"description,omitempty"` + EnableAutoMerge *bool `json:"allow_auto_merge,omitempty"` + EnableIssues *bool `json:"has_issues,omitempty"` + EnableMergeCommit *bool `json:"allow_merge_commit,omitempty"` + EnableProjects *bool `json:"has_projects,omitempty"` + EnableDiscussions *bool `json:"has_discussions,omitempty"` + EnableRebaseMerge *bool `json:"allow_rebase_merge,omitempty"` + EnableSquashMerge *bool `json:"allow_squash_merge,omitempty"` + EnableWiki *bool `json:"has_wiki,omitempty"` + Homepage *string `json:"homepage,omitempty"` + IsTemplate *bool `json:"is_template,omitempty"` + SecurityAndAnalysis *SecurityAndAnalysisInput `json:"security_and_analysis,omitempty"` + SquashMergeCommitTitle *string `json:"squash_merge_commit_title,omitempty"` + SquashMergeCommitMessage *string `json:"squash_merge_commit_message,omitempty"` + Visibility *string `json:"visibility,omitempty"` } func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobra.Command { @@ -120,7 +131,15 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr When the %[1]s--visibility%[1]s flag is used, %[1]s--accept-visibility-change-consequences%[1]s flag is required. For information on all the potential consequences, see . - `, "`"), + + When the %[1]s--enable-squash-merge%[1]s flag is used, %[1]s--squash-merge-commit-message%[1]s + can be used to change the default squash merge commit message behavior: + + - %[1]s%[2]s%[1]s: uses commit title and message for 1 commit, or pull request title and list of commits for 2 or more + - %[1]s%[3]s%[1]s: uses pull request title + - %[1]s%[4]s%[1]s: uses pull request title and list of commits + - %[1]s%[5]s%[1]s: uses pull request title and description + `, "`", squashMsgDefault, squashMsgPRTitle, squashMsgPRTitleCommits, squashMsgPRTitleDescription), Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` # Enable issues and wiki @@ -162,6 +181,19 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr return cmdutil.FlagErrorf("use of --visibility flag requires --accept-visibility-change-consequences flag") } + if opts.Edits.squashMergeCommitMsg != nil { + if opts.Edits.EnableSquashMerge == nil { + return cmdutil.FlagErrorf("--squash-merge-commit-message requires --enable-squash-merge") + } + if !*opts.Edits.EnableSquashMerge { + return cmdutil.FlagErrorf("--squash-merge-commit-message cannot be used when --enable-squash-merge=false") + } + if err := validateSquashMergeCommitMsg(*opts.Edits.squashMergeCommitMsg); err != nil { + return err + } + transformSquashMergeOpts(&opts.Edits) + } + if hasSecurityEdits(opts.Edits) { opts.Edits.SecurityAndAnalysis = transformSecurityAndAnalysisOpts(opts) } @@ -192,6 +224,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr cmdutil.NilBoolFlag(cmd, &opts.Edits.DeleteBranchOnMerge, "delete-branch-on-merge", "", "Delete head branch when pull requests are merged") cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowForking, "allow-forking", "", "Allow forking of an organization repository") cmdutil.NilBoolFlag(cmd, &opts.Edits.AllowUpdateBranch, "allow-update-branch", "", "Allow a pull request head branch that is behind its base branch to be updated") + cmdutil.NilStringFlag(cmd, &opts.Edits.squashMergeCommitMsg, "squash-merge-commit-message", "", "The default value for a squash merge commit message: {default|pr-title|pr-title-commits|pr-title-description}") cmd.Flags().StringSliceVar(&opts.AddTopics, "add-topic", nil, "Add repository topic") cmd.Flags().StringSliceVar(&opts.RemoveTopics, "remove-topic", nil, "Remove repository topic") cmd.Flags().BoolVar(&opts.AcceptVisibilityChangeConsequences, "accept-visibility-change-consequences", false, "Accept the consequences of changing the repository visibility") @@ -474,6 +507,20 @@ func interactiveRepoEdit(opts *EditOptions, r *api.Repository) error { return fmt.Errorf("you need to allow at least one merge strategy") } + if enableSquashMerge { + squashMsgOptions := validSquashMsgValues + idx, err := p.Select( + "Default squash merge commit message", + squashMsgDefault, + squashMsgOptions) + if err != nil { + return err + } + selected := squashMsgOptions[idx] + opts.Edits.squashMergeCommitMsg = &selected + transformSquashMergeOpts(&opts.Edits) + } + opts.Edits.EnableAutoMerge = &r.AutoMergeAllowed c, err := p.Confirm("Enable Auto Merge?", r.AutoMergeAllowed) if err != nil { @@ -634,3 +681,39 @@ func transformSecurityAndAnalysisOpts(opts *EditOptions) *SecurityAndAnalysisInp } return securityOptions } + +var validSquashMsgValues = []string{squashMsgDefault, squashMsgPRTitle, squashMsgPRTitleCommits, squashMsgPRTitleDescription} + +func validateSquashMergeCommitMsg(value string) error { + if slices.Contains(validSquashMsgValues, value) { + return nil + } + return cmdutil.FlagErrorf("invalid value for --squash-merge-commit-message: %q. Valid values are: %s", value, strings.Join(validSquashMsgValues, ", ")) +} + +// transformSquashMergeOpts maps the user-facing squash merge commit message option +// to the two API fields: squash_merge_commit_title and squash_merge_commit_message. +func transformSquashMergeOpts(edits *EditRepositoryInput) { + if edits.squashMergeCommitMsg == nil { + return + } + var title, message string + switch *edits.squashMergeCommitMsg { + case squashMsgDefault: + title = "COMMIT_OR_PR_TITLE" + message = "COMMIT_MESSAGES" + case squashMsgPRTitle: + title = "PR_TITLE" + message = "BLANK" + case squashMsgPRTitleCommits: + title = "PR_TITLE" + message = "COMMIT_MESSAGES" + case squashMsgPRTitleDescription: + title = "PR_TITLE" + message = "PR_BODY" + default: + return + } + edits.SquashMergeCommitTitle = &title + edits.SquashMergeCommitMessage = &message +} diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 868e300facd..d4b297a1902 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -91,6 +91,34 @@ func TestNewCmdEdit(t *testing.T) { }, }, }, + { + name: "squash merge commit message with enable-squash-merge", + args: "--enable-squash-merge --squash-merge-commit-message pr-title", + wantOpts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + Edits: EditRepositoryInput{ + squashMergeCommitMsg: sp("pr-title"), + EnableSquashMerge: bp(true), + SquashMergeCommitTitle: sp("PR_TITLE"), + SquashMergeCommitMessage: sp("BLANK"), + }, + }, + }, + { + name: "squash merge commit message without enable-squash-merge", + args: "--squash-merge-commit-message default", + wantErr: "--squash-merge-commit-message requires --enable-squash-merge", + }, + { + name: "squash merge commit message with invalid value", + args: "--enable-squash-merge --squash-merge-commit-message blah", + wantErr: `invalid value for --squash-merge-commit-message: "blah". Valid values are: default, pr-title, pr-title-commits, pr-title-description`, + }, + { + name: "squash merge commit message with enable-squash-merge=false", + args: "--enable-squash-merge=false --squash-merge-commit-message default", + wantErr: "--squash-merge-commit-message cannot be used when --enable-squash-merge=false", + }, } for _, tt := range tests { @@ -235,6 +263,26 @@ func Test_editRun(t *testing.T) { })) }, }, + { + name: "set squash merge commit message to pr-title-description", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + Edits: EditRepositoryInput{ + EnableSquashMerge: bp(true), + SquashMergeCommitTitle: sp("PR_TITLE"), + SquashMergeCommitMessage: sp("PR_BODY"), + }, + }, + httpStubs: func(t *testing.T, r *httpmock.Registry) { + r.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) { + assert.Equal(t, true, payload["allow_squash_merge"]) + assert.Equal(t, "PR_TITLE", payload["squash_merge_commit_title"]) + assert.Equal(t, "PR_BODY", payload["squash_merge_commit_message"]) + })) + }, + }, { name: "does not have sufficient permissions for security edits", opts: EditOptions{ @@ -633,7 +681,7 @@ func Test_editRun_interactive(t *testing.T) { }, }, { - name: "updates repo merge options", + name: "updates repo merge options without squash", opts: EditOptions{ Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), InteractiveMode: true, @@ -691,6 +739,72 @@ func Test_editRun_interactive(t *testing.T) { })) }, }, + { + name: "updates repo merge options with squash and commit message", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + InteractiveMode: true, + }, + promptStubs: func(pm *prompter.MockPrompter) { + pm.RegisterMultiSelect("What do you want to edit?", nil, editList, + func(_ string, _, opts []string) ([]int, error) { + return []int{4}, nil + }) + pm.RegisterMultiSelect("Allowed merge strategies", nil, + []string{allowMergeCommits, allowSquashMerge, allowRebaseMerge}, + func(_ string, _, opts []string) ([]int, error) { + return []int{1}, nil + }) + pm.RegisterSelect("Default squash merge commit message", + []string{"default", "pr-title", "pr-title-commits", "pr-title-description"}, + func(_, _ string, opts []string) (int, error) { + return prompter.IndexFor(opts, "pr-title-description") + }) + pm.RegisterConfirm("Enable Auto Merge?", func(_ string, _ bool) (bool, error) { + return false, nil + }) + pm.RegisterConfirm("Automatically delete head branches after merging?", func(_ string, _ bool) (bool, error) { + return false, nil + }) + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { + "data": { + "repository": { + "description": "old description", + "homePageUrl": "https://url.com", + "defaultBranchRef": { + "name": "main" + }, + "isInOrganization": false, + "squashMergeAllowed": false, + "rebaseMergeAllowed": false, + "mergeCommitAllowed": true, + "deleteBranchOnMerge": false, + "repositoryTopics": { + "nodes": [{ + "topic": { + "name": "x" + } + }] + } + } + } + }`)) + reg.Register( + httpmock.REST("PATCH", "repos/OWNER/REPO"), + httpmock.RESTPayload(200, `{}`, func(payload map[string]interface{}) { + assert.Equal(t, false, payload["allow_merge_commit"]) + assert.Equal(t, true, payload["allow_squash_merge"]) + assert.Equal(t, false, payload["allow_rebase_merge"]) + assert.Equal(t, "PR_TITLE", payload["squash_merge_commit_title"]) + assert.Equal(t, "PR_BODY", payload["squash_merge_commit_message"]) + })) + }, + }, } for _, tt := range tests { @@ -818,6 +932,69 @@ func Test_transformSecurityAndAnalysisOpts(t *testing.T) { } } +func Test_transformSquashMergeOpts(t *testing.T) { + tests := []struct { + name string + input string + wantTitle string + wantMessage string + }{ + { + name: "default", + input: "default", + wantTitle: "COMMIT_OR_PR_TITLE", + wantMessage: "COMMIT_MESSAGES", + }, + { + name: "pr-title", + input: "pr-title", + wantTitle: "PR_TITLE", + wantMessage: "BLANK", + }, + { + name: "pr-title-commits", + input: "pr-title-commits", + wantTitle: "PR_TITLE", + wantMessage: "COMMIT_MESSAGES", + }, + { + name: "pr-title-description", + input: "pr-title-description", + wantTitle: "PR_TITLE", + wantMessage: "PR_BODY", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + edits := &EditRepositoryInput{ + squashMergeCommitMsg: sp(tt.input), + } + transformSquashMergeOpts(edits) + assert.Equal(t, tt.wantTitle, *edits.SquashMergeCommitTitle) + assert.Equal(t, tt.wantMessage, *edits.SquashMergeCommitMessage) + }) + } +} + +func Test_transformSquashMergeOpts_unknownInput(t *testing.T) { + edits := &EditRepositoryInput{ + squashMergeCommitMsg: sp("unknown-value"), + } + transformSquashMergeOpts(edits) + assert.Nil(t, edits.SquashMergeCommitTitle) + assert.Nil(t, edits.SquashMergeCommitMessage) +} + +func Test_validateSquashMergeCommitMsg(t *testing.T) { + assert.NoError(t, validateSquashMergeCommitMsg("default")) + assert.NoError(t, validateSquashMergeCommitMsg("pr-title")) + assert.NoError(t, validateSquashMergeCommitMsg("pr-title-commits")) + assert.NoError(t, validateSquashMergeCommitMsg("pr-title-description")) + assert.Error(t, validateSquashMergeCommitMsg("blah")) + assert.Error(t, validateSquashMergeCommitMsg("")) +} + func sp(v string) *string { return &v }