-
Notifications
You must be signed in to change notification settings - Fork 13
[TASK-14061] Update claim flow redirect actions for guest users #1185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds URL-driven step tracking (step=claim|bank) with helpers to set/remove it, auto-triggers claim/bank flows from URL (including auto-claim via Peanut wallet), propagates sanitized Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Pre-merge checks (2 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/Common/ActionList.tsx (2)
142-148
: Race: redirect_uri may miss step=claim due to async URL updateaddParamStep likely updates the URL via router.replace/history; reading window.location immediately after can omit the step param. Compute redirectUri explicitly to guarantee inclusion.
Apply:
- addParamStep('claim') - // push to setup page with redirect uri, to prevent the user from losing the flow context - const redirectUri = encodeURIComponent( - window.location.pathname + window.location.search + window.location.hash - ) - router.push(`/setup?redirect_uri=${redirectUri}`) + addParamStep('claim') + // push to setup page with redirect uri, to prevent the user from losing the flow context + const url = new URL(window.location.href) + url.searchParams.set('step', 'claim') + const redirectUri = encodeURIComponent(url.pathname + url.search + url.hash) + router.push(`/setup?redirect_uri=${redirectUri}`)
72-81
: Gate bank flow on both GuestKycNeeded and ReceiverKycNeededCurrently only GuestKycNeeded triggers the verification modal; ReceiverKycNeeded should be blocked too, otherwise users without KYC can enter the bank flow.
- if (claimType === BankClaimType.GuestKycNeeded) { + if ( + claimType === BankClaimType.GuestKycNeeded || + claimType === BankClaimType.ReceiverKycNeeded + ) { addParamStep('bank') setShowVerificationModal(true) } else {src/components/Global/PostSignupActionManager/index.tsx (1)
27-43
: Validate redirect to avoid external navigationEven if we intend to store only relative URLs, add a defensive check to prevent open-redirects in case of tampering or legacy values.
- const checkClaimModalAfterKYC = () => { - const redirectUrl = getFromLocalStorage('redirect') - if (user?.user.kycStatus === 'approved' && redirectUrl) { + const checkClaimModalAfterKYC = () => { + const redirectUrl = getFromLocalStorage('redirect') + if (user?.user.kycStatus === 'approved' && redirectUrl) { + if (typeof redirectUrl !== 'string') return + // disallow external/absolute URLs + try { + const url = new URL(redirectUrl, window.location.origin) + if (url.origin !== window.location.origin) { + localStorage.removeItem('redirect') + return + } + } catch { + localStorage.removeItem('redirect') + return + } const matchedAction = POST_SIGNUP_ACTIONS.find((action) => action.pathPattern.test(redirectUrl))
🧹 Nitpick comments (8)
src/utils/general.utils.ts (1)
1216-1218
: Prefer pathname+search+hash over.replace(origin, '')
for reliability.Using
href.replace(origin, '')
works, but constructing the relative path directly is simpler and avoids edge cases where the origin appears elsewhere in the URL string.export const saveRedirectUrl = () => { const currentUrl = new URL(window.location.href) - const relativeUrl = currentUrl.href.replace(currentUrl.origin, '') + const relativeUrl = currentUrl.pathname + currentUrl.search + currentUrl.hash saveToLocalStorage('redirect', relativeUrl) }Additionally, consider centralizing redirect sanitization with a helper to ensure internal-only redirects across the app:
// utils (new helper) export const toInternalRelativeUrl = (raw: string | null): string | null => { if (!raw) return null try { const u = new URL(raw, window.location.origin) if (u.origin !== window.location.origin) return null return u.pathname + u.search + u.hash } catch { return raw.startsWith('/') ? raw : `/${raw}` } }src/components/Setup/Views/SetupPasskey.tsx (1)
25-26
: Consider includingsearchParams
in the effect’s dependency list.The effect reads
searchParams
but only depends on[address, user]
. If the URL changes post-signup, the redirect check won’t re-run.Example:
useEffect(() => { /* ... */ }, [address, user, searchParams])src/components/Claim/Claim.tsx (1)
262-269
: Effect should depend on the URL step param; make comparison resilient.The effect reads
searchParams
but only depends onuser
. Add it (or derivestepFromURL
as state) and normalize case.- useEffect(() => { - const stepFromURL = searchParams.get('step') - if (user?.user.kycStatus === 'approved' && stepFromURL === 'bank') { - setClaimBankFlowStep(ClaimBankFlowStep.BankCountryList) - } - }, [user]) + useEffect(() => { + const stepFromURL = searchParams.get('step') + if (user?.user.kycStatus === 'approved' && stepFromURL?.toLowerCase() === 'bank') { + setClaimBankFlowStep(ClaimBankFlowStep.BankCountryList) + } + }, [user, searchParams])src/components/Global/ActionModal/index.tsx (1)
43-44
: Custom content slot added — consider composition and layout guard
- The
customContent
prop works, but consider accepting children instead for standard React composition; this avoids API sprawl and keeps the modal extensible.- If you keep
customContent
, wrap it with a width/spacing container to avoid layout regressions across call sites.Example minimal guard:
- {customContent && customContent} + {customContent && <div className="w-full">{customContent}</div>}Also applies to: 68-69, 134-135
src/components/Global/GuestVerificationModal/index.tsx (1)
12-13
: Encoderedirect_uri
and skipsaveRedirectUrl
on explicit signup flow
- Serialize
redirect_uri
withURLSearchParams
/encodeURIComponent
to prevent parsing issues and match other components.- Only call
saveRedirectUrl()
in the fallback branch to avoid double-routing whenshouldShowVerificationModalOnSignup
is true.Apply:
@@ src/components/Global/GuestVerificationModal/index.tsx:38 - saveRedirectUrl() - if (shouldShowVerificationModalOnSignup) { - router.push('/setup?redirect_uri=/profile/identity-verification') + if (shouldShowVerificationModalOnSignup) { + const params = new URLSearchParams({ redirect_uri: '/profile/identity-verification' }) + router.push(`/setup?${params.toString()}`) return } + // fallback flow + saveRedirectUrl() router.push('/setup')src/components/Common/ActionList.tsx (2)
98-103
: Optional: preserve bank step for request-guest pathFor consistency with the claim-guest path, consider adding the step param before opening the guest verification modal so post-signup can resume into bank.
- if (requestType === BankRequestType.GuestKycNeeded) { - setIsGuestVerificationModalOpen(true) + if (requestType === BankRequestType.GuestKycNeeded) { + addParamStep('bank') + setIsGuestVerificationModalOpen(true)
64-66
: Naming nit: amountInUsd isn’t guaranteed USDformatUnits yields token units, not USD. Rename for clarity or convert properly upstream.
- const amountInUsd = parseFloat(formatUnits(claimLinkData.amount, claimLinkData.tokenDecimals)) - if (method.id === 'bank' && amountInUsd < 1) { + const amount = parseFloat(formatUnits(claimLinkData.amount, claimLinkData.tokenDecimals)) + if (method.id === 'bank' && amount < 1) {- const amountInUsd = usdAmount ? parseFloat(usdAmount) : 0 - if (method.id === 'bank' && amountInUsd < 1) { + const amount = usdAmount ? parseFloat(usdAmount) : 0 + if (method.id === 'bank' && amount < 1) {Also applies to: 91-93
src/components/Global/PostSignupActionManager/post-signup-action.consts.ts (1)
6-6
: Optional: tighten the path regex to avoid query-string false positives/claim/ will also match ?step=claim on non-claim pages. If claim routes always start with “/claim”, prefer a path-anchored regex.
- pathPattern: /claim/, + pathPattern: /^\/claim(\/|\?|$)/,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
src/components/Claim/Claim.tsx
(3 hunks)src/components/Claim/Link/Initial.view.tsx
(3 hunks)src/components/Claim/useClaimLink.tsx
(2 hunks)src/components/Common/ActionList.tsx
(5 hunks)src/components/Global/ActionModal/index.tsx
(3 hunks)src/components/Global/GuestLoginCta/index.tsx
(3 hunks)src/components/Global/GuestVerificationModal/index.tsx
(2 hunks)src/components/Global/PostSignupActionManager/index.tsx
(3 hunks)src/components/Global/PostSignupActionManager/post-signup-action.consts.ts
(1 hunks)src/components/Setup/Views/SetupPasskey.tsx
(3 hunks)src/components/Setup/Views/Welcome.tsx
(3 hunks)src/utils/general.utils.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-20T09:08:19.266Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#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.
Applied to files:
src/components/Global/PostSignupActionManager/index.tsx
📚 Learning: 2024-10-29T16:06:38.812Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#495
File: src/components/Create/useCreateLink.tsx:647-657
Timestamp: 2024-10-29T16:06:38.812Z
Learning: In the React code for `useCreateLink` in `src/components/Create/useCreateLink.tsx`, the `switchNetwork` function used within `useCallback` hooks is stable and does not need to be included in the dependency arrays.
Applied to files:
src/components/Claim/useClaimLink.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.
Applied to files:
src/components/Claim/Link/Initial.view.tsx
🧬 Code graph analysis (4)
src/components/Global/PostSignupActionManager/index.tsx (2)
src/context/authContext.tsx (1)
useAuth
(182-188)src/utils/general.utils.ts (1)
getFromLocalStorage
(126-148)
src/components/Claim/useClaimLink.tsx (1)
src/hooks/wallet/useWallet.ts (1)
useWallet
(21-123)
src/components/Claim/Claim.tsx (1)
src/context/ClaimBankFlowContext.tsx (1)
useClaimBankFlow
(135-141)
src/components/Global/PostSignupActionManager/post-signup-action.consts.ts (1)
src/components/Global/Icons/Icon.tsx (1)
IconName
(64-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (3)
src/components/Claim/Link/Initial.view.tsx (1)
790-797
: Good cleanup of URL state on modal closeRemoving
step
on close prevents stale URL-driven flows. Looks correct.src/components/Global/PostSignupActionManager/index.tsx (1)
45-47
: LGTM: effect gating on user + routerRunning the modal logic only when KYC is approved reduces flicker and unnecessary work.
src/components/Global/PostSignupActionManager/post-signup-action.consts.ts (1)
8-11
: Copy and icon update looks goodThe new success-oriented copy and check icon align with the KYC-complete state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/components/Claim/Link/Initial.view.tsx
(3 hunks)src/components/Global/GuestLoginCta/index.tsx
(3 hunks)src/components/Profile/views/IdentityVerification.view.tsx
(2 hunks)src/components/Setup/Views/SetupPasskey.tsx
(3 hunks)src/components/Setup/Views/Welcome.tsx
(2 hunks)src/utils/general.utils.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/components/Setup/Views/SetupPasskey.tsx
- src/components/Global/GuestLoginCta/index.tsx
- src/components/Setup/Views/Welcome.tsx
- src/utils/general.utils.ts
- src/components/Claim/Link/Initial.view.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1185
File: src/components/Claim/useClaimLink.tsx:14-0
Timestamp: 2025-09-05T07:31:11.365Z
Learning: In the peanut-ui codebase, `window.history.replaceState` is preferred over `router.replace` when immediate/synchronous URL parameter updates are required, as `router.replace` is asynchronous and doesn't guarantee instant URL changes that subsequent code can rely on. This pattern is used consistently across usePaymentInitiator.ts, Confirm.payment.view.tsx, and useClaimLink.tsx.
📚 Learning: 2025-08-20T09:08:19.266Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#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.
Applied to files:
src/components/Profile/views/IdentityVerification.view.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally pr looks good, but one flow not working, shared on discord
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Claim/Link/Initial.view.tsx (1)
173-175
: Don’t block auto-claim on empty recipient.addressThe early return prevents auto-claim for Peanut Wallet users when recipient state isn’t set yet, even though claim uses username/address from auth/wallet.
- if (recipient.address === '') return + if (!autoClaim && recipient.address === '') returnContext: sendLinksApi.claim/autoClaimLink accept username or wallet address as the recipient. This guards only non-auto paths.
♻️ Duplicate comments (1)
src/components/Claim/Link/Initial.view.tsx (1)
629-636
: Auto-claim effect: include missing deps and status check to avoid stale closuresAdd handleClaimLink, claimLinkData.status, and removeParamStep to deps. This also aligns with the earlier comment thread.
useEffect(() => { const stepFromURL = searchParams.get('step') if (user && claimLinkData.status !== 'CLAIMED' && stepFromURL === 'claim' && isPeanutWallet) { removeParamStep() handleClaimLink(false, true) } -}, [user, searchParams, isPeanutWallet]) +}, [user, claimLinkData.status, handleClaimLink, searchParams, removeParamStep, isPeanutWallet])
🧹 Nitpick comments (3)
src/app/api/auto-claim/route.ts (2)
37-41
: Preserve upstream error details for better UX and debuggingReturn the upstream error body (JSON or text) rather than only statusText.
- if (!response.ok) { - return NextResponse.json( - { error: `Failed to claim link: ${response.statusText}` }, - { status: response.status } - ) - } + if (!response.ok) { + const errText = await response.text() + let message = errText + try { + const json = JSON.parse(errText) + message = json.message || json.error || errText + } catch {} + return NextResponse.json({ error: `Failed to claim link: ${message}` }, { status: response.status }) + }
44-46
: Remove sensitive response loggingAvoid logging full claim responses; they may include sensitive data.
- const responseText = await response.text() - console.log('response', responseText) - return new NextResponse(responseText, { + const responseText = await response.text() + return new NextResponse(responseText, { headers: { 'Content-Type': 'application/json', }, })src/components/Claim/Link/Initial.view.tsx (1)
629-636
: Optional: debounce URL-driven auto-claimIf this effect can run multiple times on rapid URL/user changes, consider a simple in-flight guard (ref flag) to ensure a single invocation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/api/auto-claim/route.ts
(1 hunks)src/components/Claim/Link/Initial.view.tsx
(5 hunks)src/services/sendLinks.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-24T13:26:10.290Z
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#1014
File: src/components/Claim/Link/Initial.view.tsx:413-413
Timestamp: 2025-07-24T13:26:10.290Z
Learning: In the peanut-ui repository, the change from `${SQUID_API_URL}/route` to `${SQUID_API_URL}/v2/route` in src/components/Claim/Link/Initial.view.tsx was a typo fix, not an API migration, as the codebase was already using Squid API v2.
Applied to files:
src/app/api/auto-claim/route.ts
src/components/Claim/Link/Initial.view.tsx
📚 Learning: 2025-04-30T21:31:27.790Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#827
File: src/components/Claim/Link/Initial.view.tsx:120-126
Timestamp: 2025-04-30T21:31:27.790Z
Learning: The `sendLinksApi.claim` function in the Peanut Protocol UI accepts both username and wallet address as the first parameter.
Applied to files:
src/services/sendLinks.ts
src/components/Claim/Link/Initial.view.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.
Applied to files:
src/components/Claim/Link/Initial.view.tsx
🧬 Code graph analysis (1)
src/app/api/auto-claim/route.ts (2)
src/utils/sentry.utils.ts (1)
fetchWithSentry
(11-89)src/constants/general.consts.ts (1)
PEANUT_API_URL
(43-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
🔇 Additional comments (4)
src/app/api/auto-claim/route.ts (1)
13-55
: Nice route—clean and focusedThe API surface and error catch-all are appropriate for the use case.
src/services/sendLinks.ts (1)
208-214
: Good addition—client method maps cleanly to the new API routeThe JSDoc and signature align with claim() and existing patterns.
src/components/Claim/Link/Initial.view.tsx (2)
794-801
: Nice cleanup on modal closeRemoving the step param on close prevents unintended re-triggers.
160-183
: Peanut auto-claim branch looks goodClear bifurcation between auto-claim and regular claim with proper recipient selection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More context - https://discord.com/channels/972435984954302464/1412404738451968082