From ecb0116d4cbdcd467c933c06ce1a7eb453e89924 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 8 Oct 2026 20:28:12 +0200 Subject: [PATCH 1/2] fix(history): fetch actionable executions separately so terminal rows cannot starve them - The History view loaded one list of the 100 newest executions across all twelve statuses, so 100+ newer failed/expired/canceled rows pushed an older pending approval out before any filter ran (#632). - Fetch the actionable statuses (pending, notified, scheduled, approved, running, paused) at MaxListLimit and the terminal statuses at DefaultListLimit; truncated now fires per class. - Bound the stale-expiry sweep to 100 rows per request: the actionable fetch can now return far more stale rows, and Lambda sweeps synchronously. - Make MockConfigStore.GetExecutionsByStatuses honour statuses, limit, ordering and the clean-completed exclusion like the real store, so two calls do not return the same canned rows twice. - Not fixed here: far-future ramp steps sort first inside the actionable class (#680); provider/account/date and scope pushdown is the next PR. Refs #632 --- internal/api/handler_history.go | 68 +++++--- ...dler_history_actionable_starvation_test.go | 157 ++++++++++++++++++ .../api/handler_history_truncation_test.go | 20 ++- internal/api/openapi.yaml | 7 +- internal/api/types.go | 8 +- internal/mocks/stores.go | 28 +++- 6 files changed, 255 insertions(+), 33 deletions(-) create mode 100644 internal/api/handler_history_actionable_starvation_test.go diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index e1d8f999d..7922fb33b 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -4,6 +4,7 @@ package api import ( "context" "fmt" + "slices" "sort" "strconv" "time" @@ -91,9 +92,11 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR }, nil } -// historyExecutionStatuses enumerates the PurchaseExecution statuses the -// History view loads alongside completed purchases. Issue #372 assumed -// approval executes synchronously (approved -> completed/failed inside one +// actionableExecutionStatuses are the PurchaseExecution statuses a user can still +// act on or that are in flight. They are fetched separately from the terminal +// class so newer terminal rows cannot push them out of the capped listing. +// +// Issue #372 assumed approval executes synchronously (approved -> completed/failed inside one // HTTP request) and so excluded "approved"; issue #621 showed that // assumption breaks under interruption: a Lambda timeout or a crash mid- // execution leaves the row stuck in "approved"/"running"/"paused", in @@ -103,6 +106,15 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR // states too, rendered with a clear "in progress" badge rather than as a // (misleading) completed row. // +// "scheduled" is included so Gmail-style pre-fire delayed executions (issue #291 +// wave-2) appear in the History view with a Revoke button before the cloud SDK +// call fires. Without this entry the row is invisible to the History UI, making +// the Revoke button unreachable (issue #290, second-wave CR Finding E). +var actionableExecutionStatuses = []string{"pending", "notified", "scheduled", "approved", "running", "paused"} + +// terminalExecutionStatuses are the finished PurchaseExecution statuses the +// History view loads alongside completed purchases. +// // "completed" is also loaded, but fetchExecutionsAsHistory synthesizes a // row for it ONLY when the execution carries a non-empty Error — the // audit-gap case where the purchase succeeded but its purchase_history @@ -118,16 +130,12 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR // partial-failure marker and is flagged IsAuditGap so its execution-level // dollars are excluded from the dashboard totals — the committed dollars are // already counted via the per-rec purchase_history rows that succeeded. -// "scheduled" is included so Gmail-style pre-fire delayed executions (issue #291 -// wave-2) appear in the History view with a Revoke button before the cloud SDK -// call fires. Without this entry the row is invisible to the History UI, making -// the Revoke button unreachable (issue #290, second-wave CR Finding E). // Both the US-spelling status (config.StatusCanceled) and the legacy British // spelling (config.LegacyStatusCanceled) are listed: during the expand-contract // rename (migration 000089) old code may still write the legacy value before // the rolling deploy completes. The contract migration (#1278) normalizes the // data once the deploy is verified stable; drop the legacy entry here then. -var historyExecutionStatuses = []string{"pending", "notified", "scheduled", "approved", "running", "paused", "completed", "partially_completed", "failed", "expired", config.StatusCanceled, config.LegacyStatusCanceled} +var terminalExecutionStatuses = []string{"completed", "partially_completed", "failed", "expired", config.StatusCanceled, config.LegacyStatusCanceled} // approvalExpiryWindow is how long a pending approval stays actionable // before the History view flips it to "expired". Aligns with the @@ -152,14 +160,13 @@ const approvalExpiryWindow = 7 * 24 * time.Hour // provider via the recs' collapsed provider, account via CloudAccountID, // date via ScheduledDate. // -// The third return value reports that the fetch reached its cap, so older -// executions may have been dropped before the filters ran. +// The third return value reports that either class reached its cap (actionable +// at MaxListLimit, terminal at DefaultListLimit), so older executions may have +// been dropped before the filters ran. func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution, bool) { - executions, err := h.config.GetExecutionsByStatuses(ctx, historyExecutionStatuses, config.DefaultListLimit) - if err != nil { - logging.Warnf("history: failed to load non-completed executions: %v", err) - return nil, nil, false - } + actionable := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, config.MaxListLimit) + terminal := h.fetchExecutionsByClass(ctx, terminalExecutionStatuses, config.DefaultListLimit) + executions := append(actionable[:len(actionable):len(actionable)], terminal...) if len(executions) == 0 { return nil, nil, false } @@ -167,9 +174,8 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF userEmailCache := h.resolveUserEmails(ctx, executions) out := make([]config.PurchaseHistoryRecord, 0, len(executions)) var staleExecs []config.PurchaseExecution - // Rows the History view can surface; clean completed executions do not - // count toward the cap signal (the store already excludes them). - surfaced := 0 + // Terminal-class rows surfaced; drives the terminal cap signal. + terminalSurfaced := 0 for _rvc := range executions { exec := executions[_rvc] // Dedup: a normal completed execution is already represented by its @@ -180,7 +186,9 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF if exec.Status == "completed" && exec.Error == "" { continue } - surfaced++ + if isTerminalExecutionStatus(exec.Status) { + terminalSurfaced++ + } // Collect stale pending/notified executions for the post-assembly // expire sweep. We do NOT mutate status here to keep the GET // read-only: the response reflects current DB state; the sweep @@ -197,7 +205,23 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF } out = append(out, executionToHistoryRow(exec, approver, createdByEmail)) } - return out, staleExecs, surfaced >= config.DefaultListLimit + capped := len(actionable) >= config.MaxListLimit || terminalSurfaced >= config.DefaultListLimit + return out, staleExecs, capped +} + +// fetchExecutionsByClass lists one status class. A listing error is logged and +// the class skipped so the other class and completed history still render. +func (h *Handler) fetchExecutionsByClass(ctx context.Context, statuses []string, limit int) []config.PurchaseExecution { + rows, err := h.config.GetExecutionsByStatuses(ctx, statuses, limit) + if err != nil { + logging.Warnf("history: failed to load executions with statuses %v: %v", statuses, err) + return nil + } + return rows +} + +func isTerminalExecutionStatus(status string) bool { + return slices.Contains(terminalExecutionStatuses, status) } // isStaleExecution reports whether the execution is a pending/notified @@ -235,6 +259,10 @@ func (h *Handler) expireStaleExecutions(staleExecs []config.PurchaseExecution) { if len(staleExecs) == 0 { return } + if len(staleExecs) > config.DefaultListLimit { + logging.Warnf("history: sweeping %d of %d stale executions", config.DefaultListLimit, len(staleExecs)) + staleExecs = staleExecs[:config.DefaultListLimit] + } if runtime.IsLambda() { h.expireStaleExecutionsSweep(staleExecs) return diff --git a/internal/api/handler_history_actionable_starvation_test.go b/internal/api/handler_history_actionable_starvation_test.go new file mode 100644 index 000000000..05e53a109 --- /dev/null +++ b/internal/api/handler_history_actionable_starvation_test.go @@ -0,0 +1,157 @@ +package api + +import ( + "context" + "errors" + "fmt" + "slices" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func executionsWithStatus(n int, status string, scheduled time.Time, idPrefix string) []config.PurchaseExecution { + execs := make([]config.PurchaseExecution, n) + for i := range execs { + execs[i] = config.PurchaseExecution{ + ExecutionID: fmt.Sprintf("%s-%d", idPrefix, i), + Status: status, + ScheduledDate: scheduled, + } + } + return execs +} + +func isActionableCall(statuses []string) bool { + return slices.Equal(statuses, actionableExecutionStatuses) +} + +func isTerminalCall(statuses []string) bool { + return slices.Equal(statuses, terminalExecutionStatuses) +} + +func newStarvationStore(ctx context.Context) *MockConfigStore { + store := new(MockConfigStore) + store.On("GetAllPurchaseHistory", ctx, mock.Anything).Return([]config.PurchaseHistoryRecord{}, nil) + store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil).Maybe() + return store +} + +func runStarvationHistory(t *testing.T, ctx context.Context, store *MockConfigStore) map[string]any { + t.Helper() + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: store} + result, err := handler.getHistory(ctx, req, map[string]string{}) + require.NoError(t, err) + return historyJSON(t, result) +} + +func TestHandler_getHistory_PendingNotStarvedByNewerTerminalRows(t *testing.T) { + ctx := context.Background() + now := time.Now() + execs := executionsWithStatus(150, "failed", now, "failed") + pending := executionsWithStatus(1, "pending", now.Add(-time.Hour), "pending") + store := newStarvationStore(ctx) + store.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(append(execs, pending...), nil) + + body := runStarvationHistory(t, ctx, store) + + ids := map[string]bool{} + for _, p := range body["purchases"].([]any) { + ids[p.(map[string]any)["purchase_id"].(string)] = true + } + assert.True(t, ids["pending-0"], "older pending row must survive 150 newer failed rows") + summary := body["summary"].(map[string]any) + assert.EqualValues(t, 1, summary["total_pending"]) +} + +func TestHandler_getHistory_FetchesDisjointClassesWithTheirLimits(t *testing.T) { + ctx := context.Background() + store := newStarvationStore(ctx) + store.On("GetExecutionsByStatuses", ctx, mock.MatchedBy(isActionableCall), config.MaxListLimit).Return([]config.PurchaseExecution{}, nil).Once() + store.On("GetExecutionsByStatuses", ctx, mock.MatchedBy(isTerminalCall), config.DefaultListLimit).Return([]config.PurchaseExecution{}, nil).Once() + + runStarvationHistory(t, ctx, store) + + store.AssertNumberOfCalls(t, "GetExecutionsByStatuses", 2) + for _, s := range actionableExecutionStatuses { + assert.NotContains(t, terminalExecutionStatuses, s, "classes must be disjoint") + } +} + +func TestHandler_getHistory_ClassListingErrorKeepsOtherClass(t *testing.T) { + now := time.Now() + tests := []struct { + name string + failActive bool + wantID string + }{ + {"terminal error keeps actionable", false, "pending-0"}, + {"actionable error keeps terminal", true, "failed-0"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + store := newStarvationStore(ctx) + actionableRet := mock.Arguments{executionsWithStatus(1, "pending", now, "pending"), nil} + terminalRet := mock.Arguments{executionsWithStatus(1, "failed", now, "failed"), nil} + if tt.failActive { + actionableRet = mock.Arguments{nil, errors.New("actionable listing failed")} + } else { + terminalRet = mock.Arguments{nil, errors.New("terminal listing failed")} + } + store.On("GetExecutionsByStatuses", ctx, mock.MatchedBy(isActionableCall), mock.Anything).Return(actionableRet...) + store.On("GetExecutionsByStatuses", ctx, mock.MatchedBy(isTerminalCall), mock.Anything).Return(terminalRet...) + + body := runStarvationHistory(t, ctx, store) + + purchases := body["purchases"].([]any) + require.Len(t, purchases, 1) + assert.Equal(t, tt.wantID, purchases[0].(map[string]any)["purchase_id"]) + }) + } +} + +func TestHandler_getHistory_TruncatedPerClass(t *testing.T) { + now := time.Now() + tests := []struct { + name string + actionable int + terminal int + wantTruncated bool + }{ + {"terminal class at cap", 0, config.DefaultListLimit, true}, + {"both under cap", config.DefaultListLimit, config.DefaultListLimit - 1, false}, + {"actionable class at cap", config.MaxListLimit, 0, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + rows := append(executionsWithStatus(tt.actionable, "pending", now, "pending"), + executionsWithStatus(tt.terminal, "failed", now, "failed")...) + store := newStarvationStore(ctx) + store.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(rows, nil) + + body := runStarvationHistory(t, ctx, store) + + assert.Equal(t, tt.wantTruncated, body["truncated"]) + }) + } +} + +func TestHandler_getHistory_StaleSweepBoundedToDefaultListLimit(t *testing.T) { + t.Setenv("AWS_LAMBDA_RUNTIME_API", "127.0.0.1:9001") + ctx := context.Background() + stale := executionsWithStatus(150, "pending", time.Now().Add(-8*24*time.Hour), "stale") + store := newStarvationStore(ctx) + store.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(stale, nil) + store.On("TransitionExecutionStatus", mock.Anything, mock.Anything, mock.Anything, "expired", mock.Anything).Return(nil, nil) + + runStarvationHistory(t, ctx, store) + + store.AssertNumberOfCalls(t, "TransitionExecutionStatus", config.DefaultListLimit) +} diff --git a/internal/api/handler_history_truncation_test.go b/internal/api/handler_history_truncation_test.go index 4fe6dfc6c..30bf2c5c5 100644 --- a/internal/api/handler_history_truncation_test.go +++ b/internal/api/handler_history_truncation_test.go @@ -69,10 +69,18 @@ func TestHandler_getHistory_TruncatedWhenCompletedAtCap(t *testing.T) { func TestHandler_getHistory_TruncatedWhenExecutionsAtCap(t *testing.T) { // The execution fetch is capped before the in-memory filters run, so a // full page means pending approvals may have been dropped. - body := runAdminHistory(t, nil, pendingExecutions(config.DefaultListLimit), map[string]string{}) + body := runAdminHistory(t, nil, failedExecutions(config.DefaultListLimit), map[string]string{}) assert.Equal(t, true, body["truncated"]) } +func failedExecutions(n int) []config.PurchaseExecution { + execs := pendingExecutions(n) + for i := range execs { + execs[i].Status = "failed" + } + return execs +} + func cleanCompletedExecutions(n int) []config.PurchaseExecution { execs := pendingExecutions(n) for i := range execs { @@ -90,11 +98,7 @@ func TestHandler_getHistory_NotTruncatedByCleanCompletedExecutions(t *testing.T) } func TestHandler_getHistory_TruncatedByFailedExecutions(t *testing.T) { - execs := pendingExecutions(config.DefaultListLimit) - for i := range execs { - execs[i].Status = "failed" - } - body := runAdminHistory(t, nil, execs, map[string]string{}) + body := runAdminHistory(t, nil, failedExecutions(config.DefaultListLimit), map[string]string{}) assert.Equal(t, true, body["truncated"]) } @@ -108,7 +112,7 @@ func TestHandler_getHistory_AuditGapCompletedExecutionsCountTowardCap(t *testing } func TestHandler_getHistory_ExecutionsLimitIsFixed(t *testing.T) { - body := runAdminHistory(t, historyRows(2), pendingExecutions(config.DefaultListLimit), map[string]string{"limit": "500"}) + body := runAdminHistory(t, historyRows(2), failedExecutions(config.DefaultListLimit), map[string]string{"limit": "500"}) assert.EqualValues(t, 500, body["limit"]) assert.EqualValues(t, config.DefaultListLimit, body["executions_limit"]) assert.Equal(t, true, body["truncated"]) @@ -133,7 +137,7 @@ func TestHandler_getHistory_ScopedUserTruncationLeaksNoCounts(t *testing.T) { ctx := context.Background() const scopedUserID = "scoped-user-id" own := scopedUserID - execs := pendingExecutions(config.DefaultListLimit) + execs := failedExecutions(config.DefaultListLimit) execs[0].CreatedByUserID = &own mockStore := new(MockConfigStore) diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index 7cef4b596..619876da5 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -3488,12 +3488,17 @@ components: True when a fetch reached its row cap, so older rows (and possibly pending approvals) may be missing from purchases and summary. May be a false positive when exactly `limit` or `executions_limit` rows exist. + Also true when the actionable executions class (pending, approved, + running, ...) reached its own cap of 1000 rows. limit: type: integer description: Row cap on completed purchase history rows for this request. executions_limit: type: integer - description: Row cap on non-completed executions (fixed, ignores the limit parameter). + description: >- + Row cap on terminal executions (failed, expired, canceled, ...), + fixed and ignoring the limit parameter. Actionable executions use a + separate cap of 1000. HistorySummary: type: object diff --git a/internal/api/types.go b/internal/api/types.go index 5ec84b3d4..d851bbb40 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -930,12 +930,14 @@ type HistoryResponse struct { Summary HistorySummary `json:"summary"` // Truncated is true when a fetch reached its row cap, so older rows (and // possibly pending approvals) may be missing from Purchases and Summary. - // It can be a false positive when exactly Limit (or ExecutionsLimit) rows exist. + // It also fires when the actionable execution class hits its MaxListLimit cap. + // It can be a false positive when exactly a cap's worth of rows exist. Truncated bool `json:"truncated"` // Limit is the row cap on completed purchase_history rows for this request. Limit int `json:"limit"` - // ExecutionsLimit is the row cap on non-completed executions, which is - // fixed regardless of the request's limit parameter. + // ExecutionsLimit is the row cap on terminal executions (failed, expired, + // canceled, ...), fixed regardless of the request's limit parameter. + // Actionable executions (pending, approved, ...) use config.MaxListLimit. ExecutionsLimit int `json:"executions_limit"` } diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index a7caa7a01..a52a1044c 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -3,6 +3,8 @@ package mocks import ( "context" "fmt" + "slices" + "sort" "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" @@ -1578,7 +1580,31 @@ func (m *MockConfigStore) GetExecutionsByStatuses(ctx context.Context, statuses if !ok { panic(fmt.Sprintf("mock: expected []config.PurchaseExecution, got %T", args.Get(0))) } - return v, args.Error(1) + return filterExecutionsLikeStore(v, statuses, limit), args.Error(1) +} + +// filterExecutionsLikeStore applies the store contract to canned rows: filter by +// status, drop clean completed rows, order by ScheduledDate DESC, then cap. +func filterExecutionsLikeStore(rows []config.PurchaseExecution, statuses []string, limit int) []config.PurchaseExecution { + if len(statuses) == 0 { + return nil + } + if limit <= 0 { + limit = config.DefaultListLimit + } + limit = min(limit, config.MaxListLimit) + var out []config.PurchaseExecution + for _, r := range rows { + if !slices.Contains(statuses, r.Status) || (r.Status == "completed" && r.Error == "") { + continue + } + out = append(out, r) + } + sort.SliceStable(out, func(i, j int) bool { return out[i].ScheduledDate.After(out[j].ScheduledDate) }) + if len(out) > limit { + out = out[:limit] + } + return out } // CountExecutionsByPlanAndStatus mocks the CountExecutionsByPlanAndStatus operation. From 269f3d5fc23510206b4c6258759ed65a51a57357 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 8 Oct 2026 22:04:48 +0200 Subject: [PATCH 2/2] fix(history): satisfy lint findings and pin the actionable cap by outcome - handler_history.go: build the merged execution list in a fresh slice instead of appending onto a re-sliced one (gocritic appendAssign). - mocks/stores.go: range the canned rows by index (gocritic rangeValCopy). - handler_history_truncation_test.go: failedExecutionsAtCap takes no parameter, as every caller passed DefaultListLimit (unparam). - Add a test that more than DefaultListLimit pending rows are all returned and not flagged truncated, so the 1000 actionable cap is pinned by outcome and not only by the limit argument (review note R2). Refs #632 --- internal/api/handler_history.go | 4 +++- .../handler_history_actionable_starvation_test.go | 14 ++++++++++++++ internal/api/handler_history_truncation_test.go | 12 ++++++------ internal/mocks/stores.go | 5 +++-- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index 7922fb33b..864938cdf 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -166,7 +166,9 @@ const approvalExpiryWindow = 7 * 24 * time.Hour func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution, bool) { actionable := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, config.MaxListLimit) terminal := h.fetchExecutionsByClass(ctx, terminalExecutionStatuses, config.DefaultListLimit) - executions := append(actionable[:len(actionable):len(actionable)], terminal...) + executions := make([]config.PurchaseExecution, 0, len(actionable)+len(terminal)) + executions = append(executions, actionable...) + executions = append(executions, terminal...) if len(executions) == 0 { return nil, nil, false } diff --git a/internal/api/handler_history_actionable_starvation_test.go b/internal/api/handler_history_actionable_starvation_test.go index 05e53a109..5a9e7a028 100644 --- a/internal/api/handler_history_actionable_starvation_test.go +++ b/internal/api/handler_history_actionable_starvation_test.go @@ -69,6 +69,20 @@ func TestHandler_getHistory_PendingNotStarvedByNewerTerminalRows(t *testing.T) { assert.EqualValues(t, 1, summary["total_pending"]) } +func TestHandler_getHistory_ReturnsMoreThanDefaultLimitPendingRows(t *testing.T) { + ctx := context.Background() + now := time.Now() + pending := executionsWithStatus(config.DefaultListLimit+50, "pending", now, "pending") + store := newStarvationStore(ctx) + store.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(pending, nil) + + body := runStarvationHistory(t, ctx, store) + + assert.Len(t, body["purchases"], config.DefaultListLimit+50) + assert.EqualValues(t, config.DefaultListLimit+50, body["summary"].(map[string]any)["total_pending"]) + assert.Equal(t, false, body["truncated"]) +} + func TestHandler_getHistory_FetchesDisjointClassesWithTheirLimits(t *testing.T) { ctx := context.Background() store := newStarvationStore(ctx) diff --git a/internal/api/handler_history_truncation_test.go b/internal/api/handler_history_truncation_test.go index 30bf2c5c5..5e022713b 100644 --- a/internal/api/handler_history_truncation_test.go +++ b/internal/api/handler_history_truncation_test.go @@ -69,12 +69,12 @@ func TestHandler_getHistory_TruncatedWhenCompletedAtCap(t *testing.T) { func TestHandler_getHistory_TruncatedWhenExecutionsAtCap(t *testing.T) { // The execution fetch is capped before the in-memory filters run, so a // full page means pending approvals may have been dropped. - body := runAdminHistory(t, nil, failedExecutions(config.DefaultListLimit), map[string]string{}) + body := runAdminHistory(t, nil, failedExecutionsAtCap(), map[string]string{}) assert.Equal(t, true, body["truncated"]) } -func failedExecutions(n int) []config.PurchaseExecution { - execs := pendingExecutions(n) +func failedExecutionsAtCap() []config.PurchaseExecution { + execs := pendingExecutions(config.DefaultListLimit) for i := range execs { execs[i].Status = "failed" } @@ -98,7 +98,7 @@ func TestHandler_getHistory_NotTruncatedByCleanCompletedExecutions(t *testing.T) } func TestHandler_getHistory_TruncatedByFailedExecutions(t *testing.T) { - body := runAdminHistory(t, nil, failedExecutions(config.DefaultListLimit), map[string]string{}) + body := runAdminHistory(t, nil, failedExecutionsAtCap(), map[string]string{}) assert.Equal(t, true, body["truncated"]) } @@ -112,7 +112,7 @@ func TestHandler_getHistory_AuditGapCompletedExecutionsCountTowardCap(t *testing } func TestHandler_getHistory_ExecutionsLimitIsFixed(t *testing.T) { - body := runAdminHistory(t, historyRows(2), failedExecutions(config.DefaultListLimit), map[string]string{"limit": "500"}) + body := runAdminHistory(t, historyRows(2), failedExecutionsAtCap(), map[string]string{"limit": "500"}) assert.EqualValues(t, 500, body["limit"]) assert.EqualValues(t, config.DefaultListLimit, body["executions_limit"]) assert.Equal(t, true, body["truncated"]) @@ -137,7 +137,7 @@ func TestHandler_getHistory_ScopedUserTruncationLeaksNoCounts(t *testing.T) { ctx := context.Background() const scopedUserID = "scoped-user-id" own := scopedUserID - execs := failedExecutions(config.DefaultListLimit) + execs := failedExecutionsAtCap() execs[0].CreatedByUserID = &own mockStore := new(MockConfigStore) diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index a52a1044c..ba42adbb4 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -1594,11 +1594,12 @@ func filterExecutionsLikeStore(rows []config.PurchaseExecution, statuses []strin } limit = min(limit, config.MaxListLimit) var out []config.PurchaseExecution - for _, r := range rows { + for i := range rows { + r := &rows[i] if !slices.Contains(statuses, r.Status) || (r.Status == "completed" && r.Error == "") { continue } - out = append(out, r) + out = append(out, *r) } sort.SliceStable(out, func(i, j int) bool { return out[i].ScheduledDate.After(out[j].ScheduledDate) }) if len(out) > limit {