Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 35 additions & 20 deletions internal/api/handler_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,13 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR
// filters) because the cap drops rows before they run. It is a single
// boolean: a count would reveal other tenants' row volume to a scoped user.
return HistoryResponse{
Summary: summary,
SummaryComplete: summaryComplete,
Purchases: all,
Truncated: len(completed) >= filters.Limit || executionsCapped,
Limit: filters.Limit,
ExecutionsLimit: config.DefaultListLimit,
Summary: summary,
SummaryComplete: summaryComplete,
Purchases: all,
Truncated: len(completed) >= filters.Limit || executionsCapped,
Limit: filters.Limit,
ExecutionsLimit: config.DefaultListLimit,
ActionableExecutionsLimit: config.MaxListLimit,
}, nil
}

Expand Down Expand Up @@ -171,17 +172,14 @@ const approvalExpiryWindow = 7 * 24 * time.Hour
// provider via the recs' collapsed provider, account via CloudAccountID,
// date via ScheduledDate.
//
// 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.
// The third return value reports that executions may be missing: either class
// reached its cap (actionable at MaxListLimit, terminal at DefaultListLimit) so
// older executions may have been dropped before the filters ran, or a class
// listing failed (issue #686).
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 := make([]config.PurchaseExecution, 0, len(actionable)+len(terminal))
executions = append(executions, actionable...)
executions = append(executions, terminal...)
executions, incomplete := h.fetchExecutionClasses(ctx)
if len(executions) == 0 {
return nil, nil, false
return nil, nil, incomplete
}
approver := h.resolvePendingApproverEmail(ctx)
userEmailCache := h.resolveUserEmails(ctx, executions)
Expand Down Expand Up @@ -218,19 +216,36 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF
}
out = append(out, executionToHistoryRow(exec, approver, createdByEmail))
}
capped := len(actionable) >= config.MaxListLimit || terminalSurfaced >= config.DefaultListLimit
capped := incomplete || terminalSurfaced >= config.DefaultListLimit
return out, staleExecs, capped
}

// fetchExecutionClasses lists the actionable and terminal classes, each with
// its own cap, and returns them concatenated (actionable first). incomplete
// reports that rows may be missing before the filters run: a class listing
// failed, or the actionable class reached its MaxListLimit cap. The terminal
// cap is judged by the caller on the rows it actually surfaces.
func (h *Handler) fetchExecutionClasses(ctx context.Context) ([]config.PurchaseExecution, bool) {
actionable, actionableOK := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, config.MaxListLimit)
terminal, terminalOK := h.fetchExecutionsByClass(ctx, terminalExecutionStatuses, config.DefaultListLimit)
executions := make([]config.PurchaseExecution, 0, len(actionable)+len(terminal))
executions = append(executions, actionable...)
executions = append(executions, terminal...)
incomplete := !actionableOK || !terminalOK || len(actionable) >= config.MaxListLimit
return executions, incomplete
}

// 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 {
// the class skipped (ok=false) so the other class and completed history still
// render; the caller flags the response as incomplete so the hidden rows are
// not silent (issue #686).
func (h *Handler) fetchExecutionsByClass(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, bool) {
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 nil, false
}
return rows
return rows, true
}

func isTerminalExecutionStatus(status string) bool {
Expand Down
28 changes: 28 additions & 0 deletions internal/api/handler_history_actionable_starvation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,38 @@ func TestHandler_getHistory_ClassListingErrorKeepsOtherClass(t *testing.T) {
purchases := body["purchases"].([]any)
require.Len(t, purchases, 1)
assert.Equal(t, tt.wantID, purchases[0].(map[string]any)["purchase_id"])
assert.Equal(t, true, body["truncated"], "a failed class listing hides rows, so the response must flag incomplete results")
})
}
}

// TestHandler_getHistory_BothClassListingsFailSetTruncated covers the
// no-executions early return: when both class listings fail the response
// carries only completed history and must still flag incomplete results.
func TestHandler_getHistory_BothClassListingsFailSetTruncated(t *testing.T) {
ctx := context.Background()
store := newStarvationStore(ctx)
store.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(nil, errors.New("listing failed"))

body := runStarvationHistory(t, ctx, store)

assert.Empty(t, body["purchases"])
assert.Equal(t, true, body["truncated"])
}

// TestHandler_getHistory_EmptyClassListingsNotTruncated guards the other side
// of the listing-error signal: two successful but empty listings are a
// complete result and must not raise the banner.
func TestHandler_getHistory_EmptyClassListingsNotTruncated(t *testing.T) {
ctx := context.Background()
store := newStarvationStore(ctx)
store.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil)

body := runStarvationHistory(t, ctx, store)

assert.Equal(t, false, body["truncated"])
}

func TestHandler_getHistory_TruncatedPerClass(t *testing.T) {
now := time.Now()
tests := []struct {
Expand Down
10 changes: 10 additions & 0 deletions internal/api/handler_history_truncation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,16 @@ func TestHandler_getHistory_ExecutionsLimitIsFixed(t *testing.T) {
assert.Equal(t, true, body["truncated"])
}

// The actionable class (pending, approved, ...) has its own, larger cap. The
// response must expose it so the truncation banner can quote the cap that was
// actually hit instead of the terminal-class cap (issue #686).
func TestHandler_getHistory_ActionableExecutionsLimitExposed(t *testing.T) {
body := runAdminHistory(t, nil, pendingExecutions(config.MaxListLimit), map[string]string{})
assert.Equal(t, true, body["truncated"])
assert.EqualValues(t, config.MaxListLimit, body["actionable_executions_limit"])
assert.EqualValues(t, config.DefaultListLimit, body["executions_limit"])
}

func TestHandler_getHistory_TruncatedHonorsCustomLimit(t *testing.T) {
body := runAdminHistory(t, historyRows(5), nil, map[string]string{"limit": "5"})
assert.Equal(t, true, body["truncated"])
Expand Down
14 changes: 11 additions & 3 deletions internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4055,16 +4055,24 @@ components:
counts them while summary_complete is true. 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.
running, ...) reached its own cap (`actionable_executions_limit`),
and when listing either execution class failed, so its rows are
missing.
limit:
type: integer
description: Row cap on completed purchase history rows for this request.
executions_limit:
type: integer
description: >-
Row cap on terminal executions (failed, expired, canceled, ...),
fixed and ignoring the limit parameter. Actionable executions use a
separate cap of 1000.
fixed and ignoring the limit parameter. Actionable executions use
the separate `actionable_executions_limit` cap.
actionable_executions_limit:
type: integer
description: >-
Row cap on actionable executions (pending, notified, scheduled,
approved, running, paused), fixed at 1000 and ignoring the limit
parameter.

HistorySummary:
type: object
Expand Down
9 changes: 7 additions & 2 deletions internal/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -969,15 +969,20 @@ type HistoryResponse struct {
// Truncated is true when a fetch reached its row cap, so older rows (and
// possibly pending approvals) may be missing from Purchases. Summary still
// counts them while SummaryComplete is true.
// It also fires when the actionable execution class hits its MaxListLimit cap.
// It also fires when the actionable execution class hits its MaxListLimit cap,
// and when an execution class listing failed, so its rows are missing.
// 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 terminal executions (failed, expired,
// canceled, ...), fixed regardless of the request's limit parameter.
// Actionable executions (pending, approved, ...) use config.MaxListLimit.
// Actionable executions (pending, approved, ...) use ActionableExecutionsLimit.
ExecutionsLimit int `json:"executions_limit"`
// ActionableExecutionsLimit is the row cap on actionable executions
// (pending, notified, scheduled, approved, running, paused), fixed at
// config.MaxListLimit regardless of the request's limit parameter.
ActionableExecutionsLimit int `json:"actionable_executions_limit"`
}

// HistorySummary provides aggregate statistics for purchase history.
Expand Down
Loading