diff --git a/api/queries_issue.go b/api/queries_issue.go index d3e0cc840..1fd1b6ac6 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -20,30 +20,38 @@ type IssuesAndTotalCount struct { } type Issue struct { - Typename string `json:"__typename"` - ID string - Number int - Title string - URL string - State string - StateReason string - Closed bool - Body string - CreatedAt time.Time - UpdatedAt time.Time - ClosedAt *time.Time - Comments Comments - Author Author - Assignees Assignees - Labels Labels - ProjectCards ProjectCards - Milestone *Milestone - ReactionGroups ReactionGroups - IsPinned bool + Typename string `json:"__typename"` + ID string + Number int + Title string + URL string + State string + StateReason string + Closed bool + Body string + ActiveLockReason string + Locked bool + CreatedAt time.Time + UpdatedAt time.Time + ClosedAt *time.Time + Comments Comments + Author Author + Assignees Assignees + Labels Labels + ProjectCards ProjectCards + Milestone *Milestone + ReactionGroups ReactionGroups + IsPinned bool } +// return values for Issue.Typename +const ( + TypeIssue string = "Issue" + TypePullRequest string = "PullRequest" +) + func (i Issue) IsPullRequest() bool { - return i.Typename == "PullRequest" + return i.Typename == TypePullRequest } type Assignees struct { diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index ab68b3617..b306b6830 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -9,6 +9,7 @@ import ( cmdDevelop "github.com/cli/cli/v2/pkg/cmd/issue/develop" cmdEdit "github.com/cli/cli/v2/pkg/cmd/issue/edit" cmdList "github.com/cli/cli/v2/pkg/cmd/issue/list" + cmdLock "github.com/cli/cli/v2/pkg/cmd/issue/lock" cmdPin "github.com/cli/cli/v2/pkg/cmd/issue/pin" cmdReopen "github.com/cli/cli/v2/pkg/cmd/issue/reopen" cmdStatus "github.com/cli/cli/v2/pkg/cmd/issue/status" @@ -42,22 +43,24 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { cmdutil.EnableRepoOverride(cmd, f) cmdutil.AddGroup(cmd, "General commands", - cmdCreate.NewCmdCreate(f, nil), cmdList.NewCmdList(f, nil), + cmdCreate.NewCmdCreate(f, nil), cmdStatus.NewCmdStatus(f, nil), ) cmdutil.AddGroup(cmd, "Targeted commands", - cmdClose.NewCmdClose(f, nil), - cmdReopen.NewCmdReopen(f, nil), cmdView.NewCmdView(f, nil), cmdComment.NewCmdComment(f, nil), - cmdDelete.NewCmdDelete(f, nil), cmdEdit.NewCmdEdit(f, nil), - cmdTransfer.NewCmdTransfer(f, nil), + cmdClose.NewCmdClose(f, nil), + cmdReopen.NewCmdReopen(f, nil), cmdDevelop.NewCmdDevelop(f, nil), + cmdTransfer.NewCmdTransfer(f, nil), + cmdLock.NewCmdLock(f, cmd.Name(), nil), + cmdLock.NewCmdUnlock(f, cmd.Name(), nil), cmdPin.NewCmdPin(f, nil), cmdUnpin.NewCmdUnpin(f, nil), + cmdDelete.NewCmdDelete(f, nil), ) return cmd diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go new file mode 100644 index 000000000..e8329974d --- /dev/null +++ b/pkg/cmd/issue/lock/lock.go @@ -0,0 +1,358 @@ +// Package lock locks and unlocks conversations on both GitHub issues and pull +// requests. +// +// Every pull request is an issue, but not every issue is a pull request. +// Therefore, this package is used in `cmd/pr` as well. +// +// A note on nomenclature for "comments", "conversations", and "discussions": +// The GitHub documentation refers to a set of comments on an issue or pull +// request as a conversation. A GitHub discussion refers to the "message board" +// for a project where announcements, questions, and answers can be posted. +package lock + +import ( + "errors" + "fmt" + "net/http" + "strings" + + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/shurcooL/githubv4" + "github.com/spf13/cobra" +) + +type iprompter interface { + Confirm(string, bool) (bool, error) + Select(string, string, []string) (int, error) +} + +// reasons contains all possible lock reasons allowed by GitHub. +// +// We don't directly construct a map so that we can maintain the reasons in +// alphabetical order. +var reasons = []string{"off_topic", "resolved", "spam", "too_heated"} + +var reasonsString = strings.Join(reasons, ", ") + +var reasonsApi = []githubv4.LockReason{ + githubv4.LockReasonOffTopic, + githubv4.LockReasonResolved, + githubv4.LockReasonSpam, + githubv4.LockReasonTooHeated, +} + +// If no reason is given (an empty string), reasonsMap will return the nil +// value, since it is not contained in the map. This, in turn, sets lock_reason +// to null in GraphQL. +var reasonsMap map[string]*githubv4.LockReason + +func init() { + reasonsMap = make(map[string]*githubv4.LockReason) + for i, reason := range reasons { + reasonsMap[reason] = &reasonsApi[i] + } +} + +type command struct { + Name string // actual command name + FullName string // complete name for the command + Typename string // return value from issue.Typename +} + +// The `FullName` should be capitalized as if starting a sentence since it is +// used in print and error statements. It's easier to manually capitalize and +// call `ToLower`, when needed, than the other way around. +var aliasIssue = command{"issue", "Issue", api.TypeIssue} +var aliasPr = command{"pr", "Pull request", api.TypePullRequest} + +var alias map[string]*command = map[string]*command{ + "issue": &aliasIssue, + "pr": &aliasPr, + api.TypeIssue: &aliasIssue, + api.TypePullRequest: &aliasPr, +} + +// Acceptable lock states for conversations. These are used in print +// statements, hence the use of strings instead of booleans. +const ( + Lock = "Lock" + Unlock = "Unlock" +) + +func fields() []string { + return []string{ + "activeLockReason", "id", "locked", "number", "title", "url", + } +} + +type LockOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter + + ParentCmd string + Reason string + SelectorArg string + Interactive bool +} + +func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, args []string) { + opts.IO = f.IOStreams + opts.HttpClient = f.HttpClient + opts.Config = f.Config + + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + opts.SelectorArg = args[0] + +} + +func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command { + opts := &LockOptions{ + ParentCmd: parentName, + Prompter: f.Prompter, + } + + c := alias[opts.ParentCmd] + short := fmt.Sprintf("Lock %s conversation", strings.ToLower(c.FullName)) + + cmd := &cobra.Command{ + Use: "lock { | }", + Short: short, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + opts.setCommonOptions(f, cmd, args) + + reasonProvided := cmd.Flags().Changed("reason") + if reasonProvided { + _, ok := reasonsMap[opts.Reason] + if !ok { + if opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() + + return cmdutil.FlagErrorf("%s Invalid reason: %v\n", + cs.FailureIconWithColor(cs.Red), opts.Reason) + } else { + return fmt.Errorf("invalid reason %s", opts.Reason) + } + } + } else if opts.IO.CanPrompt() { + opts.Interactive = true + } + + if runF != nil { + return runF(Lock, opts) + } + return lockRun(Lock, opts) + }, + } + + msg := fmt.Sprintf("Optional reason for locking conversation (%v).", reasonsString) + + cmd.Flags().StringVarP(&opts.Reason, "reason", "r", "", msg) + return cmd +} + +func NewCmdUnlock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command { + opts := &LockOptions{ParentCmd: parentName} + + c := alias[opts.ParentCmd] + short := fmt.Sprintf("Unlock %s conversation", strings.ToLower(c.FullName)) + + cmd := &cobra.Command{ + Use: "unlock { | }", + Short: short, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + opts.setCommonOptions(f, cmd, args) + + if runF != nil { + return runF(Unlock, opts) + } + return lockRun(Unlock, opts) + }, + } + + return cmd +} + +// reason creates a sentence fragment so that the lock reason can be used in a +// sentence. +// +// e.g. "resolved" -> " as RESOLVED" +func reason(reason string) string { + result := "" + if reason != "" { + result = fmt.Sprintf(" as %s", strings.ToUpper(reason)) + } + return result +} + +// status creates a string showing the result of a successful lock/unlock that +// is parameterized on a bunch of options. +// +// Example output: "Locked as RESOLVED: Issue #31 (Title of issue)" +func status(state string, lockable *api.Issue, opts *LockOptions) string { + return fmt.Sprintf("%sed%s: %s #%d (%s)", + state, reason(opts.Reason), alias[opts.ParentCmd].FullName, lockable.Number, lockable.Title) +} + +// lockRun will lock or unlock a conversation. +func lockRun(state string, opts *LockOptions) error { + cs := opts.IO.ColorScheme() + + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, fields()) + + parent := alias[opts.ParentCmd] + + if err != nil { + return err + } else if parent.Typename != issuePr.Typename { + currentType := alias[parent.Typename] + correctType := alias[issuePr.Typename] + + return fmt.Errorf("%s %s #%d not found, but found %s #%d. Use `gh %s %s %d` instead", + cs.FailureIconWithColor(cs.Red), + currentType.FullName, issuePr.Number, + strings.ToLower(correctType.FullName), issuePr.Number, + correctType.Name, strings.ToLower(state), issuePr.Number) + } + + if opts.Interactive { + options := []string{"None", "Off topic", "Resolved", "Spam", "Too heated"} + selected, err := opts.Prompter.Select("Lock reason?", "", options) + if err != nil { + return err + } + if selected > 0 { + opts.Reason = reasons[selected-1] + } + } + + successMsg := fmt.Sprintf("%s %s\n", + cs.SuccessIconWithColor(cs.Green), status(state, issuePr, opts)) + + switch state { + case Lock: + if !issuePr.Locked { + err = lockLockable(httpClient, baseRepo, issuePr, opts) + } else { + var relocked bool + relocked, err = relockLockable(httpClient, baseRepo, issuePr, opts) + + if !relocked { + successMsg = fmt.Sprintf("%s #%d already locked%s. Nothing changed.\n", + parent.FullName, issuePr.Number, reason(issuePr.ActiveLockReason)) + } + } + + case Unlock: + if issuePr.Locked { + err = unlockLockable(httpClient, baseRepo, issuePr, opts) + } else { + successMsg = fmt.Sprintf("%s #%d already unlocked. Nothing changed.\n", + parent.FullName, issuePr.Number) + } + default: + panic("bad state") + } + + if err != nil { + return err + } + + if opts.IO.IsStdoutTTY() { + fmt.Fprint(opts.IO.Out, successMsg) + } + + return nil +} + +// lockLockable will lock an issue or pull request +func lockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api.Issue, opts *LockOptions) error { + var mutation struct { + LockLockable struct { + LockedRecord struct { + Locked bool + } + } `graphql:"lockLockable(input: $input)"` + } + + variables := map[string]interface{}{ + "input": githubv4.LockLockableInput{ + LockableID: lockable.ID, + LockReason: reasonsMap[opts.Reason], + }, + } + + gql := api.NewClientFromHTTP(httpClient) + return gql.Mutate(repo.RepoHost(), "LockLockable", &mutation, variables) +} + +// unlockLockable will unlock an issue or pull request +func unlockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api.Issue, opts *LockOptions) error { + + var mutation struct { + UnlockLockable struct { + UnlockedRecord struct { + Locked bool + } + } `graphql:"unlockLockable(input: $input)"` + } + + variables := map[string]interface{}{ + "input": githubv4.UnlockLockableInput{ + LockableID: lockable.ID, + }, + } + + gql := api.NewClientFromHTTP(httpClient) + return gql.Mutate(repo.RepoHost(), "UnlockLockable", &mutation, variables) +} + +// relockLockable will unlock then lock an issue or pull request. A common use +// case would be to change the reason for locking. +// +// The current api doesn't allow you to send a single lock request to update a +// lockable item that is already locked; it will just ignore that request. You +// need to first unlock then lock with a new reason. +func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api.Issue, opts *LockOptions) (bool, error) { + if !opts.IO.CanPrompt() { + return false, errors.New("already locked") + } + + prompt := fmt.Sprintf("%s #%d already locked%s. Unlock and lock again%s?", + alias[opts.ParentCmd].FullName, lockable.Number, reason(lockable.ActiveLockReason), reason(opts.Reason)) + + relocked, err := opts.Prompter.Confirm(prompt, true) + if err != nil { + return false, err + } else if !relocked { + return relocked, nil + } + + err = unlockLockable(httpClient, repo, lockable, opts) + if err != nil { + return relocked, err + } + + err = lockLockable(httpClient, repo, lockable, opts) + if err != nil { + return relocked, err + } + + return relocked, nil +} diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go new file mode 100644 index 000000000..c6eb902ef --- /dev/null +++ b/pkg/cmd/issue/lock/lock_test.go @@ -0,0 +1,686 @@ +package lock + +import ( + "bytes" + "io" + "net/http" + "strings" + "testing" + + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/internal/prompter" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func Test_NewCmdLock(t *testing.T) { + cases := []struct { + name string + args string + want LockOptions + wantErr string + tty bool + }{ + { + name: "sets reason", + args: "--reason off_topic 451", + want: LockOptions{ + Reason: "off_topic", + SelectorArg: "451", + }, + }, + { + name: "no args", + wantErr: "accepts 1 arg(s), received 0", + }, + { + name: "no flags", + args: "451", + want: LockOptions{ + SelectorArg: "451", + }, + }, + { + name: "bad reason", + args: "--reason bad 451", + wantErr: "invalid reason bad", + }, + { + name: "bad reason tty", + args: "--reason bad 451", + tty: true, + wantErr: "X Invalid reason: bad\n", + }, + { + name: "interactive", + args: "451", + tty: true, + want: LockOptions{ + SelectorArg: "451", + Interactive: true, + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(tt.tty) + ios.SetStdinTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + f := &cmdutil.Factory{ + IOStreams: ios, + } + var opts *LockOptions + cmd := NewCmdLock(f, "issue", func(_ string, o *LockOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + assert.NoError(t, err) + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + return + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.want.Reason, opts.Reason) + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + assert.Equal(t, tt.want.Interactive, opts.Interactive) + }) + } +} + +func Test_NewCmdUnlock(t *testing.T) { + cases := []struct { + name string + args string + want LockOptions + wantErr string + tty bool + }{ + { + name: "no args", + wantErr: "accepts 1 arg(s), received 0", + }, + { + name: "no flags", + args: "451", + want: LockOptions{ + SelectorArg: "451", + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + ios.SetStdoutTTY(tt.tty) + ios.SetStdinTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + f := &cmdutil.Factory{ + IOStreams: ios, + } + var opts *LockOptions + cmd := NewCmdUnlock(f, "issue", func(_ string, o *LockOptions) error { + opts = o + return nil + }) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + argv, err := shlex.Split(tt.args) + assert.NoError(t, err) + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + return + } else { + assert.NoError(t, err) + } + + assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg) + }) + } +} + +func Test_runLock(t *testing.T) { + cases := []struct { + name string + opts LockOptions + promptStubs func(*testing.T, *prompter.PrompterMock) + httpStubs func(*testing.T, *httpmock.Registry) + wantOut string + wantErrOut string + wantErr string + tty bool + state string + }{ + { + name: "lock issue nontty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + }, + { + name: "lock issue tty", + tty: true, + opts: LockOptions{ + Interactive: true, + SelectorArg: "451", + ParentCmd: "issue", + }, + state: Lock, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + promptStubs: func(t *testing.T, pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, d string, opts []string) (int, error) { + if p == "Lock reason?" { + assert.Equal(t, []string{"None", "Off topic", "Resolved", "Spam", "Too heated"}, opts) + + return prompter.IndexFor(opts, "Too heated") + } + + return -1, prompter.NoSuchPromptErr(p) + } + }, + wantOut: "✓ Locked as TOO_HEATED: Issue #451 (traverse the library)\n", + }, + { + name: "lock issue with explicit reason tty", + tty: true, + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + Reason: "off_topic", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + wantOut: "✓ Locked as OFF_TOPIC: Issue #451 (traverse the library)\n", + }, + { + name: "unlock issue tty", + tty: true, + state: Unlock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + }, + wantOut: "✓ Unlocked: Issue #451 (traverse the library)\n", + }, + { + name: "unlock issue nontty", + state: Unlock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + }, + }, + { + name: "lock issue with explicit reason nontty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + Reason: "off_topic", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + }, + { + name: "relock issue tty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + Reason: "off_topic", + }, + tty: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + promptStubs: func(t *testing.T, pm *prompter.PrompterMock) { + pm.ConfirmFunc = func(p string, d bool) (bool, error) { + if p == "Issue #451 already locked. Unlock and lock again as OFF_TOPIC?" { + return true, nil + } + + return false, prompter.NoSuchPromptErr(p) + } + }, + wantOut: "✓ Locked as OFF_TOPIC: Issue #451 (traverse the library)\n", + }, + { + name: "relock issue nontty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "issue", + Reason: "off_topic", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "Issue" }}}}`)) + }, + wantErr: "already locked", + }, + + { + name: "lock pr nontty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + }, + { + name: "lock pr tty", + tty: true, + opts: LockOptions{ + Interactive: true, + SelectorArg: "451", + ParentCmd: "pr", + }, + state: Lock, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + promptStubs: func(t *testing.T, pm *prompter.PrompterMock) { + pm.SelectFunc = func(p, d string, opts []string) (int, error) { + if p == "Lock reason?" { + return prompter.IndexFor(opts, "Too heated") + } + + return -1, prompter.NoSuchPromptErr(p) + } + }, + wantOut: "✓ Locked as TOO_HEATED: Pull request #451 (traverse the library)\n", + }, + { + name: "lock pr with explicit reason tty", + tty: true, + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + Reason: "off_topic", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + wantOut: "✓ Locked as OFF_TOPIC: Pull request #451 (traverse the library)\n", + }, + { + name: "lock pr with explicit nontty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + Reason: "off_topic", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + }, + { + name: "unlock pr tty", + state: Unlock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + }, + }, + { + name: "unlock pr nontty", + tty: true, + state: Unlock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + }, + wantOut: "✓ Unlocked: Pull request #451 (traverse the library)\n", + }, + { + name: "relock pr tty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + Reason: "off_topic", + }, + tty: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + promptStubs: func(t *testing.T, pm *prompter.PrompterMock) { + pm.ConfirmFunc = func(p string, d bool) (bool, error) { + if p == "Pull request #451 already locked. Unlock and lock again as OFF_TOPIC?" { + return true, nil + } + + return false, prompter.NoSuchPromptErr(p) + } + }, + wantOut: "✓ Locked as OFF_TOPIC: Pull request #451 (traverse the library)\n", + }, + { + name: "relock pr nontty", + state: Lock, + opts: LockOptions{ + SelectorArg: "451", + ParentCmd: "pr", + Reason: "off_topic", + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 451, + "locked": true, + "title": "traverse the library", + "__typename": "PullRequest" }}}}`)) + }, + wantErr: "already locked", + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.httpStubs != nil { + tt.httpStubs(t, reg) + } + + pm := &prompter.PrompterMock{} + if tt.promptStubs != nil { + tt.promptStubs(t, pm) + } + + ios, _, stdout, stderr := iostreams.Test() + ios.SetStdoutTTY(tt.tty) + ios.SetStdinTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + + tt.opts.Prompter = pm + tt.opts.IO = ios + tt.opts.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("OWNER/REPO") + } + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + err := lockRun(tt.state, &tt.opts) + output := &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + } + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.wantOut, output.String()) + assert.Equal(t, tt.wantErrOut, output.Stderr()) + } + }) + } +} + +func TestReasons(t *testing.T) { + assert.Equal(t, len(reasons), len(reasonsApi)) + + for _, reason := range reasons { + assert.Equal(t, strings.ToUpper(reason), string(*reasonsMap[reason])) + } +} diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index b46c71e9e..bdd004d15 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -2,6 +2,7 @@ package pr import ( "github.com/MakeNowJust/heredoc" + cmdLock "github.com/cli/cli/v2/pkg/cmd/issue/lock" cmdCheckout "github.com/cli/cli/v2/pkg/cmd/pr/checkout" cmdChecks "github.com/cli/cli/v2/pkg/cmd/pr/checks" cmdClose "github.com/cli/cli/v2/pkg/cmd/pr/close" @@ -48,11 +49,13 @@ func NewCmdPR(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) cmd.AddCommand(cmdDiff.NewCmdDiff(f, nil)) cmd.AddCommand(cmdList.NewCmdList(f, nil)) + cmd.AddCommand(cmdLock.NewCmdLock(f, cmd.Name(), nil)) cmd.AddCommand(cmdMerge.NewCmdMerge(f, nil)) cmd.AddCommand(cmdReady.NewCmdReady(f, nil)) cmd.AddCommand(cmdReopen.NewCmdReopen(f, nil)) cmd.AddCommand(cmdReview.NewCmdReview(f, nil)) cmd.AddCommand(cmdStatus.NewCmdStatus(f, nil)) + cmd.AddCommand(cmdLock.NewCmdUnlock(f, cmd.Name(), nil)) cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdChecks.NewCmdChecks(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index 81ebee963..b5e4db996 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -58,6 +58,7 @@ type EditOptions struct { type EditRepositoryInput struct { 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"` @@ -151,6 +152,7 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr cmdutil.NilBoolFlag(cmd, &opts.Edits.EnableAutoMerge, "enable-auto-merge", "", "Enable auto-merge functionality") 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") cmd.Flags().StringSliceVar(&opts.AddTopics, "add-topic", nil, "Add repository topic") cmd.Flags().StringSliceVar(&opts.RemoveTopics, "remove-topic", nil, "Remove repository topic") diff --git a/pkg/cmd/repo/edit/edit_test.go b/pkg/cmd/repo/edit/edit_test.go index 41193b057..7302b5169 100644 --- a/pkg/cmd/repo/edit/edit_test.go +++ b/pkg/cmd/repo/edit/edit_test.go @@ -128,6 +128,23 @@ func Test_editRun(t *testing.T) { })) }, }, + { + name: "allow update branch", + opts: EditOptions{ + Repository: ghrepo.NewWithHost("OWNER", "REPO", "github.com"), + Edits: EditRepositoryInput{ + AllowUpdateBranch: bp(true), + }, + }, + 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, 1, len(payload)) + assert.Equal(t, true, payload["allow_update_branch"]) + })) + }, + }, } for _, tt := range tests { @@ -339,3 +356,7 @@ func Test_editRun_interactive(t *testing.T) { func sp(v string) *string { return &v } + +func bp(b bool) *bool { + return &b +}