diff --git a/internal/data/graphql_test_helpers_test.go b/internal/data/graphql_test_helpers_test.go new file mode 100644 index 000000000..c4f28eec0 --- /dev/null +++ b/internal/data/graphql_test_helpers_test.go @@ -0,0 +1,18 @@ +package data + +import ( + "net/http" + "testing" + + gh "github.com/cli/go-gh/v2/pkg/api" + graphqltest "github.com/dlvhdr/gh-dash/v4/internal/testhelpers/graphql" +) + +// newMockGraphQLClient creates a GraphQL client backed by the given handler. +// It is a package-local wrapper around graphqltest.NewMockGraphQLClient so +// that internal/data tests can call it without importing the test-helper +// package name explicitly. +func newMockGraphQLClient(t *testing.T, handler http.Handler) *gh.GraphQLClient { + t.Helper() + return graphqltest.NewMockGraphQLClient(t, handler) +} diff --git a/internal/data/prapi.go b/internal/data/prapi.go index da2ff88d2..bfd1f064b 100644 --- a/internal/data/prapi.go +++ b/internal/data/prapi.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "sync" "time" "charm.land/log/v2" @@ -17,6 +18,14 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) +type AutoMergeRequest struct { + EnabledAt time.Time `graphql:"enabledAt"` + MergeMethod string `graphql:"mergeMethod"` + EnabledBy struct { + Login string `graphql:"login"` + } `graphql:"enabledBy"` +} + type SuggestedReviewer struct { IsAuthor bool IsCommenter bool @@ -64,6 +73,7 @@ type EnrichedPullRequestData struct { } type PullRequestData struct { + ID string Number int Title string Body string @@ -96,6 +106,7 @@ type PullRequestData struct { Files ChangedFiles `graphql:"files(first: 5)"` IsDraft bool IsInMergeQueue bool + AutoMergeRequest *AutoMergeRequest Commits Commits `graphql:"commits(last: 1)"` Labels PRLabels `graphql:"labels(first: 6)"` MergeStateStatus MergeStateStatus `graphql:"mergeStateStatus"` @@ -465,6 +476,7 @@ type PullRequestsResponse struct { } var ( + clientMu sync.Mutex client *gh.GraphQLClient cachedClient *gh.GraphQLClient ) @@ -488,6 +500,7 @@ func IsEnrichmentCacheCleared() bool { func FetchPullRequests(query string, limit int, pageInfo *PageInfo) (PullRequestsResponse, error) { var err error + clientMu.Lock() if client == nil { if config.IsFeatureEnabled(config.FF_MOCK_DATA) { log.Info("using mock data", "server", "https://localhost:3000") @@ -501,6 +514,7 @@ func FetchPullRequests(query string, limit int, pageInfo *PageInfo) (PullRequest client, err = gh.DefaultGraphQLClient() } } + clientMu.Unlock() if err != nil { return PullRequestsResponse{}, err @@ -545,12 +559,15 @@ func FetchPullRequests(query string, limit int, pageInfo *PageInfo) (PullRequest func FetchPullRequest(prUrl string) (EnrichedPullRequestData, error) { var err error + clientMu.Lock() if client == nil { client, err = gh.DefaultGraphQLClient() if err != nil { + clientMu.Unlock() return EnrichedPullRequestData{}, err } } + clientMu.Unlock() var queryResult struct { Resource struct { @@ -573,3 +590,168 @@ func FetchPullRequest(prUrl string) (EnrichedPullRequestData, error) { return queryResult.Resource.PullRequest, nil } + +// PRMergeStatus represents the outcome of a merge action. +type PRMergeStatus struct { + State string // OPEN, CLOSED, MERGED + HasAutoMerge bool +} + +// mergeMethodForRepo returns the GraphQL merge method string to use based on +// which methods the repository allows, in priority order: MERGE > SQUASH > REBASE. +// +// NOTE: Future enhancement — when multiple merge methods are allowed, consider: +// - Prompting the user to select from the available options before merging +// - Splitting MergePR into separate MergePR / SquashPR / RebasePR task functions +// so the user can invoke them with explicit keybindings +func mergeMethodForRepo(repo Repository) (graphql.String, error) { + switch { + case repo.AllowMergeCommit: + return "MERGE", nil + case repo.AllowSquashMerge: + return "SQUASH", nil + case repo.AllowRebaseMerge: + return "REBASE", nil + default: + return "", fmt.Errorf("repository has no allowed merge methods") + } +} + +// enableAutoMerge calls the enablePullRequestAutoMerge GraphQL mutation and +// returns the resulting PRMergeStatus. +func enableAutoMerge(prNodeID string, mergeMethod graphql.String) (PRMergeStatus, error) { + var mutation struct { + EnablePullRequestAutoMerge struct { + PullRequest struct { + State string `graphql:"state"` + AutoMergeRequest *struct { + EnabledAt time.Time `graphql:"enabledAt"` + } `graphql:"autoMergeRequest"` + } `graphql:"pullRequest"` + } `graphql:"enablePullRequestAutoMerge(input: $input)"` + } + type EnablePullRequestAutoMergeInput struct { + PullRequestID string `json:"pullRequestId"` + MergeMethod graphql.String `json:"mergeMethod"` + } + variables := map[string]any{ + "input": EnablePullRequestAutoMergeInput{ + PullRequestID: prNodeID, + MergeMethod: mergeMethod, + }, + } + if err := client.Mutate("EnablePullRequestAutoMerge", &mutation, variables); err != nil { + return PRMergeStatus{}, err + } + hasAutoMerge := mutation.EnablePullRequestAutoMerge.PullRequest.AutoMergeRequest != nil + log.Info("Auto-merge enabled for PR", "nodeID", prNodeID, "hasAutoMerge", hasAutoMerge) + return PRMergeStatus{ + State: mutation.EnablePullRequestAutoMerge.PullRequest.State, + HasAutoMerge: hasAutoMerge, + }, nil +} + +// MergePullRequest performs the appropriate merge action via a single GraphQL +// mutation, returning the outcome directly from the mutation response without +// a follow-up query. +// +// The mutation chosen depends on the PR's current mergeStateStatus +// https://docs.github.com/en/graphql/reference/enums#mergestatestatus +// - CLEAN, UNSTABLE, or HAS_HOOKS → mergePullRequest +// - BLOCKED → enablePullRequestAutoMerge +// - BEHIND → enablePullRequestAutoMerge +// - DIRTY → error +// - UNKNOWN → log warning, attempt enablePullRequestAutoMerge +// +// Note: draft PRs are not handled here; callers should check IsDraft before +// invoking this function. +func MergePullRequest(prNodeID string, mergeStateStatus MergeStateStatus, repo Repository) (PRMergeStatus, error) { + clientMu.Lock() + if client == nil { + var err error + if config.IsFeatureEnabled(config.FF_MOCK_DATA) { + log.Info("using mock data", "server", "https://localhost:3000") + http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{InsecureSkipVerify: true} + client, err = gh.NewGraphQLClient(gh.ClientOptions{Host: "localhost:3000", AuthToken: "fake-token"}) + } else { + client, err = gh.DefaultGraphQLClient() + } + if err != nil { + clientMu.Unlock() + return PRMergeStatus{}, err + } + } + clientMu.Unlock() + + log.Debug("Performing PR merge via GraphQL", "nodeID", prNodeID, "mergeStateStatus", mergeStateStatus) + + switch mergeStateStatus { + case "CLEAN", "UNSTABLE", "HAS_HOOKS": + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { + return PRMergeStatus{}, err + } + + var mutation struct { + MergePullRequest struct { + PullRequest struct { + State string `graphql:"state"` + } `graphql:"pullRequest"` + } `graphql:"mergePullRequest(input: $input)"` + } + type MergePullRequestInput struct { + PullRequestID string `json:"pullRequestId"` + MergeMethod graphql.String `json:"mergeMethod"` + } + variables := map[string]any{ + "input": MergePullRequestInput{ + PullRequestID: prNodeID, + MergeMethod: mergeMethod, + }, + } + if err := client.Mutate("MergePullRequest", &mutation, variables); err != nil { + return PRMergeStatus{}, err + } + log.Info("PR merged directly", "nodeID", prNodeID, "state", mutation.MergePullRequest.PullRequest.State) + return PRMergeStatus{State: mutation.MergePullRequest.PullRequest.State}, nil + + case "DIRTY": + return PRMergeStatus{}, fmt.Errorf("PR has merge conflicts, please resolve locally") + + case "BLOCKED", "BEHIND": + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { + return PRMergeStatus{}, err + } + return enableAutoMerge(prNodeID, mergeMethod) + + case "UNKNOWN": + // UNKNOWN is returned by GitHub when the merge state cannot be + // determined (e.g. checks are still pending or the state is + // temporarily unavailable). We optimistically attempt to enable + // auto-merge so that the PR merges automatically once the + // blocking condition clears. The warning is intentionally + // log-only; surfacing it to the user is left as a future + // enhancement (see: https://github.com/dlvhdr/gh-dash/discussions/546). + log.Warn("Unknown merge state status, attempting auto-merge", "status", mergeStateStatus) + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { + return PRMergeStatus{}, err + } + return enableAutoMerge(prNodeID, mergeMethod) + + default: + // Any future MergeStateStatus values not listed above fall here. + // We apply the same optimistic enableAutoMerge strategy rather + // than hard-failing, so that newly-introduced GitHub statuses + // don't silently break the merge workflow. As with UNKNOWN, + // the warning is log-only for now; a user-visible notification + // is a future enhancement. + log.Warn("Unrecognised merge state status, attempting auto-merge", "status", mergeStateStatus) + mergeMethod, err := mergeMethodForRepo(repo) + if err != nil { + return PRMergeStatus{}, err + } + return enableAutoMerge(prNodeID, mergeMethod) + } +} diff --git a/internal/data/prapi_test.go b/internal/data/prapi_test.go index 79af55c19..1abf2fcc4 100644 --- a/internal/data/prapi_test.go +++ b/internal/data/prapi_test.go @@ -1,9 +1,13 @@ package data import ( + "io" + "net/http" + "strings" "testing" gh "github.com/cli/go-gh/v2/pkg/api" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -78,3 +82,278 @@ func TestSetClient(t *testing.T) { require.True(t, IsEnrichmentCacheCleared()) }) } + +// setMockMergeClient installs a GraphQL client backed by the given handler and +// resets the package-level clients afterwards. +func setMockMergeClient(t *testing.T, handler http.Handler) { + t.Helper() + SetClient(newMockGraphQLClient(t, handler)) + t.Cleanup(func() { + client = nil + cachedClient = nil + }) +} + +// mergePullRequestResponse returns a JSON body for a successful mergePullRequest mutation. +func mergePullRequestResponse(state string) string { + return `{"data":{"mergePullRequest":{"pullRequest":{"state":"` + state + `"}}}}` +} + +// enableAutoMergeResponse returns a JSON body for a successful enablePullRequestAutoMerge mutation. +func enableAutoMergeResponse(hasAutoMerge bool) string { + autoMergeValue := "null" + if hasAutoMerge { + autoMergeValue = `{"enabledAt":"2024-01-01T00:00:00Z"}` + } + return `{"data":{"enablePullRequestAutoMerge":{"pullRequest":{"state":"OPEN","autoMergeRequest":` + autoMergeValue + `}}}}` +} + +// graphqlErrorResponse returns a JSON body representing a GraphQL-level error. +func graphqlErrorResponse(message string) string { + return `{"errors":[{"message":"` + message + `","locations":[],"path":[]}]}` +} + +// mergeMockHandler returns an http.Handler that serves controlled responses for +// each of the merge-related GraphQL mutations. Each response body is +// keyed by the operation name embedded in the request body by the GraphQL client. +func mergeMockHandler(t *testing.T, responses map[string]string) http.Handler { + t.Helper() + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + bodyStr := string(body) + + w.Header().Set("Content-Type", "application/json") + switch { + case strings.Contains(bodyStr, "mutation MergePullRequest"): + resp, ok := responses["MergePullRequest"] + if !ok { + t.Error("unexpected call to MergePullRequest mutation") + w.WriteHeader(http.StatusInternalServerError) + return + } + _, _ = io.WriteString(w, resp) + case strings.Contains(bodyStr, "mutation EnablePullRequestAutoMerge"): + resp, ok := responses["EnablePullRequestAutoMerge"] + if !ok { + t.Error("unexpected call to EnablePullRequestAutoMerge mutation") + w.WriteHeader(http.StatusInternalServerError) + return + } + _, _ = io.WriteString(w, resp) + default: + t.Errorf("unexpected GraphQL request body: %s", bodyStr) + w.WriteHeader(http.StatusInternalServerError) + } + }) +} + +// --- mergeMethodForRepo tests --- + +func TestMergeMethodForRepo(t *testing.T) { + tests := []struct { + name string + repo Repository + expectedMethod string + expectError bool + }{ + { + name: "only merge commits allowed", + repo: Repository{AllowMergeCommit: true}, + expectedMethod: "MERGE", + }, + { + name: "only squash merge allowed", + repo: Repository{AllowSquashMerge: true}, + expectedMethod: "SQUASH", + }, + { + name: "only rebase merge allowed", + repo: Repository{AllowRebaseMerge: true}, + expectedMethod: "REBASE", + }, + { + name: "all three allowed - MERGE wins", + repo: Repository{AllowMergeCommit: true, AllowSquashMerge: true, AllowRebaseMerge: true}, + expectedMethod: "MERGE", + }, + { + name: "merge and squash allowed - MERGE wins", + repo: Repository{AllowMergeCommit: true, AllowSquashMerge: true}, + expectedMethod: "MERGE", + }, + { + name: "squash and rebase allowed - SQUASH wins", + repo: Repository{AllowSquashMerge: true, AllowRebaseMerge: true}, + expectedMethod: "SQUASH", + }, + { + name: "no methods allowed returns error", + repo: Repository{}, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + method, err := mergeMethodForRepo(tt.repo) + if tt.expectError { + require.Error(t, err) + assert.Empty(t, method) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectedMethod, string(method)) + } + }) + } +} + +// --- MergePullRequest tests --- + +func TestMergePullRequest_DirectMerge_Clean(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": mergePullRequestResponse("MERGED"), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_node123", "CLEAN", repo) + + require.NoError(t, err) + assert.Equal(t, "MERGED", status.State) + assert.False(t, status.HasAutoMerge) +} + +func TestMergePullRequest_DirectMerge_Unstable(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": mergePullRequestResponse("MERGED"), + })) + + repo := Repository{AllowSquashMerge: true} + status, err := MergePullRequest("PR_node456", "UNSTABLE", repo) + + require.NoError(t, err) + assert.Equal(t, "MERGED", status.State) + assert.False(t, status.HasAutoMerge) +} + +func TestMergePullRequest_Blocked_EnablesAutoMerge(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": enableAutoMergeResponse(true), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_node789", "BLOCKED", repo) + + require.NoError(t, err) + assert.Equal(t, "OPEN", status.State) + assert.True(t, status.HasAutoMerge) +} + +func TestMergePullRequest_AutoMergeEnabled(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": enableAutoMergeResponse(true), + })) + + repo := Repository{AllowMergeCommit: true} + // Any status other than CLEAN, UNSTABLE, or BLOCKED falls through to auto-merge. + status, err := MergePullRequest("PR_node999", "BEHIND", repo) + + require.NoError(t, err) + assert.True(t, status.HasAutoMerge) +} + +// TestMergePullRequest_MockDataFlag_UsesLocalServer verifies that when the +// FF_MOCK_DATA environment variable is set, MergePullRequest initialises the +// GraphQL client targeting localhost:3000 rather than calling +// gh.DefaultGraphQLClient(). Because the production code hardcodes the host +// and also replaces http.DefaultTransport's TLS config, we cannot intercept +// the connection via a test server. Instead we confirm the behaviour +// indirectly: with FF_MOCK_DATA set and client == nil, MergePullRequest must +// return an error that references localhost:3000 (connection refused), proving +// it attempted the mock-data host rather than the real GitHub API (which would +// produce a credentials error, not a dial error to localhost). +func TestMergePullRequest_MockDataFlag_UsesLocalServer(t *testing.T) { + // Ensure client starts nil so MergePullRequest performs lazy init. + originalClient := client + client = nil + t.Cleanup(func() { client = originalClient }) + + // Activate the feature flag. No server is listening on localhost:3000, + // so the call will fail — but with a dial error to localhost:3000, which + // is what we want to assert. + t.Setenv("FF_MOCK_DATA", "1") + + repo := Repository{AllowMergeCommit: true} + _, err := MergePullRequest("PR_mockflag", "CLEAN", repo) + + require.Error(t, err, "MergePullRequest should fail when no mock server is running") + assert.Contains(t, err.Error(), "localhost:3000", + "error should reference localhost:3000, confirming the mock-data init path was taken") +} + +func TestMergePullRequest_DirectMerge_HasHooks(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": mergePullRequestResponse("MERGED"), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_hooks", "HAS_HOOKS", repo) + + require.NoError(t, err) + assert.Equal(t, "MERGED", status.State) + assert.False(t, status.HasAutoMerge) +} + +func TestMergePullRequest_Behind_EnablesAutoMerge(t *testing.T) { + setMockMergeClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": enableAutoMergeResponse(true), + })) + + repo := Repository{AllowMergeCommit: true} + status, err := MergePullRequest("PR_behind", "BEHIND", repo) + + require.NoError(t, err) + assert.True(t, status.HasAutoMerge) +} + +// TestClientInit_ConcurrentCallsAreSafe verifies that concurrent calls to +// FetchPullRequests, FetchPullRequest, and MergePullRequest do not race on the +// package-level client variable. With -race this test will fail unless every +// lazy-init path is protected by clientMu. +// +// The test intentionally leaves client == nil so each goroutine tries to +// initialise it. With FF_MOCK_DATA unset the real gh.DefaultGraphQLClient() +// is called, which will return an error (no credentials in CI) — that is fine; +// we only care that the race detector does not fire. +func TestClientInit_ConcurrentCallsAreSafe(t *testing.T) { + // Reset to nil so every goroutine races to initialise. + originalClient := client + client = nil + t.Cleanup(func() { client = originalClient }) + + const goroutines = 8 + done := make(chan struct{}) + go func() { + defer close(done) + errs := make(chan error, goroutines*3) + for i := range goroutines { + go func(i int) { + _, err := FetchPullRequests("repo:test/test", 1, nil) + errs <- err + }(i) + go func(i int) { + _, err := FetchPullRequest("https://github.com/test/test/pull/1") + errs <- err + }(i) + go func(i int) { + _, err := MergePullRequest("PR_race", "CLEAN", + Repository{AllowMergeCommit: true}) + errs <- err + }(i) + } + for range goroutines * 3 { + <-errs + } + }() + <-done +} diff --git a/internal/data/repository.go b/internal/data/repository.go index 72c9d9a49..e6e11b111 100644 --- a/internal/data/repository.go +++ b/internal/data/repository.go @@ -18,5 +18,8 @@ type Repository struct { Name string NameWithOwner string IsArchived bool + AllowMergeCommit bool `graphql:"mergeCommitAllowed"` + AllowSquashMerge bool `graphql:"squashMergeAllowed"` + AllowRebaseMerge bool `graphql:"rebaseMergeAllowed"` BranchProtectionRules BranchProtectionRules `graphql:"branchProtectionRules(first: 1)"` } diff --git a/internal/testhelpers/graphql/helpers.go b/internal/testhelpers/graphql/helpers.go new file mode 100644 index 000000000..4d621afae --- /dev/null +++ b/internal/testhelpers/graphql/helpers.go @@ -0,0 +1,39 @@ +// Package graphqltest provides shared test helpers for creating mock GraphQL +// clients backed by in-process HTTP handlers. +package graphqltest + +import ( + "net/http" + "net/http/httptest" + "testing" + + gh "github.com/cli/go-gh/v2/pkg/api" + "github.com/stretchr/testify/require" +) + +// LocalRoundTripper is an http.RoundTripper that routes requests directly to +// an in-process handler, avoiding a real network connection. +type LocalRoundTripper struct { + Handler http.Handler +} + +// RoundTrip implements http.RoundTripper. +func (l LocalRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + w := httptest.NewRecorder() + l.Handler.ServeHTTP(w, req) + return w.Result(), nil +} + +// NewMockGraphQLClient creates a *gh.GraphQLClient backed by the given handler. +// It is intended for use in tests that need to intercept GraphQL mutations +// without a real network connection. +func NewMockGraphQLClient(t *testing.T, handler http.Handler) *gh.GraphQLClient { + t.Helper() + c, err := gh.NewGraphQLClient(gh.ClientOptions{ + Transport: LocalRoundTripper{Handler: handler}, + Host: "localhost:3000", + AuthToken: "fake-token", + }) + require.NoError(t, err) + return c +} diff --git a/internal/tui/components/branch/branch.go b/internal/tui/components/branch/branch.go index 77cf1282d..6f1ea70ab 100644 --- a/internal/tui/components/branch/branch.go +++ b/internal/tui/components/branch/branch.go @@ -20,6 +20,12 @@ type Branch struct { PR *data.PullRequestData Data git.Branch Columns []table.Column + // AutoMergeEnabled is a UI-layer flag set when the user enables auto-merge + // via the TUI. It is NOT fetched from the API; it allows the branch + // section to show the auto-merge icon immediately without waiting for a + // full refresh. The flag is stored on a map in reposection.Model and + // re-applied each time updateBranchesWithPrs() rebuilds this struct. + AutoMergeEnabled bool } func (b *Branch) getTextStyle() lipgloss.Style { @@ -57,6 +63,9 @@ func (b *Branch) renderState() string { switch b.PR.State { case "OPEN": + if b.PR.AutoMergeRequest != nil || b.AutoMergeEnabled { + return mergeCellStyle.Foreground(b.Ctx.Theme.WarningText).Render(constants.AutoMergeIcon) + } if b.PR.IsDraft { return mergeCellStyle.Foreground(b.Ctx.Theme.FaintText).Render(constants.DraftIcon) } else { @@ -207,8 +216,14 @@ func (b *Branch) renderBaseName() string { } func (b *Branch) RenderState() string { + if b.PR == nil { + return "" + } switch b.PR.State { case "OPEN": + if b.PR.AutoMergeRequest != nil || b.AutoMergeEnabled { + return constants.AutoMergeIcon + " Auto-merge" + } if b.PR.IsDraft { return constants.DraftIcon + " Draft" } else { diff --git a/internal/tui/components/branch/branch_test.go b/internal/tui/components/branch/branch_test.go new file mode 100644 index 000000000..67bc7f917 --- /dev/null +++ b/internal/tui/components/branch/branch_test.go @@ -0,0 +1,78 @@ +package branch + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" +) + +// newTestBranch creates a minimal Branch with a PR having the given state and +// auto-merge configuration. +func newTestBranch(prState string, autoMergeRequest *data.AutoMergeRequest, autoMergeEnabled bool) Branch { + return Branch{ + PR: &data.PullRequestData{ + State: prState, + AutoMergeRequest: autoMergeRequest, + }, + AutoMergeEnabled: autoMergeEnabled, + } +} + +// TestRenderState_AutoMerge_ViaAutoMergeRequest verifies that RenderState +// returns the auto-merge icon when AutoMergeRequest is non-nil. +func TestRenderState_AutoMerge_ViaAutoMergeRequest(t *testing.T) { + b := newTestBranch("OPEN", &data.AutoMergeRequest{}, false) + state := b.RenderState() + assert.True(t, + strings.Contains(state, constants.AutoMergeIcon), + "RenderState should include AutoMergeIcon when AutoMergeRequest is set; got %q", state) +} + +// TestRenderState_AutoMerge_ViaLocalFlag verifies that RenderState returns the +// auto-merge icon when the local AutoMergeEnabled flag is true. +func TestRenderState_AutoMerge_ViaLocalFlag(t *testing.T) { + b := newTestBranch("OPEN", nil, true) + state := b.RenderState() + assert.True(t, + strings.Contains(state, constants.AutoMergeIcon), + "RenderState should include AutoMergeIcon when AutoMergeEnabled flag is true; got %q", state) +} + +// TestRenderState_Open_NoDraft verifies the normal open PR state. +func TestRenderState_Open_NoDraft(t *testing.T) { + b := newTestBranch("OPEN", nil, false) + state := b.RenderState() + assert.Contains(t, state, constants.OpenIcon) + assert.NotContains(t, state, constants.AutoMergeIcon) +} + +// TestRenderState_Draft verifies the draft PR state. +func TestRenderState_Draft(t *testing.T) { + b := Branch{ + PR: &data.PullRequestData{ + State: "OPEN", + IsDraft: true, + }, + } + state := b.RenderState() + assert.Contains(t, state, constants.DraftIcon) + assert.Contains(t, state, "Draft") +} + +// TestRenderState_Merged verifies the merged PR state. +func TestRenderState_Merged(t *testing.T) { + b := newTestBranch("MERGED", nil, false) + state := b.RenderState() + assert.Contains(t, state, constants.MergedIcon) +} + +// TestRenderState_Closed verifies the closed PR state. +func TestRenderState_Closed(t *testing.T) { + b := newTestBranch("CLOSED", nil, false) + state := b.RenderState() + assert.Contains(t, state, constants.ClosedIcon) +} diff --git a/internal/tui/components/prrow/data.go b/internal/tui/components/prrow/data.go index b35e0d8d4..92e8b7ac0 100644 --- a/internal/tui/components/prrow/data.go +++ b/internal/tui/components/prrow/data.go @@ -7,9 +7,10 @@ import ( ) type Data struct { - Primary *data.PullRequestData - Enriched data.EnrichedPullRequestData - IsEnriched bool + Primary *data.PullRequestData + Enriched data.EnrichedPullRequestData + IsEnriched bool + AutoMergeEnabled bool // Set locally when auto-merge is enabled via the UI; not queried from the API. } func (data Data) GetTitle() string { @@ -35,3 +36,14 @@ func (data Data) GetUpdatedAt() time.Time { func (data Data) GetCreatedAt() time.Time { return data.Primary.CreatedAt } + +// GetPrimaryPRData returns the underlying PullRequestData, satisfying the +// primaryPRDataProvider interface consumed by tasks.extractPullRequestData +// (see internal/tui/components/tasks/pr.go). The method is on a pointer +// receiver, so only *Data satisfies primaryPRDataProvider — a caller holding +// a plain Data value (non-pointer) will not match the interface case and +// extractPullRequestData will return nil. All prrow rows are stored and +// passed as *Data throughout the codebase, so this is the expected usage. +func (d *Data) GetPrimaryPRData() *data.PullRequestData { + return d.Primary +} diff --git a/internal/tui/components/prrow/prrow.go b/internal/tui/components/prrow/prrow.go index c325f75fb..1ee43c3ca 100644 --- a/internal/tui/components/prrow/prrow.go +++ b/internal/tui/components/prrow/prrow.go @@ -80,6 +80,9 @@ func (pr *PullRequest) renderState() string { return mergeCellStyle.Foreground(pr.Ctx.Theme.WarningText). Render(constants.MergeQueueIcon) } + if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.AutoMergeEnabled { + return mergeCellStyle.Foreground(pr.Ctx.Theme.WarningText).Render(constants.AutoMergeIcon) + } if pr.Data.Primary.IsDraft { return mergeCellStyle.Foreground(pr.Ctx.Theme.FaintText).Render(constants.DraftIcon) } else { @@ -302,6 +305,9 @@ func (pr *PullRequest) RenderState() string { if pr.Data.Primary.IsInMergeQueue { return constants.MergeQueueIcon + " Queued" } + if pr.Data.Primary.AutoMergeRequest != nil || pr.Data.AutoMergeEnabled { + return constants.AutoMergeIcon + " Auto-merge" + } if pr.Data.Primary.IsDraft { return constants.DraftIcon + " Draft" } else { diff --git a/internal/tui/components/prssection/prssection.go b/internal/tui/components/prssection/prssection.go index e834a45c4..c0e762e17 100644 --- a/internal/tui/components/prssection/prssection.go +++ b/internal/tui/components/prssection/prssection.go @@ -187,6 +187,11 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { currPr.Primary.State = "MERGED" currPr.Primary.Mergeable = "" } + if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { + // Set a local flag so the auto-merge icon renders immediately + // without waiting for a full refresh. + currPr.AutoMergeEnabled = true + } m.Prs[i] = currPr m.SetIsLoading(false) m.Table.SetRows(m.BuildRows()) diff --git a/internal/tui/components/prssection/prssection_test.go b/internal/tui/components/prssection/prssection_test.go index 0f6974371..5ef52faae 100644 --- a/internal/tui/components/prssection/prssection_test.go +++ b/internal/tui/components/prssection/prssection_test.go @@ -2,15 +2,19 @@ package prssection import ( "testing" + "time" tea "charm.land/bubbletea/v2" "github.com/stretchr/testify/require" + "github.com/dlvhdr/gh-dash/v4/internal/config" "github.com/dlvhdr/gh-dash/v4/internal/data" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prompt" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" "github.com/dlvhdr/gh-dash/v4/internal/tui/components/section" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" ) // newTestModel creates a minimal Model with the prompt confirmation box @@ -36,6 +40,39 @@ func newTestModel(action string) Model { return m } +// newFullTestModel creates a Model via the real NewModel constructor (so that +// all internal sub-models — Table, SearchBar, etc. — are properly wired) and +// then injects a single PR row with the given PR number. +// +// ctx.Config, ctx.Theme, and ctx.Styles are all populated so that +// m.Update() can invoke BuildRows() end-to-end without panicking. +func newFullTestModel(prNumber int) Model { + thm := *theme.DefaultTheme + s := context.InitStyles(thm) + cfg := &config.Config{ + // A non-nil ThemeConfig is required because table.NewModel and + // prrow.ToTableRow both dereference Config.Theme.Ui.Table fields. + Theme: &config.ThemeConfig{}, + } + + ctx := &context.ProgramContext{ + Config: cfg, + Theme: thm, + Styles: s, + StartTask: func(task context.Task) tea.Cmd { + return func() tea.Msg { return nil } + }, + } + + // Use the real constructor so the embedded Table, SearchBar, and + // PromptConfirmationBox are fully initialised with the right ctx. + m := NewModel(0, ctx, config.PrsSectionConfig{}, time.Time{}, time.Time{}) + m.Prs = []prrow.Data{ + {Primary: &data.PullRequestData{Number: prNumber}}, + } + return m +} + func TestConfirmation_AcceptWithEmptyInput(t *testing.T) { // Pressing Enter without typing anything should confirm, since the // prompt says (Y/n) indicating Y is the default. @@ -105,6 +142,31 @@ func TestConfirmation_CancelWithCtrlC(t *testing.T) { _ = cmd } +func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { + // Test that when UpdatePRMsg with AutoMergeEnabled=true is processed via + // m.Update(), the AutoMergeEnabled flag on prrow.Data is set to true. + // + // This test uses newFullTestModel() which wires up ctx.Config, ctx.Theme, + // and ctx.Styles so that Update() can call BuildRows() end-to-end. + m := newFullTestModel(42) + + require.False(t, m.Prs[0].AutoMergeEnabled, "AutoMergeEnabled should start false") + + autoMerge := true + msg := tasks.UpdatePRMsg{ + PrNumber: 42, + AutoMergeEnabled: &autoMerge, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + require.True(t, updated.Prs[0].AutoMergeEnabled, + "AutoMergeEnabled should be set to true after processing AutoMergeEnabled update") + require.Nil(t, updated.Prs[0].Primary.AutoMergeRequest, + "AutoMergeRequest should remain nil (only real API data should populate it)") +} + func TestConfirmation_AllActions(t *testing.T) { actions := []string{"close", "reopen", "ready", "merge", "update", "approveWorkflows"} diff --git a/internal/tui/components/reposection/commands.go b/internal/tui/components/reposection/commands.go index 308b2a5b4..efee9da5f 100644 --- a/internal/tui/components/reposection/commands.go +++ b/internal/tui/components/reposection/commands.go @@ -16,6 +16,10 @@ import ( "github.com/dlvhdr/gh-dash/v4/internal/tui/context" ) +// UpdatePRMsg carries PR state changes emitted by repo-section-specific +// commands (e.g. inline comment, assignee edits). Tasks that are shared +// between the PR-list section and the repo section (such as MergePR, ClosePR) +// emit tasks.UpdatePRMsg instead, which is handled directly in Update(). type UpdatePRMsg struct { PrNumber int IsClosed *bool diff --git a/internal/tui/components/reposection/reposection.go b/internal/tui/components/reposection/reposection.go index e91de1924..0b8ec1acb 100644 --- a/internal/tui/components/reposection/reposection.go +++ b/internal/tui/components/reposection/reposection.go @@ -29,11 +29,15 @@ const SectionType = "repo" type Model struct { section.BaseModel - repo *git.Repo - Branches []branch.Branch - Prs []data.PullRequestData - isRefreshSetUp bool - refreshId int + repo *git.Repo + Branches []branch.Branch + Prs []data.PullRequestData + // autoMergeEnabledPRs tracks PR numbers for which the user has enabled + // auto-merge via the TUI. It persists across updateBranchesWithPrs() + // rebuilds, which recreate m.Branches from scratch on every Update call. + autoMergeEnabledPRs map[int]bool + isRefreshSetUp bool + refreshId int } func NewModel( @@ -60,6 +64,7 @@ func NewModel( m.repo = &git.Repo{Branches: []git.Branch{}} m.Branches = []branch.Branch{} m.Prs = []data.PullRequestData{} + m.autoMergeEnabledPRs = make(map[int]bool) m.isRefreshSetUp = false return m @@ -169,6 +174,24 @@ func (m *Model) Update(msg tea.Msg) (section.Section, tea.Cmd) { m.Prs = append(m.Prs, *msg.NewPr) } + case tasks.UpdatePRMsg: + for i, pr := range m.Prs { + if pr.Number != msg.PrNumber { + continue + } + if msg.IsMerged != nil && *msg.IsMerged { + m.Prs[i].State = "MERGED" + m.Prs[i].Mergeable = "" + } + if msg.AutoMergeEnabled != nil && *msg.AutoMergeEnabled { + // Record the flag in the persistent map so it survives + // updateBranchesWithPrs() rebuilds. + m.autoMergeEnabledPRs[msg.PrNumber] = true + } + m.Table.SetRows(m.BuildRows()) + break + } + case repoMsg: m.repo = msg.repo m.SetIsLoading(false) @@ -366,6 +389,9 @@ func (m *Model) updateBranchesWithPrs() { for _, ref := range m.repo.Branches { b := branch.Branch{Ctx: m.Ctx, Data: ref, Columns: m.Table.Columns} b.PR = findPRForRef(m.Prs, ref.Name) + if b.PR != nil { + b.AutoMergeEnabled = m.autoMergeEnabledPRs[b.PR.Number] + } branches = append(branches, b) } diff --git a/internal/tui/components/reposection/reposection_test.go b/internal/tui/components/reposection/reposection_test.go new file mode 100644 index 000000000..2fe3a9415 --- /dev/null +++ b/internal/tui/components/reposection/reposection_test.go @@ -0,0 +1,129 @@ +package reposection + +import ( + "testing" + "time" + + tea "charm.land/bubbletea/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dlvhdr/gh-dash/v4/internal/config" + "github.com/dlvhdr/gh-dash/v4/internal/data" + "github.com/dlvhdr/gh-dash/v4/internal/git" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/tasks" + "github.com/dlvhdr/gh-dash/v4/internal/tui/context" + "github.com/dlvhdr/gh-dash/v4/internal/tui/theme" +) + +// newTestRepoModel creates a Model via the real NewModel constructor so that +// all internal sub-models are properly wired. A single git branch and a +// matching PR are injected so that UpdatePRMsg can find the PR and +// updateBranchesWithPrs can populate m.Branches. +func newTestRepoModel(prNumber int) Model { + thm := *theme.DefaultTheme + s := context.InitStyles(thm) + cfg := &config.Config{ + // A non-nil ThemeConfig is required because ToTableRow dereferences + // Config.Theme.Ui.Table fields. + Theme: &config.ThemeConfig{}, + } + ctx := &context.ProgramContext{ + Config: cfg, + Theme: thm, + Styles: s, + StartTask: func(task context.Task) tea.Cmd { + return func() tea.Msg { return nil } + }, + } + + t := config.RepoView + m := NewModel(0, ctx, config.PrsSectionConfig{Type: &t}, time.Time{}) + + const branchName = "feature/test" + // Inject a real git.Repo so updateBranchesWithPrs has something to iterate. + m.repo = &git.Repo{ + Branches: []git.Branch{ + {Name: branchName}, + }, + } + m.Prs = []data.PullRequestData{ + {Number: prNumber, HeadRefName: branchName}, + } + return m +} + +// TestUpdatePRMsg_AutoMergeEnabled_SetsFlag verifies that when an UpdatePRMsg +// with AutoMergeEnabled=true is processed: +// - the PR number is recorded in the autoMergeEnabledPRs map, and +// - the corresponding Branch in m.Branches has AutoMergeEnabled set to true. +func TestUpdatePRMsg_AutoMergeEnabled_SetsFlag(t *testing.T) { + m := newTestRepoModel(42) + + require.False(t, m.autoMergeEnabledPRs[42], "autoMergeEnabledPRs should start empty") + + autoMerge := true + msg := tasks.UpdatePRMsg{ + PrNumber: 42, + AutoMergeEnabled: &autoMerge, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + assert.True(t, updated.autoMergeEnabledPRs[42], + "autoMergeEnabledPRs[42] should be true after processing UpdatePRMsg") + + // The flag must also be visible on the corresponding Branch so that + // renderState / RenderState can show the auto-merge icon immediately. + require.Len(t, updated.Branches, 1, "expected one branch after update") + assert.True(t, updated.Branches[0].AutoMergeEnabled, + "Branch.AutoMergeEnabled should be true after processing UpdatePRMsg") + + // AutoMergeRequest must not be touched — only real API data should set it. + assert.Nil(t, updated.Branches[0].PR.AutoMergeRequest, + "AutoMergeRequest should remain nil (only real API data should populate it)") +} + +// TestUpdatePRMsg_IsMerged_SetsState verifies that when an UpdatePRMsg with +// IsMerged=true is processed, the PR state is set to "MERGED". +func TestUpdatePRMsg_IsMerged_SetsState(t *testing.T) { + m := newTestRepoModel(7) + m.Prs[0].State = "OPEN" + + isMerged := true + msg := tasks.UpdatePRMsg{ + PrNumber: 7, + IsMerged: &isMerged, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + assert.Equal(t, "MERGED", updated.Prs[0].State) + assert.Empty(t, updated.Prs[0].Mergeable) +} + +// TestUpdatePRMsg_UnknownPR_NoChange verifies that a message for an unknown +// PR number does not mutate any existing PR or branch. +func TestUpdatePRMsg_UnknownPR_NoChange(t *testing.T) { + m := newTestRepoModel(1) + m.Prs[0].State = "OPEN" + + autoMerge := true + msg := tasks.UpdatePRMsg{ + PrNumber: 999, + AutoMergeEnabled: &autoMerge, + } + + result, _ := m.Update(msg) + updated := result.(*Model) + + assert.False(t, updated.autoMergeEnabledPRs[1], + "unrelated PR should not have AutoMergeEnabled set in the map") + assert.Equal(t, "OPEN", updated.Prs[0].State, + "unrelated PR state should be unchanged") + require.Len(t, updated.Branches, 1) + assert.False(t, updated.Branches[0].AutoMergeEnabled, + "unrelated branch should not have AutoMergeEnabled set") +} diff --git a/internal/tui/components/tasks/pr.go b/internal/tui/components/tasks/pr.go index bb8bc127c..6a9b9f32b 100644 --- a/internal/tui/components/tasks/pr.go +++ b/internal/tui/components/tasks/pr.go @@ -26,6 +26,7 @@ type UpdatePRMsg struct { NewComment *data.Comment ReadyForReview *bool IsMerged *bool + AutoMergeEnabled *bool // Auto-merge was enabled (waiting for checks) AddedAssignees *data.Assignees RemovedAssignees *data.Assignees } @@ -165,14 +166,10 @@ func PRReady(ctx *context.ProgramContext, section SectionIdentifier, pr data.Row func MergePR(ctx *context.ProgramContext, section SectionIdentifier, pr data.RowData) tea.Cmd { prNumber := pr.GetNumber() - c := exec.Command( - "gh", - "pr", - "merge", - fmt.Sprint(prNumber), - "-R", - pr.GetRepoNameWithOwner(), - ) + + // Extract the underlying PullRequestData to obtain the node ID, merge state + // status, and repository merge-method settings required for the GraphQL mutation. + prData := extractPullRequestData(pr) taskId := fmt.Sprintf("merge_%d", prNumber) task := context.Task{ @@ -184,20 +181,82 @@ func MergePR(ctx *context.ProgramContext, section SectionIdentifier, pr data.Row } startCmd := ctx.StartTask(task) - return tea.Batch(startCmd, tea.ExecProcess(c, func(err error) tea.Msg { - isMerged := err == nil && c.ProcessState.ExitCode() == 0 + return tea.Batch(startCmd, func() tea.Msg { + if prData == nil { + return constants.TaskFinishedMsg{ + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: fmt.Errorf("could not resolve PR data for PR #%d", prNumber), + Msg: UpdatePRMsg{PrNumber: prNumber}, + } + } + + if prData.IsDraft { + return constants.TaskFinishedMsg{ + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: fmt.Errorf("cannot merge a draft PR, please publish it first"), + Msg: UpdatePRMsg{PrNumber: prNumber}, + } + } + + status, err := data.MergePullRequest(prData.ID, prData.MergeStateStatus, prData.Repository) + if err != nil { + log.Error("Failed to merge PR via GraphQL", "pr", prNumber, "err", err) + return constants.TaskFinishedMsg{ + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: err, + Msg: UpdatePRMsg{PrNumber: prNumber}, + } + } + + var finishedText string + updateMsg := UpdatePRMsg{PrNumber: prNumber} + + if status.State == "MERGED" { + isMerged := true + updateMsg.IsMerged = &isMerged + finishedText = fmt.Sprintf("PR #%d has been merged", prNumber) + } else if status.HasAutoMerge { + autoMergeEnabled := true + updateMsg.AutoMergeEnabled = &autoMergeEnabled + finishedText = fmt.Sprintf("Auto-merge enabled for PR #%d", prNumber) + } else { + finishedText = fmt.Sprintf("PR #%d merge action completed", prNumber) + } return constants.TaskFinishedMsg{ - SectionId: section.Id, - SectionType: section.Type, - TaskId: taskId, - Err: err, - Msg: UpdatePRMsg{ - PrNumber: prNumber, - IsMerged: &isMerged, - }, + SectionId: section.Id, + SectionType: section.Type, + TaskId: taskId, + Err: nil, + FinishedText: finishedText, + Msg: updateMsg, } - })) + }) +} + +// primaryPRDataProvider is implemented by row types that wrap a PullRequestData +// (e.g. prrow.Data in the PR list section). +type primaryPRDataProvider interface { + GetPrimaryPRData() *data.PullRequestData +} + +// extractPullRequestData retrieves the underlying *data.PullRequestData from a +// RowData value. RowData is implemented by both prrow.Data (PR list rows) and +// *data.PullRequestData (branch section rows), so we handle both cases. +func extractPullRequestData(pr data.RowData) *data.PullRequestData { + switch v := pr.(type) { + case *data.PullRequestData: + return v + case primaryPRDataProvider: + return v.GetPrimaryPRData() + } + return nil } func CreatePR( diff --git a/internal/tui/components/tasks/pr_test.go b/internal/tui/components/tasks/pr_test.go index 322262d6d..0dbf46600 100644 --- a/internal/tui/components/tasks/pr_test.go +++ b/internal/tui/components/tasks/pr_test.go @@ -2,11 +2,20 @@ package tasks import ( "fmt" + "io" + "net/http" + "strings" "testing" + "time" tea "charm.land/bubbletea/v2" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/dlvhdr/gh-dash/v4/internal/data" + graphqltest "github.com/dlvhdr/gh-dash/v4/internal/testhelpers/graphql" + "github.com/dlvhdr/gh-dash/v4/internal/tui/components/prrow" + "github.com/dlvhdr/gh-dash/v4/internal/tui/constants" "github.com/dlvhdr/gh-dash/v4/internal/tui/context" ) @@ -131,3 +140,205 @@ func TestApproveWorkflows_SectionIdentifierPropagation(t *testing.T) { }) } } + +// --- extractPullRequestData tests --- + +// unknownRowData is a RowData implementation that is neither *data.PullRequestData +// nor a primaryPRDataProvider — used to exercise the nil fallback branch. +type unknownRowData struct{} + +func (u unknownRowData) GetRepoNameWithOwner() string { return "owner/repo" } +func (u unknownRowData) GetTitle() string { return "title" } +func (u unknownRowData) GetNumber() int { return 1 } +func (u unknownRowData) GetUrl() string { return "https://github.com/owner/repo/pull/1" } +func (u unknownRowData) GetUpdatedAt() time.Time { return time.Time{} } + +func TestExtractPullRequestData_DirectPRData(t *testing.T) { + prData := &data.PullRequestData{ID: "PR_xyz"} + result := extractPullRequestData(prData) + assert.Same(t, prData, result, "should return the same *data.PullRequestData pointer") +} + +func TestExtractPullRequestData_PrrowData(t *testing.T) { + prData := &data.PullRequestData{ID: "PR_abc"} + row := &prrow.Data{Primary: prData} + result := extractPullRequestData(row) + assert.Same(t, prData, result, "should return the Primary field from prrow.Data") +} + +func TestExtractPullRequestData_PrrowData_NilPrimary(t *testing.T) { + row := &prrow.Data{Primary: nil} + result := extractPullRequestData(row) + assert.Nil(t, result) +} + +func TestExtractPullRequestData_NilInput(t *testing.T) { + // A bare nil becomes a nil interface value; the type switch has no + // matching case and falls through to return nil. + result := extractPullRequestData(nil) + assert.Nil(t, result) +} + +func TestExtractPullRequestData_UnknownRowDataType(t *testing.T) { + result := extractPullRequestData(unknownRowData{}) + assert.Nil(t, result, "unknown RowData type with no GetPrimaryPRData() method should return nil") +} + +// --- MergePR tests --- + +// setMockClient installs a mock GraphQL client backed by handler and resets it after the test. +func setMockClient(t *testing.T, handler http.Handler) { + t.Helper() + data.SetClient(graphqltest.NewMockGraphQLClient(t, handler)) + t.Cleanup(func() { data.SetClient(nil) }) +} + +// mergeMockHandler serves controlled responses keyed by GraphQL operation name. +func mergeMockHandler(t *testing.T, responses map[string]string) http.Handler { + t.Helper() + return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + require.NoError(t, err) + bodyStr := string(body) + + w.Header().Set("Content-Type", "application/json") + switch { + case strings.Contains(bodyStr, "mutation MergePullRequest"): + _, _ = io.WriteString(w, responses["MergePullRequest"]) + case strings.Contains(bodyStr, "mutation EnablePullRequestAutoMerge"): + _, _ = io.WriteString(w, responses["EnablePullRequestAutoMerge"]) + default: + t.Errorf("unexpected GraphQL request body: %s", bodyStr) + w.WriteHeader(http.StatusInternalServerError) + } + }) +} + +// noopCtx returns a minimal ProgramContext whose StartTask is a no-op. +// tea.Batch(nil, workFn) returns workFn directly, so calling the result of +// MergePR() will invoke the work function synchronously in tests. +func noopCtx() *context.ProgramContext { + return &context.ProgramContext{ + StartTask: func(task context.Task) tea.Cmd { return nil }, + } +} + +// prDataFor builds a *data.PullRequestData with the fields MergePR needs. +func prDataFor(id string, status data.MergeStateStatus, repo data.Repository) *data.PullRequestData { + return &data.PullRequestData{ + ID: id, + Number: 42, + MergeStateStatus: status, + Repository: repo, + } +} + +// runMergePR calls MergePR and immediately invokes the returned tea.Cmd to +// obtain the TaskFinishedMsg synchronously (no bubbletea runtime required). +func runMergePR(t *testing.T, pr data.RowData) constants.TaskFinishedMsg { + t.Helper() + ctx := noopCtx() + section := SectionIdentifier{Id: 1, Type: "prs"} + cmd := MergePR(ctx, section, pr) + require.NotNil(t, cmd, "MergePR should return a non-nil tea.Cmd") + msg := cmd() + finished, ok := msg.(constants.TaskFinishedMsg) + require.True(t, ok, "expected TaskFinishedMsg, got %T", msg) + return finished +} + +func TestMergePR_DirectMerge(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": `{"data":{"mergePullRequest":{"pullRequest":{"state":"MERGED"}}}}`, + })) + + pr := prDataFor("PR_clean", "CLEAN", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.NoError(t, msg.Err) + assert.Equal(t, "PR #42 has been merged", msg.FinishedText) + update, ok := msg.Msg.(UpdatePRMsg) + require.True(t, ok) + require.NotNil(t, update.IsMerged) + assert.True(t, *update.IsMerged) + assert.Nil(t, update.AutoMergeEnabled) +} + +func TestMergePR_Blocked_EnablesAutoMerge(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": `{"data":{"enablePullRequestAutoMerge":{"pullRequest":{"state":"OPEN","autoMergeRequest":{"enabledAt":"2024-01-01T00:00:00Z"}}}}}`, + })) + + pr := prDataFor("PR_blocked", "BLOCKED", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.NoError(t, msg.Err) + assert.Equal(t, "Auto-merge enabled for PR #42", msg.FinishedText) + update, ok := msg.Msg.(UpdatePRMsg) + require.True(t, ok) + require.NotNil(t, update.AutoMergeEnabled) + assert.True(t, *update.AutoMergeEnabled) + assert.Nil(t, update.IsMerged) +} + +func TestMergePR_AutoMergeEnabled(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "EnablePullRequestAutoMerge": `{"data":{"enablePullRequestAutoMerge":{"pullRequest":{"state":"OPEN","autoMergeRequest":{"enabledAt":"2024-01-01T00:00:00Z"}}}}}`, + })) + + pr := prDataFor("PR_behind", "BEHIND", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.NoError(t, msg.Err) + assert.Equal(t, "Auto-merge enabled for PR #42", msg.FinishedText) + update, ok := msg.Msg.(UpdatePRMsg) + require.True(t, ok) + require.NotNil(t, update.AutoMergeEnabled) + assert.True(t, *update.AutoMergeEnabled) + assert.Nil(t, update.IsMerged) +} + +func TestMergePR_NilPRData_ReturnsError(t *testing.T) { + // unknownRowData has no *data.PullRequestData and no GetPrimaryPRData(), + // so extractPullRequestData returns nil, triggering the nil-guard error path. + msg := runMergePR(t, unknownRowData{}) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "could not resolve PR data") +} + +func TestMergePR_DraftPR_ReturnsError(t *testing.T) { + // No mock client needed: the IsDraft guard fires before any network call. + pr := &data.PullRequestData{ + ID: "PR_draft", + Number: 42, + IsDraft: true, + MergeStateStatus: "CLEAN", + Repository: data.Repository{AllowMergeCommit: true}, + } + msg := runMergePR(t, pr) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "draft") +} + +func TestMergePR_GraphQLError_ReturnsError(t *testing.T) { + setMockClient(t, mergeMockHandler(t, map[string]string{ + "MergePullRequest": `{"errors":[{"message":"Pull request is not mergeable","locations":[],"path":[]}]}`, + })) + + pr := prDataFor("PR_clean2", "CLEAN", data.Repository{AllowMergeCommit: true}) + msg := runMergePR(t, pr) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "Pull request is not mergeable") +} + +func TestMergePR_RepoWithNoAllowedMethods_ReturnsError(t *testing.T) { + // Empty repository - no allowed merge methods + pr := prDataFor("PR_nomethods", "CLEAN", data.Repository{}) + msg := runMergePR(t, pr) + + require.Error(t, msg.Err) + assert.Contains(t, msg.Err.Error(), "no allowed merge methods") +} diff --git a/internal/tui/constants/constants.go b/internal/tui/constants/constants.go index c29c5de5e..a9c548a66 100644 --- a/internal/tui/constants/constants.go +++ b/internal/tui/constants/constants.go @@ -54,6 +54,7 @@ const ( LabelsIcon = "󰌖" MergedIcon = "" MergeQueueIcon = "" // \uf4db nf-oct-git_merge_queue + AutoMergeIcon = "󰦼" // \uF09BC nf-md-arrow_decision_auto OpenIcon = "" SelectionIcon = "❯" diff --git a/internal/tui/constants/progressMsg.go b/internal/tui/constants/progressMsg.go index 864c17891..2c284a4f6 100644 --- a/internal/tui/constants/progressMsg.go +++ b/internal/tui/constants/progressMsg.go @@ -8,6 +8,12 @@ type TaskFinishedMsg struct { SectionType string Err error Msg tea.Msg + // FinishedText overrides the FinishedText set on the Task at creation time + // when non-empty. Only applied on successful completion (Err == nil). + // Use this when the task's outcome determines the displayed message + // (e.g. "merged" vs "auto-merge enabled" vs "queued") and the final text + // cannot be known until the task completes. + FinishedText string } type ClearTaskMsg struct { diff --git a/internal/tui/markdown/markdownRenderer.go b/internal/tui/markdown/markdownRenderer.go index 307c153ba..020d35077 100644 --- a/internal/tui/markdown/markdownRenderer.go +++ b/internal/tui/markdown/markdownRenderer.go @@ -20,6 +20,12 @@ func InitializeMarkdownStyle(hasDarkBackground bool) { } func GetMarkdownRenderer(width int) glamour.TermRenderer { + if markdownStyle == nil { + // BackgroundColorMsg has not arrived yet (e.g. the sidebar is rendered + // before the terminal reports its background colour). Fall back to the + // dark style so rendering does not panic. + InitializeMarkdownStyle(true) + } markdownRenderer, err := glamour.NewTermRenderer( glamour.WithStyles(*markdownStyle), glamour.WithWordWrap(width), diff --git a/internal/tui/ui.go b/internal/tui/ui.go index a0a0f39bb..57cd55170 100644 --- a/internal/tui/ui.go +++ b/internal/tui/ui.go @@ -658,6 +658,10 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { task.Error = msg.Err } else { task.State = context.TaskFinished + // Use override finished text if provided + if msg.FinishedText != "" { + task.FinishedText = msg.FinishedText + } } now := time.Now() task.FinishedTime = &now