From e25c36a7b161ac21646d4422bc76f90a5a2fab6d Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 19 Mar 2022 15:39:51 -0400 Subject: [PATCH 01/34] Stub out files for lock command Lock will lock and unlock both issues and pull requests --- pkg/cmd/issue/lock/lock.go | 3 +++ pkg/cmd/issue/lock/lock_test.go | 1 + 2 files changed, 4 insertions(+) create mode 100644 pkg/cmd/issue/lock/lock.go create mode 100644 pkg/cmd/issue/lock/lock_test.go diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go new file mode 100644 index 000000000..adab2dd77 --- /dev/null +++ b/pkg/cmd/issue/lock/lock.go @@ -0,0 +1,3 @@ +// Package lock locks and unlocks conversations on both GitHub issues and pull +// requests. +package lock diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go new file mode 100644 index 000000000..371688338 --- /dev/null +++ b/pkg/cmd/issue/lock/lock_test.go @@ -0,0 +1 @@ +package lock From 631bea2e6db04e227ea9c93b2c15691e04d872b7 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Mon, 21 Mar 2022 02:17:19 -0400 Subject: [PATCH 02/34] Draft first design As originally designed in the issue discussion, a single function `NewCmdLock()` with a parameter to lock or unlock was proposed. However, after playing around with a couple different designs, it seems best to create two separate public functions and one private function to do the common work. Using two public functions seems to make sense because the api for locking is different from the api for unlocking. Therefore, the documentation for both are different and keeping them in separate functions would make it easier to maintain the documentation. --- pkg/cmd/issue/issue.go | 3 + pkg/cmd/issue/lock/lock.go | 147 +++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index c807ecd0e..a4c354e33 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -8,6 +8,7 @@ import ( cmdDelete "github.com/cli/cli/v2/pkg/cmd/issue/delete" 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" cmdReopen "github.com/cli/cli/v2/pkg/cmd/issue/reopen" cmdStatus "github.com/cli/cli/v2/pkg/cmd/issue/status" cmdTransfer "github.com/cli/cli/v2/pkg/cmd/issue/transfer" @@ -41,8 +42,10 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) cmd.AddCommand(cmdList.NewCmdList(f, nil)) + cmd.AddCommand(cmdLock.NewCmdLock(f, nil)) cmd.AddCommand(cmdReopen.NewCmdReopen(f, nil)) cmd.AddCommand(cmdStatus.NewCmdStatus(f, nil)) + cmd.AddCommand(cmdLock.NewCmdUnlock(f, nil)) cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil)) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index adab2dd77..a302d1e91 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -1,3 +1,150 @@ // 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, we should be able to use this in the package 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 ( + "fmt" + "net/http" + "strings" + + "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghrepo" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/spf13/cobra" +) + +type state uint + +// Acceptable lock states for conversations +const ( + Unlock state = iota + Lock +) + +var reasons = []string{"off-topic", "resolved", "spam", "too-heated"} +var reasonsString = strings.Join(reasons, ", ") +var reasonsJson = []string{"off-topic", "resolved", "spam", "too heated"} +var reasonsMap map[string]string + +func init() { + reasonsMap = make(map[string]string) + for i, reason := range reasons { + reasonsMap[reason] = reasonsJson[i] + } +} + +type LockOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + PadlockState state + Reason string + ParentCmd string + IssueNumber string +} + +func setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, opts *LockOptions, args []string) { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + // Set what type of conversation + opts.ParentCmd = cmd.Parent().Name() + + opts.IssueNumber = args[0] +} + +// NewCmdLock will lock the conversation for an issue or pull request +func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { + + // TODO: Currently doesn't work. Besides some minor changes, code is copy + // and pasted from NewCmdReopen(). + opts := &LockOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + PadlockState: Lock, + } + + cmd := &cobra.Command{ + Use: "lock { | }", + Short: "Lock a conversation", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + + reasonProvided := cmd.Flags().Changed("message") + if reasonProvided { + jsonReason, ok := reasonsMap[opts.Reason] + if !ok { + return cmdutil.FlagErrorf("Invalid reason: %v.\nMust be one of the following: %v.\nAborting lock.", + opts.Reason, reasonsString) + } + + opts.Reason = jsonReason + } + + setCommonOptions(f, cmd, opts, args) + + if runF != nil { + return runF(opts) + } + return padlock(opts) + }, + } + + msg := fmt.Sprintf("Optional reason for locking conversation. Must be one of the following: %v.", reasonsString) + + cmd.Flags().StringVarP(&opts.Reason, "message", "m", "", msg) + return cmd +} + +// NewCmdUnlock will unlock the conversation for an issue or pull request +func NewCmdUnlock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { + + // TODO: Currently doesn't work. Besides some minor changes, code is copy + // and pasted from NewCmdReopen(). + opts := &LockOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + PadlockState: Unlock, + } + + cmd := &cobra.Command{ + Use: "unlock { | }", + Short: "Unlock a conversation", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + + setCommonOptions(f, cmd, opts, args) + + if runF != nil { + return runF(opts) + } + return padlock(opts) + }, + } + + return cmd +} + +// padlock will lock or unlock a conversation. +func padlock(opts *LockOptions) error { + switch opts.PadlockState { + case Lock: + fmt.Printf("Locking %v #%v\n", opts.ParentCmd, opts.IssueNumber) + case Unlock: + fmt.Printf("Unlocking %v #%v\n", opts.ParentCmd, opts.IssueNumber) + } + + return nil +} From 72a92fa09dc33eecb3569468b2286241c769300b Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 30 Mar 2022 03:01:28 -0400 Subject: [PATCH 03/34] Added lock subcommands to pr command --- pkg/cmd/pr/pr.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index 43090b690..2e3005aff 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, 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, nil)) cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdChecks.NewCmdChecks(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) From 4b226b1bf8bc2341d526b4b8992e0224e2bde329 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 30 Mar 2022 03:09:50 -0400 Subject: [PATCH 04/34] Other changes - Changed function to method - Moved additional common options to method - Remove redundant documentation - Cobra sets documentation in the Command struct. --- pkg/cmd/issue/lock/lock.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index a302d1e91..3ee3e9cc0 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -53,7 +53,12 @@ type LockOptions struct { IssueNumber string } -func setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, opts *LockOptions, args []string) { +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 @@ -63,15 +68,9 @@ func setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, opts *LockOptions, opts.IssueNumber = args[0] } -// NewCmdLock will lock the conversation for an issue or pull request func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { - // TODO: Currently doesn't work. Besides some minor changes, code is copy - // and pasted from NewCmdReopen(). opts := &LockOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - Config: f.Config, PadlockState: Lock, } @@ -92,7 +91,7 @@ func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comman opts.Reason = jsonReason } - setCommonOptions(f, cmd, opts, args) + opts.setCommonOptions(f, cmd, args) if runF != nil { return runF(opts) @@ -107,15 +106,9 @@ func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comman return cmd } -// NewCmdUnlock will unlock the conversation for an issue or pull request func NewCmdUnlock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { - // TODO: Currently doesn't work. Besides some minor changes, code is copy - // and pasted from NewCmdReopen(). opts := &LockOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - Config: f.Config, PadlockState: Unlock, } @@ -125,7 +118,7 @@ func NewCmdUnlock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comm Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - setCommonOptions(f, cmd, opts, args) + opts.setCommonOptions(f, cmd, args) if runF != nil { return runF(opts) From c0f5345c68b73e57e9cc9d296b52ff35bed2b75d Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sun, 10 Apr 2022 01:49:50 -0400 Subject: [PATCH 05/34] Export return values for Issue.Typename These are needed to know if an issue is an actual issue or a pull request. --- api/queries_issue.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index 4146bfeaa..a454dc256 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -40,8 +40,14 @@ type Issue struct { ReactionGroups ReactionGroups } +// 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 { From ffcda8fdd4ae42304fb69b8210e328245c323751 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sun, 10 Apr 2022 01:52:48 -0400 Subject: [PATCH 06/34] Map parent commands to typenames --- pkg/cmd/issue/lock/lock.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 3ee3e9cc0..fc6d0e9f4 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -35,6 +35,12 @@ var reasonsString = strings.Join(reasons, ", ") var reasonsJson = []string{"off-topic", "resolved", "spam", "too heated"} var reasonsMap map[string]string +// typenames maps from the parent command to issue.Typename +var typenames = map[string]string{ + "issue": api.TypeIssue, + "pr": api.TypePullRequest, +} + func init() { reasonsMap = make(map[string]string) for i, reason := range reasons { From 4f37c54ca039d567e7ab3570c92eaea52cde01a6 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sun, 10 Apr 2022 01:54:52 -0400 Subject: [PATCH 07/34] Add lock-related values to issue Add queries for "ActiveLockReason" and "Locked" --- api/queries_issue.go | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index a454dc256..008e3dbc2 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -20,24 +20,26 @@ type IssuesAndTotalCount struct { } type Issue struct { - Typename string `json:"__typename"` - ID string - Number int - Title string - URL string - State 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 + Typename string `json:"__typename"` + ID string + Number int + Title string + URL string + State 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 } // return values for Issue.Typename From fa470fc499916824d847ecc509073ad4e1b90cd6 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sun, 10 Apr 2022 01:57:38 -0400 Subject: [PATCH 08/34] Rearrange --- pkg/cmd/issue/lock/lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index fc6d0e9f4..a43354797 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -31,8 +31,8 @@ const ( ) var reasons = []string{"off-topic", "resolved", "spam", "too-heated"} -var reasonsString = strings.Join(reasons, ", ") var reasonsJson = []string{"off-topic", "resolved", "spam", "too heated"} +var reasonsString = strings.Join(reasons, ", ") var reasonsMap map[string]string // typenames maps from the parent command to issue.Typename From 364dd38bc03f7ab0b5a2a3a9dccc50f60e90a21d Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sun, 10 Apr 2022 01:58:04 -0400 Subject: [PATCH 09/34] Get graphql queries and mutations working - Fix error if found an issue while using `gh pr lock/unlock` or vice versa - Added additional types - Used githubv4 types - Added "relock" state - If the conversation is already locked you have two choices: try to lock it again or do nothing. Do nothing is easy. But, if you want to change the lock reason, you need to first unlock the conversation and then lock it again. - Added survey to confirm if you want to relock - Added formatted print statements --- pkg/cmd/issue/lock/lock.go | 217 ++++++++++++++++++++++++++++++------- 1 file changed, 179 insertions(+), 38 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index a43354797..c830a343d 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -11,52 +11,75 @@ package lock import ( + "context" "fmt" "net/http" "strings" + "github.com/AlecAivazis/survey/v2" + "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/ghinstance" "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" + graphql "github.com/cli/shurcooL-graphql" + "github.com/shurcooL/githubv4" "github.com/spf13/cobra" ) -type state uint +// the reason for not just declaring a map is so that I can put the keys 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, +} +var reasonsMap map[string]*githubv4.LockReason + +func init() { + reasonsMap = make(map[string]*githubv4.LockReason) + for i, reason := range reasons { + reasonsMap[reason] = &reasonsApi[i] + } + + // If no reason given, set lock_reason to null in graphql + reasonsMap[""] = nil +} + +type command struct { + FullName string // complete name for the command + Typename string // return value from issue.Typename +} + +var cmds map[string]command = map[string]command{ + "issue": {"Issue", api.TypeIssue}, + "pr": {"Pull request", api.TypePullRequest}, +} // Acceptable lock states for conversations const ( - Unlock state = iota - Lock + Unlock string = "Unlock" + Lock string = "Lock" + Relock string = "Relock" ) -var reasons = []string{"off-topic", "resolved", "spam", "too-heated"} -var reasonsJson = []string{"off-topic", "resolved", "spam", "too heated"} -var reasonsString = strings.Join(reasons, ", ") -var reasonsMap map[string]string - -// typenames maps from the parent command to issue.Typename -var typenames = map[string]string{ - "issue": api.TypeIssue, - "pr": api.TypePullRequest, -} - -func init() { - reasonsMap = make(map[string]string) - for i, reason := range reasons { - reasonsMap[reason] = reasonsJson[i] - } -} - type LockOptions struct { - HttpClient func() (*http.Client, error) - Config func() (config.Config, error) - IO *iostreams.IOStreams - BaseRepo func() (ghrepo.Interface, error) - PadlockState state - Reason string + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + Fields []string + PadlockState string ParentCmd string - IssueNumber string + Reason string + SelectorArg string } func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, args []string) { @@ -68,10 +91,11 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command // support `-R, --repo` override opts.BaseRepo = f.BaseRepo - // Set what type of conversation - opts.ParentCmd = cmd.Parent().Name() + opts.SelectorArg = args[0] + + opts.Fields = []string{ + "id", "number", "title", "url", "locked", "activeLockReason"} - opts.IssueNumber = args[0] } func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { @@ -86,15 +110,17 @@ func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comman Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + opts.setCommonOptions(f, cmd, args) + reasonProvided := cmd.Flags().Changed("message") if reasonProvided { - jsonReason, ok := reasonsMap[opts.Reason] + _, ok := reasonsMap[opts.Reason] if !ok { - return cmdutil.FlagErrorf("Invalid reason: %v.\nMust be one of the following: %v.\nAborting lock.", - opts.Reason, reasonsString) - } + cs := opts.IO.ColorScheme() - opts.Reason = jsonReason + return cmdutil.FlagErrorf("%s Invalid reason: %v.\nSee help for options.\nAborting lock.", + cs.FailureIconWithColor(cs.Red), opts.Reason) + } } opts.setCommonOptions(f, cmd, args) @@ -138,12 +164,127 @@ func NewCmdUnlock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comm // padlock will lock or unlock a conversation. func padlock(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, opts.Fields) + + parent := cmds[opts.ParentCmd] + + switch { + case err != nil: + return err + case parent.Typename != issuePr.Typename: + return fmt.Errorf("%s #%d not found, but found %s #%d", + parent.Typename, issuePr.Number, + issuePr.Typename, issuePr.Number) + case opts.PadlockState == Lock && issuePr.Locked: + opts.PadlockState = Relock + } + + var confirm bool + reason := " " + if opts.Reason != "" { + reason = fmt.Sprintf(" (%s) ", opts.Reason) + } + successMsg := fmt.Sprintf("%s %sed%s%s #%d (%s)\n", + cs.SuccessIconWithColor(cs.Green), opts.PadlockState, reason, + parent.FullName, issuePr.Number, issuePr.Title) + switch opts.PadlockState { case Lock: - fmt.Printf("Locking %v #%v\n", opts.ParentCmd, opts.IssueNumber) + err = lockLockable(httpClient, baseRepo, issuePr, opts) + case Relock: + shouldRelock := &survey.Confirm{ + Message: fmt.Sprintf("%s #%d already locked. Unlock and lock again?", parent.FullName, issuePr.Number), + Default: true, + } + err = survey.AskOne(shouldRelock, &confirm) + if err != nil { + return err + } + + if confirm { + err = relockLockable(httpClient, baseRepo, issuePr, opts) + } else { + successMsg = fmt.Sprintf("%s %s #%d (%s) unchanged\n", + cs.SuccessIconWithColor(cs.Green), parent.FullName, issuePr.Number, issuePr.Title) + } case Unlock: - fmt.Printf("Unlocking %v #%v\n", opts.ParentCmd, opts.IssueNumber) + err = unlockLockable(httpClient, baseRepo, issuePr, opts) } + if err != nil { + return err + } + + fmt.Fprint(opts.IO.ErrOut, 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 := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) + + return gql.MutateNamed(context.Background(), "LockLockable", &mutation, variables) +} + +// unlockLockable will lock 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 := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) + + return gql.MutateNamed(context.Background(), "UnlockLockable", &mutation, variables) +} + +func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api.Issue, opts *LockOptions) error { + opts.PadlockState = Unlock + err := unlockLockable(httpClient, repo, lockable, opts) + if err != nil { + return err + } + + opts.PadlockState = Lock + err = lockLockable(httpClient, repo, lockable, opts) + if err != nil { + return err + } + + opts.PadlockState = Relock return nil } From e019ff9f022ecc661aa96ce2fd4a1e7e7b46f1dd Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Mon, 11 Apr 2022 01:38:42 -0400 Subject: [PATCH 10/34] Modify documentation if called from issue or pr --- pkg/cmd/issue/issue.go | 4 ++-- pkg/cmd/issue/lock/lock.go | 24 ++++++++++++------------ pkg/cmd/pr/pr.go | 4 ++-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index a4c354e33..2fb51580d 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -42,10 +42,10 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) cmd.AddCommand(cmdList.NewCmdList(f, nil)) - cmd.AddCommand(cmdLock.NewCmdLock(f, nil)) + cmd.AddCommand(cmdLock.NewCmdLock(f, cmd.Name())) cmd.AddCommand(cmdReopen.NewCmdReopen(f, nil)) cmd.AddCommand(cmdStatus.NewCmdStatus(f, nil)) - cmd.AddCommand(cmdLock.NewCmdUnlock(f, nil)) + cmd.AddCommand(cmdLock.NewCmdUnlock(f, cmd.Name())) cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil)) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index c830a343d..58e3bc026 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -98,15 +98,19 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command } -func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { +func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { opts := &LockOptions{ + ParentCmd: parentName, PadlockState: Lock, } + c := cmds[opts.ParentCmd] + short := fmt.Sprintf("Lock %s conversation", strings.ToLower(c.FullName)) + cmd := &cobra.Command{ Use: "lock { | }", - Short: "Lock a conversation", + Short: short, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -123,11 +127,6 @@ func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comman } } - opts.setCommonOptions(f, cmd, args) - - if runF != nil { - return runF(opts) - } return padlock(opts) }, } @@ -138,23 +137,24 @@ func NewCmdLock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Comman return cmd } -func NewCmdUnlock(f *cmdutil.Factory, runF func(*LockOptions) error) *cobra.Command { +func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { opts := &LockOptions{ + ParentCmd: parentName, PadlockState: Unlock, } + c := cmds[opts.ParentCmd] + short := fmt.Sprintf("Unlock %s conversation", strings.ToLower(c.FullName)) + cmd := &cobra.Command{ Use: "unlock { | }", - Short: "Unlock a conversation", + Short: short, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { opts.setCommonOptions(f, cmd, args) - if runF != nil { - return runF(opts) - } return padlock(opts) }, } diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index 2e3005aff..d5265f405 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -49,13 +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, nil)) + cmd.AddCommand(cmdLock.NewCmdLock(f, cmd.Name())) 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, nil)) + cmd.AddCommand(cmdLock.NewCmdUnlock(f, cmd.Name())) cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdChecks.NewCmdChecks(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) From 32f8283c77ff6188250b0d3f9183902b8701f228 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Fri, 29 Apr 2022 00:17:53 -0400 Subject: [PATCH 11/34] Refactor things - Switch to underscores - Revise error message --- pkg/cmd/issue/lock/lock.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 58e3bc026..c78a48af9 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -31,7 +31,7 @@ import ( // the reason for not just declaring a map is so that I can put the keys in // alphabetical order -var reasons = []string{"off-topic", "resolved", "spam", "too-heated"} +var reasons = []string{"off_topic", "resolved", "spam", "too_heated"} var reasonsString = strings.Join(reasons, ", ") var reasonsApi = []githubv4.LockReason{ @@ -122,7 +122,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { if !ok { cs := opts.IO.ColorScheme() - return cmdutil.FlagErrorf("%s Invalid reason: %v.\nSee help for options.\nAborting lock.", + return cmdutil.FlagErrorf("%s Invalid reason: %v.\n Aborting lock. See help for options.", cs.FailureIconWithColor(cs.Red), opts.Reason) } } From 7504f5ec00613e76e5e9e6e4574e0a6660faa651 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Fri, 29 Apr 2022 00:19:28 -0400 Subject: [PATCH 12/34] Remove opts.PadlockState Rather than saving the intended lock state and calling a method depending on the lock state, just call the method directly. By the time you need to the padlock state, you already know which method to use; no need to first change the lock state than call the method. Also, refactored print/error messages that are conditional. --- pkg/cmd/issue/lock/lock.go | 114 ++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 53 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index c78a48af9..76bcb4668 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -75,11 +75,10 @@ type LockOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) - Fields []string - PadlockState string - ParentCmd string - Reason string - SelectorArg string + Fields []string + ParentCmd string + Reason string + SelectorArg string } func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, args []string) { @@ -100,10 +99,7 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { - opts := &LockOptions{ - ParentCmd: parentName, - PadlockState: Lock, - } + opts := &LockOptions{ParentCmd: parentName} c := cmds[opts.ParentCmd] short := fmt.Sprintf("Lock %s conversation", strings.ToLower(c.FullName)) @@ -127,7 +123,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { } } - return padlock(opts) + return padlock(Lock, opts) }, } @@ -139,10 +135,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { - opts := &LockOptions{ - ParentCmd: parentName, - PadlockState: Unlock, - } + opts := &LockOptions{ParentCmd: parentName} c := cmds[opts.ParentCmd] short := fmt.Sprintf("Unlock %s conversation", strings.ToLower(c.FullName)) @@ -155,15 +148,29 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { opts.setCommonOptions(f, cmd, args) - return padlock(opts) + return padlock(Unlock, opts) }, } return cmd } +func reason(reason string) string { + result := "" + if reason != "" { + result = fmt.Sprintf(" as %s", reason) + } + return result +} + +func status(state string, lockable *api.Issue, opts *LockOptions) string { + + return fmt.Sprintf("%sed%s: %s #%d (%s)", + state, reason(opts.Reason), cmds[opts.ParentCmd].FullName, lockable.Number, lockable.Title) +} + // padlock will lock or unlock a conversation. -func padlock(opts *LockOptions) error { +func padlock(state string, opts *LockOptions) error { cs := opts.IO.ColorScheme() httpClient, err := opts.HttpClient() @@ -180,42 +187,29 @@ func padlock(opts *LockOptions) error { return err case parent.Typename != issuePr.Typename: return fmt.Errorf("%s #%d not found, but found %s #%d", - parent.Typename, issuePr.Number, - issuePr.Typename, issuePr.Number) - case opts.PadlockState == Lock && issuePr.Locked: - opts.PadlockState = Relock + parent.Typename, issuePr.Number, issuePr.Typename, issuePr.Number) } - var confirm bool - reason := " " - if opts.Reason != "" { - reason = fmt.Sprintf(" (%s) ", opts.Reason) + // If trying to lock an already locked conversation, mark as relock. + if state == Lock && issuePr.Locked { + state = Relock } - successMsg := fmt.Sprintf("%s %sed%s%s #%d (%s)\n", - cs.SuccessIconWithColor(cs.Green), opts.PadlockState, reason, - parent.FullName, issuePr.Number, issuePr.Title) - switch opts.PadlockState { + successMsg := fmt.Sprintf("%s %s\n", + cs.SuccessIconWithColor(cs.Green), status(state, issuePr, opts)) + + switch state { case Lock: err = lockLockable(httpClient, baseRepo, issuePr, opts) - case Relock: - shouldRelock := &survey.Confirm{ - Message: fmt.Sprintf("%s #%d already locked. Unlock and lock again?", parent.FullName, issuePr.Number), - Default: true, - } - err = survey.AskOne(shouldRelock, &confirm) - if err != nil { - return err - } - - if confirm { - err = relockLockable(httpClient, baseRepo, issuePr, opts) - } else { - successMsg = fmt.Sprintf("%s %s #%d (%s) unchanged\n", - cs.SuccessIconWithColor(cs.Green), parent.FullName, issuePr.Number, issuePr.Title) - } case Unlock: err = unlockLockable(httpClient, baseRepo, issuePr, opts) + case Relock: + relocked, errRelock := relockLockable(httpClient, baseRepo, issuePr, opts) + err = errRelock + if !relocked { + successMsg = fmt.Sprintf("%s Unchanged: %s #%d (%s)\n", + cs.SuccessIconWithColor(cs.Green), parent.FullName, issuePr.Number, issuePr.Title) + } } if err != nil { @@ -272,19 +266,33 @@ func unlockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap return gql.MutateNamed(context.Background(), "UnlockLockable", &mutation, variables) } -func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api.Issue, opts *LockOptions) error { - opts.PadlockState = Unlock - err := unlockLockable(httpClient, repo, lockable, opts) - if err != nil { - return err +// relockLockable will unlock then lock an issue or pull request. A common use +// case would be to change the reason for locking. +func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api.Issue, opts *LockOptions) (bool, error) { + + var relocked bool + shouldRelock := &survey.Confirm{ + Message: fmt.Sprintf("%s #%d locked%s. Unlock and lock again?", + cmds[opts.ParentCmd].FullName, lockable.Number, reason(strings.ToLower(lockable.ActiveLockReason))), + Default: true, + } + + err := survey.AskOne(shouldRelock, &relocked) + if err != nil { + return relocked, err + } else if !relocked { + return relocked, nil + } + + err = unlockLockable(httpClient, repo, lockable, opts) + if err != nil { + return relocked, err } - opts.PadlockState = Lock err = lockLockable(httpClient, repo, lockable, opts) if err != nil { - return err + return relocked, err } - opts.PadlockState = Relock - return nil + return relocked, nil } From 8918cf6815ccabb786352e8eda5ca01f62c1a655 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 12 Apr 2022 04:59:50 -0400 Subject: [PATCH 13/34] Skip unlock api call if already unlocked --- pkg/cmd/issue/lock/lock.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 76bcb4668..304089d9b 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -202,6 +202,13 @@ func padlock(state string, opts *LockOptions) error { case Lock: err = lockLockable(httpClient, baseRepo, issuePr, opts) case Unlock: + // if already unlocked, skip the api call to unlock + if !issuePr.Locked { + successMsg = fmt.Sprintf("%s #%d already unlocked. Nothing changed.\n", + parent.FullName, issuePr.Number) + break + } + err = unlockLockable(httpClient, baseRepo, issuePr, opts) case Relock: relocked, errRelock := relockLockable(httpClient, baseRepo, issuePr, opts) From fc1b929a8136973836eb12c1a3fb9f3fd77f761d Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 12 Apr 2022 05:00:28 -0400 Subject: [PATCH 14/34] Remove opts.PadlockState I noticed that PadlockState didn't really have anything to do with the LockOptions and it was easy to call an incorrect locking function that didn't match the PadlockState. Now, you pass in the state as an argument and you simply call the appropriate function instead of setting PadlockState and then calling the correct function. - Other touch ups and refactoring --- pkg/cmd/issue/lock/lock.go | 64 ++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 304089d9b..38e33aab3 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -53,20 +53,26 @@ func init() { } type command struct { + Name string // actual command name FullName string // complete name for the command Typename string // return value from issue.Typename } -var cmds map[string]command = map[string]command{ - "issue": {"Issue", api.TypeIssue}, - "pr": {"Pull request", api.TypePullRequest}, +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 +// Acceptable lock states for conversations. These are used in print +// statements, hence the use of strings instead of booleans. const ( Unlock string = "Unlock" Lock string = "Lock" - Relock string = "Relock" ) type LockOptions struct { @@ -101,7 +107,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { opts := &LockOptions{ParentCmd: parentName} - c := cmds[opts.ParentCmd] + c := alias[opts.ParentCmd] short := fmt.Sprintf("Lock %s conversation", strings.ToLower(c.FullName)) cmd := &cobra.Command{ @@ -137,7 +143,7 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { opts := &LockOptions{ParentCmd: parentName} - c := cmds[opts.ParentCmd] + c := alias[opts.ParentCmd] short := fmt.Sprintf("Unlock %s conversation", strings.ToLower(c.FullName)) cmd := &cobra.Command{ @@ -158,7 +164,7 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { func reason(reason string) string { result := "" if reason != "" { - result = fmt.Sprintf(" as %s", reason) + result = fmt.Sprintf(" as %s", strings.ToUpper(reason)) } return result } @@ -166,7 +172,7 @@ func reason(reason string) string { func status(state string, lockable *api.Issue, opts *LockOptions) string { return fmt.Sprintf("%sed%s: %s #%d (%s)", - state, reason(opts.Reason), cmds[opts.ParentCmd].FullName, lockable.Number, lockable.Title) + state, reason(opts.Reason), alias[opts.ParentCmd].FullName, lockable.Number, lockable.Title) } // padlock will lock or unlock a conversation. @@ -180,43 +186,39 @@ func padlock(state string, opts *LockOptions) error { issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, opts.Fields) - parent := cmds[opts.ParentCmd] + parent := alias[opts.ParentCmd] switch { case err != nil: return err + case parent.Typename != issuePr.Typename: return fmt.Errorf("%s #%d not found, but found %s #%d", - parent.Typename, issuePr.Number, issuePr.Typename, issuePr.Number) - } - - // If trying to lock an already locked conversation, mark as relock. - if state == Lock && issuePr.Locked { - state = Relock + alias[parent.Typename].FullName, issuePr.Number, + strings.ToLower(alias[issuePr.Typename].FullName), issuePr.Number) } successMsg := fmt.Sprintf("%s %s\n", cs.SuccessIconWithColor(cs.Green), status(state, issuePr, opts)) - switch state { - case Lock: + switch { + case state == Lock && !issuePr.Locked: err = lockLockable(httpClient, baseRepo, issuePr, opts) - case Unlock: - // if already unlocked, skip the api call to unlock - if !issuePr.Locked { - successMsg = fmt.Sprintf("%s #%d already unlocked. Nothing changed.\n", - parent.FullName, issuePr.Number) - break - } - err = unlockLockable(httpClient, baseRepo, issuePr, opts) - case Relock: + case state == Lock && issuePr.Locked: relocked, errRelock := relockLockable(httpClient, baseRepo, issuePr, opts) err = errRelock if !relocked { - successMsg = fmt.Sprintf("%s Unchanged: %s #%d (%s)\n", - cs.SuccessIconWithColor(cs.Green), parent.FullName, issuePr.Number, issuePr.Title) + successMsg = fmt.Sprintf("%s #%d already locked%s. Nothing changed.\n", + parent.FullName, issuePr.Number, reason(issuePr.ActiveLockReason)) } + + case state == Unlock && issuePr.Locked: + err = unlockLockable(httpClient, baseRepo, issuePr, opts) + + case state == Unlock && !issuePr.Locked: + successMsg = fmt.Sprintf("%s #%d already unlocked. Nothing changed.\n", + parent.FullName, issuePr.Number) } if err != nil { @@ -279,8 +281,8 @@ func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap var relocked bool shouldRelock := &survey.Confirm{ - Message: fmt.Sprintf("%s #%d locked%s. Unlock and lock again?", - cmds[opts.ParentCmd].FullName, lockable.Number, reason(strings.ToLower(lockable.ActiveLockReason))), + Message: fmt.Sprintf("%s #%d locked%s. Unlock and lock again%s?", + alias[opts.ParentCmd].FullName, lockable.Number, reason(lockable.ActiveLockReason), reason(opts.Reason)), Default: true, } From e0573fa68d0aa42575eff7a9120ede7615164eb3 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 27 Apr 2022 00:43:18 -0400 Subject: [PATCH 15/34] Use `if/else if` on error checking Use switch statements for regular branching code. --- pkg/cmd/issue/lock/lock.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 38e33aab3..874ccf395 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -188,11 +188,9 @@ func padlock(state string, opts *LockOptions) error { parent := alias[opts.ParentCmd] - switch { - case err != nil: + if err != nil { return err - - case parent.Typename != issuePr.Typename: + } else if parent.Typename != issuePr.Typename { return fmt.Errorf("%s #%d not found, but found %s #%d", alias[parent.Typename].FullName, issuePr.Number, strings.ToLower(alias[issuePr.Typename].FullName), issuePr.Number) From c429726b5346cd7afe6c2423e80b42649f95d8ce Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 27 Apr 2022 01:07:43 -0400 Subject: [PATCH 16/34] Polished error message --- pkg/cmd/issue/lock/lock.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 874ccf395..6ac1e8d70 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -191,9 +191,13 @@ func padlock(state string, opts *LockOptions) error { if err != nil { return err } else if parent.Typename != issuePr.Typename { - return fmt.Errorf("%s #%d not found, but found %s #%d", - alias[parent.Typename].FullName, issuePr.Number, - strings.ToLower(alias[issuePr.Typename].FullName), issuePr.Number) + currentType := alias[parent.Typename] + correctType := alias[issuePr.Typename] + + return fmt.Errorf("%s #%d not found, but found %s #%d. Use `gh %s %s %d` instead", + currentType.FullName, issuePr.Number, + correctType.FullName, issuePr.Number, + correctType.Name, strings.ToLower(state), issuePr.Number) } successMsg := fmt.Sprintf("%s %s\n", From 3723ab923028e0d6cffbf8147d3a940275fc1550 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Fri, 29 Apr 2022 00:06:39 -0400 Subject: [PATCH 17/34] Polished things --- pkg/cmd/issue/lock/lock.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 6ac1e8d70..47b0c8bbc 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -71,8 +71,8 @@ var alias map[string]*command = map[string]*command{ // Acceptable lock states for conversations. These are used in print // statements, hence the use of strings instead of booleans. const ( - Unlock string = "Unlock" - Lock string = "Lock" + Lock = "Lock" + Unlock = "Unlock" ) type LockOptions struct { @@ -124,7 +124,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { if !ok { cs := opts.IO.ColorScheme() - return cmdutil.FlagErrorf("%s Invalid reason: %v.\n Aborting lock. See help for options.", + return cmdutil.FlagErrorf("%s Invalid reason: %v\nAborting lock. See help for options.", cs.FailureIconWithColor(cs.Red), opts.Reason) } } @@ -196,7 +196,7 @@ func padlock(state string, opts *LockOptions) error { return fmt.Errorf("%s #%d not found, but found %s #%d. Use `gh %s %s %d` instead", currentType.FullName, issuePr.Number, - correctType.FullName, issuePr.Number, + strings.ToLower(correctType.FullName), issuePr.Number, correctType.Name, strings.ToLower(state), issuePr.Number) } @@ -283,7 +283,7 @@ func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap var relocked bool shouldRelock := &survey.Confirm{ - Message: fmt.Sprintf("%s #%d locked%s. Unlock and lock again%s?", + Message: fmt.Sprintf("%s #%d already locked%s. Unlock and lock again%s?", alias[opts.ParentCmd].FullName, lockable.Number, reason(lockable.ActiveLockReason), reason(opts.Reason)), Default: true, } From 11fde2ee29a99c61d12f396f6953d51809686e1b Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 27 Apr 2022 01:43:35 -0400 Subject: [PATCH 18/34] Restructured switch statement As much as I like keeping statements as flat as possible, this not-as-flat revision just seems easier to read. --- pkg/cmd/issue/lock/lock.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 47b0c8bbc..0af2c45bb 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -203,24 +203,27 @@ func padlock(state string, opts *LockOptions) error { successMsg := fmt.Sprintf("%s %s\n", cs.SuccessIconWithColor(cs.Green), status(state, issuePr, opts)) - switch { - case state == Lock && !issuePr.Locked: - err = lockLockable(httpClient, baseRepo, 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) - case state == Lock && issuePr.Locked: - relocked, errRelock := relockLockable(httpClient, baseRepo, issuePr, opts) - err = errRelock - if !relocked { - successMsg = fmt.Sprintf("%s #%d already locked%s. Nothing changed.\n", - parent.FullName, issuePr.Number, reason(issuePr.ActiveLockReason)) + if !relocked { + successMsg = fmt.Sprintf("%s #%d already locked%s. Nothing changed.\n", + parent.FullName, issuePr.Number, reason(issuePr.ActiveLockReason)) + } } - case state == Unlock && issuePr.Locked: - err = unlockLockable(httpClient, baseRepo, issuePr, opts) - - case state == Unlock && !issuePr.Locked: - successMsg = fmt.Sprintf("%s #%d already unlocked. Nothing changed.\n", - parent.FullName, issuePr.Number) + 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) + } } if err != nil { From 718bb80fbd1b00f376a31e8ad5417e4d82c805c1 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Fri, 29 Apr 2022 00:30:02 -0400 Subject: [PATCH 19/34] Add failure icon --- pkg/cmd/issue/lock/lock.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 0af2c45bb..9f4b6c045 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -194,7 +194,8 @@ func padlock(state string, opts *LockOptions) error { currentType := alias[parent.Typename] correctType := alias[issuePr.Typename] - return fmt.Errorf("%s #%d not found, but found %s #%d. Use `gh %s %s %d` instead", + 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) From cf822e4d9ceb565b767feb8c572295051cf40f39 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 23 Aug 2022 01:35:19 -0400 Subject: [PATCH 20/34] Remove direct calls to graphql library Similar to commit 45f1a71c8bae265ecd9985812e32a6f9d34027c5 --- pkg/cmd/issue/lock/lock.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 9f4b6c045..6edace756 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -11,7 +11,6 @@ package lock import ( - "context" "fmt" "net/http" "strings" @@ -19,12 +18,10 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" - "github.com/cli/cli/v2/internal/ghinstance" "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" - graphql "github.com/cli/shurcooL-graphql" "github.com/shurcooL/githubv4" "github.com/spf13/cobra" ) @@ -254,9 +251,8 @@ func lockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api. }, } - gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) - - return gql.MutateNamed(context.Background(), "LockLockable", &mutation, variables) + gql := api.NewClientFromHTTP(httpClient) + return gql.Mutate(repo.RepoHost(), "LockLockable", &mutation, variables) } // unlockLockable will lock an issue or pull request @@ -276,9 +272,8 @@ func unlockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap }, } - gql := graphql.NewClient(ghinstance.GraphQLEndpoint(repo.RepoHost()), httpClient) - - return gql.MutateNamed(context.Background(), "UnlockLockable", &mutation, variables) + 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 From 2970555cea696d62fba5f25b63779fddd2dd4406 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 23 Aug 2022 14:24:13 -0400 Subject: [PATCH 21/34] Update comments --- pkg/cmd/issue/lock/lock.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 6edace756..0a832e9d0 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -2,7 +2,7 @@ // requests. // // Every pull request is an issue, but not every issue is a pull request. -// Therefore, we should be able to use this in the package cmd/pr as well. +// 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 @@ -26,8 +26,8 @@ import ( "github.com/spf13/cobra" ) -// the reason for not just declaring a map is so that I can put the keys in -// alphabetical order +// A slice is used here instead of a map in order to put options in alphabetical +// order. var reasons = []string{"off_topic", "resolved", "spam", "too_heated"} var reasonsString = strings.Join(reasons, ", ") @@ -55,6 +55,9 @@ type command struct { 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} @@ -158,6 +161,10 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { 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 != "" { @@ -166,6 +173,10 @@ func reason(reason string) string { 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)", @@ -255,7 +266,7 @@ func lockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *api. return gql.Mutate(repo.RepoHost(), "LockLockable", &mutation, variables) } -// unlockLockable will lock an issue or pull request +// 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 { @@ -278,6 +289,10 @@ func unlockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap // 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) { var relocked bool From 4bb6e0d585f71a1f70a55ab2adadff0db2bf92b8 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 23 Aug 2022 14:29:21 -0400 Subject: [PATCH 22/34] Alphabetize fields --- pkg/cmd/issue/lock/lock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 0a832e9d0..cc0db7216 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -99,7 +99,7 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command opts.SelectorArg = args[0] opts.Fields = []string{ - "id", "number", "title", "url", "locked", "activeLockReason"} + "activeLockReason", "id", "locked", "number", "title", "url"} } From 077ab39738e694d1f9471c78ff1c62074e3a1346 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 23 Aug 2022 23:16:15 -0400 Subject: [PATCH 23/34] Implicitly return the nil value when no lock reason given While I usually like explicitly setting all values, getting rid of setting the empty string streamlines map construction and testing. The reasonsMap will return the nil value because the empty string is not in the map. --- pkg/cmd/issue/lock/lock.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index cc0db7216..c81cc8090 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -26,9 +26,12 @@ import ( "github.com/spf13/cobra" ) -// A slice is used here instead of a map in order to put options in alphabetical -// order. +// 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{ @@ -37,6 +40,10 @@ var reasonsApi = []githubv4.LockReason{ 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() { @@ -44,9 +51,6 @@ func init() { for i, reason := range reasons { reasonsMap[reason] = &reasonsApi[i] } - - // If no reason given, set lock_reason to null in graphql - reasonsMap[""] = nil } type command struct { From 7a5ed081858e59ad98c71e4c80f82ca91845bab2 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Tue, 23 Aug 2022 23:24:42 -0400 Subject: [PATCH 24/34] Test reasonsMap construction Make sure reasons were added in the correct order. --- pkg/cmd/issue/lock/lock_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index 371688338..1544f6293 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -1 +1,16 @@ package lock + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +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])) + } +} From 25209ad6b1b32cce64d9a913b2fdfdd7d6fa90e7 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Thu, 8 Dec 2022 20:13:48 +0000 Subject: [PATCH 25/34] Add `--allow-update-branch` to the `repo edit` command --- pkg/cmd/repo/edit/edit.go | 2 ++ pkg/cmd/repo/edit/edit_test.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) 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 +} From b52d452168e3a278e72f80f656c2ee046daa87b4 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 16 Dec 2022 13:01:09 -0800 Subject: [PATCH 26/34] wip: tweak output, update to Prompter, start on tests --- pkg/cmd/issue/issue.go | 4 +- pkg/cmd/issue/lock/lock.go | 66 ++++++++++++++++++++------------- pkg/cmd/issue/lock/lock_test.go | 62 +++++++++++++++++++++++++++++++ pkg/cmd/pr/pr.go | 4 +- 4 files changed, 107 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index a02b7f7a3..03afd47f8 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -45,10 +45,10 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) cmd.AddCommand(cmdList.NewCmdList(f, nil)) - cmd.AddCommand(cmdLock.NewCmdLock(f, cmd.Name())) + cmd.AddCommand(cmdLock.NewCmdLock(f, cmd.Name(), nil)) cmd.AddCommand(cmdReopen.NewCmdReopen(f, nil)) cmd.AddCommand(cmdStatus.NewCmdStatus(f, nil)) - cmd.AddCommand(cmdLock.NewCmdUnlock(f, cmd.Name())) + cmd.AddCommand(cmdLock.NewCmdUnlock(f, cmd.Name(), nil)) cmd.AddCommand(cmdView.NewCmdView(f, nil)) cmd.AddCommand(cmdComment.NewCmdComment(f, nil)) cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil)) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index c81cc8090..0b8e4c6ed 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -11,11 +11,11 @@ package lock import ( + "errors" "fmt" "net/http" "strings" - "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/v2/api" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" @@ -26,6 +26,13 @@ import ( "github.com/spf13/cobra" ) +type iprompter interface { + Confirm(string, bool) (bool, error) +} + +// TODO cmd tests +// TODO run tests + // reasons contains all possible lock reasons allowed by GitHub. // // We don't directly construct a map so that we can maintain the reasons in @@ -84,6 +91,7 @@ type LockOptions struct { Config func() (config.Config, error) IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) + Prompter iprompter Fields []string ParentCmd string @@ -92,7 +100,6 @@ type LockOptions struct { } func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, args []string) { - opts.IO = f.IOStreams opts.HttpClient = f.HttpClient opts.Config = f.Config @@ -107,10 +114,11 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command } -func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { - +func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command { opts := &LockOptions{ParentCmd: parentName} + opts.Prompter = f.Prompter + c := alias[opts.ParentCmd] short := fmt.Sprintf("Lock %s conversation", strings.ToLower(c.FullName)) @@ -119,21 +127,28 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { Short: short, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - opts.setCommonOptions(f, cmd, args) reasonProvided := cmd.Flags().Changed("message") if reasonProvided { _, ok := reasonsMap[opts.Reason] if !ok { - cs := opts.IO.ColorScheme() + if opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() - return cmdutil.FlagErrorf("%s Invalid reason: %v\nAborting lock. See help for options.", - cs.FailureIconWithColor(cs.Red), opts.Reason) + return cmdutil.FlagErrorf("%s Invalid reason: %v\nAborting lock. See help for options.", + cs.FailureIconWithColor(cs.Red), opts.Reason) + + } else { + return fmt.Errorf("invalid reason %s", opts.Reason) + } } } - return padlock(Lock, opts) + if runF != nil { + return runF(Lock, opts) + } + return lock(Lock, opts) }, } @@ -143,8 +158,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string) *cobra.Command { return cmd } -func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { - +func NewCmdUnlock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command { opts := &LockOptions{ParentCmd: parentName} c := alias[opts.ParentCmd] @@ -158,7 +172,10 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string) *cobra.Command { opts.setCommonOptions(f, cmd, args) - return padlock(Unlock, opts) + if runF != nil { + return runF(Unlock, opts) + } + return lock(Unlock, opts) }, } @@ -182,13 +199,12 @@ func reason(reason string) string { // // 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) } -// padlock will lock or unlock a conversation. -func padlock(state string, opts *LockOptions) error { +// lock will lock or unlock a conversation. +func lock(state string, opts *LockOptions) error { cs := opts.IO.ColorScheme() httpClient, err := opts.HttpClient() @@ -243,14 +259,15 @@ func padlock(state string, opts *LockOptions) error { return err } - fmt.Fprint(opts.IO.ErrOut, successMsg) + if opts.IO.IsStdoutTTY() { + fmt.Fprint(opts.IO.ErrOut, 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 { @@ -298,17 +315,16 @@ func unlockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap // 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) { - - var relocked bool - shouldRelock := &survey.Confirm{ - Message: fmt.Sprintf("%s #%d already locked%s. Unlock and lock again%s?", - alias[opts.ParentCmd].FullName, lockable.Number, reason(lockable.ActiveLockReason), reason(opts.Reason)), - Default: true, + if !opts.IO.CanPrompt() { + return false, errors.New("already locked") } - err := survey.AskOne(shouldRelock, &relocked) + 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 relocked, err + return false, err } else if !relocked { return relocked, nil } diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index 1544f6293..ad9d8735d 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -1,12 +1,74 @@ package lock import ( + "bytes" + "io" "strings" "testing" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "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 + }{ + // TODO + } + + 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) + }) + } +} + +func Test_lock(t *testing.T) { + // TODO +} + +func Test_NewCmdUnlock(t *testing.T) { + // TODO +} + func TestReasons(t *testing.T) { assert.Equal(t, len(reasons), len(reasonsApi)) diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index d5265f405..eea93d4e9 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -49,13 +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())) + 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())) + 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)) From 4c28e32e9d9e33dbc5c402b4c0204dce53b816e0 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 19 Dec 2022 17:57:04 -0800 Subject: [PATCH 27/34] WIP on adding tests, add prompt, tweak things --- pkg/cmd/issue/lock/lock.go | 44 ++++++++------ pkg/cmd/issue/lock/lock_test.go | 101 ++++++++++++++++++++++++++++++-- 2 files changed, 124 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 0b8e4c6ed..2d33b78f8 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -28,11 +28,9 @@ import ( type iprompter interface { Confirm(string, bool) (bool, error) + Select(string, string, []string) (int, error) } -// TODO cmd tests -// TODO run tests - // reasons contains all possible lock reasons allowed by GitHub. // // We don't directly construct a map so that we can maintain the reasons in @@ -97,6 +95,7 @@ type LockOptions struct { ParentCmd string Reason string SelectorArg string + Interactive bool } func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command, args []string) { @@ -111,13 +110,13 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command opts.Fields = []string{ "activeLockReason", "id", "locked", "number", "title", "url"} - } func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command { - opts := &LockOptions{ParentCmd: parentName} - - opts.Prompter = f.Prompter + opts := &LockOptions{ + ParentCmd: parentName, + Prompter: f.Prompter, + } c := alias[opts.ParentCmd] short := fmt.Sprintf("Lock %s conversation", strings.ToLower(c.FullName)) @@ -129,32 +128,33 @@ func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOp RunE: func(cmd *cobra.Command, args []string) error { opts.setCommonOptions(f, cmd, args) - reasonProvided := cmd.Flags().Changed("message") + 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\nAborting lock. See help for options.", + 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 lock(Lock, opts) + return runLock(Lock, opts) }, } - msg := fmt.Sprintf("Optional reason for locking conversation. Must be one of the following: %v.", reasonsString) + msg := fmt.Sprintf("Optional reason for locking conversation (%v).", reasonsString) - cmd.Flags().StringVarP(&opts.Reason, "message", "m", "", msg) + cmd.Flags().StringVarP(&opts.Reason, "reason", "r", "", msg) return cmd } @@ -169,13 +169,12 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string, runF func(string, *Lock 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 lock(Unlock, opts) + return runLock(Unlock, opts) }, } @@ -203,8 +202,8 @@ func status(state string, lockable *api.Issue, opts *LockOptions) string { state, reason(opts.Reason), alias[opts.ParentCmd].FullName, lockable.Number, lockable.Title) } -// lock will lock or unlock a conversation. -func lock(state string, opts *LockOptions) error { +// runLock will lock or unlock a conversation. +func runLock(state string, opts *LockOptions) error { cs := opts.IO.ColorScheme() httpClient, err := opts.HttpClient() @@ -229,6 +228,17 @@ func lock(state string, opts *LockOptions) error { 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)) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index ad9d8735d..f4145173d 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -20,7 +20,45 @@ func Test_NewCmdLock(t *testing.T) { wantErr string tty bool }{ - // TODO + { + 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 { @@ -57,15 +95,70 @@ func Test_NewCmdLock(t *testing.T) { 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_lock(t *testing.T) { - // TODO +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_NewCmdUnlock(t *testing.T) { +func Test_runLock(t *testing.T) { // TODO } From 95419f9006ac574c60c1be8500f6b81c80a8fa43 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 19 Dec 2022 18:12:08 -0800 Subject: [PATCH 28/34] further WIP --- pkg/cmd/issue/lock/lock.go | 8 ++-- pkg/cmd/issue/lock/lock_test.go | 67 ++++++++++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 2d33b78f8..4b8beec41 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -148,7 +148,7 @@ func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOp if runF != nil { return runF(Lock, opts) } - return runLock(Lock, opts) + return lockRun(Lock, opts) }, } @@ -174,7 +174,7 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string, runF func(string, *Lock if runF != nil { return runF(Unlock, opts) } - return runLock(Unlock, opts) + return lockRun(Unlock, opts) }, } @@ -202,8 +202,8 @@ func status(state string, lockable *api.Issue, opts *LockOptions) string { state, reason(opts.Reason), alias[opts.ParentCmd].FullName, lockable.Number, lockable.Title) } -// runLock will lock or unlock a conversation. -func runLock(state string, opts *LockOptions) error { +// lockRun will lock or unlock a conversation. +func lockRun(state string, opts *LockOptions) error { cs := opts.IO.ColorScheme() httpClient, err := opts.HttpClient() diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index f4145173d..02ca85bf3 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -3,16 +3,21 @@ package lock import ( "bytes" "io" + "net/http" "strings" "testing" + "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) { + // TODO parent name stuff? cases := []struct { name string args string @@ -101,6 +106,7 @@ func Test_NewCmdLock(t *testing.T) { } func Test_NewCmdUnlock(t *testing.T) { + // TODO parent name stuff? cases := []struct { name string args string @@ -159,7 +165,66 @@ func Test_NewCmdUnlock(t *testing.T) { } func Test_runLock(t *testing.T) { - // TODO + 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 an issue", + // TODO + }, + // TODO + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + reg.StubRepoInfoResponse("OWNER", "REPO", "trunk") + 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() + // TODO do i need to bother with this + ios.SetStdoutTTY(tt.tty) + ios.SetStdinTTY(tt.tty) + ios.SetStderrTTY(tt.tty) + + opts := LockOptions{ + Prompter: pm, + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + } + + err := lockRun(tt.state, &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) { From cee5e6e42a4c9b774e13dce5f176d9df7e50d0c5 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 20 Dec 2022 13:00:52 -0800 Subject: [PATCH 29/34] WIP on tests --- pkg/cmd/issue/lock/lock.go | 15 ++++-- pkg/cmd/issue/lock/lock_test.go | 84 +++++++++++++++++++++++++++++---- 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index 4b8beec41..a5600958d 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -84,6 +84,12 @@ const ( 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) @@ -91,7 +97,6 @@ type LockOptions struct { BaseRepo func() (ghrepo.Interface, error) Prompter iprompter - Fields []string ParentCmd string Reason string SelectorArg string @@ -108,8 +113,6 @@ func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, cmd *cobra.Command opts.SelectorArg = args[0] - opts.Fields = []string{ - "activeLockReason", "id", "locked", "number", "title", "url"} } func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command { @@ -211,7 +214,7 @@ func lockRun(state string, opts *LockOptions) error { return err } - issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, opts.Fields) + issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, fields()) parent := alias[opts.ParentCmd] @@ -263,6 +266,8 @@ func lockRun(state string, opts *LockOptions) error { successMsg = fmt.Sprintf("%s #%d already unlocked. Nothing changed.\n", parent.FullName, issuePr.Number) } + default: + panic("bad state") } if err != nil { @@ -270,7 +275,7 @@ func lockRun(state string, opts *LockOptions) error { } if opts.IO.IsStdoutTTY() { - fmt.Fprint(opts.IO.ErrOut, successMsg) + fmt.Fprint(opts.IO.Out, successMsg) } return nil diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index 02ca85bf3..a4a7c10e6 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -2,11 +2,13 @@ package lock import ( "bytes" + "fmt" "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" @@ -177,16 +179,76 @@ func Test_runLock(t *testing.T) { state string }{ { - name: "lock an issue", + name: "lock an issue", + 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, + "url": "https://github.com/OWNER/REPO/issues/451", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation LockLockable\b`), + httpmock.StringResponse(` + { "data": { + "lockLockable": { + "lockedRecord": { + "locked": true }}}}`)) + }, + }, + { + name: "lock an 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, + "url": "https://github.com/OWNER/REPO/issues/451", + "__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?" { + return prompter.IndexFor(opts, "Too heated") + } + + return -1, prompter.NoSuchPromptErr(p) + } + }, + wantOut: "✓ Locked as TOO_HEATED: Issue #451 ()\n", + // TODO lock tty + // TODO lock with explicit reason + // TODO relock + // TODO unlock + // TODO }, - // TODO } for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} - reg.StubRepoInfoResponse("OWNER", "REPO", "trunk") defer reg.Verify(t) if tt.httpStubs != nil { tt.httpStubs(t, reg) @@ -203,19 +265,21 @@ func Test_runLock(t *testing.T) { ios.SetStdinTTY(tt.tty) ios.SetStderrTTY(tt.tty) - opts := LockOptions{ - Prompter: pm, - IO: ios, - HttpClient: func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - }, + 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, &opts) + err := lockRun(tt.state, &tt.opts) output := &test.CmdOut{ OutBuf: stdout, ErrBuf: stderr, } + fmt.Printf("DBG %#v\n", reg) if tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) } else { From bb958490ea6391ccccb8cf9d546b632c56b3699c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 20 Dec 2022 13:14:41 -0800 Subject: [PATCH 30/34] WIP tests --- pkg/cmd/issue/lock/lock_test.go | 72 ++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index a4a7c10e6..5a916b29e 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -2,7 +2,6 @@ package lock import ( "bytes" - "fmt" "io" "net/http" "strings" @@ -179,7 +178,7 @@ func Test_runLock(t *testing.T) { state string }{ { - name: "lock an issue", + name: "lock an issue nontty", state: "Lock", opts: LockOptions{ SelectorArg: "451", @@ -217,6 +216,7 @@ func Test_runLock(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 451, + "title": "traverse the library", "url": "https://github.com/OWNER/REPO/issues/451", "__typename": "Issue" }}}}`)) reg.Register( @@ -236,14 +236,67 @@ func Test_runLock(t *testing.T) { return -1, prompter.NoSuchPromptErr(p) } }, - wantOut: "✓ Locked as TOO_HEATED: Issue #451 ()\n", - // TODO lock tty - // TODO lock with explicit reason - // TODO relock - // TODO unlock - - // TODO + wantOut: "✓ Locked as TOO_HEATED: Issue #451 (traverse the library)\n", }, + { + name: "lock an issue with explicit reason", + 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", + "url": "https://github.com/OWNER/REPO/issues/451", + "__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: 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", + "url": "https://github.com/OWNER/REPO/issues/451", + "__typename": "Issue" }}}}`)) + reg.Register( + httpmock.GraphQL(`mutation UnlockLockable\b`), + httpmock.StringResponse(` + { "data": { + "unlockLockable": { + "unlockedRecord": { + "locked": false }}}}`)) + }, + wantOut: "✓ Unlocked: Issue #451 (traverse the library)\n", + }, + // TODO lock with explicit nontty + // TODO relock + // TODO unlock nontty } for _, tt := range cases { @@ -279,7 +332,6 @@ func Test_runLock(t *testing.T) { OutBuf: stdout, ErrBuf: stderr, } - fmt.Printf("DBG %#v\n", reg) if tt.wantErr != "" { assert.EqualError(t, err, tt.wantErr) } else { From a1fe708048c0177f210deca174ea938aae037e9a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 20 Dec 2022 13:15:14 -0800 Subject: [PATCH 31/34] todos --- pkg/cmd/issue/lock/lock_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index 5a916b29e..5e3bd90b0 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -297,6 +297,8 @@ func Test_runLock(t *testing.T) { // TODO lock with explicit nontty // TODO relock // TODO unlock nontty + + // TODO all of the above but with pull requests } for _, tt := range cases { @@ -313,7 +315,6 @@ func Test_runLock(t *testing.T) { } ios, _, stdout, stderr := iostreams.Test() - // TODO do i need to bother with this ios.SetStdoutTTY(tt.tty) ios.SetStdinTTY(tt.tty) ios.SetStderrTTY(tt.tty) From 8d9b47e5556475992ae8e827a789daff265d95ef Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 20 Dec 2022 15:20:21 -0800 Subject: [PATCH 32/34] stub out rest of tests --- pkg/cmd/issue/lock/lock_test.go | 66 ++++++++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index 5e3bd90b0..756b7f634 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -179,7 +179,7 @@ func Test_runLock(t *testing.T) { }{ { name: "lock an issue nontty", - state: "Lock", + state: Lock, opts: LockOptions{ SelectorArg: "451", ParentCmd: "issue", @@ -209,7 +209,7 @@ func Test_runLock(t *testing.T) { SelectorArg: "451", ParentCmd: "issue", }, - state: "Lock", + state: Lock, httpStubs: func(t *testing.T, reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`query IssueByNumber\b`), @@ -241,7 +241,7 @@ func Test_runLock(t *testing.T) { { name: "lock an issue with explicit reason", tty: true, - state: "Lock", + state: Lock, opts: LockOptions{ SelectorArg: "451", ParentCmd: "issue", @@ -267,9 +267,9 @@ func Test_runLock(t *testing.T) { wantOut: "✓ Locked as OFF_TOPIC: Issue #451 (traverse the library)\n", }, { - name: "unlock issue", + name: "unlock issue tty", tty: true, - state: "Unlock", + state: Unlock, opts: LockOptions{ SelectorArg: "451", ParentCmd: "issue", @@ -294,11 +294,59 @@ func Test_runLock(t *testing.T) { }, wantOut: "✓ Unlocked: Issue #451 (traverse the library)\n", }, - // TODO lock with explicit nontty - // TODO relock - // TODO unlock nontty + { + name: "unlock issue nontty", + state: Lock, + // TODO + }, + { + name: "lock issue with explicit nontty", + state: Lock, + // TODO + }, + { + name: "relock issue tty", + state: Lock, + // TODO + }, + { + name: "relock issue nontty", + state: Lock, + // TODO + }, - // TODO all of the above but with pull requests + { + name: "lock pr nontty", + // TODO + }, + { + name: "lock pr tty", + // TODO + }, + { + name: "lock pr with explicit reason", + // TODO + }, + { + name: "lock pr with explicit nontty", + // TODO + }, + { + name: "unlock pr tty", + // TODO + }, + { + name: "unlock pr nontty", + // TODO + }, + { + name: "relock pr tty", + // TODO + }, + { + name: "relock pr nontty", + // TODO + }, } for _, tt := range cases { From 6fce78e689b46226046ff6c48b6a01df4008bff8 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 20 Dec 2022 15:34:14 -0800 Subject: [PATCH 33/34] WIP all but relocking --- pkg/cmd/issue/lock/lock_test.go | 219 ++++++++++++++++++++++++++++---- 1 file changed, 197 insertions(+), 22 deletions(-) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index 756b7f634..eef6ac6b8 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -178,7 +178,7 @@ func Test_runLock(t *testing.T) { state string }{ { - name: "lock an issue nontty", + name: "lock issue nontty", state: Lock, opts: LockOptions{ SelectorArg: "451", @@ -190,7 +190,6 @@ func Test_runLock(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 451, - "url": "https://github.com/OWNER/REPO/issues/451", "__typename": "Issue" }}}}`)) reg.Register( httpmock.GraphQL(`mutation LockLockable\b`), @@ -202,7 +201,7 @@ func Test_runLock(t *testing.T) { }, }, { - name: "lock an issue tty", + name: "lock issue tty", tty: true, opts: LockOptions{ Interactive: true, @@ -217,7 +216,6 @@ func Test_runLock(t *testing.T) { { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 451, "title": "traverse the library", - "url": "https://github.com/OWNER/REPO/issues/451", "__typename": "Issue" }}}}`)) reg.Register( httpmock.GraphQL(`mutation LockLockable\b`), @@ -239,7 +237,7 @@ func Test_runLock(t *testing.T) { wantOut: "✓ Locked as TOO_HEATED: Issue #451 (traverse the library)\n", }, { - name: "lock an issue with explicit reason", + name: "lock issue with explicit reason tty", tty: true, state: Lock, opts: LockOptions{ @@ -254,7 +252,6 @@ func Test_runLock(t *testing.T) { { "data": { "repository": { "hasIssuesEnabled": true, "issue": { "number": 451, "title": "traverse the library", - "url": "https://github.com/OWNER/REPO/issues/451", "__typename": "Issue" }}}}`)) reg.Register( httpmock.GraphQL(`mutation LockLockable\b`), @@ -282,7 +279,6 @@ func Test_runLock(t *testing.T) { "number": 451, "locked": true, "title": "traverse the library", - "url": "https://github.com/OWNER/REPO/issues/451", "__typename": "Issue" }}}}`)) reg.Register( httpmock.GraphQL(`mutation UnlockLockable\b`), @@ -296,13 +292,53 @@ func Test_runLock(t *testing.T) { }, { name: "unlock issue nontty", - state: Lock, - // TODO + 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 nontty", + name: "lock issue with explicit reason nontty", state: Lock, - // TODO + 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", @@ -316,28 +352,167 @@ func Test_runLock(t *testing.T) { }, { - name: "lock pr nontty", - // TODO + 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", - // TODO + 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", - // TODO + 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", - // TODO + 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", - // TODO + 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", - // TODO + 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", From c3cceae9ea3b753f7218ab94f28452c29bc49958 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 20 Dec 2022 15:44:29 -0800 Subject: [PATCH 34/34] finish tests --- pkg/cmd/issue/lock/lock.go | 2 +- pkg/cmd/issue/lock/lock_test.go | 126 ++++++++++++++++++++++++++++++-- 2 files changed, 119 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/issue/lock/lock.go b/pkg/cmd/issue/lock/lock.go index a5600958d..e8329974d 100644 --- a/pkg/cmd/issue/lock/lock.go +++ b/pkg/cmd/issue/lock/lock.go @@ -334,7 +334,7 @@ func relockLockable(httpClient *http.Client, repo ghrepo.Interface, lockable *ap return false, errors.New("already locked") } - prompt := fmt.Sprintf("%s #%d already locked%s. Unlock and lock again%s?", + 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) diff --git a/pkg/cmd/issue/lock/lock_test.go b/pkg/cmd/issue/lock/lock_test.go index eef6ac6b8..c6eb902ef 100644 --- a/pkg/cmd/issue/lock/lock_test.go +++ b/pkg/cmd/issue/lock/lock_test.go @@ -18,7 +18,6 @@ import ( ) func Test_NewCmdLock(t *testing.T) { - // TODO parent name stuff? cases := []struct { name string args string @@ -107,7 +106,6 @@ func Test_NewCmdLock(t *testing.T) { } func Test_NewCmdUnlock(t *testing.T) { - // TODO parent name stuff? cases := []struct { name string args string @@ -228,6 +226,8 @@ func Test_runLock(t *testing.T) { 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") } @@ -343,12 +343,66 @@ func Test_runLock(t *testing.T) { { name: "relock issue tty", state: Lock, - // TODO + 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, - // TODO + 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", }, { @@ -515,12 +569,68 @@ func Test_runLock(t *testing.T) { wantOut: "✓ Unlocked: Pull request #451 (traverse the library)\n", }, { - name: "relock pr tty", - // TODO + 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", - // TODO + 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", }, }