Merge pull request #2224 from divbhasin/1190-limit-merge-methods

#1190 - limit merge methods

api/queries_repo.go

@@ -33,6 +33,10 @@ type Repository struct {
 
 	Parent *Repository
 
+	MergeCommitAllowed bool
+	RebaseMergeAllowed bool
+	SquashMergeAllowed bool
+
 	// pseudo-field that keeps track of host name of this repo
 	hostname string
 }
@@ -108,6 +112,9 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
 			parent {
 				...repo
 			}
+			mergeCommitAllowed
+			rebaseMergeAllowed
+			squashMergeAllowed
 		}
 	}`
 	variables := map[string]interface{}{

pkg/cmd/pr/merge/merge.go

@@ -145,14 +145,26 @@ func mergeRun(opts *MergeOptions) error {
 		mergeMethod := opts.MergeMethod
 
 		if opts.InteractiveMode {
-			mergeMethod, deleteBranch, err = prInteractiveMerge(opts, crossRepoPR)
+			r, err := api.GitHubRepo(apiClient, baseRepo)
+			if err != nil {
+				return err
+			}
+			mergeMethod, err = mergeMethodSurvey(r)
 			if err != nil {
-				if errors.Is(err, cancelError) {
-					fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
-					return cmdutil.SilentError
-				}
 				return err
 			}
+			deleteBranch, err = deleteBranchSurvey(opts, crossRepoPR)
+			if err != nil {
+				return err
+			}
+			confirm, err := confirmSurvey()
+			if err != nil {
+				return err
+			}
+			if !confirm {
+				fmt.Fprintln(opts.IO.ErrOut, "Cancelled.")
+				return cmdutil.SilentError
+			}
 		}
 
 		err = api.PullRequestMerge(apiClient, baseRepo, pr, mergeMethod, opts.Body)
@@ -237,20 +249,43 @@ func mergeRun(opts *MergeOptions) error {
 	return nil
 }
 
-var cancelError = errors.New("cancelError")
+func mergeMethodSurvey(baseRepo *api.Repository) (api.PullRequestMergeMethod, error) {
+	type mergeOption struct {
+		title  string
+		method api.PullRequestMergeMethod
+	}
 
-func prInteractiveMerge(opts *MergeOptions, crossRepoPR bool) (api.PullRequestMergeMethod, bool, error) {
-	mergeMethodQuestion := &survey.Question{
-		Name: "mergeMethod",
-		Prompt: &survey.Select{
-			Message: "What merge method would you like to use?",
-			Options: []string{"Create a merge commit", "Rebase and merge", "Squash and merge"},
-			Default: "Create a merge commit",
-		},
+	var mergeOpts []mergeOption
+	if baseRepo.MergeCommitAllowed {
+		opt := mergeOption{title: "Create a merge commit", method: api.PullRequestMergeMethodMerge}
+		mergeOpts = append(mergeOpts, opt)
+	}
+	if baseRepo.RebaseMergeAllowed {
+		opt := mergeOption{title: "Rebase and merge", method: api.PullRequestMergeMethodRebase}
+		mergeOpts = append(mergeOpts, opt)
+	}
+	if baseRepo.SquashMergeAllowed {
+		opt := mergeOption{title: "Squash and merge", method: api.PullRequestMergeMethodSquash}
+		mergeOpts = append(mergeOpts, opt)
 	}
 
-	qs := []*survey.Question{mergeMethodQuestion}
+	var surveyOpts []string
+	for _, v := range mergeOpts {
+		surveyOpts = append(surveyOpts, v.title)
+	}
+
+	mergeQuestion := &survey.Select{
+		Message: "What merge method would you like to use?",
+		Options: surveyOpts,
+		Default: "Create a merge commit",
+	}
+
+	var result int
+	err := prompt.SurveyAskOne(mergeQuestion, &result)
+	return mergeOpts[result].method, err
+}
 
+func deleteBranchSurvey(opts *MergeOptions, crossRepoPR bool) (bool, error) {
 	if !crossRepoPR && !opts.IsDeleteBranchIndicated {
 		var message string
 		if opts.CanDeleteLocalBranch {
@@ -259,49 +294,24 @@ func prInteractiveMerge(opts *MergeOptions, crossRepoPR bool) (api.PullRequestMe
 			message = "Delete the branch on GitHub?"
 		}
 
-		deleteBranchQuestion := &survey.Question{
-			Name: "deleteBranch",
-			Prompt: &survey.Confirm{
-				Message: message,
-				Default: false,
-			},
-		}
-		qs = append(qs, deleteBranchQuestion)
-	}
-
-	qs = append(qs, &survey.Question{
-		Name: "isConfirmed",
-		Prompt: &survey.Confirm{
-			Message: "Submit?",
+		var result bool
+		submit := &survey.Confirm{
+			Message: message,
 			Default: false,
-		},
-	})
-
-	answers := struct {
-		MergeMethod  int
-		DeleteBranch bool
-		IsConfirmed  bool
-	}{
-		DeleteBranch: opts.DeleteBranch,
+		}
+		err := prompt.SurveyAskOne(submit, &result)
+		return result, err
 	}
 
-	err := prompt.SurveyAsk(qs, &answers)
-	if err != nil {
-		return 0, false, fmt.Errorf("could not prompt: %w", err)
-	}
-	if !answers.IsConfirmed {
-		return 0, false, cancelError
-	}
+	return opts.DeleteBranch, nil
+}
 
-	var mergeMethod api.PullRequestMergeMethod
-	switch answers.MergeMethod {
-	case 0:
-		mergeMethod = api.PullRequestMergeMethodMerge
-	case 1:
-		mergeMethod = api.PullRequestMergeMethodRebase
-	case 2:
-		mergeMethod = api.PullRequestMergeMethodSquash
+func confirmSurvey() (bool, error) {
+	var confirm bool
+	submit := &survey.Confirm{
+		Message: "Submit?",
+		Default: true,
 	}
-
-	return mergeMethod, answers.DeleteBranch, nil
+	err := prompt.SurveyAskOne(submit, &confirm)
+	return confirm, err
 }

pkg/cmd/pr/merge/merge_test.go

@@ -560,6 +560,14 @@ func TestPRMerge_interactive(t *testing.T) {
 			"id": "THE-ID",
 			"number": 3
 		}] } } } }`))
+	http.Register(
+		httpmock.GraphQL(`query RepositoryInfo\b`),
+		httpmock.StringResponse(`
+		{ "data": { "repository": {
+			"mergeCommitAllowed": true,
+			"rebaseMergeAllowed": true,
+			"squashMergeAllowed": true
+		} } }`))
 	http.Register(
 		httpmock.GraphQL(`mutation PullRequestMerge\b`),
 		httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
@@ -582,20 +590,9 @@ func TestPRMerge_interactive(t *testing.T) {
 	as, surveyTeardown := prompt.InitAskStubber()
 	defer surveyTeardown()
 
-	as.Stub([]*prompt.QuestionStub{
-		{
-			Name:  "mergeMethod",
-			Value: 0,
-		},
-		{
-			Name:  "deleteBranch",
-			Value: true,
-		},
-		{
-			Name:  "isConfirmed",
-			Value: true,
-		},
-	})
+	as.StubOne(0)    // Merge method survey
+	as.StubOne(true) // Delete branch survey
+	as.StubOne(true) // Confirm submit survey
 
 	output, err := runCommand(http, "blueberries", true, "")
 	if err != nil {
@@ -618,6 +615,14 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) {
 			"id": "THE-ID",
 			"number": 3
 		}] } } } }`))
+	http.Register(
+		httpmock.GraphQL(`query RepositoryInfo\b`),
+		httpmock.StringResponse(`
+		{ "data": { "repository": {
+			"mergeCommitAllowed": true,
+			"rebaseMergeAllowed": true,
+			"squashMergeAllowed": true
+		} } }`))
 	http.Register(
 		httpmock.GraphQL(`mutation PullRequestMerge\b`),
 		httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
@@ -640,16 +645,8 @@ func TestPRMerge_interactiveWithDeleteBranch(t *testing.T) {
 	as, surveyTeardown := prompt.InitAskStubber()
 	defer surveyTeardown()
 
-	as.Stub([]*prompt.QuestionStub{
-		{
-			Name:  "mergeMethod",
-			Value: 0,
-		},
-		{
-			Name:  "isConfirmed",
-			Value: true,
-		},
-	})
+	as.StubOne(0)    // Merge method survey
+	as.StubOne(true) // Confirm submit survey
 
 	output, err := runCommand(http, "blueberries", true, "-d")
 	if err != nil {
@@ -672,6 +669,14 @@ func TestPRMerge_interactiveCancelled(t *testing.T) {
 			"id": "THE-ID",
 			"number": 3
 		}] } } } }`))
+	http.Register(
+		httpmock.GraphQL(`query RepositoryInfo\b`),
+		httpmock.StringResponse(`
+		{ "data": { "repository": {
+			"mergeCommitAllowed": true,
+			"rebaseMergeAllowed": true,
+			"squashMergeAllowed": true
+		} } }`))
 
 	cs, cmdTeardown := run.Stub()
 	defer cmdTeardown(t)
@@ -681,20 +686,9 @@ func TestPRMerge_interactiveCancelled(t *testing.T) {
 	as, surveyTeardown := prompt.InitAskStubber()
 	defer surveyTeardown()
 
-	as.Stub([]*prompt.QuestionStub{
-		{
-			Name:  "mergeMethod",
-			Value: 0,
-		},
-		{
-			Name:  "deleteBranch",
-			Value: true,
-		},
-		{
-			Name:  "isConfirmed",
-			Value: false,
-		},
-	})
+	as.StubOne(0)     // Merge method survey
+	as.StubOne(true)  // Delete branch survey
+	as.StubOne(false) // Confirm submit survey
 
 	output, err := runCommand(http, "blueberries", true, "")
 	if !errors.Is(err, cmdutil.SilentError) {
@@ -703,3 +697,17 @@ func TestPRMerge_interactiveCancelled(t *testing.T) {
 
 	assert.Equal(t, "Cancelled.\n", output.Stderr())
 }
+
+func Test_mergeMethodSurvey(t *testing.T) {
+	repo := &api.Repository{
+		MergeCommitAllowed: false,
+		RebaseMergeAllowed: true,
+		SquashMergeAllowed: true,
+	}
+	as, surveyTeardown := prompt.InitAskStubber()
+	defer surveyTeardown()
+	as.StubOne(0) // Select first option which is rebase merge
+	method, err := mergeMethodSurvey(repo)
+	assert.Nil(t, err)
+	assert.Equal(t, api.PullRequestMergeMethodRebase, method)
+}