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

Adding Maestro credit card ranges #2223

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

linediconsine
Copy link

Add Maestro brand credit cards ranges regexp

I used the ranges provided credit-card-type here

This should solve a problem that I am facing with some Mastro cards on a work project

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)
  • References provided in PR (where applicable)

Copy link
Member

@rubiin rubiin left a comment

Choose a reason for hiding this comment

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

update readme as well

@matthewmayer
Copy link

@linediconsine this would be useful for the @faker-js/faker project. Would you be interested in rebasing this PR to get it merged, or if you don't have time I could make a new PR?

@linediconsine linediconsine force-pushed the fix/Add-Maestro-credit-card-ranges branch from 78921e6 to a816c87 Compare January 26, 2024 17:57
@linediconsine
Copy link
Author

linediconsine commented Jan 26, 2024

@rubiin Updated Readme and solved conflicts
Let me know if I need any other fix here

Thanks
(and sorry for the delay)

@linediconsine linediconsine force-pushed the fix/Add-Maestro-credit-card-ranges branch from a816c87 to 1ab5728 Compare January 26, 2024 18:40
@linediconsine linediconsine force-pushed the fix/Add-Maestro-credit-card-ranges branch from 1ab5728 to 1cfa3ad Compare January 26, 2024 18:47
Copy link

codecov bot commented Jan 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (b958bd7) to head (1cfa3ad).
Report is 53 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2223      +/-   ##
==========================================
- Coverage   99.95%   99.95%   -0.01%     
==========================================
  Files         107      107              
  Lines        2454     2449       -5     
  Branches      619      619              
==========================================
- Hits         2453     2448       -5     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rubiin
rubiin previously approved these changes Jan 27, 2024
WikiRik
WikiRik previously approved these changes Jan 27, 2024
@linediconsine
Copy link
Author

@profnandaa @tux-tn @pano9000 could you please review my PR ?
Thanks

@rubiin rubiin dismissed stale reviews from WikiRik and themself via 6c004b4 August 25, 2024 05:06
Copy link
Contributor

@pano9000 pano9000 left a comment

Choose a reason for hiding this comment

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

Regexp needs some tweaking still :-)

@@ -99,7 +99,7 @@ Validator | Description
**isBoolean(str [, options])** | check if the string is a boolean.<br/>`options` is an object which defaults to `{ loose: false }`. If `loose` is set to false, the validator will strictly match ['true', 'false', '0', '1']. If `loose` is set to true, the validator will also match 'yes', 'no', and will match a valid boolean string of any case. (e.g.: ['true', 'True', 'TRUE']).
**isBtcAddress(str)** | check if the string is a valid BTC address.
**isByteLength(str [, options])** | check if the string's length (in UTF-8 bytes) falls in a range.<br/><br/>`options` is an object which defaults to `{ min: 0, max: undefined }`.
**isCreditCard(str [, options])** | check if the string is a credit card number.<br/><br/> `options` is an optional object that can be supplied with the following key(s): `provider` is an optional key whose value should be a string, and defines the company issuing the credit card. Valid values include `['amex', 'dinersclub', 'discover', 'jcb', 'mastercard', 'unionpay', 'visa']` or blank will check for any provider.
**isCreditCard(str [, options])** | check if the string is a credit card number.<br/><br/> `options` is an optional object that can be supplied with the following key(s): `provider` is an optional key whose value should be a string, and defines the company issuing the credit card. Valid values include `['amex', 'dinersclub', 'discover', 'jcb', 'mastercard', 'unionpay', 'visa','maestro']` or blank will check for any provider.
Copy link
Contributor

Choose a reason for hiding this comment

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

  • could you kindly correctly alphabetically sort the values?
  • nitpicking: could you get rid of the extra space at the end the of the line :-)

@@ -7,6 +7,7 @@ const cards = {
discover: /^6(?:011|5[0-9][0-9])[0-9]{12,15}$/,
jcb: /^(?:2131|1800|35\d{3})\d{11}$/,
mastercard: /^5[1-5][0-9]{2}|(222[1-9]|22[3-9][0-9]|2[3-6][0-9]{2}|27[01][0-9]|2720)[0-9]{12}$/, // /^[25][1-7][0-9]{14}$/;
maestro: /^(493698|50(0[0-9]{3}|4(1[0-7][0-4]|7[6-9][0-9]))|50[678](7[789][0-9]|9[0-9]{2})|5[6-9]|63|67|6)\d{6,19}$/,
Copy link
Contributor

Choose a reason for hiding this comment

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

same nitpick as above: could you kindly correctly sort by alphabet, -> maestro should be before mastercard :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not 100% sure the regexp is 100% correct, e.g. this would return true for the following number:

6312345 - which has a length of 7 digits only

according to the source you provided the length is supposed to be
lengths: [12, 13, 14, 15, 16, 17, 18, 19]

'6283875070985593',
],
invalid: [
'foo',
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add some tests for too short numbers as well :-)

@pano9000 pano9000 removed the ✅ LGTM label Sep 7, 2024
@pano9000
Copy link
Contributor

pano9000 commented Sep 7, 2024

kind of related PR: #1701

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