Skip to content

Commit e91eec5

Browse files
committed
feat(db): nullable MonthlyCost on PurchaseHistoryRecord (closes #255)
Migration 000063 drops NOT NULL on purchase_history.monthly_cost so rows from providers without a monthly recurring breakdown (Azure/GCP all-upfront) can store NULL instead of a misleading 0.0. - PurchaseHistoryRecord.MonthlyCost: float64 -> *float64 - InventoryCommitment.MonthlyCost: float64 -> *float64 - All scan loops use sql.NullFloat64; nil DB value -> nil pointer - execution.go: remove derefFloat64; pass pointer straight through - handler_inventory.go: skip nil MonthlyCost in coverage aggregation - handler_history.go: sumRecommendationMonthlyCostPtr returns nil when every rec has nil MonthlyCost (renders as -- not $0.00) - Frontend: monthly_cost typed number|null; inventory cell renders -- - All test fixtures and assertions updated to use *float64 (pf helper)
1 parent 4956d66 commit e91eec5

21 files changed

Lines changed: 110 additions & 53 deletions

‎frontend/src/api/types.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,8 @@ export interface InventoryCommitment {
660660
start_date: string;
661661
end_date: string;
662662
upfront_cost: number;
663-
monthly_cost: number;
663+
/** null when the provider API did not return a monthly recurring breakdown; "$X.XX" when populated, "—" when null. */
664+
monthly_cost: number | null;
664665
estimated_savings: number;
665666
/**
666667
* Always "active" today — the backend filters expired rows before

‎frontend/src/inventory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ function buildCommitmentRow(c: api.InventoryCommitment): HTMLTableRowElement {
182182
appendCell(tr, String(c.count));
183183
appendCell(tr, `${c.term_years}y`);
184184
appendCell(tr, c.payment_option ?? '');
185-
appendCell(tr, formatCurrency(c.monthly_cost));
185+
appendCell(tr, c.monthly_cost != null ? formatCurrency(c.monthly_cost) : '—');
186186
appendCell(tr, formatDate(c.end_date));
187187

188188
return tr;

‎frontend/src/types.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ export interface HistoryPurchase {
187187
term: number;
188188
payment?: string;
189189
upfront_cost: number;
190-
monthly_cost?: number;
190+
monthly_cost?: number | null;
191191
estimated_savings: number;
192192
account_id?: string;
193193
plan_name?: string;

‎internal/api/handler_history.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -380,17 +380,15 @@ func projectRecommendationFields(row *config.PurchaseHistoryRecord, exec config.
380380
row.Payment = r.Payment
381381
row.UpfrontCost = r.UpfrontCost
382382
row.EstimatedSavings = r.Savings
383-
if r.MonthlyCost != nil {
384-
row.MonthlyCost = *r.MonthlyCost
385-
}
383+
row.MonthlyCost = r.MonthlyCost
386384
return
387385
}
388386
row.Region = "multiple"
389387
row.ResourceType = fmt.Sprintf("%d commitment(s)", len(recs))
390388
row.Service = collapseRecommendationService(recs)
391389
row.Term = collapseRecommendationTerm(recs)
392390
row.Payment = collapseRecommendationPayment(recs)
393-
row.MonthlyCost = sumRecommendationMonthlyCost(recs)
391+
row.MonthlyCost = sumRecommendationMonthlyCostPtr(recs)
394392
row.UpfrontCost = exec.TotalUpfrontCost
395393
row.EstimatedSavings = exec.EstimatedSavings
396394
}
@@ -486,21 +484,24 @@ func collapseRecommendationAccount(recs []config.RecommendationRecord) string {
486484
return first
487485
}
488486

489-
// sumRecommendationMonthlyCost adds up the per-rec MonthlyCost values in a
490-
// multi-rec execution so the Approval queue's Monthly Cost cell shows the
491-
// committed recurring spend for the full basket. Nil per-rec entries
492-
// contribute 0 (the provider API did not return a monthly breakdown for
493-
// that rec) — the same treatment as the single-rec branch, which only
494-
// copies MonthlyCost when non-nil and otherwise leaves the row's field at
495-
// the zero value.
496-
func sumRecommendationMonthlyCost(recs []config.RecommendationRecord) float64 {
487+
// sumRecommendationMonthlyCostPtr sums the per-rec MonthlyCost values for a
488+
// multi-rec execution row in the Approval queue. Nil entries (provider API
489+
// did not return a monthly breakdown) are skipped. Returns nil when every rec
490+
// has a nil MonthlyCost so the row renders as "—" rather than "$0.00".
491+
// Returns a pointer to the accumulated total otherwise.
492+
func sumRecommendationMonthlyCostPtr(recs []config.RecommendationRecord) *float64 {
497493
var total float64
494+
anyNonNil := false
498495
for _, r := range recs {
499496
if r.MonthlyCost != nil {
500497
total += *r.MonthlyCost
498+
anyNonNil = true
501499
}
502500
}
503-
return total
501+
if !anyNonNil {
502+
return nil
503+
}
504+
return &total
504505
}
505506

506507
// MaxHistoryDateRangeDays caps the inclusive start/end window the History

‎internal/api/handler_history_test.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,8 @@ func TestHandler_getHistory_InProgressRowMapsRecFields(t *testing.T) {
478478
assert.Equal(t, 1, row.Count)
479479
assert.Equal(t, 0.0, row.UpfrontCost, "upfront must come from the rec")
480480
assert.Equal(t, 1.2333, row.EstimatedSavings, "savings must come from the rec")
481-
assert.Equal(t, 2.117, row.MonthlyCost, "monthly cost must come from the rec")
481+
require.NotNil(t, row.MonthlyCost, "monthly cost must come from the rec")
482+
assert.InDelta(t, 2.117, *row.MonthlyCost, 1e-9, "monthly cost must come from the rec")
482483
assert.Equal(t, "no-upfront", row.Payment, "payment must come from the rec, not be left blank (#733)")
483484
assert.NotEmpty(t, row.StatusDescription,
484485
"in-progress rows must carry a human-readable status description, not render as a finished purchase")
@@ -1177,7 +1178,8 @@ func TestHandler_getHistory_ApprovalQueueColumnsPopulated(t *testing.T) {
11771178

11781179
assert.Equal(t, accountID, row.AccountID, "Account must fall back to rec.CloudAccountID when exec.CloudAccountID is nil (#733)")
11791180
assert.Equal(t, "all-upfront", row.Payment, "Payment must be copied from the single rec (#733)")
1180-
assert.Equal(t, 7.5, row.MonthlyCost, "MonthlyCost must come from the rec")
1181+
require.NotNil(t, row.MonthlyCost, "MonthlyCost must come from the rec")
1182+
assert.InDelta(t, 7.5, *row.MonthlyCost, 1e-9, "MonthlyCost must come from the rec")
11811183
})
11821184

11831185
t.Run("multi-rec uniform pending row collapses Account + Payment, sums MonthlyCost", func(t *testing.T) {
@@ -1214,7 +1216,8 @@ func TestHandler_getHistory_ApprovalQueueColumnsPopulated(t *testing.T) {
12141216

12151217
assert.Equal(t, accountID, row.AccountID, "Account must collapse to the shared rec value (#733)")
12161218
assert.Equal(t, "no-upfront", row.Payment, "Payment must collapse to the shared rec value (#733)")
1217-
assert.InDelta(t, 7.5, row.MonthlyCost, 1e-9, "MonthlyCost must sum across recs (#733)")
1219+
require.NotNil(t, row.MonthlyCost, "MonthlyCost must sum across recs (#733)")
1220+
assert.InDelta(t, 7.5, *row.MonthlyCost, 1e-9, "MonthlyCost must sum across recs (#733)")
12181221
})
12191222

12201223
t.Run("multi-rec heterogeneous Payment collapses to empty for honest dash fallback", func(t *testing.T) {

‎internal/api/handler_inventory.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,9 @@ func (h *Handler) getCoverageBreakdown(ctx context.Context, req *events.LambdaFu
145145
if !isActiveCommitment(p, now) {
146146
continue
147147
}
148-
coveredByKey[p.Provider+":"+p.Service] += p.MonthlyCost
148+
if p.MonthlyCost != nil {
149+
coveredByKey[p.Provider+":"+p.Service] += *p.MonthlyCost
150+
}
149151
}
150152

151153
// --- on-demand gap: recommendations -------------------------------------

‎internal/api/handler_inventory_test.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) {
7474
Term: 1,
7575
Payment: "no-upfront",
7676
Timestamp: now.AddDate(0, -6, 0),
77-
MonthlyCost: 100.0,
77+
MonthlyCost: float64Ptr(100.0),
7878
EstimatedSavings: 30.0,
7979
},
8080
// Expired: bought 2 years ago, 1-year term.
@@ -87,7 +87,7 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) {
8787
Count: 1,
8888
Term: 1,
8989
Timestamp: now.AddDate(-2, 0, 0),
90-
MonthlyCost: 50.0,
90+
MonthlyCost: float64Ptr(50.0),
9191
EstimatedSavings: 15.0,
9292
},
9393
}
@@ -117,7 +117,8 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) {
117117
assert.Equal(t, 2, row.Count)
118118
assert.Equal(t, 1, row.TermYears)
119119
assert.Equal(t, "no-upfront", row.PaymentOption)
120-
assert.Equal(t, 100.0, row.MonthlyCost)
120+
require.NotNil(t, row.MonthlyCost)
121+
assert.Equal(t, 100.0, *row.MonthlyCost)
121122
assert.Equal(t, 30.0, row.EstimatedSavings)
122123
assert.Equal(t, "active", row.Status)
123124
assert.False(t, row.StartDate.IsZero())
@@ -141,7 +142,7 @@ func TestHandler_listActiveCommitments_AccountFilter(t *testing.T) {
141142
Timestamp: now.AddDate(0, -3, 0),
142143
Term: 1,
143144
Count: 1,
144-
MonthlyCost: 80.0,
145+
MonthlyCost: float64Ptr(80.0),
145146
EstimatedSavings: 20.0,
146147
},
147148
}
@@ -378,7 +379,7 @@ func TestHandler_getCoverageBreakdown_Integration(t *testing.T) {
378379
Service: "ec2",
379380
Timestamp: now.AddDate(-1, 0, 1), // active: 1y term started ~1y ago
380381
Term: 1,
381-
MonthlyCost: 150.0,
382+
MonthlyCost: float64Ptr(150.0),
382383
},
383384
}
384385
recs := []config.RecommendationRecord{

‎internal/api/handler_per_account_perms_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,7 @@ func TestPerAccountPerms_CoverageBreakdown_RecsFilteredByAllowedAccounts(t *test
750750
Service: "ec2",
751751
Timestamp: now.AddDate(0, -6, 0),
752752
Term: 1,
753-
MonthlyCost: 200.0,
753+
MonthlyCost: float64Ptr(200.0),
754754
}
755755

756756
// Recommendation for account A: on-demand gap the scoped user is allowed to see.
@@ -830,7 +830,7 @@ func TestPerAccountPerms_CoverageBreakdown_AdminSeesAll(t *testing.T) {
830830
Service: "ec2",
831831
Timestamp: now.AddDate(0, -6, 0),
832832
Term: 1,
833-
MonthlyCost: 200.0,
833+
MonthlyCost: float64Ptr(200.0),
834834
}
835835
recA := config.RecommendationRecord{
836836
Provider: "aws", Service: "ec2",

‎internal/api/types.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,11 @@ type InventoryCommitment struct {
508508
PaymentOption string `json:"payment_option,omitempty"`
509509
StartDate time.Time `json:"start_date"`
510510
EndDate time.Time `json:"end_date"`
511-
UpfrontCost float64 `json:"upfront_cost"`
512-
MonthlyCost float64 `json:"monthly_cost"`
511+
UpfrontCost float64 `json:"upfront_cost"`
512+
// MonthlyCost is nil when the source purchase_history row has a NULL
513+
// monthly_cost (provider did not return a monthly breakdown). The
514+
// frontend renders "—" for nil and "$X.XX" when non-nil.
515+
MonthlyCost *float64 `json:"monthly_cost"`
513516
EstimatedSavings float64 `json:"estimated_savings"`
514517
Status string `json:"status"`
515518
}

‎internal/config/store_postgres.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1405,6 +1405,7 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string,
14051405
for rows.Next() {
14061406
var record PurchaseHistoryRecord
14071407
var planID, planName, cloudAccountID sql.NullString
1408+
var monthlyCost sql.NullFloat64
14081409

14091410
err := rows.Scan(
14101411
&record.AccountID,
@@ -1418,7 +1419,7 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string,
14181419
&record.Term,
14191420
&record.Payment,
14201421
&record.UpfrontCost,
1421-
&record.MonthlyCost,
1422+
&monthlyCost,
14221423
&record.EstimatedSavings,
14231424
&planID,
14241425
&planName,
@@ -1429,6 +1430,13 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string,
14291430
return nil, fmt.Errorf("failed to scan purchase history: %w", err)
14301431
}
14311432

1433+
// Handle nullable monthly_cost: nil means "provider did not return a
1434+
// monthly breakdown"; 0.0 means "explicitly $0 recurring charge".
1435+
if monthlyCost.Valid {
1436+
v := monthlyCost.Float64
1437+
record.MonthlyCost = &v
1438+
}
1439+
14321440
// Handle nullable strings
14331441
if planID.Valid {
14341442
record.PlanID = planID.String

0 commit comments

Comments
 (0)