Repository navigation
refactor(frontend/plans): drop redundant setPlanAccounts call on create path (#743 follow-up) #745
Description
Summary
Surfaced by CodeRabbit during review of #743 (nitpick — out of scope for that
PR). In frontend/src/plans.ts around the plan save flow (currently L644-661),
after api.createPlan succeeds the frontend also calls api.setPlanAccounts
unconditionally, even though the backend createPlan handler now inserts
plan_accounts rows atomically as part of the same request (per the
universal-plan fix landed in #743).
This results in two writes to plan_accounts for the create path:
- Backend
POST /api/plans->SetPlanAccountsinsidecreatePlan. - Frontend follow-up
PUT /api/plans/:id/accounts->SetPlanAccountsagain.
The second call uses DELETE+INSERT (per SetPlanAccounts contract), so the
end state is the same and the user-visible behaviour is correct. The current
inline comment in plans.ts rationalises the redundant call as a defensive
"belt-and-suspenders" against the backend ever diverging from the atomic
contract — which is genuine, but with #743 now hard-enforcing the atomic
insert, that defense is over-engineered.
What to change
Track wasExistingPlan (or capture a preSavePlanId snapshot) and only invoke
api.setPlanAccounts on the update path. CR's suggested diff:
try {
let savedPlanId = planId;
if (planId) {
await api.updatePlan(planId, plan as unknown as api.CreatePlanRequest);
+ // Update flow: push the selected account list via the dedicated endpoint.
+ await api.setPlanAccounts(planId, accountIds);
} else {
const created = await api.createPlan(plan as unknown as api.CreatePlanRequest) as unknown as { id: string };
savedPlanId = created.id;
+ // Create flow: backend inserted plan_accounts atomically from
+ // target_accounts in the POST body (see internal/api/handler_plans.go
+ // createPlan). No follow-up account-write needed.
}
-
- if (savedPlanId) {
- await api.setPlanAccounts(savedPlanId, accountIds);
- }Why not in #743
#743's scope was eliminating universal plans (back-end + DB + back-door close +
modal validation). Trimming the frontend's redundant call is a separate
optimisation that doesn't affect correctness. The current code is documented
("belt-and-suspenders") so it's intentional, not an oversight.
References
- CodeRabbit review on PR fix(plans): eliminate universal plans — require target_accounts on creation #743 (2026-05-27): nitpick Update Deployment Model to Terraform #1 (
frontend/src/plans.ts:653-661). - Atomic plan_accounts insert behaviour landed in fix(plans): eliminate universal plans — require target_accounts on creation #743 (
internal/api/handler_plans.gocreatePlan).
Activity
refactor(frontend/plans): drop redundant setPlanAccounts call (#745)
refactor(frontend/plans): drop redundant setPlanAccounts call (#745)
refactor(frontend/plans): drop redundant setPlanAccounts call (#745)