From 28c2d042e7862ee79a15b01d8d49becb460fd11e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 22 Jan 2021 21:53:50 +0100 Subject: [PATCH] Extend `@me` replacing behavior to `issue list` --- pkg/cmd/issue/create/create.go | 14 ++++---- pkg/cmd/issue/list/list.go | 23 ++++++++++--- pkg/cmd/issue/list/list_test.go | 25 ++++++++++++++ pkg/cmd/pr/create/create.go | 8 ++++- pkg/cmd/pr/shared/params.go | 57 +++++++++++++++++++++++--------- pkg/cmd/pr/shared/params_test.go | 7 ++-- 6 files changed, 104 insertions(+), 30 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index df768ff03..91d58d763 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" prShared "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -115,9 +116,15 @@ func createRun(opts *CreateOptions) (err error) { milestones = []string{opts.Milestone} } + meReplacer := shared.NewMeReplacer(apiClient, baseRepo.RepoHost()) + assignees, err := meReplacer.ReplaceSlice(opts.Assignees) + if err != nil { + return err + } + tb := prShared.IssueMetadataState{ Type: prShared.IssueMetadata, - Assignees: opts.Assignees, + Assignees: assignees, Labels: opts.Labels, Projects: opts.Projects, Milestones: milestones, @@ -133,11 +140,6 @@ func createRun(opts *CreateOptions) (err error) { } } - opts.Assignees, err = prShared.ReplaceAtMeLogin(opts.Assignees, apiClient, baseRepo) - if err != nil { - return err - } - if opts.WebMode { openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") if opts.Title != "" || opts.Body != "" || len(opts.Assignees) > 0 { diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index 017556af6..7be8d9aa1 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" issueShared "github.com/cli/cli/pkg/cmd/issue/shared" + "github.com/cli/cli/pkg/cmd/pr/shared" prShared "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -91,15 +92,29 @@ func listRun(opts *ListOptions) error { isTerminal := opts.IO.IsStdoutTTY() + meReplacer := shared.NewMeReplacer(apiClient, baseRepo.RepoHost()) + filterAssignee, err := meReplacer.Replace(opts.Assignee) + if err != nil { + return err + } + filterAuthor, err := meReplacer.Replace(opts.Author) + if err != nil { + return err + } + filterMention, err := meReplacer.Replace(opts.Mention) + if err != nil { + return err + } + if opts.WebMode { issueListURL := ghrepo.GenerateRepoURL(baseRepo, "issues") openURL, err := prShared.ListURLWithQuery(issueListURL, prShared.FilterOptions{ Entity: "issue", State: opts.State, - Assignee: opts.Assignee, + Assignee: filterAssignee, Labels: opts.Labels, - Author: opts.Author, - Mention: opts.Mention, + Author: filterAuthor, + Mention: filterMention, Milestone: opts.Milestone, }) if err != nil { @@ -111,7 +126,7 @@ func listRun(opts *ListOptions) error { return utils.OpenInBrowser(openURL) } - listResult, err := api.IssueList(apiClient, baseRepo, opts.State, opts.Labels, opts.Assignee, opts.LimitResults, opts.Author, opts.Mention, opts.Milestone) + listResult, err := api.IssueList(apiClient, baseRepo, opts.State, opts.Labels, filterAssignee, opts.LimitResults, filterAuthor, filterMention, opts.Milestone) if err != nil { return err } diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 59952cda8..003cb4407 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -147,6 +147,31 @@ No issues match your search in OWNER/REPO `, output.String()) } +func TestIssueList_atMe(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "monalisa"} } }`)) + http.Register( + httpmock.GraphQL(`query IssueList\b`), + httpmock.GraphQLQuery(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { "nodes": [] } + } } }`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "monalisa", params["assignee"].(string)) + assert.Equal(t, "monalisa", params["author"].(string)) + assert.Equal(t, "monalisa", params["mention"].(string)) + })) + + _, err := runCommand(http, true, "-a @me -A @me --mention @me") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } +} + func TestIssueList_withInvalidLimitFlag(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 6c7e98627..25e34c9e4 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -386,10 +386,16 @@ func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadata milestoneTitles = []string{opts.Milestone} } + meReplacer := shared.NewMeReplacer(ctx.Client, ctx.BaseRepo.RepoHost()) + assignees, err := meReplacer.ReplaceSlice(opts.Assignees) + if err != nil { + return nil, err + } + state := &shared.IssueMetadataState{ Type: shared.PRMetadata, Reviewers: opts.Reviewers, - Assignees: opts.Assignees, + Assignees: assignees, Labels: opts.Labels, Projects: opts.Projects, Milestones: milestoneTitles, diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index b20ccd223..722a981d7 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -81,11 +81,6 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par return nil } - var err error - if tb.Assignees, err = ReplaceAtMeLogin(tb.Assignees, client, baseRepo); err != nil { - return err - } - if err := fillMetadata(client, baseRepo, tb); err != nil { return err } @@ -196,17 +191,47 @@ func quoteValueForQuery(v string) string { return v } -// ReplaceAtMeLogin iterates over the list of specified login names, replacing -// any "@me" mentions with the current user LoginName. -func ReplaceAtMeLogin(logins []string, client *api.Client, repo ghrepo.Interface) ([]string, error) { - for i, u := range logins { - if strings.EqualFold(u, "@me") { - login, err := api.CurrentLoginName(client, repo.RepoHost()) - if err != nil { - return logins, fmt.Errorf("@me resolve: failed obtaining user id: %w", err) - } - logins[i] = login +// MeReplacer resolves usages of `@me` to the handle of the currently logged in user. +type MeReplacer struct { + apiClient *api.Client + hostname string + login string +} + +func NewMeReplacer(apiClient *api.Client, hostname string) *MeReplacer { + return &MeReplacer{ + apiClient: apiClient, + hostname: hostname, + } +} + +func (r *MeReplacer) currentLogin() (string, error) { + if r.login != "" { + return r.login, nil + } + login, err := api.CurrentLoginName(r.apiClient, r.hostname) + if err != nil { + return "", fmt.Errorf("failed resolving `@me` to your user handle: %w", err) + } + r.login = login + return login, nil +} + +func (r *MeReplacer) Replace(handle string) (string, error) { + if handle == "@me" { + return r.currentLogin() + } + return handle, nil +} + +func (r *MeReplacer) ReplaceSlice(handles []string) ([]string, error) { + res := make([]string, len(handles)) + for i, h := range handles { + var err error + res[i], err = r.Replace(h) + if err != nil { + return nil, err } } - return logins, nil + return res, nil } diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 3d7a75541..fa5b9f47a 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -78,7 +78,7 @@ func Test_listURLWithQuery(t *testing.T) { } } -func TestReplaceAtMeLogin(t *testing.T) { +func TestMeReplacer_Replace(t *testing.T) { rtSuccess := &httpmock.Registry{} rtSuccess.Register( httpmock.GraphQL(`query UserCurrent\b`), @@ -129,13 +129,14 @@ func TestReplaceAtMeLogin(t *testing.T) { logins: []string{"some", "@me", "other"}, }, verify: rtFailure.Verify, - want: []string{"some", "@me", "other"}, + want: []string(nil), wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ReplaceAtMeLogin(tt.args.logins, tt.args.client, tt.args.repo) + me := NewMeReplacer(tt.args.client, tt.args.repo.RepoHost()) + got, err := me.ReplaceSlice(tt.args.logins) if (err != nil) != tt.wantErr { t.Errorf("ReplaceAtMeLogin() error = %v, wantErr %v", err, tt.wantErr) return