Skip to content

fix(frontend): show totals in the bulk approval confirm - #1362

Merged
IAmJSD merged 3 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-707
Oct 24, 2026
Merged

IAmJSD merged 3 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-707

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

#781 already made the Execute now and Run now confirms show what is bought, for which account, term and payment option, and the amounts. Main (#1364/#711) now also gives the bulk "Send N bulk purchases for approval?" confirm the commitment summary, with one line per commitment and its account. Two gaps remained against #707: the bulk confirm did not say that its totals cover every request, and its button ("Send all for approval") did not carry the amount, unlike the single-purchase confirm. This PR closes both.

Closes #707

What changed

  • frontend/src/app.ts: handleFanOutExecute builds the summary lines with main's commitmentLine, as before, and computes commitmentTotals over them. The confirm button now uses the shared executeConfirmLabel(false, totals.upfront), so it reads "Send for Approval ($X upfront on approval)", the same as the single-purchase confirm. The intro gains one sentence: "Totals below are across all requests."
  • Tests: new case "bulk approval confirm states the totals across all buckets (fix(frontend): Execute now and Run now confirms show no amount or account #707)" in purchase-modal-fanout-payment.test.ts; a reworded mock comment in purchase-execution-toast.test.ts; new tests-e2e/bulk-approval-confirm.spec.ts.

Why this approach

It reuses the commitmentSummary component from #781 and the existing executeConfirmLabel, so the bulk confirm reads like the single confirm and no new UI is invented. A submittable bucket's recs are already priced for that bucket's payment option (isSubmittableBucket), so the upfront total on the button is the amount that will be approved.

Screenshots

Screenshots were taken before this branch was merged with main; before = main at a14dc8f. The before shot is not shown, because main at a14dc8f had no commitment summary on the bulk confirm and current main does. Headless Chromium at 1440x900 against the repo's mocked e2e API.

After
bulk-confirm after

Bulk approval confirm for 3 buckets. Compared with current main, the changes are the intro sentence "Totals below are across all requests." and the upfront amount on the button. The summary band and the per-commitment lines come from main.

Verification

Run on this branch's head 30ab91f0.

  • Merged with main at 5b52c4e; conflicts in frontend/src/app.ts, frontend/src/__tests__/purchase-execution-toast.test.ts and frontend/src/__tests__/purchase-modal-submit.test.ts resolved, see the merge commit.
  • npm run lint: 0 errors. Typecheck clean, build OK.
  • Jest: 3864 of 3865 passed, 1 skipped. The new test checks the title, upfront $8,000, monthly $100, term total $9,200, the lines, no "Charged today", and the amount on the button.
  • Playwright: 201 passed, including the new bulk-approval-confirm.spec.ts (totals computed from the fixture rows, 3 lines, button label, Cancel posts nothing).
  • Secrets scan clean, planted key flagged.

Notes for review

  • The confirm button changed from "Send all for approval" to the shared "Send for Approval ($X upfront on approval)" to match the single path. Easy to keep "Send all" if preferred. When every bucket is no-upfront the button reads "($0.00 upfront on approval)", the same as the single-purchase confirm.
  • No test asserts the new intro sentence yet.
  • The screenshot shows "Monthly $550" although every line is All upfront. That comes from the e2e fixture, whose all-upfront rows carry a non-zero monthly_cost; the single-purchase confirm shows the same figure.
  • Everything else the issue lists (Execute now, Run now, payment-matched wording, disabled trigger) is on main via fix(frontend): money confirms state what is bought and how much #781 and is not repeated here.

Overlaps

Related to #709 (RI exchange confirm), which also builds on commitmentSummary but in the RI-exchange files, so no conflict.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

The Execute now and Run now confirms already state what is bought and how
much (LeanerCloud#781), but the bulk (fan-out) "Send N bulk purchases for approval?"
confirm still said only how many requests and emails would go out. It now
uses the same commitment summary as the single-purchase confirm: the
combined upfront on approval, monthly and total over term across every
bucket, one line per commitment with account and region, and a confirm
button that names the upfront amount. The rec-to-summary-line mapping is
shared by both paths.

Refs LeanerCloud#707

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD IAmJSD added priority/p2 Backlog-worthy triaged Item has been triaged severity/high Significant harm urgency/this-quarter Within the quarter impact/many Affects most users effort/s Hours type/bug Defect labels Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Conflicts:
- frontend/src/app.ts: main's LeanerCloud#1364 (Refs LeanerCloud#711) added commitmentLine()
  to commitmentSummary.ts and already gives the bulk confirm the same
  Upfront on approval / Monthly / Total over term summary this branch
  added. Took main's commitmentLine for both the single and the fan-out
  confirm and dropped this branch's now-duplicate toCommitmentLine helper
  (and its LocalRecommendation/CommitmentLine imports). Kept this
  branch's remaining LeanerCloud#707 behavior on top: the "Totals below are across
  all requests." intro sentence and the amount on the confirm button via
  executeConfirmLabel(false, totals.upfront) instead of main's plain
  "Send all for approval".
- frontend/src/__tests__/purchase-execution-toast.test.ts: comment on
  the getAccountName mock; merged both wordings (LeanerCloud#707/LeanerCloud#711).
- frontend/src/__tests__/purchase-modal-submit.test.ts: main split this
  suite over a shared harness (LeanerCloud#1349). Took main's file and moved this
  branch's "bulk approval confirm states the totals across all buckets
  (LeanerCloud#707)" test into purchase-modal-fanout-payment.test.ts, next to the
  fan-out payment-change test it sat beside before the split, adding the
  confirmDialog and formatCurrency imports it needs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE 30ab91f0bdb051c76d5861d19e742939c8c56902

Blocking:

@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

CI on 30ab91f0 failed only because the Security Scanning job hung for 50 minutes uploading Trivy results to GitHub and timed out; no scan found anything. Head 9a7230b3 is the approved 30ab91f0 plus a conflict-free merge of main, which gives a fresh CI run. Its added and removed lines against main are identical to the reviewed diff, so the approval above carries over to 9a7230b3.

@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 9a7230b3 merged onto current main (6122fa24; merge-tree dc090cc2; Linux, Node, headless Chromium):

  • This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
  • The combined tree of every UI PR in this merge wave (tree dc090cc2, main plus 17 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4097 passed, 4098 total), and the full Playwright suite (232 passed).
  • CI is green on 9a7230b3 (CI Success, Run pre-commit hooks).
  • The independent review above approves this exact head.
  • Nothing was applied to any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 24, 2026 20:24
@IAmJSD
IAmJSD merged commit f2ab673 into LeanerCloud:main Oct 24, 2026
29 checks passed
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at 9a7230b3.

IAmJSD added a commit to IAmJSD/cloud-commitments-platform that referenced this pull request Oct 24, 2026
Bring in f2ab673 (bulk approval confirm totals, LeanerCloud#1362) before fixing
the toast placement review finding. It merges cleanly; none of its files
overlap this branch.

Refs LeanerCloud#742

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IAmJSD added a commit to IAmJSD/cloud-commitments-platform that referenced this pull request Oct 24, 2026
Conflicts:
- frontend/src/app.ts: import block. Kept this branch's fanout-retry
  import (showFanOutRetry, fanOutRetryLabel, FanOutBucketOutcome) and
  main's setupMobileNav import from nav/mobile-nav (LeanerCloud#1340).

handleFanOutExecute merged cleanly: main's combined bulk-approval
totals and upfront-naming confirm label (LeanerCloud#1362) now sit alongside this
branch's retry-label restore in the finally block. A retry passes only
the narrowed failed buckets, so the confirm totals cover just the
requests being resent.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IAmJSD added a commit to IAmJSD/cloud-commitments-platform that referenced this pull request Oct 24, 2026
…etry e2e

Main's LeanerCloud#1362 renamed the fan-out confirm button from "Send all for
approval" to "Send for Approval ($X upfront on approval)", naming the
combined upfront of the buckets being sent. The partial-failure spec
still clicked the old label and timed out. It now matches the new
label with the amount as a pattern, since the first send and the retry
carry different totals.

Refs LeanerCloud#757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IAmJSD added a commit that referenced this pull request Oct 25, 2026
…#1390)

* fix(frontend): keep the bulk purchase modal open after a partial failure and retry only failed buckets

When some buckets of a multi-bucket ("fan-out") purchase failed, the modal
closed and the selection was cleared before the outcome was classified, and
the toast truncated the reasons (3) and labels (5). Users had to find and
reselect the failed rows in a large table, at the risk of resubmitting the
buckets that had already gone through.

On any bucket failure the modal now stays open:
- each bucket section shows "Submitted: awaiting approval [from X]" /
  "Submitted: joined existing request ..." or "Failed: <full reason>";
- submitted buckets are dimmed and their payment selects are locked;
- a banner (role=alert) states how many were submitted and failed, that
  submitted buckets will not be sent again, and that closing the dialog
  does not cancel them;
- the fan-out state is narrowed to the failed buckets, so the submit
  button becomes "Retry N failed" and resubmits only those. A failed
  bucket's payment can still be changed before retrying.
The sticky failure toast is skipped in this mode (the modal has the full
detail, and the toast covered the Retry button), and the Archera offer
waits until the purchase modal closes. A fully successful retry closes
the modal as before.

Buckets get a stable id (copies from getFanOutBuckets() lose identity)
and sections carry data-bucket-id so results map back to their section.
Callers without ids keep the previous close-and-toast behavior.

Refs #757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(frontend): cover partial bulk purchase failure and retry

Jest: the modal stays open with per-bucket status, submitted buckets are
locked, the button reads "Retry N failed", a retry posts only the failed
buckets and closes the modal (and opens the Archera offer) once all
succeed, a second partial failure keeps earlier successes counted, an
all-failed run also stays open, buckets without ids keep the old
behavior, and failure reasons cannot inject markup.
Playwright: three providers, Azure fails once, the dialog shows the
failure, Retry 1 failed posts only the Azure bucket and closes.

Refs #757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(frontend): match the bulk-approval confirm label in the fan-out retry e2e

Main's #1362 renamed the fan-out confirm button from "Send all for
approval" to "Send for Approval ($X upfront on approval)", naming the
combined upfront of the buckets being sent. The partial-failure spec
still clicked the old label and timed out. It now matches the new
label with the amount as a pattern, since the first send and the retry
carry different totals.

Refs #757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(frontend): use the semibold weight token for the fan-out bucket status

Main's typography foundation (#1378) forbids literal font weights in
modals.css that duplicate a token. The fan-out status line used a
literal 600; use var(--cudly-fw-semibold) instead, same weight.

Refs #757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix(frontend): only enter fan-out retry mode while the submitting modal is open

showFanOutRetry only checked that #purchase-details still held fan-out
sections. Closing the modal (Escape or Cancel) while the POSTs were in
flight leaves those sections in the hidden modal, so a failed bucket put
its banner there and suppressed the failure toast: the user got no
notice at all. Bucket ids were also per-index and reused on every open,
so results settling after the user opened a new bulk modal marked its
sections submitted and narrowed its buckets to the old failed ids.

Bucket ids now carry the purchase-modal open token, and retry mode is
entered only when #purchase-modal is visible and still holds the
submitted bucket ids. Otherwise the toasts fire as on main. If a newer
purchase modal is open, it is not closed or cleared. The #1503
payment-adjustment toast also uses the default timeout in retry mode
instead of staying on top of the Retry button.

Refs #757

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/many Affects most users priority/p2 Backlog-worthy severity/high Significant harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(frontend): Execute now and Run now confirms show no amount or account

1 participant