diff --git a/api/queries_comments.go b/api/queries_comments.go new file mode 100644 index 000000000..70ea382a1 --- /dev/null +++ b/api/queries_comments.go @@ -0,0 +1,65 @@ +package api + +import ( + "context" + "time" + + "github.com/cli/cli/internal/ghrepo" + "github.com/shurcooL/githubv4" +) + +type Comments struct { + Nodes []Comment + TotalCount int + PageInfo PageInfo +} + +type Comment struct { + Author Author + AuthorAssociation string + Body string + CreatedAt time.Time + IncludesCreatedEdit bool + ReactionGroups ReactionGroups +} + +type PageInfo struct { + HasNextPage bool + EndCursor string +} + +func CommentsForIssue(client *Client, repo ghrepo.Interface, issue *Issue) (*Comments, error) { + type response struct { + Repository struct { + Issue struct { + Comments Comments `graphql:"comments(first: 100, after: $endCursor)"` + } `graphql:"issue(number: $number)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(repo.RepoOwner()), + "repo": githubv4.String(repo.RepoName()), + "number": githubv4.Int(issue.Number), + "endCursor": (*githubv4.String)(nil), + } + + gql := graphQLClient(client.http, repo.RepoHost()) + + var comments []Comment + for { + var query response + err := gql.QueryNamed(context.Background(), "CommentsForIssue", &query, variables) + if err != nil { + return nil, err + } + + comments = append(comments, query.Repository.Issue.Comments.Nodes...) + if !query.Repository.Issue.Comments.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Repository.Issue.Comments.PageInfo.EndCursor) + } + + return &Comments{Nodes: comments, TotalCount: len(comments)}, nil +} diff --git a/api/queries_issue.go b/api/queries_issue.go index 08e0cf1d9..17f32eabd 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 Comments + Author Author Assignees struct { Nodes []struct { Login string @@ -65,12 +61,17 @@ type Issue struct { Milestone struct { Title string } + ReactionGroups ReactionGroups } type IssuesDisabledError struct { error } +type Author struct { + Login string +} + const fragments = ` fragment issue on Issue { number @@ -341,7 +342,22 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e author { login } - comments { + comments(last: 1) { + nodes { + author { + login + } + authorAssociation + body + createdAt + includesCreatedEdit + reactionGroups { + content + users { + totalCount + } + } + } totalCount } number @@ -370,9 +386,15 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } totalCount } - milestone{ + milestone { title } + reactionGroups { + content + users { + totalCount + } + } } } }` diff --git a/api/reaction_groups.go b/api/reaction_groups.go new file mode 100644 index 000000000..381504dd7 --- /dev/null +++ b/api/reaction_groups.go @@ -0,0 +1,31 @@ +package api + +type ReactionGroups []ReactionGroup + +type ReactionGroup struct { + Content string + Users ReactionGroupUsers +} + +type ReactionGroupUsers struct { + TotalCount int +} + +func (rg ReactionGroup) Count() int { + return rg.Users.TotalCount +} + +func (rg ReactionGroup) Emoji() string { + return reactionEmoji[rg.Content] +} + +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..e30a9e1f8 --- /dev/null +++ b/api/reaction_groups_test.go @@ -0,0 +1,100 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_String(t *testing.T) { + tests := map[string]struct { + rg ReactionGroup + emoji string + count int + }{ + "empty reaction group": { + rg: ReactionGroup{}, + emoji: "", + count: 0, + }, + "unknown reaction group": { + rg: ReactionGroup{ + Content: "UNKNOWN", + Users: ReactionGroupUsers{TotalCount: 1}, + }, + emoji: "", + count: 1, + }, + "thumbs up reaction group": { + rg: ReactionGroup{ + Content: "THUMBS_UP", + Users: ReactionGroupUsers{TotalCount: 2}, + }, + emoji: "\U0001f44d", + count: 2, + }, + "thumbs down reaction group": { + rg: ReactionGroup{ + Content: "THUMBS_DOWN", + Users: ReactionGroupUsers{TotalCount: 3}, + }, + emoji: "\U0001f44e", + count: 3, + }, + "laugh reaction group": { + rg: ReactionGroup{ + Content: "LAUGH", + Users: ReactionGroupUsers{TotalCount: 4}, + }, + emoji: "\U0001f604", + count: 4, + }, + "hooray reaction group": { + rg: ReactionGroup{ + Content: "HOORAY", + Users: ReactionGroupUsers{TotalCount: 5}, + }, + emoji: "\U0001f389", + count: 5, + }, + "confused reaction group": { + rg: ReactionGroup{ + Content: "CONFUSED", + Users: ReactionGroupUsers{TotalCount: 6}, + }, + emoji: "\U0001f615", + count: 6, + }, + "heart reaction group": { + rg: ReactionGroup{ + Content: "HEART", + Users: ReactionGroupUsers{TotalCount: 7}, + }, + emoji: "\u2764\ufe0f", + count: 7, + }, + "rocket reaction group": { + rg: ReactionGroup{ + Content: "ROCKET", + Users: ReactionGroupUsers{TotalCount: 8}, + }, + emoji: "\U0001f680", + count: 8, + }, + "eyes reaction group": { + rg: ReactionGroup{ + Content: "EYES", + Users: ReactionGroupUsers{TotalCount: 9}, + }, + emoji: "\U0001f440", + count: 9, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tt.emoji, tt.rg.Emoji()) + assert.Equal(t, tt.count, tt.rg.Count()) + }) + } +} diff --git a/pkg/cmd/issue/shared/lookup.go b/pkg/cmd/issue/shared/lookup.go index 90a729599..6e716f6da 100644 --- a/pkg/cmd/issue/shared/lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -12,49 +12,48 @@ import ( ) 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 + issueNumber, baseRepo := issueMetadataFromURL(arg) + + 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) + } } - baseRepo, err = baseRepoFn() - if err != nil { - return nil, nil, fmt.Errorf("could not determine base repo: %w", err) + if baseRepo == nil { + var err error + baseRepo, err = baseRepoFn() + if err != nil { + return nil, nil, fmt.Errorf("could not determine base repo: %w", err) + } } - 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) return issue, baseRepo, err } 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) { 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..d2cd27f30 --- /dev/null +++ b/pkg/cmd/issue/view/fixtures/issueView_previewFullComments.json @@ -0,0 +1,308 @@ +{ + "data": { + "repository": { + "issue": { + "comments": { + "nodes": [ + { + "author": { + "login": "monalisa" + }, + "authorAssociation": "NONE", + "body": "Comment 1", + "createdAt": "2020-01-01T12:00:00Z", + "includesCreatedEdit": true, + "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 + } + } + } + } +} 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_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..d8754edcb 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 bool } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -65,6 +66,7 @@ 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().BoolVarP(&opts.Comments, "comments", "c", false, "View issue comments") return cmd } @@ -76,20 +78,29 @@ func viewRun(opts *ViewOptions) error { } apiClient := api.NewClientFromHTTP(httpClient) - issue, _, err := issueShared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + issue, repo, err := issueShared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) if err != nil { return err } - openURL := issue.URL - if opts.WebMode { + openURL := issue.URL if opts.IO.IsStdoutTTY() { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) } return utils.OpenInBrowser(openURL) } + if opts.Comments { + opts.IO.StartProgressIndicator() + comments, err := api.CommentsForIssue(apiClient, repo, issue) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + issue.Comments = *comments + } + opts.IO.DetectTerminalTheme() err = opts.IO.StartPager() @@ -101,6 +112,11 @@ func viewRun(opts *ViewOptions) error { if opts.IO.IsStdoutTTY() { return printHumanIssuePreview(opts.IO, issue) } + + if opts.Comments { + return printRawIssueComments(opts.IO.Out, issue) + } + return printRawIssuePreview(opts.IO.Out, issue) } @@ -119,12 +135,31 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { fmt.Fprintf(out, "assignees:\t%s\n", assignees) fmt.Fprintf(out, "projects:\t%s\n", projects) fmt.Fprintf(out, "milestone:\t%s\n", issue.Milestone.Title) - fmt.Fprintln(out, "--") fmt.Fprintln(out, issue.Body) return nil } +func printRawIssueComments(out io.Writer, issue *api.Issue) error { + var b strings.Builder + for _, comment := range issue.Comments.Nodes { + fmt.Fprint(&b, rawIssueComment(comment)) + } + fmt.Fprint(out, b.String()) + return nil +} + +func rawIssueComment(comment api.Comment) 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.Fprintf(&b, "edited:\t%t\n", comment.IncludesCreatedEdit) + 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 +168,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.Fprintf(out, + "%s • %s opened %s • %s\n", + issueStateTitleWithColor(cs, issue.State), issue.Author.Login, utils.FuzzyAgo(ago), utils.Pluralize(issue.Comments.TotalCount, "comment"), - ))) + ) + + // Reactions + if reactions := reactionGroupList(issue.ReactionGroups); 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 +201,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 := issueCommentList(io, issue.Comments) if err != nil { return err } - fmt.Fprintln(out, md) + fmt.Fprint(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 issueCommentList(io *iostreams.IOStreams, comments api.Comments) (string, error) { + var b strings.Builder + cs := io.ColorScheme() + retrievedCount := len(comments.Nodes) + hiddenCount := comments.TotalCount - retrievedCount + + if hiddenCount > 0 { + fmt.Fprint(&b, cs.Gray(fmt.Sprintf("———————— Not showing %s ————————", utils.Pluralize(hiddenCount, "comment")))) + fmt.Fprintf(&b, "\n\n\n") + } + + for i, comment := range comments.Nodes { + last := i+1 == retrievedCount + cmt, err := formatIssueComment(io, comment, last) + if err != nil { + return "", err + } + fmt.Fprint(&b, cmt) + if last { + fmt.Fprintln(&b) + } + } + + if hiddenCount > 0 { + fmt.Fprint(&b, cs.Gray("Use --comments to view the full conversation")) + fmt.Fprintln(&b) + } + + return b.String(), nil +} + +func formatIssueComment(io *iostreams.IOStreams, comment api.Comment, newest bool) (string, error) { + var b strings.Builder + cs := io.ColorScheme() + + // Header + fmt.Fprint(&b, cs.Bold(comment.Author.Login)) + if comment.AuthorAssociation != "NONE" { + fmt.Fprint(&b, cs.Bold(fmt.Sprintf(" (%s)", strings.ToLower(comment.AuthorAssociation)))) + } + fmt.Fprint(&b, cs.Bold(fmt.Sprintf(" • %s", utils.FuzzyAgoAbbr(time.Now(), comment.CreatedAt)))) + if comment.IncludesCreatedEdit { + fmt.Fprint(&b, cs.Bold(" • edited")) + } + if newest { + fmt.Fprint(&b, cs.Bold(" • ")) + fmt.Fprint(&b, cs.CyanBold("Newest comment")) + } + fmt.Fprintln(&b) + + // Reactions + if reactions := reactionGroupList(comment.ReactionGroups); 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))) @@ -219,3 +339,27 @@ func issueProjectList(issue api.Issue) string { } return list } + +func reactionGroupList(rgs api.ReactionGroups) string { + var rs []string + + for _, rg := range rgs { + if r := formatReactionGroup(rg); r != "" { + rs = append(rs, r) + } + } + + return strings.Join(rs, " • ") +} + +func formatReactionGroup(rg api.ReactionGroup) string { + c := rg.Count() + if c == 0 { + return "" + } + e := rg.Emoji() + if e == "" { + return "" + } + return fmt.Sprintf("%v %s", c, e) +} diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 0567c87ff..2b94c5a70 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -2,12 +2,13 @@ package view import ( "bytes" + "fmt" "io/ioutil" "net/http" "os/exec" - "reflect" "testing" + "github.com/briandowns/spinner" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" @@ -15,16 +16,11 @@ import ( "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" + "github.com/cli/cli/utils" "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 +82,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 +115,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 +188,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 +204,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 +213,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 +227,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 +236,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 +254,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 +328,161 @@ 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 + fixtures map[string]string + expectedOutputs []string + wantsErr bool + }{ + "without comments flag": { + cli: "123", + fixtures: map[string]string{ + "IssueByNumber": "./fixtures/issueView_previewSingleComment.json", + }, + expectedOutputs: []string{ + `some title`, + `some body`, + `———————— Not showing 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 comments flag": { + cli: "123 --comments", + fixtures: map[string]string{ + "IssueByNumber": "./fixtures/issueView_previewSingleComment.json", + "CommentsForIssue": "./fixtures/issueView_previewFullComments.json", + }, + expectedOutputs: []string{ + `some title`, + `some body`, + `monalisa • Jan 1, 2020 • edited`, + `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 invalid comments flag": { + cli: "123 --comments 3", + wantsErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + stubSpinner() + http := &httpmock.Registry{} + defer http.Verify(t) + for name, file := range tc.fixtures { + name := fmt.Sprintf(`query %s\b`, name) + http.Register(httpmock.GraphQL(name), httpmock.FileResponse(file)) + } + 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 + fixtures map[string]string + expectedOutputs []string + wantsErr bool + }{ + "without comments flag": { + cli: "123", + fixtures: map[string]string{ + "IssueByNumber": "./fixtures/issueView_previewSingleComment.json", + }, + expectedOutputs: []string{ + `title:\tsome title`, + `state:\tOPEN`, + `author:\tmarseilles`, + `comments:\t5`, + `some body`, + }, + }, + "with comments flag": { + cli: "123 --comments", + fixtures: map[string]string{ + "IssueByNumber": "./fixtures/issueView_previewSingleComment.json", + "CommentsForIssue": "./fixtures/issueView_previewFullComments.json", + }, + expectedOutputs: []string{ + `author:\tmonalisa`, + `association:\t`, + `edited:\ttrue`, + `Comment 1`, + `author:\tjohnnytest`, + `association:\tcontributor`, + `edited:\tfalse`, + `Comment 2`, + `author:\telvisp`, + `association:\tmember`, + `edited:\tfalse`, + `Comment 3`, + `author:\tloislane`, + `association:\towner`, + `edited:\tfalse`, + `Comment 4`, + `author:\tmarseilles`, + `association:\tcollaborator`, + `edited:\tfalse`, + `Comment 5`, + }, + }, + "with invalid comments flag": { + cli: "123 --comments 3", + wantsErr: true, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + for name, file := range tc.fixtures { + name := fmt.Sprintf(`query %s\b`, name) + http.Register(httpmock.GraphQL(name), httpmock.FileResponse(file)) + } + 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...) + }) + } +} + +func stubSpinner() { + utils.StartSpinner = func(_ *spinner.Spinner) {} + utils.StopSpinner = func(_ *spinner.Spinner) {} } 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) + } + } +}