validator.js icon indicating copy to clipboard operation
validator.js copied to clipboard

Feat: enchance isCreditCard

Open brianwhaley opened this issue 1 year ago • 14 comments

Add regex validation for AmEx card

brianwhaley avatar Jul 27 '22 05:07 brianwhaley

hey @rubiin i have refactored the isCreditCard file, ~~will be submitting~~ just submitted a new PR ~~shortly~~. Thanks for you suggestions!

brianwhaley avatar Jul 27 '22 21:07 brianwhaley

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.

codecov[bot] avatar Jul 27 '22 21:07 codecov[bot]

Thank you @rubiin. @profnandaa when you have an opportunity please do look at the PR. Much appreciated!

brianwhaley avatar Jul 28 '22 12:07 brianwhaley

apologies, thought this was approved and started cleaning up too soon. PR is reopened and waiting for approval from @profnandaa

brianwhaley avatar Jul 28 '22 16:07 brianwhaley

@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

brianwhaley avatar Jul 28 '22 17:07 brianwhaley

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!

brianwhaley avatar Jul 29 '22 13:07 brianwhaley

Lets wait for @profnandaa

rubiin avatar Jul 29 '22 14:07 rubiin

@profnandaa and @rubiin , is there an additional person that can approve this PR? I would like to get it merged to main. Thank you!

brianwhaley avatar Aug 05 '22 03:08 brianwhaley

@WikiRik would you be able to review this PR ? Thank you!

brianwhaley avatar Aug 10 '22 17:08 brianwhaley

@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

WikiRik avatar Aug 10 '22 17:08 WikiRik

@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!

brianwhaley avatar Aug 22 '22 00:08 brianwhaley

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 avatar Aug 22 '22 12:08 braaar

@braaar thanks for the feedback, made the adjustment for that.

cc: @profnandaa and @rubiin

brianwhaley avatar Aug 22 '22 20:08 brianwhaley

@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!

brianwhaley avatar Sep 17 '22 18:09 brianwhaley

Sorry for the delayed review on this!

profnandaa avatar Oct 17 '22 04:10 profnandaa

@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.

ST-DDT avatar Sep 01 '23 12:09 ST-DDT

@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 avatar Sep 01 '23 14:09 brianwhaley

@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

CoolPlayLin avatar Sep 10 '23 03:09 CoolPlayLin

#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

WikiRik avatar Sep 15 '23 18:09 WikiRik