fix: various cashout fixes #489
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces several modifications across multiple files. Key changes include updating the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 1
🧹 Outside diff range and nitpick comments (2)
src/app/api/peanut/iban/validate-bank-account-number/route.ts (1)
10-12: Enhance error message specificity.The current error message doesn't specify which parameter is missing. Consider providing more detailed feedback.
- return new NextResponse('Bad Request: missing required parameters', { status: 400 }) + const missingParams = []; + if (!bankAccountNumber) missingParams.push('bankAccountNumber'); + if (!apiKey) missingParams.push('apiKey'); + return new NextResponse(`Bad Request: missing required parameters: ${missingParams.join(', ')}`, { status: 400 })src/utils/cashout.utils.ts (1)
Line range hint
384-388: Consider simplifying the return statement.The if-else structure can be simplified to directly return the comparison result.
- if (response.status !== 200) { - return false - } else { - return true - } + return response.status === 200
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- .env.example (1 hunks)
- src/app/api/peanut/iban/validate-bank-account-number/route.ts (1 hunks)
- src/components/Cashout/Components/Initial.view.tsx (1 hunks)
- src/components/Global/KYCComponent/index.tsx (1 hunks)
- src/constants/general.consts.ts (1 hunks)
- src/utils/cashout.utils.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .env.example
- src/components/Global/KYCComponent/index.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Cashout/Components/Initial.view.tsx (1)
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.
🔇 Additional comments (7)
src/app/api/peanut/iban/validate-bank-account-number/route.ts (3)
20-22: LGTM! Request body update is consistent.The request body parameter has been correctly updated to use the new
bankAccountNumbername, maintaining consistency with the variable renaming.
14-14: Verify PEANUT_API_URL configuration.The endpoint URL has been updated to match the new naming convention. Please ensure that
consts.PEANUT_API_URLis properly configured in all environments.#!/bin/bash # Check for PEANUT_API_URL configuration rg -l "PEANUT_API_URL" --type ts
7-7: Verify client-side compatibility with the renamed parameter.The variable rename from
bankAccounttobankAccountNumberimproves clarity, but ensure all client-side code has been updated to use the new parameter name.src/constants/general.consts.ts (1)
15-15: Verify URL concatenation patterns in API calls.Let's ensure that the API URL is being properly concatenated in all API calls to prevent double-slash issues.
✅ Verification successful
URL concatenation patterns are properly handled across the codebase
All API endpoint URLs are constructed using template literals with proper path joining. The base URL has trailing slashes removed, and endpoints are concatenated with a forward slash separator. No instances of double-slash issues were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for PEANUT_API_URL usage patterns echo "Checking PEANUT_API_URL usage patterns..." rg "PEANUT_API_URL.*/" -A 2 # Look for potential double slash issues rg "PEANUT_API_URL.*\+" -A 2 # Look for string concatenation rg "PEANUT_API_URL.*\`" -A 2 # Look for template literal usageLength of output: 18032
src/utils/cashout.utils.ts (2)
373-374: LGTM! Good improvements to input handling and naming.The changes improve the code by:
- Normalizing input by removing whitespace
- Using more descriptive parameter names
- Making the API endpoint name more specific
Also applies to: 380-380
374-374: Verify API endpoint renaming across the codebase.Let's ensure the API endpoint renaming is consistent across all files.
✅ Verification successful
The results show that the new endpoint
validate-bank-account-numberis used consistently in both the API route definition (src/app/api/peanut/iban/validate-bank-account-number/route.ts) and its usage in the utility file (src/utils/cashout.utils.ts). There are no references to any old endpoint format found in the codebase.Let's verify the implementation to ensure complete consistency:
API endpoint is consistently implemented and used
The API endpoint
/api/peanut/iban/validate-bank-account-numberis correctly implemented in the route handler and properly consumed in the utility function. Both the implementation and usage follow the same naming convention and structure, with no traces of old endpoint patterns in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old API endpoint and verify the new endpoint usage # Test 1: Check for old endpoint references echo "Checking for old endpoint references..." rg -l "validate-bank-account[^-]" # Test 2: Verify new endpoint usage echo "Verifying new endpoint usage..." rg -l "validate-bank-account-number"Length of output: 323
Script:
#!/bin/bash # Check the actual implementation and usage echo "API Route Implementation:" cat src/app/api/peanut/iban/validate-bank-account-number/route.ts echo -e "\nAPI Usage:" rg -A 2 -B 2 "validate-bank-account-number" src/utils/cashout.utils.tsLength of output: 1943
src/components/Cashout/Components/Initial.view.tsx (1)
114-114: LGTM! Improved error message clarity.The updated error message provides better guidance by directing users to support when they encounter bank account validation issues, rather than leaving them with technical details about account support.
| ] | ||
|
|
||
| export const PEANUT_API_URL = process.env.PEANUT_API_URL || 'https://api.peanut.to' | ||
| export const PEANUT_API_URL = (process.env.PEANUT_API_URL || 'https://api.peanut.to').replace(/\/$/, '') // remove any accidental trailing slash |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using URL constructor for robust URL handling.
While removing trailing slashes is good practice, consider using the URL constructor for more robust URL handling and validation:
-export const PEANUT_API_URL = (process.env.PEANUT_API_URL || 'https://api.peanut.to').replace(/\/$/, '') // remove any accidental trailing slash
+export const PEANUT_API_URL = (() => {
+ try {
+ const url = new URL(process.env.PEANUT_API_URL || 'https://api.peanut.to');
+ return url.href.replace(/\/$/, '');
+ } catch (e) {
+ console.error('Invalid PEANUT_API_URL format');
+ return 'https://api.peanut.to';
+ }
+})()This approach provides several benefits:
- Validates URL format
- Handles malformed URLs gracefully
- Still maintains the trailing slash removal
📝 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.
| export const PEANUT_API_URL = (process.env.PEANUT_API_URL || 'https://api.peanut.to').replace(/\/$/, '') // remove any accidental trailing slash | |
| export const PEANUT_API_URL = (() => { | |
| try { | |
| const url = new URL(process.env.PEANUT_API_URL || 'https://api.peanut.to'); | |
| return url.href.replace(/\/$/, ''); | |
| } catch (e) { | |
| console.error('Invalid PEANUT_API_URL format'); | |
| return 'https://api.peanut.to'; | |
| } | |
| })() |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/components/Cashout/Components/Initial.view.tsx (1 hunks)
- src/utils/cashout.utils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/cashout.utils.ts
🧰 Additional context used
📓 Learnings (1)
src/components/Cashout/Components/Initial.view.tsx (1)
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.
| const recipientBankAccount = selectedBankAccount || newBankAccount | ||
|
|
||
| const validAccount = await utils.validateBankAccount(recipientBankAccount) | ||
| if (!validAccount) { | ||
| console.error('Invalid bank account') | ||
| setErrorState({ | ||
| showError: true, | ||
| errorMessage: 'Invalid bank account. Reach out to support if you need help.', | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
Fix undefined variables and improve error handling.
Several issues need to be addressed in this code segment:
-
The variables
selectedBankAccountandnewBankAccountappear to be undefined. The code should use thebankAccountNumberstate variable that's already being managed by the component. -
According to the previous learning from PR [ISSUE-252] refactor(cashout): use ValidatedInput for account number #484, linked bank accounts should not generate error states as the
ValidatedInputcomponent will handle error messages. -
The error handling is incomplete as it doesn't set the loading state to 'Idle' when returning after an error.
Apply this diff to fix the issues:
- const recipientBankAccount = selectedBankAccount || newBankAccount
-
- const validAccount = await utils.validateBankAccount(recipientBankAccount)
+ const validAccount = await utils.validateBankAccount(bankAccountNumber)
if (!validAccount) {
console.error('Invalid bank account')
setErrorState({
showError: true,
errorMessage: 'Invalid bank account. Reach out to support if you need help.',
})
+ setLoadingState('Idle')
return
}Committable suggestion was skipped due to low confidence.
No description provided.