Conversation
SP 112 Prod release - Lockup
hot-fix: banner on semantic req page
…able chore: withdraw temp unavaillable for non-euro sepa countries msg
Signed-off-by: Hugo Montenegro <hugo@peanut.to>
made invite code validation more lax and added numbers
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 184 files out of 291 files are above the max files limit of 100. Please upgrade to Pro plan to get higher limits. You can disable this status message by setting the WalkthroughUpdates two redirect targets in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Heterogeneous edits span configuration, utility logic, refactor, and UI/UX changes; review requires checking URL safety, deterministic suffix logic, invite-link behavior, conditional UI disabling, and a small styling adjustment. Possibly related PRs
Suggested labelsenhancement Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
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: 0
🧹 Nitpick comments (1)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
43-49: Consider clarifying the comment.The comment "non-eur sepa countries" could be more precise. SEPA includes countries with various currencies (PLN, CZK, etc.), not just EUR. Consider rewording to: "SEPA countries using non-EUR currencies (excluding USD and MXN which are handled separately)" for better clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
redirects.json(1 hunks)src/app/(mobile-ui)/points/page.tsx(2 hunks)src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx(3 hunks)src/app/[...recipient]/client.tsx(1 hunks)src/utils/general.utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-08T17:13:13.155Z
Learnt from: Zishan-7
PR: peanutprotocol/peanut-ui#1299
File: src/app/(mobile-ui)/points/page.tsx:41-51
Timestamp: 2025-10-08T17:13:13.155Z
Learning: In `src/app/(mobile-ui)/points/page.tsx`, the icon name "invite-heart" is intentionally used (not "inviter-heart") when displaying who invited the current user, as this is a deliberate design choice despite semantic differences with UserHeader usage.
Applied to files:
src/app/(mobile-ui)/points/page.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/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
🧬 Code graph analysis (2)
src/app/(mobile-ui)/points/page.tsx (1)
src/utils/general.utils.ts (1)
generateInviteCodeLink(1349-1354)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
src/components/0_Bruddle/Button.tsx (1)
Button(76-267)
🔇 Additional comments (7)
redirects.json (1)
40-40: Verify both redirects point to the same destination.Both
/pintsand/eventsnow redirect to the identical Luma URL (https://luma.com/zwh7l3ro). Please confirm this is intentional and not a copy-paste error.Also applies to: 45-45
src/app/[...recipient]/client.tsx (1)
470-470: Verify z-index layering doesn't conflict with other UI elements.The banner now has
z-50on mobile viewports, which is a high stacking priority. Confirm this doesn't obscure navigation, modals, dropdowns, or other interactive elements that might appear over the banner.src/utils/general.utils.ts (3)
1340-1347: LGTM!The simple character-code hash is appropriate for generating a cosmetic suffix. While collisions are possible (e.g., anagrams produce identical hashes), this is acceptable since the suffix is purely decorative and doesn't affect functionality.
1349-1354: LGTM!Clean refactor that extracts the suffix generation logic into a dedicated helper function. The invite code format remains consistent, and returning both
inviteLinkandinviteCodeprovides better API ergonomics for consumers.
1356-1372: LGTM!Proper URL decoding and sanitization with fallback handling. The function correctly leverages the existing
sanitizeRedirectURLlogic to prevent open redirect vulnerabilities.src/app/(mobile-ui)/points/page.tsx (1)
14-15: LGTM!Excellent refactor that eliminates code duplication by leveraging the new
generateInviteCodeLinkutility. The destructured return value provides cleaner code compared to manual string concatenation.Also applies to: 30-30
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
255-270: LGTM!Solid implementation of the non-Euro SEPA handling:
- Clear warning UI with appropriate styling
- Buttons correctly disabled when service is unavailable
- User-friendly messaging about temporary unavailability
The UX gracefully informs users about the limitation without breaking the flow.
Also applies to: 274-274, 291-291, 294-294
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (2)
46-52: Consider extracting the supported currencies to a constant.The hardcoded currency exclusions (EUR, USD, MXN) make it unclear why these specific currencies are allowed while others are temporarily unavailable. Consider extracting these to a named constant with documentation explaining the business logic.
+// Currencies currently supported for bank withdrawals +const SUPPORTED_WITHDRAWAL_CURRENCIES = ['EUR', 'USD', 'MXN'] as const + // non-eur sepa countries that are currently experiencing issues -const isNonEuroSepaCountry = !!( - nonEuroCurrency && - nonEuroCurrency !== 'EUR' && - nonEuroCurrency !== 'USD' && - nonEuroCurrency !== 'MXN' -) +const isNonEuroSepaCountry = !!( + nonEuroCurrency && + !SUPPORTED_WITHDRAWAL_CURRENCIES.includes(nonEuroCurrency) +)
271-286: Clear and user-friendly warning UI.The warning message effectively communicates the temporary unavailability. The styling and structure are appropriate for this use case.
Consider enhancing the message with actionable guidance:
<p className="mt-1 text-xs text-yellow-700"> - Withdrawals to {nonEuroCurrency} bank accounts are temporarily unavailable. - Please try again later. + Withdrawals to {nonEuroCurrency} bank accounts are temporarily unavailable. + You can withdraw to EUR, USD, or MXN accounts in the meantime. </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/(mobile-ui)/points/page.tsx(2 hunks)src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx(3 hunks)src/app/[...recipient]/client.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/(mobile-ui)/points/page.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 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
🧬 Code graph analysis (1)
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
src/components/0_Bruddle/Button.tsx (1)
Button(76-267)
🪛 GitHub Actions: Tests
src/app/(mobile-ui)/withdraw/[country]/bank/page.tsx
[warning] 1-1: Code style issues found by Prettier. Run 'pnpm prettier --write' to fix.
⏰ 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/app/(mobile-ui)/withdraw/[country]/bank/page.tsx (1)
290-290: Disabled button states correctly implemented.The logic properly prevents withdrawals for non-Euro SEPA countries in both the Retry and Withdraw button paths. The label change to "Temporarily Unavailable" provides clear user feedback.
Also applies to: 307-310
src/app/[...recipient]/client.tsx (1)
497-497: Verify z-index handling for mobile stacking context.The z-50 on mobile creates potential stacking conflicts with other components using the same z-index value: tooltips, drawers, modals, loading overlays, and the QR scanner (all z-50). Stacking order will depend on DOM ordering when z-index values are equal. The QR scanner, in particular, is a full-screen overlay that could appear behind this banner unintentionally.
Consider using a lower z-index on mobile (e.g., z-20 or z-30) unless this banner must consistently overlay interactive elements, or verify that this component and conflicting overlays are never rendered simultaneously.
3 participants
pull main