diff --git a/api/queries_comments.go b/api/queries_comments.go index 17a7a9f44..16eba6152 100644 --- a/api/queries_comments.go +++ b/api/queries_comments.go @@ -132,3 +132,51 @@ func CommentCreate(client *Client, repoHost string, params CommentCreateInput) ( return mutation.AddComment.CommentEdge.Node.URL, nil } + +func commentsFragment() string { + return `comments(last: 1) { + nodes { + author { + login + } + authorAssociation + body + createdAt + includesCreatedEdit + ` + reactionGroupsFragment() + ` + } + totalCount + }` +} + +func (c Comment) AuthorLogin() string { + return c.Author.Login +} + +func (c Comment) Association() string { + return c.AuthorAssociation +} + +func (c Comment) Content() string { + return c.Body +} + +func (c Comment) Created() time.Time { + return c.CreatedAt +} + +func (c Comment) IsEdited() bool { + return c.IncludesCreatedEdit +} + +func (c Comment) Reactions() ReactionGroups { + return c.ReactionGroups +} + +func (c Comment) Status() string { + return "" +} + +func (c Comment) Link() string { + return "" +} diff --git a/api/queries_pr.go b/api/queries_pr.go index ad4f23644..7e2f05140 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -15,19 +15,6 @@ import ( "github.com/shurcooL/githubv4" ) -type PullRequestReviewState int - -const ( - ReviewApprove PullRequestReviewState = iota - ReviewRequestChanges - ReviewComment -) - -type PullRequestReviewInput struct { - Body string - State PullRequestReviewState -} - type PullRequestsPayload struct { ViewerCreated PullRequestAndTotalCount ReviewRequested PullRequestAndTotalCount @@ -103,14 +90,6 @@ type PullRequest struct { } TotalCount int } - Reviews struct { - Nodes []struct { - Author struct { - Login string - } - State string - } - } Assignees struct { Nodes []struct { Login string @@ -139,6 +118,7 @@ type PullRequest struct { } Comments Comments ReactionGroups ReactionGroups + Reviews PullRequestReviews } type NotFoundError struct { @@ -220,6 +200,18 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { return } +func (pr *PullRequest) DisplayableReviews() PullRequestReviews { + published := []PullRequestReview{} + for _, prr := range pr.Reviews.Nodes { + //Dont display pending reviews + //Dont display commenting reviews without top level comment body + if prr.State != "PENDING" && !(prr.State == "COMMENTED" && prr.Body == "") { + published = append(published, prr) + } + } + return PullRequestReviews{Nodes: published, TotalCount: len(published)} +} + func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNumber int) (io.ReadCloser, error) { url := fmt.Sprintf("%srepos/%s/pulls/%d", ghinstance.RESTPrefix(baseRepo.RepoHost()), ghrepo.FullName(baseRepo), prNumber) @@ -570,15 +562,6 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu } totalCount } - reviews(last: 100) { - nodes { - author { - login - } - state - } - totalCount - } assignees(first: 100) { nodes { login @@ -605,30 +588,8 @@ 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 - } - } + ` + commentsFragment() + ` + ` + reactionGroupsFragment() + ` } } }` @@ -703,15 +664,6 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea } totalCount } - reviews(last: 100) { - nodes { - author { - login - } - state - } - totalCount - } assignees(first: 100) { nodes { login @@ -738,30 +690,8 @@ 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 - } - } + ` + commentsFragment() + ` + ` + reactionGroupsFragment() + ` } } } @@ -906,34 +836,6 @@ func isBlank(v interface{}) bool { } } -func AddReview(client *Client, repo ghrepo.Interface, pr *PullRequest, input *PullRequestReviewInput) error { - var mutation struct { - AddPullRequestReview struct { - ClientMutationID string - } `graphql:"addPullRequestReview(input:$input)"` - } - - state := githubv4.PullRequestReviewEventComment - switch input.State { - case ReviewApprove: - state = githubv4.PullRequestReviewEventApprove - case ReviewRequestChanges: - state = githubv4.PullRequestReviewEventRequestChanges - } - - body := githubv4.String(input.Body) - variables := map[string]interface{}{ - "input": githubv4.AddPullRequestReviewInput{ - PullRequestID: pr.ID, - Event: &state, - Body: &body, - }, - } - - gql := graphQLClient(client.http, repo.RepoHost()) - return gql.MutateNamed(context.Background(), "PullRequestReviewAdd", &mutation, variables) -} - func PullRequestList(client *Client, repo ghrepo.Interface, vars map[string]interface{}, limit int) (*PullRequestAndTotalCount, error) { type prBlock struct { Edges []struct { diff --git a/api/queries_pr_review.go b/api/queries_pr_review.go new file mode 100644 index 000000000..7378db111 --- /dev/null +++ b/api/queries_pr_review.go @@ -0,0 +1,135 @@ +package api + +import ( + "context" + "time" + + "github.com/cli/cli/internal/ghrepo" + "github.com/shurcooL/githubv4" +) + +type PullRequestReviewState int + +const ( + ReviewApprove PullRequestReviewState = iota + ReviewRequestChanges + ReviewComment +) + +type PullRequestReviewInput struct { + Body string + State PullRequestReviewState +} + +type PullRequestReviews struct { + Nodes []PullRequestReview + PageInfo PageInfo + TotalCount int +} + +type PullRequestReview struct { + Author Author + AuthorAssociation string + Body string + CreatedAt time.Time + IncludesCreatedEdit bool + ReactionGroups ReactionGroups + State string + URL string +} + +func AddReview(client *Client, repo ghrepo.Interface, pr *PullRequest, input *PullRequestReviewInput) error { + var mutation struct { + AddPullRequestReview struct { + ClientMutationID string + } `graphql:"addPullRequestReview(input:$input)"` + } + + state := githubv4.PullRequestReviewEventComment + switch input.State { + case ReviewApprove: + state = githubv4.PullRequestReviewEventApprove + case ReviewRequestChanges: + state = githubv4.PullRequestReviewEventRequestChanges + } + + body := githubv4.String(input.Body) + variables := map[string]interface{}{ + "input": githubv4.AddPullRequestReviewInput{ + PullRequestID: pr.ID, + Event: &state, + Body: &body, + }, + } + + gql := graphQLClient(client.http, repo.RepoHost()) + return gql.MutateNamed(context.Background(), "PullRequestReviewAdd", &mutation, variables) +} + +func ReviewsForPullRequest(client *Client, repo ghrepo.Interface, pr *PullRequest) (*PullRequestReviews, error) { + type response struct { + Repository struct { + PullRequest struct { + Reviews PullRequestReviews `graphql:"reviews(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 reviews []PullRequestReview + for { + var query response + err := gql.QueryNamed(context.Background(), "ReviewsForPullRequest", &query, variables) + if err != nil { + return nil, err + } + + reviews = append(reviews, query.Repository.PullRequest.Reviews.Nodes...) + if !query.Repository.PullRequest.Reviews.PageInfo.HasNextPage { + break + } + variables["endCursor"] = githubv4.String(query.Repository.PullRequest.Reviews.PageInfo.EndCursor) + } + + return &PullRequestReviews{Nodes: reviews, TotalCount: len(reviews)}, nil +} + +func (prr PullRequestReview) AuthorLogin() string { + return prr.Author.Login +} + +func (prr PullRequestReview) Association() string { + return prr.AuthorAssociation +} + +func (prr PullRequestReview) Content() string { + return prr.Body +} + +func (prr PullRequestReview) Created() time.Time { + return prr.CreatedAt +} + +func (prr PullRequestReview) IsEdited() bool { + return prr.IncludesCreatedEdit +} + +func (prr PullRequestReview) Reactions() ReactionGroups { + return prr.ReactionGroups +} + +func (prr PullRequestReview) Status() string { + return prr.State +} + +func (prr PullRequestReview) Link() string { + return prr.URL +} diff --git a/api/reaction_groups.go b/api/reaction_groups.go index 381504dd7..849fe4b36 100644 --- a/api/reaction_groups.go +++ b/api/reaction_groups.go @@ -29,3 +29,12 @@ var reactionEmoji = map[string]string{ "ROCKET": "\U0001f680", "EYES": "\U0001f440", } + +func reactionGroupsFragment() string { + return `reactionGroups { + content + users { + totalCount + } + }` +} diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 811f88088..24867b32b 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -46,9 +46,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman Display the title, body, and other information about an issue. With '--web', open the issue in a web browser instead. - `), - Example: heredoc.Doc(` - `), + `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override @@ -110,11 +108,11 @@ func viewRun(opts *ViewOptions) error { defer opts.IO.StopPager() if opts.IO.IsStdoutTTY() { - return printHumanIssuePreview(opts.IO, issue) + return printHumanIssuePreview(opts, issue) } if opts.Comments { - fmt.Fprint(opts.IO.Out, prShared.RawCommentList(issue.Comments)) + fmt.Fprint(opts.IO.Out, prShared.RawCommentList(issue.Comments, api.PullRequestReviews{})) return nil } @@ -141,11 +139,11 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { return nil } -func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { - out := io.Out +func printHumanIssuePreview(opts *ViewOptions, issue *api.Issue) error { + out := opts.IO.Out now := time.Now() ago := now.Sub(issue.CreatedAt) - cs := io.ColorScheme() + cs := opts.IO.ColorScheme() // Header (Title and State) fmt.Fprintln(out, cs.Bold(issue.Title)) @@ -182,21 +180,23 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { } // Body - fmt.Fprintln(out) + var md string + var err error if issue.Body == "" { - issue.Body = "_No description provided_" + md = fmt.Sprintf("\n %s\n\n", cs.Gray("No description provided")) + } else { + style := markdown.GetStyle(opts.IO.TerminalTheme()) + md, err = markdown.Render(issue.Body, style, "") + if err != nil { + return err + } } - style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(issue.Body, style, "") - if err != nil { - return err - } - fmt.Fprint(out, md) - fmt.Fprintln(out) + fmt.Fprintf(out, "\n%s\n", md) // Comments if issue.Comments.TotalCount > 0 { - comments, err := prShared.CommentList(io, issue.Comments) + preview := !opts.Comments + comments, err := prShared.CommentList(opts.IO, issue.Comments, api.PullRequestReviews{}, preview) if err != nil { return err } diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index 82890eac9..a1664aa07 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -368,7 +368,7 @@ func TestIssueView_tty_Comments(t *testing.T) { `some title`, `some body`, `———————— Not showing 4 comments ————————`, - `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `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`, @@ -383,16 +383,16 @@ func TestIssueView_tty_Comments(t *testing.T) { expectedOutputs: []string{ `some title`, `some body`, - `monalisa • Jan 1, 2020 • edited`, + `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`, + `johnnytest \(Contributor\) • Jan 1, 2020`, `Comment 2`, - `elvisp \(member\) • Jan 1, 2020`, + `elvisp \(Member\) • Jan 1, 2020`, `Comment 3`, - `loislane \(owner\) • Jan 1, 2020`, + `loislane \(Owner\) • Jan 1, 2020`, `Comment 4`, - `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `marseilles \(Collaborator\) • Jan 1, 2020 • Newest comment`, `Comment 5`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, }, diff --git a/pkg/cmd/pr/review/review.go b/pkg/cmd/pr/review/review.go index 4bb3317cf..4fb23ec97 100644 --- a/pkg/cmd/pr/review/review.go +++ b/pkg/cmd/pr/review/review.go @@ -60,13 +60,13 @@ func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Co Example: heredoc.Doc(` # approve the pull request of the current branch $ gh pr review --approve - + # leave a review comment for the current branch $ gh pr review --comment -b "interesting" - + # add a review for a specific pull request $ gh pr review 123 - + # request changes on a specific pull request $ gh pr review 123 -r -b "needs more ASCII art" `), diff --git a/pkg/cmd/pr/shared/comments.go b/pkg/cmd/pr/shared/comments.go index f0d820c1f..9f27416a2 100644 --- a/pkg/cmd/pr/shared/comments.go +++ b/pkg/cmd/pr/shared/comments.go @@ -2,6 +2,7 @@ package shared import ( "fmt" + "sort" "strings" "time" @@ -11,37 +12,55 @@ import ( "github.com/cli/cli/utils" ) -func RawCommentList(comments api.Comments) string { +type Comment interface { + AuthorLogin() string + Association() string + Content() string + Created() time.Time + IsEdited() bool + Link() string + Reactions() api.ReactionGroups + Status() string +} + +func RawCommentList(comments api.Comments, reviews api.PullRequestReviews) string { + sortedComments := sortComments(comments, reviews) var b strings.Builder - for _, comment := range comments.Nodes { + for _, comment := range sortedComments { fmt.Fprint(&b, formatRawComment(comment)) } return b.String() } -func formatRawComment(comment api.Comment) string { +func formatRawComment(comment 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.Fprintf(&b, "author:\t%s\n", comment.AuthorLogin()) + fmt.Fprintf(&b, "association:\t%s\n", strings.ToLower(comment.Association())) + fmt.Fprintf(&b, "edited:\t%t\n", comment.IsEdited()) + fmt.Fprintf(&b, "status:\t%s\n", formatRawCommentStatus(comment.Status())) fmt.Fprintln(&b, "--") - fmt.Fprintln(&b, comment.Body) + fmt.Fprintln(&b, comment.Content()) fmt.Fprintln(&b, "--") return b.String() } -func CommentList(io *iostreams.IOStreams, comments api.Comments) (string, error) { +func CommentList(io *iostreams.IOStreams, comments api.Comments, reviews api.PullRequestReviews, preview bool) (string, error) { + sortedComments := sortComments(comments, reviews) + if preview && len(sortedComments) > 0 { + sortedComments = sortedComments[len(sortedComments)-1:] + } var b strings.Builder cs := io.ColorScheme() - retrievedCount := len(comments.Nodes) - hiddenCount := comments.TotalCount - retrievedCount + totalCount := comments.TotalCount + reviews.TotalCount + retrievedCount := len(sortedComments) + hiddenCount := 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 { + for i, comment := range sortedComments { last := i+1 == retrievedCount cmt, err := formatComment(io, comment, last) if err != nil { @@ -61,18 +80,21 @@ func CommentList(io *iostreams.IOStreams, comments api.Comments) (string, error) return b.String(), nil } -func formatComment(io *iostreams.IOStreams, comment api.Comment, newest bool) (string, error) { +func formatComment(io *iostreams.IOStreams, comment 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(comment.AuthorLogin())) + if comment.Status() != "" { + fmt.Fprint(&b, formatCommentStatus(cs, comment.Status())) } - fmt.Fprint(&b, cs.Bold(fmt.Sprintf(" • %s", utils.FuzzyAgoAbbr(time.Now(), comment.CreatedAt)))) - if comment.IncludesCreatedEdit { - fmt.Fprint(&b, cs.Bold(" • edited")) + if comment.Association() != "NONE" { + fmt.Fprint(&b, cs.Bold(fmt.Sprintf(" (%s)", strings.Title(strings.ToLower(comment.Association()))))) + } + fmt.Fprint(&b, cs.Bold(fmt.Sprintf(" • %s", utils.FuzzyAgoAbbr(time.Now(), comment.Created())))) + if comment.IsEdited() { + fmt.Fprint(&b, cs.Bold(" • Edited")) } if newest { fmt.Fprint(&b, cs.Bold(" • ")) @@ -81,20 +103,82 @@ func formatComment(io *iostreams.IOStreams, comment api.Comment, newest bool) (s fmt.Fprintln(&b) // Reactions - if reactions := ReactionGroupList(comment.ReactionGroups); reactions != "" { + if reactions := ReactionGroupList(comment.Reactions()); reactions != "" { fmt.Fprint(&b, reactions) fmt.Fprintln(&b) } // Body - if comment.Body != "" { + var md string + var err error + if comment.Content() == "" { + md = fmt.Sprintf("\n %s\n\n", cs.Gray("No body provided")) + } else { style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(comment.Body, style, "") + md, err = markdown.Render(comment.Content(), style, "") if err != nil { return "", err } - fmt.Fprint(&b, md) + } + fmt.Fprint(&b, md) + + // Footer + if comment.Link() != "" { + fmt.Fprintf(&b, cs.Gray("View the full review: %s\n\n"), comment.Link()) } return b.String(), nil } + +func sortComments(cs api.Comments, rs api.PullRequestReviews) []Comment { + comments := cs.Nodes + reviews := rs.Nodes + var sorted []Comment = make([]Comment, len(comments)+len(reviews)) + + var i int + for _, c := range comments { + sorted[i] = c + i++ + } + for _, r := range reviews { + sorted[i] = r + i++ + } + + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].Created().Before(sorted[j].Created()) + }) + + return sorted +} + +const ( + approvedStatus = "APPROVED" + changesRequestedStatus = "CHANGES_REQUESTED" + commentedStatus = "COMMENTED" + dismissedStatus = "DISMISSED" +) + +func formatCommentStatus(cs *iostreams.ColorScheme, status string) string { + switch status { + case approvedStatus: + return fmt.Sprintf(" %s", cs.Green("approved")) + case changesRequestedStatus: + return fmt.Sprintf(" %s", cs.Red("requested changes")) + case commentedStatus, dismissedStatus: + return fmt.Sprintf(" %s", strings.ToLower(status)) + } + + return "" +} + +func formatRawCommentStatus(status string) string { + if status == approvedStatus || + status == changesRequestedStatus || + status == commentedStatus || + status == dismissedStatus { + return strings.ReplaceAll(strings.ToLower(status), "_", " ") + } + + return "none" +} diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewFullComments.json b/pkg/cmd/pr/view/fixtures/prViewPreviewFullComments.json index cadbf294b..1ee02364a 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewFullComments.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewFullComments.json @@ -69,7 +69,7 @@ }, "authorAssociation": "CONTRIBUTOR", "body": "Comment 2", - "createdAt": "2020-01-01T12:00:00Z", + "createdAt": "2020-01-03T12:00:00Z", "includesCreatedEdit": false, "reactionGroups": [ { @@ -128,7 +128,7 @@ }, "authorAssociation": "MEMBER", "body": "Comment 3", - "createdAt": "2020-01-01T12:00:00Z", + "createdAt": "2020-01-05T12:00:00Z", "includesCreatedEdit": false, "reactionGroups": [ { @@ -187,7 +187,7 @@ }, "authorAssociation": "OWNER", "body": "Comment 4", - "createdAt": "2020-01-01T12:00:00Z", + "createdAt": "2020-01-07T12:00:00Z", "includesCreatedEdit": false, "reactionGroups": [ { @@ -246,7 +246,7 @@ }, "authorAssociation": "COLLABORATOR", "body": "Comment 5", - "createdAt": "2020-01-01T12:00:00Z", + "createdAt": "2020-01-09T12:00:00Z", "includesCreatedEdit": false, "reactionGroups": [ { diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewManyReviews.json b/pkg/cmd/pr/view/fixtures/prViewPreviewManyReviews.json new file mode 100644 index 000000000..5645f7df4 --- /dev/null +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewManyReviews.json @@ -0,0 +1,67 @@ +{ + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [ + { + "author": { + "login": "123" + }, + "state": "COMMENTED" + }, + { + "author": { + "login": "def" + }, + "state": "CHANGES_REQUESTED" + }, + { + "author": { + "login": "abc" + }, + "state": "APPROVED" + }, + { + "author": { + "login": "DEF" + }, + "state": "COMMENTED" + }, + { + "author": { + "login": "xyz" + }, + "state": "APPROVED" + }, + { + "author": { + "login": "" + }, + "state": "APPROVED" + }, + { + "author": { + "login": "hubot" + }, + "state": "CHANGES_REQUESTED" + }, + { + "author": { + "login": "hubot" + }, + "state": "DISMISSED" + }, + { + "author": { + "login": "monalisa" + }, + "state": "PENDING" + } + ], + "totalCount": 9 + } + } + } + } +} diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewNoReviews.json b/pkg/cmd/pr/view/fixtures/prViewPreviewNoReviews.json new file mode 100644 index 000000000..92e1a5a75 --- /dev/null +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewNoReviews.json @@ -0,0 +1 @@ +{ "data": { "repository": { "pullRequest": { "reviews": { } } } } } diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewReviews.json b/pkg/cmd/pr/view/fixtures/prViewPreviewReviews.json new file mode 100644 index 000000000..393003fd9 --- /dev/null +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewReviews.json @@ -0,0 +1,318 @@ +{ + "data": { + "repository": { + "pullRequest": { + "reviews": { + "nodes": [ + { + "author": { + "login": "sam" + }, + "authorAssociation": "NONE", + "body": "Review 1", + "createdAt": "2020-01-02T12: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": 1 + } + }, + { + "content": "THUMBS_UP", + "users": { + "totalCount": 1 + } + } + ], + "state": "COMMENTED", + "url": "https://github.com/OWNER/REPO/pull/12#pullrequestreview-1" + }, + { + "author": { + "login": "matt" + }, + "authorAssociation": "OWNER", + "body": "Review 2", + "createdAt": "2020-01-04T12:00:00Z", + "includesCreatedEdit": false, + "reactionGroups": [ + { + "content": "CONFUSED", + "users": { + "totalCount": 1 + } + }, + { + "content": "EYES", + "users": { + "totalCount": 1 + } + }, + { + "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 + } + } + ], + "state": "CHANGES_REQUESTED", + "url": "https://github.com/OWNER/REPO/pull/12#pullrequestreview-2" + }, + { + "author": { + "login": "leah" + }, + "authorAssociation": "MEMBER", + "body": "Review 3", + "createdAt": "2020-01-06T12:00:00Z", + "includesCreatedEdit": true, + "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 + } + } + ], + "state": "APPROVED", + "url": "https://github.com/OWNER/REPO/pull/12#pullrequestreview-3" + }, + { + "author": { + "login": "louise" + }, + "authorAssociation": "NONE", + "body": "Review 4", + "createdAt": "2020-01-08T12: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 + } + } + ], + "state": "DISMISSED", + "url": "https://github.com/OWNER/REPO/pull/12#pullrequestreview-4" + }, + { + "author": { + "login": "david" + }, + "authorAssociation": "NONE", + "body": "Review 5", + "createdAt": "2020-01-10T12: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 + } + } + ], + "state": "PENDING", + "url": "https://github.com/OWNER/REPO/pull/12#pullrequestreview-5" + } + ], + "totalCount": 5 + } + } + } + } +} diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewSingleComment.json b/pkg/cmd/pr/view/fixtures/prViewPreviewSingleComment.json index c6ddc4321..71d58fe83 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewSingleComment.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewSingleComment.json @@ -93,7 +93,7 @@ }, "authorAssociation": "COLLABORATOR", "body": "Comment 5", - "createdAt": "2020-01-01T12:00:00Z", + "createdAt": "2020-01-09T12:00:00Z", "includesCreatedEdit": false, "reactionGroups": [ { diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json index 0ca6124e6..c6f801477 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json @@ -21,28 +21,6 @@ ], "totalcount": 1 }, - "reviews": { - "nodes": [ - { - "author": { - "login": "3" - }, - "state": "COMMENTED" - }, - { - "author": { - "login": "2" - }, - "state": "APPROVED" - }, - { - "author": { - "login": "1" - }, - "state": "CHANGES_REQUESTED" - } - ] - }, "assignees": { "nodes": [ { diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json index 4d2bd57af..6ff594fec 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json @@ -33,64 +33,6 @@ ], "totalcount": 1 }, - "reviews": { - "nodes": [ - { - "author": { - "login": "123" - }, - "state": "COMMENTED" - }, - { - "author": { - "login": "def" - }, - "state": "CHANGES_REQUESTED" - }, - { - "author": { - "login": "abc" - }, - "state": "APPROVED" - }, - { - "author": { - "login": "DEF" - }, - "state": "COMMENTED" - }, - { - "author": { - "login": "xyz" - }, - "state": "APPROVED" - }, - { - "author": { - "login": "" - }, - "state": "APPROVED" - }, - { - "author": { - "login": "hubot" - }, - "state": "CHANGES_REQUESTED" - }, - { - "author": { - "login": "hubot" - }, - "state": "DISMISSED" - }, - { - "author": { - "login": "monalisa" - }, - "state": "PENDING" - } - ] - }, "assignees": { "nodes": [], "totalcount": 0 diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index ca3f0580d..b2f84ff78 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -6,6 +6,7 @@ import ( "net/http" "sort" "strings" + "sync" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" @@ -80,13 +81,9 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } func viewRun(opts *ViewOptions) error { - httpClient, err := opts.HttpClient() - if err != nil { - return err - } - apiClient := api.NewClientFromHTTP(httpClient) - - pr, repo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + opts.IO.StartProgressIndicator() + pr, err := retrievePullRequest(opts) + opts.IO.StopProgressIndicator() if err != nil { return err } @@ -101,16 +98,6 @@ func viewRun(opts *ViewOptions) error { 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() @@ -120,11 +107,11 @@ func viewRun(opts *ViewOptions) error { defer opts.IO.StopPager() if connectedToTerminal { - return printHumanPrPreview(opts.IO, pr) + return printHumanPrPreview(opts, pr) } if opts.Comments { - fmt.Fprint(opts.IO.Out, shared.RawCommentList(pr.Comments)) + fmt.Fprint(opts.IO.Out, shared.RawCommentList(pr.Comments, pr.Reviews)) return nil } @@ -157,9 +144,9 @@ func printRawPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { return nil } -func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { - out := io.Out - cs := io.ColorScheme() +func printHumanPrPreview(opts *ViewOptions, pr *api.PullRequest) error { + out := opts.IO.Out + cs := opts.IO.ColorScheme() // Header (Title and State) fmt.Fprintln(out, cs.Bold(pr.Title)) @@ -201,21 +188,23 @@ func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { } // Body - fmt.Fprintln(out) + var md string + var err error if pr.Body == "" { - pr.Body = "_No description provided_" + md = fmt.Sprintf("\n %s\n\n", cs.Gray("No description provided")) + } else { + style := markdown.GetStyle(opts.IO.TerminalTheme()) + md, err = markdown.Render(pr.Body, style, "") + if err != nil { + return err + } } - style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(pr.Body, style, "") - if err != nil { - return err - } - fmt.Fprint(out, md) - fmt.Fprintln(out) + fmt.Fprintf(out, "\n%s\n", md) - // Comments - if pr.Comments.TotalCount > 0 { - comments, err := shared.CommentList(io, pr.Comments) + // Reviews and Comments + if pr.Comments.TotalCount > 0 || pr.Reviews.TotalCount > 0 { + preview := !opts.Comments + comments, err := shared.CommentList(opts.IO, pr.Comments, pr.DisplayableReviews(), preview) if err != nil { return err } @@ -405,3 +394,51 @@ func prStateWithDraft(pr *api.PullRequest) string { return pr.State } + +func retrievePullRequest(opts *ViewOptions) (*api.PullRequest, error) { + httpClient, err := opts.HttpClient() + if err != nil { + return nil, err + } + + apiClient := api.NewClientFromHTTP(httpClient) + + pr, repo, err := shared.PRFromArgs(apiClient, opts.BaseRepo, opts.Branch, opts.Remotes, opts.SelectorArg) + if err != nil { + return nil, err + } + + if opts.BrowserMode { + return pr, nil + } + + var errp, errc error + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + var reviews *api.PullRequestReviews + reviews, errp = api.ReviewsForPullRequest(apiClient, repo, pr) + pr.Reviews = *reviews + }() + + if opts.Comments { + wg.Add(1) + go func() { + defer wg.Done() + var comments *api.Comments + comments, errc = api.CommentsForPullRequest(apiClient, repo, pr) + pr.Comments = *comments + }() + } + + wg.Wait() + + if errp != nil { + err = errp + } + if errc != nil { + err = errc + } + return pr, err +} diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index e61ccd614..ff2f255cf 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -168,13 +168,16 @@ func TestPRView_Preview_nontty(t *testing.T) { tests := map[string]struct { branch string args string - fixture string + fixtures map[string]string expectedOutputs []string }{ "Open PR without metadata": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreview.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreview.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are from a fork\n`, `state:\tOPEN\n`, @@ -190,12 +193,15 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Open PR with metadata by number": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewWithMetadataByNumber.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewWithMetadataByNumber.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are from a fork\n`, - `reviewers:\t2 \(Approved\), 3 \(Commented\), 1 \(Requested\)\n`, + `reviewers:\t1 \(Requested\)\n`, `assignees:\tmarseilles, monaco\n`, `labels:\tone, two, three, four, five\n`, `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, @@ -204,9 +210,12 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Open PR with reviewers by number": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewWithReviewersByNumber.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewWithReviewersByNumber.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewManyReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are from a fork\n`, `state:\tOPEN\n`, @@ -220,14 +229,18 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Open PR with metadata by branch": { - branch: "master", - args: "blueberries", - fixture: "./fixtures/prViewPreviewWithMetadataByBranch.json", + branch: "master", + args: "blueberries", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prViewPreviewWithMetadataByBranch.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are a good fruit`, `state:\tOPEN`, `author:\tnobody`, `assignees:\tmarseilles, monaco\n`, + `reviewers:\t\n`, `labels:\tone, two, three, four, five\n`, `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, `milestone:\tuluru\n`, @@ -235,14 +248,18 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Open PR for the current branch": { - branch: "blueberries", - args: "", - fixture: "./fixtures/prView.json", + branch: "blueberries", + args: "", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prView.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are a good fruit`, `state:\tOPEN`, `author:\tnobody`, `assignees:\t\n`, + `reviewers:\t\n`, `labels:\t\n`, `projects:\t\n`, `milestone:\t\n`, @@ -250,23 +267,30 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Open PR wth empty body for the current branch": { - branch: "blueberries", - args: "", - fixture: "./fixtures/prView_EmptyBody.json", + branch: "blueberries", + args: "", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prView_EmptyBody.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are a good fruit`, `state:\tOPEN`, `author:\tnobody`, `assignees:\t\n`, + `reviewers:\t\n`, `labels:\t\n`, `projects:\t\n`, `milestone:\t\n`, }, }, "Closed PR": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewClosedState.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewClosedState.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `state:\tCLOSED\n`, `author:\tnobody\n`, @@ -279,9 +303,12 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Merged PR": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewMergedState.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewMergedState.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `state:\tMERGED\n`, `author:\tnobody\n`, @@ -294,30 +321,38 @@ func TestPRView_Preview_nontty(t *testing.T) { }, }, "Draft PR": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewDraftState.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewDraftState.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are from a fork\n`, `state:\tDRAFT\n`, `author:\tnobody\n`, `labels:`, `assignees:`, + `reviewers:`, `projects:`, `milestone:`, `\*\*blueberries taste good\*\*`, }, }, "Draft PR by branch": { - branch: "master", - args: "blueberries", - fixture: "./fixtures/prViewPreviewDraftStatebyBranch.json", + branch: "master", + args: "blueberries", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prViewPreviewDraftStatebyBranch.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `title:\tBlueberries are a good fruit\n`, `state:\tDRAFT\n`, `author:\tnobody\n`, `labels:`, `assignees:`, + `reviewers:`, `projects:`, `milestone:`, `\*\*blueberries taste good\*\*`, @@ -329,7 +364,10 @@ func TestPRView_Preview_nontty(t *testing.T) { t.Run(name, func(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) + 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, tc.branch, false, tc.args) if err != nil { @@ -347,13 +385,16 @@ func TestPRView_Preview(t *testing.T) { tests := map[string]struct { branch string args string - fixture string + fixtures map[string]string expectedOutputs []string }{ "Open PR without metadata": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreview.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreview.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are from a fork`, `Open.*nobody wants to merge 12 commits into master from blueberries`, @@ -362,13 +403,16 @@ func TestPRView_Preview(t *testing.T) { }, }, "Open PR with metadata by number": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewWithMetadataByNumber.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewWithMetadataByNumber.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are from a fork`, `Open.*nobody wants to merge 12 commits into master from blueberries`, - `Reviewers:.*2 \(.*Approved.*\), 3 \(Commented\), 1 \(.*Requested.*\)\n`, + `Reviewers:.*1 \(.*Requested.*\)\n`, `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`, @@ -378,9 +422,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Open PR with reviewers by number": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewWithReviewersByNumber.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewWithReviewersByNumber.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewManyReviews.json", + }, expectedOutputs: []string{ `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`, @@ -389,9 +436,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Open PR with metadata by branch": { - branch: "master", - args: "blueberries", - fixture: "./fixtures/prViewPreviewWithMetadataByBranch.json", + branch: "master", + args: "blueberries", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prViewPreviewWithMetadataByBranch.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are a good fruit`, `Open.*nobody wants to merge 8 commits into master from blueberries`, @@ -404,9 +454,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Open PR for the current branch": { - branch: "blueberries", - args: "", - fixture: "./fixtures/prView.json", + branch: "blueberries", + args: "", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prView.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are a good fruit`, `Open.*nobody wants to merge 8 commits into master from blueberries`, @@ -415,9 +468,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Open PR wth empty body for the current branch": { - branch: "blueberries", - args: "", - fixture: "./fixtures/prView_EmptyBody.json", + branch: "blueberries", + args: "", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prView_EmptyBody.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are a good fruit`, `Open.*nobody wants to merge 8 commits into master from blueberries`, @@ -425,9 +481,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Closed PR": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewClosedState.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewClosedState.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are from a fork`, `Closed.*nobody wants to merge 12 commits into master from blueberries`, @@ -436,9 +495,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Merged PR": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewMergedState.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewMergedState.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are from a fork`, `Merged.*nobody wants to merge 12 commits into master from blueberries`, @@ -447,9 +509,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Draft PR": { - branch: "master", - args: "12", - fixture: "./fixtures/prViewPreviewDraftState.json", + branch: "master", + args: "12", + fixtures: map[string]string{ + "PullRequestByNumber": "./fixtures/prViewPreviewDraftState.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are from a fork`, `Draft.*nobody wants to merge 12 commits into master from blueberries`, @@ -458,9 +523,12 @@ func TestPRView_Preview(t *testing.T) { }, }, "Draft PR by branch": { - branch: "master", - args: "blueberries", - fixture: "./fixtures/prViewPreviewDraftStatebyBranch.json", + branch: "master", + args: "blueberries", + fixtures: map[string]string{ + "PullRequestForBranch": "./fixtures/prViewPreviewDraftStatebyBranch.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewNoReviews.json", + }, expectedOutputs: []string{ `Blueberries are a good fruit`, `Draft.*nobody wants to merge 8 commits into master from blueberries`, @@ -474,7 +542,10 @@ func TestPRView_Preview(t *testing.T) { t.Run(name, func(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.Register(httpmock.GraphQL(`query PullRequest(ByNumber|ForBranch)\b`), httpmock.FileResponse(tc.fixture)) + 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, tc.branch, true, tc.args) if err != nil { @@ -731,14 +802,15 @@ func TestPRView_tty_Comments(t *testing.T) { branch: "master", cli: "123", fixtures: map[string]string{ - "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewReviews.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`, + `———————— Not showing 8 comments ————————`, + `marseilles \(Collaborator\) • Jan 9, 2020 • Newest comment`, `4 \x{1f389} • 5 \x{1f604} • 6 \x{1f680}`, `Comment 5`, `Use --comments to view the full conversation`, @@ -750,21 +822,36 @@ func TestPRView_tty_Comments(t *testing.T) { cli: "123 --comments", fixtures: map[string]string{ "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewReviews.json", "CommentsForPullRequest": "./fixtures/prViewPreviewFullComments.json", }, expectedOutputs: []string{ `some title`, `some body`, - `monalisa • Jan 1, 2020 • edited`, + `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`, + `sam commented • Jan 2, 2020`, + `1 \x{1f44e} • 1 \x{1f44d}`, + `Review 1`, + `View the full review: https://github.com/OWNER/REPO/pull/12#pullrequestreview-1`, + `johnnytest \(Contributor\) • Jan 3, 2020`, `Comment 2`, - `elvisp \(member\) • Jan 1, 2020`, + `matt requested changes \(Owner\) • Jan 4, 2020`, + `1 \x{1f615} • 1 \x{1f440}`, + `Review 2`, + `View the full review: https://github.com/OWNER/REPO/pull/12#pullrequestreview-2`, + `elvisp \(Member\) • Jan 5, 2020`, `Comment 3`, - `loislane \(owner\) • Jan 1, 2020`, + `leah approved \(Member\) • Jan 6, 2020 • Edited`, + `Review 3`, + `View the full review: https://github.com/OWNER/REPO/pull/12#pullrequestreview-3`, + `loislane \(Owner\) • Jan 7, 2020`, `Comment 4`, - `marseilles \(collaborator\) • Jan 1, 2020 • Newest comment`, + `louise dismissed • Jan 8, 2020`, + `Review 4`, + `View the full review: https://github.com/OWNER/REPO/pull/12#pullrequestreview-4`, + `marseilles \(Collaborator\) • Jan 9, 2020 • Newest comment`, `Comment 5`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, @@ -808,7 +895,8 @@ func TestPRView_nontty_Comments(t *testing.T) { branch: "master", cli: "123", fixtures: map[string]string{ - "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewReviews.json", }, expectedOutputs: []string{ `title:\tsome title`, @@ -823,28 +911,54 @@ func TestPRView_nontty_Comments(t *testing.T) { cli: "123 --comments", fixtures: map[string]string{ "PullRequestByNumber": "./fixtures/prViewPreviewSingleComment.json", + "ReviewsForPullRequest": "./fixtures/prViewPreviewReviews.json", "CommentsForPullRequest": "./fixtures/prViewPreviewFullComments.json", }, expectedOutputs: []string{ `author:\tmonalisa`, - `association:\t`, + `association:\tnone`, `edited:\ttrue`, + `status:\tnone`, `Comment 1`, + `author:\tsam`, + `association:\tnone`, + `edited:\tfalse`, + `status:\tcommented`, + `Review 1`, `author:\tjohnnytest`, `association:\tcontributor`, `edited:\tfalse`, + `status:\tnone`, `Comment 2`, + `author:\tmatt`, + `association:\towner`, + `edited:\tfalse`, + `status:\tchanges requested`, + `Review 2`, `author:\telvisp`, `association:\tmember`, `edited:\tfalse`, + `status:\tnone`, `Comment 3`, + `author:\tleah`, + `association:\tmember`, + `edited:\ttrue`, + `status:\tapproved`, + `Review 3`, `author:\tloislane`, `association:\towner`, `edited:\tfalse`, + `status:\tnone`, `Comment 4`, + `author:\tlouise`, + `association:\tnone`, + `edited:\tfalse`, + `status:\tdismissed`, + `Review 4`, `author:\tmarseilles`, `association:\tcollaborator`, `edited:\tfalse`, + `status:\tnone`, `Comment 5`, }, },