From c843a4fa13a206db8331b52f93305c6a19b955ab Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Fri, 13 Nov 2020 11:16:54 +0300 Subject: [PATCH] Add issue comment viewing --- api/queries_issue.go | 57 ++- api/reaction_groups.go | 52 +++ api/reaction_groups_test.go | 55 +++ pkg/cmd/issue/shared/lookup.go | 51 +-- .../view/fixtures/issueView_preview.json | 8 +- .../issueView_previewClosedState.json | 2 +- .../issueView_previewFullComments.json | 383 ++++++++++++++++++ .../issueView_previewSingleComment.json | 147 +++++++ .../issueView_previewThreeComments.json | 265 ++++++++++++ .../issueView_previewWithEmptyBody.json | 2 +- .../issueView_previewWithMetadata.json | 60 ++- pkg/cmd/issue/view/view.go | 138 ++++++- pkg/cmd/issue/view/view_test.go | 210 +++++++++- pkg/iostreams/color.go | 24 +- utils/utils.go | 16 + utils/utils_test.go | 27 +- 16 files changed, 1408 insertions(+), 89 deletions(-) create mode 100644 api/reaction_groups.go create mode 100644 api/reaction_groups_test.go create mode 100644 pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json create mode 100644 pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json create mode 100644 pkg/cmd/issue/view/fixtures/issueView_previewThreeComments.json diff --git a/api/queries_issue.go b/api/queries_issue.go index 08e0cf1d9..1aa83ad89 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -33,12 +33,8 @@ type Issue struct { Body string CreatedAt time.Time UpdatedAt time.Time - Comments struct { - TotalCount int - } - Author struct { - Login string - } + Comments IssueComments + Author Author Assignees struct { Nodes []struct { Login string @@ -65,12 +61,31 @@ type Issue struct { Milestone struct { Title string } + ReactionGroups ReactionGroups } type IssuesDisabledError struct { error } +type IssueComments struct { + Nodes []IssueComment + TotalCount int +} + +type IssueComment struct { + Author Author + AuthorAssociation string + Body string + CreatedAt time.Time + IncludesCreatedEdit bool + ReactionGroups ReactionGroups +} + +type Author struct { + Login string +} + const fragments = ` fragment issue on Issue { number @@ -320,7 +335,7 @@ loop: return &res, nil } -func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, error) { +func IssueByNumber(client *Client, repo ghrepo.Interface, number, comments int) (*Issue, error) { type response struct { Repository struct { Issue Issue @@ -329,7 +344,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } query := ` - query IssueByNumber($owner: String!, $repo: String!, $issue_number: Int!) { + query IssueByNumber($owner: String!, $repo: String!, $issue_number: Int!, $comments: Int!) { repository(owner: $owner, name: $repo) { hasIssuesEnabled issue(number: $issue_number) { @@ -341,7 +356,22 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e author { login } - comments { + comments(last: $comments) { + nodes { + author { + login + } + authorAssociation + body + createdAt + includesCreatedEdit + reactionGroups { + content + users { + totalCount + } + } + } totalCount } number @@ -370,9 +400,15 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } totalCount } - milestone{ + milestone { title } + reactionGroups { + content + users { + totalCount + } + } } } }` @@ -381,6 +417,7 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e "owner": repo.RepoOwner(), "repo": repo.RepoName(), "issue_number": number, + "comments": comments, } var resp response diff --git a/api/reaction_groups.go b/api/reaction_groups.go new file mode 100644 index 000000000..68c527079 --- /dev/null +++ b/api/reaction_groups.go @@ -0,0 +1,52 @@ +package api + +import ( + "fmt" + "strings" +) + +type ReactionGroups []ReactionGroup + +type ReactionGroup struct { + Content string + Users ReactionGroupUsers +} + +type ReactionGroupUsers struct { + TotalCount int +} + +func (rg ReactionGroup) String() string { + c := rg.Users.TotalCount + if c == 0 { + return "" + } + e := reactionEmoji[rg.Content] + if e == "" { + return "" + } + return fmt.Sprintf("%v %s", c, e) +} + +func (rgs ReactionGroups) String() string { + var rs []string + + for _, rg := range rgs { + if r := rg.String(); r != "" { + rs = append(rs, r) + } + } + + return strings.Join(rs, " • ") +} + +var reactionEmoji = map[string]string{ + "THUMBS_UP": "\U0001f44d", + "THUMBS_DOWN": "\U0001f44e", + "LAUGH": "\U0001f604", + "HOORAY": "\U0001f389", + "CONFUSED": "\U0001f615", + "HEART": "\u2764\ufe0f", + "ROCKET": "\U0001f680", + "EYES": "\U0001f440", +} diff --git a/api/reaction_groups_test.go b/api/reaction_groups_test.go new file mode 100644 index 000000000..f2fa7b1d4 --- /dev/null +++ b/api/reaction_groups_test.go @@ -0,0 +1,55 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_String(t *testing.T) { + tests := map[string]struct { + rgs ReactionGroups + output string + }{ + "empty reaction groups": { + rgs: []ReactionGroup{}, + output: `^$`, + }, + "non-empty reaction groups": { + rgs: []ReactionGroup{ + ReactionGroup{ + Content: "LAUGH", + Users: ReactionGroupUsers{TotalCount: 0}, + }, + ReactionGroup{ + Content: "HOORAY", + Users: ReactionGroupUsers{TotalCount: 1}, + }, + ReactionGroup{ + Content: "CONFUSED", + Users: ReactionGroupUsers{TotalCount: 0}, + }, + ReactionGroup{ + Content: "HEART", + Users: ReactionGroupUsers{TotalCount: 2}, + }, + }, + output: `^1 \x{1f389} • 2 \x{2764}\x{fe0f}$`, + }, + "reaction groups with unmapped emoji": { + rgs: []ReactionGroup{ + ReactionGroup{ + Content: "UNKNOWN", + Users: ReactionGroupUsers{TotalCount: 1}, + }, + }, + output: `^$`, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert.Regexp(t, tt.output, tt.rgs.String()) + }) + } +} diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index 90a729599..09be5bab6 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -11,52 +11,55 @@ import ( "github.com/cli/cli/internal/ghrepo" ) -func IssueFromArg(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), arg string) (*api.Issue, ghrepo.Interface, error) { - issue, baseRepo, err := issueFromURL(apiClient, arg) - if err != nil { - return nil, nil, err - } - if issue != nil { - return issue, baseRepo, nil +func IssueWithCommentsFromArg(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), arg string, comments int) (*api.Issue, ghrepo.Interface, error) { + issueNumber, baseRepo := issueMetadataFromURL(arg) + + if baseRepo == nil { + var err error + baseRepo, err = baseRepoFn() + if err != nil { + return nil, nil, fmt.Errorf("could not determine base repo: %w", err) + } } - baseRepo, err = baseRepoFn() - if err != nil { - return nil, nil, fmt.Errorf("could not determine base repo: %w", err) + if issueNumber == 0 { + var err error + issueNumber, err = strconv.Atoi(strings.TrimPrefix(arg, "#")) + if err != nil { + return nil, nil, fmt.Errorf("invalid issue format: %q", arg) + } } - issueNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")) - if err != nil { - return nil, nil, fmt.Errorf("invalid issue format: %q", arg) - } - - issue, err = issueFromNumber(apiClient, baseRepo, issueNumber) + issue, err := issueFromNumber(apiClient, baseRepo, issueNumber, comments) return issue, baseRepo, err } +func IssueFromArg(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), arg string) (*api.Issue, ghrepo.Interface, error) { + return IssueWithCommentsFromArg(apiClient, baseRepoFn, arg, 0) +} + var issueURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/issues/(\d+)`) -func issueFromURL(apiClient *api.Client, s string) (*api.Issue, ghrepo.Interface, error) { +func issueMetadataFromURL(s string) (int, ghrepo.Interface) { u, err := url.Parse(s) if err != nil { - return nil, nil, nil + return 0, nil } if u.Scheme != "https" && u.Scheme != "http" { - return nil, nil, nil + return 0, nil } m := issueURLRE.FindStringSubmatch(u.Path) if m == nil { - return nil, nil, nil + return 0, nil } repo := ghrepo.NewWithHost(m[1], m[2], u.Hostname()) issueNumber, _ := strconv.Atoi(m[3]) - issue, err := issueFromNumber(apiClient, repo, issueNumber) - return issue, repo, err + return issueNumber, repo } -func issueFromNumber(apiClient *api.Client, repo ghrepo.Interface, issueNumber int) (*api.Issue, error) { - return api.IssueByNumber(apiClient, repo, issueNumber) +func issueFromNumber(apiClient *api.Client, repo ghrepo.Interface, issueNumber, comments int) (*api.Issue, error) { + return api.IssueByNumber(apiClient, repo, issueNumber, comments) } diff --git a/pkg/cmd/issue/view/fixtures/issueView_preview.json b/pkg/cmd/issue/view/fixtures/issueView_preview.json index e25090a61..65fc5ef51 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_preview.json +++ b/pkg/cmd/issue/view/fixtures/issueView_preview.json @@ -7,21 +7,21 @@ "body": "**bold story**", "title": "ix of coins", "state": "OPEN", - "created_at": "2011-01-26T19:01:12Z", + "createdAt": "2011-01-26T19:01:12Z", "author": { "login": "marseilles" }, "assignees": { "nodes": [], - "totalcount": 0 + "totalCount": 0 }, "labels": { "nodes": [], - "totalcount": 0 + "totalCount": 0 }, "projectcards": { "nodes": [], - "totalcount": 0 + "totalCount": 0 }, "milestone": { "title": "" diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewClosedState.json b/pkg/cmd/issue/view/fixtures/issueView_previewClosedState.json index 978927125..4665c47e1 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_previewClosedState.json +++ b/pkg/cmd/issue/view/fixtures/issueView_previewClosedState.json @@ -7,7 +7,7 @@ "body": "**bold story**", "title": "ix of coins", "state": "CLOSED", - "created_at": "2011-01-26T19:01:12Z", + "createdAt": "2011-01-26T19:01:12Z", "author": { "login": "marseilles" }, diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json b/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json new file mode 100644 index 000000000..2805c9694 --- /dev/null +++ b/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json @@ -0,0 +1,383 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "some body", + "title": "some title", + "state": "OPEN", + "createdAt": "2020-01-01T12:00:00Z", + "author": { + "login": "marseilles" + }, + "assignees": { + "nodes": [], + "totalCount": 0 + }, + "labels": { + "nodes": [], + "totalCount": 0 + }, + "projectcards": { + "nodes": [], + "totalCount": 0 + }, + "milestone": { + "title": "" + }, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ], + "comments": { + "nodes": [ + { + "author": { + "login": "monalisa" + }, + "authorAssociation": "NONE", + "body": "Comment 1", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 1 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 2 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 3 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 4 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 5 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 6 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 7 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 8 + } + } + ] + }, + { + "author": { + "login": "johnnytest" + }, + "authorAssociation": "CONTRIBUTOR", + "body": "Comment 2", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + }, + { + "author": { + "login": "elvisp" + }, + "authorAssociation": "MEMBER", + "body": "Comment 3", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + }, + { + "author": { + "login": "loislane" + }, + "authorAssociation": "OWNER", + "body": "Comment 4", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + }, + { + "author": { + "login": "marseilles" + }, + "authorAssociation": "COLLABORATOR", + "body": "Comment 5", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + } + ], + "totalCount": 5 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json b/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json new file mode 100644 index 000000000..87fc7bffc --- /dev/null +++ b/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json @@ -0,0 +1,147 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "some body", + "title": "some title", + "state": "OPEN", + "createdAt": "2020-01-01T12:00:00Z", + "author": { + "login": "marseilles" + }, + "assignees": { + "nodes": [], + "totalCount": 0 + }, + "labels": { + "nodes": [], + "totalCount": 0 + }, + "projectcards": { + "nodes": [], + "totalCount": 0 + }, + "milestone": { + "title": "" + }, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ], + "comments": { + "nodes": [ + { + "author": { + "login": "marseilles" + }, + "authorAssociation": "COLLABORATOR", + "body": "Comment 5", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + } + ], + "totalCount": 5 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewThreeComments.json b/pkg/cmd/issue/view/fixtures/issueView_previewThreeComments.json new file mode 100644 index 000000000..74d5945e1 --- /dev/null +++ b/pkg/cmd/issue/view/fixtures/issueView_previewThreeComments.json @@ -0,0 +1,265 @@ +{ + "data": { + "repository": { + "hasIssuesEnabled": true, + "issue": { + "number": 123, + "body": "some body", + "title": "some title", + "state": "OPEN", + "createdAt": "2020-01-01T12:00:00Z", + "author": { + "login": "marseilles" + }, + "assignees": { + "nodes": [], + "totalCount": 0 + }, + "labels": { + "nodes": [], + "totalCount": 0 + }, + "projectcards": { + "nodes": [], + "totalCount": 0 + }, + "milestone": { + "title": "" + }, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ], + "comments": { + "nodes": [ + { + "author": { + "login": "elvisp" + }, + "authorAssociation": "MEMBER", + "body": "Comment 3", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + }, + { + "author": { + "login": "loislane" + }, + "authorAssociation": "OWNER", + "body": "Comment 4", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + }, + { + "author": { + "login": "marseilles" + }, + "authorAssociation": "COLLABORATOR", + "body": "Comment 5", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 0 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 0 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 0 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 0 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 0 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + } + ], + "totalCount": 5 + }, + "url": "https://github.com/OWNER/REPO/issues/123" + } + } + } +} diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewWithEmptyBody.json b/pkg/cmd/issue/view/fixtures/issueView_previewWithEmptyBody.json index 6e87a42b6..104f134be 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_previewWithEmptyBody.json +++ b/pkg/cmd/issue/view/fixtures/issueView_previewWithEmptyBody.json @@ -7,7 +7,7 @@ "body": "", "title": "ix of coins", "state": "OPEN", - "created_at": "2011-01-26T19:01:12Z", + "createdAt": "2011-01-26T19:01:12Z", "author": { "login": "marseilles" }, diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json b/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json index 246bbd77b..9bce5f745 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json +++ b/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json @@ -7,7 +7,7 @@ "body": "**bold story**", "title": "ix of coins", "state": "OPEN", - "created_at": "2011-01-26T19:01:12Z", + "createdAt": "2011-01-26T19:01:12Z", "author": { "login": "marseilles" }, @@ -20,7 +20,7 @@ "login": "monaco" } ], - "totalcount": 2 + "totalCount": 2 }, "labels": { "nodes": [ @@ -40,7 +40,7 @@ "name": "five" } ], - "totalcount": 5 + "totalCount": 5 }, "projectcards": { "nodes": [ @@ -77,13 +77,63 @@ } } ], - "totalcount": 3 + "totalCount": 3 }, "milestone": { "title": "uluru" }, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 8 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 7 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 6 + } + }, + { + "content": "HOORAY", + "users": { + "totalCount": 5 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 4 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 3 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 2 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 1 + } + } + ], "comments": { - "totalcount": 9 + "totalCount": 9 }, "url": "https://github.com/OWNER/REPO/issues/123" } diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 2bada530c..759789036 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -29,6 +29,7 @@ type ViewOptions struct { SelectorArg string WebMode bool + Comments int } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -65,6 +66,8 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open an issue in the browser") + cmd.Flags().IntVarP(&opts.Comments, "comments", "c", 1, "View issue comments") + cmd.Flags().Lookup("comments").NoOptDefVal = "30" return cmd } @@ -76,7 +79,7 @@ func viewRun(opts *ViewOptions) error { } apiClient := api.NewClientFromHTTP(httpClient) - issue, _, err := issueShared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + issue, _, err := issueShared.IssueWithCommentsFromArg(apiClient, opts.BaseRepo, opts.SelectorArg, opts.Comments) if err != nil { return err } @@ -122,9 +125,33 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { fmt.Fprintln(out, "--") fmt.Fprintln(out, issue.Body) + fmt.Fprintln(out, "--") + + if len(issue.Comments.Nodes) > 0 { + fmt.Fprintf(out, rawIssueComments(issue.Comments)) + } + return nil } +func rawIssueComments(comments api.IssueComments) string { + var b strings.Builder + for _, comment := range comments.Nodes { + fmt.Fprintf(&b, rawIssueComment(comment)) + } + return b.String() +} + +func rawIssueComment(comment api.IssueComment) string { + var b strings.Builder + fmt.Fprintf(&b, "author:\t%s\n", comment.Author.Login) + fmt.Fprintf(&b, "association:\t%s\n", strings.ToLower(comment.AuthorAssociation)) + fmt.Fprintln(&b, "--") + fmt.Fprintln(&b, comment.Body) + fmt.Fprintln(&b, "--") + return b.String() +} + func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { out := io.Out now := time.Now() @@ -133,16 +160,21 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { // Header (Title and State) fmt.Fprintln(out, cs.Bold(issue.Title)) - fmt.Fprint(out, issueStateTitleWithColor(cs, issue.State)) - fmt.Fprintln(out, cs.Gray(fmt.Sprintf( - " • %s opened %s • %s", + fmt.Fprintln(out, fmt.Sprintf( + "%s • %s opened %s • %s", + issueStateTitleWithColor(cs, issue.State), issue.Author.Login, utils.FuzzyAgo(ago), utils.Pluralize(issue.Comments.TotalCount, "comment"), - ))) + )) + + // Reactions + if reactions := issue.ReactionGroups.String(); reactions != "" { + fmt.Fprint(out, reactions) + fmt.Fprintln(out) + } // Metadata - fmt.Fprintln(out) if assignees := issueAssigneeList(*issue); assignees != "" { fmt.Fprint(out, cs.Bold("Assignees: ")) fmt.Fprintln(out, assignees) @@ -161,22 +193,102 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { } // Body - if issue.Body != "" { - fmt.Fprintln(out) - style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(issue.Body, style, "") + fmt.Fprintln(out) + if issue.Body == "" { + issue.Body = "_No description provided_" + } + style := markdown.GetStyle(io.TerminalTheme()) + md, err := markdown.Render(issue.Body, style, "") + if err != nil { + return err + } + fmt.Fprint(out, md) + fmt.Fprintln(out) + + // Comments + if issue.Comments.TotalCount > 0 { + comments, err := issueComments(io, issue.Comments) if err != nil { return err } - fmt.Fprintln(out, md) + fmt.Fprintf(out, comments) } - fmt.Fprintln(out) // Footer - fmt.Fprintf(out, cs.Gray("View this issue on GitHub: %s\n"), issue.URL) + fmt.Fprintf(out, cs.Gray("View this issue on GitHub: %s"), issue.URL) + return nil } +func issueComments(io *iostreams.IOStreams, comments api.IssueComments) (string, error) { + var b strings.Builder + cs := io.ColorScheme() + retrievedCount := len(comments.Nodes) + hiddenCount := comments.TotalCount - retrievedCount + + if hiddenCount > 0 { + fmt.Fprintf(&b, cs.Gray(fmt.Sprintf("———————— Hiding %v comments ————————", hiddenCount))) + fmt.Fprintf(&b, "\n\n\n") + } + + for i, comment := range comments.Nodes { + last := i+1 == retrievedCount + cmt, err := issueComment(io, comment, last) + if err != nil { + return "", err + } + fmt.Fprintf(&b, cmt) + if last { + fmt.Fprintln(&b) + } + } + + if hiddenCount > 0 { + fmt.Fprintf(&b, cs.Gray("Use --comments to view the full conversation")) + fmt.Fprintln(&b) + } + + return b.String(), nil +} + +func issueComment(io *iostreams.IOStreams, comment api.IssueComment, newest bool) (string, error) { + var b strings.Builder + cs := io.ColorScheme() + + // Header + fmt.Fprintf(&b, cs.Bold(comment.Author.Login)) + if comment.AuthorAssociation != "NONE" { + fmt.Fprintf(&b, cs.Bold(fmt.Sprintf(" (%s)", strings.ToLower(comment.AuthorAssociation)))) + } + fmt.Fprintf(&b, cs.Bold(fmt.Sprintf(" • %s", utils.FuzzyAgoAbbr(time.Now(), comment.CreatedAt)))) + if comment.IncludesCreatedEdit { + fmt.Fprintf(&b, cs.Bold(" • edited")) + } + if newest { + fmt.Fprintf(&b, cs.Bold(" • ")) + fmt.Fprintf(&b, cs.CyanBold("Newest comment")) + } + fmt.Fprintln(&b) + + // Reactions + if reactions := comment.ReactionGroups.String(); reactions != "" { + fmt.Fprint(&b, reactions) + fmt.Fprintln(&b) + } + + // Body + if comment.Body != "" { + style := markdown.GetStyle(io.TerminalTheme()) + md, err := markdown.Render(comment.Body, style, "") + if err != nil { + return "", err + } + fmt.Fprint(&b, md) + } + + return b.String(), nil +} + func issueStateTitleWithColor(cs *iostreams.ColorScheme, state string) string { colorFunc := cs.ColorFromString(prShared.ColorForState(state)) return colorFunc(strings.Title(strings.ToLower(state))) diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 0567c87ff..cb1b1604e 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "net/http" "os/exec" - "reflect" "testing" "github.com/cli/cli/internal/config" @@ -16,15 +15,9 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" "github.com/google/shlex" + "github.com/stretchr/testify/assert" ) -func eq(t *testing.T, got interface{}, expected interface{}) { - t.Helper() - if !reflect.DeepEqual(got, expected) { - t.Errorf("expected: %v, got: %v", expected, got) - } -} - func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) @@ -86,14 +79,14 @@ func TestIssueView_web(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/issues/123 in your browser.\n") + assert.Equal(t, "", output.String()) + assert.Equal(t, "Opening github.com/OWNER/REPO/issues/123 in your browser.\n", output.Stderr()) if seenCmd == nil { t.Fatal("expected a command to run") } url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/123") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) } func TestIssueView_web_numberArgWithHash(t *testing.T) { @@ -119,14 +112,14 @@ func TestIssueView_web_numberArgWithHash(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/issues/123 in your browser.\n") + assert.Equal(t, "", output.String()) + assert.Equal(t, "Opening github.com/OWNER/REPO/issues/123 in your browser.\n", output.Stderr()) if seenCmd == nil { t.Fatal("expected a command to run") } url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/123") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) } func TestIssueView_nontty_Preview(t *testing.T) { @@ -192,7 +185,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { t.Errorf("error running `issue view`: %v", err) } - eq(t, output.Stderr(), "") + assert.Equal(t, "", output.Stderr()) test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) @@ -208,7 +201,7 @@ func TestIssueView_tty_Preview(t *testing.T) { fixture: "./fixtures/issueView_preview.json", expectedOutputs: []string{ `ix of coins`, - `Open.*marseilles opened about 292 years ago.*9 comments`, + `Open.*marseilles opened about 9 years ago.*9 comments`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, }, @@ -217,7 +210,8 @@ func TestIssueView_tty_Preview(t *testing.T) { fixture: "./fixtures/issueView_previewWithMetadata.json", expectedOutputs: []string{ `ix of coins`, - `Open.*marseilles opened about 292 years ago.*9 comments`, + `Open.*marseilles opened about 9 years ago.*9 comments`, + `8 \x{1f615} • 7 \x{1f440} • 6 \x{2764}\x{fe0f} • 5 \x{1f389} • 4 \x{1f604} • 3 \x{1f680} • 2 \x{1f44e} • 1 \x{1f44d}`, `Assignees:.*marseilles, monaco\n`, `Labels:.*one, two, three, four, five\n`, `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, @@ -230,7 +224,8 @@ func TestIssueView_tty_Preview(t *testing.T) { fixture: "./fixtures/issueView_previewWithEmptyBody.json", expectedOutputs: []string{ `ix of coins`, - `Open.*marseilles opened about 292 years ago.*9 comments`, + `Open.*marseilles opened about 9 years ago.*9 comments`, + `No description provided`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, }, }, @@ -238,7 +233,7 @@ func TestIssueView_tty_Preview(t *testing.T) { fixture: "./fixtures/issueView_previewClosedState.json", expectedOutputs: []string{ `ix of coins`, - `Closed.*marseilles opened about 292 years ago.*9 comments`, + `Closed.*marseilles opened about 9 years ago.*9 comments`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, }, @@ -256,7 +251,7 @@ func TestIssueView_tty_Preview(t *testing.T) { t.Errorf("error running `issue view`: %v", err) } - eq(t, output.Stderr(), "") + assert.Equal(t, "", output.Stderr()) test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) @@ -330,11 +325,182 @@ func TestIssueView_web_urlArg(t *testing.T) { t.Errorf("error running command `issue view`: %v", err) } - eq(t, output.String(), "") + assert.Equal(t, "", output.String()) if seenCmd == nil { t.Fatal("expected a command to run") } url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/123") + assert.Equal(t, "https://github.com/OWNER/REPO/issues/123", url) +} + +func TestIssueView_tty_Comments(t *testing.T) { + tests := map[string]struct { + cli string + fixture string + expectedOutputs []string + wantsErr bool + }{ + "without comments flag": { + cli: "123", + fixture: "./fixtures/issueView_previewSingleComment.json", + expectedOutputs: []string{ + `some title`, + `some body`, + `———————— Hiding 4 comments ————————`, + `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `Comment 5`, + `Use --comments to view the full conversation`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + "with default comments flag": { + cli: "123 --comments", + fixture: "./fixtures/issueView_previewFullComments.json", + expectedOutputs: []string{ + `some title`, + `some body`, + `monalisa • Jan 1, 2020`, + `1 \x{1f615} • 2 \x{1f440} • 3 \x{2764}\x{fe0f} • 4 \x{1f389} • 5 \x{1f604} • 6 \x{1f680} • 7 \x{1f44e} • 8 \x{1f44d}`, + `Comment 1`, + `johnnytest \(contributor\) • Jan 1, 2020`, + `Comment 2`, + `elvisp \(member\) • Jan 1, 2020`, + `Comment 3`, + `loislane \(owner\) • Jan 1, 2020`, + `Comment 4`, + `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `Comment 5`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + "with specified comments flag": { + cli: "123 --comments=3", + fixture: "./fixtures/issueView_previewThreeComments.json", + expectedOutputs: []string{ + `some title`, + `some body`, + `———————— Hiding 2 comments ————————`, + `elvisp \(member\) • Jan 1, 2020`, + `Comment 3`, + `loislane \(owner\) • Jan 1, 2020`, + `Comment 4`, + `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `Comment 5`, + `Use --comments to view the full conversation`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + "with incorrect comments flag": { + cli: "123 --comments 3", + fixture: "", + wantsErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + if tc.fixture != "" { + http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + } + output, err := runCommand(http, true, tc.cli) + if tc.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, "", output.Stderr()) + test.ExpectLines(t, output.String(), tc.expectedOutputs...) + }) + } +} + +func TestIssueView_nontty_Comments(t *testing.T) { + tests := map[string]struct { + cli string + fixture string + expectedOutputs []string + wantsErr bool + }{ + "without comments flag": { + cli: "123", + fixture: "./fixtures/issueView_previewSingleComment.json", + expectedOutputs: []string{ + `title:\tsome title`, + `author:\tmarseilles`, + `comments:\t5`, + `some body`, + `author:\tmarseilles`, + `association:\tcollaborator`, + `Comment 5`, + }, + }, + "with default comments flag": { + cli: "123 --comments", + fixture: "./fixtures/issueView_previewFullComments.json", + expectedOutputs: []string{ + `title:\tsome title`, + `author:\tmarseilles`, + `comments:\t5`, + `some body`, + `author:\tmonalisa`, + `association:\t`, + `Comment 1`, + `author:\tjohnnytest`, + `association:\tcontributor`, + `Comment 2`, + `author:\telvisp`, + `association:\tmember`, + `Comment 3`, + `author:\tloislane`, + `association:\towner`, + `Comment 4`, + `author:\tmarseilles`, + `association:\tcollaborator`, + `Comment 5`, + }, + }, + "with specified comments flag": { + cli: "123 --comments=3", + fixture: "./fixtures/issueView_previewThreeComments.json", + expectedOutputs: []string{ + `title:\tsome title`, + `author:\tmarseilles`, + `comments:\t5`, + `some body`, + `author:\telvisp`, + `association:\tmember`, + `Comment 3`, + `author:\tloislane`, + `association:\towner`, + `Comment 4`, + `author:\tmarseilles`, + `association:\tcollaborator`, + `Comment 5`, + }, + }, + "with incorrect comments flag": { + cli: "123 --comments 3", + fixture: "", + wantsErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + if tc.fixture != "" { + http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + } + output, err := runCommand(http, false, tc.cli) + if tc.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, "", output.Stderr()) + test.ExpectLines(t, output.String(), tc.expectedOutputs...) + }) + } } diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 6fc2bd023..972caab3d 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -9,14 +9,15 @@ import ( ) var ( - magenta = ansi.ColorFunc("magenta") - cyan = ansi.ColorFunc("cyan") - red = ansi.ColorFunc("red") - yellow = ansi.ColorFunc("yellow") - blue = ansi.ColorFunc("blue") - green = ansi.ColorFunc("green") - gray = ansi.ColorFunc("black+h") - bold = ansi.ColorFunc("default+b") + magenta = ansi.ColorFunc("magenta") + cyan = ansi.ColorFunc("cyan") + red = ansi.ColorFunc("red") + yellow = ansi.ColorFunc("yellow") + blue = ansi.ColorFunc("blue") + green = ansi.ColorFunc("green") + gray = ansi.ColorFunc("black+h") + bold = ansi.ColorFunc("default+b") + cyanBold = ansi.ColorFunc("cyan+b") gray256 = func(t string) string { return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t) @@ -107,6 +108,13 @@ func (c *ColorScheme) Cyan(t string) string { return cyan(t) } +func (c *ColorScheme) CyanBold(t string) string { + if !c.enabled { + return t + } + return cyanBold(t) +} + func (c *ColorScheme) Blue(t string) string { if !c.enabled { return t diff --git a/utils/utils.go b/utils/utils.go index 602b8ab1e..4b58508ef 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -59,6 +59,22 @@ func FuzzyAgo(ago time.Duration) string { return fmtDuration(int(ago.Hours()/24/365), "year") } +func FuzzyAgoAbbr(now time.Time, createdAt time.Time) string { + ago := now.Sub(createdAt) + + if ago < time.Hour { + return fmt.Sprintf("%d%s", int(ago.Minutes()), "m") + } + if ago < 24*time.Hour { + return fmt.Sprintf("%d%s", int(ago.Hours()), "h") + } + if ago < 30*24*time.Hour { + return fmt.Sprintf("%d%s", int(ago.Hours())/24, "d") + } + + return createdAt.Format("Jan _2, 2006") +} + func Humanize(s string) string { // Replaces - and _ with spaces. replace := "_-" diff --git a/utils/utils_test.go b/utils/utils_test.go index 0891c2a39..5dc7b2478 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -6,7 +6,6 @@ import ( ) func TestFuzzyAgo(t *testing.T) { - cases := map[string]string{ "1s": "less than a minute ago", "30s": "less than a minute ago", @@ -36,3 +35,29 @@ func TestFuzzyAgo(t *testing.T) { } } } + +func TestFuzzyAgoAbbr(t *testing.T) { + const form = "2006-Jan-02 15:04:05" + now, _ := time.Parse(form, "2020-Nov-22 14:00:00") + + cases := map[string]string{ + "2020-Nov-22 14:00:00": "0m", + "2020-Nov-22 13:59:00": "1m", + "2020-Nov-22 13:30:00": "30m", + "2020-Nov-22 13:00:00": "1h", + "2020-Nov-22 02:00:00": "12h", + "2020-Nov-21 14:00:00": "1d", + "2020-Nov-07 14:00:00": "15d", + "2020-Oct-24 14:00:00": "29d", + "2020-Oct-23 14:00:00": "Oct 23, 2020", + "2019-Nov-22 14:00:00": "Nov 22, 2019", + } + + for createdAt, expected := range cases { + d, _ := time.Parse(form, createdAt) + fuzzy := FuzzyAgoAbbr(now, d) + if fuzzy != expected { + t.Errorf("unexpected fuzzy duration abbr value: %s for %s", fuzzy, createdAt) + } + } +}