fix(api/history): honor provider/account_ids/start/end query params in Purchase History handler - #716
Conversation
…n Purchase History handler The /api/history handler accepted provider, account_ids, start, and end query params from the frontend (Purchase History date filter row 3.4 and the Purchases-page Global filter rows 4.2/4.4/4.5) but silently dropped them: fetchPurchaseHistory only forwarded account_id and limit to the store, and fetchExecutionsAsHistory ignored every filter. Visible filter affordances were no-ops. Parse the four query params in a shared historyFilters struct so both halves of the merged /api/history response (the SQL purchase_history rows and the synthesised execution rows) apply the same scope: - provider validated via the existing whitelist (aws/azure/gcp/"all"/"") - account_ids parsed via parseAccountIDs (UUIDs, MaxAccountIDsPerRequest) - start/end as YYYY-MM-DD with a 366-day cap mirroring PR #529 (issue #414) to prevent a full-table-scan DoS New store method GetPurchaseHistoryFiltered pushes the filter set into a WHERE clause built like buildRecommendationFilter; each predicate is applied only when its filter is non-empty. The legacy GetPurchaseHistory / GetAllPurchaseHistory methods are preserved for the no-filter fast path (dashboard, inventory, analytics) and for the legacy singular account_id param, which targets a different (VARCHAR(20)) column than the cloud_account_id UUIDs the new filter consumes and must not be coerced into the new filter's clause. Tests cover each filter independently and combined, the legacy account_id fast path, malformed/inverted/oversized dates returning 400, the 366-day boundary (366 accepted, 367 rejected), and the pgxmock SQL shape for the new store method including the no-filter / partial-filter / limit-clamp variants. Closes #701
cristim
added
triaged
|
Warning Review limit reached
More reviews will be available in 25 minutes and 46 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 (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…mers (refs #701) (#741) * fix(purchases): wire topbar filter chips to all three Purchases consumers (#701) Three sub-flows of the global filter on the Purchases page still failed after PR #716: 1. Provider filter (4.2): selecting a provider did nothing to Purchase History or Approval Queue because setupHistoryHandlers() was exported from history.ts but never called in app.ts. 2. Account filter (4.4): the Savings History chart disappeared when an account UUID was selected because the analytics SQL only matched the legacy account_id VARCHAR(20) column, not cloud_account_id UUID FK. The topbar chip always sends cloud_accounts.id (a UUID), so the WHERE clause returned 0 rows and the chart fell into the empty-state branch. 3. Empty-state UX (4.4/4.5): showEmptyState() always showed "No savings history data available yet." regardless of whether a filter caused the empty result, giving no signal to the user. Changes: - frontend/src/app.ts: import and call setupHistoryHandlers() after initSavingsHistory() so provider/account chip changes reload Purchase History and Approval Queue. - internal/api/analytics_postgres.go: add OR cloud_account_id::text = $3 to the WHERE clause in QueryHistory and QueryBreakdown so UUID-based account filtering includes rows written by current purchase flows. - frontend/src/modules/savings-history.ts: add buildFilterDesc() helper and update showEmptyState() to show "No savings data for the selected filter (AWS)" when a filter is active vs the original message when not. - Tests: new Go tests for UUID account filter path; new TS tests for setupHistoryHandlers subscriptions and filter-aware empty-state copy; update history mocks in index/sidebar-anchors/purchase-execution-toast tests to include setupHistoryHandlers. * fix(savings-history): address CR findings on PR #741 Three CodeRabbit findings addressed: 1. (Major, line 67) Use explicit error state on fetch failure: add showErrorState() that sets distinct heading/help copy ("Failed to load savings history. / Please retry.") instead of the no-data empty state, so users can tell a filter-scope empty from a real outage. Update the catch path to call showErrorState with the extracted error message. 2. (Minor, line 125) Treat the 'all' provider sentinel as unfiltered in buildFilterDesc: skip pushing provider.toUpperCase() when the value is 'all' (case-insensitive), so the empty-state heading reads "No savings history data available yet." rather than "No savings data for the selected filter (ALL)." when no real provider chip is selected. 3. (Minor, line 184) Tighten SQL expectations in TestQueryHistory_CloudAccountIDFilter and TestQueryBreakdown_CloudAccountIDFilter: prefix regexes with (?s) and extend to include the dual-column account filter WHERE fragment, so removing that clause would now cause the mock expectations to fail instead of passing silently. Tests: all 66 savings-history frontend tests pass; Go analytics tests pass (including the two tightened CloudAccountID tests).
1 participant
Closes #701. Backend
fetchPurchaseHistorywas silently ignoringprovider,account_ids,start, andendquery params; visible UI filters were no-ops.Changes
historyFiltersstruct ininternal/api/handler_history.go:Provider(validated viavalidateProvider, accepts empty + aws/azure/gcp)LegacyAccountID(VARCHAR(20) —purchase_history.account_idlegacy column, honored only on the no-other-filters fast path)AccountIDs []string(UUIDs —purchase_history.cloud_account_id— parsed via existingparseAccountIDs, UUID-checked + capped at 200)HasDate / Start / End(YYYY-MM-DDvalidation, inclusive end-of-day, capped at 366 days to mirror sec(api): analytics date range has no upper bound — full-table scan DoS via start=1970-01-01&end=2100-12-31 #414/PR sec(api): cap analytics date range at 366 days to prevent DoS (closes #414) #529's DoS bound)Limit(existing)store_postgresquery branches that appendAND provider = $? AND cloud_account_id = ANY($?) AND scheduled_date BETWEEN $? AND $?only when the corresponding filter is set.Tests (12 new + 4 pgxmock)
TestHandler_getHistory_FilterParams× 6: provider alone, provider=all, account_ids alone, legacy account_id ignored when new filters present, legacy fast-path, start/end alone, combined.TestHandler_getHistory_FilterValidation× 6: invalid provider, non-UUID account, malformed start/end, inverted range, >366 days.TestHandler_getHistory_DateRangeBoundary× 2: 366 accepted, 367 rejected.TestParseHistoryDateRange× 4: empty/end-only/start-only/inclusive end.store_postgres_pgxmock_test.go× 4: all filters, no filters, partial, limit clamp.Scope
Closes #701 only. Issue #503 (Savings History chart re-query) requires a separate frontend wiring fix in
frontend/src/modules/savings-history.ts— different layer; bundling would expand scope.Verification
go testacross api/config/mocks/scheduler/server/purchase/analytics: 2530 pass