fix: reset the withdraw flow when a send method click enters it - #2627
Conversation
Browser back from /withdraw?method=crypto never runs the in-app NavHeader reset, so selectedMethod survives in the app-wide WithdrawFlowContext. The next Bank click then skips method selection, lands on the crypto amount step, and Continue routes to /withdraw/crypto?method=bank — the crypto flow wearing a bank marker. The mirror direction (stale bank method hijacking an Exchange or Wallet click) breaks the same way. A fresh method click is a fresh statement of intent, and SendRouterView's handleMethodClick is the only place the ?method= marker is minted — so clear abandoned flow state there, with the existing resetWithdrawFlow(), before navigating. Inside /withdraw, selectedMethod stays the routing source of truth (7edcf09's invariant is untouched).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSend method selection now resets withdrawal flow state before navigating to bank, crypto, Mercado Pago, or Pix methods. The reset also closes the compatibility modal. Contacts navigation does not reset withdrawal state. Tests verify the behavior and call ordering. ChangesSend withdrawal flow reset
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Code-analysis diffPainscore total: 6345.36 → 6346 (+0.64) 🆕 New findings (12)
✅ Resolved (11)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Send/__tests__/send-states.test.tsx (1)
325-355: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover every withdrawal method and verify call order.
This block checks reset-before-navigation ordering only for
bank. The crypto test checks both calls but not their order, and no test coversmercadopagoorpix, which have separate branches insrc/components/Send/views/SendRouter.view.tsx. Add parameterized cases for all four withdrawal methods with the expected route and the sameinvocationCallOrderassertion.🤖 Prompt for 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. In `@src/components/Send/__tests__/send-states.test.tsx` around lines 325 - 355, Expand the withdrawal navigation tests around the existing bank and exchange-or-wallet cases into parameterized coverage for bank, crypto, mercadopago, and pix, using each action card’s expected route. For every case, assert resetWithdrawFlow is called once, router push receives the expected route, and resetWithdrawFlow occurs before navigation; preserve the Peanut contacts test without reset behavior.
🤖 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.
Nitpick comments:
In `@src/components/Send/__tests__/send-states.test.tsx`:
- Around line 325-355: Expand the withdrawal navigation tests around the
existing bank and exchange-or-wallet cases into parameterized coverage for bank,
crypto, mercadopago, and pix, using each action card’s expected route. For every
case, assert resetWithdrawFlow is called once, router push receives the expected
route, and resetWithdrawFlow occurs before navigation; preserve the Peanut
contacts test without reset behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e50a357-c304-4e4d-970f-84d6e06033a7
📒 Files selected for processing (2)
src/components/Send/__tests__/send-states.test.tsxsrc/components/Send/views/SendRouter.view.tsx
Adversarial mutation pass showed a partial revert of the mercadopago/pix reset calls shipped undetected (the harness only rendered bank + exchange cards), and the exchange test survived a reset-after-push reorder. Close both holes.
Adversarial pass found the reset misses showCompatibilityModal: browser-back with the modal open leaves it armed, so the next /withdraw/crypto entry pops it immediately over a null withdrawData. Pre-existing gap in every reset call site (home mount, crypto unmount); this PR leans on the function harder, so make it actually reset the flow.
Closes the CodeRabbit nitpick: every withdraw-bound method now pins both the reset call and its ordering against navigation.
|
@coderabbitai full review The single nitpick (method coverage + call order) is addressed: 7e73e9f added the Mercado Pago / Pix reset test + exchange ordering, 1aee6e7 added ordering assertions to both manteca clicks. Every withdraw-bound method now pins reset + route + reset-before-push ordering; the contacts test pins no-reset. Also since your review: 8e5347a makes resetWithdrawFlow close showCompatibilityModal (adversarial-pass finding). |
|
|
|
@coderabbitai do the review now bro, 11 minutes passed |
|
🐇🔍 ✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Send/__tests__/send-states.test.tsx (1)
172-176: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd provider-level coverage for
showCompatibilityModalclosure inresetWithdrawFlow.The mock at line 172 replaces
useWithdrawFlow, so the route tests verify only mock calls and order. They cannot detect a regression inresetWithdrawFlowinternals, including thesetShowCompatibilityModal(false)call on line 106 ofWithdrawFlowContext.tsx. The existingAddWithdrawRouterView.test.tsxuses the real context provider but does not test the reset logic. Add a provider-level test that opens the modal (by simulating the charge-setup flow that callssetShowCompatibilityModal(true)at line 258 ofcrypto/page.tsx), invokesresetWithdrawFlow, and asserts that the modal closes.🤖 Prompt for 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. In `@src/components/Send/__tests__/send-states.test.tsx` around lines 172 - 176, Add a provider-level test using the real WithdrawFlowContext provider rather than the mocked useWithdrawFlow in send-states.test.tsx. Simulate the charge-setup flow that triggers setShowCompatibilityModal(true), invoke resetWithdrawFlow, and assert that the compatibility modal closes, covering the reset logic in WithdrawFlowContext and its showCompatibilityModal state.
🤖 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.
Nitpick comments:
In `@src/components/Send/__tests__/send-states.test.tsx`:
- Around line 172-176: Add a provider-level test using the real
WithdrawFlowContext provider rather than the mocked useWithdrawFlow in
send-states.test.tsx. Simulate the charge-setup flow that triggers
setShowCompatibilityModal(true), invoke resetWithdrawFlow, and assert that the
compatibility modal closes, covering the reset logic in WithdrawFlowContext and
its showCompatibilityModal state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a97c04c-1d6f-49b0-92fd-463e2b20be97
📒 Files selected for processing (3)
src/components/Send/__tests__/send-states.test.tsxsrc/components/Send/views/SendRouter.view.tsxsrc/context/WithdrawFlowContext.tsx
Both send-states and withdraw-states mock the context, so nothing verified the actual reset semantics — the CodeRabbit r2 nitpick and the adversarial panel's accepted gap. One probe test against the real provider pins every cleared field, including the compatibility-modal closure this PR added.
|
@coderabbitai review r2 nitpick addressed in the latest commit: |
|
|
2 participants
Summary
Browser/device back from
/withdraw?method=cryptonever runs the in-app NavHeader reset, soselectedMethodsurvives in the app-wideWithdrawFlowContext. The next Bank click on/sendthen skips method selection, lands on the crypto amount step, and Continue routes to/withdraw/crypto?method=bank— the crypto flow wearing a bank marker. The mirror direction (stale bank method hijacking an Exchange or Wallet click) breaks the same way.Fix: a fresh method click is a fresh statement of intent.
SendRouterView.handleMethodClickis the only click-site that mints the?method=marker, so call the pre-existingresetWithdrawFlow()there before navigating, in all four withdraw-bound cases (bank,exchange-or-wallet,mercadopago,pix). Plus one completeness fix the adversarial pass surfaced:resetWithdrawFlow()now also closesshowCompatibilityModal, which every reset call site (home mount, crypto-page unmount, these clicks) was leaving armed.withdraw/page.tsxis untouched: inside the withdraw flow,selectedMethodstays the routing source of truth (the7edcf0919invariant and its pinned test survive unmodified) — we only guarantee the context is fresh at entry.Task
TASK-21203 — https://app.notion.com/p/3b483811757981418f7ff0efe4d33221
Verification (3-agent adversarial panel, all lenses traced to code)
startTransition-wrapped route render, the provider never remounts, and the step-sync effect self-heals as a third safety net. Mirror direction traced clean.rawTokenAmountis component-local; context amount is only written on Continue). The manteca page reads zero context fields and self-resets on mount./homehas always reset the flow on mount, so "state survives leaving the flow" was never a supported invariant, and every re-entry path guards against cleared context.Design notes / accepted trade-offs
/withdraw: the within-flow back-navigations that echo?method=(AddWithdrawCountriesList) want state preserved and are correctly untouched. Browser-forward cannot recreate a mismatch (a fresh click truncates forward history)./withdrawentries (home tile, bottom nav, KYC modal, exchange-rate widget) still resume a staleselectedMethodat the amount step. That path is internally consistent (no marker/context mismatch — Continue routes where the UI says) and matches the codebase's blessed resume semantics.?method=URL marker andWithdrawFlowContextremain two sources of truth — this is the third bug from that split.Risks / breaking changes
main→ back-merge debt main→dev after merge. No conflict expected:handleMethodClickis byte-identical onmainanddev, and open PR fix(send): keep the send framing past the amount step #2625 does not touchSendRouter.view.tsx.QA
/send→ Exchange or Wallet → browser back (swipe/hardware/browser — not the in-app‹) → Bank → must land on bank method selection, not the crypto amount step./send→ Bank → pick a saved account → browser back → Exchange or Wallet → must land on the crypto amount step cleanly.‹back paths unchanged. Plain/withdraw(no marker) unchanged./withdraw/crypto?method=bankcan never occur — it is legitimately mintable (send → bank → "show all methods" → Crypto card → Continue).Screenshots:⚠️ NONE — the change is which pre-existing screen appears after a click given abandoned state; both screens are unchanged pixel-wise. Manual repro steps above are the visual evidence.
Summary by CodeRabbit