Repository navigation
feat(plans): surface legacy no-account plans as Unassigned (closes #973) - #994
Merged
Merged
Conversation
Plans created before target_accounts was required (#743) have zero rows in plan_accounts and are invisible in account-filtered views because the JOIN on plan_accounts excludes them. Backend: buildListPlansQuery now uses LEFT JOIN + OR NOT EXISTS so that zero-account plans are included alongside matched-account plans when an account filter is active. A computed boolean column "unassigned" (true for zero-account plans, false otherwise) is selected so callers can bucket the two groups without a second query. The no-filter case continues to return all plans and sets unassigned=false. PurchasePlan gains an Unassigned field that is omitted from JSON when false. Frontend: renderPlans splits plans into assigned and unassigned buckets. Assigned plans render as before. Unassigned plans are appended under a clearly labeled "Unassigned" section header (class unassigned-plans-header). Account-scoped actions (Add Purchases, Edit, enable toggle) are suppressed for unassigned plans; History and Delete remain available. Account-name resolution is skipped for unassigned plans because they have no plan_accounts rows. Tests: backend adds TestPGXMock_ListPurchasePlans_UnassignedIncluded (zero-account plan flagged true, assigned plan flagged false) and TestHandler_HandleRequest_ListPlans_UnassignedFlagged (API-level regression guard). Frontend adds two loadPlans tests: one asserting the Unassigned section appears with the correct order, another asserting it is absent when all plans are assigned.
cristim
added
triaged
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Add a testcontainers-backed integration test (TestPostgresStoreDB_ListPurchasePlans_UnassignedBucket) that exercises the LEFT JOIN + OR NOT EXISTS query introduced in #973 against a real Postgres instance. Seed layout: - planA assigned to accountX: must appear with Unassigned=false - planB with zero plan_accounts rows: must appear with Unassigned=true - planC assigned to accountY only: must NOT appear in accountX filter The discriminating assertion (planB present in accountX-filtered result) fails on the pre-fix INNER JOIN code and passes with the LEFT JOIN fix, so a regression back to INNER JOIN will be caught by CI. Also fix two pre-existing compile errors in store_postgres_test.go (package config_test): add the config. qualifier to PurchasePlanFilter and inline the unexported pf() helper that was inaccessible from the external test package.
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
target_accountswas required (fix(plans): eliminate universal plans — require target_accounts on creation #743) have zeroplan_accountsrows and are silently excluded from account-filtered views (the INNER JOIN onplan_accountsdrops them entirely)buildListPlansQuerynow uses LEFT JOIN +OR NOT EXISTSto include zero-account plans alongside matched-account plans; a computedunassignedboolean column tags them so callers can bucket the two groups without a second queryrenderPlanssplits plans into assigned/unassigned buckets; unassigned plans render under a clearly labeled "Unassigned" section (read-only: account-scoped actions suppressed, History/Delete remain)Backend approach
buildListPlansQueryaddsfalse AS unassignedin the no-filter path (all plans returned), and in the filtered path:plan_accountsinstead of INNER JOINWHERE pa.account_id IN (...) OR NOT EXISTS (SELECT 1 FROM plan_accounts WHERE plan_id = pp.id)(NOT EXISTS ...) AS unassignedas a computed columnPurchasePlangains anUnassigned boolfield (json:"unassigned,omitempty"). Tenant/permission scoping is unchanged -- the existingrequirePermission+requirePlanAccessgates apply tolistPlansas before.Frontend bucket location
An
<div class="plans-section-header unassigned-plans-header">section is appended after all assigned plan cards. Unassigned plan cards omit the Add Purchases, Edit, and enable toggle actions (they have no account scope to purchase against). History and Delete remain available.Tests
Backend:
TestPGXMock_ListPurchasePlans_UnassignedIncluded-- zero-account plan flaggedUnassigned=true, assigned plan flaggedfalseTestHandler_HandleRequest_ListPlans_UnassignedFlagged-- API-level regression guard: both plans appear in response with correctunassignedvaluesFrontend:
unassigned plan renders under Unassigned section, assigned plan does not (issue #973)-- asserts section header present and ordering correctno Unassigned section when all plans are assigned-- asserts header absentTest plan
go build ./...go test ./internal/config/...(560 pass)go test ./internal/api/...(1465 pass)npm run buildnpm test(2360 pass, 1 pre-existing skip)plan_accountsrows, select an account filter, verify plan appears under "Unassigned" in the UIcloses #973
🤖 Generated with claude-flow