Skip to content

Commit 9351ca8

Browse files
committed
update for new status field from api
1 parent 4d83e3c commit 9351ca8

8 files changed

Lines changed: 125 additions & 75 deletions

File tree

cmd/merge.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ func runMergeInteractive(cfg *config.Config, client github.ClientOps, stackNumbe
325325
return nil
326326
case out.Failed:
327327
cfg.Errorf("merge failed: %s", out.Message)
328-
cfg.Printf("The stack is atomic, so nothing was merged.")
328+
cfg.Printf("Stack merges are atomic, so nothing was merged.")
329329
return mergeFailureExit(out.Message)
330330
case out.WatchStopped:
331331
cfg.Infof("Stopped watching. Merge is still in progress. Check the pull requests on GitHub.")
@@ -353,10 +353,15 @@ func runMergeHeadless(cfg *config.Config, client github.ClientOps, base string,
353353
return ErrAPIFailure
354354
}
355355

356-
if res.Merged {
356+
if res.IsMerged() {
357357
mergedSuccess(cfg, list, base, res.Details.SHA)
358358
return nil
359359
}
360+
if res.IsFailed() {
361+
cfg.Errorf("merge failed: %s", res.Details.Message)
362+
cfg.Printf("Stack merges are atomic, so nothing was merged.")
363+
return mergeFailureExit(res.Details.Message)
364+
}
360365

361366
uuid := res.Details.UUID
362367
if uuid == "" {
@@ -380,13 +385,13 @@ func runMergeHeadless(cfg *config.Config, client github.ClientOps, base string,
380385
cfg.Errorf("failed to check merge status: %s", err)
381386
return ErrAPIFailure
382387
}
383-
if status.Merged {
388+
if status.IsMerged() {
384389
mergedSuccess(cfg, list, base, status.Details.SHA)
385390
return nil
386391
}
387-
if !status.Queued {
392+
if status.IsFailed() {
388393
cfg.Errorf("merge failed: %s", status.Details.Message)
389-
cfg.Printf("The stack is atomic, so nothing was merged.")
394+
cfg.Printf("Stack merges are atomic, so nothing was merged.")
390395
return mergeFailureExit(status.Details.Message)
391396
}
392397
}
@@ -416,9 +421,15 @@ func mergeFuncs(client github.ClientOps) (mergeview.SubmitFunc, mergeview.PollFu
416421
}
417422

418423
func toMergeStatus(res *github.AsyncMergeResult) mergeview.MergeStatus {
424+
status := mergeview.StatusPending
425+
switch {
426+
case res.IsMerged():
427+
status = mergeview.StatusMerged
428+
case res.IsFailed():
429+
status = mergeview.StatusFailed
430+
}
419431
return mergeview.MergeStatus{
420-
Queued: res.Queued,
421-
Merged: res.Merged,
432+
Status: status,
422433
Message: res.Details.Message,
423434
UUID: res.Details.UUID,
424435
SHA: res.Details.SHA,

cmd/merge_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,10 @@ func TestRunMerge_NoArg_MergesWholeStack(t *testing.T) {
9393
},
9494
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
9595
gotPR, gotMethod = pr, method
96-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
96+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
9797
},
9898
GetAsyncMergeResultFn: func(pr int, uuid string) (*github.AsyncMergeResult, error) {
99-
return &github.AsyncMergeResult{Merged: true, Details: github.AsyncMergeDetails{SHA: "abc1234"}}, nil
99+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusMerged, Details: github.AsyncMergeDetails{SHA: "abc1234"}}, nil
100100
},
101101
}
102102

@@ -119,7 +119,7 @@ func TestRunMerge_StackNumberArg(t *testing.T) {
119119
},
120120
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
121121
gotPR = pr
122-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
122+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
123123
},
124124
}
125125

@@ -144,7 +144,7 @@ func TestRunMerge_PRNumberArg(t *testing.T) {
144144
},
145145
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
146146
gotPR = pr
147-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
147+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
148148
},
149149
}
150150

@@ -166,7 +166,7 @@ func TestRunMerge_SquashFlag(t *testing.T) {
166166
},
167167
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
168168
gotMethod = method
169-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
169+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
170170
},
171171
}
172172

@@ -314,10 +314,10 @@ func TestRunMerge_PollFailedConflict(t *testing.T) {
314314
return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
315315
},
316316
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
317-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
317+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
318318
},
319319
GetAsyncMergeResultFn: func(pr int, uuid string) (*github.AsyncMergeResult, error) {
320-
return &github.AsyncMergeResult{Queued: false, Merged: false, Details: github.AsyncMergeDetails{Message: "Merge conflict: could not merge."}}, nil
320+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusFailed, Details: github.AsyncMergeDetails{Message: "Merge conflict: could not merge."}}, nil
321321
},
322322
}
323323

@@ -336,7 +336,7 @@ func TestRunMerge_AlreadyMergedOnSubmit(t *testing.T) {
336336
return remoteStack(7, "main", openStackPR(1, "b1"), openStackPR(2, "b2")), nil
337337
},
338338
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
339-
return &github.AsyncMergeResult{Merged: true, Details: github.AsyncMergeDetails{SHA: "abc"}}, nil
339+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusMerged, Details: github.AsyncMergeDetails{SHA: "abc"}}, nil
340340
},
341341
}
342342

@@ -428,7 +428,7 @@ func TestRunMerge_DefaultMethodFallsBackToAllowed(t *testing.T) {
428428
},
429429
MergeStackAsyncFn: func(pr int, method string) (*github.AsyncMergeResult, error) {
430430
gotMethod = method
431-
return &github.AsyncMergeResult{Queued: true, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
431+
return &github.AsyncMergeResult{Status: github.AsyncMergeStatusPending, Details: github.AsyncMergeDetails{UUID: "u"}}, nil
432432
},
433433
}
434434

internal/github/merge_async.go

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ func (c RepoMergeConfig) Allows(method string) bool {
6565
}
6666

6767
// AsyncMergeDetails is the polymorphic "details" object shared by the submit and
68-
// poll responses. Fields are populated based on the current state: a queued
69-
// request carries UUID/MergeMethod/ExpectedHeadSHA, an already-merged result
70-
// carries SHA, and a failed/not-mergeable result carries only Message.
68+
// poll responses. Fields are populated based on the current state: a pending
69+
// request carries UUID/MergeMethod/ExpectedHeadSHA, a merged result carries SHA,
70+
// and a failed/not-mergeable result carries only Message.
7171
type AsyncMergeDetails struct {
7272
Message string `json:"message"`
7373
UUID string `json:"uuid"`
@@ -77,18 +77,33 @@ type AsyncMergeDetails struct {
7777
}
7878

7979
// AsyncMergeResult is the response body returned by both the submit and poll
80-
// async merge endpoints. The Queued/Merged flags distinguish an enqueued merge
81-
// (202) from an already-merged pull request (200).
80+
// async merge endpoints. Status is one of the AsyncMergeStatus* values:
81+
// "pending" (running in the background), "merged" (completed), or "failed".
8282
type AsyncMergeResult struct {
83-
Queued bool `json:"queued"`
84-
Merged bool `json:"merged"`
83+
Status string `json:"status"`
8584
Details AsyncMergeDetails `json:"details"`
8685
}
8786

88-
// InProgress reports whether the merge is still queued (running in the
89-
// background).
90-
func (r *AsyncMergeResult) InProgress() bool {
91-
return r != nil && r.Queued && !r.Merged
87+
// Async merge status values returned in the response's "status" field.
88+
const (
89+
AsyncMergeStatusPending = "pending"
90+
AsyncMergeStatusMerged = "merged"
91+
AsyncMergeStatusFailed = "failed"
92+
)
93+
94+
// IsMerged reports whether the merge completed successfully.
95+
func (r *AsyncMergeResult) IsMerged() bool {
96+
return r != nil && r.Status == AsyncMergeStatusMerged
97+
}
98+
99+
// IsFailed reports whether the merge was attempted but did not complete.
100+
func (r *AsyncMergeResult) IsFailed() bool {
101+
return r != nil && r.Status == AsyncMergeStatusFailed
102+
}
103+
104+
// IsPending reports whether the merge is still running in the background.
105+
func (r *AsyncMergeResult) IsPending() bool {
106+
return r != nil && r.Status == AsyncMergeStatusPending
92107
}
93108

94109
// RepoMergeConfig fetches the repository's allowed merge methods and the

internal/github/merge_async_test.go

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ func testAsyncClient(t *testing.T, status int, respBody string, rec *recordedReq
5050

5151
func TestMergeStackAsync_Accepted(t *testing.T) {
5252
var rec recordedRequest
53-
body := `{"queued":true,"merged":false,"details":{"message":"Merge request enqueued.","uuid":"u-1","merge_method":"squash","expected_head_sha":"abc"}}`
53+
body := `{"status":"pending","details":{"message":"Merge request enqueued.","uuid":"u-1","merge_method":"squash","expected_head_sha":"abc"}}`
5454
c := testAsyncClient(t, http.StatusAccepted, body, &rec)
5555

5656
res, err := c.MergeStackAsync(42, "squash")
@@ -60,33 +60,32 @@ func TestMergeStackAsync_Accepted(t *testing.T) {
6060
assert.Equal(t, "/repos/o/r/pulls/42/merge-async", rec.path)
6161
assert.JSONEq(t, `{"merge_method":"squash"}`, rec.body)
6262

63-
assert.True(t, res.Queued)
64-
assert.False(t, res.Merged)
63+
assert.True(t, res.IsPending())
64+
assert.False(t, res.IsMerged())
6565
assert.Equal(t, "u-1", res.Details.UUID)
6666
assert.Equal(t, "squash", res.Details.MergeMethod)
67-
assert.True(t, res.InProgress())
6867
}
6968

7069
func TestMergeStackAsync_AlreadyMerged(t *testing.T) {
71-
body := `{"queued":false,"merged":true,"details":{"message":"Pull request is already merged.","sha":"deadbeef"}}`
70+
body := `{"status":"merged","details":{"message":"Pull request is already merged.","sha":"deadbeef"}}`
7271
res, err := testAsyncClient(t, http.StatusOK, body, nil).MergeStackAsync(42, "merge")
7372
require.NoError(t, err)
74-
assert.True(t, res.Merged)
73+
assert.True(t, res.IsMerged())
7574
assert.Equal(t, "deadbeef", res.Details.SHA)
7675
}
7776

7877
func TestMergeStackAsync_ExistingRequestConflict(t *testing.T) {
7978
// The go-gh REST client discards the 409 body, so we can't recover the
8079
// existing UUID; the request surfaces as a clear "already exists" error.
81-
_, err := testAsyncClient(t, http.StatusConflict, `{"queued":true,"merged":false,"details":{"uuid":"u-2"}}`, nil).MergeStackAsync(42, "merge")
80+
_, err := testAsyncClient(t, http.StatusConflict, `{"status":"pending","details":{"uuid":"u-2"}}`, nil).MergeStackAsync(42, "merge")
8281
require.Error(t, err)
8382
assert.Contains(t, err.Error(), "already exists")
8483
}
8584

8685
func TestMergeStackAsync_NotMergeable(t *testing.T) {
8786
// A 400 preflight failure is reported as a clear error (the specific
8887
// details.message isn't recoverable through the REST client).
89-
_, err := testAsyncClient(t, http.StatusBadRequest, `{"queued":false,"merged":false,"details":{"message":"Pull request is closed."}}`, nil).MergeStackAsync(42, "merge")
88+
_, err := testAsyncClient(t, http.StatusBadRequest, `{"status":"failed","details":{"message":"Pull request is closed."}}`, nil).MergeStackAsync(42, "merge")
9089
require.Error(t, err)
9190
assert.Contains(t, err.Error(), "can no longer be merged")
9291
}
@@ -106,12 +105,11 @@ func TestGetAsyncMergeResult_States(t *testing.T) {
106105
tests := []struct {
107106
name string
108107
body string
109-
wantQueued bool
110-
wantMerged bool
108+
wantStatus string
111109
}{
112-
{"pending", `{"queued":true,"merged":false,"details":{"message":"Merge request is in progress.","uuid":"u","merge_method":"merge","expected_head_sha":"abc"}}`, true, false},
113-
{"merged", `{"queued":false,"merged":true,"details":{"message":"Pull request was merged.","sha":"abc"}}`, false, true},
114-
{"failed", `{"queued":false,"merged":false,"details":{"message":"Merge conflict."}}`, false, false},
110+
{"pending", `{"status":"pending","details":{"message":"Merge request is in progress.","uuid":"u","merge_method":"merge","expected_head_sha":"abc"}}`, AsyncMergeStatusPending},
111+
{"merged", `{"status":"merged","details":{"message":"Pull request was merged.","sha":"abc"}}`, AsyncMergeStatusMerged},
112+
{"failed", `{"status":"failed","details":{"message":"Merge conflict."}}`, AsyncMergeStatusFailed},
115113
}
116114
for _, tt := range tests {
117115
t.Run(tt.name, func(t *testing.T) {
@@ -120,8 +118,7 @@ func TestGetAsyncMergeResult_States(t *testing.T) {
120118
require.NoError(t, err)
121119
assert.Equal(t, http.MethodGet, rec.method)
122120
assert.Equal(t, "/repos/o/r/pulls/42/merge-async/u", rec.path)
123-
assert.Equal(t, tt.wantQueued, res.Queued)
124-
assert.Equal(t, tt.wantMerged, res.Merged)
121+
assert.Equal(t, tt.wantStatus, res.Status)
125122
})
126123
}
127124
}
@@ -150,12 +147,24 @@ func TestRepoMergeConfig_AllowedMethods(t *testing.T) {
150147
assert.Empty(t, empty.AllowedMethods())
151148
}
152149

153-
func TestAsyncMergeResult_InProgress(t *testing.T) {
154-
assert.True(t, (&AsyncMergeResult{Queued: true}).InProgress())
155-
assert.False(t, (&AsyncMergeResult{Queued: true, Merged: true}).InProgress())
156-
assert.False(t, (&AsyncMergeResult{}).InProgress())
150+
func TestAsyncMergeResult_Status(t *testing.T) {
151+
pending := &AsyncMergeResult{Status: AsyncMergeStatusPending}
152+
assert.True(t, pending.IsPending())
153+
assert.False(t, pending.IsMerged())
154+
assert.False(t, pending.IsFailed())
155+
156+
merged := &AsyncMergeResult{Status: AsyncMergeStatusMerged}
157+
assert.True(t, merged.IsMerged())
158+
assert.False(t, merged.IsPending())
159+
160+
failed := &AsyncMergeResult{Status: AsyncMergeStatusFailed}
161+
assert.True(t, failed.IsFailed())
162+
assert.False(t, failed.IsMerged())
163+
157164
var nilRes *AsyncMergeResult
158-
assert.False(t, nilRes.InProgress())
165+
assert.False(t, nilRes.IsPending())
166+
assert.False(t, nilRes.IsMerged())
167+
assert.False(t, nilRes.IsFailed())
159168
}
160169

161170
// sanity check that the submit body omits merge_method when empty.

internal/github/mock_client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func (m *MockClient) MergeStackAsync(prNumber int, method string) (*AsyncMergeRe
135135
return m.MergeStackAsyncFn(prNumber, method)
136136
}
137137
return &AsyncMergeResult{
138-
Queued: true,
138+
Status: AsyncMergeStatusPending,
139139
Details: AsyncMergeDetails{
140140
Message: "Merge request enqueued.",
141141
UUID: "mock-uuid",
@@ -149,7 +149,7 @@ func (m *MockClient) GetAsyncMergeResult(prNumber int, uuid string) (*AsyncMerge
149149
return m.GetAsyncMergeResultFn(prNumber, uuid)
150150
}
151151
return &AsyncMergeResult{
152-
Merged: true,
152+
Status: AsyncMergeStatusMerged,
153153
Details: AsyncMergeDetails{
154154
Message: "Pull request was merged.",
155155
SHA: "mockmergesha",

internal/tui/mergeview/model.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -262,15 +262,19 @@ func (m Model) handleSubmitDone(msg submitDoneMsg) (tea.Model, tea.Cmd) {
262262
m.status = msg.status
263263
m.message = msg.status.Message
264264

265-
switch {
266-
case msg.status.Merged:
265+
switch msg.status.Status {
266+
case StatusMerged:
267267
m.merged = true
268268
return m.finish()
269-
case msg.status.Queued && msg.status.UUID != "":
270-
// Enqueued (or an existing request adopted): start polling.
271-
return m, m.pollTickCmd()
269+
case StatusFailed:
270+
m.failed = true
271+
return m.finish()
272272
default:
273-
// Not queued and not merged: the PR could not be merged (e.g. 400).
273+
// Pending (enqueued, or an existing request adopted): poll if we have a
274+
// UUID; otherwise this is unexpected, so treat it as a failure.
275+
if msg.status.UUID != "" {
276+
return m, m.pollTickCmd()
277+
}
274278
m.failed = true
275279
return m.finish()
276280
}
@@ -287,15 +291,16 @@ func (m Model) handlePollDone(msg pollDoneMsg) (tea.Model, tea.Cmd) {
287291
m.status = msg.status
288292
m.message = msg.status.Message
289293

290-
switch {
291-
case msg.status.Merged:
294+
switch msg.status.Status {
295+
case StatusMerged:
292296
m.merged = true
293297
return m.finish()
294-
case msg.status.Queued:
295-
return m, m.pollTickCmd()
296-
default:
298+
case StatusFailed:
297299
m.failed = true
298300
return m.finish()
301+
default:
302+
// Still pending: keep polling.
303+
return m, m.pollTickCmd()
299304
}
300305
}
301306

0 commit comments

Comments
 (0)