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 CNPJ and CPF validation formats #172

Merged
merged 1 commit into from
Oct 7, 2015

Conversation

LuanP
Copy link
Contributor

@LuanP LuanP commented Oct 7, 2015

Changed the re.sub to avoid cleaning CNPJ and CPF numbers sent in the wrong format.

Removed errors_numbersonly since the regex will take care of validating and raising a ValidationError with the error_format message.

If the message of errors_numbersonly error must be kept, just let me know.

@LuanP
Copy link
Contributor Author

LuanP commented Oct 7, 2015

I'd like to bring the validations out of the clean method. Allowing users to use the validation functions directly. Could I do that?

Thanks

@claudep
Copy link
Member

claudep commented Oct 7, 2015

When reading the test updates, it's not obvious for me what you are trying to solve with this patch. If the only result is to have a different error message, it's not worth it I think. If your changes allow to catch more errors, please reflect that in the tests also.

@LuanP
Copy link
Contributor Author

LuanP commented Oct 7, 2015

I've added a CNPJ number to the tests:
https://github.com/LuanP/django-localflavor/blob/change_cpf_and_cnpj_regexp/tests/test_br.py#L34

This PR makes that invalid.

@claudep
Copy link
Member

claudep commented Oct 7, 2015

Good, a similar test for CPF would be nice.

@LuanP LuanP force-pushed the change_cpf_and_cnpj_regexp branch from d9a23e9 to 9698221 Compare October 7, 2015 12:09
@LuanP
Copy link
Contributor Author

LuanP commented Oct 7, 2015

claudep added a commit that referenced this pull request Oct 7, 2015
[br] Added CNPJ and CPF validation formats
@claudep claudep merged commit a98c805 into django:master Oct 7, 2015
@LuanP
Copy link
Contributor Author

LuanP commented Oct 7, 2015

@claudep Do you have any ideas about what happened with the builds py27-master and py34-master that raised an AssertionError in the Chilean RUT? Every other env has passed successfully.

Anything else I should do?

I'll create another PR to add validation formats to the BRProcessoField too and add tests to it, if that's ok.

Thanks.

@claudep
Copy link
Member

claudep commented Oct 7, 2015

I'm investigating the issue about test failures. But it's unrelated to this patch, that's why I merged it.

@LuanP
Copy link
Contributor Author

LuanP commented Oct 7, 2015

Awesome! Thanks.

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.

2 participants