From caf07f757fbd8c67d21e1c25ddcbc631079a4ef5 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 26 May 2026 17:37:41 +0200 Subject: [PATCH] fix(api/history): exclude cancelled executions from KPI dollar totals summarizePurchaseHistory had no case for the "cancelled" status in its switch, so cancelled rows fell through to the default path and added their UpfrontCost and EstimatedSavings to TotalUpfront, TotalMonthlySavings, and TotalAnnualSavings -- as if the purchase had been approved. A cancelled purchase represents zero committed spend and zero realized savings, so it must be excluded from all three aggregates (and from TotalCompleted). Fix: add `case "cancelled": continue` to the switch, matching the existing pattern for pending/failed/expired rows. Update the comment on HistorySummary.TotalPurchases and the inline comment in the loop to document the cancelled exclusion. Regression tests (issue #736): - TestSummarizePurchaseHistory_CancelledExcludedFromKPIs: N completed rows + 1 pending + 2 pre-existing cancelled rows; asserts only the completed rows contribute to the dollar totals. - TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs: mirrors the QA reproduction scenario -- baseline KPI totals captured, then a cancelled row added; asserts all three KPIs and TotalCompleted are unchanged. --- internal/api/handler_history.go | 13 ++++-- internal/api/handler_history_test.go | 66 ++++++++++++++++++++++++++++ internal/api/types.go | 6 +-- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index 0d7e74d13..195ca9a24 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -443,10 +443,10 @@ func summarizePurchaseHistory(purchases []config.PurchaseHistoryRecord) HistoryS summary := HistorySummary{TotalPurchases: len(purchases)} for _, p := range purchases { // Non-completed rows count toward TotalPurchases and their specific - // bucket (pending / in-progress / failed / expired) but are excluded - // from the dollar totals — the money hasn't been committed for any of - // those states. "completed" and unset (legacy DB rows that pre-date - // the status field) both count as completed. + // bucket (pending / in-progress / failed / expired / cancelled) but + // are excluded from the dollar totals — the money hasn't been committed + // for any of those states. "completed" and unset (legacy DB rows that + // pre-date the status field) both count as completed. switch p.Status { case "pending", "notified": summary.TotalPending++ @@ -463,6 +463,11 @@ func summarizePurchaseHistory(purchases []config.PurchaseHistoryRecord) HistoryS case "expired": summary.TotalExpired++ continue + case "cancelled": + // A cancelled purchase represents zero committed spend and zero + // realized savings (issue #736). Exclude from all dollar KPIs and + // from TotalCompleted — the money was never committed. + continue } summary.TotalCompleted++ // Audit-gap completed rows (issue #621) are synthesised execution rows diff --git a/internal/api/handler_history_test.go b/internal/api/handler_history_test.go index 4e1c14281..b757d9dc0 100644 --- a/internal/api/handler_history_test.go +++ b/internal/api/handler_history_test.go @@ -666,3 +666,69 @@ func TestHandler_getHistory_CompletedExecutionNotDuplicated(t *testing.T) { assert.Equal(t, 1, resp.Summary.TotalCompleted) assert.Equal(t, 400.0, resp.Summary.TotalUpfront) } + +// TestSummarizePurchaseHistory_CancelledExcludedFromKPIs is the regression +// test for issue #736. Cancelling a pending purchase must not add its upfront +// cost or savings to the KPI totals. Specifically: +// - TotalUpfront, TotalMonthlySavings, TotalAnnualSavings must reflect only +// the approved/completed rows. +// - TotalCompleted must not include cancelled rows. +// - A pre-existing cancelled row in the dataset must also be excluded. +func TestSummarizePurchaseHistory_CancelledExcludedFromKPIs(t *testing.T) { + purchases := []config.PurchaseHistoryRecord{ + // Three completed rows that should contribute to the KPI totals. + {Status: "completed", UpfrontCost: 100.0, EstimatedSavings: 10.0}, + {Status: "completed", UpfrontCost: 200.0, EstimatedSavings: 20.0}, + {Status: "", UpfrontCost: 50.0, EstimatedSavings: 5.0}, // legacy row, no status + // One pending row that should be counted as pending, not completed. + {Status: "pending", UpfrontCost: 999.0, EstimatedSavings: 99.0}, + // Two cancelled rows — the regression case from issue #736. + // Neither must appear in the dollar KPIs or TotalCompleted. + {Status: "cancelled", UpfrontCost: 500.0, EstimatedSavings: 50.0}, + {Status: "cancelled", UpfrontCost: 750.0, EstimatedSavings: 75.0}, + } + + summary := summarizePurchaseHistory(purchases) + + assert.Equal(t, 6, summary.TotalPurchases, "all rows count toward TotalPurchases") + assert.Equal(t, 3, summary.TotalCompleted, "cancelled rows must not inflate TotalCompleted") + assert.Equal(t, 1, summary.TotalPending) + + assert.InDelta(t, 350.0, summary.TotalUpfront, 0.001, + "cancelled upfront cost must not be included in TotalUpfront (issue #736)") + assert.InDelta(t, 35.0, summary.TotalMonthlySavings, 0.001, + "cancelled savings must not be included in TotalMonthlySavings (issue #736)") + assert.InDelta(t, 420.0, summary.TotalAnnualSavings, 0.001, + "TotalAnnualSavings = TotalMonthlySavings * 12 and must exclude cancelled (issue #736)") +} + +// TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs mirrors the +// QA reproduction scenario from issue #736: start with N approved purchases, +// observe KPI totals, then add a cancelled execution and assert the totals +// are unchanged. +func TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs(t *testing.T) { + // Baseline: three approved (completed) rows. + baseline := []config.PurchaseHistoryRecord{ + {Status: "completed", UpfrontCost: 100.0, EstimatedSavings: 10.0}, + {Status: "completed", UpfrontCost: 200.0, EstimatedSavings: 20.0}, + {Status: "completed", UpfrontCost: 300.0, EstimatedSavings: 30.0}, + } + before := summarizePurchaseHistory(baseline) + + // After: same rows plus one cancelled execution (the pending that got cancelled). + withCancelled := append(baseline, config.PurchaseHistoryRecord{ //nolint:gocritic + Status: "cancelled", + UpfrontCost: 999.0, + EstimatedSavings: 99.0, + }) + after := summarizePurchaseHistory(withCancelled) + + assert.Equal(t, before.TotalUpfront, after.TotalUpfront, + "cancelling a pending purchase must not change TotalUpfront (issue #736)") + assert.Equal(t, before.TotalMonthlySavings, after.TotalMonthlySavings, + "cancelling a pending purchase must not change TotalMonthlySavings (issue #736)") + assert.Equal(t, before.TotalAnnualSavings, after.TotalAnnualSavings, + "cancelling a pending purchase must not change TotalAnnualSavings (issue #736)") + assert.Equal(t, before.TotalCompleted, after.TotalCompleted, + "cancelling a pending purchase must not change TotalCompleted (issue #736)") +} diff --git a/internal/api/types.go b/internal/api/types.go index ed08efbec..6b4ec4f64 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -728,9 +728,9 @@ type HistoryResponse struct { // HistorySummary provides aggregate statistics for purchase history. // TotalPurchases is the total count of rows (completed + all non-completed // states); the per-state counters break it down so the UI can render -// meaningful totals. Dollar totals count completed rows only — none of the -// non-completed states have committed money yet, so folding them in would -// inflate "what I have spent". +// meaningful totals. Dollar totals count completed rows only: pending, +// in-progress, failed, expired, and cancelled rows are all excluded because +// no money was committed for any of those states. type HistorySummary struct { TotalPurchases int `json:"total_purchases"` TotalCompleted int `json:"total_completed"`