feat: enforce 10-digit NUBAN validation on Swap (fix 11-digit acceptance)#372
Open
sundayonah wants to merge 3 commits intomainfrom
Open
feat: enforce 10-digit NUBAN validation on Swap (fix 11-digit acceptance)#372sundayonah wants to merge 3 commits intomainfrom
sundayonah wants to merge 3 commits intomainfrom
Conversation
sundayonah
requested review from
5ran6,
chibie and
onahprosper
as code owners
Contributor
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdded digit-only length validation for account identifiers in both the recipients POST API and the RecipientDetailsForm: strips non-digits, requires 6 digits for Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (UI)
participant Form as RecipientDetailsForm
participant API as Recipients API
participant DB as Database
participant Tracker as Error Tracker
rect rgba(200,200,255,0.5)
User->>Form: Enter accountIdentifier
Form->>Form: extract digits, determine required length (6 or 10)
alt invalid length
Form-->>User: show length error (6 or 10 digits)
else valid length
Form->>API: POST sanitized digits / request name-resolve
API->>API: validate digits length again
alt invalid length
API->>Tracker: track validation error
API-->>Form: 400 with required-length message
else valid
API->>DB: insert/upsert sanitized digits & trimmed institution_code
DB-->>API: confirm write
API-->>Form: 200 OK / recipient data
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@app/api/v1/recipients/route.ts`:
- Around line 155-176: Validation currently strips non-digits into the variable
digits and uses digits.length to validate, but the code later persists the raw
accountIdentifier (trimmed) which may retain spaces or non-digit chars; update
the persistence to save the sanitized digits variable (or a normalized version
like digits.trim()) instead of accountIdentifier.trim(), and ensure any
downstream uses (e.g., recipient creation code around the save) expect the
numeric form; also standardize the error message text to use consistent
capitalization ("account number") in both NextResponse.json branches so messages
match.
In `@app/components/recipient/RecipientDetailsForm.tsx`:
- Around line 392-415: Extract the repeated ternary (selectedInstitution?.code
=== "SAFAKEPC" ? 6 : 10) into a single derived constant (e.g.,
requiredAccountLen) at the component level, then replace all usages in
maxLength, the register validate function for "accountIdentifier", and the
useEffect that currently computes requiredLen so they all reference
requiredAccountLen; ensure validation logic still compares digits.length to
requiredAccountLen and error messages remain conditional based on whether
requiredAccountLen === 10.
- Around line 394-396: The text input in RecipientDetailsForm (the input with
type="text" and inputMode="numeric") currently allows letters to be typed or
pasted; add an onInput handler on that input element in the RecipientDetailsForm
component that strips non-digit characters (e.g., replace non-digits with empty
string) so maxLength counts only digits and pasted text is sanitized; ensure the
handler uses e.currentTarget.value (or the equivalent event target) to update
the input value in-place.
🧹 Nitpick comments (2)
🤖 Fix all nitpicks with AI agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@app/components/recipient/RecipientDetailsForm.tsx`: - Around line 392-415: Extract the repeated ternary (selectedInstitution?.code === "SAFAKEPC" ? 6 : 10) into a single derived constant (e.g., requiredAccountLen) at the component level, then replace all usages in maxLength, the register validate function for "accountIdentifier", and the useEffect that currently computes requiredLen so they all reference requiredAccountLen; ensure validation logic still compares digits.length to requiredAccountLen and error messages remain conditional based on whether requiredAccountLen === 10. - Around line 394-396: The text input in RecipientDetailsForm (the input with type="text" and inputMode="numeric") currently allows letters to be typed or pasted; add an onInput handler on that input element in the RecipientDetailsForm component that strips non-digit characters (e.g., replace non-digits with empty string) so maxLength counts only digits and pasted text is sanitized; ensure the handler uses e.currentTarget.value (or the equivalent event target) to update the input value in-place.app/components/recipient/RecipientDetailsForm.tsx (2)
392-415: Consider extracting duplicaterequiredLenlogic.The expression
selectedInstitution?.code === "SAFAKEPC" ? 6 : 10is repeated in three places: theuseEffect(Line 253),maxLength(Line 398), andvalidate(Line 406). A derived constant at the component level would reduce duplication and the risk of these falling out of sync.Suggested refactor
Add near the other derived values (e.g., after Line 46):
const requiredAccountLen = selectedInstitution?.code === "SAFAKEPC" ? 6 : 10;Then reference
requiredAccountLenin all three locations.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@app/components/recipient/RecipientDetailsForm.tsx` around lines 392 - 415, Extract the repeated ternary (selectedInstitution?.code === "SAFAKEPC" ? 6 : 10) into a single derived constant (e.g., requiredAccountLen) at the component level, then replace all usages in maxLength, the register validate function for "accountIdentifier", and the useEffect that currently computes requiredLen so they all reference requiredAccountLen; ensure validation logic still compares digits.length to requiredAccountLen and error messages remain conditional based on whether requiredAccountLen === 10.
394-396: Optional: filter non-digit input on keystrokes.Since the input is
type="text", users can type or paste letters. While validation catches this, you could strip non-digits on input for a smoother UX:onInput={(e) => { e.currentTarget.value = e.currentTarget.value.replace(/\D/g, ""); }}This would make
maxLengthcount only actual digits and prevent confusion.🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@app/components/recipient/RecipientDetailsForm.tsx` around lines 394 - 396, The text input in RecipientDetailsForm (the input with type="text" and inputMode="numeric") currently allows letters to be typed or pasted; add an onInput handler on that input element in the RecipientDetailsForm component that strips non-digit characters (e.g., replace non-digits with empty string) so maxLength counts only digits and pasted text is sanitized; ensure the handler uses e.currentTarget.value (or the equivalent event target) to update the input value in-place.
…nitized digits for consistent database format
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/api/v1/recipients/route.ts (1)
155-176:institutionCodeshould be trimmed before the comparison on Line 157.On Line 234,
institutionCodeis trimmed for DB storage, but on Line 157 it's compared raw. If the client sends" SAFAKEPC ", the 6-digit branch won't match and the user gets an incorrect "enter a valid 10-digit account number" error for a SAFAKEPC account.Proposed fix
- const requiredLen = institutionCode === "SAFAKEPC" ? 6 : 10; + const trimmedInstitutionCode = institutionCode.trim(); + const requiredLen = trimmedInstitutionCode === "SAFAKEPC" ? 6 : 10;Then reuse
trimmedInstitutionCodeon Line 234 as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/v1/recipients/route.ts` around lines 155 - 176, Trim institutionCode before using it to decide requiredLen and reuse that trimmed value later; specifically, create or move the trimmedInstitutionCode (or compute const trimmedInstitutionCode = String(institutionCode).trim()) above the validation block that computes requiredLen and use trimmedInstitutionCode in the ternary check (for "SAFAKEPC") and replace later raw institutionCode use on DB storage with trimmedInstitutionCode so both validation and storage use the same normalized value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/api/v1/recipients/route.ts`:
- Around line 225-235: The saved_recipients upsert was previously persisting
un-sanitized account identifiers; replace any use of accountIdentifier.trim()
with the sanitized digits variable so the DB stores only digits — ensure the
upsert object in the route.ts upsert call uses account_identifier: digits (and
keep name.trim(), institution.trim(), institution_code.trim(),
normalized_wallet_address/wallet_address as shown) to maintain consistent,
sanitized storage.
---
Nitpick comments:
In `@app/api/v1/recipients/route.ts`:
- Around line 155-176: Trim institutionCode before using it to decide
requiredLen and reuse that trimmed value later; specifically, create or move the
trimmedInstitutionCode (or compute const trimmedInstitutionCode =
String(institutionCode).trim()) above the validation block that computes
requiredLen and use trimmedInstitutionCode in the ternary check (for "SAFAKEPC")
and replace later raw institutionCode use on DB storage with
trimmedInstitutionCode so both validation and storage use the same normalized
value.
… in recipient API
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Purpose: Enforce the NUBAN rule that account numbers must be exactly 10 digits (or 6 for SAFAKEPC) on the Swap flow. Previously, 11-digit values (e.g. OPay-style phone numbers like
08012345678) were accepted, the UI showed a successful account name resolution, and the transaction later failed at execution. This change validates length before resolving the account name so invalid input is rejected in the UI and at the API.Background: NUBAN (Nigerian Uniform Bank Account Number) is 10 digits. The previous check used
minLength: 10, so 11+ digits still passed. We now require exact length (10 or 6 for SAFAKEPC) and block submission / API calls when the length is wrong.Changes:
RecipientDetailsForm (Swap screen):
type="number"→type="text"withinputMode="numeric"andmaxLength10 (or 6 for SAFAKEPC).validatethat strips non-digits and requires length 10 (or 6 for SAFAKEPC). Invalid input shows: "Please enter a valid 10-digit account Number." (or the 6-digit message for SAFAKEPC).API routes (defence in depth):
/api/v1/account/verify: Before calling the aggregator, validates thataccountIdentifier(digits only) has length 10 or 6 (SAFAKEPC). Otherwise returns 400 with the same error message./api/v1/recipients: Before saving a beneficiary, same length check so 11-digit (or other invalid length) account numbers are never stored.Impacts:
Alternatives considered: Normalising 11-digit numbers (e.g. strip leading
0) was considered for backward compatibility but not implemented so we stay aligned with NUBAN and avoid provider-specific assumptions.References
#Closes #371
Testing
Manual testing:
Swap screen – 11-digit rejection
08012345678) in Account Number.8012345678). Expected: Account name resolves if valid; no length error.10-digit acceptance
Saving beneficiary
Checklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit