Repository navigation
fix(frontend): floor the marketplace consent dialog's remaining term - #383
Conversation
The marketplace consent dialog computed the remaining months with Math.max(0, Math.round(...)), while the backend's computeRemainingMonths (internal/api/handler_marketplace.go) floors and clamps to a minimum of 1. For any fractional remainder of half a month or more, the two diverged: the dialog would show and the user would authorize one price, while a different, lower price was actually submitted server-side. A comment on the dialog claimed the two calculations mirrored each other exactly. Change to Math.max(1, Math.floor(...)) to match the backend exactly. Two existing tests pinned timestamps at exact month boundaries (assuming round semantics); moved them off the boundary with a half-month margin so they stay deterministic under floor and updated their expected figures. Co-Authored-By: claude-flow <ruv@ruv.net>
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 5 minutes for your next included review. View limit detailsLimit details: You’ve used all 4 included reviews currently available. Your 32 included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
cristim
added
urgency/this-sprint
|
Independent adversarial review + local verification: MERGE at 677c842. The consent dialog's remaining term, residual per unit, 0.95 buyer discount and 12% fee now match computeRemainingMonths / marketplaceResidualPerUnit in handler_marketplace.go line by line (floor, min 1). Pre-fix test reproduces the issue's 7 months / $665 / $585.2; passes after; lint/build OK. |
Summary
The marketplace consent dialog computed the list price it shows by rounding the remaining months with a floor of zero, while the backend recomputes the price by flooring with a floor of one. For any fractional remainder of half a month or more, the user authorized one amount and a different, lower amount was actually submitted.
Root cause
frontend/src/history.tscomputedremainingMonths = Math.max(0, Math.round(termMonths - elapsedMonths)), while the backend'scomputeRemainingMonths(internal/api/handler_marketplace.go:447) computesint(math.Floor(remaining))clamped to a minimum of 1. A comment on the dialog claimed the two calculations mirror each other "EXACTLY", but they used different rounding functions and different floors.Worked example from the issue: a 1-year, $1,200-upfront, count-1 RI purchased 5.4 months ago. Remaining is 6.6. Pre-fix the dialog rounds to 7 months and shows a list price of $665 (net $585.20); the backend floors to 6 and actually lists at $570 (net $501.60).
Fix
Changed to
Math.max(1, Math.floor(termMonths - elapsedMonths)), matchingcomputeRemainingMonthsexactly.Regression test
Added
frontend/src/__tests__/history-marketplace-sell-button.test.ts: "issue #240: floors the remaining term like the backend, not rounds it", reproducing the issue's exact worked example (1yr, $1,200 upfront, count 1, purchased 5.4 months ago) and asserting the dialog shows 6 months / $570 / $501.6, not the pre-fix 7 months / $665 / $585.Two pre-existing tests in the same file ("shows the correctly-prorated residual for a 3yr RI ~6 months in" and "shows the backend-matching per-unit price...") pinned their fixture timestamps at exact month boundaries, which is only stable under
roundsemantics -- underfloor, real wall-clock drift between building the timestamp and the click handler re-computing elapsed time could tip the result either side of the boundary. Moved both to a half-month margin off the boundary (same expected remaining-months/price values for the first; updated values for the second, which was previously sitting exactly on a 12-month boundary) so they stay deterministic.Proved pre-fix failure:
git stash push -- frontend/src/history.tsthen ran the new test -- failed withExpected substring: "6 months"/ actual"...Remaining term7 months...$665...$585.2..."(matching the issue's stated pre-fix figures exactly). Restored viagit stash popand reran: passes.Verification
npm test -- history-- 233 passed (13 suites)npx eslint src/history.ts src/__tests__/history-marketplace-sell-button.test.ts-- no issuesnpm run build-- webpack production build succeeds (pre-existing entrypoint-size warning only)Closes #240