Skip to content

fix(analytics): Savings chart shows only the first account when several are selected (account_ids ignored) #1429

Closed

Description

@IAmJSD

Moved out of #25. #25's provider part was fixed by LeanerCloud/cloud-commitments-cli#1122, and #25 is being closed. This is the account part, from audit finding A11-014 (2026-09-02 audit, docs/audits/codebase-audit-2026-09-02.md).

Audit finding A11-014 (medium), as recorded on #25

getSavingsAnalytics (frontend/src/api/history.ts:43-45) sends both account_ids=a,b,c and account_id=a, but the analytics handlers read only the singular param (accountID := params["account_id"] at internal/api/handler_analytics.go:40, :131, :197, then resolveSingleAccountFilterIDs) and account_ids is parsed nowhere for these routes. So with three accounts selected the chart silently renders only account A's savings while the chip row advertises three, an under-reported figure with no indication that two were dropped. The sibling getHistory (history.ts:22) sends only the plural form, which internal/api/handler_history.go:678 does honour, so the two panels on the same page disagree for the same filter. Either plumb account_ids through the analytics path alongside provider, or have the caller disable/annotate the chart when more than one account is selected.

Still true on main (6b586cc), from reading the code

  • frontend/src/api/history.ts:33-48: getSavingsAnalytics sets account_ids to the full list and account_id to the first id only. The code comment says the plural form is "for forward-compatibility".
  • internal/api/handler_analytics.go: getAnalyticsTrends (:47), getHistoryAnalytics (:155) and getHistoryBreakdown (:199) all pass only params["account_id"] to resolveAnalyticsAccountFilter. Nothing reads account_ids.
  • Callers: the dashboard (frontend/src/dashboard.ts:1177, :1348) and the Savings History module (frontend/src/modules/savings-history.ts:120).
  • GET /history does parse account_ids (internal/api/handler_history.go:748, parseAccountIDs), so the purchase list and the savings chart disagree when several accounts are selected.

Impact

With more than one account selected, the savings chart and totals show only the first account's savings, with no warning. Users read an under-reported savings figure.

Suggested fix

Parse account_ids in the analytics handlers with the existing parseAccountIDs (UUID-validated, capped at 200), and check scope for every id, the same way /history does. Pass the union of the dual-column filters (UUIDs plus external ids per provider) to QueryHistory / QueryBreakdown. Keep the singular account_id working for older clients. If both are sent, the plural wins. The frontend can then stop sending the singular form.

Acceptance criteria

  • With two or more accounts selected, /history/analytics (and the trends and breakdown endpoints) return savings for all of them.
  • A scoped user who asks for an account outside their scope gets the same answer as for a single out-of-scope account_id today.
  • Handler and store tests cover multiple accounts, including rows that carry only the external account id.
  • A frontend test shows that the chart request carries every selected account.

Refs #25, A11-014.

Filed by IAmJSD's agent at IAmJSD's request (acting owner while the maintainers are away).

Activity

  1. added a commit that references this issue on Oct 26, 2026
    01fb35a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions