Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions internal/data/prapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
73 changes: 52 additions & 21 deletions internal/tui/components/prview/prview.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand Down
114 changes: 113 additions & 1 deletion internal/tui/components/prview/reviewers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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)
}