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

Fix email constraint. InvalidConstraint need a model field, not a string #77

Merged
merged 1 commit into from
Nov 27, 2015

Conversation

qtdzz
Copy link
Contributor

@qtdzz qtdzz commented Nov 2, 2015

A small and very rare bug when generating user model.

@gregmuellegger
Copy link
Owner

Hi thanks for the proposal. Can you describe very quickly when you hit this bug? I don't get yet what this PR is changing in the semantics.

@qtdzz
Copy link
Contributor Author

qtdzz commented Nov 25, 2015

Hi,
The bug happens when the generated email is not unique. Actually it is very rare (or impossible) to reach this bug. I recognized it when I referenced "unique_email" to make my own constraint. But when my constraint is not satisfied, it was crashed instead of raising InvalidConstraint.
I took a look at the constraints.py and found that it takes a model field as parameter, not a string.
Thanks for your consideration :)

gregmuellegger added a commit that referenced this pull request Nov 27, 2015
Fix email constraint. InvalidConstraint need a model field, not a string
@gregmuellegger gregmuellegger merged commit 6e25e72 into gregmuellegger:master Nov 27, 2015
@gregmuellegger
Copy link
Owner

Oops, I didn't see that. This went unnoticed since 2010 :) Thanks for the fix and the explanation.

@gregmuellegger
Copy link
Owner

Do you need a new release with this fix urgently? Otherwise it will just be part of the next random release :)

@qtdzz
Copy link
Contributor Author

qtdzz commented Nov 27, 2015

Hi,
I think you don't need to make a new release because it may never reach that bug. It is just helpful for people who refer that point to make their own constraints 👍

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