fix: nan onramp request link #1184
Conversation
WalkthroughUpdated payload amount computation in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/hooks/useCreateOnramp.ts (6)
48-49: Rounding: verify 2 decimals are correct for the target currency.If
amountis not a fiat with 2 decimals, consider currency-specific precision (e.g., 0, 2, 6). I can help wire a decimals map.
52-57: Avoid sendingBearer undefined.Set the Authorization header only when the token exists.
- const response = await fetch('/api/proxy/bridge/onramp/create', { + const headers: Record<string, string> = { 'Content-Type': 'application/json' } + if (jwtToken) headers.Authorization = `Bearer ${jwtToken}` + const response = await fetch('/api/proxy/bridge/onramp/create', { method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${jwtToken}`, - }, + headers,
51-67: Add a client-side timeout to the fetch.Prevent hanging requests with
AbortController.- const response = await fetch('/api/proxy/bridge/onramp/create', { + const controller = new AbortController() + const t = setTimeout(() => controller.abort(), 20000) + const response = await fetch('/api/proxy/bridge/onramp/create', { method: 'POST', headers, + signal: controller.signal, body: JSON.stringify({ amount, chargeId, source: { currency, paymentRail, }, recipientAddress, }), }) + clearTimeout(t)
77-77: Copy tweak: align error text with onramp flow.“bank transfer” sounds off here.
- setError('Failed to create bank transfer. Please try again.') + setError('Failed to create onramp. Please try again.')
37-49: Runtime guard for missing amount.If
usdAmountis an empty string,amountcould remainundefinedand be omitted from the payload. Add an explicit check.try { const jwtToken = Cookies.get('jwt-token') const { currency, paymentRail } = getCurrencyConfig(country.id, 'onramp') if (usdAmount) { // Get currency configuration for the country const price = await getCurrencyPrice(currency) amount = (Number(usdAmount) * price).toFixed(2) } + if (amount == null) { + throw new Error('Amount is required') + }
41-45: Minor: remove redundant comment.“Get currency configuration for the country” restates the code; safe to delete.
📜 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 (1)
src/hooks/useCreateOnramp.ts(1 hunks)
⏰ 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 (1)
src/hooks/useCreateOnramp.ts (1)
48-48: Fix eliminates NaN in usdAmount path.Switching to
Number(usdAmount)resolves the NaN caused whenamountwas undefined. Good catch.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/components/Request/views/ReqFulfillBankFlowManager.tsx (3)
37-37: Don’t conflate validation and submission errors in one state.
errorMessageis used for both amount validation and user-update failures; the amount effect can clear a user-update error. Split intovalidationErrorandsubmitError, and render both as needed.Apply minimally:
-const [errorMessage, setErrorMessage] = useState<string | null>(null) +const [validationError, setValidationError] = useState<string | null>(null) +const [submitError, setSubmitError] = useState<string | null>(null)Then replace reads/writes accordingly (see related suggestions below).
124-142: Robust error handling for unknown errors; don’t let the amount validation effect clear submit errors.
- Cast
errorsafely to string.- If you keep split states, use
setSubmitErrorhere and ensure the amount validation effect only touchesvalidationError.Apply:
- setErrorMessage(null) + setSubmitError(null) @@ - } catch (error: any) { - setErrorMessage(error.message) - return { error: error.message } + } catch (error: unknown) { + const msg = error instanceof Error ? error.message : String(error) + setSubmitError(msg) + return { error: msg }
220-225: Button gating and error rendering are currently tied only to the user-details view.If the minimum-amount validation is meant to gate the entire onramp flow, also prevent confirming in the
OnrampConfirmationstep (see earlier suggestion) and consider showing the validation error earlier in the flow.- disabled={!isUserDetailsFormValid || isUpdatingUser || !!errorMessage} + disabled={!isUserDetailsFormValid || isUpdatingUser || !!validationError || !!submitError}And:
-{errorMessage && <ErrorAlert description={errorMessage} />} +{(submitError || validationError) && ( + <ErrorAlert description={(submitError ?? validationError)!} /> +)}
📜 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 (1)
src/components/Request/views/ReqFulfillBankFlowManager.tsx(7 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:25:45.170Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:28:25.280Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.
Learnt from: Hugo0
PR: peanutprotocol/peanut-ui#420
File: src/components/Offramp/Offramp.consts.ts:27-28
Timestamp: 2024-10-08T20:13:42.967Z
Learning: In `src/components/Offramp/Offramp.consts.ts`, the `MIN_CASHOUT_LIMIT` is set to $10 because smaller amounts are impractical due to approximately $1 fee per cashout.
📚 Learning: 2024-10-07T15:25:45.170Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:25:45.170Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(...)` return strings, ensuring that `calculatedFee` consistently returns a string without the need for additional type conversion.
Applied to files:
src/components/Request/views/ReqFulfillBankFlowManager.tsx
📚 Learning: 2024-10-07T15:28:25.280Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#422
File: src/components/Request/Pay/Views/Initial.view.tsx:76-78
Timestamp: 2024-10-07T15:28:25.280Z
Learning: In `src/components/Request/Pay/Views/Initial.view.tsx`, both `txFee` and `utils.formatTokenAmount(estimatedGasCost, 3)` return strings, ensuring consistent return types for `calculatedFee`.
Applied to files:
src/components/Request/views/ReqFulfillBankFlowManager.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/Request/views/ReqFulfillBankFlowManager.tsx
🧬 Code graph analysis (1)
src/components/Request/views/ReqFulfillBankFlowManager.tsx (1)
src/app/actions/currency.ts (1)
getCurrencyPrice(7-58)
⏰ 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 (1)
src/components/Request/views/ReqFulfillBankFlowManager.tsx (1)
74-114: Discard validationError suggestion: variable doesn’t exist in this component. The proposed diff referencesvalidationError, but no such variable is declared or used in ReqFulfillBankFlowManager.tsx—please ignore this change.Likely an incorrect or invalid review comment.
No description provided.