[Django] #28171: empty_permitted=True contradicts use_required_attribute=True
Django
------------------------------------------+------------------------
Reporter: Vlastimil Zíma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
It is possible to easily create a form which does not provide expected
behavior. If a form is created with `empty_permitted=True`, it still
renders a required attribute into its inputs, although it clearly
shouldn't.
{{{
#!python
from django import forms
class FooForm(forms.Form):
foo = forms.CharField()
unicode(FooForm(empty_permitted=True))
>>> u'<tr><th><label for="id_foo">Foo:</label></th><td><input id="id_foo"
name="foo" type="text" required /></td></tr>'
}}}
I suggest to disable `use_required_attribute` if `empty_permitted` is
enabled and/or raise exception if both attributes are set up
contradictory. Even though `empty_permitted` is not a documented
attribute, incorrect usage should be limited if possible.
--
Ticket URL: <https://code.djangoproject.com/ticket/28171>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
arguments conflict
--------------------------------------+------------------------------------
Reporter: Vlastimil Zíma | Owner: nobody
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Raising an exception if the arguments are in conflict seems like the best
approach to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:1>
Django
arguments conflict
Reporter: Vlastimil Zíma | Owner: Naveen
Type: | Yadav
Cleanup/optimization | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Naveen Yadav):
* owner: nobody => Naveen Yadav
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:2>
Django
arguments conflict
-------------------------------------+-------------------------------------
Reporter: Vlastimil Zíma | Owner: Naveen
Type: | Yadav
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Naveen Yadav):
Hi,
working on my first ticket on django !!
i am raising the ValidatioError from
[https://github.com/django/django/blob/master/django/forms/forms.py#L418
for that couple of test cases are failing because of following :
creating a formset on certain
[https://github.com/django/django/blob/master/django/forms/formsets.py#L170]
set **empty_permitted** to True.
Not sure should we allow error in this case also.
Any pointers will be appreciated.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:3>
Django
arguments conflict
-------------------------------------+-------------------------------------
Reporter: Vlastimil Zíma | Owner: Naveen
Type: | Yadav
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I expect a `ValueError` to be raised in `BaseForm.__init__()` if the
values of `empty_permitted` and `use_required_attribute` are incompatible.
This is a programming error, not a validation error that the user can do
anything about.
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:4>
Django
arguments conflict
-------------------------------------+-------------------------------------
Reporter: Vlastimil Zíma | Owner: Naveen
Type: | Yadav
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Naveen Yadav):
Thanks for suggestion Tim.
I tried raising {{{ ValueError }}} which causes lot of tests to fail
because of this
line[https://github.com/django/django/blob/master/django/forms/formsets.py#L172]
all related to formset. I guess when forms are created through formset
both values `empty_permitted` and `use_required_attribute` are same.
Should i place check if form(s) is created via {{{formset}}} then error
should not be raised ?
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:5>
Django
arguments conflict
-------------------------------------+-------------------------------------
Reporter: Vlastimil Zíma | Owner: Naveen
Type: | Yadav
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Vlastimil Zíma):
If formset create form with `empty_permitted`, it has to set
`use_required_attribute` to `False` as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:6>
Django
arguments conflict
Reporter: Vlastimil Zíma | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Naveen Yadav):
* owner: Naveen Yadav => (none)
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:7>
Django
arguments conflict
Reporter: Vlastimil Zíma | Owner: Herbert
Type: | Fortes
Cleanup/optimization | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Herbert Fortes):
* owner: (none) => Herbert Fortes
* status: new => assigned
Comment:
Hi,
I created a Pull Request to close this ticket:
https://github.com/django/django/pull/9691
Regards,
Herbert
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:8>
Django
arguments conflict
-------------------------------------+-------------------------------------
Reporter: Vlastimil Zíma | Owner: Herbert
Type: | Fortes
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"d368784bacc7e58b426f29937ee842aa14d439ad" d368784]:
{{{
#!CommitTicketReference repository=""
revision="d368784bacc7e58b426f29937ee842aa14d439ad"
Fixed #28171 -- Added an exception if Form's empty_permitted and
use_required_attribute arguments conflict.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28171#comment:9>