Repository navigation
fix(api): filter upcoming purchases by the provider and account chips - #1555
Conversation
GET /api/dashboard/upcoming took no filter parameters and was scoped by the session only, so the Home widget could not follow the topbar chips the rest of the page uses. The frontend applies the provider chip client-side and shows a "not scoped" note for the account chip. The endpoint now accepts provider, account_ids and the legacy account_id with the same validation and precedence as /api/dashboard/summary (providerFilterParam, resolveDashboardAccountScope). The account selection is intersected with the session's allowed_accounts, so an out-of-scope account matches no rows instead of widening the result. A row matches the account filter when any account its execution can buy in is selected: a plan-level parent covers every plan account, a fan-out child only its own. The session scope check itself is unchanged, and no parameters means the previous result. Refs LeanerCloud#713 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the git-secrets account-ID detector Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
govulncheck in the Docker image build now fails on these two advisories, both fixed in x/net v0.62.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE Blocking:
Non-blocking:
Attacked:
Body-stale:
Reviewer: rG1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths. |
|
Local verification at |
|
Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on |
Root cause
GET /api/dashboard/upcoming(getUpcomingPurchases,internal/api/handler_dashboard.go) took no query parameters. It returned every pending execution the session'sallowed_accountscould see, so the Home "Upcoming Scheduled Purchases" widget couldn't follow the topbar provider and account chips that the rest of Home sends to/api/dashboard/summary. #1186 already fixed the error state. #1365 then applied the provider chip in the browser and added a "the account filter does not apply here" note, because the rows carry no account. As cristim's comment on the issue says, the open part is backend filter parameters plus the frontend sending the chip state. This PR is the backend part.Fix
getUpcomingPurchasesnow readsprovider,account_idsand the legacyaccount_id. Validation and precedence matchgetDashboardSummary:providerFilterParam:""orallmeans every provider; anything else is a 400.parseAccountIDs: a malformed list or more than 200 ids is a 400.account_idswins overaccount_id.resolveDashboardAccountScope, the same resolver the summary uses. That resolver intersects it with the session'sallowed_accounts, so the out-of-scope behaviour matches the other dashboard routes: an account outside the caller's scope is dropped, and a selection with no in-scope account matches no rows (200, empty list). It never widens the result. The existing per-row session check (isExecutionInScope) still runs first and is unchanged, so the filter can only remove rows.internal/api/handler_dashboard_upcoming_filter.go):provider, i.e. its plan's first service. That's the value the widget shows and the value fix(frontend): scope Home upcoming purchases to the provider chip #1365 filters on today.executionAccounts. A plan-level parent fans out to every plan account, so it matches when any of them is selected. A fan-out child (cloud_account_idset) matches only its own account.account_idcan carry a raw external number.provider=alland an emptyaccount_ids, the response is identical to main.upcomingInScope, followingplannedPurchasesInScope, to keepgetUpcomingPurchasesunder the gocyclo limit.internal/api/openapi.yamldocuments the three parameters and the new 400 response ongetUpcomingPurchases.Refs #713
Concurrency
No DB writes, no migration and no new store method. The handler only reads (pending executions, plans, plan accounts, cloud accounts), the same set the summary and planned-purchases reads already use. There's no new race to test.
Verification
Unit/mock evidence: real handler code against
MockConfigStorefixtures (four plans, five pending executions across Production, Staging and a GCP account, including a multi-account plan-level parent and its Staging fan-out child). One test also goes throughHandleRequestwith a query string, to cover the router wiring.New tests in
internal/api/handler_dashboard_upcoming_filter_test.go:TestGetUpcomingPurchases_NoFilterUnchanged: no parameters, andprovider=allwith an emptyaccount_ids, both return every row. This test passes before and after, by design.TestGetUpcomingPurchases_ProviderFilter: gcp, aws and azure (empty).TestGetUpcomingPurchases_InvalidFilterIs400: unknown provider; non-UUIDaccount_ids.TestGetUpcomingPurchases_AccountFilter:account_idholding an external number;account_idswinning overaccount_id;TestGetUpcomingPurchases_ProviderAndAccountCombine.TestGetUpcomingPurchases_OutOfScopeAccount, for a session restricted to Production and GCP:account_ids=<Staging>gives an empty 200;TestHandleRequest_DashboardUpcomingAppliesQueryFilters: through the router.Fail-before, on main 7b86e38 with only the test file added:
Pass-after, on this branch: all of them pass, along with the existing
TestHandler_getUpcomingPurchases*andTestHandleRequest_DashboardUpcomingFlagsStepWithoutRecommendations.Mutation probes, run locally and reverted:
matchesAnyfailsTestGetUpcomingPurchases_OutOfScopeAccount."") bucket match failsTestGetUpcomingPurchases_AccountFilter.Other commands run on this branch, all clean (run with
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true):go build ./...go vet ./...go vet -tags integration ./...go test ./internal/api/ -count=1: ok (48s)go test ./internal/mocks/ -count=1: okgolangci-lint run --allow-parallel-runners ./internal/api/(v2.10.1): 0 issuesgocyclo -over 10 -ignore "_test\.go" internal/api/handler_dashboard.go internal/api/handler_dashboard_upcoming_filter.go: no outputCombined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree:
queue/platform@400d256b= origin/main7b86e386plus these heads, merged in this order:Linux, Go 1.26.9,
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers. The full suite ran onf7c48020, which differs from this tree only by test-only commits on fix/issue-30, fix/issue-713 and fix/issue-263 that replace placeholder account-number literals with named constants:go build ./...,go vet ./...,go vet -tags integration ./...and the fullgo test -race -short -count=1 ./....--build-tags integration.gocyclo -over 10is silent on the changed non-test files.-tags integration -count=1):internal/analytics,internal/api,internal/auth,internal/config,internal/database/postgres/migrations,internal/purchase,internal/schedulerandinternal/serverall pass. Migrations 000149, 000150 and 000151 apply in order on this tree.approval-details,settings-accounts,api) pass (221 tests).accountkey); fixed with named constants. Re-run on this tree: git-secrets, using the repo'sscripts/setup-git-secrets.shpatterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged.scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL..tftest.hclfiles were not run locally (CI runs them). Nothing was applied to any cloud.Out of scope
Frontend follow-up for the UI owner
Once this lands, the widget can drop its client-side workaround from #1365:
frontend/src/api/dashboard.ts: givegetUpcomingPurchasesthe same signature and query building asgetDashboardSummary. That meansproviderplusaccount_ids(comma-joined), e.g./dashboard/upcoming?provider=…&account_ids=…. An emptyprovideris accepted as "all".frontend/src/dashboard.ts: passstate.getCurrentProvider()andstate.getCurrentAccountIDs()at both call sites, the initial load (around line 167) andretryUpcomingPurchases(around line 501). The widget then refetches whenever Home reloads for a chip change.UPCOMING_ACCOUNT_SCOPE_NOTE"The account filter does not apply here" note and its rendering inrenderUpcomingPurchases.scopeUpcomingToProviderbecomes redundant: it's harmless to keep, but it can go. KeepupcomingEmptyMessage, so an empty filtered result still names the provider. Update thelib/upcoming-scope.tsheader comment, which says the endpoint takes no filter parameters.Maintainer: please mirror the issue's triage labels on this PR:
priority/p2,severity/high,urgency/this-quarter,impact/all-users,effort/s,type/bug,triaged.🤖 Generated with Claude Code