Skip to content

Commit dca027c

Browse files
authored
fix(purchases): add 'paused' to valid_status CHECK constraint so Pause succeeds (#779)
* fix(purchases): pause flow writes 'paused' status; migration adds it to valid_status The Pause button surfaced the raw Postgres CHECK constraint error to the user. Added 'paused' to the valid_status CHECK constraint via a new migration (000055), and added a regression test that verifies ineligible-status transitions return a clean 409. Closes #772. * test(api/purchases): also assert mockAuth expectations on ineligible-pause regression CR nitpick: `ValidateSession` was expected on `mockAuth` but never asserted in the t.Cleanup block, so a regression that bypasses auth could leave this test falsely green. Add `mockAuth.AssertExpectations(t)` alongside the existing `mockStore.AssertExpectations(t)` so both surfaces are verified. Matches the project-wide "always assert mock expectations" rule.
1 parent 5f3923d commit dca027c

3 files changed

Lines changed: 64 additions & 0 deletions

File tree

‎internal/api/handler_purchases_test.go‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,6 +1176,49 @@ func TestHandler_pausePlannedPurchase_NilExecution(t *testing.T) {
11761176
assert.Nil(t, result)
11771177
}
11781178

1179+
// TestHandler_pausePlannedPurchase_IneligibleStatus verifies that attempting to
1180+
// pause an execution whose current status is not in the allowed set (e.g.
1181+
// 'completed') surfaces a 409 with a clear message rather than leaking the raw
1182+
// Postgres CHECK constraint error (SQLSTATE 23514). This is the regression test
1183+
// for issue #772.
1184+
func TestHandler_pausePlannedPurchase_IneligibleStatus(t *testing.T) {
1185+
ctx := context.Background()
1186+
mockStore := new(MockConfigStore)
1187+
mockAuth := new(MockAuthService)
1188+
t.Cleanup(func() {
1189+
mockStore.AssertExpectations(t)
1190+
mockAuth.AssertExpectations(t)
1191+
})
1192+
1193+
adminSession := &Session{
1194+
UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
1195+
Email: "admin@example.com",
1196+
Role: "admin",
1197+
}
1198+
1199+
mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
1200+
// Store returns ErrExecutionNotInExpectedStatus when the row is 'completed'
1201+
// and cannot be transitioned to 'paused'.
1202+
mockStore.On("TransitionExecutionStatus", ctx, "11111111-1111-1111-1111-111111111111", []string{"pending", "running"}, "paused").
1203+
Return(nil, fmt.Errorf("%w: execution 11111111-1111-1111-1111-111111111111 cannot transition from %q to %q",
1204+
config.ErrExecutionNotInExpectedStatus, "completed", "paused"))
1205+
1206+
handler := &Handler{config: mockStore, auth: mockAuth}
1207+
1208+
req := &events.LambdaFunctionURLRequest{
1209+
Headers: map[string]string{"Authorization": "Bearer admin-token"},
1210+
}
1211+
result, err := handler.pausePlannedPurchase(ctx, req, "11111111-1111-1111-1111-111111111111")
1212+
require.Error(t, err, "pausing a completed execution must fail")
1213+
assert.Nil(t, result)
1214+
1215+
// Must be a 409 client error, not a 500.
1216+
ce, ok := IsClientError(err)
1217+
require.True(t, ok, "expected ClientError, got %T: %v", err, err)
1218+
assert.Equal(t, 409, ce.code, "ineligible-status pause must return 409")
1219+
assert.Contains(t, ce.message, "cannot be paused", "error message must name the action")
1220+
}
1221+
11791222
func TestHandler_resumePlannedPurchase_NilExecution(t *testing.T) {
11801223
ctx := context.Background()
11811224
mockStore := new(MockConfigStore)
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
-- Remove 'paused' from the valid_status check constraint.
2+
-- Any rows currently in 'paused' state must be manually transitioned before
3+
-- rolling back this migration.
4+
5+
ALTER TABLE purchase_executions
6+
DROP CONSTRAINT valid_status;
7+
8+
ALTER TABLE purchase_executions
9+
ADD CONSTRAINT valid_status
10+
CHECK (status IN ('pending', 'running', 'notified', 'approved', 'cancelled', 'completed', 'failed'));
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-- Add 'paused' to the valid_status check constraint on purchase_executions.
2+
-- The pausePlannedPurchase handler sets status = 'paused' when a user clicks
3+
-- Pause on a scheduled execution; the constraint omitted this value, causing
4+
-- a CHECK violation (SQLSTATE 23514) at runtime.
5+
6+
ALTER TABLE purchase_executions
7+
DROP CONSTRAINT valid_status;
8+
9+
ALTER TABLE purchase_executions
10+
ADD CONSTRAINT valid_status
11+
CHECK (status IN ('pending', 'running', 'notified', 'approved', 'cancelled', 'completed', 'failed', 'paused'));

0 commit comments

Comments
 (0)