fix(send): keep the send framing past the amount step - #2625
Merged
Conversation
Send has no destination screens of its own — SendRouter navigates into the
withdraw routes (`/withdraw?method=crypto`, `?method=bank`), so `?method=` is
the only signal that the user framed this as a send. `/withdraw` honours it
("Send", "Amount to send") and forwards it to `/withdraw/crypto?method=crypto`,
but that page never read searchParams. The marker arrived and was dropped, so
every screen after the amount step reverted to withdraw copy: pick
Send → Exchange or Wallet, and the next screen says "You're withdrawing".
The word itself is not the bug. PeanutActionDetailsCard maps one
transactionType to one title, and WITHDRAW is genuinely both a withdrawal and
a send depending on how the user arrived — so it has already been flipped in
opposite directions twice (abd71b8 → "sending", d532b6a → "withdrawing"),
each engineer right about the flow in front of them. Flipping it a third time
would just re-break the real withdraw flow. Give the card the discriminator it
was missing instead.
isFromSendFlow is a narrow presentation flag, not a new transactionType: the
transaction really is a withdraw, and arrow-up/wallet-outline are already
correct for both framings, so only the verb branches. Same reason
PaymentSuccessView keeps isWithdrawFlow — it also suppresses the recipient
render and picks the "to" prefix, both right for a send to an address.
useSendFlowOrigin replaces four copies of the marker rule that had drifted into
three different definitions, which is how the screens came apart in the first
place. Manteca (Pix/Mercado Pago) has the same defect on its own page and is a
deliberate follow-up.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe withdrawal flow now uses ChangesSend-flow withdrawal framing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SendFlow
participant WithdrawPage
participant WithdrawView
participant PaymentSuccessView
SendFlow->>WithdrawPage: Open withdrawal with method marker
WithdrawPage->>WithdrawView: Pass isFromSendFlow
WithdrawView->>PaymentSuccessView: Continue with send-flow state
PaymentSuccessView-->>SendFlow: Render send-specific success title
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Code-analysis diffPainscore total: 7056.43 → 7064.45 (+8.02) 🆕 New findings (101)
…and 81 more. ✅ Resolved (98)
…and 78 more. 📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/`(mobile-ui)/withdraw/[country]/bank/page.tsx:
- Line 451: Update the primary CTA label in the bank page to use fromSendFlow,
so send flows display “Send” consistently with the header and action card while
other flows retain “Withdraw”.
In `@src/components/AddWithdraw/DynamicBankAccountForm.tsx`:
- Around line 107-109: The existing-account redirect in
DynamicBankAccountForm.tsx must append method=bank when framedAsSend is true. In
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx, update both the no-amount
and no-account redirects to preserve method=bank when fromSendFlow is true;
leave other redirect behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c0ba862c-08fd-4b9a-a66f-d09c542f3151
📒 Files selected for processing (17)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsxsrc/app/(mobile-ui)/withdraw/__tests__/withdraw-states.test.tsxsrc/app/(mobile-ui)/withdraw/crypto/page.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/AddWithdrawRouterView.tsxsrc/components/AddWithdraw/DynamicBankAccountForm.tsxsrc/components/Global/PeanutActionDetailsCard/__tests__/index.test.tsxsrc/components/Global/PeanutActionDetailsCard/index.tsxsrc/components/Withdraw/views/Confirm.withdraw.view.tsxsrc/components/Withdraw/views/Initial.withdraw.view.tsxsrc/features/payments/shared/components/PaymentSuccessView.tsxsrc/hooks/useSendFlowOrigin.test.tssrc/hooks/useSendFlowOrigin.tssrc/i18n/app/messages/en.jsonsrc/i18n/app/messages/es-419.jsonsrc/i18n/app/messages/pt-BR.json
CodeRabbit caught two instances of the exact seam this PR exists to close, on the bank side rather than the crypto side. The bank review CTA still read "Withdraw" while the header and amount card above it already read "Send" — the same screen disagreeing with itself. Three bank redirects also dropped ?method=bank, so the step the user was sent back to silently reverted to withdraw copy: the existing-saved-account push in DynamicBankAccountForm, and the no-amount / no-account replaces in the bank page. A fourth in the same file (the unsupported-country bounce) had the same defect and is fixed here too; that one needed the hook hoisted above the effect that uses it.
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
Author
|
@coderabbitai full review (The incremental pass reported "does not re-review already reviewed commits" and did not look at |
Contributor
|
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/AddWithdraw/AddWithdrawCountriesList.tsx`:
- Around line 57-59: Update the isBankFromSend value in AddWithdrawCountriesList
to only be true when the current flow is withdraw, matching
AddWithdrawRouterView; ensure the guarded value passed to
CountryList.enforceSupportedCountries does not apply send-only filtering to
add-money users.
In `@src/components/AddWithdraw/DynamicBankAccountForm.tsx`:
- Around line 445-446: Update the component props around isFromSendFlow so
actionDetailsProps is spread before the explicit isFromSendFlow={framedAsSend}
prop, or remove that key from actionDetailsProps, ensuring the flow-derived
value remains authoritative.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0672ec1a-64e3-444a-b8ec-8be979fe84f2
📒 Files selected for processing (17)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsxsrc/app/(mobile-ui)/withdraw/__tests__/withdraw-states.test.tsxsrc/app/(mobile-ui)/withdraw/crypto/page.tsxsrc/app/(mobile-ui)/withdraw/page.tsxsrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/AddWithdrawRouterView.tsxsrc/components/AddWithdraw/DynamicBankAccountForm.tsxsrc/components/Global/PeanutActionDetailsCard/__tests__/index.test.tsxsrc/components/Global/PeanutActionDetailsCard/index.tsxsrc/components/Withdraw/views/Confirm.withdraw.view.tsxsrc/components/Withdraw/views/Initial.withdraw.view.tsxsrc/features/payments/shared/components/PaymentSuccessView.tsxsrc/hooks/useSendFlowOrigin.test.tssrc/hooks/useSendFlowOrigin.tssrc/i18n/app/messages/en.jsonsrc/i18n/app/messages/es-419.jsonsrc/i18n/app/messages/pt-BR.json
There was a problem hiding this comment.
🟡 Changes recommended
/withdraw/crypto currently hard-codes the back/redirect marker to ?method=crypto, which can silently flip an incoming ?method=bank marker and alter back-navigation semantics in that edge path.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR fixes inconsistent “Withdraw” vs “Send” framing when users enter the withdraw routes via the Send flow by preserving and consistently reading the ?method= marker across subsequent steps, and by adding a narrow presentation flag to reframe copy without changing transaction semantics.
Changes:
- Introduces
useSendFlowOrigin()to centralize detection of send-origin via?method=bank|crypto. - Threads an
isFromSendFlowpresentation prop through withdraw recipient/confirm/success UI (includingPeanutActionDetailsCardandPaymentSuccessView) to render “Send” copy when appropriate. - Adds/updates unit tests and i18n strings to prevent regressions and ensure the marker survives key navigation hops.
File summaries
| File | Description |
|---|---|
| src/i18n/app/messages/en.json | Adds “sending” / “justSent” / “youreSending” strings used by reframed send copy. |
| src/i18n/app/messages/es-419.json | Adds corresponding send-framing strings for es-419. |
| src/i18n/app/messages/pt-BR.json | Adds corresponding send-framing strings for pt-BR. |
| src/hooks/useSendFlowOrigin.ts | New hook: single owner for determining send-origin from ?method=. |
| src/hooks/useSendFlowOrigin.test.ts | Tests the marker rule and guards against treating unknown methods as send. |
| src/features/payments/shared/components/PaymentSuccessView.tsx | Adds isFromSendFlow to reframe the withdraw success title (“just sent” vs “withdrew”). |
| src/components/Withdraw/views/Initial.withdraw.view.tsx | Adds isFromSendFlow to keep header/card copy consistent with Send framing. |
| src/components/Withdraw/views/Confirm.withdraw.view.tsx | Adds isFromSendFlow to keep header/CTA/loading copy consistent with Send framing. |
| src/components/Global/PeanutActionDetailsCard/index.tsx | Adds isFromSendFlow to disambiguate “You’re withdrawing” vs “You’re sending” for withdraw transaction types. |
| src/components/Global/PeanutActionDetailsCard/tests/index.test.tsx | New regression tests for withdraw-vs-send title branching on isFromSendFlow. |
| src/components/AddWithdraw/DynamicBankAccountForm.tsx | Preserves ?method=bank on navigation to keep downstream screens framed as Send when applicable. |
| src/components/AddWithdraw/AddWithdrawRouterView.tsx | Consolidates send-origin logic via the hook (with local flow guard). |
| src/components/AddWithdraw/AddWithdrawCountriesList.tsx | Consolidates bank-from-send detection via the hook. |
| src/app/(mobile-ui)/withdraw/page.tsx | Replaces ad-hoc marker parsing with useSendFlowOrigin() in the withdraw amount step. |
| src/app/(mobile-ui)/withdraw/crypto/page.tsx | Reads send-origin and preserves marker on back/redirect; threads isFromSendFlow into subviews/success. |
| src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx | Reads send-origin via the hook and preserves marker across redirects/CTAs/success. |
| src/app/(mobile-ui)/withdraw/tests/withdraw-states.test.tsx | Adds regression test that the ?method=crypto marker is forwarded to /withdraw/crypto. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Three follow-ups from CodeRabbit's full pass, all cases of the marker being read too eagerly rather than too little. The marker alone doesn't mean "send": the add-money flow navigates with its own ?method=bank, so AddWithdrawCountriesList — which serves both flows — needs the same `flow === 'withdraw'` guard its sibling AddWithdrawRouterView already had. (CodeRabbit's stated consequence pointed at enforceSupportedCountries, which actually lives in the sibling and was already guarded; the guard is still right here for the navigation branches.) Behaviour is unchanged from before this PR — the previous expression had the same gap. Entering as /withdraw?method=bank and then picking Crypto lands on /withdraw/crypto?method=bank, where the back target hard-coded ?method=crypto and silently rewrote the marker, changing the amount step's back behaviour. The hook now exposes the raw value so callers forward it verbatim instead of re-deriving it. actionDetailsProps is a Partial of the card's full props, so now that isFromSendFlow is one of them a caller could override the flow-guarded value. Moved after the spread to keep the derived value authoritative.
Contributor
Author
|
@coderabbitai full review |
Contributor
|
Contributor
Author
|
@coderabbitai full review |
Contributor
✅ Action performedFull review finished. |
Contributor
Agent review — advisory (T2)Verdict: approve-advisory. High-quality, well-tested, correctly-scoped fix that answers a real recurring smell the right way. No breakage, perf, or security concerns. One gate: add a task link. Risk tier
Paired PR
A. Breakage — pass (one nuance)
B. Performance — pass
C. Quality — pass (exemplary)
D. Security — pass
CI note
Task link — gate
Bottom lineThe model of how to handle a ping-ponged line — it stops the flip-war by adding the discriminator both prior authors were missing, proves it with a mutation-checked regression test, and consolidates the drifted rule into a single hook. Genuinely strong work. Automated advisory review. |
kushagrasarathe
approved these changes
Aug 6, 2026
kushagrasarathe
left a comment
kushagrasarathe
left a comment
Contributor
There was a problem hiding this comment.
Approved on behalf of @kushagrasarathe (human reviewer).
No blocking defects. Required CI gates (typecheck, unit, e2e, format, deploy) are green; the eslint failure is the same pre-existing dev debt (ReConsentModal / AdvisoryPreemptModal, both untouched here) — 0 new errors from this PR.
This is the right resolution to a line that had been flipped twice (abd71b8 ↔ d532b6a): it adds the missing isFromSendFlow discriminator instead of flipping a third time, with a mutation-verified regression test.
Follow-ups (non-blocking):
- Add a task link to the PR body.
- Optional: acknowledge that AddWithdrawCountriesList's added
&& flow === 'withdraw'guard isn't strictly zero-behaviour — it's a good change (fixes an add-money false-positive), just name it.
Full review: see the review comment on this PR.
dev landed six i18n commits, including a pass that dedups app copy across
locales. One real conflict, in es-419's peanutActionDetailsCard block: this
branch added `youreSending` while dev retranslated `youWillReceive`
("Recibirás" -> "Vas a recibir").
Resolved as a union — dev's retranslation wins for its own key, this branch's
new key is kept. Neither side loses an entry, which is the hazard git flags
here.
Everything else auto-merged, including dev's `t('confirm.sponsoredByPeanut')`
-> `tCommon('sponsoredByPeanut')` move inside Confirm.withdraw.view.tsx, which
sits next to this branch's CTA change in the same file.
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
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
Pick Send → Exchange or Wallet, enter an amount, and the next screen says "You're withdrawing".
Send has no destination screens of its own —
SendRouternavigates into the withdraw routes:?method=is therefore the only signal that the user framed this as a send./withdrawhonours it (NavHeader "Send", "Amount to send") and forwards it to/withdraw/crypto?method=crypto— but that page never calleduseSearchParams(). The marker arrived and was dropped, so every screen after the amount step reverted to withdraw copy. That's what makes it jarring: the screen before says Send, the screen after says Withdrawing.Why this isn't a one-word fix
PeanutActionDetailsCardmaps onetransactionTypeto one title, andWITHDRAWis genuinely both a withdrawal and a send depending on how the user arrived. So the line has already been flipped in opposite directions twice:abd71b882d532b6a65d532b6a65names the seam exactly — "the frame is named by user intent while the card was named by the underlying mechanism" — and then picks a side. Both engineers were right about the flow in front of them. Flipping it a third time would re-break the real withdraw flow and get flipped again. This PR adds the discriminator that was missing instead.Design notes / accepted trade-offs
isFromSendFlowis a presentation flag, not a newtransactionType. The transaction really is a withdraw; only the verb differs. A new union member would have forced matching edits ingetIcon/getAvatarIcon/getAvatarBackgroundColor/getAvatarTextColorpurely to reproduce identical visuals —arrow-upandwallet-outlineare already right for both framings. One ternary also coversWITHDRAW_BANK_ACCOUNT, which is why the bank path came nearly free.PaymentSuccessViewkeepsisWithdrawFlow={true}. Tempting to flip it, but it also suppresses the recipient render (this caller passes norecipientName, so flipping renders<AddressLink>with an undefined address) and selects the "to" prefix over "for" — both correct for a send to an address. Only the title needed reframing, soisFromSendFlowis deliberately narrow.useSendFlowOriginreplaces four copies of the marker rule that had drifted into three different definitions (['bank','crypto'].includes(m)vsm === 'bank'vsm === 'bank' && flow === 'withdraw'). That drift is how the screens came apart. Each call site keeps its own local guard, so this is a zero-behaviour-change consolidation.?method=. Otherwise back from the recipient screen lands on a bare/withdrawand the amount step silently reverts to withdraw copy — the same bug one hop earlier.Smell verdict — adds none; reveals one, deliberately not fixed here. This PR gives the
marker a single reader (
useSendFlowOrigin), but nothing owns writing it: the?method=…query string is now hand-built at six call sites (three pre-existing, threeadded here). A
sendFlowQuery()writer alongside the hook would close that, but doing itproperly means editing the three pre-existing sites too — scope creep on a copy fix. The
three new sites deliberately match the surrounding style, so this is not made worse.
Worth a small follow-up, and it would be the natural companion to the deferred Manteca work.
Risks / breaking changes
Low — copy and one optional prop only. No route, ledger, transaction, or backend behaviour is touched.
isFromSendFlowdefaults tofalseeverywhere, so every existing caller keeps today's wording. No cross-repo action needed.The one behaviour worth reviewing: back-navigation targets from
/withdraw/cryptonow carry?method=cryptowhen the user came from send.QA
/withdrawdirectly (no?method=) still reads Withdraw / You're withdrawing everywhere — this is the halfd532b6a65was protecting.Gate: prettier ✅ · typecheck ✅ · 206 suites / 2647 tests ✅ ·
next build✅ · eslint 0 new errors (5 remaining warnings are pre-existingexhaustive-depsin touched files).Tests
src/hooks/useSendFlowOrigin.test.ts— the marker rule, incl. that an unrecognisedmethod(e.g.pix) must not read as a send.src/components/Global/PeanutActionDetailsCard/__tests__/index.test.tsx— the regression guard against the ping-pong, covering bothWITHDRAWandWITHDRAW_BANK_ACCOUNTin both framings. Verified non-vacuous by mutation: removing the fix fails exactly the two send assertions.withdraw-states.test.tsx— asserts the?method=cryptomarker survives the Continue hop, which is the hop that dropped it.Out of scope
Manteca (Pix / Mercado Pago) — reached from Send → Pix / Mercado Pago and has the identical defect, but
withdraw/manteca/page.tsxnever reads?method=at all and carries ~6 hardcoded withdraw strings plus its own success screen. Deferred deliberately;useSendFlowOriginmakes it a small follow-up.Screenshots
Captured against a real local sandbox
Header reads Send; card reads You're sending. Before this PR the same screen read
Withdraw / You're withdrawing — the user had just come from a step that said "Send".
Also verified live in the same session:
/withdraw?method=crypto→ "Send" / "Amount to send"/withdraw/crypto?method=crypto, marker intact/withdraw/cryptoreturns to/withdraw?method=crypto— pre-PR this went to a bare/withdrawand flipped back to "Withdraw"/withdraw(no marker) still reads "Withdraw" — the halfd532b6a65was protectingBefore → after
Entering via Send → Exchange or Wallet (
?method=crypto) or Send → Bank (?method=bank):Initial.withdraw.view)Confirm.withdraw.view)Entering via
/withdrawdirectly (no?method=): every one of the above is unchanged. That regression is covered by unit tests and is the halfd532b6a65was protecting.Translations added for
es-419andpt-BRalongsideen.es-ARis a 2 KB partial locale that does not carry the siblingyoureWithdrawing/withdrewkeys either, so it is intentionally untouched and falls back.Summary by CodeRabbit
New Features
Bug Fixes
Tests