Opened 11 years ago
Closed 11 years ago
#21397 closed Bug (fixed)
forms.TypedChoiceField validation problem
Reported by: | Mikhail Mitrofanov | Owned by: | Claude Paroz |
---|---|---|---|
Component: | Forms | Version: | 1.6 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I get ValidationError on this code:
POST_DAYS_CHOICE = ( (7, '1 week'), (14, '2 weeks'), (30, '1 month'), (60, '2 months'), (90, '3 months'), ) def days_to_till_date(val): print val try: till_date = date.today() + timedelta(int(val)) except ValueError: till_date = date.today() + timedelta(30) return till_date class PostForm(forms.ModelForm): till_date = forms.TypedChoiceField(label='Period', choices=POST_DAYS_CHOICE, coerce=days_to_till_date, required=True, initial=30)
This is was removed from TypedChoiceField class:
def validate(self, value): pass
Attachments (1)
Change History (12)
comment:1 by , 11 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
by , 11 years ago
Attachment: | 21397-1.diff added |
---|
comment:2 by , 11 years ago
Has patch: | set |
---|
Attached a patch with a possible fix. Test suite passes with SQLite. I'll let other core developers judge if this warrants to be added in a stable release.
comment:3 by , 11 years ago
Thinking a bit more about this, I think that it's too risky to push my patch in the 1.6 stable branch. We can document the regression in the 1.6 release notes, but you will probably have to create a custom field to workaround the issue until the next release.
Please, next time try to test your project with beta or at least RC releases, so we can catch those issues before the official release.
comment:4 by , 11 years ago
Hi, no problem. I just test my project with 1.6 release. I create custom field.
comment:5 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Assigned to me, but would love to receive another core dev opinion.
comment:6 by , 11 years ago
Severity: | Release blocker → Normal |
---|---|
Triage Stage: | Accepted → Ready for checkin |
While you did a great job at putting a non-invasive patch together I'm not sure we should commit to backport it.
First because it doesn't meet any of the policy criterias and second because it concerns an undocumented and corner use case.
IMHO the small risk of introducing another regression, there's a small one given the original to_python
override, is not worth the backport.
On the other hand -- apart the missing a release note -- the patch looks RFC for master.
comment:7 by , 11 years ago
Proposal for a 1.6 release note addition (misc backwards incompatibilities):
* Due to a change in the form validation workflow, :class:`~django.forms.TypedChoiceField` ``coerce`` method should always return a value present in the ``choices`` field attribute. That limitation should be lift again in Django 1.7.
Does it look sane?
comment:11 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
This was changed in [9883551d50e].
Interesting use case, which is currently not tested in Django. Having the coerced value not in choices is a bit of a corner case (and not clearly documented), but I admit this might be considered as a regression.
I'll see if I can come up with a fix which is not too invasive...
In any case, the documentation will have to be clarified on this point.