diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 638df837..3eb2ee2e 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -17,13 +17,24 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) +type SuggestedReviewer struct { + IsAuthor bool + IsCommenter bool + Reviewer struct { + Login string + } +} + type EnrichedPullRequestData struct { - Url string - Number int - Repository Repository - Commits CommitsWithStatusChecks `graphql:"commits(last: 1)"` - Comments CommentsWithBody `graphql:"comments(last: 50, orderBy: { field: UPDATED_AT, direction: DESC })"` - ReviewThreads ReviewThreadsWithComments `graphql:"reviewThreads(last: 50)"` + Url string + Number int + Repository Repository + Commits CommitsWithStatusChecks `graphql:"commits(last: 1)"` + Comments CommentsWithBody `graphql:"comments(last: 50, orderBy: { field: UPDATED_AT, direction: DESC })"` + ReviewThreads ReviewThreadsWithComments `graphql:"reviewThreads(last: 50)"` + ReviewRequests ReviewRequests `graphql:"reviewRequests(last: 100)"` + Reviews Reviews `graphql:"reviews(last: 100)"` + SuggestedReviewers []SuggestedReviewer } type PullRequestData struct { diff --git a/internal/tui/components/prview/prview.go b/internal/tui/components/prview/prview.go index f3dbbe79..24650b79 100644 --- a/internal/tui/components/prview/prview.go +++ b/internal/tui/components/prview/prview.go @@ -325,21 +325,36 @@ func (m *Model) renderLabels() string { } type reviewerItem struct { - text string - hasOwnerIcon bool + text string } func (m *Model) renderRequestedReviewers() string { - reviewRequests := m.pr.Data.Primary.ReviewRequests.Nodes - reviews := m.pr.Data.Primary.Reviews.Nodes + if !m.pr.Data.IsEnriched { + return lipgloss.JoinVertical( + lipgloss.Left, + m.ctx.Styles.Common.MainTextStyle.Underline(true).Bold(true).Render( + fmt.Sprintf("%s Reviewers", constants.CodeReviewIcon)), + "", + lipgloss.JoinHorizontal(lipgloss.Top, m.ctx.Styles.Common.WaitingGlyph, " ", m.ctx.Styles.Common.FaintTextStyle.Render("Loading...")), + ) + } + + reviewRequests := m.pr.Data.Enriched.ReviewRequests.Nodes + reviews := m.pr.Data.Enriched.Reviews.Nodes + suggestedReviewers := m.pr.Data.Enriched.SuggestedReviewers - if len(reviewRequests) == 0 && len(reviews) == 0 { + if len(reviewRequests) == 0 && len(reviews) == 0 && len(suggestedReviewers) == 0 { return "" } reviewStates := make(map[string]string) for _, review := range reviews { login := review.Author.Login + existingState := reviewStates[login] + // Don't override APPROVED or CHANGES_REQUESTED with COMMENTED + if review.State == "COMMENTED" && (existingState == "APPROVED" || existingState == "CHANGES_REQUESTED") { + continue + } reviewStates[login] = review.State } @@ -366,7 +381,6 @@ func (m *Model) renderRequestedReviewers() string { stateIcon = m.ctx.Styles.Common.WaitingDotGlyph } - hasOwnerIcon := false if req.IsTeam() { reviewerStr += reviewerStyle.Render(displayName) } else { @@ -376,32 +390,52 @@ func (m *Model) renderRequestedReviewers() string { if req.AsCodeOwner { reviewerStr = lipgloss.JoinHorizontal(lipgloss.Top, faintStyle.Render(constants.OwnerIcon), " ", reviewerStr) - hasOwnerIcon = true } reviewerStr = lipgloss.JoinHorizontal(lipgloss.Top, stateIcon, " ", reviewerStr) - reviewerItems = append(reviewerItems, reviewerItem{text: reviewerStr, hasOwnerIcon: hasOwnerIcon}) + reviewerItems = append(reviewerItems, reviewerItem{text: reviewerStr}) } - for _, review := range reviews { - login := review.Author.Login + for login, state := range reviewStates { if shownReviewers[login] { continue } - if review.State != "APPROVED" && review.State != "CHANGES_REQUESTED" { + if state != "APPROVED" && state != "CHANGES_REQUESTED" && state != "COMMENTED" { continue } shownReviewers[login] = true - var reviewerStr string - if review.State == "APPROVED" { - reviewerStr = successStyle.Render(constants.ApprovedIcon) + " " - } else { - reviewerStr = errorStyle.Render(constants.ChangesRequestedIcon) + " " + var stateIcon string + switch state { + case "APPROVED": + stateIcon = successStyle.Render(constants.ApprovedIcon) + case "CHANGES_REQUESTED": + stateIcon = errorStyle.Render(constants.ChangesRequestedIcon) + case "COMMENTED": + stateIcon = m.ctx.Styles.Common.CommentGlyph } - reviewerStr += reviewerStyle.Render("@" + login) + reviewerStr := stateIcon + " " + reviewerStyle.Render("@"+login) - reviewerItems = append(reviewerItems, reviewerItem{text: reviewerStr, hasOwnerIcon: false}) + reviewerItems = append(reviewerItems, reviewerItem{text: reviewerStr}) + } + + // Show suggested reviewers (= code owners) who haven't been requested or reviewed yet + for _, suggested := range suggestedReviewers { + login := suggested.Reviewer.Login + if shownReviewers[login] { + continue + } + if suggested.IsAuthor { + continue + } + shownReviewers[login] = true + + reviewerStr := lipgloss.JoinHorizontal(lipgloss.Top, + faintStyle.Render(constants.OwnerIcon), " ", + faintStyle.Render("@"+login), + ) + + reviewerItems = append(reviewerItems, reviewerItem{text: reviewerStr}) } if len(reviewerItems) == 0 { @@ -416,9 +450,6 @@ func (m *Model) renderRequestedReviewers() string { for i, item := range reviewerItems { itemWidth := lipgloss.Width(item.text) separator := ", " - if item.hasOwnerIcon { - separator = " , " - } separatorWidth := lipgloss.Width(separator) // Check if adding this item would exceed the width diff --git a/internal/tui/components/prview/reviewers_test.go b/internal/tui/components/prview/reviewers_test.go index 8a42699c..c71618b4 100644 --- a/internal/tui/components/prview/reviewers_test.go +++ b/internal/tui/components/prview/reviewers_test.go @@ -38,7 +38,12 @@ func newTestModelWithWidth(t *testing.T, prData *data.PullRequestData, width int m.pr = &prrow.PullRequest{ Ctx: ctx, Data: &prrow.Data{ - Primary: prData, + Primary: prData, + IsEnriched: true, + Enriched: data.EnrichedPullRequestData{ + ReviewRequests: prData.ReviewRequests, + Reviews: prData.Reviews, + }, }, } if width > 0 { @@ -190,6 +195,31 @@ func TestRenderRequestedReviewers(t *testing.T) { }, wantContains: []string{"Reviewers", "@bob", constants.ChangesRequestedIcon}, }, + "reviewer who only commented": { + reviewRequests: []data.ReviewRequestNode{}, + reviews: []data.Review{ + {Author: struct{ Login string }{Login: "charlie"}, State: "COMMENTED"}, + }, + wantContains: []string{"Reviewers", "@charlie", constants.CommentIcon}, + }, + "reviewer who approved then commented": { + reviewRequests: []data.ReviewRequestNode{}, + reviews: []data.Review{ + {Author: struct{ Login string }{Login: "alice"}, State: "APPROVED"}, + {Author: struct{ Login string }{Login: "alice"}, State: "COMMENTED"}, + }, + wantContains: []string{"Reviewers", "@alice", constants.ApprovedIcon}, + wantNotContain: []string{constants.CommentIcon}, + }, + "reviewer who requested changes then commented": { + reviewRequests: []data.ReviewRequestNode{}, + reviews: []data.Review{ + {Author: struct{ Login string }{Login: "bob"}, State: "CHANGES_REQUESTED"}, + {Author: struct{ Login string }{Login: "bob"}, State: "COMMENTED"}, + }, + wantContains: []string{"Reviewers", "@bob", constants.ChangesRequestedIcon}, + wantNotContain: []string{constants.CommentIcon}, + }, "mix of pending and completed reviews": { reviewRequests: []data.ReviewRequestNode{ { @@ -293,3 +323,85 @@ func TestRenderRequestedReviewersWrapping(t *testing.T) { require.Greater(t, len(lines), 3, "expected output to wrap to multiple lines, got %d lines: %q", len(lines), got) } + +func TestRenderRequestedReviewersLoading(t *testing.T) { + cfg, err := config.ParseConfig(config.Location{ + ConfigFlag: "../../../config/testdata/test-config.yml", + }) + require.NoError(t, err) + + thm := theme.ParseTheme(&cfg) + ctx := &context.ProgramContext{ + Config: &cfg, + Theme: thm, + Styles: context.InitStyles(thm), + } + + m := NewModel(ctx) + m.ctx = ctx + m.pr = &prrow.PullRequest{ + Ctx: ctx, + Data: &prrow.Data{ + Primary: &data.PullRequestData{}, + IsEnriched: false, // Not yet enriched - should show loading + }, + } + + got := m.renderRequestedReviewers() + + require.True(t, strings.Contains(got, "Reviewers"), + "expected output to contain 'Reviewers' title, got: %q", got) + require.True(t, strings.Contains(got, "Loading..."), + "expected output to contain 'Loading...', got: %q", got) +} + +func TestRenderSuggestedReviewers(t *testing.T) { + cfg, err := config.ParseConfig(config.Location{ + ConfigFlag: "../../../config/testdata/test-config.yml", + }) + require.NoError(t, err) + + thm := theme.ParseTheme(&cfg) + ctx := &context.ProgramContext{ + Config: &cfg, + Theme: thm, + Styles: context.InitStyles(thm), + } + + m := NewModel(ctx) + m.ctx = ctx + m.pr = &prrow.PullRequest{ + Ctx: ctx, + Data: &prrow.Data{ + Primary: &data.PullRequestData{}, + IsEnriched: true, + Enriched: data.EnrichedPullRequestData{ + ReviewRequests: data.ReviewRequests{}, + Reviews: data.Reviews{}, + SuggestedReviewers: []data.SuggestedReviewer{ + { + IsAuthor: false, + IsCommenter: false, + Reviewer: struct{ Login string }{Login: "codeowner1"}, + }, + { + IsAuthor: true, // Should be skipped + IsCommenter: false, + Reviewer: struct{ Login string }{Login: "author"}, + }, + }, + }, + }, + } + + got := m.renderRequestedReviewers() + + require.True(t, strings.Contains(got, "Reviewers"), + "expected output to contain 'Reviewers' title, got: %q", got) + require.True(t, strings.Contains(got, "@codeowner1"), + "expected output to contain suggested reviewer '@codeowner1', got: %q", got) + require.True(t, strings.Contains(got, constants.OwnerIcon), + "expected output to contain owner icon for suggested reviewer, got: %q", got) + require.False(t, strings.Contains(got, "@author"), + "expected output to NOT contain '@author' (PR author), got: %q", got) +}