FIX: incorrect exchange rate shown for non-euro using European countries#1173
FIX: incorrect exchange rate shown for non-euro using European countries#1173Zishan-7 merged 2 commits intopeanut-wallet-devfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds country-aware currency resolution and path fields to country mappings, passes a resolved nonEuroCurrency into ExchangeRate, updates ExchangeRate and its hooks to support an enabled flag and dual-source rate fetching (USD→non-EUR when applicable), and adjusts SavedAccounts country lookup to prefer mapping path. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/Common/SavedAccountsView.tsx (1)
90-90: Guard against missing path when building navigation URLIf countryInfo exists but path is missing, this will render /withdraw/undefined/bank. Guard the path:
- const path = countryInfo ? `/withdraw/${countryInfo.path}/bank` : '/withdraw' + const path = countryInfo?.path ? `/withdraw/${countryInfo.path}/bank` : '/withdraw'
🧹 Nitpick comments (10)
src/hooks/useExchangeRate.ts (2)
30-31: Sensible default for enabledDefaulting enabled = true preserves existing behavior; callers can opt out. Consider documenting this in the hook JSDoc.
94-95: Add minor hardening to the query options
- Normalize currency codes in the queryKey/URL to avoid cache duplication (e.g., 'usd' vs 'USD').
- Consider retry: 1 to keep UI snappy on transient failures.
Example (conceptual):
- const from = sourceCurrency?.toUpperCase(); const to = destinationCurrency?.toUpperCase();
src/constants/countryCurrencyMapping.ts (2)
7-7: path?: string addition is appropriate; clarify slug contractPlease document that path is a lowercase, hyphenated slug (no diacritics). This will prevent mismatches across consumers.
15-38: Ensure slug consistency and future-proof lookupsThe slugs look consistent (e.g., czech-republic, united-kingdom). To avoid brittle string compares downstream, consider exporting a derived map keyed by path and by country for O(1) lookups.
src/components/Common/SavedAccountsView.tsx (1)
79-81: Optional: avoid broken flag images for unknown codesWhen countryCodeMap misses, threeLetterCountryCode is used and may not exist on flagcdn. Consider hiding the flag unless you have a valid 2-letter ISO code.
src/hooks/useGetExchangeRate.tsx (1)
18-52: Avoid stale loading state when toggling enabled to falseIf enabled becomes false mid-lifecycle, explicitly clear loading to prevent transient UI inconsistencies.
- useEffect(() => { - const fetchExchangeRate = async () => { + useEffect(() => { + if (!enabled) { + setIsFetchingRate(false) + return + } + const fetchExchangeRate = async () => { setIsFetchingRate(true) // reset previous value to avoid showing a stale rate for a new account type setExchangeRate(null)src/components/ExchangeRate/index.tsx (2)
6-8: Typo in prop name: nonEruoCurrency → nonEuroCurrencyRecommend correcting the spelling across the codebase for clarity and future discoverability. If changing now is risky, schedule a follow-up rename.
I can generate a codemod to rename this across the repo with minimal churn.
29-36: Simplify number formatting and tighten copy
- nonEruoExchangeRate is a number; no need for parseFloat(...toString()).
- Consider ending the sentence with a period.
- displayValue = nonEruoExchangeRate - ? `1 USD = ${parseFloat(nonEruoExchangeRate.toString()).toFixed(4)} ${nonEruoCurrency}` + displayValue = nonEruoExchangeRate + ? `1 USD = ${nonEruoExchangeRate.toFixed(4)} ${nonEruoCurrency}` : '-' @@ - moreInfoText = - "This is an approximate value. The actual amount received may vary based on your bank's exchange rate" + moreInfoText = + "This is an approximate value. The actual amount received may vary based on your bank's exchange rate."src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (2)
34-41: Normalize params once and reuse; reduce repeated lowercasingPre-normalizing the country segment avoids repeating toLowerCase and makes the intent clearer.
Apply:
- const params = useParams() - const country = params.country as string - - const nonEruoCurrency = countryCurrencyMappings.find( - (currency) => - country.toLowerCase() === currency.country.toLowerCase() || - currency.path?.toLowerCase() === country.toLowerCase() - )?.currencyCode + const params = useParams() + const normalizedCountry = (params.country as string).toLowerCase() + + const nonEruoCurrency = countryCurrencyMappings.find((currency) => { + return ( + normalizedCountry === currency.country.toLowerCase() || + currency.path?.toLowerCase() === normalizedCountry + ) + })?.currencyCodeIf you prefer stronger typing, you can also do:
-const params = useParams() +const { country: countryParam } = useParams<{ country: string }>() +const normalizedCountry = countryParam.toLowerCase()
238-238: Prop name confirmation (spelling) and API syncYou’re passing nonEruoCurrency to ExchangeRate. If the prop in ExchangeRate is intentionally spelled this way (per PR), LGTM. Otherwise, align to nonEuroCurrency across both files in this PR to avoid confusion.
📜 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/app/(mobile-ui)/withdraw/[country]/bank/page.tsx(3 hunks)src/components/Common/SavedAccountsView.tsx(1 hunks)src/components/ExchangeRate/index.tsx(1 hunks)src/constants/countryCurrencyMapping.ts(1 hunks)src/hooks/useExchangeRate.ts(3 hunks)src/hooks/useGetExchangeRate.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-08-14T14:42:54.411Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
Applied to files:
src/components/Common/SavedAccountsView.tsxsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsxsrc/constants/countryCurrencyMapping.ts
📚 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/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
📚 Learning: 2025-08-13T18:22:01.941Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1094
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:0-0
Timestamp: 2025-08-13T18:22:01.941Z
Learning: In the DynamicBankAccountForm component, the countryName parameter from useParams will always resemble a country title, not a URL slug.
Applied to files:
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
📚 Learning: 2025-08-12T17:47:28.362Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1089
File: src/app/api/bridge/exchange-rate/route.ts:4-19
Timestamp: 2025-08-12T17:47:28.362Z
Learning: In the Bridge exchange rate API route (src/app/api/bridge/exchange-rate/route.ts), the ExchangeRateResponse interface uses numeric types for rates because the route converts string values from the Bridge API to floats using parseFloat() before returning the response.
Applied to files:
src/hooks/useExchangeRate.tssrc/components/ExchangeRate/index.tsxsrc/hooks/useGetExchangeRate.tsx
🧬 Code graph analysis (2)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
src/constants/countryCurrencyMapping.ts (1)
countryCurrencyMappings(10-48)
src/components/ExchangeRate/index.tsx (3)
src/hooks/useGetExchangeRate.tsx (2)
IExchangeRate(5-8)useGetExchangeRate(14-55)src/hooks/useExchangeRate.ts (1)
useExchangeRate(26-150)src/components/Payment/PaymentInfoRow.tsx (1)
PaymentInfoRow(7-81)
⏰ 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 (8)
src/hooks/useExchangeRate.ts (1)
11-11: Gated fetch via optional enabled flag looks goodAdding enabled?: boolean is a clean way to short-circuit requests when unused. No concerns.
src/constants/countryCurrencyMapping.ts (1)
40-44: Align US/MX path values with route directories
Nosrc/app/(mobile-ui)/withdraw/usaor…/mexicofolders exist—update thepathfields to match your actual route slugs or add those route directories.⛔ Skipped due to learnings
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".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".src/hooks/useGetExchangeRate.tsx (2)
7-8: Enabled flag is a good additionInterface change is backward compatible and matches the new usage.
14-17: Initialize isFetchingRate from enabled — goodPrevents a loading spinner when the hook is disabled.
src/components/ExchangeRate/index.tsx (3)
11-17: Gated dual-source fetching is correct
- useGetExchangeRate disabled when nonEruoCurrency is present.
- useExchangeRate enabled only when nonEruoCurrency exists and correctly uses USD→dest with amount=1.
Looks good.
37-40: Formatting is correct hereexchangeRate is a string; parseFloat(...).toFixed(4) is appropriate.
10-17: Resolved:/api/exchange-ratereturns numericratematching hook expectationsrc/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
15-15: Importing useParams is fine hereNo issues with bringing in useParams alongside useRouter for this client component.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/ExchangeRate/index.tsx (3)
10-17: Avoid triggering the legacy hook for US accountsMinor UX optimization: don’t enable useGetExchangeRate for AccountType.US; we already know the rate is 1 and currently we show a brief loading flicker.
Apply this diff:
- const { exchangeRate, isFetchingRate } = useGetExchangeRate({ accountType, enabled: !nonEuroCurrency }) + const { exchangeRate, isFetchingRate } = useGetExchangeRate({ + accountType, + enabled: accountType !== AccountType.US && !nonEuroCurrency, + })
37-40: Consider aligning type flow to avoid parseFloat hereYou parseFloat(exchangeRate) because useGetExchangeRate returns a string. Per the retrieved learning (Bridge route returns numeric rates), consider updating useGetExchangeRate to surface a number to eliminate parsing at call sites and reduce NaN risks.
Would you like me to propose a small PR to change useGetExchangeRate’s state to number | null and adapt getExchangeRate’s response typing?
12-12: Fix typo and simplify number formatting
- Rename
nonEruoExchangeRate→nonEuroExchangeRate(lines 12, 30–31).- Replace
parseFloat(nonEruoExchangeRate.toString()).toFixed(4)withnonEuroExchangeRate.toFixed(4).- const { exchangeRate: nonEruoExchangeRate, isLoading } = useExchangeRate({ + const { exchangeRate: nonEuroExchangeRate, isLoading } = useExchangeRate({- displayValue = nonEruoExchangeRate - ? `1 USD = ${parseFloat(nonEruoExchangeRate.toString()).toFixed(4)} ${nonEuroCurrency}` + displayValue = nonEuroExchangeRate + ? `1 USD = ${nonEuroExchangeRate.toFixed(4)} ${nonEuroCurrency}` : '-'
📜 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 (2)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx(3 hunks)src/components/ExchangeRate/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T17:47:28.362Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1089
File: src/app/api/bridge/exchange-rate/route.ts:4-19
Timestamp: 2025-08-12T17:47:28.362Z
Learning: In the Bridge exchange rate API route (src/app/api/bridge/exchange-rate/route.ts), the ExchangeRateResponse interface uses numeric types for rates because the route converts string values from the Bridge API to floats using parseFloat() before returning the response.
Applied to files:
src/components/ExchangeRate/index.tsx
🧬 Code graph analysis (1)
src/components/ExchangeRate/index.tsx (3)
src/hooks/useGetExchangeRate.tsx (2)
IExchangeRate(5-8)useGetExchangeRate(14-55)src/hooks/useExchangeRate.ts (1)
useExchangeRate(26-150)src/components/Payment/PaymentInfoRow.tsx (1)
PaymentInfoRow(7-81)
⏰ 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 (2)
src/components/ExchangeRate/index.tsx (2)
4-4: LGTM: adds hook needed for USD→non-EUR pathImport is correct and scoped.
6-8: Prop API extension looks goodExtending IExchangeRate and adding nonEuroCurrency keeps the public API minimal.
kushagrasarathe
left a comment
kushagrasarathe
left a comment
There was a problem hiding this comment.
@Zishan-7 lgtm, left one suggestion
2 participants
TASK-14343
More info on why we used the Frankfurter API for getting exchange rates https://discord.com/channels/972435984954302464/1412418820756213790