#13342 closed Bug (wontfix)
MultiValueField - incorrectly use 'required' attribute of MultiValueField for child field validation
Reported by: | krejcik | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | fields required MultiValueField |
Cc: | flisky | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
MultiValueField is composed from several child fields
Validation logic in it's clean method should be:
- if MultiValueField is required, at least one of the child values must be non-empty (ok)
- if a child field is required, it's value must be non-empty (bug in implementation, MultiValueField.required is used instead of child.required to check condition)
- clean decompressed values by each child fields and compress together (ok)
Attachments (1)
Change History (6)
by , 15 years ago
Attachment: | fields.patch added |
---|
comment:1 by , 15 years ago
Description: | modified (diff) |
---|---|
milestone: | 1.2 |
Resolution: | → wontfix |
Status: | new → closed |
follow-up: 3 comment:2 by , 13 years ago
Easy pickings: | unset |
---|---|
Resolution: | wontfix |
Severity: | → Normal |
Status: | closed → reopened |
Type: | → Bug |
UI/UX: | unset |
Is it allowed to reopen this ticket please?
Sorry if it is not the right procedure, but I'd like to post some notes about this ticket and consequent decision made.
Behaviour proposed by krejcik sounds good to me because when I have a required MultiValueField, my idea is to require that the whole stuff that resides inside validates. Not each single piece, but as one big picture. What there is inside is not myproblem.
Each field that are inside the MultiValueField knows by itself if it is required or not, and because of this they raises validation errors or not.
My example is a PlaceField(forms.MultiValueField)
developed to set an address which is composed by:
- Name
- Address
- ZIP
- City
- State
Name or address could be blank (not at the same time both), so can be ZIP.
My needing is that a place MUST be specificed, so PlaceField must be a required=True field,
BUT to specify a place is NOT REQUIRED to specify address and ZIP so I SHOULD have the ability
to tell that nested fields address and ZIP are not required, and, IMHO, Django must respect the constraint.
My PlaceField sounds like this
class PlaceField(forms.MultiValueField): widget = PlaceWidget def __init__(self, *args, **kw): fields = ( forms.IntegerField(), # was: label=_("id") forms.CharField(label=_("name")), forms.CharField(label=_("address"), required=False), forms.CharField(label=_("zip"), required=False), forms.CharField(label=_("city")), forms.ChoiceField(label=_("state"),choices=PlaceField.STATE_CHOICES), ) super(PlaceField, self).__init__(fields, *args, **kw)
or am I totally wrong?
comment:3 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
The best practice for "won't fix" tickets is what Russel said: "please bring it up on Django-developers". Especially with a ticket this old. Personally, I think that the current behaviour is at least non-obvious, so it should be documented more explicitly. But we can discuss this further on the mailing list.
comment:4 by , 13 years ago
Cc: | added |
---|
The proposed behavior strikes me as a fairly major change to the existing behavior, but it isn't clear to me that the existing behaviour is wrong, or that the proposed behavior is more correct.
The report and patch doesn't give any test cases to validate why the proposed change would be correct, so I'm closing wontfix. If you want to argue your case, please bring it up on Django-developers.
Regardless of the decision, the ticket isn't 1.2 critical.