sync: merge main into dev — bridge KYC state machine#1949
Closed
kushagrasarathe wants to merge 19 commits into
Closed
sync: merge main into dev — bridge KYC state machine#1949kushagrasarathe wants to merge 19 commits into
kushagrasarathe wants to merge 19 commits into
Conversation
…andling
the bridge KYC state handling in transfer flows (withdraw, deposit, claim, qr-pay)
had multiple issues: wrong priority order, missing ToS checks, redirects instead of
inline flows, and broken rejection handling.
changes:
- new `useBridgeTransferReadiness` hook: unified pre-transfer gate with correct
priority (hard rejection → tos → fixable rejection → enrollment → ready)
- `useBridgeTosStatus`: expanded to check `tos_acceptance`/`tos_v2_acceptance` in
rail `additionalRequirements` metadata, not just `REQUIRES_INFORMATION` status
- `InitiateKycModal`: added `blocked` variant (contact support CTA), `error` prop
for failed self-heal attempts
- withdraw/bank, add-money/bank, AddWithdrawCountriesList: replaced fragmented
`needsBridgeEnrollment` + `guardWithTos` + `bridgeRejection` checks with unified gate
- BankFlowManager (claim flow): added ToS guard + rejection handling for non-guest paths
- qr-pay: replaced redirect-based KYC (`router.push('/profile/identity-verification')`)
with inline sumsub SDK initiation (`handleInitiateKyc('LATAM')`)
- BridgeTosStep: show confirmation modal before iframe, prevent modal flash during
confirmation with `isConfirming` state
- KycProviderRejection, ActivationCTAs, RegionsVerification: replaced broken
`$crisp.push` calls with `setIsSupportModalOpen`
- useSumsubKycFlow: fixed race condition where `fetchCurrentStatus` closes SDK wrapper
on first attempt by guarding with `showWrapperRef`
- useProviderRejectionStatus: permanent rejections now show generic support message
instead of misleading "upload a clearer photo"
- DynamicBankAccountForm: `__silent__` sentinel to prevent form navigation when gate blocks
- close kyc modal via `sumsubFlow.showWrapper` effect instead of in `onVerify` callback
…ata cast, gate fallthrough - BridgeTosStep: wrap confirmBridgeTosAndAwaitRails in try/catch to prevent isConfirming getting stuck on error - useBridgeTosStatus: use Array.isArray guard for additionalRequirements metadata - all gate checks: always return when gate is not ready, even if guardWithTos returns false (defensive)
…lt, dedup variant mapping
- useSumsubKycFlow: save/restore prevStatusRef on crossRegion failure to prevent
suppressing subsequent legitimate APPROVED transitions
- DynamicBankAccountForm: replace __silent__ magic string with typed { silent: true }
- useBridgeTransferReadiness: extract getKycModalVariant() and getGateProviderMessage()
helpers to deduplicate gate→modal mapping across 4 files
- BankFlowManager: only close kyc modal if sdk opened (check showWrapper), preserving
error visibility on failure
…minate SDK auto-close race the fetchCurrentStatus effect could still race with handleInitiateKyc in rare cases — initiatingRef resets in the finally block before React batches the showWrapper state update. adding userInitiatedRef as a guard permanently disables the background fetch after any user-initiated flow, since the SDK and websocket handle status updates from that point.
Users paying with a Pix QR were told to ask the merchant for a Mercado Pago QR, which is wrong and confusing. Branch the message on qrType so Pix scans get Pix-specific guidance. Also capture qr_decoding_error_shown so we can measure how often this fallback fires and which rails are most affected.
Per CodeRabbit review on #1948: ARGENTINA_QR3 was hitting the default branch and getting told to ask the merchant for a Mercado Pago QR — QR3 and Mercado Pago are distinct Argentine standards, so the copy was actively misleading. Now branches PIX, ARGENTINA_QR3, and the MP default explicitly.
In Argentina, Mercado Pago is the dominant rail and a workable fallback when a QR3 code fails to decode — so suggesting "ask for a Mercado Pago QR" is the most useful guidance for QR3 users too. Only Pix needs the rail-specific copy because Brazil has no equivalent fallback. Reverts the previous QR3-specific message; adds a comment explaining the regional reasoning so future readers don't trip on the same point CodeRabbit raised.
…hine fix: bridge KYC state machine — unified gate, inline ToS, rejection handling
Signed-off-by: Juan José Ramírez <70615692+jjramirezn@users.noreply.github.com>
…rror-message [TASK-19529] fix(qr-pay): Pix-specific copy on QR decoding error + capture event
# Conflicts: # src/app/(mobile-ui)/add-money/[country]/bank/page.tsx # src/app/(mobile-ui)/qr-pay/page.tsx # src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx # src/components/AddWithdraw/AddWithdrawCountriesList.tsx # src/components/Kyc/BridgeTosStep.tsx # src/constants/analytics.consts.ts
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThis PR centralizes bridge transfer readiness into a prioritized gate, rewires KYC/TOS flows and support-modal integration across add-money, withdraw, and QR pay pages and related components, expands KYC modal variants and Sumsub flow guards, and adds SimpleFi QR payment handling. ChangesBridge Transfer Readiness & KYC Unification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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
Comment |
Contributor
Code-analysis diffPainscore total: 5774.88 → 5761.61 (-13.27) 🆕 New findings (115)
…and 95 more. ✅ Resolved (107)
…and 87 more. 📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — 🔴 12 failingSuites
🔴 Failing tests
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![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: 4
🧹 Nitpick comments (2)
src/components/Kyc/BridgeTosStep.tsx (1)
60-79: 💤 Low valueConsider retry handling for ToS confirmation failures.
When
confirmBridgeTosAndAwaitRailsfails (lines 68-69), the error is displayed but the "Try again" button (line 96-97) will callhandleAcceptTerms, which fetches the ToS link again rather than retrying the confirmation. Since the user already accepted in the iframe, they'd need to go through the iframe flow again unnecessarily.This may be acceptable as a rare edge case (network failure during confirmation), but consider whether a dedicated retry for the confirmation step would improve the UX.
🤖 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/Kyc/BridgeTosStep.tsx` around lines 60 - 79, The handler handleIframeClose currently treats a failed confirmBridgeTosAndAwaitRails as a generic error and the UI's "Try again" button calls handleAcceptTerms (which re-opens the iframe); change this by recording that the iframe closed with source 'tos_accepted' (e.g., store lastTosAccepted boolean or lastCloseSource state) and implement a retryConfirmBridgeTos function that calls confirmBridgeTosAndAwaitRails, sets setIsConfirming, and calls onComplete/onError like the original try/catch; update the "Try again" button logic to call retryConfirmBridgeTos when lastCloseSource === 'tos_accepted' (otherwise fall back to handleAcceptTerms), keeping existing state updates (setShowIframe, setError, setIsConfirming) consistent with handleIframeClose.src/hooks/__tests__/useBridgeTransferReadiness.test.ts (1)
69-73: 💤 Low valueConsider adding a test case for unapproved Sumsub user.
The current tests cover scenarios where
isSumsubApproved: true, but there's no explicit test for when a user hasn't completed Sumsub KYC at all (isSumsubApproved: false,isBridgeApproved: false, no rejections, no TOS needed). The hook would returnready, which may be intentional (letting downstream flows handle initial KYC) but worth documenting with a test.🤖 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/hooks/__tests__/useBridgeTransferReadiness.test.ts` around lines 69 - 73, Add a new unit test in useBridgeTransferReadiness.test.ts that calls the existing setup helper with isSumsubApproved: false and isBridgeApproved: false (and no rejections/TOS flags), then renders the hook via renderHook(() => useBridgeTransferReadiness()) and asserts the expected gate type (e.g., expect(result.current.gate.type).toBe('ready')) so the unapproved-Sumsub scenario is explicitly covered; reference the setup helper, renderHook, and result.current.gate.type to locate where to add the test.
🤖 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)/qr-pay/page.tsx:
- Line 125: The call to useQrKycGate in page.tsx passes paymentProcessor but the
hook's current signature takes no args; fix by making the call site and hook
signature consistent—either remove the argument from the call in page.tsx
(change const { shouldBlockPay, kycGateState } = useQrKycGate()) or update the
hook definition of useQrKycGate to accept and use the paymentProcessor
parameter; locate the hook implementation named useQrKycGate and the call site
in page.tsx to apply the matching change.
- Around line 39-46: The import failures happen because parseSimpleFiQr, the
SimpleFiQrData type, and the SIMPLEFI_* members of EQrType (used by this file's
SimpleFi branches) are not exported from the DirectSendQR utils; update that
module to re-export parseSimpleFiQr, export the SimpleFiQrData type, and
add/export the EQrType entries for SIMPLEFI_* (or export the full EQrType enum)
so callers can reference EQrType.SIMPLEFI_*. Ensure NAME_BY_QR_TYPE and QrType
remain exported and that parseSimpleFiQr signature matches the consumer usage in
page.tsx (and other locations mentioned).
- Around line 11-12: The imports simplefiApi and SimpleFiQrPaymentResponse from
'@/services/simplefi' are unresolved and break typechecking; until the SimpleFi
service/types are merged, remove the broken imports in page.tsx and replace them
with local stubs: declare a temporary SimpleFiQrPaymentResponse type/interface
matching the minimal fields used in this file and create a local simplefiApi
stub (or lazily import it) that implements only the methods this page calls;
ensure all references in the page use the local stub names so the page builds
and add a TODO comment to revert to '@/services/simplefi' once the service is
merged.
- Around line 1553-1557: The CTA is reading qrPayment!.details.merchant.name
which is null in the SimpleFi success path; update the Button
onClick/router.push call to use the SimpleFi merchant source instead by
replacing qrPayment!.details.merchant.name with the SimpleFi variables (prefer
merchantName or fallback to simpleFiQrData?.merchant?.name) so the URL uses
merchantName ?? simpleFiQrData?.merchant?.name; keep the existing
formatNumberForDisplay usage for amount and ensure the change is applied in the
Button onClick handler that builds the `/request?amount=...&merchant=...` URL.
---
Nitpick comments:
In `@src/components/Kyc/BridgeTosStep.tsx`:
- Around line 60-79: The handler handleIframeClose currently treats a failed
confirmBridgeTosAndAwaitRails as a generic error and the UI's "Try again" button
calls handleAcceptTerms (which re-opens the iframe); change this by recording
that the iframe closed with source 'tos_accepted' (e.g., store lastTosAccepted
boolean or lastCloseSource state) and implement a retryConfirmBridgeTos function
that calls confirmBridgeTosAndAwaitRails, sets setIsConfirming, and calls
onComplete/onError like the original try/catch; update the "Try again" button
logic to call retryConfirmBridgeTos when lastCloseSource === 'tos_accepted'
(otherwise fall back to handleAcceptTerms), keeping existing state updates
(setShowIframe, setError, setIsConfirming) consistent with handleIframeClose.
In `@src/hooks/__tests__/useBridgeTransferReadiness.test.ts`:
- Around line 69-73: Add a new unit test in useBridgeTransferReadiness.test.ts
that calls the existing setup helper with isSumsubApproved: false and
isBridgeApproved: false (and no rejections/TOS flags), then renders the hook via
renderHook(() => useBridgeTransferReadiness()) and asserts the expected gate
type (e.g., expect(result.current.gate.type).toBe('ready')) so the
unapproved-Sumsub scenario is explicitly covered; reference the setup helper,
renderHook, and result.current.gate.type to locate where to add the test.
🪄 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: d72d9ff3-2e39-4731-8f9a-5f8626a24fbf
📒 Files selected for processing (18)
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/constants/analytics.consts.tssrc/hooks/__tests__/useBridgeTransferReadiness.test.tssrc/hooks/useBridgeTosStatus.tssrc/hooks/useBridgeTransferReadiness.tssrc/hooks/useProviderRejectionStatus.tssrc/hooks/useSumsubKycFlow.ts
Contributor
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)/qr-pay/page.tsx:
- Around line 975-976: The current isLoadingPaymentData expression
short-circuits on !shouldBlockPay and can allow undefined currency to pass
through when shouldBlockPay is true; update the expression in page.tsx
(isLoadingPaymentData) to always treat missing currency as loading (e.g., make
the expression start with !currency || ... or otherwise include a top-level
currency check) so subsequent unguarded reads like currency.price are safe even
when pay is blocked; keep other conditions (isFirstLoad, paymentProcessor,
paymentLock) unchanged.
🪄 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: 1794ec45-ac85-401c-96d3-4343e089d863
📒 Files selected for processing (1)
src/app/(mobile-ui)/qr-pay/page.tsx
Comment on lines
+975
to
+976
| const isLoadingPaymentData = | ||
| !shouldBlockPay && (isFirstLoad || (paymentProcessor === 'MANTECA' && !paymentLock) || !currency) |
Contributor
There was a problem hiding this comment.
Prevent fallthrough render with undefined currency when pay is blocked.
At Line 975, isLoadingPaymentData short-circuits on !shouldBlockPay. If shouldBlockPay is true, the page can skip loading while currency is still unset (query is disabled at Line 366), then hit unguarded reads like currency.price in the main render path.
💡 Minimal fix
- const isLoadingPaymentData =
- !shouldBlockPay && (isFirstLoad || (paymentProcessor === 'MANTECA' && !paymentLock) || !currency)
+ const isLoadingPaymentData =
+ isFirstLoad || (paymentProcessor === 'MANTECA' && !paymentLock) || !currency📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isLoadingPaymentData = | |
| !shouldBlockPay && (isFirstLoad || (paymentProcessor === 'MANTECA' && !paymentLock) || !currency) | |
| const isLoadingPaymentData = | |
| isFirstLoad || (paymentProcessor === 'MANTECA' && !paymentLock) || !currency |
🤖 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)/qr-pay/page.tsx around lines 975 - 976, The current
isLoadingPaymentData expression short-circuits on !shouldBlockPay and can allow
undefined currency to pass through when shouldBlockPay is true; update the
expression in page.tsx (isLoadingPaymentData) to always treat missing currency
as loading (e.g., make the expression start with !currency || ... or otherwise
include a top-level currency check) so subsequent unguarded reads like
currency.price are safe even when pay is blocked; keep other conditions
(isFirstLoad, paymentProcessor, paymentLock) unchanged.
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
3 participants
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.
Syncs main into dev to deploy the bridge KYC state machine changes to staging. See #1943 for full details and testing.