diff --git a/api/queries_comments.go b/api/queries_comments.go index 70ea382a1..ccbabfe5d 100644 --- a/api/queries_comments.go +++ b/api/queries_comments.go @@ -63,3 +63,39 @@ func CommentsForIssue(client *Client, repo ghrepo.Interface, issue *Issue) (*Com return &Comments{Nodes: comments, TotalCount: len(comments)}, nil } + +func CommentsForPullRequest(client *Client, repo ghrepo.Interface, pr *PullRequest) (*Comments, error) { + type response struct { + Repository struct { + PullRequest struct { + Comments Comments `graphql:"comments(first: 100, after: $endCursor)"` + } `graphql:"pullRequest(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(pr.Number), + "endCursor": (*githubv4.String)(nil), + } + + gql := graphQLClient(client.http, repo.RepoHost()) + + var comments []Comment + for { + var query response + err := gql.QueryNamed(context.Background(), "CommentsForPullRequest", &query, variables) + if err != nil { + return nil, err + } + + comments = append(comments, query.Repository.PullRequest.Comments.Nodes...) + if !query.Repository.PullRequest.Comments.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Repository.PullRequest.Comments.PageInfo.EndCursor) + } + + return &Comments{Nodes: comments, TotalCount: len(comments)}, nil +} diff --git a/api/queries_pr.go b/api/queries_pr.go index 4433ef373..ad4f23644 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -137,6 +137,8 @@ type PullRequest struct { Milestone struct { Title string } + Comments Comments + ReactionGroups ReactionGroups } type NotFoundError struct { @@ -603,6 +605,30 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu milestone{ title } + comments(last: 1) { + nodes { + author { + login + } + authorAssociation + body + createdAt + includesCreatedEdit + reactionGroups { + content + users { + totalCount + } + } + } + totalCount + } + reactionGroups { + content + users { + totalCount + } + } } } }` @@ -712,6 +738,30 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea milestone{ title } + comments(last: 1) { + nodes { + author { + login + } + authorAssociation + body + createdAt + includesCreatedEdit + reactionGroups { + content + users { + totalCount + } + } + } + totalCount + } + reactionGroups { + content + users { + totalCount + } + } } } } diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index d8754edcb..e8ff2b274 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -114,7 +114,8 @@ func viewRun(opts *ViewOptions) error { } if opts.Comments { - return printRawIssueComments(opts.IO.Out, issue) + fmt.Fprint(opts.IO.Out, prShared.RawCommentList(issue.Comments)) + return nil } return printRawIssuePreview(opts.IO.Out, issue) @@ -140,26 +141,6 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { 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() @@ -177,7 +158,7 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { ) // Reactions - if reactions := reactionGroupList(issue.ReactionGroups); reactions != "" { + if reactions := prShared.ReactionGroupList(issue.ReactionGroups); reactions != "" { fmt.Fprint(out, reactions) fmt.Fprintln(out) } @@ -215,7 +196,7 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { // Comments if issue.Comments.TotalCount > 0 { - comments, err := issueCommentList(io, issue.Comments) + comments, err := prShared.CommentList(io, issue.Comments) if err != nil { return err } @@ -228,75 +209,6 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { 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))) @@ -339,27 +251,3 @@ 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/pr/shared/comments.go b/pkg/cmd/pr/shared/comments.go new file mode 100644 index 000000000..f0d820c1f --- /dev/null +++ b/pkg/cmd/pr/shared/comments.go @@ -0,0 +1,100 @@ +package shared + +import ( + "fmt" + "strings" + "time" + + "github.com/cli/cli/api" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/markdown" + "github.com/cli/cli/utils" +) + +func RawCommentList(comments api.Comments) string { + var b strings.Builder + for _, comment := range comments.Nodes { + fmt.Fprint(&b, formatRawComment(comment)) + } + return b.String() +} + +func formatRawComment(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 CommentList(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 := formatComment(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 formatComment(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 +} diff --git a/pkg/cmd/pr/shared/reaction_groups.go b/pkg/cmd/pr/shared/reaction_groups.go new file mode 100644 index 000000000..caf972672 --- /dev/null +++ b/pkg/cmd/pr/shared/reaction_groups.go @@ -0,0 +1,32 @@ +package shared + +import ( + "fmt" + "strings" + + "github.com/cli/cli/api" +) + +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/pr/view/fixtures/prViewPreviewFullComments.json b/pkg/cmd/pr/view/fixtures/prViewPreviewFullComments.json new file mode 100644 index 000000000..cadbf294b --- /dev/null +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewFullComments.json @@ -0,0 +1,308 @@ +{ + "data": { + "repository": { + "pullRequest": { + "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/pr/view/fixtures/prViewPreviewSingleComment.json b/pkg/cmd/pr/view/fixtures/prViewPreviewSingleComment.json new file mode 100644 index 000000000..c6ddc4321 --- /dev/null +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewSingleComment.json @@ -0,0 +1,155 @@ +{ + "data": { + "repository": { + "pullRequest": { + "number": 12, + "title": "some title", + "state": "OPEN", + "body": "some body", + "url": "https://github.com/OWNER/REPO/pull/12", + "author": { + "login": "nobody" + }, + "assignees": { + "nodes": [], + "totalcount": 0 + }, + "labels": { + "nodes": [], + "totalcount": 0 + }, + "projectcards": { + "nodes": [], + "totalcount": 0 + }, + "milestone": { + "title": "" + }, + "commits": { + "totalCount": 12 + }, + "baseRefName": "master", + "headRefName": "blueberries", + "headRepositoryOwner": { + "login": "hubot" + }, + "isCrossRepository": true, + "isDraft": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 1 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 2 + } + }, + { + "content": "HEART", + "users": { + "totalCount": 3 + } + }, + { + "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": 4 + } + }, + { + "content": "LAUGH", + "users": { + "totalCount": 5 + } + }, + { + "content": "ROCKET", + "users": { + "totalCount": 6 + } + }, + { + "content": "THUMBS_DOWN", + "users": { + "totalCount": 0 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 0 + } + } + ] + } + ], + "totalCount": 5 + } + } + } + } +} diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 9dcdb8f8b..8384d7af3 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -30,6 +30,7 @@ type ViewOptions struct { SelectorArg string BrowserMode bool + Comments bool } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -73,6 +74,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } cmd.Flags().BoolVarP(&opts.BrowserMode, "web", "w", false, "Open a pull request in the browser") + cmd.Flags().BoolVarP(&opts.Comments, "comments", "c", false, "View pull request comments") return cmd } @@ -84,21 +86,31 @@ func viewRun(opts *ViewOptions) error { } apiClient := api.NewClientFromHTTP(httpClient) - pr, _, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + pr, repo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) if err != nil { return err } - openURL := pr.URL connectedToTerminal := opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() if opts.BrowserMode { + openURL := pr.URL if connectedToTerminal { 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.CommentsForPullRequest(apiClient, repo, pr) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + pr.Comments = *comments + } + opts.IO.DetectTerminalTheme() err = opts.IO.StartPager() @@ -111,6 +123,11 @@ func viewRun(opts *ViewOptions) error { return printHumanPrPreview(opts.IO, pr) } + if opts.Comments { + fmt.Fprint(opts.IO.Out, shared.RawCommentList(pr.Comments)) + return nil + } + return printRawPrPreview(opts.IO, pr) } @@ -142,20 +159,24 @@ func printRawPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { out := io.Out - cs := io.ColorScheme() // Header (Title and State) fmt.Fprintln(out, cs.Bold(pr.Title)) - fmt.Fprintf(out, "%s", shared.StateTitleWithColor(cs, *pr)) - fmt.Fprintln(out, cs.Gray(fmt.Sprintf( - " • %s wants to merge %s into %s from %s", + fmt.Fprintf(out, + "%s • %s wants to merge %s into %s from %s\n", + shared.StateTitleWithColor(cs, *pr), pr.Author.Login, utils.Pluralize(pr.Commits.TotalCount, "commit"), pr.BaseRefName, pr.HeadRefName, - ))) - fmt.Fprintln(out) + ) + + // Reactions + if reactions := shared.ReactionGroupList(pr.ReactionGroups); reactions != "" { + fmt.Fprint(out, reactions) + fmt.Fprintln(out) + } // Metadata if reviewers := prReviewerList(*pr, cs); reviewers != "" { @@ -180,19 +201,30 @@ func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { } // Body - if pr.Body != "" { - fmt.Fprintln(out) - style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(pr.Body, style, "") + fmt.Fprintln(out) + if pr.Body == "" { + pr.Body = "_No description provided_" + } + style := markdown.GetStyle(io.TerminalTheme()) + md, err := markdown.Render(pr.Body, style, "") + if err != nil { + return err + } + fmt.Fprint(out, md) + fmt.Fprintln(out) + + // Comments + if pr.Comments.TotalCount > 0 { + comments, err := shared.CommentList(io, pr.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 pull request on GitHub: %s\n"), pr.URL) + fmt.Fprintf(out, cs.Gray("View this pull request on GitHub: %s"), pr.URL) + return nil } diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 6dc591243..678629831 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -2,13 +2,14 @@ package view import ( "bytes" + "fmt" "io/ioutil" "net/http" "os/exec" - "reflect" "strings" "testing" + "github.com/briandowns/spinner" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" @@ -18,6 +19,7 @@ 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" "github.com/stretchr/testify/require" @@ -64,6 +66,15 @@ func Test_NewCmdView(t *testing.T) { isTTY: true, wantErr: "argument required when using the --repo flag", }, + { + name: "comments", + args: "123 -c", + isTTY: true, + want: ViewOptions{ + SelectorArg: "123", + Comments: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -104,13 +115,6 @@ func Test_NewCmdView(t *testing.T) { } } -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, branch string, isTTY bool, cli string) (*test.CmdOut, error) { io, _, stdout, stderr := iostreams.Test() io.SetStdoutTTY(isTTY) @@ -332,7 +336,7 @@ func TestPRView_Preview_nontty(t *testing.T) { t.Errorf("error running command `%v`: %v", tc.args, err) } - eq(t, output.Stderr(), "") + assert.Equal(t, "", output.Stderr()) test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) @@ -370,7 +374,7 @@ func TestPRView_Preview(t *testing.T) { `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone:.*uluru\n`, `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, }, "Open PR with reviewers by number": { @@ -381,7 +385,7 @@ func TestPRView_Preview(t *testing.T) { `Blueberries are from a fork`, `Reviewers:.*DEF \(.*Commented.*\), def \(.*Changes requested.*\), ghost \(.*Approved.*\), hubot \(Commented\), xyz \(.*Approved.*\), 123 \(.*Requested.*\), Team 1 \(.*Requested.*\), abc \(.*Requested.*\)\n`, `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, }, "Open PR with metadata by branch": { @@ -396,7 +400,7 @@ func TestPRView_Preview(t *testing.T) { `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, `Milestone:.*uluru\n`, `blueberries taste good`, - `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10\n`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`, }, }, "Open PR for the current branch": { @@ -477,7 +481,7 @@ func TestPRView_Preview(t *testing.T) { t.Errorf("error running command `%v`: %v", tc.args, err) } - eq(t, output.Stderr(), "") + assert.Equal(t, "", output.Stderr()) test.ExpectLines(t, output.String(), tc.expectedOutputs...) }) @@ -506,8 +510,8 @@ func TestPRView_web_currentBranch(t *testing.T) { t.Errorf("error running command `pr view`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/pull/10 in your browser.\n") + assert.Equal(t, "", output.String()) + assert.Equal(t, "Opening github.com/OWNER/REPO/pull/10 in your browser.\n", output.Stderr()) if seenCmd == nil { t.Fatal("expected a command to run") @@ -567,13 +571,13 @@ func TestPRView_web_numberArg(t *testing.T) { t.Errorf("error running command `pr 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/pull/23") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_numberArgWithHash(t *testing.T) { @@ -598,13 +602,13 @@ func TestPRView_web_numberArgWithHash(t *testing.T) { t.Errorf("error running command `pr 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/pull/23") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_urlArg(t *testing.T) { @@ -628,13 +632,13 @@ func TestPRView_web_urlArg(t *testing.T) { t.Errorf("error running command `pr 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/pull/23") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_branchArg(t *testing.T) { @@ -661,13 +665,13 @@ func TestPRView_web_branchArg(t *testing.T) { t.Errorf("error running command `pr 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/pull/23") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/23", url) } func TestPRView_web_branchWithOwnerArg(t *testing.T) { @@ -695,11 +699,171 @@ func TestPRView_web_branchWithOwnerArg(t *testing.T) { t.Errorf("error running command `pr 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/hubot/REPO/pull/23") + assert.Equal(t, "https://github.com/hubot/REPO/pull/23", url) +} + +func TestPRView_tty_Comments(t *testing.T) { + tests := map[string]struct { + branch string + cli string + fixtures map[string]string + expectedOutputs []string + wantsErr bool + }{ + "without comments flag": { + branch: "master", + cli: "123", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + }, + expectedOutputs: []string{ + `some title`, + `1 \x{1f615} • 2 \x{1f440} • 3 \x{2764}\x{fe0f}`, + `some body`, + `———————— Not showing 4 comments ————————`, + `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `4 \x{1f389} • 5 \x{1f604} • 6 \x{1f680}`, + `Comment 5`, + `Use --comments to view the full conversation`, + `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, + }, + }, + "with comments flag": { + branch: "master", + cli: "123 --comments", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "CommentsForPullRequest": "./fixtures/prViewPreviewFullComments.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 pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, + }, + }, + "with invalid comments flag": { + branch: "master", + cli: "123 --comments 3", + wantsErr: true, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + stubSpinner() + http := &httpmock.Registry{} + defer http.Verify(t) + for name, file := range tt.fixtures { + name := fmt.Sprintf(`query %s\b`, name) + http.Register(httpmock.GraphQL(name), httpmock.FileResponse(file)) + } + output, err := runCommand(http, tt.branch, true, tt.cli) + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, "", output.Stderr()) + test.ExpectLines(t, output.String(), tt.expectedOutputs...) + }) + } +} + +func TestPRView_nontty_Comments(t *testing.T) { + tests := map[string]struct { + branch string + cli string + fixtures map[string]string + expectedOutputs []string + wantsErr bool + }{ + "without comments flag": { + branch: "master", + cli: "123", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + }, + expectedOutputs: []string{ + `title:\tsome title`, + `state:\tOPEN`, + `author:\tnobody`, + `url:\thttps://github.com/OWNER/REPO/pull/12`, + `some body`, + }, + }, + "with comments flag": { + branch: "master", + cli: "123 --comments", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "CommentsForPullRequest": "./fixtures/prViewPreviewFullComments.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": { + branch: "master", + cli: "123 --comments 3", + wantsErr: true, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + for name, file := range tt.fixtures { + name := fmt.Sprintf(`query %s\b`, name) + http.Register(httpmock.GraphQL(name), httpmock.FileResponse(file)) + } + output, err := runCommand(http, tt.branch, false, tt.cli) + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, "", output.Stderr()) + test.ExpectLines(t, output.String(), tt.expectedOutputs...) + }) + } +} + +func stubSpinner() { + utils.StartSpinner = func(_ *spinner.Spinner) {} + utils.StopSpinner = func(_ *spinner.Spinner) {} }