Permalink
Browse files

Fixed #19733 - deprecated ModelForms without 'fields' or 'exclude', a…

…nd added '__all__' shortcut

This also updates all dependent functionality, including modelform_factory
 and modelformset_factory, and the generic views `ModelFormMixin`,
 `CreateView` and `UpdateView` which gain a new `fields` attribute.
  • Loading branch information...
1 parent 1e37cb3 commit f026a519aea8f3ea7ca339bfbbb007e1ee0068b0 @spookylukey spookylukey committed Feb 21, 2013
Showing with 578 additions and 201 deletions.
  1. +13 −1 django/contrib/admin/options.py
  2. +1 −0 django/contrib/auth/forms.py
  3. +7 −2 django/contrib/contenttypes/generic.py
  4. +1 −0 django/contrib/flatpages/forms.py
  5. +3 −1 django/contrib/formtools/tests/wizard/test_forms.py
  6. +1 −0 django/contrib/formtools/tests/wizard/wizardtests/tests.py
  7. +55 −2 django/forms/models.py
  8. +10 −1 django/views/generic/edit.py
  9. +2 −0 docs/ref/class-based-views/generic-editing.txt
  10. +12 −0 docs/ref/class-based-views/mixins-editing.txt
  11. +29 −5 docs/ref/contrib/admin/index.txt
  12. +8 −0 docs/ref/forms/models.txt
  13. +52 −0 docs/releases/1.6.txt
  14. +1 −0 docs/topics/auth/customizing.txt
  15. +17 −20 docs/topics/class-based-views/generic-editing.txt
  16. +83 −80 docs/topics/forms/modelforms.txt
  17. +2 −0 tests/admin_validation/tests.py
  18. +1 −0 tests/bug639/models.py
  19. +1 −0 tests/foreign_object/tests.py
  20. +1 −0 tests/forms_tests/tests/test_regressions.py
  21. +4 −0 tests/forms_tests/tests/tests.py
  22. +1 −0 tests/generic_relations/tests.py
  23. +43 −1 tests/generic_views/test_edit.py
  24. +1 −0 tests/generic_views/test_forms.py
  25. +9 −0 tests/generic_views/views.py
  26. +1 −0 tests/i18n/forms.py
  27. +5 −5 tests/inline_formsets/tests.py
  28. +99 −12 tests/model_forms/tests.py
  29. +51 −6 tests/model_forms_regress/tests.py
  30. +45 −44 tests/model_formsets/tests.py
  31. +15 −13 tests/model_formsets_regress/tests.py
  32. +2 −0 tests/model_inheritance_regress/tests.py
  33. +1 −8 tests/modeladmin/tests.py
  34. +1 −0 tests/timezones/forms.py
@@ -5,7 +5,7 @@
from django.conf import settings
from django.forms.formsets import all_valid, DELETION_FIELD_NAME
from django.forms.models import (modelform_factory, modelformset_factory,
- inlineformset_factory, BaseInlineFormSet)
+ inlineformset_factory, BaseInlineFormSet, modelform_defines_fields)
from django.contrib.contenttypes.models import ContentType
from django.contrib.admin import widgets, helpers
from django.contrib.admin.util import (unquote, flatten_fieldsets, get_deleted_objects,
@@ -488,6 +488,10 @@ def get_form(self, request, obj=None, **kwargs):
"formfield_callback": partial(self.formfield_for_dbfield, request=request),
}
defaults.update(kwargs)
+
+ if defaults['fields'] is None and not modelform_defines_fields(defaults['form']):
+ defaults['fields'] = forms.ALL_FIELDS
+
try:
return modelform_factory(self.model, **defaults)
except FieldError as e:
@@ -523,6 +527,10 @@ def get_changelist_form(self, request, **kwargs):
"formfield_callback": partial(self.formfield_for_dbfield, request=request),
}
defaults.update(kwargs)
+ if (defaults.get('fields') is None
+ and not modelform_defines_fields(defaults.get('form'))):
+ defaults['fields'] = forms.ALL_FIELDS
+
return modelform_factory(self.model, **defaults)
def get_changelist_formset(self, request, **kwargs):
@@ -1527,6 +1535,10 @@ def is_valid(self):
return result
defaults['form'] = DeleteProtectedModelForm
+
+ if defaults['fields'] is None and not modelform_defines_fields(defaults['form']):
+ defaults['fields'] = forms.ALL_FIELDS
+
return inlineformset_factory(self.parent_model, self.model, **defaults)
def get_fieldsets(self, request, obj=None):
@@ -130,6 +130,7 @@ class UserChangeForm(forms.ModelForm):
class Meta:
model = User
+ fields = '__all__'
def __init__(self, *args, **kwargs):
super(UserChangeForm, self).__init__(*args, **kwargs)
@@ -13,8 +13,9 @@
from django.db.models.fields.related import ForeignObject, ForeignObjectRel
from django.db.models.related import PathInfo
from django.db.models.sql.where import Constraint
-from django.forms import ModelForm
-from django.forms.models import BaseModelFormSet, modelformset_factory, save_instance
+from django.forms import ModelForm, ALL_FIELDS
+from django.forms.models import (BaseModelFormSet, modelformset_factory, save_instance,
+ modelform_defines_fields)
from django.contrib.admin.options import InlineModelAdmin, flatten_fieldsets
from django.contrib.contenttypes.models import ContentType
from django.utils import six
@@ -480,6 +481,10 @@ def get_formset(self, request, obj=None, **kwargs):
"exclude": exclude
}
defaults.update(kwargs)
+
+ if defaults['fields'] is None and not modelform_defines_fields(defaults['form']):
+ defaults['fields'] = ALL_FIELDS
+
return generic_inlineformset_factory(self.model, **defaults)
class GenericStackedInline(GenericInlineModelAdmin):
@@ -12,6 +12,7 @@ class FlatpageForm(forms.ModelForm):
class Meta:
model = FlatPage
+ fields = '__all__'
def clean_url(self):
url = self.cleaned_data['url']
@@ -60,9 +60,11 @@ class Meta:
class TestModelForm(forms.ModelForm):
class Meta:
model = TestModel
+ fields = '__all__'
-TestModelFormSet = forms.models.modelformset_factory(TestModel, form=TestModelForm, extra=2)
+TestModelFormSet = forms.models.modelformset_factory(TestModel, form=TestModelForm, extra=2,
+ fields='__all__')
class TestWizard(WizardView):
@@ -15,6 +15,7 @@
class UserForm(forms.ModelForm):
class Meta:
model = User
+ fields = '__all__'
UserFormSet = forms.models.modelformset_factory(User, form=UserForm, extra=2)
@@ -5,6 +5,8 @@
from __future__ import absolute_import, unicode_literals
+import warnings
+
from django.core.exceptions import ValidationError, NON_FIELD_ERRORS, FieldError
from django.forms.fields import Field, ChoiceField
from django.forms.forms import BaseForm, get_declared_fields
@@ -22,8 +24,12 @@
__all__ = (
'ModelForm', 'BaseModelForm', 'model_to_dict', 'fields_for_model',
'save_instance', 'ModelChoiceField', 'ModelMultipleChoiceField',
+ 'ALL_FIELDS',
)
+ALL_FIELDS = '__all__'
+
+
def construct_instance(form, instance, fields=None, exclude=None):
"""
Constructs and returns a model instance from the bound ``form``'s
@@ -211,7 +217,7 @@ def __new__(cls, name, bases, attrs):
# of ('foo',)
for opt in ['fields', 'exclude']:
value = getattr(opts, opt)
- if isinstance(value, six.string_types):
+ if isinstance(value, six.string_types) and value != ALL_FIELDS:
msg = ("%(model)s.Meta.%(opt)s cannot be a string. "
"Did you mean to type: ('%(value)s',)?" % {
'model': new_class.__name__,
@@ -222,6 +228,20 @@ def __new__(cls, name, bases, attrs):
if opts.model:
# If a model is defined, extract form fields from it.
+
+ if opts.fields is None and opts.exclude is None:
+ # This should be some kind of assertion error once deprecation
+ # cycle is complete.
+ warnings.warn("Creating a ModelForm without either the 'fields' attribute "
+ "or the 'exclude' attribute is deprecated - form %s "
+ "needs updating" % name,
+ PendingDeprecationWarning)
+
+ if opts.fields == ALL_FIELDS:
+ # sentinel for fields_for_model to indicate "get the list of
+ # fields from the model"
+ opts.fields = None
+
fields = fields_for_model(opts.model, opts.fields,
opts.exclude, opts.widgets, formfield_callback)
# make sure opts.fields doesn't specify an invalid field
@@ -394,7 +414,8 @@ def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
Returns a ModelForm containing form fields for the given model.
``fields`` is an optional list of field names. If provided, only the named
- fields will be included in the returned fields.
+ fields will be included in the returned fields. If omitted or '__all__',
+ all fields will be used.
``exclude`` is an optional list of field names. If provided, the named
fields will be excluded from the returned fields, even if they are listed
@@ -434,6 +455,15 @@ def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
'formfield_callback': formfield_callback
}
+ # The ModelFormMetaclass will trigger a similar warning/error, but this will
+ # be difficult to debug for code that needs updating, so we produce the
+ # warning here too.
+ if (getattr(Meta, 'fields', None) is None and
+ getattr(Meta, 'exclude', None) is None):
+ warnings.warn("Calling modelform_factory without defining 'fields' or "
+ "'exclude' explicitly is deprecated",
+ PendingDeprecationWarning, stacklevel=2)
+
# Instatiate type(form) in order to use the same metaclass as form.
return type(form)(class_name, (form,), form_class_attrs)
@@ -701,6 +731,21 @@ def modelformset_factory(model, form=ModelForm, formfield_callback=None,
"""
Returns a FormSet class for the given Django model class.
"""
+ # modelform_factory will produce the same warning/error, but that will be
+ # difficult to debug for code that needs upgrading, so we produce the
+ # warning here too. This logic is reproducing logic inside
+ # modelform_factory, but it can be removed once the deprecation cycle is
+ # complete, since the validation exception will produce a helpful
+ # stacktrace.
+ meta = getattr(form, 'Meta', None)
+ if meta is None:
+ meta = type(str('Meta'), (object,), {})
+ if (getattr(meta, 'fields', fields) is None and
+ getattr(meta, 'exclude', exclude) is None):
+ warnings.warn("Calling modelformset_factory without defining 'fields' or "
+ "'exclude' explicitly is deprecated",
+ PendingDeprecationWarning, stacklevel=2)
+
form = modelform_factory(model, form=form, fields=fields, exclude=exclude,
formfield_callback=formfield_callback,
widgets=widgets)
@@ -1091,3 +1136,11 @@ def _has_changed(self, initial, data):
initial_set = set([force_text(value) for value in self.prepare_value(initial)])
data_set = set([force_text(value) for value in data])
return data_set != initial_set
+
+
+def modelform_defines_fields(form_class):
+ return (form_class is not None and (
+ hasattr(form_class, '_meta') and
+ (form_class._meta.fields is not None or
+ form_class._meta.exclude is not None)
+ ))
@@ -1,3 +1,5 @@
+import warnings
+
from django.forms import models as model_forms
from django.core.exceptions import ImproperlyConfigured
from django.http import HttpResponseRedirect
@@ -95,7 +97,14 @@ def get_form_class(self):
# Try to get a queryset and extract the model class
# from that
model = self.get_queryset().model
- return model_forms.modelform_factory(model)
+
+ fields = getattr(self, 'fields', None)
+ if fields is None:
+ warnings.warn("Using ModelFormMixin (base class of %s) without "
+ "the 'fields' attribute is deprecated." % self.__class__.__name__,
+ PendingDeprecationWarning)
+
+ return model_forms.modelform_factory(model, fields=fields)
def get_form_kwargs(self):
"""
@@ -110,6 +110,7 @@ CreateView
class AuthorCreate(CreateView):
model = Author
+ fields = ['name']
UpdateView
----------
@@ -152,6 +153,7 @@ UpdateView
class AuthorUpdate(UpdateView):
model = Author
+ fields = ['name']
DeleteView
----------
@@ -116,6 +116,18 @@ ModelFormMixin
by examining ``self.object`` or
:attr:`~django.views.generic.detail.SingleObjectMixin.queryset`.
+ .. attribute:: fields
+
+ .. versionadded:: 1.6
+
+ A list of names of fields. This is interpreted the same way as the
+ ``Meta.fields`` attribute of :class:`~django.forms.ModelForm`.
+
+ This is a required attribute if you are generating the form class
+ automatically (e.g. using ``model``). Omitting this attribute will
+ result in all fields being used, but this behaviour is deprecated
+ and will be removed in Django 1.8.
+
.. attribute:: success_url
The URL to redirect to when the form is successfully processed.
@@ -337,6 +337,22 @@ subclass::
.. admonition:: Note
+ .. versionchanged:: 1.6
+
+ If you define the ``Meta.model`` attribute on a
+ :class:`~django.forms.ModelForm`, you must also define the
+ ``Meta.fields`` attribute (or the ``Meta.exclude`` attribute). However,
+ since the admin has its own way of defining fields, the ``Meta.fields``
+ attribute will be ignored.
+
+ If the ``ModelForm`` is only going to be used for the admin, the easiest
+ solution is to omit the ``Meta.model`` attribute, since ``ModelAdmin``
+ will provide the correct model to use. Alternatively, you can set
+ ``fields = []`` in the ``Meta`` class to satisfy the validation on the
+ ``ModelForm``.
+
+ .. admonition:: Note
+
If your ``ModelForm`` and ``ModelAdmin`` both define an ``exclude``
option then ``ModelAdmin`` takes precedence::
@@ -1283,13 +1299,24 @@ templates used by the :class:`ModelAdmin` views:
on the changelist page. To use a custom form, for example::
class MyForm(forms.ModelForm):
- class Meta:
- model = MyModel
+ pass
class MyModelAdmin(admin.ModelAdmin):
def get_changelist_form(self, request, **kwargs):
return MyForm
+ .. admonition:: Note
+
+ .. versionchanged:: 1.6
+
+ If you define the ``Meta.model`` attribute on a
+ :class:`~django.forms.ModelForm`, you must also define the
+ ``Meta.fields`` attribute (or the ``Meta.exclude`` attribute). However,
+ ``ModelAdmin`` ignores this value, overriding it with the
+ :attr:`ModelAdmin.list_editable` attribute. The easiest solution is to
+ omit the ``Meta.model`` attribute, since ``ModelAdmin`` will provide the
+ correct model to use.
+
.. method:: ModelAdmin.get_changelist_formset(self, request, **kwargs)
Returns a :ref:`ModelFormSet <model-formsets>` class for use on the
@@ -1490,9 +1517,6 @@ needed. Now within your form you can add your own custom validation for
any field::
class MyArticleAdminForm(forms.ModelForm):
- class Meta:
- model = Article
-
def clean_name(self):
# do something that validates your data
return self.cleaned_data["name"]
@@ -25,6 +25,14 @@ Model Form Functions
See :ref:`modelforms-factory` for example usage.
+ .. versionchanged:: 1.6
+
+ You must provide the list of fields explicitly, either via keyword arguments
+ ``fields`` or ``exclude``, or the corresponding attributes on the form's
+ inner ``Meta`` class. See :ref:`modelforms-selecting-fields` for more
+ information. Omitting any definition of the fields to use will result in all
+ fields being used, but this behaviour is deprecated.
+
.. function:: modelformset_factory(model, form=ModelForm, formfield_callback=None, formset=BaseModelFormSet, extra=1, can_delete=False, can_order=False, max_num=None, fields=None, exclude=None, widgets=None, validate_max=False)
Returns a ``FormSet`` class for the given ``model`` class.
Oops, something went wrong.

0 comments on commit f026a51

Please sign in to comment.