Re: [Django] #8620: ModelForm.Meta.exclude only excludes model fields, not form fields
Django
---------------------------------------+----------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Milestone: | Component: Forms
Version: SVN | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
---------------------------------------+----------------------------
Changes (by tomchristie):
* cc: tom@… (added)
* easy: => 0
--
Ticket URL: <http://code.djangoproject.com/ticket/8620#comment:11>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
---------------------------------------+----------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Milestone: | Component: Forms
Version: SVN | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
---------------------------------------+----------------------------
Changes (by anonymous):
* ui_ux: => 0
Comment:
The problem still occurs in Django 1.3
Also, if you set form fields to Required=False, they are still required in
form validation.
Here is an example :
{{{
class RegistrationForm(forms.ModelForm):
#---------------------------------------------------- Extra form
fields
email_confirm = forms.EmailField(widget = forms.TextInput(attrs =
{'size':20}))
password_confirm = forms.CharField(required=False, widget =
forms.PasswordInput(attrs = {'size':20}))
eula = forms.BooleanField(required=False, label=u"CGU")
#===========================================================================
# Metadata
#===========================================================================
class Meta:
model = User
fields = ('username', 'email', 'email_confirm', 'password',
'first_name', 'last_name')
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:12>
Django
Type: New feature | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Changes (by koenb):
* needs_docs: 1 => 0
* needs_tests: 1 => 0
* needs_better_patch: 1 => 0
Comment:
Added an updated patch, that only implements leaving out an explicitly
declared field with Meta.exclude, and does not require adding explicitly
declared fields into the Meta.fields list like the previous patch did
(which was backwards incompatible).
Added a note to the documentation also.
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:13>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Component: Forms | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
Comment (by codysomerville):
See duplicate ticket:13971 for additional discussion/patches on this
issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:14>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Component: Forms | Version: SVN
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
* cc: cody.somerville@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:15>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
* cc: mmitar@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:16>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
Comment (by timo):
I think this should be fine backwards compatibility-wise, but a posted a
[https://github.com/django/django/pull/1409 PR] just to give greater
visibility to the patch before I commit it.
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:17>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
Comment (by mjtamlyn):
At the moment, I'm definitely -0 on this change in functionality. I
believe that `fields` and `exclude` are shortcuts and are secondary to
actually defining the fields - i.e. the field definitions on the form
should *always* take precedence. I believe that the subclassing issue can
be solved by setting `username = None`on the subclass.
In particular this introduces a disparity between `Form` and `ModelForm` -
I don't want to recommend this as a subclassing route if it doesn't
actually work in the general case.
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:18>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
Comment (by loic84):
This may or may not be the right solution to this issue, but this PR
allows shadowing a declarative field and would enable the `username =
None` example from the previous comment:
https://github.com/django/django/pull/1382.
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:19>
Django
-----------------------------+--------------------------------------
Reporter: levity | Owner: shimonrura
Type: New feature | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+--------------------------------------
* cc: loic@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:20>
Django
-----------------------------+------------------------------------
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------
* status: new => assigned
* owner: shimonrura => loic84
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:21>
Django
-----------------------------+------------------------------------
Reporter: levity | Owner: loic84
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"54cd930baf49c45e3e91338a9f4a56d19090ff25"]:
{{{
#!CommitTicketReference repository=""
revision="54cd930baf49c45e3e91338a9f4a56d19090ff25"
Clarfied the ModelForm docs with respect to generated vs. declared fields.
The ModelForm docs suggested that fields defined declaratively override
default fields generated from the form Meta.
This is conceptually wrong, especially with inheritance in mind. Meta is
usually defined on the topmost ModelForm subclass, while fields can come
from anywhere in the MRO, especially base classes; therefore we suggested
that something defined in a base class override something from a subclass.
This patch rephrases the docs around the idea that Meta is used to
generate
*missing* fields.
Refs #8620, #19617.
Thanks @mjtamlyn and @timgraham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:22>
Django
-----------------------------+------------------------------------
Reporter: levity | Owner: loic84
Type: New feature | Status: assigned
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------
Comment (by Tim Graham <timograham@…>):
In [changeset:"8222a48253e61f4d3c1d30156a5f9e0d59a7c56c"]:
{{{
#!CommitTicketReference repository=""
revision="8222a48253e61f4d3c1d30156a5f9e0d59a7c56c"
[1.6.x] Clarfied the ModelForm docs with respect to generated vs. declared
fields.
The ModelForm docs suggested that fields defined declaratively override
default fields generated from the form Meta.
This is conceptually wrong, especially with inheritance in mind. Meta is
usually defined on the topmost ModelForm subclass, while fields can come
from anywhere in the MRO, especially base classes; therefore we suggested
that something defined in a base class override something from a subclass.
This patch rephrases the docs around the idea that Meta is used to
generate
*missing* fields.
Refs #8620, #19617.
Thanks @mjtamlyn and @timgraham for the review.
Backport of 54cd930baf from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:23>
Django
-----------------------------+------------------------------------
Reporter: levity | Owner: loic84
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b16dd1fe019b38d0dcdf4a400e9377fb06b33178"]:
{{{
#!CommitTicketReference repository=""
revision="b16dd1fe019b38d0dcdf4a400e9377fb06b33178"
Fixed #8620 -- Updated the Form metaclass to support excluding fields by
shadowing them.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:24>
Django
-----------------------------+------------------------------------
Reporter: levity | Owner: loic84
Type: New feature | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------+------------------------------------
Comment (by Marc Tamlyn <marc.tamlyn@…>):
In [changeset:"be733bf672eebb7d03061a293a2b0b03c727ab44"]:
{{{
#!CommitTicketReference repository=""
revision="be733bf672eebb7d03061a293a2b0b03c727ab44"
[1.7.x] Fixed #22510 -- Harden field removal to only None.
Refs #8620.
If we allow any value to remove form fields then we get name clashes
with method names, media classes etc. There was a backwards
incompatibility introduced meaning ModelForm subclasses with declared
fields called media or clean would lose those fields.
Field removal is now only permitted by using the sentinel value None.
The docs have been slightly reworded to refer to removal of fields
rather than shadowing.
Thanks to gcbirzan for the report and initial patch, and several of the
core team for opinions.
Backport of 9fb0f5dddc4cf7f2d294af1bcde2c359cffd90a5 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8620#comment:25>