Skip to content

fix(organization): render a refused coupon's reason as text, not the raw body - #1769

Merged
dawsontoth merged 3 commits into
stagefrom
fix/coupon-toast-problem-details
Sep 30, 2026
Merged

dawsontoth merged 3 commits into
stagefrom
fix/coupon-toast-problem-details

Conversation

@dawsontoth

Copy link
Copy Markdown
Contributor

Summary

A rejected coupon crashes Studio against a central manager running Harper 5. onAddCouponToOrganizationSubmit accepts 400/409 through validateStatus and returns the body unchanged. AddCouponModal then puts that body straight into toast.error({ description }).

A Harper 5 central manager sends ClientErrors as RFC 9457 Problem Details, for example { type, code, title, status, instance }. React cannot render an object as a child, so the root <Toaster> throws React error #31.

Found in the daily RUM review (2026-09-30). On dev.studio, one session hit POST /Coupon → 400, and 180ms later got Minified React error #31 … object with keys {type, code, title, status, instance}. The component stack is sonner's toast inside Toaster, which is mounted at the app root. The central manager's Coupon.post throws 400 for a coupon Stripe rejects and 409 for one already applied (central-manager/src/resources/Coupon.js), so every refusal took this path. Prod is not affected yet, but it will be once its central manager moves to Harper 5.

Fix

  • The mutation now resolves a 400/409 through describeError(...).message. That function already maps Problem Details (title and detail), legacy strings and { error | message } bodies, and it never returns an empty string. The mutation's return type is now Promise<string | undefined>.
  • The modal renders that string. The || 'Failed to add coupon.' fallback could no longer be reached, so it was removed.
  • validateStatus is the only such call site that renders the body. The other one, getSearchByValue's 404, is not rendered.

Decisions

  • A refusal still resolves. It resolves as string | undefined rather than throwing, so a rejected coupon stays a form answer and doesn't reach the global toast or RUM.
  • The inline text leaves out the Problem Details code. The global toast uses that code as its heading; this dialog shows only the title and detail.

Test plan

  • New addCouponToOrganization.test.ts covers three cases: a 204 success (and pins the POST path and payload), a Problem Details 409, and a plain-text 400.
  • Mutation check: reverting the fix turns 2 of the 3 tests red.
  • Full gate: vitest (382 files), tsc -b, oxlint and dprint are all clean.
  • Not run: rendering the modal. The fact that toast.error receives a string is enforced by the types, not by a render test.

Cross-model review

There were two rounds, with codex (gpt-5.6-sol), gemini and harper-domain; independent=true and it converged.

  • Rejected: gemini's round-1 "major", that describeError could yield an empty message and show a fake success. The adjudicator rejected it with line citations: message defaults to non-empty and errorText never returns ''.
  • Taken: the comment trim, the payload assertion and removing the dead fallback.
  • Rejected: the request for .ts import extensions, since the repo convention is extensionless.
  • Cursor contributed nothing. Its leg failed no-receipt on the 1Password SSH agent.

Human-Review-Need: 3 @ 79306c2

🤖 Generated with Claude Code

dawsontoth and others added 2 commits September 30, 2026 10:30
…raw body

AddCouponModal passed the central manager's 400/409 body straight into a toast
description. A Harper 5 central manager answers with an RFC 9457 object, and
React cannot render one, so a rejected coupon threw React error #31 inside the
root Toaster (seen in RUM on dev.studio, right after a POST /Coupon 400).

Resolve the refusal as text through describeError, which already maps both the
problem-details shape and the legacy string.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dawsontoth
dawsontoth requested a review from a team as a code owner September 30, 2026 14:42
@dawsontoth dawsontoth added the rum From real user monitoring where we aim to keep users happy label Sep 30, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the coupon submission logic for organizations to handle 400 and 409 error responses by extracting the error message using describeError and returning it, and adds corresponding unit tests. Feedback on the changes points out a potential bug where an empty error string would evaluate to falsy in AddCouponModal, incorrectly triggering a success toast. It is recommended to strictly check for error === undefined and restore the fallback error message.

Comment thread src/features/organization/modals/AddCouponModal.tsx
@github-actions

github-actions Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 66.41% 9846 / 14825
🔵 Statements 66.6% 10518 / 15791
🔵 Functions 59.67% 2533 / 4245
🔵 Branches 61.21% 7439 / 12152
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/features/organization/modals/AddCouponModal.tsx 46.66% 25% 33.33% 46.66% 48, 54-66, 76-112
src/features/organization/mutations/addCouponToOrganization.ts 83.33% 50% 66.66% 83.33% 19
Generated in workflow #2035 for commit e25853d by the Vitest Coverage Report Action

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change safely converts rejected coupon responses into renderable text. The focused tests cover successful, Problem Details, and plain-text responses.

—
Reviewed 79306c2

…o text

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

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rejected coupons are converted to a string through describeError before the toast, which stops a Harper 5 Problem Details object from crashing the root toaster. The empty-body false-success case is already in the thread and is pinned by the new test plus describeError's non-empty default. No remaining blocking defects on the changed lines.

—
Reviewed e25853d

@cb1kenobi cb1kenobi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rejected coupons are converted to a string through describeError before the toast, so a Harper 5 Problem Details object no longer crashes the root toaster. The empty-body false-success case was already raised on AddCouponModal and is pinned by the new test plus describeError's non-empty default. No remaining blocking defects on the changed lines.

—
Reviewed e25853d

@dawsontoth
dawsontoth merged commit 41752a5 into stage Sep 30, 2026
2 checks passed
@dawsontoth
dawsontoth deleted the fix/coupon-toast-problem-details branch September 30, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rum From real user monitoring where we aim to keep users happy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants