From cd82ed725324624c4ff42beabeb8cdee203635ff Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Sat, 3 Jan 2026 06:50:18 +0900 Subject: [PATCH 1/2] feat: fetch all/more reviewers + show "suggested" (code owner) reviewers too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move ReviewRequests and Reviews from PullRequestData to EnrichedPullRequestData, increasing the limit from 5 to 100. Show a "Loading…" state while enriched data is being fetched. Show reviewers who only left a COMMENTED review (not just APPROVED or CHANGES_REQUESTED). Show suggestedReviewers (code owners who haven't been requested yet) via GitHub's GraphQL API, displayed in faint style with the owner icon. --- internal/data/prapi.go | 23 +++-- internal/tui/components/prview/prview.go | 65 +++++++++---- .../tui/components/prview/reviewers_test.go | 96 ++++++++++++++++++- 3 files changed, 158 insertions(+), 26 deletions(-) diff --git a/internal/data/prapi.go b/internal/data/prapi.go index 638df8372..3eb2ee2e7 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 f3dbbe798..14d843bb4 100644 --- a/internal/tui/components/prview/prview.go +++ b/internal/tui/components/prview/prview.go @@ -325,15 +325,25 @@ 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 "" } @@ -366,7 +376,6 @@ func (m *Model) renderRequestedReviewers() string { stateIcon = m.ctx.Styles.Common.WaitingDotGlyph } - hasOwnerIcon := false if req.IsTeam() { reviewerStr += reviewerStyle.Render(displayName) } else { @@ -376,11 +385,10 @@ 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 { @@ -388,20 +396,42 @@ func (m *Model) renderRequestedReviewers() string { if shownReviewers[login] { continue } - if review.State != "APPROVED" && review.State != "CHANGES_REQUESTED" { + if review.State != "APPROVED" && review.State != "CHANGES_REQUESTED" && review.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 review.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 +446,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 8a42699c2..74715da7b 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,13 @@ 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}, + }, "mix of pending and completed reviews": { reviewRequests: []data.ReviewRequestNode{ { @@ -293,3 +305,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) +} From 7bfcd4398c5c2ed5b1c3023c97d78cfe93e20fd2 Mon Sep 17 00:00:00 2001 From: sideshowbarker Date: Sat, 3 Jan 2026 23:50:01 +0900 Subject: [PATCH 2/2] fix: don't let COMMENTED review override APPROVED or CHANGES_REQUESTED Make a COMMENTED state never override an APPROVED or CHANGES_REQUESTED state, for the purposes of the state indicator we show in Reviewers. Make the indicator stay at the most-significant state. Fixes https://github.com/dlvhdr/gh-dash/pull/728#issuecomment-3707088759 --- internal/tui/components/prview/prview.go | 12 ++++++++---- .../tui/components/prview/reviewers_test.go | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/internal/tui/components/prview/prview.go b/internal/tui/components/prview/prview.go index 14d843bb4..24650b795 100644 --- a/internal/tui/components/prview/prview.go +++ b/internal/tui/components/prview/prview.go @@ -350,6 +350,11 @@ func (m *Model) renderRequestedReviewers() string { 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 } @@ -391,18 +396,17 @@ func (m *Model) renderRequestedReviewers() string { 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" && review.State != "COMMENTED" { + if state != "APPROVED" && state != "CHANGES_REQUESTED" && state != "COMMENTED" { continue } shownReviewers[login] = true var stateIcon string - switch review.State { + switch state { case "APPROVED": stateIcon = successStyle.Render(constants.ApprovedIcon) case "CHANGES_REQUESTED": diff --git a/internal/tui/components/prview/reviewers_test.go b/internal/tui/components/prview/reviewers_test.go index 74715da7b..c71618b4e 100644 --- a/internal/tui/components/prview/reviewers_test.go +++ b/internal/tui/components/prview/reviewers_test.go @@ -202,6 +202,24 @@ func TestRenderRequestedReviewers(t *testing.T) { }, 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{ {