fix(api): filter purchases by account UUID AND external id (closes #701, #498, #866)

[cristim]

What

The global Provider/Account filter returned no data for many accounts (QA rows 305/306 #701, 384/431 #498, 452 #866). This is the real fix for the root cause that prior patches missed.

Closes #701, closes #498, closes #866.

Root cause

purchase_history rows carry two account identifiers, and either one may be the only one populated on a given row:

• account_id — the cloud-provider external number (e.g. an AWS account number). Always populated.
• cloud_account_id — the internal cloud_accounts UUID FK. Added in migration 000011 with no backfill, so it is NULL on every direct-execute / ambient / pre-000011 row.

The top-bar Account chip emits the UUID (parseAccountIDs hard-validates UUID).

• Prior fixes filtered on cloud_account_id (the sparse UUID column), so WHERE matched nothing for NULL-cloud_account_id rows → "no data" / "No purchase history yet" (bug(api/history): handler ignores provider/account_ids/start/end query params from Purchase History + Purchases-page Global filters #701, ux(home): Account filter doesn't affect Home page Savings-over-time graph (data identical across accounts) #498).
• Row 452 (Main Header global filter not propagated to Inventory & Coverage subpages (QA 2.7+2.14, 2.8+2.15) #866) was the mirror: a UUID sent to a query filtering the external account_id column also matched nothing.

The analytics SQL even had a dual-column shape already, but it was bound with a single value ($3 = '' OR account_id = $3 OR cloud_account_id::text = $3), so the account_id = $3 branch was dead when $3 was a UUID.

Why prior fixes failed

They committed to one column. Whichever column they picked, the other representation's rows disappeared. There is no single column that is reliably populated for the chip's UUID across all historical rows.

The fix: resolve-at-query, dual-column match

1. A shared resolver (Handler.resolveAccountFilterIDs / resolveSingleAccountFilterIDs in scoping.go) loads cloud_accounts once (reusing the same ListCloudAccounts used by resolveAccountNamesByID) and maps each requested UUID to its (provider, external_id) pair. External ids are taken only from the user's known accounts (and cloud_accounts has UNIQUE(provider, external_id)), so a reused external number across providers can't leak.

2. GetPurchaseHistoryFiltered now takes a PurchaseHistoryFilter struct (UUIDs + external ids) and matches both columns:

   (cloud_account_id = ANY($uuids) OR account_id = ANY($externals))

3. Every account-filtered read path routes through this: /api/history (incl. the in-memory matchesExecution mirror for pending executions), /history/analytics + /history/breakdown, /dashboard/summary, and /api/inventory/commitments.

The UUID wire contract is unchanged (frontend untouched). A one-time cloud_account_id backfill on legacy rows is intentionally deferred to a follow-up — this PR is the resolve-at-query fix so the filter works today regardless of backfill state.

Keystone regression test

TestPGXMock_GetPurchaseHistoryFiltered_ExternalIDOnly (store level): a row with cloud_account_id IS NULL + a real account_id is returned when filtering by that account's UUID. Plus end-to-end keystones for /api/history, dashboard commitment metrics, analytics, and matchesExecution — each fails before the fix (single-column drops the row) and passes after.

Index

Both columns are already indexed (idx_purchase_history_account_timestamp on account_id from 000002; idx_purchase_history_cloud_account on cloud_account_id from 000011), so no new migration is required.

Verification

• go build ./... ✓
• gofmt -l, go vet ./... clean
• go test ./internal/api/... ./internal/config/... ./internal/scheduler/... ./internal/server/... ./internal/purchase/... ./internal/analytics/... green (2788 tests), incl. -race on the api package.

Closes the sheet rows 305, 306, 384, 431, 452.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved handling of accounts identified only by external IDs (without cloud UUIDs) across analytics, history, and inventory endpoints.

• Refactoring

  • Enhanced account filtering to support dual-column matching across analytics and dashboard features, improving query scope accuracy and account resolution reliability.

• Tests

  • Updated test coverage for account filtering logic and handler routing.

[coderabbitai]

Warning

Review limit reached

@cristim, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 15 minutes and 4 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0ad630a3-eabf-45b3-a1dd-cb96eb7a0d8f

📥 Commits

Reviewing files that changed from the base of the PR and between 1613f95 and 5f0842f.

📒 Files selected for processing (16)

• internal/api/analytics_postgres.go
• internal/api/analytics_postgres_test.go
• internal/api/handler_analytics.go
• internal/api/handler_analytics_test.go
• internal/api/handler_dashboard.go
• internal/api/handler_dashboard_test.go
• internal/api/handler_history.go
• internal/api/handler_history_test.go
• internal/api/handler_inventory.go
• internal/api/handler_per_account_perms_test.go
• internal/api/scoping.go
• internal/api/types.go
• internal/config/interfaces.go
• internal/config/store_postgres.go
• internal/config/store_postgres_pgxmock_test.go
• internal/config/types.go

📝 Walkthrough

Walkthrough

This PR refactors account filtering across purchase history, analytics, and dashboard endpoints to support dual-column matching: both cloud account UUIDs (cloud_account_id) and legacy external account numbers (account_id). A new PurchaseHistoryFilter struct consolidates filter parameters, and dual-column SQL predicates are applied consistently in Postgres queries, in-memory execution filtering, and handler resolution logic.

Changes

Dual-Column Account Filtering

Layer / File(s)	Summary
PurchaseHistoryFilter contract and Postgres store implementation internal/config/types.go, internal/config/interfaces.go, internal/config/store_postgres.go, internal/config/store_postgres_pgxmock_test.go	New PurchaseHistoryFilter struct consolidates provider, dual account IDs, date range, and limit; StoreInterface.GetPurchaseHistoryFiltered signature updated to accept struct; Postgres store adds appendAccountPredicate helper to build (cloud_account_id = ANY(...) OR account_id = ANY(...)) predicates; pgxmock tests verify dual-column SQL shapes and edge cases (UUID-only, external-ID-only).
Analytics Postgres client dual-account queries internal/api/analytics_postgres.go, internal/api/analytics_postgres_test.go, internal/api/types.go	QueryHistory and QueryBreakdown methods now accept separate accountUUIDs and accountExternalIDs slices instead of single accountID; new accountFilterClause helper builds parameter-bound OR condition; test coverage includes dual-column predicate validation and argument binding.
Account filter resolution helpers internal/api/scoping.go	Added resolveAccountFilterIDs to map UUID cloud accounts to dual identifier sets; added resolveSingleAccountFilterIDs to interpret legacy account ID as either known UUID or external number, with deduplication and fallback logic.
History handler dual-column filtering internal/api/handler_history.go, internal/api/handler_history_test.go	New resolveHistoryAccountFilter populates dual-id fields on historyFilters; new accountMatchesFilters matches execution against both UUID and external IDs; fetchPurchaseHistory calls GetPurchaseHistoryFiltered with struct-based filter; tests extended with external-ID-only account coverage.
Dashboard handler account scope and metrics internal/api/handler_dashboard.go, internal/api/handler_dashboard_test.go	New resolveDashboardAccountScope method resolves account_ids/account_id params to dual identifier slices; calculateCommitmentMetrics calls GetPurchaseHistoryFiltered with both UUID and external ID filters when either is non-empty; tests refactored to verify dual-column filtering.
Analytics and inventory handlers using dual filters internal/api/handler_analytics.go, internal/api/handler_analytics_test.go, internal/api/handler_inventory.go, internal/api/handler_inventory_test.go, internal/api/handler_coverage_test.go, internal/api/handler_per_account_perms_test.go	Analytics endpoints (getHistoryAnalytics, getHistoryBreakdown) resolve account_id to dual filters before calling analytics client; inventory fetchCommitmentRecords calls GetPurchaseHistoryFiltered with struct-based filter; all handler tests updated to expect dual-account parameters and struct-based mock calls.
Test mocks and fixtures internal/analytics/collector_test.go, internal/api/mocks_test.go, internal/mocks/stores.go, internal/purchase/mocks_test.go, internal/scheduler/scheduler_test.go, internal/server/test_helpers_test.go, .gitallowed	Mock implementations (MockConfigStore.GetPurchaseHistoryFiltered) updated to accept PurchaseHistoryFilter struct; allowlist updated with fixture value (999988887777) for issue #956 test coverage.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• LeanerCloud/CUDly#309: Per-account permission handler tests adjust mocked QueryHistory/QueryBreakdown calls to match dual account-filtering API shape.
• LeanerCloud/CUDly#881: Both PRs modify the Inventory backend to route account_id through dual-column filtered purchase-history path via GetPurchaseHistoryFiltered.
• LeanerCloud/CUDly#738: Both PRs modify handler_history.go's matchesExecution account-id filtering logic when exec.CloudAccountID is nil and recommendation-level account info is used.

Suggested labels

severity/high, urgency/this-sprint, effort/l, bug

Poem

🐰 Dual paths through the cloud now align,
UUID and account_id, in tandem combine—
No orphaned rows left in SQL's cold store,
Both key forms match; the history restored! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 65.22% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: implementing dual-column account filtering by both UUID and external ID across the purchase history query paths, with specific issue references.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/account-filter-id-representation

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 35 minutes and 5 seconds before sending another message.

[cristim]

Rebased onto latest origin/feat/multicloud-web-frontend (base had advanced past the merges of #953 and others). Pushed b1d18f3e6.

Conflict resolution:

• internal/api/scoping.go (resolveAccountNamesByID): purely cosmetic comment overlap with merged fix(api/inventory): show account name in Commitments view #953, which added the same nameByID[ExternalID] = a.Name dual-keying. Function bodies were identical on both sides; kept the richer comment wording. Additive, no behavior change.
• internal/api/handler_inventory_test.go (TestHandler_listActiveCommitments_AccountFilter): the auto-merge silently combined fix(api/inventory): show account name in Commitments view #953's mock expectation (AccountIDs: ["acc-1"]) with a stray ListCloudAccountsFn fixture from base that set ExternalID: "acc-1", which made the dual-column resolver classify "acc-1" as an external id. Restored this branch's intended fixture ({ID: "acc-1", Name: "Account One"}) so "acc-1" resolves as a known UUID matched on cloud_account_id only, consistent with the test's own doc comment.

Verification (local): gofmt -l clean, go vet ./... clean, go build ./... success, go test ./internal/api/... ./internal/config/... -count=1 green (1990 passed), including the keystone TestPGXMock_GetPurchaseHistoryFiltered_ExternalIDOnly.

Not re-pinging CodeRabbit this push: the latest CR comment is a rate-limit notice; the CR-watch sweep will re-trigger once the limit clears.

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/api/analytics_postgres_test.go`:
- Around line 187-188: The test hard-codes a casted predicate
`cloud_account_id::text = ANY(...)` which prevents use of existing UUID indexes;
update the SQL in internal/api/analytics_postgres.go to compare UUID values
directly (use `cloud_account_id = ANY($n::uuid[])` or `cloud_account_id =
ANY($n)` with the binder providing uuid[]), adjust the code that builds the
filtered analytics WHERE clause accordingly, and then change the pgxmock
expectations in internal/api/analytics_postgres_test.go (the mock.ExpectQuery
regex and WithArgs calls around the lines matching `SELECT date_trunc('day',
timestamp)` ) to expect the new predicate (e.g. `cloud_account_id =
ANY\(\$3::uuid\[\]\) OR account_id = ANY\(\$4\)` ) and bind UUID arrays instead
of string arrays so the test verifies UUID-typed comparisons.

In `@internal/api/handler_dashboard.go`:
- Around line 25-26: The dashboard commitment metrics are not being restricted
to the session's allowed_accounts: before calling calculateCommitmentMetrics (or
before it calls GetAllPurchaseHistory), intersect the account lists returned by
resolveDashboardAccountScope (accountUUIDs, accountExternalIDs) with the
session's allowed_accounts so out-of-scope accounts are removed; alternatively,
after calculateCommitmentMetrics returns, filter its purchase-history rows and
the resulting values used for ActiveCommitments, CommittedMonthly,
CurrentCoverage, and YTDSavings using the same allowed-account gate used for
recommendations to ensure no inaccessible accounts are included. Target
resolveDashboardAccountScope, calculateCommitmentMetrics, GetAllPurchaseHistory
and the variables ActiveCommitments/CommittedMonthly/CurrentCoverage/YTDSavings
when applying the intersection/filter.

In `@internal/config/types.go`:
- Around line 482-488: The ExternalIDs []string field loses the (provider,
external_id) pairing and must be replaced with a provider-coupled shape so SQL
can distinguish duplicates across providers; change the type definition
(ExternalIDs) to carry provider context (e.g., map[string][]string or a slice of
a small struct like ResolvedAccount{Provider string; ExternalID string}) and
update any code that populates or consumes ExternalIDs (the callers that resolve
AccountIDs and the SQL builder that currently uses ExternalIDs to produce
account_id = ANY(...)) so they populate and use the provider-aware structure
instead of the bare []string.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9ffb56af-4aa2-4244-a1dc-3094a22149b3

📥 Commits

Reviewing files that changed from the base of the PR and between cf521ca and 1613f95.

📒 Files selected for processing (25)

• .gitallowed
• internal/analytics/collector_test.go
• internal/api/analytics_postgres.go
• internal/api/analytics_postgres_test.go
• internal/api/handler_analytics.go
• internal/api/handler_analytics_test.go
• internal/api/handler_coverage_test.go
• internal/api/handler_dashboard.go
• internal/api/handler_dashboard_test.go
• internal/api/handler_history.go
• internal/api/handler_history_test.go
• internal/api/handler_inventory.go
• internal/api/handler_inventory_test.go
• internal/api/handler_per_account_perms_test.go
• internal/api/mocks_test.go
• internal/api/scoping.go
• internal/api/types.go
• internal/config/interfaces.go
• internal/config/store_postgres.go
• internal/config/store_postgres_pgxmock_test.go
• internal/config/types.go
• internal/mocks/stores.go
• internal/purchase/mocks_test.go
• internal/scheduler/scheduler_test.go
• internal/server/test_helpers_test.go

[cristim]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[cristim]

Addressed all 3 CodeRabbit findings (pushed 5f0842ff8). Per-finding disposition:

1. internal/config/types.go — Keep provider attached to resolved external IDs (Major): FIXED.
Replaced the bare ExternalIDs []string with ExternalIDsByProvider map[string][]string so the (provider, external_id) pairing survives end-to-end. The predicate is now built per provider:
(cloud_account_id = ANY($u) OR (provider = $p AND account_id = ANY($e)) OR ...).
Threaded through PurchaseHistoryFilter, appendAccountPredicate (store), the analytics accountFilterClause / QueryHistory / QueryBreakdown, the resolvers (resolveAccountFilterIDs / resolveSingleAccountFilterIDs, which have the provider from cloud_accounts), the in-memory accountMatchesFilters (now provider-scoped via the execution's provider), the historyFilters, the inventory handler, and the dashboard. A raw legacy external number with unknown provider is grouped under the "" key, which matches account_id with no provider gate (legacy behaviour preserved). Regression tests prove an external id under provider A does not match provider B rows: TestPGXMock_GetPurchaseHistoryFiltered_MultiProviderExternalIDs (store) and TestQueryHistory_CrossProviderScoped (analytics), plus the updated dual-column tests.

2. internal/api/handler_dashboard.go — Scope dashboard commitment metrics by allowed_accounts (Major): FIXED.
Added resolveAllowedAccountScope: when the session is account-restricted and no explicit account filter is supplied, the commitment KPIs (ActiveCommitments, CommittedMonthly, CurrentCoverage, YTDSavings) are scoped to the session's allowed_accounts instead of falling through to GetAllPurchaseHistory. A zero-account scope short-circuits to zeroed KPIs (a nil filter would match all rows in the store). Unrestricted/admin sessions keep the all-accounts behaviour. Regression test: TestPerAccountPerms_DashboardSummary_CommitmentMetricsExcludeOtherAccounts (account B's commitment never reaches a scoped session's KPIs); the existing ..._AggregatesAllowedSubsetOnly test now also asserts GetAllPurchaseHistory is not called. Extracted fetchCommitmentPurchases to keep calculateCommitmentMetrics under the gocyclo limit.

3. internal/api/analytics_postgres_test.go — index-friendly predicate (Quick win): FIXED.
Dropped the cloud_account_id::text cast in the analytics predicate so cloud_account_id = ANY($n) compares UUIDs directly and the plain UUID index from migration 000011 can back it. Updated the pgxmock expectations to the new provider-gated SQL shape and arg binding.

Verification (local): gofmt -l clean, go vet ./... clean, go build ./... succeeds, go test ./internal/api/... ./internal/config/... -count=1 = 1994 passed (incl. the keystone TestPGXMock_GetPurchaseHistoryFiltered_ExternalIDOnly and the new cross-provider + scoped-dashboard regressions), and SKIP=terraform_validate pre-commit run --all-files all hooks pass (gocyclo green).

@coderabbitai review

[coderabbitai]

@cristim Thanks for the thorough write-up! All three findings look well-addressed:

1. Provider-scoped external IDs — ExternalIDsByProvider map[string][]string is the right shape; the per-provider predicate (provider = $p AND account_id = ANY($e)) prevents cross-provider false matches, and the "" key preserves legacy behaviour. The new cross-provider regression tests (TestPGXMock_GetPurchaseHistoryFiltered_MultiProviderExternalIDs, TestQueryHistory_CrossProviderScoped) are exactly the kind of keystone coverage needed here.

2. allowed_accounts scoping on dashboard KPIs — resolveAllowedAccountScope + the zero-account short-circuit + the new TestPerAccountPerms_DashboardSummary_CommitmentMetricsExcludeOtherAccounts regression closes the data-leakage gap cleanly. Extracting fetchCommitmentPurchases to stay under gocyclo limit is a nice bonus.

3. Index-friendly UUID predicate — dropping the ::text cast so the migration-000011 UUID index can be used is the right call, and the updated pgxmock expectations confirm the SQL shape is clean.

Kicking off the incremental review now against 5f0842ff8.

(ﾐ^ OwO^ﾐ)

---

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.