Skip to content
Open
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
3 changes: 2 additions & 1 deletion frontend/src/api/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@ export interface InventoryCommitment {
start_date: string;
end_date: string;
upfront_cost: number;
monthly_cost: number;
/** null when the provider API did not return a monthly recurring breakdown; "$X.XX" when populated, "—" when null. */
monthly_cost: number | null;
estimated_savings: number;
/**
* Always "active" today — the backend filters expired rows before
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/inventory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ function buildCommitmentRow(c: api.InventoryCommitment): HTMLTableRowElement {
appendCell(tr, String(c.count));
appendCell(tr, `${c.term_years}y`);
appendCell(tr, c.payment_option ?? '');
appendCell(tr, formatCurrency(c.monthly_cost));
appendCell(tr, c.monthly_cost != null ? formatCurrency(c.monthly_cost) : '—');
appendCell(tr, formatCurrency(c.estimated_savings));
appendCell(tr, formatDate(c.end_date));

Expand Down
2 changes: 1 addition & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export interface HistoryPurchase {
term: number;
payment?: string;
upfront_cost: number;
monthly_cost?: number;
monthly_cost?: number | null;
estimated_savings: number;
account_id?: string;
plan_name?: string;
Expand Down
27 changes: 14 additions & 13 deletions internal/api/handler_history.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,17 +380,15 @@ func projectRecommendationFields(row *config.PurchaseHistoryRecord, exec config.
row.Payment = r.Payment
row.UpfrontCost = r.UpfrontCost
row.EstimatedSavings = r.Savings
if r.MonthlyCost != nil {
row.MonthlyCost = *r.MonthlyCost
}
row.MonthlyCost = r.MonthlyCost
return
}
row.Region = "multiple"
row.ResourceType = fmt.Sprintf("%d commitment(s)", len(recs))
row.Service = collapseRecommendationService(recs)
row.Term = collapseRecommendationTerm(recs)
row.Payment = collapseRecommendationPayment(recs)
row.MonthlyCost = sumRecommendationMonthlyCost(recs)
row.MonthlyCost = sumRecommendationMonthlyCostPtr(recs)
row.UpfrontCost = exec.TotalUpfrontCost
row.EstimatedSavings = exec.EstimatedSavings
}
Expand Down Expand Up @@ -486,21 +484,24 @@ func collapseRecommendationAccount(recs []config.RecommendationRecord) string {
return first
}

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

// MaxHistoryDateRangeDays caps the inclusive start/end window the History
Expand Down
9 changes: 6 additions & 3 deletions internal/api/handler_history_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,8 @@ func TestHandler_getHistory_InProgressRowMapsRecFields(t *testing.T) {
assert.Equal(t, 1, row.Count)
assert.Equal(t, 0.0, row.UpfrontCost, "upfront must come from the rec")
assert.Equal(t, 1.2333, row.EstimatedSavings, "savings must come from the rec")
assert.Equal(t, 2.117, row.MonthlyCost, "monthly cost must come from the rec")
require.NotNil(t, row.MonthlyCost, "monthly cost must come from the rec")
assert.InDelta(t, 2.117, *row.MonthlyCost, 1e-9, "monthly cost must come from the rec")
assert.Equal(t, "no-upfront", row.Payment, "payment must come from the rec, not be left blank (#733)")
assert.NotEmpty(t, row.StatusDescription,
"in-progress rows must carry a human-readable status description, not render as a finished purchase")
Expand Down Expand Up @@ -1177,7 +1178,8 @@ func TestHandler_getHistory_ApprovalQueueColumnsPopulated(t *testing.T) {

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

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

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

t.Run("multi-rec heterogeneous Payment collapses to empty for honest dash fallback", func(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion internal/api/handler_inventory.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ func (h *Handler) getCoverageBreakdown(ctx context.Context, req *events.LambdaFu
if !isActiveCommitment(p, now) {
continue
}
coveredByKey[p.Provider+":"+p.Service] += p.MonthlyCost
if p.MonthlyCost != nil {
coveredByKey[p.Provider+":"+p.Service] += *p.MonthlyCost
}
}

// --- on-demand gap: recommendations -------------------------------------
Expand Down
11 changes: 6 additions & 5 deletions internal/api/handler_inventory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) {
Term: 1,
Payment: "no-upfront",
Timestamp: now.AddDate(0, -6, 0),
MonthlyCost: 100.0,
MonthlyCost: float64Ptr(100.0),
EstimatedSavings: 30.0,
},
// Expired: bought 2 years ago, 1-year term.
Expand All @@ -87,7 +87,7 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) {
Count: 1,
Term: 1,
Timestamp: now.AddDate(-2, 0, 0),
MonthlyCost: 50.0,
MonthlyCost: float64Ptr(50.0),
EstimatedSavings: 15.0,
},
}
Expand Down Expand Up @@ -117,7 +117,8 @@ func TestHandler_listActiveCommitments_FiltersExpired(t *testing.T) {
assert.Equal(t, 2, row.Count)
assert.Equal(t, 1, row.TermYears)
assert.Equal(t, "no-upfront", row.PaymentOption)
assert.Equal(t, 100.0, row.MonthlyCost)
require.NotNil(t, row.MonthlyCost)
assert.Equal(t, 100.0, *row.MonthlyCost)
assert.Equal(t, 30.0, row.EstimatedSavings)
assert.Equal(t, "active", row.Status)
assert.False(t, row.StartDate.IsZero())
Expand All @@ -141,7 +142,7 @@ func TestHandler_listActiveCommitments_AccountFilter(t *testing.T) {
Timestamp: now.AddDate(0, -3, 0),
Term: 1,
Count: 1,
MonthlyCost: 80.0,
MonthlyCost: float64Ptr(80.0),
EstimatedSavings: 20.0,
},
}
Expand Down Expand Up @@ -426,7 +427,7 @@ func TestHandler_getCoverageBreakdown_Integration(t *testing.T) {
Service: "ec2",
Timestamp: now.AddDate(-1, 0, 1), // active: 1y term started ~1y ago
Term: 1,
MonthlyCost: 150.0,
MonthlyCost: float64Ptr(150.0),
},
}
recs := []config.RecommendationRecord{
Expand Down
4 changes: 2 additions & 2 deletions internal/api/handler_per_account_perms_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -750,7 +750,7 @@ func TestPerAccountPerms_CoverageBreakdown_RecsFilteredByAllowedAccounts(t *test
Service: "ec2",
Timestamp: now.AddDate(0, -6, 0),
Term: 1,
MonthlyCost: 200.0,
MonthlyCost: float64Ptr(200.0),
}

// Recommendation for account A: on-demand gap the scoped user is allowed to see.
Expand Down Expand Up @@ -830,7 +830,7 @@ func TestPerAccountPerms_CoverageBreakdown_AdminSeesAll(t *testing.T) {
Service: "ec2",
Timestamp: now.AddDate(0, -6, 0),
Term: 1,
MonthlyCost: 200.0,
MonthlyCost: float64Ptr(200.0),
}
recA := config.RecommendationRecord{
Provider: "aws", Service: "ec2",
Expand Down
35 changes: 19 additions & 16 deletions internal/api/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,22 +496,25 @@ type ServiceSavings struct {
// The field stays in the response shape so a future "expiring soon"
// sub-state has a slot without a breaking API change.
type InventoryCommitment struct {
ID string `json:"id"`
Provider string `json:"provider"`
AccountID string `json:"account_id"`
AccountName string `json:"account_name,omitempty"`
Service string `json:"service"`
ResourceType string `json:"resource_type,omitempty"`
Region string `json:"region"`
Count int `json:"count"`
TermYears int `json:"term_years"`
PaymentOption string `json:"payment_option,omitempty"`
StartDate time.Time `json:"start_date"`
EndDate time.Time `json:"end_date"`
UpfrontCost float64 `json:"upfront_cost"`
MonthlyCost float64 `json:"monthly_cost"`
EstimatedSavings float64 `json:"estimated_savings"`
Status string `json:"status"`
ID string `json:"id"`
Provider string `json:"provider"`
AccountID string `json:"account_id"`
AccountName string `json:"account_name,omitempty"`
Service string `json:"service"`
ResourceType string `json:"resource_type,omitempty"`
Region string `json:"region"`
Count int `json:"count"`
TermYears int `json:"term_years"`
PaymentOption string `json:"payment_option,omitempty"`
StartDate time.Time `json:"start_date"`
EndDate time.Time `json:"end_date"`
UpfrontCost float64 `json:"upfront_cost"`
// MonthlyCost is nil when the source purchase_history row has a NULL
// monthly_cost (provider did not return a monthly breakdown). The
// frontend renders "—" for nil and "$X.XX" when non-nil.
MonthlyCost *float64 `json:"monthly_cost"`
EstimatedSavings float64 `json:"estimated_savings"`
Status string `json:"status"`
}

// InventoryCommitmentsResponse is the envelope returned by
Expand Down
10 changes: 9 additions & 1 deletion internal/config/store_postgres.go
Original file line number Diff line number Diff line change
Expand Up @@ -1444,6 +1444,7 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string,
for rows.Next() {
var record PurchaseHistoryRecord
var planID, planName, cloudAccountID sql.NullString
var monthlyCost sql.NullFloat64

err := rows.Scan(
&record.AccountID,
Expand All @@ -1457,7 +1458,7 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string,
&record.Term,
&record.Payment,
&record.UpfrontCost,
&record.MonthlyCost,
&monthlyCost,
&record.EstimatedSavings,
&planID,
&planName,
Expand All @@ -1468,6 +1469,13 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string,
return nil, fmt.Errorf("failed to scan purchase history: %w", err)
}

// Handle nullable monthly_cost: nil means "provider did not return a
// monthly breakdown"; 0.0 means "explicitly $0 recurring charge".
if monthlyCost.Valid {
v := monthlyCost.Float64
record.MonthlyCost = &v
}

// Handle nullable strings
if planID.Valid {
record.PlanID = planID.String
Expand Down
8 changes: 7 additions & 1 deletion internal/config/store_postgres_additional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ func (s *additionalMockStore) queryPurchaseHistory(ctx context.Context, query st
records := make([]PurchaseHistoryRecord, 0)
for rows.Next() {
var record PurchaseHistoryRecord
var monthlyCost sql.NullFloat64
var planID, planName sql.NullString

err := rows.Scan(
Expand All @@ -138,7 +139,7 @@ func (s *additionalMockStore) queryPurchaseHistory(ctx context.Context, query st
&record.Term,
&record.Payment,
&record.UpfrontCost,
&record.MonthlyCost,
&monthlyCost,
&record.EstimatedSavings,
&planID,
&planName,
Expand All @@ -148,6 +149,11 @@ func (s *additionalMockStore) queryPurchaseHistory(ctx context.Context, query st
return nil, err
}

if monthlyCost.Valid {
v := monthlyCost.Float64
record.MonthlyCost = &v
}

if planID.Valid {
record.PlanID = planID.String
}
Expand Down
13 changes: 10 additions & 3 deletions internal/config/store_postgres_comprehensive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ func (s *mockablePostgresStore) queryPurchaseHistory(ctx context.Context, query
for rows.Next() {
var record PurchaseHistoryRecord
var planID, planName sql.NullString
var monthlyCost sql.NullFloat64

err := rows.Scan(
&record.AccountID,
Expand All @@ -426,7 +427,7 @@ func (s *mockablePostgresStore) queryPurchaseHistory(ctx context.Context, query
&record.Term,
&record.Payment,
&record.UpfrontCost,
&record.MonthlyCost,
&monthlyCost,
&record.EstimatedSavings,
&planID,
&planName,
Expand All @@ -436,6 +437,11 @@ func (s *mockablePostgresStore) queryPurchaseHistory(ctx context.Context, query
return nil, err
}

if monthlyCost.Valid {
v := monthlyCost.Float64
record.MonthlyCost = &v
}

if planID.Valid {
record.PlanID = planID.String
}
Expand Down Expand Up @@ -1277,7 +1283,7 @@ func TestSavePurchaseHistory_Success(t *testing.T) {
Term: 3,
Payment: "all-upfront",
UpfrontCost: 2250.00,
MonthlyCost: 0,
MonthlyCost: pf(0),
EstimatedSavings: 450.00,
PlanID: "plan-123",
PlanName: "Production RDS Plan",
Expand Down Expand Up @@ -1973,7 +1979,8 @@ func TestQueryPurchaseHistory_AllFieldsPresent(t *testing.T) {
assert.Equal(t, 3, record.Term)
assert.Equal(t, "all-upfront", record.Payment)
assert.Equal(t, 3000.0, record.UpfrontCost)
assert.Equal(t, 0.0, record.MonthlyCost)
require.NotNil(t, record.MonthlyCost, "monthly_cost 0.0 from DB must scan as non-nil pointer")
assert.Equal(t, 0.0, *record.MonthlyCost)
assert.Equal(t, 600.0, record.EstimatedSavings)
assert.Equal(t, "search-plan", record.PlanID)
assert.Equal(t, "OpenSearch Production", record.PlanName)
Expand Down
2 changes: 1 addition & 1 deletion internal/config/store_postgres_coverage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ func TestPostgresStore_SavePurchaseHistory_NilDB(t *testing.T) {
Term: 3,
Payment: "all-upfront",
UpfrontCost: 2250.00,
MonthlyCost: 0,
MonthlyCost: pf(0),
EstimatedSavings: 450.00,
PlanID: "plan-123",
PlanName: "RDS Plan",
Expand Down
2 changes: 1 addition & 1 deletion internal/config/store_postgres_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ func TestPostgresStoreDB_PurchaseHistory(t *testing.T) {
Term: 3,
Payment: "all-upfront",
UpfrontCost: 2250.00,
MonthlyCost: 0,
MonthlyCost: pf(0),
EstimatedSavings: 450.00,
PlanID: "",
PlanName: "Test Plan",
Expand Down
2 changes: 1 addition & 1 deletion internal/config/store_postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func TestPostgresStore_PurchaseHistory(t *testing.T) {
Term: 3,
Payment: "all-upfront",
UpfrontCost: 2250.00,
MonthlyCost: 0,
MonthlyCost: pf(0),
EstimatedSavings: 450.00,
// PlanID intentionally left empty since it needs to be a valid UUID
PlanName: "Test Plan",
Expand Down
4 changes: 4 additions & 0 deletions internal/config/store_postgres_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import (
"github.com/stretchr/testify/assert"
)

// pf returns a pointer to the given float64 value. Used in test struct
// literals where a *float64 field must be initialised from a constant.
func pf(v float64) *float64 { return &v }

// TestTimeFromTTL tests the timeFromTTL helper function
func TestTimeFromTTL(t *testing.T) {
tests := []struct {
Expand Down
Loading
Loading