fix(web): keep Sales order panels closed after pending writes

[mforce]

Closing a Sales order-detail panel during a pending write or detail refresh could let the late result reopen it. Panel writes now capture the order ID and publish panel state only while that order remains active. Writes, reads, list refreshes, and the distinct order/payment idempotency-key ordering are preserved; payment confirmations remain page-owned.

Refs #703, PR5. Only SalesPage.tsx and SalesPage.test.tsx change. Close also discards the line editor synchronously, preventing a stale draft from covering newly fetched values after reopening. Both modal action bodies and all 94 original Sales tests remain byte-identical. #703 stays open for PR6; #708/#709 remain separate chores. Pre-existing behavior is tracked separately: #712 covers the primary Open-read race; #713 covers live-editor reconciliation after a refetch or status change.

Validation on f56fc12ee246e8779d0b1e0b82e9e7a742a563d5, independently rerun by the driver:

• 2,664 tests passed across 119 files; Sales 117 = 94 + 23. Typecheck, coverage floors, build, and service-worker verification passed.
• Restoring the base source with the new tests gives 14 failures, including Close during writes and reads.
• 10 proof mutations killed; 2 survive. Removing the payments publication guard is not observable through the closed panel's DOM; reopening clears payments. A separate reach probe proves that publication executes. The late editor cleanup guard also survives because Close already resets the editor. Both limits are recorded.
• Every mutation restored to a typechecked source and a passing 117-test Sales suite. Clean diff/marker checks and exact modal/test-prefix comparisons passed.
• Final-head CI and reviewer status are tracked on this PR; approval on an earlier commit is not final-head clearance. Backend/security checks are CI-attested, not local executions.

Summary by CodeRabbit

• Bug Fixes
  • The order panel can now be closed while updates, refreshes, or payment actions are in progress.
  • Prevented completed background operations from updating a different order after switching or closing the panel.
  • Success messages are no longer shown after an action is abandoned by closing the panel.
  • Preserved unsaved line-item edits when reopening the same order.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 5e56f9ea-30ea-4164-b1eb-241309fb9642

📥 Commits

Reviewing files that changed from the base of the PR and between c46ee2f and f56fc12.

📒 Files selected for processing (2)

• web/src/routes/SalesPage.test.tsx
• web/src/routes/SalesPage.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

• web/src/routes/SalesPage.tsx

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

SalesPage now guards asynchronous order and payment updates with captured order IDs. New tests cover panel dismissal during in-flight operations, success-message behavior, list refreshes, idempotency-key policies, payment voids, and editor-state preservation.

Changes

Sales panel lifecycle

Layer / File(s)	Summary
Active-order tracking and refresh guards web/src/routes/SalesPage.tsx	The page records the active order ID and applies order and payment results only when that order remains active.
Order mutation lifecycle guards web/src/routes/SalesPage.tsx	Confirm, cancel, and order-void flows use captured order IDs before updating order state or success messages. Both panel Close buttons reset the active order and edit-item state.
Panel lifecycle and write contracts web/src/routes/SalesPage.test.tsx	Tests cover dismissal during writes and refreshes, suppressed success messages, list refreshes, idempotency-key reuse and rotation, payment void behavior, and preserved editor state.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to f56fc

Sales panels remain closed after late order or payment operations settle, while associated refresh and idempotency behavior remains covered. No concrete current-head merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant SalesPage
  participant OrderAPI
  participant PaymentAPI
  participant ActiveOrderState
  SalesPage->>OrderAPI: Submit mutation with captured order ID
  OrderAPI-->>SalesPage: Return mutation result and refreshed order
  SalesPage->>ActiveOrderState: Apply result if captured ID remains active
  SalesPage->>PaymentAPI: Refresh payments for captured order ID
  PaymentAPI-->>SalesPage: Return payment data
  SalesPage->>ActiveOrderState: Apply payment data if captured ID remains active

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The description explains the problem, implementation, scope, issue references, validation results, mutation-testing limits, and known follow-up work. It does not reproduce the checklist, but it provid…
Title check	✅ Passed	The title is concise, conventional, and accurately identifies the primary change: keeping Sales order panels closed during pending writes.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/703-sales-panel-liveness

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mforce]

@coderabbitai review

Please review the complete PR diff at c46ee2f, including panel Close during pending writes and preserved idempotency-key ordering.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

target='c46ee2f53003cb6038b9331d4ed48e5963a24e5b'
git status --short
git show --no-ext-diff --format=fuller --stat "$target"
base="$(git merge-base "${target}^" "$target")"
printf '\n--- merge base: %s ---\n' "$base"
git diff --no-ext-diff --check "$base" "$target"
printf '\n--- changed paths ---\n'
git diff --no-ext-diff --name-status "$base" "$target"
printf '\n--- full diff ---\n'
git diff --no-ext-diff --unified=80 "$base" "$target" -- \
  ':(glob)**/SalesPage.tsx' ':(glob)**/SalesPage.test.tsx'

printf '\n--- candidate files ---\n'
fd -a -t f 'SalesPage(\.test)?\.tsx$' .

printf '\n--- AST outlines ---\n'
while IFS= read -r file; do
  ast-grep outline "$file" --items all --view expanded
done < <(fd -a -t f 'SalesPage(\.test)?\.tsx$' .)

printf '\n--- liveness/key symbols with context ---\n'
rg -n -C 5 -P '\b(activeOrder|setActiveOrder|idempotency|clearKey|setPayments|setOrder|setOrders|onClose|Close)\b' \
  -g 'SalesPage.tsx' -g 'SalesPage.test.tsx' .

Length of output: 50956

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

target='c46ee2f53003cb6038b9331d4ed48e5963a24e5b'

printf '%s\n' '--- component outline ---'
git show "$target:web/src/routes/SalesPage.tsx" > /tmp/SalesPage.tsx
ast-grep outline /tmp/SalesPage.tsx --items all --view expanded

printf '%s\n' '--- complete active/close/effect call-site map ---'
git show "$target:web/src/routes/SalesPage.tsx" |
  rg -n -C 10 -P '\b(setActive(?:State)?|activeIdRef|onOpen|onAddItem|onUpdateItem|onRemoveItem|onConfirm|onCancel|onVoid|onVoidPayment|onRecordPayment|refreshPayments|onClose)\b'

printf '%s\n' '--- source: effects through handlers ---'
git show "$target:web/src/routes/SalesPage.tsx" | sed -n '330,680p'

printf '%s\n' '--- source: panel rendering and Close handler ---'
git show "$target:web/src/routes/SalesPage.tsx" | sed -n '680,1040p'

printf '%s\n' '--- test outline and test infrastructure ---'
git show "$target:web/src/routes/SalesPage.test.tsx" > /tmp/SalesPage.test.tsx
ast-grep outline /tmp/SalesPage.test.tsx --items all --view expanded
git show "$target:web/src/routes/SalesPage.test.tsx" | sed -n '1,330p'

Length of output: 82130

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

target='c46ee2f53003cb6038b9331d4ed48e5963a24e5b'
base='778faab93951300be38cfe13f2f0be5d90585e6a'

git show "$target:web/src/routes/SalesPage.tsx" > /tmp/SalesPage-current.tsx

printf '%s\n' '--- direct active state mutations ---'
rg -n -C 2 -P '\bsetActiveState\s*\(|\bsetActive\s*\(' /tmp/SalesPage-current.tsx

printf '%s\n' '--- changed async state/key operations with current line numbers ---'
git diff --no-ext-diff --unified=12 "$base" "$target" -- web/src/routes/SalesPage.tsx |
  rg -n -C 8 -P '^\+.*\b(await|getOrder|listOrderPayments|setActive|setPayments|setEditItemId|setMessage|clearKey|activeIdRef)\b|^@@'

printf '%s\n' '--- all await-to-publication handler blocks ---'
sed -n '480,705p' /tmp/SalesPage-current.tsx

printf '%s\n' '--- scope/key behavior versus parent ---'
git diff --no-ext-diff --word-diff=plain "$base" "$target" -- web/src/routes/SalesPage.tsx |
  rg -n -C 3 -P '\b(keyFor|clearKey|scope)\b'

printf '%s\n' '--- test selectors added by this commit ---'
git diff --no-ext-diff --unified=2 "$base" "$target" -- web/src/routes/SalesPage.test.tsx |
  rg -n -C 2 -P '^\+\s*(it|describe|for \(|it\.each)|^\+\s*expect\('

Length of output: 41300

---

@mforce No blocking findings.

I reviewed commit c46ee2f53003cb6038b9331d4ed48e5963a24e5b.

The panel Close path now clears the active-order reference synchronously. Late item, order, and payment refresh results publish panel state only when their captured order ID is still active. The patch also retains the existing idempotency-key ordering for item/order writes and void-payment retries.

The added tests cover Close during both write and refresh phases, stale status messages, key reuse and rotation, payment behavior, and line-editor preservation.

Runtime execution was not available in this review sandbox. The static review found no issue.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mforce]

@coderabbitai review

New head f56fc12 fixes a stale line editor after Close: dismissal resets the editor synchronously, and reopening tests now use freshly changed server values. Please review this head; the earlier approval covers c46ee2f.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base='c46ee2f53003cb6038b9331d4ed48e5963a24e5b'
head='f56fc12ee246e8779d0b1e0b82e9e7a742a563d5'

git status --short
git merge-base --is-ancestor "$base" "$head"
printf '%s\n' '--- commits ---'
git show --no-ext-diff --format=fuller --stat "$head"
printf '%s\n' '--- diff check ---'
git diff --no-ext-diff --check "$base" "$head"
printf '%s\n' '--- changed paths ---'
git diff --no-ext-diff --name-status "$base" "$head"
printf '%s\n' '--- complete incremental diff ---'
git diff --no-ext-diff --unified=100 "$base" "$head" -- \
  web/src/routes/SalesPage.tsx web/src/routes/SalesPage.test.tsx

printf '%s\n' '--- component and test outlines ---'
ast-grep outline web/src/routes/SalesPage.tsx --items all --view expanded
ast-grep outline web/src/routes/SalesPage.test.tsx --items all --view expanded

printf '%s\n' '--- close/editor/reopen state paths ---'
rg -n -C 12 -P '\b(onClose|setEditItemId|editItemId|setActiveState|activeIdRef|onOpen|onUpdateItem|onAddItem|onRemoveItem)\b' \
  web/src/routes/SalesPage.tsx web/src/routes/SalesPage.test.tsx

printf '%s\n' '--- current close and panel rendering sections ---'
sed -n '430,820p' web/src/routes/SalesPage.tsx
printf '%s\n' '--- changed test sections ---'
git diff --no-ext-diff --unified=20 "$base" "$head" -- web/src/routes/SalesPage.test.tsx

Length of output: 100987

---

@mforce No blocking findings.

I reviewed the incremental diff from c46ee2f5 to f56fc12e.

closeOrderPanel now clears active and editItemId in the same Close path. Both Draft and non-Draft panel Close buttons use this path.

A late onUpdateItem result cannot restore the dismissed editor. The existing activeIdRef.current === id guards prevent publication after Close.

The revised test covers both idle and pending writes. It also verifies that reopening uses the changed server line values and starts outside edit mode.

git diff --check found no whitespace errors. Runtime test execution was unavailable in this sandbox.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mforce]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mforce]

PR5 verification is complete at f56fc12ee246e8779d0b1e0b82e9e7a742a563d5: 2,664 tests, typecheck, coverage, build, service-worker verification, and all CI checks passed. The two modal action bodies and 94 original Sales tests are unchanged. Round1's stale-editor regression was fixed and reviewed on this head.

Two review rounds ran. Opus, Sonnet, Qwen, and CodeRabbit's current-head comment report no merge-blocking defect. The workflow's file-based count is honestly NOT-CLEAN, 0/2: the reviews found pre-existing behavior in the same touched file. #712 tracks Close losing to a primary Open read; #713 tracks live-editor reconciliation on refetch/status changes. Both were reproduced with base source as well as the reviewed source (the #712 establishing-read body is unchanged). They remain separate from PR5's abandoned-write/follow-up-read fix.

No further source changes or repeated unchanged-head review round are underway. Scoped acceptance with those follow-ups is the owner's decision; no merge or auto-merge is armed. The formal CodeRabbit approval must also name this head before the merge ask; a current comment/green skipped status does not substitute for that record.

[mforce]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mforce]

PR5 is at the owner merge gate on f56fc12ee246e8779d0b1e0b82e9e7a742a563d5. The owner independently reviewed this head and approved the scoped fix; #712/#713 remain separate pre-existing follow-ups. The historical file-based review count remains NOT-CLEAN 0/2 rather than being relabelled.

I retriggered CodeRabbit with the exact standalone command. It accepted the request and selected c46ee2f5..f56fc12e, but after the ten-minute bound its formal approval still names the parent and the new review remains pending. Under the owner's explicit authorization, I accept the existing current-head seat coverage: Opus, Sonnet, Qwen, CodeRabbit's incremental static review, independent driver verification, and the owner's independent review. This is an explicit coverage decision, not a claim that the old approval covers the final head. Read any new bot findings before actual merge.

All non-bot checks pass; commit-image publication is intentionally skipped. Driver-verified: 2,664 tests, coverage floors, typecheck/build/service worker, causal base-red and mutation checks, clean trees/markers, preserved original tests and modal bodies. Backend/security execution is CI-attested; local browser evidence is route component tests plus the exact-component CDP probe.

The completed merge packet is recorded locally in 55-merge-ask-pr5.md and passes its checker. No merge or auto-merge has been performed. #703 stays open for PR6's fail-open decision.