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

POS-1253 Allow Canadian routing numbers #125

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

citybasecnocon
Copy link
Contributor

@citybasecnocon citybasecnocon commented Jul 31, 2024

Description

The impetus for this set of changes to the Redux Freeform library are meant to allow Canadian routing numbers in the Manual Entry form used in POS Frontend. While we want to allow Canadian numbers for recording purposes, we do not need to make sure the Canadian numbers are all real routing numbers because Canadian checks cannot be processed as ACH transactions anyway. According to Greg Weppler,

The routing number is definitely used for payments, like for submitting an ACH payment, but Canadian checks can't be processed as an ACH. Canadian checking (Canadian routing numbers) will only ever be reported, and will fail if added to an ACH file.

So it doesn't matter if the Canadian routing numbers are for real banks because we can't process those as ACH transactions even if they were real numbers. Therefore, I've added a new validator that preserves the pre-existing checksum validation for U.S. routing numbers and adds functionality that allows Canadian numbers to be entered and recorded, but not used in any real transactions.

This new validator, isAmericanOrCanadianRoutingNumber can be applied to any routing number input field that wants to allow the entry of Canadian routing numbers without disrupting the validation for U.S. numbers.

View in preview environment.

Risk Assessment

None. This is an update to a library that will not impact anything until its installed in POS Frontend. See https://github.com/CityBaseInc/pos-frontend/pull/526.

Changes

  • Add content to validation API README for isAmericanOrCanadianRoutingNumber validator.
  • Add tests for isAmericanOrCanadianRoutingNumber validator
  • Add new isAmericanOrCanadianRoutingNumber validator to allow Canadian values

Code of Conduct

https://github.com/CityBaseInc/redux-freeform/blob/master/CODE_OF_CONDUCT.md

Concerns

(Optional)

Screenshots

9-Digit Routing Number Checksum is Valid

Screenshot 2024-08-01 at 11 20 48 AM

9-Digit Routing Number Checksum is Invalid

Screenshot 2024-08-01 at 11 20 23 AM

Routing Number Length is Too Short

Screenshot 2024-07-31 at 5 28 09 AM

An 8-digit number is valid

Screenshot 2024-07-31 at 5 28 16 AM

Routing Number Length is Too Long

Screenshot 2024-07-31 at 5 28 39 AM

@citybasecnocon citybasecnocon changed the title Pos 1253 allow canadian routing numbers POS-1253 Allow Canadian routing numbers Jul 31, 2024
@citybasecnocon citybasecnocon self-assigned this Jul 31, 2024
@citybasecnocon citybasecnocon marked this pull request as ready for review July 31, 2024 11:22
@citybasecnocon citybasecnocon force-pushed the POS-1253-allow-canadian-routing-numbers branch from e2c0d00 to 1f409b4 Compare July 31, 2024 11:24
@citybasecnocon citybasecnocon requested review from sjt003, mattr-cb, otrig-cb and a team July 31, 2024 11:32
@jpena-cb
Copy link

I believe using checksum is important for detecting fraud. Is there a way to handle this in a country-specific way?

@citybasecnocon
Copy link
Contributor Author

citybasecnocon commented Jul 31, 2024

I believe using checksum is important for detecting fraud. Is there a way to handle this in a country-specific way?

@jpena-cb You're correct. The reason I have to remove it is because Canadian routing numbers are sometimes 9 digits instead of 8, so there's not a reliable way to tell which type of number you're dealing with before applying validations.

@citybasecnocon citybasecnocon marked this pull request as draft August 1, 2024 12:33
@citybasecnocon citybasecnocon force-pushed the POS-1253-allow-canadian-routing-numbers branch 3 times, most recently from 643e34e to 83c215b Compare August 1, 2024 17:23
@citybasecnocon citybasecnocon marked this pull request as ready for review August 1, 2024 17:44
@citybasecnocon
Copy link
Contributor Author

@jpena-cb Could you take another look at this PR when you get a chance?

jpena-cb
jpena-cb previously approved these changes Aug 2, 2024
Copy link

@jpena-cb jpena-cb left a comment

Choose a reason for hiding this comment

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

Looks good!

@mattr-cb mattr-cb force-pushed the POS-1253-allow-canadian-routing-numbers branch from 83c215b to c19b85c Compare December 9, 2024 19:32
yarn 1.22.19
nodejs 20.17.0
Copy link

Choose a reason for hiding this comment

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

I added this, it matches the dockerfile

@mattr-cb
Copy link

mattr-cb commented Dec 9, 2024

Tests needed to be updated to match these changes 2559d6b#diff-56017878dbd7a7cfc83c47102fb03e5116813578d085592361f7247fbc0e70a5R566

@mattr-cb mattr-cb force-pushed the POS-1253-allow-canadian-routing-numbers branch from c19b85c to ac03769 Compare December 9, 2024 19:43
@mattr-cb mattr-cb force-pushed the POS-1253-allow-canadian-routing-numbers branch from ac03769 to a8195cb Compare December 9, 2024 19:49
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.

5 participants