[TASK-14948] feat: Exchange rate redirect#1304
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds currency- and balance-aware CTA routing to the exchange-rate page, changes ExchangeRateWidget’s Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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 (2)
src/components/AddWithdraw/AddWithdrawRouterView.tsx (1)
72-79: Consider refactoring the conditional override pattern.The current approach of first computing
shouldShowAllMethodsvia ternary (line 72) and then conditionally overriding it (lines 76-79) works but is fragile—this pattern recalculates and reassigns on every render. A cleaner approach would be to compute this value once usinguseMemoor incorporate thecurrencyCodecheck directly into a single conditional expression.Example refactor using
useMemo:- // determine if we should show the full list of methods (countries/crypto) instead of the default view - let shouldShowAllMethods = flow === 'withdraw' ? showAllWithdrawMethods : localShowAllMethods - const setShouldShowAllMethods = flow === 'withdraw' ? setShowAllWithdrawMethods : setLocalShowAllMethods - const [isLoadingPreferences, setIsLoadingPreferences] = useState(true) - - // if currencyCode is present, show all methods - if (currencyCode) { - shouldShowAllMethods = true - } + // determine if we should show the full list of methods (countries/crypto) instead of the default view + const shouldShowAllMethods = useMemo(() => { + if (currencyCode) return true + return flow === 'withdraw' ? showAllWithdrawMethods : localShowAllMethods + }, [currencyCode, flow, showAllWithdrawMethods, localShowAllMethods]) + const setShouldShowAllMethods = flow === 'withdraw' ? setShowAllWithdrawMethods : setLocalShowAllMethods + const [isLoadingPreferences, setIsLoadingPreferences] = useState(true)src/app/(mobile-ui)/profile/exchange-rate/page.tsx (1)
16-55: Consider refactoring duplicate routing logic.Cases 1 (lines 21-24) and 3 (lines 43-46) have identical logic: both route to
/add-moneywith the source currency's country path. These could be consolidated into a single condition to reduce duplication and improve maintainability.Example refactor:
const handleCtaAction = (sourceCurrency: string, destinationCurrency: string) => { let route = '/add-money' let countryPath: string | undefined = '' - // Case 1: source currency is not usd and destination currency is usd -> redirect to add-money/sourceCurrencyCountry page - if (sourceCurrency !== 'USD' && destinationCurrency === 'USD') { - countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path - route = '/add-money' - } - - // Case 2: source currency is usd and destination currency is not usd -> redirect to withdraw/destinationCurrencyCountry page - if (sourceCurrency === 'USD' && destinationCurrency !== 'USD') { + // If source is USD and destination is not USD, check balance to determine flow + if (sourceCurrency === 'USD' && destinationCurrency !== 'USD') { const formattedBalance = printableUsdc(balance ?? 0n) // if there is no balance, redirect to add-money if (parseFloat(formattedBalance) <= 0) { - countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path + countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === 'USD')?.path route = '/add-money' } else { countryPath = countryCurrencyMappings.find( (currency) => currency.currencyCode === destinationCurrency )?.path route = '/withdraw' } + } else if (sourceCurrency !== 'USD') { + // For any non-USD source currency, route to add-money with source country + countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path + route = '/add-money' } - - // Case 3: source currency is not usd and destination currency is not usd -> redirect to add-money/sourceCurrencyCountry page - if (sourceCurrency !== 'USD' && destinationCurrency !== 'USD') { - countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path - route = '/add-money' - } if (!countryPath) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx(1 hunks)src/components/AddWithdraw/AddWithdrawRouterView.tsx(2 hunks)src/components/Common/CountryList.tsx(2 hunks)src/components/Global/ExchangeRateWidget/index.tsx(2 hunks)src/constants/countryCurrencyMapping.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-18T09:30:42.901Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1230
File: src/app/(mobile-ui)/withdraw/page.tsx:92-97
Timestamp: 2025-09-18T09:30:42.901Z
Learning: In src/app/(mobile-ui)/withdraw/page.tsx, the useEffect that calls setShowAllWithdrawMethods(true) when amountFromContext exists is intentionally designed to run only on component mount (empty dependency array), not when amountFromContext changes. This is the correct behavior for the withdraw flow where showing all methods should only happen on initial load when an amount is already present.
Applied to files:
src/components/AddWithdraw/AddWithdrawRouterView.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
PR: peanutprotocol/peanut-ui#869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/components/AddWithdraw/AddWithdrawRouterView.tsx
🧬 Code graph analysis (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (3)
src/hooks/wallet/useWallet.ts (1)
useWallet(14-98)src/constants/countryCurrencyMapping.ts (1)
countryCurrencyMappings(10-48)src/utils/balance.utils.ts (1)
printableUsdc(32-38)
⏰ 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 (5)
src/constants/countryCurrencyMapping.ts (1)
46-47: LGTM!The addition of
pathproperties for Brazil and Argentina follows the same pattern as other country entries and uses consistent lowercase naming. This enables routing functionality for these LATAM countries.src/components/Common/CountryList.tsx (1)
50-54: LGTM!The URL query parameter handling for
currencyCodeenables deep-linking to a pre-filtered country list. The initial search term is correctly seeded from the URL parameter, allowing users to land on a page with the relevant currency options already filtered. The existing filtering logic (lines 87-88) will match against both country names and currency codes.src/components/Global/ExchangeRateWidget/index.tsx (2)
14-14: LGTM!The updated
ctaActionsignature correctly passes the selected currencies to the parent component, enabling currency-aware routing logic. This is a breaking change to the component's public API, but it's necessary for the desired functionality.
215-215: LGTM!The button's
onClickhandler correctly invokes thectaActionwith the currentsourceCurrencyanddestinationCurrencyvalues.src/app/(mobile-ui)/profile/exchange-rate/page.tsx (1)
57-60: LGTM!The empty dependency array ensures
fetchBalanceis called only on component mount, which is appropriate for initializing the balance state in this flow. This pattern is consistent with similar patterns elsewhere in the codebase.Based on learnings
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (1)
28-34: Balance check has been corrected.The balance check now correctly uses
<= 0to detect zero or insufficient balance, addressing the previous critical issue where< 0would never be true for unsigned bigint balances.
🧹 Nitpick comments (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (1)
57-60: AddfetchBalanceto the dependency array.The
useEffectcallsfetchBalance()but omits it from the dependency array. WhilefetchBalanceis likely stable (wrapped inuseCallbackper theuseWalletimplementation), React best practices recommend including all dependencies to avoid subtle bugs and satisfy the exhaustive-deps lint rule.useEffect(() => { // Fetch latest balance fetchBalance() - }, []) + }, [fetchBalance])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-07T20:22:11.092Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#958
File: src/app/actions/tokens.ts:266-266
Timestamp: 2025-07-07T20:22:11.092Z
Learning: In `src/app/actions/tokens.ts`, within the `fetchWalletBalances` function, using the non-null assertion operator `!` on `process.env.MOBULA_API_KEY!` is intentional and correct, and should not be flagged for replacement with explicit validation.
Applied to files:
src/app/(mobile-ui)/profile/exchange-rate/page.tsx
🧬 Code graph analysis (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (3)
src/hooks/wallet/useWallet.ts (1)
useWallet(14-98)src/constants/countryCurrencyMapping.ts (1)
countryCurrencyMappings(10-48)src/utils/balance.utils.ts (1)
printableUsdc(32-38)
⏰ 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
kushagrasarathe
left a comment
kushagrasarathe
left a comment
There was a problem hiding this comment.
lgtm, resolve cr comments, also codefreeze so do not merge
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (1)
28-34: Good fix: zero-balance check and USD→USD case are covered.
- parseFloat(... ) <= 0 is correct for zero balance.
- USD→USD routing added with balance-based branch.
Also applies to: 48-54
🧹 Nitpick comments (3)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (3)
16-63: Avoid hardcoded EUR fallback; use context-specific currencyCode.If countryPath is missing (e.g., EUR or unmapped), defaulting to
?currencyCode=EURignores the user-selected currencies. Track a fallbackCurrencyCode per branch and use it in the query.- const handleCtaAction = (sourceCurrency: string, destinationCurrency: string) => { - let route = '/add-money' - let countryPath: string | undefined = '' + const handleCtaAction = (sourceCurrency: string, destinationCurrency: string) => { + let route = '/add-money' + let countryPath: string | undefined + let fallbackCurrencyCode: string | undefined @@ - if (sourceCurrency !== 'USD' && destinationCurrency === 'USD') { + if (sourceCurrency !== 'USD' && destinationCurrency === 'USD') { countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path route = '/add-money' + fallbackCurrencyCode = sourceCurrency } @@ - if (sourceCurrency === 'USD' && destinationCurrency !== 'USD') { - const formattedBalance = printableUsdc(balance ?? 0n) + if (sourceCurrency === 'USD' && destinationCurrency !== 'USD') { + const formattedBalance = printableUsdc(balance ?? 0n) // if there is no balance, redirect to add-money if (parseFloat(formattedBalance) <= 0) { countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path route = '/add-money' + fallbackCurrencyCode = sourceCurrency } else { countryPath = countryCurrencyMappings.find( (currency) => currency.currencyCode === destinationCurrency )?.path route = '/withdraw' + fallbackCurrencyCode = destinationCurrency } } @@ - if (sourceCurrency !== 'USD' && destinationCurrency !== 'USD') { + if (sourceCurrency !== 'USD' && destinationCurrency !== 'USD') { countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === sourceCurrency)?.path route = '/add-money' + fallbackCurrencyCode = sourceCurrency } @@ - if (sourceCurrency === 'USD' && destinationCurrency === 'USD') { - const formattedBalance = parseFloat(printableUsdc(balance ?? 0n)) + if (sourceCurrency === 'USD' && destinationCurrency === 'USD') { + const formattedBalance = parseFloat(printableUsdc(balance ?? 0n)) // Either redirect to a relevant page or show an error // For now, routing to USA add-money page as an example countryPath = countryCurrencyMappings.find((currency) => currency.currencyCode === 'USD')?.path route = formattedBalance <= 0 ? '/add-money' : '/withdraw' + fallbackCurrencyCode = 'USD' } @@ - if (!countryPath) { - const redirectRoute = `${route}?currencyCode=EUR` + if (!countryPath) { + const redirectRoute = `${route}?currencyCode=${fallbackCurrencyCode ?? 'EUR'}` router.push(redirectRoute) } else { const redirectRoute = `${route}/${countryPath}` router.push(redirectRoute) }
10-10: Remove redundant balance fetch (or add deps).useWallet already fetches on address changes. The page-level effect can double-fire and violates exhaustive-deps.
Option A (remove):
-import { useEffect } from 'react' @@ - useEffect(() => { - // Fetch latest balance - fetchBalance() - }, []) + // Balance is fetched by useWallet; call fetchBalance on demand if needed.Option B (keep effect, fix deps):
- useEffect(() => { - fetchBalance() - }, []) + useEffect(() => { + fetchBalance() + }, [fetchBalance])Also applies to: 65-68
16-47: Optional: normalize currency codes and simplify branching.
- Normalize inputs to uppercase to harden against unexpected casing.
- Use else-if or early returns to make the mutually-exclusive cases explicit.
Example:
const src = sourceCurrency.toUpperCase(); const dst = destinationCurrency.toUpperCase();- Chain as
if (...) { ...; return } else if (...) { ...; return } ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-07T20:22:11.092Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#958
File: src/app/actions/tokens.ts:266-266
Timestamp: 2025-07-07T20:22:11.092Z
Learning: In `src/app/actions/tokens.ts`, within the `fetchWalletBalances` function, using the non-null assertion operator `!` on `process.env.MOBULA_API_KEY!` is intentional and correct, and should not be flagged for replacement with explicit validation.
Applied to files:
src/app/(mobile-ui)/profile/exchange-rate/page.tsx
🧬 Code graph analysis (1)
src/app/(mobile-ui)/profile/exchange-rate/page.tsx (3)
src/hooks/wallet/useWallet.ts (1)
useWallet(14-98)src/constants/countryCurrencyMapping.ts (1)
countryCurrencyMappings(10-48)src/utils/balance.utils.ts (1)
printableUsdc(32-38)
⏰ 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/app/(mobile-ui)/profile/exchange-rate/page.tsx (1)
8-8: No changes needed forprintableUsdcimport. The barrel atsrc/utils/index.tsre-exportsprintableUsdc, soimport { printableUsdc } from '@/utils'is correct.
2 participants
Context - https://discord.com/channels/972435984954302464/1425426674677321730