Skip to content

fix: should be clearer for US users how to enter their bank account #514

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Nov 5, 2024
Merged

fix: should be clearer for US users how to enter their bank account #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c61b1ce
fix: should be clearer for US users how to enter their bank account
Hugo0 Nov 5, 2024
a5facd7
feat: Add explanatory description
panosfilianos Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/Cashout/Components/Initial.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ export const InitialCashoutView = ({
setErrorState({
showError: true,
errorMessage:
'Invalid bank account. Please make sure your account is supported',
'Invalid bank account. For US bank accounts, enter your bank routing number (9 digits) followed by your account number \
(example: 1112223330001234567 where routing number is: 111222333 and account number: 0001234567)',
})
} else {
setErrorState({
Expand Down
20 changes: 16 additions & 4 deletions src/components/Cashout/Components/RecipientInfo.comp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,27 @@ export const RecipientInfoComponent = ({ className }: { className?: string }) =>
{/* TODO: these need to be standardized. It's dumb to have 10000 tw classes in here but also in Faq.comp.tsx etc... */}
<Menu.Items className="shadow-primary-4 absolute bottom-full left-0 z-[999] mb-1 mr-1 w-64 border border-n-1 bg-white px-4 py-2 md:left-0 md:right-auto">
<Menu.Item as={'label'} className={'text-h8 font-normal'}>
You can claim directly to your IBAN OR US bank account. Click{' '}
You can claim directly to your IBAN OR US bank account.
<br />
<br />
For US bank accounts, enter your
bank routing number (9 digits) and account number together.
<br />
<br />
Example US bank account: 1112223330001234567, with:
<br />
Routing number (9 digits): 111222333
<br />
Account number: 0001234567
<br />
<br />
<a
href="https://docs.peanut.to/app/cashout/supported-geographies"
target="_blank"
className="underline"
>
here
</a>{' '}
to see if your region is supported.
Supported regions
</a>
</Menu.Item>
</Menu.Items>
</Transition>
Expand Down
4 changes: 3 additions & 1 deletion src/components/Global/GeneralRecipientInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ const GeneralRecipientInput = ({ placeholder, recipient, onUpdate, className }:
type = 'iban'
isValid = await utils.validateBankAccount(recipient)
if (!isValid) errorMessage.current = 'Invalid IBAN, country not supported'
} else if (/^[0-9]{6,17}$/.test(recipient)) {
} else if (/^[0-9]{6,26}$/.test(recipient)) {
// routing number: 9 digits
// account number: 8-12 digits (can go up to 17)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: These checks may prove not to be enough down the line (as more geographies are added, maybe there is overlap in format detection). For now, I'm good with this.

type = 'us'
isValid = await utils.validateBankAccount(recipient)
if (!isValid) errorMessage.current = 'Invalid US account number'
Comment on lines +37 to 42
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding routing number validation

To further improve validation accuracy and user experience, consider adding specific routing number validation.

Add a helper function to validate the routing number format:

const isValidRoutingNumber = (routing: string): boolean => {
  if (!/^\d{9}$/.test(routing)) return false;
  
  // Implement checksum validation
  // Each routing number contains a checksum as its last digit
  const digits = routing.split('').map(Number);
  const checksum = (
    7 * (digits[0] + digits[3] + digits[6]) +
    3 * (digits[1] + digits[4] + digits[7]) +
    9 * (digits[2] + digits[5])
  ) % 10;
  
  return checksum === digits[8];
};

Then update the validation:

 } else if (/^[0-9]{9}[0-9]{8,12}$/.test(recipient)) {
     type = 'us'
+    const routingNumber = recipient.slice(0, 9);
+    if (!isValidRoutingNumber(routingNumber)) {
+      errorMessage.current = 'Invalid routing number. Please check and try again.';
+      return false;
+    }
     isValid = await utils.validateBankAccount(recipient)

⚠️ Potential issue

Strengthen the US bank account validation logic

The current validation has several potential issues:

  1. The regex /^[0-9]{6,26}$/ is too permissive:
    • Allows numbers shorter than required (routing number is 9 digits, account number is 8-12 digits)
    • Accepts numbers longer than typical US bank account formats
  2. No specific validation of routing number format
  3. No clear separation between routing and account numbers

Consider implementing this more robust validation:

-} else if (/^[0-9]{6,26}$/.test(recipient)) {
-    // routing number: 9 digits
-    // account number: 8-12 digits (can go up to 17)
+} else if (/^[0-9]{9}[0-9]{8,12}$/.test(recipient)) {
+    // Validate US bank account format:
+    // First 9 digits: routing number
+    // Following 8-12 digits: account number
     type = 'us'
     isValid = await utils.validateBankAccount(recipient)
-    if (!isValid) errorMessage.current = 'Invalid US account number'
+    if (!isValid) errorMessage.current = 'Please enter your 9-digit routing number followed by your 8-12 digit account number'

This change:

  • Enforces the exact format required for US bank accounts
  • Makes it clear that routing number must come first
  • Provides a more helpful error message
📝 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.

Suggested change
} else if (/^[0-9]{6,26}$/.test(recipient)) {
// routing number: 9 digits
// account number: 8-12 digits (can go up to 17)
type = 'us'
isValid = await utils.validateBankAccount(recipient)
if (!isValid) errorMessage.current = 'Invalid US account number'
} else if (/^[0-9]{9}[0-9]{8,12}$/.test(recipient)) {
// Validate US bank account format:
// First 9 digits: routing number
// Following 8-12 digits: account number
type = 'us'
isValid = await utils.validateBankAccount(recipient)
if (!isValid) errorMessage.current = 'Please enter your 9-digit routing number followed by your 8-12 digit account number'

Expand Down
3 changes: 2 additions & 1 deletion src/components/Global/LinkAccountComponent/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,8 @@ export const GlobaLinkAccountComponent = ({ accountNumber, onCompleted }: IGloba

if (isIBAN(sanitizedAccountNumber)) {
setAccountDetailsValue('type', 'iban')
} else if (/^[0-9]{6,17}$/.test(sanitizedAccountNumber)) {
// this code is also repeated in GeneralRecipientInput.tsx
} else if (/^[0-9]{6,26}$/.test(sanitizedAccountNumber)) {
Comment on lines +101 to +102
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Extract shared validation logic into a common utility.

The comment indicates code duplication between components. Consider extracting the bank account validation logic into a shared utility function to improve maintainability.

// Create a new file: src/utils/bankValidation.ts
+export const validateBankAccountFormat = (accountNumber: string): { isValid: boolean; type: 'iban' | 'us' | null } => {
+  const sanitizedAccount = accountNumber.replaceAll(/\s/g, '').toLowerCase()
+  if (isIBAN(sanitizedAccount)) {
+    return { isValid: true, type: 'iban' }
+  }
+  if (/^[0-9]{6,26}$/.test(sanitizedAccount)) {
+    return { isValid: true, type: 'us' }
+  }
+  return { isValid: false, type: null }
+}

// In LinkAccountComponent and GeneralRecipientInput:
-// this code is also repeated in GeneralRecipientInput.tsx
-} else if (/^[0-9]{6,26}$/.test(sanitizedAccountNumber)) {
+const validation = validateBankAccountFormat(accountNumber)
+if (validation.isValid) {
+  setAccountDetailsValue('type', validation.type)

Committable suggestion skipped: line range outside the PR's diff.

setAccountDetailsValue('type', 'us')
} else {
setIbanFormError('accountNumber', { message: 'Invalid account number' })
Expand Down