Skip to content

Sales: an abandoned attempt's success hijacks the dialog session that replaced it #477

Description

@mforce

Follow-up to #474 / PR #476, which merged while the third review round was still open. Two findings arrived against that PR's head and are not in main:

Part 1 shipped in PR #478 (d157a97b): SalesPage now keeps the page's failures and the open dialog's in separate state, so neither can overwrite the other. This issue stays open for part 2, which is untouched. Applying the same split to the other dialog screens is #479.

Part 1, for the record (fixed)

1. A background read wipes the open dialog's own message (codex P2)

SalesPage keeps one error slot shared by every action. A confirmed order's listOrderPayments read does not set busy, so the user can open New order and submit while it is still pending. If createOrder rejects first, its actionable 422 shows in the dialog — and then the later payment-read rejection overwrites the same slot, so the form's own explanation disappears from the still-open modal, replaced by a message about a different order.

Fix (already written, see the PR that closes this): split the state in two. error is the page's, untagged; dialogError is whichever dialog is open (they are modal, so at most one ever is, and each clears it on open). run() routes by a DIALOG_SCOPES list. Neither slot can overwrite the other, and each attempt clears only its own — so trying something new never silently drops a failure the user has not dealt with.

This also removes code: shownInDialog goes, the page copy is unconditional again, the scope tag on the page error goes, and the dismissal no longer has to clear a message.

2. An abandoned attempt's SUCCESS hijacks the session that replaced it (still open)

Neither dialog trigger is gated on busy, so this sequence is reachable:

  1. Open New order, pick customer A, submit → the POST is in flight.
  2. Cancel. The dialog closes; the request is still out.
  3. Open New order again, pick customer B, start filling in. (Submit is disabled — still busy — but the form is live.)
  4. Step 1's POST succeeds. Its success path runs unconditionally: setActive(await getOrder(created.id)) swaps the order panel to customer A's new order, and setCreatingOrder(false) force-closes the dialog customer B's data was being typed into, discarding it.

record-payment has the milder version: a stray "Payment recorded" message and a force-closed dialog.

The abandoned-attempt marker added for #474 covers only the failure path — it suppresses a stale error, not a stale success's side effects. Fixing this properly needs a per-session generation (an attempt id captured when the dialog opens, compared before the success path touches shared state), which is more than the error-rendering fix should carry.

Pre-existing on main; not introduced by #476. Found by an internal reviewer during that PR's third round.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions