Skip to content
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

Added routing number validation #5848

Merged

Conversation

Santhosh-Sellavel
Copy link
Collaborator

Details

Implemented the routing number validation algorithm that was confirmed here & used it to validate.

Fixed Issues

$ #5762

Tests & QA Steps

  1. Go to Workspace
  2. Click Connect Bank Account
  3. Choose Connect Manually
  4. Enter Invalid routing numbers, eg (123456789)
  5. On click on Save & continue
  6. Will show invalid routing number on the same page itself.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

WEB

Mobile Web

mWEB

Desktop

DESKTOP

iOS

iOS

Android

Android

@Santhosh-Sellavel Santhosh-Sellavel requested a review from a team as a code owner October 14, 2021 16:09
@MelvinBot MelvinBot requested review from Beamanator and removed request for a team October 14, 2021 16:09
@kevinksullivan
Copy link
Contributor

Just to clarify @Santhosh-Sellavel @Beamanator this impacts N6, and should not be held.

@Santhosh-Sellavel
Copy link
Collaborator Author

@kevinksullivan Since the issue has n6-label, I put hold in title. Will remove now.

@Santhosh-Sellavel Santhosh-Sellavel changed the title [HOLD N6] Added validate routing number Added validate routing number Oct 16, 2021
@Santhosh-Sellavel Santhosh-Sellavel changed the title Added validate routing number Added routing number validation Oct 16, 2021
@kevinksullivan
Copy link
Contributor

Thanks @Santhosh-Sellavel . I went ahead and removed the label on the issue.

Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

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

A few tiny requested changes, overall looks & tests great 👍

src/libs/actions/BankAccounts.js Outdated Show resolved Hide resolved
src/libs/actions/BankAccounts.js Show resolved Hide resolved
src/libs/actions/BankAccounts.js Outdated Show resolved Hide resolved
src/libs/actions/BankAccounts.js Show resolved Hide resolved
src/libs/actions/BankAccounts.js Outdated Show resolved Hide resolved
@Beamanator
Copy link
Contributor

Code looking good, but I'll re-review once iOS tests pass (probably failing due to this)

Copy link
Contributor

@Beamanator Beamanator left a comment

Choose a reason for hiding this comment

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

LGTM 💪

@Beamanator Beamanator merged commit d4b934f into Expensify:main Oct 21, 2021
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @Beamanator in version: 1.1.8-10 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants