-
Notifications
You must be signed in to change notification settings - Fork 13
hot-fix: poland guest link claim issue #1209
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
Caution Review failedThe pull request is closed. WalkthroughAdds an early Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✨ 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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/sdkErrorHandler.utils.tsx (1)
53-57
: Fix user-facing typos/wording
- "Bulk is not able on this chain" → "Bulk is not available on this chain"
- "occured" → "occurred"
Apply:
- return 'Bulk is not able on this chain, please try another chain.' + return 'Bulk is not available on this chain, please try another chain.' ... - return 'A network error occured. Please refresh and try again.' + return 'A network error occurred. Please refresh and try again.'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/actions/external-accounts.ts
(2 hunks)src/components/AddWithdraw/DynamicBankAccountForm.tsx
(1 hunks)src/components/Claim/Link/views/BankFlowManager.view.tsx
(1 hunks)src/utils/sdkErrorHandler.utils.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
src/components/Claim/Link/views/BankFlowManager.view.tsx
📚 Learning: 2025-08-26T15:25:53.328Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1132
File: src/app/[...recipient]/client.tsx:394-397
Timestamp: 2025-08-26T15:25:53.328Z
Learning: In `src/components/Common/ActionListDaimoPayButton.tsx`, the `handleCompleteDaimoPayment` function should not display error messages to users when DB update fails because the Daimo payment itself has succeeded - showing errors would be confusing since the payment was successful.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.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/Claim/Link/views/BankFlowManager.view.tsx
📚 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/Claim/Link/views/BankFlowManager.view.tsx
📚 Learning: 2024-11-18T21:36:11.486Z
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#535
File: src/components/Claim/Claim.tsx:142-146
Timestamp: 2024-11-18T21:36:11.486Z
Learning: In `src/components/Claim/Claim.tsx`, external calls like token price fetching and cross-chain details retrieval are already encapsulated within existing `try...catch` blocks, so additional error handling may be unnecessary.
Applied to files:
src/components/Claim/Link/views/BankFlowManager.view.tsx
🧬 Code graph analysis (2)
src/app/actions/external-accounts.ts (1)
src/interfaces/interfaces.ts (1)
IBridgeAccount
(162-198)
src/components/Claim/Link/views/BankFlowManager.view.tsx (1)
src/utils/withdraw.utils.ts (1)
getCountryCodeForWithdraw
(137-147)
⏰ 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/utils/sdkErrorHandler.utils.tsx (1)
115-115
: Fallback message change looks goodClearer escalation path for unexpected errors. No control-flow impact.
src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
401-405
: Single prioritized ErrorAlert = good UXPrioritizing submissionError over parent error prevents duplicate banners.
205-208
: Normalize country codes and treat US/MX as non-IBAN (support 2- and 3-letter codes)Current checks only match "MX" and "USA" — 3-letter codes like "MEX" will be misrouted to the IBAN path. Normalize to uppercase and accept both 2- and 3-letter variants.
- const isMx = country.toUpperCase() === 'MX' - const isUs = country.toUpperCase() === 'USA' - const isIban = isUs || isMx ? false : isIBANCountry(country) + const C = country.toUpperCase() + const isUs = C === 'USA' || C === 'US' + const isMx = C === 'MEX' || C === 'MX' + const isIban = isUs || isMx ? false : isIBANCountry(C)Confirm other callers may also send 3-letter codes so both formats remain supported.
src/app/actions/external-accounts.ts (1)
13-13
: Return type extension is appropriateAdding optional
source
preserves structured error info for invalid parameters.src/components/Claim/Link/views/BankFlowManager.view.tsx (1)
463-467
: ```shell
#!/bin/bash
set -euo pipefailecho "PWD:"; pwd
echo
echo "Root listing (top-level):"; ls -la | sed -n '1,200p' || true
echoecho "Finding BankFlowManager.view.tsx with find..."
files=$(find . -type f -name "BankFlowManager.view.tsx" 2>/dev/null || true)
if [ -n "$files" ]; then
for f in $files; do
echo "---- $f ----"
sed -n '430,490p' "$f" || true
done
else
echo "BankFlowManager.view.tsx not found with find."
fi
echoecho "Searching repository for getCountryCodeForWithdraw (rg -uu to ignore default ignores)..."
rg -n --hidden -uu -S "getCountryCodeForWithdraw" -C3 || true
echoecho "Searching for function/definition of getCountryCodeForWithdraw..."
rg -n --hidden -uu -S "function getCountryCodeForWithdraw|const getCountryCodeForWithdraw|export .* getCountryCodeForWithdraw" -C5 || true
echoecho "If files were found for getCountryCodeForWithdraw, printing first 240 lines of each:"
files=$(rg -n --hidden -uu -S "getCountryCodeForWithdraw" -l || true)
for f in $files; do
echo "---- $f ----"
sed -n '1,240p' "$f" || true
done
echoecho "Searching for DynamicBankAccountForm occurrences..."
rg -n --hidden -uu -S "DynamicBankAccountForm" -C3 || true
echoecho "Searching for explicit MX checks (country === 'MX' or similar)..."
rg -n --hidden -uu -S "country\s*===\s*'MX'|country\s*===\s*"MX"|country\s*==\s*'MX'|country\s*==\s*"MX"" -C3 || true
echoecho "Searching for selectedCountry?.id usages..."
rg -n --hidden -uu -S "selectedCountry?.\s*id|selectedCountry?.id|selectedCountry.id\b" -C3 || true
echoecho "Searching for country code mapping helpers/variables (countryCodeMap, iso3, iso2, alpha2, alpha3)..."
rg -n --hidden -uu -S "countryCodeMap|countryCodes|alpha2|alpha3|iso3166|iso2|iso3|country_code" -C3 || true
echoecho "Done."
</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
const threeLetterCountryCode = getCountryCodeForWithdraw(selectedCountry.id) | ||
const payloadWithCountry = { | ||
...payload, | ||
country: selectedCountry.id, | ||
countryCode: threeLetterCountryCode, | ||
address: { | ||
...payload.address, | ||
country: threeLetterCountryCode, | ||
}, | ||
country: threeLetterCountryCode, | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Verify top-level country
field is still part of AddBankAccountPayload
You’re now setting:
countryCode: threeLetterCountryCode
address.country: threeLetterCountryCode
country: threeLetterCountryCode
(top-level)
If the API no longer expects country
, this may trigger invalid_parameters
with source "country". If redundant, drop it:
const payloadWithCountry = {
...payload,
countryCode: threeLetterCountryCode,
address: {
...payload.address,
country: threeLetterCountryCode,
},
- country: threeLetterCountryCode,
}
Run to confirm the payload shape:
🏁 Script executed:
#!/bin/bash
# Locate AddBankAccountPayload and usages of top-level `country`
rg -nC2 'export\s+type\s+AddBankAccountPayload|export\s+interface\s+AddBankAccountPayload' src
rg -nC2 '\bAddBankAccountPayload\b' src | sed -n '1,200p'
rg -nP '\bcountry\s*:' src/app/actions/types --glob '!**/*.test.*'
Length of output: 6451
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="src/app/actions/types/users.types.ts"
echo "=== inspecting: $file ==="
if [ ! -f "$file" ]; then
echo "FILE_NOT_FOUND: $file"
exit 0
fi
echo "----- file contents (first 400 lines) -----"
sed -n '1,400p' "$file" || true
echo
echo "----- grep for 'country' in that file (with context) -----"
rg -n --hidden --no-ignore-vcs '\bcountry\b' "$file" -n -C2 || true
echo
echo "----- locations referencing AddBankAccountPayload in src (context) -----"
rg -n --hidden --no-ignore-vcs '\bAddBankAccountPayload\b' src -C2 || true
echo
echo "----- grep for 'country' occurrences across src (lines only) -----"
rg -n --hidden --no-ignore-vcs '\bcountry\s*[:?]' src || true
Length of output: 13643
Remove top-level country
from payload — not part of AddBankAccountPayload
AddBankAccountPayload (src/app/actions/types/users.types.ts) defines countryCode and address.country but does NOT include a top-level country
; sending it may trigger API validation errors. Remove the top-level country
field.
Location: src/components/Claim/Link/views/BankFlowManager.view.tsx (around lines 318–327)
const payloadWithCountry = {
...payload,
countryCode: threeLetterCountryCode,
address: {
...payload.address,
country: threeLetterCountryCode,
},
- country: threeLetterCountryCode,
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const threeLetterCountryCode = getCountryCodeForWithdraw(selectedCountry.id) | |
const payloadWithCountry = { | |
...payload, | |
country: selectedCountry.id, | |
countryCode: threeLetterCountryCode, | |
address: { | |
...payload.address, | |
country: threeLetterCountryCode, | |
}, | |
country: threeLetterCountryCode, | |
} | |
const threeLetterCountryCode = getCountryCodeForWithdraw(selectedCountry.id) | |
const payloadWithCountry = { | |
...payload, | |
countryCode: threeLetterCountryCode, | |
address: { | |
...payload.address, | |
country: threeLetterCountryCode, | |
}, | |
} |
🤖 Prompt for AI Agents
In src/components/Claim/Link/views/BankFlowManager.view.tsx around lines
318-327, the payload construction adds a top-level "country" property which is
not part of AddBankAccountPayload and may fail API validation; remove the
top-level "country" key and only set countryCode and address.country (keep
spreading payload.address and payload otherwise intact) so the resulting payload
contains countryCode and address.country but no top-level country field.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Hugo Montenegro <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.