Merged
Conversation
KycStatusItem: check for REQUIRES_EXTRA_INFORMATION bridge rails and show "Action needed" with pending pill instead of "Verified". useSumsubKycFlow: don't short-circuit on APPROVED when a token is returned (additional-docs flow needs the SDK to open). Don't sync APPROVED status when token present to prevent useEffect from firing onKycSuccess prematurely. SumsubKycWrapper: ignore onApplicantStatusChanged events within 3s of SDK init to prevent pre-existing APPROVED status from auto-closing.
- update verification-in-progress modal title and description - use InfoCard for additional docs requirements display - add DUPLICATE_EMAIL reject label for bridge submission errors - fix country flag and region label ordering (US/EU/UK/MX) - update fallback requirement description copy
remove duplicated bridgeKycStatus websocket listeners and manual status tracking in favor of the centralized useKycStatus hook.
…dling - deduplicate BRIDGE_UNLOCK_ITEMS across regions - preserve specific error messages from backend in useCreateOnramp
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThis PR refactors KYC status handling across multiple components by replacing WebSocket-based live KYC state with dedicated hooks (useKycStatus), while simultaneously adding new Sumsub KYC status update handlers. Additionally, it includes UI text updates, label changes, timing guards for SDK initialization, and refinements to KYC flow logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested labels
🚥 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 |
Contributor
Author
|
@coderabbitai review in depth. Evaluate:
Also, write a very concise testing guide for QA |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me gather the PR details and changed files first. 🏁 Script executed: Length of output: 29700 Good, I have the full diff. Let me now inspect a few key files for deeper analysis. 🏁 Script executed: Length of output: 51202
🔴 Critical Bug —
|
| Area | Assessment |
|---|---|
| WebSocket deduplication | Removing useWebSocket + liveKycStatus state from 3 components (add-money bank page, AddWithdrawCountriesList, BankFlowManager) reduces 3 redundant WS listeners. useSumsubKycFlow already maintains its own socket, so no regression. Net perf win. |
useSumsubKycFlow additional-docs fix |
The condition !(response.data.status === 'APPROVED' && response.data.token) correctly prevents premature onKycSuccess before the SDK opens. Sound logic. |
StartVerificationModal DRY |
Extracting QR_PAYMENTS and BRIDGE_UNLOCK_ITEMS is clean. The europe/north-america consolidation to all-Bridge-rails is intentional (Bridge KYC is now unified). |
KycStatusItem – hasBridgeDocsNeeded |
Correctly derived from user?.rails, properly memoized, propagates "Action needed" / pending pill even when bridge KYC status itself is not set. Good defensive UI. |
sumsub-reject-labels.consts.ts |
New DUPLICATE_EMAIL entry is a real edge case that would previously fall through to the generic fallback. |
onSumsubKycStatusUpdate in history pages |
Consistent with the existing onKycStatusUpdate pattern; ensures Sumsub status transitions also trigger a user refetch. |
useKycStatus migration |
Using isUserKycApproved from the unified hook is cleaner than the per-component `liveKycStatus |
🧪 QA Testing Guide
1. Bank account add — KYC approved user (critical path)
- Log in as a KYC-approved user → Add Money → Bank → fill form → submit
- ✅ Expect: Bank account added successfully, navigated to withdraw page
- ❌ Bug present: Page crashes with
ReferenceError
2. Bank account add — KYC not started
- Log in as non-KYC user → Add Money → Bank → fill form → submit
- ✅ Expect: Sumsub KYC modal opens
3. Additional-docs flow (APPROVED + token)
- Trigger a flow where backend returns
status: APPROVEDwith atoken(additional documents needed) - ✅ Expect: SDK opens, NOT
onKycSuccessfired prematurely
4. Onramp bank transfer error handling
- With an invalid amount or unsupported country → trigger onramp creation
- ✅ Expect: User sees an error message (should be backend-specific, currently generic)
5. Sumsub KYC status via WebSocket in History / HomeHistory
- While KYC is in progress → open History page → complete KYC from another tab
- ✅ Expect: User data is refreshed automatically via
onSumsubKycStatusUpdate
6. KycStatusItem — bridge docs needed
- Simulate a user with a rail in
REQUIRES_EXTRA_INFORMATIONstate (Bridge provider) - ✅ Expect: "Action needed" subtitle and pending pill appear in KYC status list even without a
verificationrecord
7. StartVerificationModal — region unlock items
- Open the verification modal for Europe and North America regions
- ✅ Expect: All Bridge rails (SEPA, UK, US ACH, MX SPEI, QR Payments) shown
8. KycVerificationInProgressModal title
- Trigger the in-progress modal post-sumsub submission
- ✅ Expect: Title reads "Verification in progress" (not "Verfication")
Note: I'm updating the long-term memory — learning
ebdef8aastated that migrating Bridge-specific flows (add-money/bank,AddWithdrawCountriesList,BankFlowManager) to the unifiedisUserKycApprovedhelper was deferred. This PR completes that migration, so the learning is now outdated.
✏️ Learnings added
Learnt from: kushagrasarathe
URL: https://github.com/peanutprotocol/peanut-ui/pull/1713
Timestamp: 2026-03-04T18:32:51.616Z
Learning: In peanutprotocol/peanut-ui, Bridge-specific add-money and withdraw flows (`src/app/(mobile-ui)/add-money/[country]/bank/page.tsx`, `src/components/AddWithdraw/AddWithdrawCountriesList.tsx`, `src/components/Claim/Link/views/BankFlowManager.view.tsx`) have been migrated (PR `#1713`) to use the unified `isUserKycApproved` from `useKycStatus()` hook instead of per-component `liveKycStatus` + WebSocket state. The migration is complete; no more direct `bridgeKycStatus === 'approved'` checks in those files.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1693
File: src/app/(mobile-ui)/add-money/[country]/bank/page.tsx:206-207
Timestamp: 2026-03-02T17:52:51.879Z
Learning: In Bridge-specific add-money and withdraw flows (e.g., src/app/(mobile-ui)/add-money/[country]/bank/page.tsx), liveKycStatus from WebSocket tracks real-time bridge KYC status changes, and bridgeKycStatus is the persisted status. These flows use direct bridgeKycStatus === 'approved' checks instead of the unified isUserKycVerified helper. Migrating to the unified verifier requires careful testing of WebSocket-based real-time status tracking and is deferred to follow-up work.
🧠 Learnings used
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 852
File: src/hooks/useTransactionHistory.ts:177-239
Timestamp: 2025-05-14T21:16:06.369Z
Learning: When reviewing code, pay attention to and resolve CodeRabbit comments as they can contain useful refactoring suggestions and insights.
Learnt from: CR
Repo: peanutprotocol/peanut-ui PR: 0
File: .cursorrules:0-0
Timestamp: 2026-02-26T21:54:25.278Z
Learning: Applies to docs/PR.md : Maintain a concise docs/PR.md during PR work with: 1) Summary of changes, 2) Risks (what might break), 3) QA guidelines (what to test)
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 594
File: src/components/Request/Pay/Views/Initial.view.tsx:188-213
Timestamp: 2024-12-31T15:08:15.640Z
Learning: Always resolve coderabbit comments when reviewing code, since they can contain useful suggestions.
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1693
File: src/app/(mobile-ui)/add-money/[country]/bank/page.tsx:206-207
Timestamp: 2026-03-02T17:52:51.879Z
Learning: In Bridge-specific add-money and withdraw flows (e.g., src/app/(mobile-ui)/add-money/[country]/bank/page.tsx), liveKycStatus from WebSocket tracks real-time bridge KYC status changes, and bridgeKycStatus is the persisted status. These flows use direct bridgeKycStatus === 'approved' checks instead of the unified isUserKycVerified helper. Migrating to the unified verifier requires careful testing of WebSocket-based real-time status tracking and is deferred to follow-up work.
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1112
File: src/components/Claim/Link/views/BankFlowManager.view.tsx:336-343
Timestamp: 2025-08-20T09:08:19.266Z
Learning: In the KYC flow implementation, `setJustCompletedKyc` must be called after `await fetchUser()` in the `handleKycSuccess` callback. Setting `justCompletedKyc` before fetching the user would cause a re-fetching loop because `handleKycSuccess` is set in a useEffect inside the KYC hook, which would cause the UI flow to get stuck in one view.
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1078
File: src/hooks/useKycFlow.ts:129-141
Timestamp: 2025-08-11T10:35:02.715Z
Learning: In the KYC flow implementation in `src/hooks/useKycFlow.ts`, when Terms of Service (ToS) is accepted, there will always be a KYC link available in the `apiResponse`. The system ensures this invariant, so defensive checks for missing KYC links after ToS acceptance are unnecessary.
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 1654
File: src/components/Card/CardGeoScreen.tsx:41-45
Timestamp: 2026-02-02T06:03:48.192Z
Learning: In peanutprotocol/peanut-ui, KYC 2.0 uses the unified entry route '/profile/identity-verification'. Components (e.g., src/components/Card/CardGeoScreen.tsx) should avoid region-specific paths like '/profile/identity-verification/europe/bridge' and navigate to the unified route instead.
![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: 6
🧹 Nitpick comments (5)
src/hooks/useCreateOnramp.ts (1)
71-71: Extract duplicated error message to a constant.The generic fallback message is duplicated on lines 71 and 81. Per DRY principle, extract it to a constant.
♻️ Proposed refactor
+const DEFAULT_ONRAMP_ERROR = 'Failed to create bank transfer. Please try again or contact support.' + export const useCreateOnramp = (): UseCreateOnrampReturn => { // ... if (!response.ok) { - let errorMessage = 'Failed to create bank transfer. Please try again or contact support.' + const errorMessage = DEFAULT_ONRAMP_ERROR setError(errorMessage) throw new Error(errorMessage) } // ... } catch (err) { console.error('Error creating onramp:', err) - setError((prev) => prev ?? 'Failed to create bank transfer. Please try again or contact support.') + setError((prev) => prev ?? DEFAULT_ONRAMP_ERROR) throw err }Also applies to: 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCreateOnramp.ts` at line 71, Extract the duplicated fallback string into a single constant (e.g., DEFAULT_BANK_TRANSFER_ERROR_MESSAGE) inside the module and replace both usages at the two sites where the string is currently inlined (the assignments in useCreateOnramp.ts around the let errorMessage = 'Failed to create bank transfer...' and the second occurrence later on) with that constant; ensure the constant is exported or scoped appropriately to the file and update any tests or references that expect the literal if needed.src/components/Kyc/states/KycRequiresDocuments.tsx (1)
24-32: Avoid keying list items by display label.On Line 29,
key={label.title}can collide when labels normalize to the same title (or repeated requirements), which can cause React reconciliation glitches. Prefer a key derived from the raw requirement value (and index if needed).Suggested fix
- requirements.map((req) => { + requirements.map((req, index) => { const label = getRequirementLabel(req) return ( <InfoCard variant="info" - key={label.title} + key={`${req}-${index}`} description={label.description} title={label.title} /> ) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Kyc/states/KycRequiresDocuments.tsx` around lines 24 - 32, The list uses label.title for the React key in the requirements.map render (see requirements.map, getRequirementLabel and InfoCard), which can collide if titles duplicate; change the key to a stable unique identifier derived from the raw requirement (e.g., use a unique id or the raw req value like req.id or `${req}-${index}` if no id exists) so keys are stable and unique across renders instead of using label.title.src/components/IdentityVerification/StartVerificationModal.tsx (1)
32-45: Use region-path keyed typing for safer mapping.
Record<string, ...>won’t catch invalid or missing region keys at compile time. PreferPartial<Record<Region['path'], ...>>(or an exact union) soselectedRegion.pathstays type-aligned with the map.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/IdentityVerification/StartVerificationModal.tsx` around lines 32 - 45, The REGION_UNLOCK_ITEMS map is typed as Record<string, ...> which doesn't ensure keys match Region['path']; change its type to Partial<Record<Region['path'], Array<string | React.ReactNode>>> (or an exact union of the region path literals) and update any callers (e.g., where selectedRegion.path is used to index REGION_UNLOCK_ITEMS) so TypeScript enforces alignment between selectedRegion.path and the map keys; ensure REGION_UNLOCK_ITEMS remains the same shape (values like QR_PAYMENTS and BRIDGE_UNLOCK_ITEMS) but with the tightened type.src/app/(mobile-ui)/history/page.tsx (1)
139-147: Deduplicate the two KYC WebSocket refresh handlers.
onKycStatusUpdateandonSumsubKycStatusUpdatenow do the same refresh work. Consider extracting a shared callback to avoid drift.Suggested refactor
+ const handleKycRealtimeUpdate = async (source: 'Bridge' | 'Sumsub', status: string) => { + console.log(`${source} KYC status updated via WebSocket:`, status) + await fetchUser() + } ... - onKycStatusUpdate: async (newStatus: string) => { - // refetch user data when kyc status changes so the status item appears immediately - console.log('KYC status updated via WebSocket:', newStatus) - await fetchUser() - }, - onSumsubKycStatusUpdate: async (newStatus: string) => { - console.log('Sumsub KYC status updated via WebSocket:', newStatus) - await fetchUser() - }, + onKycStatusUpdate: async (newStatus: string) => handleKycRealtimeUpdate('Bridge', newStatus), + onSumsubKycStatusUpdate: async (newStatus: string) => handleKycRealtimeUpdate('Sumsub', newStatus),As per coding guidelines: Follow DRY (Don't Repeat Yourself) - reuse existing code and abstract shared functionality; use shared consts from src/constants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(mobile-ui)/history/page.tsx around lines 139 - 147, Both onKycStatusUpdate and onSumsubKycStatusUpdate duplicate the same refresh logic; extract a single handler (e.g., handleKycStatusUpdate) that logs the incoming status and calls fetchUser, then replace both onKycStatusUpdate and onSumsubKycStatusUpdate with references to that shared function; ensure you import/use any shared status constants from src/constants if appropriate and keep the logging message consistent (include the newStatus) to avoid drift.src/components/Kyc/SumsubKycWrapper.tsx (1)
110-113: Extract the 3s status guard to a named constant.The new guard is good, but
3000is a magic number in the event filter path. Naming it will make intent and future tuning clearer.Suggested refactor
+const SUMSUB_EARLY_STATUS_GRACE_MS = 3000 ... - if (Date.now() - sdkInitTime < 3000) { + if (Date.now() - sdkInitTime < SUMSUB_EARLY_STATUS_GRACE_MS) {As per coding guidelines: Follow DRY (Don't Repeat Yourself) - reuse existing code and abstract shared functionality; use shared consts from src/constants.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Kyc/SumsubKycWrapper.tsx` around lines 110 - 113, Replace the magic number 3000 in the event filter within SumsubKycWrapper.tsx (the guard that compares Date.now() - sdkInitTime < 3000 inside the onApplicantStatusChanged/SDK init path) with a named constant (e.g. SUMSUB_STATUS_GUARD_MS) defined in your shared constants module (src/constants); add an import for that constant at the top of the file and use it in the conditional so the guard reads Date.now() - sdkInitTime < SUMSUB_STATUS_GUARD_MS, keeping intent explicit and reusable for future tuning.
🤖 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`:
- Line 67: The code calls await fetchUser() but never assigns its result and
continues to rely on the stale hook values from useKycStatus(); fix by assigning
the refetch result to a local freshUser (e.g., const freshUser = await
fetchUser()) and derive the Bridge KYC flags from that freshUser (or re-evaluate
KYC using the freshUser data) instead of using the previously-captured
isUserKycApproved/isUserBridgeKycUnderReview from useKycStatus(); update the
conditional branches that reference isUserKycApproved and
isUserBridgeKycUnderReview (lines using freshUser?.accounts and later checks) to
use the refreshed values so decisions are based on the newly fetched user state
in AddWithdrawCountriesList.
In `@src/components/IdentityVerification/StartVerificationModal.tsx`:
- Around line 40-45: The region-to-feature mapping in StartVerificationModal.tsx
incorrectly reuses BRIDGE_UNLOCK_ITEMS for 'europe' and 'north-america' and only
exposes QR_PAYMENTS for 'rest-of-the-world', causing mismatches with the modal
copy; fix by creating and using region-appropriate feature arrays (e.g.,
EUROPE_UNLOCK_ITEMS, NORTH_AMERICA_UNLOCK_ITEMS, REST_OF_WORLD_ITEMS or
country-specific lists like ARGENTINA_PAYMENTS/BRAZIL_PAYMENTS) instead of
reusing BRIDGE_UNLOCK_ITEMS/QR_PAYMENTS, or alternatively update the modal copy
to state these are platform-wide payment methods; update the mapping keys
'europe', 'north-america', and 'rest-of-the-world' in the modal to reference the
new constants and ensure QR_PAYMENTS is only included where country-specific
support exists.
In `@src/components/Kyc/KycStatusItem.tsx`:
- Around line 75-81: The computed hasBridgeDocsNeeded currently scans global
user?.rails and can flag BRIDGE docs-needed for the wrong KYC region; change the
predicate to only consider rails for the current KYC region by filtering by the
region identifier used in this component (e.g., compare r.rail.region or
r.region to the component's region/selectedRegion/kycRegion variable), so
hasBridgeDocsNeeded only returns true when a BRIDGE rail in the same region has
status 'REQUIRES_EXTRA_INFORMATION' and provider.code === 'BRIDGE'.
In `@src/components/Kyc/KycVerificationInProgressModal.tsx`:
- Line 76: The title prop in the KycVerificationInProgressModal component has a
typo; change the string passed to title from "Verfication in progress" to
"Verification in progress" by updating the title attribute used in the
KycVerificationInProgressModal JSX/props to the correctly spelled phrase.
In `@src/components/Kyc/states/KycRequiresDocuments.tsx`:
- Line 22: In the KycRequiresDocuments React component replace the CSS token on
the paragraph that currently uses "text-gray-1" with the repo-standard
"text-grey-1" so the secondary copy follows the project's typography/color
convention; update the JSX in the KycRequiresDocuments component (look for the
<p className="text-xs text-gray-1"> line) to use "text-grey-1" and scan the same
file for any other instances of "text-gray-1" to change them as well.
In `@src/constants/bridge-requirements.consts.ts`:
- Line 29: The fallback description in
src/constants/bridge-requirements.consts.ts currently hardcodes "identity
document" (description: 'Please resubmit your identity document,'), which can
mislead users for non-identity requirement types; update that fallback to a
neutral message such as "Please resubmit the required document." Replace the
hardcoded identity wording in the fallback branch that sets description to use a
generic phrase so unknown requirement keys (address/tax/source-of-funds) do not
prompt the wrong upload.
---
Nitpick comments:
In `@src/app/`(mobile-ui)/history/page.tsx:
- Around line 139-147: Both onKycStatusUpdate and onSumsubKycStatusUpdate
duplicate the same refresh logic; extract a single handler (e.g.,
handleKycStatusUpdate) that logs the incoming status and calls fetchUser, then
replace both onKycStatusUpdate and onSumsubKycStatusUpdate with references to
that shared function; ensure you import/use any shared status constants from
src/constants if appropriate and keep the logging message consistent (include
the newStatus) to avoid drift.
In `@src/components/IdentityVerification/StartVerificationModal.tsx`:
- Around line 32-45: The REGION_UNLOCK_ITEMS map is typed as Record<string, ...>
which doesn't ensure keys match Region['path']; change its type to
Partial<Record<Region['path'], Array<string | React.ReactNode>>> (or an exact
union of the region path literals) and update any callers (e.g., where
selectedRegion.path is used to index REGION_UNLOCK_ITEMS) so TypeScript enforces
alignment between selectedRegion.path and the map keys; ensure
REGION_UNLOCK_ITEMS remains the same shape (values like QR_PAYMENTS and
BRIDGE_UNLOCK_ITEMS) but with the tightened type.
In `@src/components/Kyc/states/KycRequiresDocuments.tsx`:
- Around line 24-32: The list uses label.title for the React key in the
requirements.map render (see requirements.map, getRequirementLabel and
InfoCard), which can collide if titles duplicate; change the key to a stable
unique identifier derived from the raw requirement (e.g., use a unique id or the
raw req value like req.id or `${req}-${index}` if no id exists) so keys are
stable and unique across renders instead of using label.title.
In `@src/components/Kyc/SumsubKycWrapper.tsx`:
- Around line 110-113: Replace the magic number 3000 in the event filter within
SumsubKycWrapper.tsx (the guard that compares Date.now() - sdkInitTime < 3000
inside the onApplicantStatusChanged/SDK init path) with a named constant (e.g.
SUMSUB_STATUS_GUARD_MS) defined in your shared constants module (src/constants);
add an import for that constant at the top of the file and use it in the
conditional so the guard reads Date.now() - sdkInitTime <
SUMSUB_STATUS_GUARD_MS, keeping intent explicit and reusable for future tuning.
In `@src/hooks/useCreateOnramp.ts`:
- Line 71: Extract the duplicated fallback string into a single constant (e.g.,
DEFAULT_BANK_TRANSFER_ERROR_MESSAGE) inside the module and replace both usages
at the two sites where the string is currently inlined (the assignments in
useCreateOnramp.ts around the let errorMessage = 'Failed to create bank
transfer...' and the second occurrence later on) with that constant; ensure the
constant is exported or scoped appropriately to the file and update any tests or
references that expect the literal if needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e67a16a-4e5b-4ea1-b024-4120d10489fd
📒 Files selected for processing (17)
src/app/(mobile-ui)/add-money/[country]/bank/page.tsxsrc/app/(mobile-ui)/history/page.tsxsrc/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/Claim/Link/views/BankFlowManager.view.tsxsrc/components/Home/HomeHistory.tsxsrc/components/IdentityVerification/StartVerificationModal.tsxsrc/components/Kyc/CountryFlagAndName.tsxsrc/components/Kyc/KycStatusItem.tsxsrc/components/Kyc/KycVerificationInProgressModal.tsxsrc/components/Kyc/SumsubKycWrapper.tsxsrc/components/Kyc/modals/KycProcessingModal.tsxsrc/components/Kyc/states/KycCompleted.tsxsrc/components/Kyc/states/KycRequiresDocuments.tsxsrc/constants/bridge-requirements.consts.tssrc/constants/sumsub-reject-labels.consts.tssrc/hooks/useCreateOnramp.tssrc/hooks/useSumsubKycFlow.ts
- remove stale freshUser reference in AddWithdrawCountriesList (bug) - fix "Verfication" typo in KycVerificationInProgressModal - revert fallback requirement description to generic copy
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/components/AddWithdraw/AddWithdrawCountriesList.tsx (2)
84-89:⚠️ Potential issue | 🟠 MajorStale KYC status check after
fetchUser().The hook value
isUserKycApprovedis captured at render time (line 67), but the check at line 88 happens afterawait fetchUser(). React hook/context values don't update mid-function execution—so even thoughfetchUser()refreshes the backend data,isUserKycApprovedstill holds the pre-fetch value.This can cause users who just completed KYC (via multi-phase flow) to incorrectly trigger the KYC initiation flow again at line 131.
Capture the return value of
fetchUser()and derive the KYC status from the fresh data:🔧 Suggested fix
- await fetchUser() + const freshUser = await fetchUser() + const isBridgeKycApproved = freshUser?.bridgeKycStatus === 'approved' // scenario (1): happy path: if the user has already completed kyc, we can add the bank account directly // email and name are now collected by sumsub — no need to check them here - if (isUserKycApproved) { - const currentAccountIds = new Set((user?.accounts ?? []).map((acc) => acc.id)) + if (isBridgeKycApproved) { + const currentAccountIds = new Set((freshUser?.accounts ?? []).map((acc) => acc.id)),
🤖 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 84 - 89, The KYC check after await fetchUser() uses the stale hook value isUserKycApproved captured at render time; instead capture the fresh user returned by fetchUser() (e.g., const fetchedUser = await fetchUser()) and derive the KYC approval from that object (use fetchedUser?.<kycFlagOrStatus> rather than isUserKycApproved), and update any subsequent uses of user (like building currentAccountIds) to use fetchedUser so the logic reflects the freshly fetched state.
131-133:⚠️ Potential issue | 🟠 MajorSame stale state issue for the negative check.
Line 131 uses the same stale
isUserKycApprovedvalue. Update to use the fresh value derived fromfetchUser():🔧 Suggested fix (after applying the fix above)
- if (!isUserKycApproved) { + if (!isBridgeKycApproved) { await sumsubFlow.handleInitiateKyc('STANDARD') },
🤖 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 131 - 133, The negative check is using a stale isUserKycApproved value; before calling sumsubFlow.handleInitiateKyc('STANDARD') re-fetch the current user state (via fetchUser()) and derive the up-to-date isUserKycApproved flag, then use that fresh value in the if condition (replace direct use of the earlier isUserKycApproved variable with the value returned from fetchUser()), keeping references to sumsubFlow.handleInitiateKyc and the same conditional logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/AddWithdraw/AddWithdrawCountriesList.tsx`:
- Around line 84-89: The KYC check after await fetchUser() uses the stale hook
value isUserKycApproved captured at render time; instead capture the fresh user
returned by fetchUser() (e.g., const fetchedUser = await fetchUser()) and derive
the KYC approval from that object (use fetchedUser?.<kycFlagOrStatus> rather
than isUserKycApproved), and update any subsequent uses of user (like building
currentAccountIds) to use fetchedUser so the logic reflects the freshly fetched
state.
- Around line 131-133: The negative check is using a stale isUserKycApproved
value; before calling sumsubFlow.handleInitiateKyc('STANDARD') re-fetch the
current user state (via fetchUser()) and derive the up-to-date isUserKycApproved
flag, then use that fresh value in the if condition (replace direct use of the
earlier isUserKycApproved variable with the value returned from fetchUser()),
keeping references to sumsubFlow.handleInitiateKyc and the same conditional
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4d4e862b-74ef-4dd1-86bd-6e4747fa4420
📒 Files selected for processing (2)
src/components/AddWithdraw/AddWithdrawCountriesList.tsxsrc/components/Kyc/KycVerificationInProgressModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Kyc/KycVerificationInProgressModal.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
KycStatusItemnow checks forREQUIRES_EXTRA_INFORMATIONBridge rails and shows "Action needed" with a pending pill instead of "Verified" when Sumsub is approved but Bridge needs more documents.useSumsubKycFlowfrom short-circuiting on APPROVED when a token is returned (additional-docs flow needs the SDK to open). Add 3-second guard inSumsubKycWrapperto ignore pre-existing APPROVED status from auto-closing the SDK.bridgeKycStatuswebsocket listeners and manual status tracking fromOnrampBankPage,AddWithdrawCountriesList, andBankFlowManagerin favor of the centralizeduseKycStatushook.BRIDGE_UNLOCK_ITEMSacross regions inStartVerificationModal.useCreateOnramp.Files changed
src/components/Kyc/KycStatusItem.tsxsrc/hooks/useSumsubKycFlow.ts,src/components/Kyc/SumsubKycWrapper.tsxKycVerificationInProgressModal.tsx,KycProcessingModal.tsx,KycCompleted.tsx,KycRequiresDocuments.tsx,CountryFlagAndName.tsxbridge-requirements.consts.ts,sumsub-reject-labels.consts.tsadd-money/.../bank/page.tsx,AddWithdrawCountriesList.tsx,BankFlowManager.view.tsxhistory/page.tsx,HomeHistory.tsxStartVerificationModal.tsx,useCreateOnramp.tsRisks
onApplicantStatusChangedcould theoretically delay a legitimate fast completion, but sumsub reviews never complete that quickly.useKycStatushook — verify the hook covers all the same status updates.Test plan
KycRequiresDocumentswith requirements list