Skip to content

refactor(frontend/plans): drop redundant setPlanAccounts call on create path (#743 follow-up) #745

Closed

Description

@cristim

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:

  1. Backend POST /api/plans -> SetPlanAccounts inside createPlan.
  2. Frontend follow-up PUT /api/plans/:id/accounts -> SetPlanAccounts again.

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

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