fix: string badge-icon fallback + reachable restart-identity modal (release-audit follow-ups)#2219
Merged
Conversation
…e restart-identity modal getBadgeIcon fell back to PEANUTMAN_LOGO (StaticImageData, typed `any` by the svg shim) while shareAssetLayout types iconUrl as string and ShareAssetD3 feeds it to a raw <img src> — an unknown badge code (the recurring FE-BADGES-drop incident) would render a broken stamp. Unwrap .src and pin the contract with a test. UnlockedRegions' provider-rejection override only promoted fixable/blocked, so the restart-identity copy + handleRestartIdentity CTA already wired in the modal were unreachable from this predicate — those users fell back to the generic start flow. Include restart-identity. Plus two lint nits from the #2218 review: unused useRouter in the payment error boundary, missing alt on the careers mascot.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR contains four isolated improvements: error recovery logging in the PaymentError component, badge icon return-type normalization with corresponding test coverage and Jest mock updates, provider rejection state routing expansion in UnlockedRegions, and an accessibility fix adding an alt attribute to an image element. ChangesUI and UX refinements
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
Contributor
Code-analysis diffPainscore total: 5815.92 → 5819.26 (+3.34) 🆕 New findings (8)
✅ Resolved (8)
📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
…st asset stub
/code-review pass on this PR converged on two structural fixes:
The predicate now reads state !== 'happy' instead of enumerating the three
non-happy members — the enumeration pattern is how restart-identity got missed
in the first place, and a future fourth state would have repeated it.
The jest asset stub (jest-transform-stub) flattened image imports to a bare
string while Next yields StaticImageData — tests of .src-reading code ran
against fiction, which is exactly how the original object-into-string fallback
shipped unnoticed. Point the mapper at a {src,width,height} stub and drop both
per-file mascot mocks this PR had added. jest-transform-stub stays in
devDependencies for now: removing it forces a pnpm lockfile re-resolve that
drags Sentry onto different otel peers — that cleanup belongs in a deps PR.
This was referenced Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-ups from the release-audit + CodeRabbit pass on #2218 — all pre-existing (zero delta in the release), bundled here so the release PR stays clean.
getBadgeIconreturns a string, always. The fallback returnedPEANUTMAN_LOGO(StaticImageData, typedanyby the svg module shim — so typecheck never caught it) whileshareAssetLayouttypesiconUrl: stringandShareAssetD3feeds it to a raw<img src>. An unknown badge code — exactly what the recurring FE-BADGES-drop incident produces — would render a broken stamp in the share asset. Now unwraps.src, with a: stringannotation and a guard test pinning both branches.restart-identityreaches its own modal.UnlockedRegions' provider-rejection override only promotedfixable/blocked; the dedicated restart-identity copy +handleRestartIdentityCTA already wired inside theprovider_rejectionActionModal were unreachable from this predicate — those users fell back to the generic start flow. Predicate now includesrestart-identity.useRouterin the payment error boundary;alt=""on the careers mascot img.Test changes: new
badge.utils.test.ts;shareAssetLayout.test.tsgets a StaticImageData-shaped mascot mock (jest's svg stub flattens imports to a bare string, so the fallback path needs the real shape).Declined CodeRabbit findings (from #2218, for the record)
claim_externalsign flip — misread semantics: incoming claims take thereceive/+branch;claim_externalis the claim-to-external-wallet path.?method=param, repeated-@invite strip — valid minors, out of scope here; noted in TODO.Risks / breaking changes
None — FE-only, no contract changes, no migrations. Blast radius: badge icon fallback (share asset + badge lists), one KYC modal predicate, an error boundary, the careers page.
QA
getBadgeIcon('NOT_A_REAL_BADGE')→ string URL (unit-tested).restart-identitynow gets the "Verify with a different document" modal instead of the generic flow./code-review findings + disposition (second commit)
state !== 'happy'(enumeration is the bug class that caused the original miss); jest asset stub → StaticImageData shape at themoduleNameMapperlevel (src/utils/__mocks__/static-image.ts), deleting both per-file mascot mocks —.src-reading code no longer tests against fiction.POST /users/identity/restartand thecountry_not_supportedreason exist only in open api PR fixed withdraw status not resetting after withdraw #914 — the restart-identity state is unreachable on today's backend, so the predicate change is inert until fixed withdraw status not resetting after withdraw #914 merges, then becomes live with a working CTA. No 404 exposure.isSumsubApprovedhere is an any-rail-enabled proxy (useCapabilities.ts:184), so a user whose only rails are blocked Mantecacountry_not_supportedones never reaches this modal — the gate should read the provider-agnosticidentityVerificationsignal instead. In TODO.processingvariant (already true for fixable/blocked); restart-identity call sites run withregionIntent=undefined(shared pattern, BE pins the level); 4 surfaces hand-roll the same rejection-state ternary cascade (sharedderiveRejectionModalPropsfollow-up in TODO).jest-transform-stubstays in devDependencies: removing it forces a lockfile re-resolve that moves Sentry onto different otel peers — deferred to a deps PR.