validator.js
validator.js copied to clipboard
Feat: enchance isCreditCard
Add regex validation for AmEx card
hey @rubiin i have refactored the isCreditCard file, ~~will be submitting~~ just submitted a new PR ~~shortly~~. Thanks for you suggestions!
Codecov Report
Merging #2008 (0eaded6) into master (1bb14e8) will not change coverage. The diff coverage is
100.00%
.
@@ Coverage Diff @@
## master #2008 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 104 104
Lines 2203 2210 +7
Branches 477 481 +4
=========================================
+ Hits 2203 2210 +7
Impacted Files | Coverage Δ | |
---|---|---|
src/lib/isCreditCard.js | 100.00% <100.00%> (ø) |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Thank you @rubiin. @profnandaa when you have an opportunity please do look at the PR. Much appreciated!
apologies, thought this was approved and started cleaning up too soon. PR is reopened and waiting for approval from @profnandaa
@rubiin - i did modify the existing isCreditCard file, and ensured it was backwards compatible. can you evaluate the diff? you will see you can still pass in a credit card number without the second param, and still get the same result. Thanks
Hi @rubiin and @profnandaa - is there an additional person that can approve this PR? I would like to get it merged to main. Thank you!
Lets wait for @profnandaa
@profnandaa and @rubiin , is there an additional person that can approve this PR? I would like to get it merged to main. Thank you!
@WikiRik would you be able to review this PR ? Thank you!
@brianwhaley I can, but my review won't get this merged faster unfortunately. We'll have to wait for @profnandaa
Either way you can still use this specific commit in other projects, see this NPM docs
@profnandaa and @rubiin , this PR has been open for almost a month. Is there someone we can add to be the final reviewer? Thank you!
I would suggest you wrap the providers
optional parameter in an object. See this issue for more context about that https://github.com/validatorjs/validator.js/issues/1874
As you can see from the discussion there this has not been cemented as the official guideline yet, but I think we should push in that direction.
@braaar thanks for the feedback, made the adjustment for that.
cc: @profnandaa and @rubiin
@braaar @profnandaa @rubiin @chriso @ezkemboi @tux-tn This PR is open for almost 2 months. It would be helpful for me and my team if we could review and close this PR. What can I do to get this over the finish line? Thank you!
Sorry for the delayed review on this!
@brianwhaley Could you please provide a source for the union pay credit card validation pattern? We tried searching for the pattern, but couldn't find any information regarding union pay other that a 60/62 prefix and some length requirements.
@brianwhaley Could you please provide a source for the union pay credit card validation pattern? We tried searching for the pattern, but couldn't find any information regarding union pay other that a 60/62 prefix and some length requirements.
I would have to check, but I believe that was there before me. Issue 1680 for validator.js was opened because Union Pay cards with 81 range fail.
https://github.com/validatorjs/validator.js/issues/1680
@brianwhaley According to https://docs.trellix.com/bundle/data-loss-prevention-11.10.x-classification-definitions-reference-guide/page/GUID-B8D29ECE-E70A-401E-B18D-B773F4FF71ED.html
The China UnionPay credit card numbers begin with 62 or 60 and is a 16-19 digit long number.
Validator Description China Union Pay Card validator The validator extracts only numbers and: Validates the length of the string and must have 16-19-digits; Verifies if the number begins with 62
#1680 used https://docs.adyen.com/development-resources/testing/test-card-numbers/#china-unionpay as a source which does include a sample number starting with 81. It does mention 'ExpressPay Credit Card (cup)', so maybe that's not exactly the same as China Union Pay Card? I'm not very familiar with creditcards but we would welcome a PR with additional sources if the current regex is wrong