Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplace multi-step Bridge ToS confirmation and manual polling with a single helper that confirms ToS and awaits rails readiness; add KycNotStarted and keep-drawer-mounted behavior; remove BridgeTosReminder; integrate BridgeTosStep into Home CTA; adjust Add/Withdraw KYC/email handling and minor API body change. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/Kyc/BridgeTosStep.tsx (1)
61-73: Consider showing loading state during ToS confirmation polling.Unlike
handleTosIframeCloseinuseMultiPhaseKycFlow.ts(which setsmodalPhase='preparing'before awaiting), this handler provides no visual feedback during the polling period after the iframe closes. The user may experience a blank or unresponsive UI for up to ~6 seconds.🔧 Suggested improvement
const handleIframeClose = useCallback( async (source?: 'manual' | 'completed' | 'tos_accepted') => { setShowIframe(false) if (source === 'tos_accepted') { + setIsLoading(true) await confirmBridgeTosAndAwaitRails(fetchUser) + setIsLoading(false) onComplete() } else { onSkip() } }, [fetchUser, onComplete, onSkip] )You'd then need to render a loading indicator when
isLoading && !showIframe && !error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Kyc/BridgeTosStep.tsx` around lines 61 - 73, The handler handleIframeClose currently hides the iframe then awaits confirmBridgeTosAndAwaitRails without setting a loading phase, causing a blank UI; before awaiting confirmBridgeTosAndAwaitRails set the modalPhase (or an isLoading flag) to the preparing/loading state (e.g., modalPhase='preparing' or setIsLoading(true)) and after the await reset it (modalPhase='' or setIsLoading(false)); also ensure the component renders a loading indicator when isLoading && !showIframe && !error so users see progress while confirmBridgeTosAndAwaitRails runs; use the existing state setters (setShowIframe, modalPhase setter or setIsLoading) and props (fetchUser, onComplete, onSkip) referenced in handleIframeClose to implement this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/Kyc/BridgeTosStep.tsx`:
- Around line 61-73: The handler handleIframeClose currently hides the iframe
then awaits confirmBridgeTosAndAwaitRails without setting a loading phase,
causing a blank UI; before awaiting confirmBridgeTosAndAwaitRails set the
modalPhase (or an isLoading flag) to the preparing/loading state (e.g.,
modalPhase='preparing' or setIsLoading(true)) and after the await reset it
(modalPhase='' or setIsLoading(false)); also ensure the component renders a
loading indicator when isLoading && !showIframe && !error so users see progress
while confirmBridgeTosAndAwaitRails runs; use the existing state setters
(setShowIframe, modalPhase setter or setIsLoading) and props (fetchUser,
onComplete, onSkip) referenced in handleIframeClose to implement this.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Kyc/BridgeTosStep.tsxsrc/components/Kyc/KycStatusDrawer.tsxsrc/hooks/useMultiPhaseKycFlow.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 the current code and only fix it if needed.
Inline comments:
In `@src/components/AddWithdraw/AddWithdrawCountriesList.tsx`:
- Around line 103-111: You fetched a fresh user snapshot (freshUser) to
determine KYC status but still build currentAccountIds from the possibly stale
context user (user?.accounts), which can pick the wrong account after an add;
change the account-diffing logic to use freshUser.accounts (or fallback to
user.accounts only if freshUser is undefined) when constructing
currentAccountIds and any subsequent account set/diff operations (see fetchUser,
freshUser, currentKycStatus, isUserKycVerified, and user?.accounts) so the
selection uses the same fresh snapshot used for KYC.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/actions/users.tssrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/DynamicBankAccountForm.tsx
💤 Files with no reviewable changes (1)
- src/components/AddWithdraw/DynamicBankAccountForm.tsx
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/Home/HomeCarouselCTA/index.tsx (1)
93-98: Consider removing redundantfetchUser()call.
BridgeTosStep.handleIframeClosealready invokesconfirmBridgeTosAndAwaitRails(fetchUser)before callingonComplete(). The additionalawait fetchUser()here results in a duplicate API request.If the intent is to ensure fresh user data post-ToS, the call inside
confirmBridgeTosAndAwaitRailsalready handles that. Removing it avoids an unnecessary network round-trip.♻️ Proposed fix
const handleTosComplete = useCallback(async () => { setShowBridgeTos(false) setCarouselCTAs((prev) => prev.filter((c) => c.id !== 'bridge-tos')) - await fetchUser() -}, [setShowBridgeTos, setCarouselCTAs, fetchUser]) +}, [setShowBridgeTos, setCarouselCTAs])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Home/HomeCarouselCTA/index.tsx` around lines 93 - 98, handleTosComplete is calling await fetchUser() redundantly because BridgeTosStep.handleIframeClose already calls confirmBridgeTosAndAwaitRails(fetchUser) which refreshes user data; remove the final await fetchUser() from handleTosComplete and keep the existing setShowBridgeTos(false) and setCarouselCTAs(...) behavior so you don't trigger a duplicate network request while preserving state updates.src/components/Kyc/states/KycNotStarted.tsx (1)
16-18: Make the primary CTA variant explicit.Please set
variant="purple"explicitly so this button remains compliant even if Button defaults change.♻️ Proposed change
- <Button className="w-full" shadowSize="4" onClick={onResume} disabled={isLoading}> + <Button variant="purple" className="w-full" shadowSize="4" onClick={onResume} disabled={isLoading}> {isLoading ? 'Loading...' : 'Continue verification'} </Button>As per coding guidelines "Primary CTA styling: use
<Button variant='purple' shadowSize='4' className='w-full'>without size prop".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Kyc/states/KycNotStarted.tsx` around lines 16 - 18, The primary CTA Button in KycNotStarted.tsx should explicitly set the variant to "purple" to prevent style regressions; update the Button element (the one using onResume and isLoading) to include variant="purple" and ensure it does not include a size prop, keeping shadowSize="4" and className="w-full" as-is so the CTA matches the Primary CTA styling guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/Home/HomeCarouselCTA/index.tsx`:
- Around line 93-98: handleTosComplete is calling await fetchUser() redundantly
because BridgeTosStep.handleIframeClose already calls
confirmBridgeTosAndAwaitRails(fetchUser) which refreshes user data; remove the
final await fetchUser() from handleTosComplete and keep the existing
setShowBridgeTos(false) and setCarouselCTAs(...) behavior so you don't trigger a
duplicate network request while preserving state updates.
In `@src/components/Kyc/states/KycNotStarted.tsx`:
- Around line 16-18: The primary CTA Button in KycNotStarted.tsx should
explicitly set the variant to "purple" to prevent style regressions; update the
Button element (the one using onResume and isLoading) to include
variant="purple" and ensure it does not include a size prop, keeping
shadowSize="4" and className="w-full" as-is so the CTA matches the Primary CTA
styling guideline.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/Home/HomeCarouselCTA/index.tsxsrc/components/Home/HomeHistory.tsxsrc/components/Kyc/BridgeTosReminder.tsxsrc/components/Kyc/KycStatusDrawer.tsxsrc/components/Kyc/KycStatusItem.tsxsrc/components/Kyc/states/KycNotStarted.tsxsrc/hooks/useHomeCarouselCTAs.tsx
💤 Files with no reviewable changes (2)
- src/components/Home/HomeHistory.tsx
- src/components/Kyc/BridgeTosReminder.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(mobile-ui)/add-money/[country]/bank/page.tsx:
- Around line 389-397: The modal is being closed before awaiting
sumsubFlow.handleInitiateKyc, hiding the isLoading state; change the onVerify
handler in InitiateKycModal so it calls await
sumsubFlow.handleInitiateKyc('STANDARD') first and only calls
setShowKycModal(false) after the promise resolves (or in a success
branch/finally if you want to close on both success/failure). Ensure the handler
still preserves sumsubFlow.isLoading as the source of truth for the modal
loading UI (use InitiateKycModal's isLoading prop) and reference the existing
InitiateKycModal, setShowKycModal, and sumsubFlow.handleInitiateKyc symbols when
making the change.
- Around line 157-163: The useEffect that bootstraps the step is blocked by the
"user === null" guard—remove that guard so the effect sets setUrlState({ step:
'inputAmount' }) whenever urlState.step is falsy; i.e., in the useEffect (the
hook bootstrapping step in page.tsx) keep the early return for if
(urlState.step) but delete or stop checking user === null so the effect can run
and avoid the loader deadlock.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/components/Kyc/KycStatusDrawer.tsxsrc/components/Kyc/KycStatusItem.tsxsrc/components/Kyc/states/KycCompleted.tsx
💤 Files with no reviewable changes (1)
- src/components/Kyc/states/KycCompleted.tsx
…fied Added isUserKycVerified utility in kyc.consts.ts that checks all KYC providers (Bridge, Manteca, Sumsub) instead of only bridge status. Replaced direct bridgeKycStatus === 'approved' checks across 12 files to support KYC 2.0 where users may be verified through any provider.
Replaced legacy isUserMantecaKycApproved with hasMantecaLimits from useLimits hook in BridgeLimitsView and LimitsPageView. Limits now derive from the rails-based /users/limits endpoint instead of direct KYC status checks.
Regions now require provider rails to be in a functional state (ENABLED/REQUIRES_INFORMATION/REQUIRES_EXTRA_INFORMATION) before showing as unlocked. Prevents showing EU/US/MX as unlocked when Bridge submission failed. Also optimized isBridgeSupportedCountry to use O(1) lookups instead of allocating arrays per call.
- Fixed SDK unresponsiveness by closing drawer before opening Sumsub SDK (drawer z-50 was blocking SDK Dialog.Panel at z-10) - Extracted closeAndStartKyc helper to deduplicate 3 close+start patterns - Aligned status labels between activity list and drawer (Processing, Completed, Failed) - Fixed rejected state: hide RejectLabelsList in terminal rejection, fixed bottom border when Rejected On is the only card row - Extracted KycFailedContent shared component for drawer and modal - Renamed KycRejectedModal to KycFailedModal for naming consistency
…ages Added 50+ reject labels from Sumsub docs covering data/address issues, document quality, selfie/liveness, fraud/forgery, and compliance cases. Previously only 14 labels were mapped, causing most rejections to show a generic fallback. Also expanded TERMINAL_REJECT_LABELS from 3 to 15 entries for accurate terminal rejection detection.
# Conflicts: # src/app/(mobile-ui)/points/invites/page.tsx # src/app/(mobile-ui)/points/page.tsx
Contributor
Author
|
@coderabbitai review in depth. Evaluate:
Also, write a very concise testing guide for QA |
Contributor
|
🚨 Risk Assessment: HIGH-MEDIUMCritical Risk Areas1. ToS Polling Logic (useMultiPhaseKycFlow.ts) export async function confirmBridgeTosAndAwaitRails(fetchUser: () => Promise<any>) {
// Hard-coded delays and retries
await new Promise((resolve) => setTimeout(resolve, 2000))
for (let i = 0; i < 3; i++) {
// Polls only 3 times with 2s intervals
2. Email Collection Removal
3. State Synchronization Issues // KycStatusItem.tsx
const [keepDrawerMounted, setKeepDrawerMounted] = useState(false)
// Drawer renders when isDrawerOpen OR keepDrawerMounted
{(isDrawerOpen || keepDrawerMounted) && (
4. Modal Timing Race Conditions // Before: close then initiate
setShowKycModal(false)
await sumsubFlow.handleInitiateKyc('LATAM')
// After: initiate then close
await sumsubFlow.handleInitiateKyc('LATAM')
setShowKycModal(false)
5. Breaking Navigation Pattern
⚡ Performance & Latency Impact: MEDIUMAdded Latency
Optimization Concerns
🎨 Code Quality & DRY: MIXED✅ Improvements
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Kyc/BridgeTosStep.tsx (1)
61-73:⚠️ Potential issue | 🟡 MinorAdd error handling for ToS confirmation failure.
If
confirmBridgeTosAndAwaitRailsthrows, the iframe is already closed butonComplete()won't be called. The user could be left in an inconsistent state where the modal dismissed but the flow didn't complete. Consider wrapping in try/catch to ensure eitheronComplete()oronSkip()is called.🛡️ Proposed fix
const handleIframeClose = useCallback( async (source?: 'manual' | 'completed' | 'tos_accepted') => { setShowIframe(false) if (source === 'tos_accepted') { - await confirmBridgeTosAndAwaitRails(fetchUser) - onComplete() + try { + await confirmBridgeTosAndAwaitRails(fetchUser) + onComplete() + } catch (error) { + console.error('Failed to confirm ToS:', error) + // Still complete the flow - the ToS was accepted in the iframe, + // backend will eventually sync via rail status tracking + onComplete() + } } else { onSkip() } }, [fetchUser, onComplete, onSkip] )Based on learnings: "The phase-transition effect (which monitors rail status via useRailStatusTracking) serves as the actual validation mechanism, making the flow resilient to transient confirmation API failures while still ensuring correct state progression."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Kyc/BridgeTosStep.tsx` around lines 61 - 73, The handleIframeClose callback should guard the confirmBridgeTosAndAwaitRails call so failures don't leave the UI inconsistent: wrap the await confirmBridgeTosAndAwaitRails(fetchUser) call in try/catch inside handleIframeClose (keep setShowIframe(false) before the try), call onComplete() when the confirmation succeeds, and call onSkip() in the catch block (optionally logging the error) to ensure one of the two callbacks always runs; reference functions/values: handleIframeClose, confirmBridgeTosAndAwaitRails, fetchUser, onComplete, onSkip, and setShowIframe.
🧹 Nitpick comments (3)
src/components/AddWithdraw/AddWithdrawCountriesList.tsx (1)
104-110: Avoid semantic collision with the sharedisUserKycVerifiedpredicate.Line 105 names a bridge-only check as
isUserKycVerified, which can drift from the shared app-level KYC predicate and confuse future changes. Prefer a bridge-specific name and nullish coalescing in Line 104.♻️ Suggested cleanup
- const currentKycStatus = freshUser?.user?.bridgeKycStatus || liveKycStatus || user?.user.bridgeKycStatus - const isUserKycVerified = currentKycStatus === 'approved' + const currentKycStatus = + freshUser?.user?.bridgeKycStatus ?? liveKycStatus ?? user?.user.bridgeKycStatus + const isBridgeKycApproved = currentKycStatus === 'approved' - if (isUserKycVerified) { + if (isBridgeKycApproved) { const currentAccountIds = new Set((freshUser?.accounts ?? user?.accounts ?? []).map((acc) => acc.id)) ... } - if (!isUserKycVerified) { + if (!isBridgeKycApproved) { await sumsubFlow.handleInitiateKyc('STANDARD') }As per coding guidelines "Follow DRY (Don't Repeat Yourself) - reuse existing code and abstract shared functionality; use shared consts from src/constants".
Also applies to: 152-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/AddWithdraw/AddWithdrawCountriesList.tsx` around lines 104 - 110, Rename the bridge-specific predicate and tighten the nullish logic: replace currentKycStatus and isUserKycVerified usage with a bridge-specific name (e.g., currentBridgeKycStatus and isBridgeKycApproved) and compute it using nullish coalescing so it reads const currentBridgeKycStatus = freshUser?.user?.bridgeKycStatus ?? liveKycStatus ?? user?.user?.bridgeKycStatus; const isBridgeKycApproved = currentBridgeKycStatus === 'approved'; update all references (including the block that uses currentAccountIds and the similar usage around the other occurrence) to use these new identifiers to avoid colliding with any shared app-level KYC predicate.src/components/Home/HomeCarouselCTA/index.tsx (1)
94-99: Avoid duplicate user refresh after Bridge ToS completion.
BridgeTosStepalready awaitsconfirmBridgeTosAndAwaitRails(fetchUser)before callingonComplete, so the extraawait fetchUser()here adds another round-trip with little upside.♻️ Suggested simplification
- const { user, fetchUser } = useAuth() + const { user } = useAuth() @@ - const handleTosComplete = useCallback(async () => { + const handleTosComplete = useCallback(() => { setShowBridgeTos(false) setCarouselCTAs((prev) => prev.filter((c) => c.id !== 'bridge-tos')) - await fetchUser() - }, [setShowBridgeTos, setCarouselCTAs, fetchUser]) + }, [setShowBridgeTos, setCarouselCTAs])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Home/HomeCarouselCTA/index.tsx` around lines 94 - 99, handleTosComplete currently calls await fetchUser() even though BridgeTosStep already awaits confirmBridgeTosAndAwaitRails(fetchUser) before invoking onComplete, causing a duplicate user refresh; remove the redundant await fetchUser() from handleTosComplete and keep only the existing state updates (setShowBridgeTos(false) and setCarouselCTAs(...)) so the user is refreshed once via confirmBridgeTosAndAwaitRails; ensure you update the handleTosComplete function signature and dependency array (useCallback) to remove the direct fetchUser dependency if no longer used.src/features/limits/views/LimitsPageView.tsx (1)
68-68: RenamehasMantecaKycto reflect limits-based semantics.At Line 68, the value represents limits availability (
hasMantecaLimits), not KYC approval. Keeping the old prop name makes downstream routing logic easier to misinterpret.♻️ Suggested rename
-interface UnlockedRegionsListProps { +interface UnlockedRegionsListProps { regions: Region[] - hasMantecaKyc: boolean + hasMantecaLimits: boolean } -const UnlockedRegionsList = ({ regions, hasMantecaKyc }: UnlockedRegionsListProps) => { +const UnlockedRegionsList = ({ regions, hasMantecaLimits }: UnlockedRegionsListProps) => { @@ - const route = getProviderRoute(region.path, hasMantecaKyc) + const route = getProviderRoute(region.path, hasMantecaLimits)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/limits/views/LimitsPageView.tsx` at line 68, The prop name hasMantecaKyc on the UnlockedRegionsList is misleading and should be renamed to hasMantecaLimits; update the usage at UnlockedRegionsList in LimitsPageView (change hasMantecaKyc to hasMantecaLimits), rename the prop in the UnlockedRegionsList component signature/props interface, and update all other references/consumers (types, tests, and any routing/logic that reads this prop) to use hasMantecaLimits so the semantics clearly reflect limits availability rather than KYC.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(mobile-ui)/add-money/[country]/bank/page.tsx:
- Around line 206-207: Replace the ad-hoc KYC check that sets currentKycStatus
and computes isUserKycVerified via direct equality against 'approved' (using
liveKycStatus and user?.user.bridgeKycStatus) with the project's shared KYC
verifier: import the exported verifier from the shared KYC module and call it
(passing kycVerifications plus the existing liveKycStatus/bridgeKycStatus
inputs) to compute isUserKycVerified, removing the manual currentKycStatus
equality check and relying on the canonical helper instead.
In `@src/components/Kyc/KycStatusDrawer.tsx`:
- Around line 63-71: The closeAndStartKyc callback currently calls
onKeepMounted?.(true) then awaits sumsubFlow.handleInitiateKyc, which can leave
the component stuck mounted if that promise rejects; modify closeAndStartKyc to
wrap the await in try/catch/finally: in try await
sumsubFlow.handleInitiateKyc(regionIntent, levelName), in catch log/handle the
error (or rethrow) and ensure any error path triggers onKeepMounted?.(false) if
the flow did not complete, and in finally ensure proper state cleanup so
handleFlowDone's cleanup is not the only path that calls onKeepMounted?.(false);
reference closeAndStartKyc, sumsubFlow.handleInitiateKyc, onKeepMounted,
onClose, and handleFlowDone when making the change.
In `@src/constants/kyc.consts.ts`:
- Around line 49-50: Replace the hard-coded string check for bridgeKycStatus
with the shared matcher: call isKycStatusApproved(user.bridgeKycStatus) instead
of comparing to 'approved', and keep the existing fallback that returns
user.kycVerifications?.some((v) => isKycStatusApproved(v.status)) ?? false so
both bridgeKycStatus and KYC verification entries use the same approval logic.
In `@src/hooks/useDetermineBankClaimType.ts`:
- Around line 29-32: The useEffect inside useDetermineBankClaimType references
isUserKycApproved (used in determineBankClaimType) but does not include it in
the effect dependency array, causing claimType to become stale; update the
dependency list for that useEffect to include isUserKycApproved so the effect
reruns when the current user's KYC status changes (locate the
determineBankClaimType async function and the useEffect that sets claimType and
add isUserKycApproved to its dependencies).
In `@src/hooks/useDetermineBankRequestType.ts`:
- Around line 29-31: The useEffect that declares determineBankRequestType reads
isUserKycApproved but does not include it in its dependency array, causing
requestType to become stale when KYC changes; update the dependency array of the
useEffect that contains determineBankRequestType to include isUserKycApproved so
the effect re-runs on KYC state changes (locate the effect using the useEffect
declaration and the determineBankRequestType async function, and ensure
requestType is recalculated when isUserKycApproved changes).
In `@src/hooks/useMultiPhaseKycFlow.ts`:
- Around line 15-30: The exported helper confirmBridgeTosAndAwaitRails in the
hook contains hard-coded polling values; move this function into a new utility
module (e.g., utils file) and stop exporting it from the hook; update all call
sites to import confirmBridgeTosAndAwaitRails from the new utils module. Replace
the literal 2000 and 3 with shared constants (e.g., POLL_INTERVAL_MS and
POLL_RETRIES) imported from the shared constants module, and keep the existing
confirmBridgeTos and fetchUser references unchanged inside the function. Ensure
the original hook no longer exports the helper and that any tests or usages are
updated to import the utility from its new location.
---
Outside diff comments:
In `@src/components/Kyc/BridgeTosStep.tsx`:
- Around line 61-73: The handleIframeClose callback should guard the
confirmBridgeTosAndAwaitRails call so failures don't leave the UI inconsistent:
wrap the await confirmBridgeTosAndAwaitRails(fetchUser) call in try/catch inside
handleIframeClose (keep setShowIframe(false) before the try), call onComplete()
when the confirmation succeeds, and call onSkip() in the catch block (optionally
logging the error) to ensure one of the two callbacks always runs; reference
functions/values: handleIframeClose, confirmBridgeTosAndAwaitRails, fetchUser,
onComplete, onSkip, and setShowIframe.
---
Nitpick comments:
In `@src/components/AddWithdraw/AddWithdrawCountriesList.tsx`:
- Around line 104-110: Rename the bridge-specific predicate and tighten the
nullish logic: replace currentKycStatus and isUserKycVerified usage with a
bridge-specific name (e.g., currentBridgeKycStatus and isBridgeKycApproved) and
compute it using nullish coalescing so it reads const currentBridgeKycStatus =
freshUser?.user?.bridgeKycStatus ?? liveKycStatus ??
user?.user?.bridgeKycStatus; const isBridgeKycApproved = currentBridgeKycStatus
=== 'approved'; update all references (including the block that uses
currentAccountIds and the similar usage around the other occurrence) to use
these new identifiers to avoid colliding with any shared app-level KYC
predicate.
In `@src/components/Home/HomeCarouselCTA/index.tsx`:
- Around line 94-99: handleTosComplete currently calls await fetchUser() even
though BridgeTosStep already awaits confirmBridgeTosAndAwaitRails(fetchUser)
before invoking onComplete, causing a duplicate user refresh; remove the
redundant await fetchUser() from handleTosComplete and keep only the existing
state updates (setShowBridgeTos(false) and setCarouselCTAs(...)) so the user is
refreshed once via confirmBridgeTosAndAwaitRails; ensure you update the
handleTosComplete function signature and dependency array (useCallback) to
remove the direct fetchUser dependency if no longer used.
In `@src/features/limits/views/LimitsPageView.tsx`:
- Line 68: The prop name hasMantecaKyc on the UnlockedRegionsList is misleading
and should be renamed to hasMantecaLimits; update the usage at
UnlockedRegionsList in LimitsPageView (change hasMantecaKyc to
hasMantecaLimits), rename the prop in the UnlockedRegionsList component
signature/props interface, and update all other references/consumers (types,
tests, and any routing/logic that reads this prop) to use hasMantecaLimits so
the semantics clearly reflect limits availability rather than KYC.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/app/(mobile-ui)/points/invites/page.tsxsrc/app/(mobile-ui)/points/page.tsxsrc/app/(mobile-ui)/withdraw/manteca/page.tsxsrc/app/actions/users.tssrc/components/AddMoney/components/MantecaAddMoney.tsxsrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/AddWithdraw/DynamicBankAccountForm.tsxsrc/components/Claim/Claim.tsxsrc/components/Claim/Link/MantecaFlowManager.tsxsrc/components/Global/PostSignupActionManager/index.tsxsrc/components/Home/HomeCarouselCTA/index.tsxsrc/components/Home/HomeHistory.tsxsrc/components/Kyc/BridgeTosReminder.tsxsrc/components/Kyc/BridgeTosStep.tsxsrc/components/Kyc/CountryRegionRow.tsxsrc/components/Kyc/KycFailedContent.tsxsrc/components/Kyc/KycStatusDrawer.tsxsrc/components/Kyc/KycStatusItem.tsxsrc/components/Kyc/modals/KycFailedModal.tsxsrc/components/Kyc/states/KycCompleted.tsxsrc/components/Kyc/states/KycFailed.tsxsrc/components/Kyc/states/KycNotStarted.tsxsrc/components/Profile/components/PublicProfile.tsxsrc/components/Profile/views/ProfileEdit.view.tsxsrc/components/Profile/views/RegionsVerification.view.tsxsrc/components/Request/direct-request/views/Initial.direct.request.view.tsxsrc/components/Send/views/Contacts.view.tsxsrc/constants/kyc.consts.tssrc/constants/sumsub-reject-labels.consts.tssrc/features/limits/hooks/useLimitsValidation.tssrc/features/limits/views/BridgeLimitsView.tsxsrc/features/limits/views/LimitsPageView.tsxsrc/hooks/useDetermineBankClaimType.tssrc/hooks/useDetermineBankRequestType.tssrc/hooks/useHomeCarouselCTAs.tsxsrc/hooks/useIdentityVerification.tsxsrc/hooks/useMultiPhaseKycFlow.tssrc/utils/general.utils.ts
💤 Files with no reviewable changes (4)
- src/components/AddWithdraw/DynamicBankAccountForm.tsx
- src/components/Kyc/BridgeTosReminder.tsx
- src/components/Home/HomeHistory.tsx
- src/components/Kyc/states/KycCompleted.tsx
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
bug fixes and improvements for KYC 2.0. Addresses legacy status checks, drawer/SDK issues, rejection UX, and region unlock logic.
Legacy bridgeKycStatus migration
bridgeKycStatus === 'approved'checks across 12 files with unifiedisUserKycVerified()utility that checks all KYC providers (Bridge, Manteca, Sumsub)/users/limitsendpoint instead of direct KYC status checksRegion unlock logic
isBridgeSupportedCountryto use O(1) lookupsKYC drawer & SDK fixes
closeAndStartKychelper to deduplicate 3 close+start patternsRejectLabelsListin terminal rejection, fixed bottom border when "Rejected on" is the only card rowDRY refactor
KycFailedContentshared component used by both drawer state and modalKycRejectedModal→KycFailedModalfor naming consistencySumsub reject labels
REJECT_LABEL_MAPfrom 14 to 65+ entries covering all Sumsub reject/resubmission labelsTERMINAL_REJECT_LABELSfrom 3 to 15 entriesOther fixes
Files changed (39 files)
bridgeKycStatus→isUserKycVerified()isUserMantecaKycApproved→hasMantecaLimitshasProviderAccess()check, O(1) country lookupTest plan