sync: cherry-pick bridge KYC state machine onto dev#1952
Conversation
Cherry-picked from PR #1943 (merged to main). Resolved conflicts: - qr-pay: kept dev's useQrKycGate() without paymentProcessor arg (no SimpleFi) - withdraw/bank: took our useBridgeTransferReadiness gate - test: cast mock types for dev's stricter ProviderRejectionInfo
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds a memoized bridge readiness hook (useBridgeTransferReadiness) and maps gate.type to ToS/KYC flows across pages and components; it centralizes KYC into the multi‑phase Sumsub flow, simplifies BridgeTosStep, extends InitiateKycModal, replaces Crisp chat with a support-modal context, and updates tests and an animations submodule. ChangesBridge Transfer Readiness & Multi-Phase KYC Refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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
Comment |
Code-analysis diffPainscore total: 5773.3 → 5782.85 (+9.55) 🆕 New findings (112)
…and 92 more. ✅ Resolved (105)
…and 85 more. 📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Claim/Link/views/BankFlowManager.view.tsx (1)
187-189:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign this approval check with the new readiness contract.
useBridgeTransferReadiness()now treats Bridgeunder_reviewasready, but this branch still hard-requiresbridgeKycStatus === 'approved'. Users who pass the gate as ready will still fail here with"User not KYC approved"when they confirm the claim. This check needs to match the state machine, or the new gate will dead-end legitimate flows.🤖 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/Claim/Link/views/BankFlowManager.view.tsx` around lines 187 - 189, The hard check throwing "User not KYC approved" in the confirmation path (the branch that inspects userForOfframp.bridgeKycStatus) must be aligned with useBridgeTransferReadiness(); either consult the readiness result instead of directly requiring 'approved', or expand the allowed statuses to include 'under_review' so the branch matches the readiness contract. Update the code in BankFlowManager.view.tsx to replace the direct equality check on userForOfframp.bridgeKycStatus (and the throw) with a readiness check using the value returned by useBridgeTransferReadiness() or by allowing both 'approved' and 'under_review' before throwing; keep the bridgeCustomerId check as-is.src/app/(mobile-ui)/add-money/[country]/bank/page.tsx (1)
431-436:⚠️ Potential issue | 🟠 Major | ⚡ Quick winResume at the warning step after ToS, not at onramp creation.
handleAmountContinue()normally opensOnrampConfirmationModal, but the ToS completion path jumps straight tohandleWarningConfirm(). That skips the risk acknowledgment modal and theDEPOSIT_AMOUNT_ENTEREDevent for users who had to accept ToS first. After ToS succeeds, resume from the warning step instead of creating the onramp immediately.🤖 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/app/`(mobile-ui)/add-money/[country]/bank/page.tsx around lines 431 - 436, The issue is that after the ToS step completes, the code jumps directly to handleWarningConfirm(), skipping the warning step and the DEPOSIT_AMOUNT_ENTERED event normally triggered by handleAmountContinue(). To fix this, modify the onComplete handler in BridgeTosStep to call handleAmountContinue() instead of handleWarningConfirm() so that the flow properly resumes from the warning step after ToS acceptance.
🤖 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/assets/animations`:
- Line 1: The repository references a non-existent submodule commit (SHA
d475d960d16a6009c2db3c5e35ef9aa31179ebfd) on the remote
https://github.com/peanutprotocol/peanut-animations.git; locate the commit
update that introduced this gitlink, verify the intended reachable commit on
that remote (or the correct tag/branch), update the submodule reference to a
valid commit (or point it to the correct branch/tag), amend the change that set
the bad SHA so the tree contains the reachable commit, and then refresh
submodules locally (sync/init/update) before pushing the corrected commit.
In `@src/components/Claim/Link/views/BankFlowManager.view.tsx`:
- Around line 510-515: The onComplete for BridgeTosStep should not re-call the
gated callback created while gate.type === 'accept_tos'; instead, after
hideTos() either invoke an ungated helper that performs the real offramp work
(e.g., createOfframpAndClaimUngated) or wait until the refreshed gate state is
ready (check gate.type === 'ready') and then call handleCreateOfframpAndClaim;
update the BridgeTosStep onComplete to call hideTos() and then trigger the
ungated helper or a gate-ready watcher rather than directly calling
handleCreateOfframpAndClaim(localBankDetails).
- Around line 518-533: The onVerify async handler of InitiateKycModal closes the
modal using a potentially stale sumsubFlow.showWrapper value; instead remove the
in-handler close logic and add a useEffect that watches sumsubFlow.showWrapper
to call setShowKycModal(false) when showWrapper becomes true. Specifically,
update the onVerify callback in the InitiateKycModal props (where
handleInitiateKyc and handleSelfHealResubmit are invoked) to only trigger the
flows, and create a useEffect that has sumsubFlow.showWrapper in its dependency
array and closes the modal (via setShowKycModal) when that value transitions to
true. Ensure this keeps error-case behavior (do not close on error) by relying
on the actual runtime showWrapper state.
In `@src/components/Kyc/BridgeTosStep.tsx`:
- Around line 83-110: The modal is being hidden during confirmation because its
visible prop uses "!isConfirming"; change the ActionModal visibility to remain
mounted when isConfirming is true (e.g., use "visible && !showIframe" instead of
including "!isConfirming"), and update the CTAs and close behavior to reflect a
pending state: disable the primary and secondary buttons and show a loading
label when isConfirming, prevent onClose (or make it a no-op) while confirming,
and ensure handleAcceptTerms and isLoading/isConfirming drive the button
text/disabled state so users see a disabled/loading modal during
confirmBridgeTosAndAwaitRails; look for ActionModal, visible, showIframe,
isConfirming, handleAcceptTerms, and the ctas array to make these changes.
---
Outside diff comments:
In `@src/app/`(mobile-ui)/add-money/[country]/bank/page.tsx:
- Around line 431-436: The issue is that after the ToS step completes, the code
jumps directly to handleWarningConfirm(), skipping the warning step and the
DEPOSIT_AMOUNT_ENTERED event normally triggered by handleAmountContinue(). To
fix this, modify the onComplete handler in BridgeTosStep to call
handleAmountContinue() instead of handleWarningConfirm() so that the flow
properly resumes from the warning step after ToS acceptance.
In `@src/components/Claim/Link/views/BankFlowManager.view.tsx`:
- Around line 187-189: The hard check throwing "User not KYC approved" in the
confirmation path (the branch that inspects userForOfframp.bridgeKycStatus) must
be aligned with useBridgeTransferReadiness(); either consult the readiness
result instead of directly requiring 'approved', or expand the allowed statuses
to include 'under_review' so the branch matches the readiness contract. Update
the code in BankFlowManager.view.tsx to replace the direct equality check on
userForOfframp.bridgeKycStatus (and the throw) with a readiness check using the
value returned by useBridgeTransferReadiness() or by allowing both 'approved'
and 'under_review' before throwing; keep the bridgeCustomerId check as-is.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f25375aa-f677-4d62-91e8-275977580d43
📒 Files selected for processing (17)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/app/(mobile-ui)/qr-pay/page.tsxsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsxsrc/assets/animationssrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/DynamicBankAccountForm.tsxsrc/components/Claim/Link/views/BankFlowManager.view.tsxsrc/components/Home/ActivationCTAs.tsxsrc/components/Kyc/BridgeTosStep.tsxsrc/components/Kyc/InitiateKycModal.tsxsrc/components/Kyc/states/KycProviderRejection.tsxsrc/components/Profile/views/RegionsVerification.view.tsxsrc/hooks/__tests__/useBridgeTransferReadiness.test.tssrc/hooks/useBridgeTosStatus.tssrc/hooks/useBridgeTransferReadiness.tssrc/hooks/useProviderRejectionStatus.tssrc/hooks/useSumsubKycFlow.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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)/add-money/__tests__/add-money-states.test.tsx:
- Around line 135-143: Remove the duplicate jest.mock for the
useMultiPhaseKycFlow hook (the earlier mock that shadowed the later one) and
consolidate all expected properties into the single mock used for tests: update
the useMultiPhaseKycFlow mockReturnValue to include isLoading, error,
showWrapper, handleInitiateKyc, and handleSelfHealResubmit (ensure the latter
two are jest.fn() where needed) so no fields are lost or defined twice; keep
only the single jest.mock for '@/hooks/useMultiPhaseKycFlow' and ensure all
tests reference that unified mock.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 11c247fc-b8dc-4a3e-aeae-70a6585dace7
📒 Files selected for processing (2)
src/app/(mobile-ui)/add-money/__tests__/add-money-states.test.tsxsrc/app/(mobile-ui)/qr-pay/__tests__/qr-pay-states.test.tsx
✅ Files skipped from review due to trivial changes (1)
- src/app/(mobile-ui)/qr-pay/tests/qr-pay-states.test.tsx
the cherry-pick from main stripped dev-only capacitor/native routing helpers (addMoneyCountryUrl, withdrawBankUrl, rewriteMethodPath, isCapacitor workaround, query param fallbacks) because main never had native app routing work. restores them while keeping all KYC gate fixes from the hotfix intact.
d475d960d no longer exists on peanut-animations remote (force-pushed or deleted). revert to dev's working pointer 7ebbc48 (decomplexify).
Cherry-picked from PR #1943 (merged to main). No SimpleFi changes included. See #1943 for full details and testing.