Skip to content

fix(purchases): exclude cancelled rows from summarizePurchaseHistory (closes #625) #821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 3 commits into from
Jul 17, 2026
Merged

fix(purchases): exclude cancelled rows from summarizePurchaseHistory (closes #625) #821

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d187f8e
fix(purchases): cite #625 in cancelled-KPI regression tests
cristim May 28, 2026
662e2bc
test(purchases): fix misspell/prealloc/govet in handler_history_test
cristim Jun 19, 2026
855a9bf
fix(lint): fix govet/misspell nolints in handler_history_test
cristim Jun 20, 2026
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
40 changes: 20 additions & 20 deletions internal/api/handler_history_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,7 @@ func TestHandler_getHistory_CompletedExecutionNotDuplicated(t *testing.T) {
}

// TestSummarizePurchaseHistory_CancelledExcludedFromKPIs is the regression
// test for issue #736. Canceling a pending purchase must not add its upfront
// test for issues #625 and #736. Canceling 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.
Expand All @@ -1710,8 +1710,8 @@ func TestSummarizePurchaseHistory_CancelledExcludedFromKPIs(t *testing.T) {
{Status: "pending", UpfrontCost: 999.0, EstimatedSavings: 99.0},
// Two canceled rows — the regression case from issue #736.
// Neither must appear in the dollar KPIs or TotalCompleted.
{Status: "canceled", UpfrontCost: 500.0, EstimatedSavings: 50.0},
{Status: "canceled", UpfrontCost: 750.0, EstimatedSavings: 75.0},
{Status: "cancelled", UpfrontCost: 500.0, EstimatedSavings: 50.0}, //nolint:misspell // DB schema value 'cancelled' -- see migration 000001_initial_schema.up.sql
{Status: "cancelled", UpfrontCost: 750.0, EstimatedSavings: 75.0}, //nolint:misspell // DB schema value 'cancelled' -- see migration 000001_initial_schema.up.sql
}

summary := summarizePurchaseHistory(purchases)
Expand All @@ -1721,17 +1721,17 @@ func TestSummarizePurchaseHistory_CancelledExcludedFromKPIs(t *testing.T) {
assert.Equal(t, 1, summary.TotalPending)

assert.InDelta(t, 350.0, summary.TotalUpfront, 0.001,
"canceled upfront cost must not be included in TotalUpfront (issue #736)")
"canceled upfront cost must not be included in TotalUpfront (issues #625, #736)")
assert.InDelta(t, 35.0, summary.TotalMonthlySavings, 0.001,
"canceled savings must not be included in TotalMonthlySavings (issue #736)")
"canceled savings must not be included in TotalMonthlySavings (issues #625, #736)")
assert.InDelta(t, 420.0, summary.TotalAnnualSavings, 0.001,
"TotalAnnualSavings = TotalMonthlySavings * 12 and must exclude canceled (issue #736)")
"TotalAnnualSavings = TotalMonthlySavings * 12 and must exclude canceled (issues #625, #736)")
}

// TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs mirrors the
// QA reproduction scenario from issue #736: start with N approved purchases,
// observe KPI totals, then add a canceled execution and assert the totals
// are unchanged.
// QA reproduction scenario from issues #625 and #736: start with N approved
// purchases, observe KPI totals, then add a canceled execution and assert the
// totals are unchanged.
// TestHandler_getHistory_LimitParsing is the 01-M1 regression guard.
// Prior to the fix, parseHistoryFilters used fmt.Sscanf to parse the limit
// query param, which silently swallows non-integer input (callers get the
Expand Down Expand Up @@ -1811,30 +1811,30 @@ func TestHandler_getHistory_LimitParsing(t *testing.T) {
})
}
}

func TestSummarizePurchaseHistory_CancelPendingDoesNotChangeKPIs(t *testing.T) {
// Baseline: three approved (completed) rows.
baseline := []config.PurchaseHistoryRecord{ //nolint:prealloc // composite literal with fixed elements; append below adds 1 more
{Status: "completed", UpfrontCost: 100.0, EstimatedSavings: 10.0},
{Status: "completed", UpfrontCost: 200.0, EstimatedSavings: 20.0},
{Status: "completed", UpfrontCost: 300.0, EstimatedSavings: 30.0},
}
baseline := make([]config.PurchaseHistoryRecord, 0, 4)
baseline = append(baseline,
config.PurchaseHistoryRecord{Status: "completed", UpfrontCost: 100.0, EstimatedSavings: 10.0},
config.PurchaseHistoryRecord{Status: "completed", UpfrontCost: 200.0, EstimatedSavings: 20.0},
config.PurchaseHistoryRecord{Status: "completed", UpfrontCost: 300.0, EstimatedSavings: 30.0},
)
before := summarizePurchaseHistory(baseline)

// After: same rows plus one canceled execution (the pending that got canceled).
withCancelled := append(baseline, config.PurchaseHistoryRecord{ //nolint:gocritic
Status: "canceled",
Status: "cancelled", //nolint:misspell // DB schema value 'cancelled' -- see migration 000001_initial_schema.up.sql
UpfrontCost: 999.0,
EstimatedSavings: 99.0,
})
after := summarizePurchaseHistory(withCancelled)

assert.Equal(t, before.TotalUpfront, after.TotalUpfront,
"canceling a pending purchase must not change TotalUpfront (issue #736)")
"canceling a pending purchase must not change TotalUpfront (issues #625, #736)")
assert.Equal(t, before.TotalMonthlySavings, after.TotalMonthlySavings,
"canceling a pending purchase must not change TotalMonthlySavings (issue #736)")
"canceling a pending purchase must not change TotalMonthlySavings (issues #625, #736)")
assert.Equal(t, before.TotalAnnualSavings, after.TotalAnnualSavings,
"canceling a pending purchase must not change TotalAnnualSavings (issue #736)")
"canceling a pending purchase must not change TotalAnnualSavings (issues #625, #736)")
assert.Equal(t, before.TotalCompleted, after.TotalCompleted,
"canceling a pending purchase must not change TotalCompleted (issue #736)")
"canceling a pending purchase must not change TotalCompleted (issues #625, #736)")
}
Loading