diff --git a/pkg/commands/git_commands/github.go b/pkg/commands/git_commands/github.go index b74815301..3a6146923 100644 --- a/pkg/commands/git_commands/github.go +++ b/pkg/commands/git_commands/github.go @@ -85,12 +85,25 @@ type PullRequestNode struct { HeadRepositoryOwner GithubRepositoryOwner `json:"headRepositoryOwner"` State string `json:"state"` IsDraft bool `json:"isDraft"` + HeadRef GithubRef `json:"headRef"` } type GithubRepositoryOwner struct { Login string `json:"login"` } +type GithubRef struct { + Target GithubGitObject `json:"target"` +} + +type GithubGitObject struct { + StatusCheckRollup GithubStatusCheckRollup `json:"statusCheckRollup"` +} + +type GithubStatusCheckRollup struct { + State string `json:"state"` +} + type graphQLRequest struct { Query string `json:"query"` Variables map[string]string `json:"variables"` @@ -121,6 +134,15 @@ func fetchPullRequestsQuery(branches []string, owner string, repo string) (strin number url isDraft + headRef { + target { + ... on Commit { + statusCheckRollup { + state + } + } + } + } headRepositoryOwner { login } @@ -231,9 +253,12 @@ func (self *GitHubCommands) fetchRecentPRsAux(endpoint string, repoOwner string, return nil, err } + return parsePullRequestsResponse(respBytes) +} + +func parsePullRequestsResponse(respBytes []byte) ([]*models.GithubPullRequest, error) { var result Response - err = json.Unmarshal(respBytes, &result) - if err != nil { + if err := json.Unmarshal(respBytes, &result); err != nil { return nil, err } @@ -246,6 +271,7 @@ func (self *GitHubCommands) fetchRecentPRsAux(endpoint string, repoOwner string, Number: node.Number, Title: node.Title, State: lo.Ternary(node.IsDraft && node.State != "CLOSED", "DRAFT", node.State), + ChecksState: node.HeadRef.Target.StatusCheckRollup.State, Url: node.Url, HeadRepositoryOwner: models.GithubRepositoryOwner{ Login: node.HeadRepositoryOwner.Login, diff --git a/pkg/commands/git_commands/github_test.go b/pkg/commands/git_commands/github_test.go index b332ba12a..ce068d750 100644 --- a/pkg/commands/git_commands/github_test.go +++ b/pkg/commands/git_commands/github_test.go @@ -76,6 +76,104 @@ func TestGraphQLEndpoint(t *testing.T) { } } +func TestFetchPullRequestsQueryFetchesOnlyAggregateCheckState(t *testing.T) { + query, variables := fetchPullRequestsQuery([]string{"feature"}, "owner", "repo") + + assert.Contains(t, query, "headRef {") + assert.Contains(t, query, "... on Commit {") + assert.Contains(t, query, "statusCheckRollup {") + assert.NotContains(t, query, "contexts") + assert.Equal(t, map[string]string{ + "owner": "owner", + "repo": "repo", + "branch1": "feature", + }, variables) +} + +func TestParsePullRequestsResponse(t *testing.T) { + t.Run("flattens aliases and normalizes drafts", func(t *testing.T) { + response := []byte(`{ + "data": { + "repository": { + "a1": { + "edges": [ + { + "node": { + "title": "Add feature", + "headRefName": "feature", + "number": 42, + "url": "https://github.com/jesseduffield/lazygit/pull/42", + "headRepositoryOwner": {"login": "contributor"}, + "state": "OPEN", + "isDraft": false, + "headRef": { + "target": { + "statusCheckRollup": {"state": "SUCCESS"} + } + } + } + } + ] + }, + "a2": { + "edges": [ + { + "node": { + "title": "Draft feature", + "headRefName": "draft-feature", + "number": 43, + "url": "https://github.com/jesseduffield/lazygit/pull/43", + "headRepositoryOwner": {"login": "contributor"}, + "state": "OPEN", + "isDraft": true, + "headRef": null + } + } + ] + } + } + } +}`) + + prs, err := parsePullRequestsResponse(response) + + assert.NoError(t, err) + assert.ElementsMatch(t, []*models.GithubPullRequest{ + { + HeadRefName: "feature", + Number: 42, + Title: "Add feature", + State: "OPEN", + ChecksState: "SUCCESS", + Url: "https://github.com/jesseduffield/lazygit/pull/42", + HeadRepositoryOwner: models.GithubRepositoryOwner{Login: "contributor"}, + }, + { + HeadRefName: "draft-feature", + Number: 43, + Title: "Draft feature", + State: "DRAFT", + Url: "https://github.com/jesseduffield/lazygit/pull/43", + HeadRepositoryOwner: models.GithubRepositoryOwner{Login: "contributor"}, + }, + }, prs) + }) + + t.Run("returns an empty slice for an empty result", func(t *testing.T) { + prs, err := parsePullRequestsResponse([]byte(`{"data":{"repository":{}}}`)) + + assert.NoError(t, err) + assert.Empty(t, prs) + }) + + t.Run("rejects malformed JSON", func(t *testing.T) { + prs, err := parsePullRequestsResponse([]byte(`{"data":`)) + + assert.Error(t, err) + assert.Nil(t, prs) + }) +} + func TestGenerateGithubPullRequestMap(t *testing.T) { cases := []struct { name string @@ -99,6 +197,7 @@ func TestGenerateGithubPullRequestMap(t *testing.T) { Number: 42, Title: "Add feature", State: "OPEN", + ChecksState: "PENDING", Url: "https://github.com/jesseduffield/lazygit/pull/42", HeadRepositoryOwner: models.GithubRepositoryOwner{Login: "jesseduffield"}, }, @@ -122,6 +221,7 @@ func TestGenerateGithubPullRequestMap(t *testing.T) { Number: 42, Title: "Add feature", State: "OPEN", + ChecksState: "PENDING", Url: "https://github.com/jesseduffield/lazygit/pull/42", HeadRepositoryOwner: models.GithubRepositoryOwner{Login: "jesseduffield"}, }, diff --git a/pkg/commands/models/github.go b/pkg/commands/models/github.go index 6477c6ee6..da7bd79db 100644 --- a/pkg/commands/models/github.go +++ b/pkg/commands/models/github.go @@ -5,6 +5,7 @@ type GithubPullRequest struct { Number int `json:"number"` Title string `json:"title"` State string `json:"state"` // "MERGED", "OPEN", "CLOSED", "DRAFT" + ChecksState string `json:"checksState"` Url string `json:"url"` HeadRepositoryOwner GithubRepositoryOwner `json:"headRepositoryOwner"` } diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index d81a1402c..a0be00329 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -850,6 +850,7 @@ type CachedPullRequest struct { Number int `yaml:"number"` Title string `yaml:"title"` State string `yaml:"state"` + ChecksState string `yaml:"checksState,omitempty"` Url string `yaml:"url"` HeadRepositoryOwner string `yaml:"headRepositoryOwner"` } diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index a73ee3bc2..cfc46b503 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -5,15 +5,12 @@ import ( "fmt" "strings" - "github.com/gookit/color" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/presentation" - "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" - "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" @@ -214,13 +211,7 @@ func (self *BranchesController) GetOnRenderToMain() func() { pr, ok := self.c.Model().PullRequestsMap[branch.Name] if ok && presentation.ShouldShowPrForBranch(pr, branch.Name, self.c.UserConfig()) { - icon := lo.Ternary(icons.IsIconEnabled(), icons.IconForRemoteUrl(pr.Url)+" ", "") - ptyTask.Prefix = style.PrintHyperlink(fmt.Sprintf("%s%s %s %s\n", - icon, - coloredStateText(pr.State), - pr.Title, - style.FgCyan.Sprintf("#%d", pr.Number)), - pr.Url) + ptyTask.Prefix = presentation.FormatPullRequestHeader(pr, self.c.Tr) ptyTask.Prefix += strings.Repeat("─", self.c.Contexts().Normal.GetView().InnerWidth()) + "\n" } } @@ -236,37 +227,6 @@ func (self *BranchesController) GetOnRenderToMain() func() { } } -func stateText(state string) string { - var icon, label string - switch state { - case "OPEN": - icon, label = " ", "Open" - case "CLOSED": - icon, label = " ", "Closed" - case "MERGED": - icon, label = " ", "Merged" - case "DRAFT": - icon, label = " ", "Draft" - default: - return "" - } - if icons.IsIconEnabled() { - return icon + label - } - return label -} - -func coloredStateText(state string) string { - if icons.IsIconEnabled() { - return fmt.Sprintf("%s%s%s", - presentation.WithPrColor(state, "", false), - presentation.WithPrColor(state, color.RGB(0xFF, 0xFF, 0xFF, false).Sprint(stateText(state)), true), - presentation.WithPrColor(state, "", false)) - } - - return presentation.WithPrColor(state, stateText(state), false) -} - func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branch) error { upstream := lo.Ternary(selectedBranch.RemoteBranchStoredLocally(), selectedBranch.ShortUpstreamRefName(), diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 29784b5ff..29fb66be2 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -1770,6 +1770,7 @@ func (self *RefreshHelper) savePullRequestsToCache(prs []*models.GithubPullReque Number: pr.Number, Title: pr.Title, State: pr.State, + ChecksState: pr.ChecksState, Url: pr.Url, HeadRepositoryOwner: pr.HeadRepositoryOwner.Login, } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index e1d387370..f536d5b64 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -674,6 +674,7 @@ func (gui *Gui) loadCachedPullRequests() []*models.GithubPullRequest { Number: cached.Number, Title: cached.Title, State: cached.State, + ChecksState: cached.ChecksState, Url: cached.Url, HeadRepositoryOwner: models.GithubRepositoryOwner{ Login: cached.HeadRepositoryOwner, diff --git a/pkg/gui/presentation/branches.go b/pkg/gui/presentation/branches.go index 2e8ab0106..f58f34dc6 100644 --- a/pkg/gui/presentation/branches.go +++ b/pkg/gui/presentation/branches.go @@ -150,6 +150,12 @@ func getBranchDisplayStrings( prIcon = "●" } coloredPrIcon = WithPrColor(pr.State, prIcon, false) + if pr.State == "OPEN" { + icon, _, textStyle := checksStatePresentation(pr.ChecksState, tr) + if icon != "" { + coloredPrIcon = textStyle.Sprint(icon) + } + } } res = append(res, coloredPrIcon) @@ -287,6 +293,79 @@ func WithPrColor(state string, text string, isBg bool) string { } } +func FormatPullRequestHeader(pr *models.GithubPullRequest, tr *i18n.TranslationSet) string { + icon := lo.Ternary(icons.IsIconEnabled(), icons.IconForRemoteUrl(pr.Url)+" ", "") + stateText := coloredPullRequestStateText(pr.State) + checksStateText := coloredChecksStateText(pr.ChecksState, tr) + numberText := style.FgCyan.Sprintf("#%d", pr.Number) + + // The checks status links to the checks tab, so it needs to be its own + // hyperlink separate from the rest of the header. + parts := []string{style.PrintHyperlink(icon+stateText, pr.Url)} + if checksStateText != "" { + parts = append(parts, style.PrintHyperlink(checksStateText, strings.TrimSuffix(pr.Url, "/")+"/checks")) + } + parts = append(parts, style.PrintHyperlink(fmt.Sprintf("%s %s\n", pr.Title, numberText), pr.Url)) + + return strings.Join(parts, " ") +} + +func pullRequestStateText(state string) string { + var icon, label string + switch state { + case "OPEN": + icon, label = " ", "Open" + case "CLOSED": + icon, label = " ", "Closed" + case "MERGED": + icon, label = " ", "Merged" + case "DRAFT": + icon, label = " ", "Draft" + default: + return "" + } + if icons.IsIconEnabled() { + return icon + label + } + return label +} + +func coloredPullRequestStateText(state string) string { + if icons.IsIconEnabled() { + return fmt.Sprintf("%s%s%s", + WithPrColor(state, "", false), + WithPrColor(state, color.RGB(0xFF, 0xFF, 0xFF, false).Sprint(pullRequestStateText(state)), true), + WithPrColor(state, "", false)) + } + + return WithPrColor(state, pullRequestStateText(state), false) +} + +func checksStatePresentation(state string, tr *i18n.TranslationSet) (string, string, style.TextStyle) { + switch state { + case "SUCCESS": + return "✓", tr.PullRequestChecksPassing, style.FgGreen + case "PENDING": + return "●", tr.PullRequestChecksPending, style.FgYellow + case "FAILURE": + return "✗", tr.PullRequestChecksFailing, style.FgRed + case "ERROR": + return "!", tr.PullRequestChecksError, style.FgRed + case "EXPECTED": + return "○", tr.PullRequestChecksExpected, style.FgDefault + default: + return "", "", style.Nothing + } +} + +func coloredChecksStateText(state string, tr *i18n.TranslationSet) string { + icon, text, textStyle := checksStatePresentation(state, tr) + if text != "" { + return textStyle.Sprintf("%s %s", icon, text) + } + return "" +} + func ShouldShowPrForBranch(pr *models.GithubPullRequest, branchName string, userConfig *config.UserConfig) bool { if !lo.Contains(userConfig.Git.MainBranches, branchName) { return true diff --git a/pkg/gui/presentation/branches_test.go b/pkg/gui/presentation/branches_test.go index 339f57cc3..3d83ca0ba 100644 --- a/pkg/gui/presentation/branches_test.go +++ b/pkg/gui/presentation/branches_test.go @@ -10,7 +10,9 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/common" "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" + "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/i18n" "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/xo/terminfo" @@ -22,6 +24,84 @@ func makeAtomic(v int32) *atomic.Int32 { return &result } +func TestFormatPullRequestHeader(t *testing.T) { + oldColorLevel := color.ForceSetColorLevel(terminfo.ColorLevelNone) + defer color.ForceSetColorLevel(oldColorLevel) + icons.SetNerdFontsVersion("") + + pr := &models.GithubPullRequest{ + Title: "Improve checks", + Number: 5871, + State: "OPEN", + ChecksState: "SUCCESS", + Url: "https://github.com/jesseduffield/lazygit/pull/5871", + } + numberText := style.FgCyan.Sprint("#5871") + tr := i18n.EnglishTranslationSet() + + t.Run("links checks separately from the rest of the header", func(t *testing.T) { + actual := FormatPullRequestHeader(pr, tr) + + expected := style.PrintHyperlink("Open", pr.Url) + + " " + + style.PrintHyperlink("✓ Passing", pr.Url+"/checks") + + " " + + style.PrintHyperlink("Improve checks "+numberText+"\n", pr.Url) + assert.Equal(t, expected, actual) + }) + + t.Run("leaves the separator unlinked when checks are unavailable", func(t *testing.T) { + prWithoutChecks := *pr + prWithoutChecks.ChecksState = "" + + actual := FormatPullRequestHeader(&prWithoutChecks, tr) + + expected := style.PrintHyperlink("Open", pr.Url) + + " " + + style.PrintHyperlink("Improve checks "+numberText+"\n", pr.Url) + assert.Equal(t, expected, actual) + }) + + t.Run("avoids a double slash in the checks URL", func(t *testing.T) { + prWithTrailingSlash := *pr + prWithTrailingSlash.Url += "/" + + actual := FormatPullRequestHeader(&prWithTrailingSlash, tr) + + assert.Contains(t, actual, "https://github.com/jesseduffield/lazygit/pull/5871/checks") + assert.NotContains(t, actual, "pull/5871//checks") + }) +} + +func TestChecksStatePresentation(t *testing.T) { + tr := i18n.EnglishTranslationSet() + testCases := []struct { + name string + state string + expectedIcon string + expectedText string + expectedStyle style.TextStyle + }{ + {name: "success", state: "SUCCESS", expectedIcon: "✓", expectedText: "Passing", expectedStyle: style.FgGreen}, + {name: "pending", state: "PENDING", expectedIcon: "●", expectedText: "Pending", expectedStyle: style.FgYellow}, + {name: "failure", state: "FAILURE", expectedIcon: "✗", expectedText: "Failing", expectedStyle: style.FgRed}, + {name: "error", state: "ERROR", expectedIcon: "!", expectedText: "Error", expectedStyle: style.FgRed}, + {name: "expected", state: "EXPECTED", expectedIcon: "○", expectedText: "Expected", expectedStyle: style.FgDefault}, + {name: "empty", state: "", expectedIcon: "", expectedText: "", expectedStyle: style.Nothing}, + {name: "unknown", state: "FUTURE_STATE", expectedIcon: "", expectedText: "", expectedStyle: style.Nothing}, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + icon, text, textStyle := checksStatePresentation(testCase.state, tr) + + assert.Equal(t, testCase.expectedIcon, icon) + assert.Equal(t, testCase.expectedText, text) + assert.Equal(t, testCase.expectedStyle, textStyle) + }) + } +} + func Test_getBranchDisplayStrings(t *testing.T) { scenarios := []struct { branch *models.Branch diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 812c30fc5..deb55d6e9 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -367,6 +367,11 @@ type TranslationSet struct { FwdNoLocalUpstream string FwdCommitsToPush string PullRequestNoUpstream string + PullRequestChecksPassing string + PullRequestChecksPending string + PullRequestChecksFailing string + PullRequestChecksError string + PullRequestChecksExpected string ErrorOccurred string ConflictLabel string PendingRebaseTodosSectionHeader string @@ -1519,6 +1524,11 @@ func EnglishTranslationSet() *TranslationSet { FwdNoLocalUpstream: "Cannot fast-forward a branch whose remote is not registered locally", FwdCommitsToPush: "Cannot fast-forward a branch with commits to push", PullRequestNoUpstream: "Cannot open a pull request for a branch with no upstream", + PullRequestChecksPassing: "Passing", + PullRequestChecksPending: "Pending", + PullRequestChecksFailing: "Failing", + PullRequestChecksError: "Error", + PullRequestChecksExpected: "Expected", ErrorOccurred: "An error occurred! Please create an issue at", ConflictLabel: "CONFLICT", PendingRebaseTodosSectionHeader: "Pending rebase todos",