fix: fixing isValid method #152
Conversation
… a valid checksum
| @@ -47,7 +48,7 @@ function validateCards(number, cards) { | |||
| } | |||
|
|
|||
| function hasCorrectLength(number) { | |||
| return number && number.length <= 19; | |||
| return number && number.length === CARD_NUMBER_LENGTH; | |||
There was a problem hiding this comment.
We cannot define that the card number must have 16 digits. For example AMEX has 15 digits, and some others up to 19, and sometimes 13.
https://www.pcidssguide.com/what-do-the-credit-card-numbers-mean/
There was a problem hiding this comment.
Alright, so following this article and the others bellow, the range of credit card number length is 12 to 19, right?!
In this way, I believe it is possible to define this range to avoid errors like the one described in the issue. What do you think?
https://www.discoverglobalnetwork.com/content/dam/discover/en_us/dgn/docs/IPP-VAR-Enabler-Compliance.pdf
https://www.forbes.com/advisor/credit-cards/what-does-your-credit-card-number-mean/
There was a problem hiding this comment.
I evaluated the list of supported cards and added the number of digits for each one:
American Express (15 digits)
Aura (19 digits)
Banescard (16 digits)
Cabal (16 digits)
Diners (14 digits)
Discover (16 digits)
Elo (16 digits)
Goodcard (16 digits)
Hipercard (16 digits)
Mastercard (16 digits)
Maxxvan (16 digits)
Visa (16 digits)
With this we can conclude that the cards must have between 14 and 19 digits.
What do you think of we add a "cardNumberLenght" property to each card in cards.js and create a method to check the minimum and maximum number of digits for a valid card? This way we avoid adding the number manually in the isValid method.
There was a problem hiding this comment.
Totally agree, looks like a good way. I can close this PR and open another one with this implementation.
There was a problem hiding this comment.
This is exactly why i did in another version :) and why in the public release i don't validate by length
What is the change?
Fix isValid method when an invalid card number is passed with a valid checksum
Why make this change?
In order to fix this issue: #139
Test plan