Repository navigation
fix(api/dashboard): tighten committed_monthly aggregation (status + unit + account scope) - #927
Conversation
…nit + account scope) Three bugs inflated the committed_monthly KPI on the Home page: 1. Status filter: isActiveCommitment now rejects rows whose Status field is non-empty and not "completed". DB-backed purchase_history rows always read back with Status="" (the column is not persisted); only synthesised rows from failed/cancelled/expired executions carry a non-empty status. Those were being counted as active commitments. 2. Unit verification: documented that EstimatedSavings is always written in monthly units (traces from PurchaseExecution.EstimatedSavings which derives from monthly recommendation savings). No runtime normalisation needed; comment added to prevent future ambiguity. 3. Multi-account scope: resolveDashboardAccountID was passing empty string to GetPurchaseHistory when account_ids had multiple entries, yielding WHERE account_id='' which returns no rows. Replaced with resolveDashboardAccountScope that routes to GetPurchaseHistoryFiltered (cloud_account_id = ANY($1)) for UUID-based filters, GetPurchaseHistory for the legacy account_id param, and GetAllPurchaseHistory when no filter is set. Also fixes the single-UUID case which was passing a UUID to the account_id (VARCHAR) column. New tests: status=failed row excluded; multi-account UUID filter routes to GetPurchaseHistoryFiltered and excludes out-of-scope accounts; no-filter path uses GetAllPurchaseHistory.
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 30 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
Three bugs were inflating the
committed_monthlyKPI on the Home page:Status filter:
isActiveCommitmentnow rejects rows whoseStatusfield is non-empty and not"completed". DB-backedpurchase_historyrows always read back withStatus=""(the column is not persisted —dynamodbav:"-"). Synthesised rows from failed/cancelled/expired executions carry a non-empty status and were being counted as active commitments.Unit verification: Confirmed and documented that
EstimatedSavingsis always written in monthly units (populated fromPurchaseExecution.EstimatedSavingswhich derives from monthly recommendation savings at purchase time). No runtime normalisation is needed; comment added to prevent future drift.Multi-account scope:
resolveDashboardAccountIDwas passing""toGetPurchaseHistorywhenaccount_idshad multiple entries, yieldingWHERE account_id = ''which returns no rows at all (silently zero, not all-accounts). ReplacedresolveDashboardAccountIDwithresolveDashboardAccountScopethat:GetPurchaseHistoryFiltered(cloudAccountUUIDs)(matchescloud_account_id = ANY($1)) for UUID-basedaccount_idsfiltersGetPurchaseHistory(legacyAccountID)for the legacy singularaccount_idparamGetAllPurchaseHistorywhen no filter is setaccount_idcolumnNew test assertions
status=failedrow is excluded fromcommittedMonthlyandactiveCommitmentsGetPurchaseHistoryFiltered(notGetPurchaseHistory), and the mock assertsGetPurchaseHistoryis never calledGetAllPurchaseHistory(updated 5 existing tests to match the new dispatch)Test plan
go test github.com/LeanerCloud/CUDly/internal/api/... -run TestHandler_calculateCommitmentMetricspasses (8 tests including 3 new)go test github.com/LeanerCloud/CUDly/internal/api/...passes (all 1387 tests)go test ./...passes (5016 tests)