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

added new bins to 'MasterCard' #44

Merged
merged 3 commits into from Aug 22, 2016
Merged

Conversation

ronaldoflima
Copy link
Contributor

@ronaldoflima ronaldoflima commented Aug 1, 2016

New initial digits will be available in October for Mastercard :
222100 - 222199
222200 - 222999
223000 - 229999
230000 - 269999
270000 - 271999
272000 - 272099

Included new beginnings for Mastercard will be available in October: 
2221
2222
223
2[3-6]
27[01]
2720
@cacontinuos
Copy link
Collaborator

You changed source files, but no tests were changed.

It might be OK if this is a refactory and no behavior has changed,
otherwise, please add a test to guarantee that your code works.

@cacontinuos
Copy link
Collaborator

@rnldfll, thanks for your PR! By analyzing the blame information on this pull request, we identified @fernahh to be a potential reviewer

@fernahh
Copy link
Contributor

fernahh commented Aug 1, 2016

@rnldfll do you have reference about this information?

@@ -20,6 +20,7 @@ const CREDIT_CARD_LIST = [
}, {
name: 'Mastercard',
regexpFull: '^5[1-5][0-9]{14}$'
//regexpFull: '^((5[1-5][0-9]{14})|(2221[0-9]{12})|(222[2-9][0-9]{12})|(22[3-9][0-9]{13})|(2[3-6][0-9]{14})|(27[01][0-9]{13})|(2720[0-9]{12}))$' //After October
Copy link
Contributor

Choose a reason for hiding this comment

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

In the PR description is mentioned 2222, but in the regex it turned into (222[2-9][0-9]{12}), which is not the same.

In the PR description is mentioned 223, but in the regex it turned into (22[3-9][0-9]{13}), which is not the same.

Also, the inner parenthesis are not necessary:

From: ^((5[1-5][0-9]{14})|(2221[0-9]{12})|(222[2-9][0-9]{12})|(22[3-9][0-9]{13})|(2[3-6][0-9]{14})|(27[01][0-9]{13})|(2720[0-9]{12}))$
To  : ^(5[1-5][0-9]{14}|2221[0-9]{12}|222[2-9][0-9]{12}|22[3-9][0-9]{13}|2[3-6][0-9]{14}|27[01][0-9]{13}|2720[0-9]{12})$

@ronaldoflima
Copy link
Contributor Author

@fernahh Yes, I received an email from a card service that uses the Mastercard and i'm using an Apache 's validator that already contains this information . Link below:
https://commons.apache.org/proper/commons-validator/apidocs/src-html/org/apache/commons/validator/routines/CreditCardValidator.html#line.56

@aureliojargas Is wrong in the PR description, the regex is ok i think. Sorry for the mistake. Please check the link above and confirm that. About the parenteses you're right, Thanks.

@fernahh
Copy link
Contributor

fernahh commented Aug 5, 2016

@rnldfll I'm awaiting your improvements identified by @aureliojargas, ok?

@ronaldoflima
Copy link
Contributor Author

@fernahh Done

@aureliojargas
Copy link
Contributor

@rnldfll You have removed the outer parenthesis, not the inner parenthesis. Please see my previous comment.

@fernahh
Copy link
Contributor

fernahh commented Aug 12, 2016

@rnldfll ping

@aureliojargas
Copy link
Contributor

👍

@fernahh
Copy link
Contributor

fernahh commented Aug 22, 2016

Thanks for PR, @rnldfll! Next week we will release a new version.

@fernahh fernahh merged commit 64e9913 into ContaAzul:master Aug 22, 2016
@fernahh fernahh mentioned this pull request Aug 22, 2016
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.

None yet

4 participants