From 8dd83597df8c2b77877af3c464248ab527235eae Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 00:33:54 +0200 Subject: [PATCH 1/2] 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. --- internal/api/handler_purchases_test.go | 40 +++++++++++++++++++ .../000055_add_paused_status.down.sql | 10 +++++ .../000055_add_paused_status.up.sql | 11 +++++ 3 files changed, 61 insertions(+) create mode 100644 internal/database/postgres/migrations/000055_add_paused_status.down.sql create mode 100644 internal/database/postgres/migrations/000055_add_paused_status.up.sql diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index c0290ca06..fbe269564 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -976,6 +976,46 @@ func TestHandler_pausePlannedPurchase_NilExecution(t *testing.T) { assert.Nil(t, result) } +// TestHandler_pausePlannedPurchase_IneligibleStatus verifies that attempting to +// pause an execution whose current status is not in the allowed set (e.g. +// 'completed') surfaces a 409 with a clear message rather than leaking the raw +// Postgres CHECK constraint error (SQLSTATE 23514). This is the regression test +// for issue #772. +func TestHandler_pausePlannedPurchase_IneligibleStatus(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + t.Cleanup(func() { mockStore.AssertExpectations(t) }) + + adminSession := &Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + Email: "admin@example.com", + Role: "admin", + } + + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + // Store returns ErrExecutionNotInExpectedStatus when the row is 'completed' + // and cannot be transitioned to 'paused'. + mockStore.On("TransitionExecutionStatus", ctx, "11111111-1111-1111-1111-111111111111", []string{"pending", "running"}, "paused"). + Return(nil, fmt.Errorf("%w: execution 11111111-1111-1111-1111-111111111111 cannot transition from %q to %q", + config.ErrExecutionNotInExpectedStatus, "completed", "paused")) + + handler := &Handler{config: mockStore, auth: mockAuth} + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"Authorization": "Bearer admin-token"}, + } + result, err := handler.pausePlannedPurchase(ctx, req, "11111111-1111-1111-1111-111111111111") + require.Error(t, err, "pausing a completed execution must fail") + assert.Nil(t, result) + + // Must be a 409 client error, not a 500. + ce, ok := IsClientError(err) + require.True(t, ok, "expected ClientError, got %T: %v", err, err) + assert.Equal(t, 409, ce.code, "ineligible-status pause must return 409") + assert.Contains(t, ce.message, "cannot be paused", "error message must name the action") +} + func TestHandler_resumePlannedPurchase_NilExecution(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) diff --git a/internal/database/postgres/migrations/000055_add_paused_status.down.sql b/internal/database/postgres/migrations/000055_add_paused_status.down.sql new file mode 100644 index 000000000..31f318e19 --- /dev/null +++ b/internal/database/postgres/migrations/000055_add_paused_status.down.sql @@ -0,0 +1,10 @@ +-- Remove 'paused' from the valid_status check constraint. +-- Any rows currently in 'paused' state must be manually transitioned before +-- rolling back this migration. + +ALTER TABLE purchase_executions + DROP CONSTRAINT valid_status; + +ALTER TABLE purchase_executions + ADD CONSTRAINT valid_status + CHECK (status IN ('pending', 'running', 'notified', 'approved', 'cancelled', 'completed', 'failed')); diff --git a/internal/database/postgres/migrations/000055_add_paused_status.up.sql b/internal/database/postgres/migrations/000055_add_paused_status.up.sql new file mode 100644 index 000000000..e5118369f --- /dev/null +++ b/internal/database/postgres/migrations/000055_add_paused_status.up.sql @@ -0,0 +1,11 @@ +-- Add 'paused' to the valid_status check constraint on purchase_executions. +-- The pausePlannedPurchase handler sets status = 'paused' when a user clicks +-- Pause on a scheduled execution; the constraint omitted this value, causing +-- a CHECK violation (SQLSTATE 23514) at runtime. + +ALTER TABLE purchase_executions + DROP CONSTRAINT valid_status; + +ALTER TABLE purchase_executions + ADD CONSTRAINT valid_status + CHECK (status IN ('pending', 'running', 'notified', 'approved', 'cancelled', 'completed', 'failed', 'paused')); From d9bae6dbb7ed54c0d12b0fa0251f14f5e0f58ccd Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 14:54:10 +0200 Subject: [PATCH 2/2] 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. --- internal/api/handler_purchases_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index fbe269564..2fcd9efdc 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -985,7 +985,10 @@ func TestHandler_pausePlannedPurchase_IneligibleStatus(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) mockAuth := new(MockAuthService) - t.Cleanup(func() { mockStore.AssertExpectations(t) }) + t.Cleanup(func() { + mockStore.AssertExpectations(t) + mockAuth.AssertExpectations(t) + }) adminSession := &Session{ UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",