Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Made ModelForms raise ImproperlyConfigured if the list of fields is n…

…ot specified.

Also applies to modelform(set)_factory and generic model views.

refs #19733.
  • Loading branch information...
commit ee4edb1eda2ac8f09eb298929282b44776930c89 1 parent 1c8dbb0
Tim Graham timgraham authored
38 django/forms/models.py
View
@@ -6,10 +6,9 @@
from __future__ import unicode_literals
from collections import OrderedDict
-import warnings
from django.core.exceptions import (
- ValidationError, NON_FIELD_ERRORS, FieldError)
+ ImproperlyConfigured, ValidationError, NON_FIELD_ERRORS, FieldError)
from django.forms.fields import Field, ChoiceField
from django.forms.forms import DeclarativeFieldsMetaclass, BaseForm
from django.forms.formsets import BaseFormSet, formset_factory
@@ -17,7 +16,6 @@
from django.forms.widgets import (SelectMultiple, HiddenInput,
MultipleHiddenInput)
from django.utils import six
-from django.utils.deprecation import RemovedInDjango18Warning
from django.utils.encoding import smart_text, force_text
from django.utils.text import get_text_list, capfirst
from django.utils.translation import ugettext_lazy as _, ugettext
@@ -266,12 +264,11 @@ def __new__(mcs, 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,
- RemovedInDjango18Warning, stacklevel=2)
+ raise ImproperlyConfigured(
+ "Creating a ModelForm without either the 'fields' attribute "
+ "or the 'exclude' attribute is prohibited; form %s "
+ "needs updating." % name
+ )
if opts.fields == ALL_FIELDS:
# Sentinel for fields_for_model to indicate "get the list of
@@ -528,14 +525,12 @@ 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",
- RemovedInDjango18Warning, stacklevel=2)
+ raise ImproperlyConfigured(
+ "Calling modelform_factory without defining 'fields' or "
+ "'exclude' explicitly is prohibited."
+ )
# Instatiate type(form) in order to use the same metaclass as form.
return type(form)(class_name, (form,), form_class_attrs)
@@ -814,20 +809,15 @@ 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",
- RemovedInDjango18Warning, stacklevel=2)
+ raise ImproperlyConfigured(
+ "Calling modelformset_factory without defining 'fields' or "
+ "'exclude' explicitly is prohibited."
+ )
form = modelform_factory(model, form=form, fields=fields, exclude=exclude,
formfield_callback=formfield_callback,
10 django/views/generic/edit.py
View
@@ -1,9 +1,6 @@
-import warnings
-
from django.core.exceptions import ImproperlyConfigured
from django.forms import models as model_forms
from django.http import HttpResponseRedirect
-from django.utils.deprecation import RemovedInDjango18Warning
from django.utils.encoding import force_text
from django.views.generic.base import TemplateResponseMixin, ContextMixin, View
from django.views.generic.detail import (SingleObjectMixin,
@@ -112,9 +109,10 @@ def get_form_class(self):
model = self.get_queryset().model
if self.fields is None:
- warnings.warn("Using ModelFormMixin (base class of %s) without "
- "the 'fields' attribute is deprecated." % self.__class__.__name__,
- RemovedInDjango18Warning)
+ raise ImproperlyConfigured(
+ "Using ModelFormMixin (base class of %s) without "
+ "the 'fields' attribute is prohibited." % self.__class__.__name__
+ )
return model_forms.modelform_factory(model, fields=self.fields)
11 docs/ref/class-based-views/mixins-editing.txt
View
@@ -129,15 +129,18 @@ ModelFormMixin
.. 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 behavior is deprecated
- and will be removed in Django 1.8.
+ result in an :exc:`~django.core.exceptions.ImproperlyConfigured`
+ exception.
+
+ .. versionchanged:: 1.8
+
+ Previously, omitting this attribute was allowed and resulted in
+ a form with all fields of the model.
.. attribute:: success_url
12 docs/ref/forms/models.txt
View
@@ -34,16 +34,16 @@ 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 behavior is deprecated.
+ information. Omitting any definition of the fields to use will result in
+ an :exc:`~django.core.exceptions.ImproperlyConfigured` exception.
+
+ .. versionchanged:: 1.8
- The ``localized_fields``, ``labels``, ``help_texts``, and
- ``error_messages`` parameters were added.
+ Previously, omitting the list of fields was allowed and resulted in
+ a form with all fields of the model.
.. 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, localized_fields=None, labels=None, help_texts=None, error_messages=None)
15 docs/topics/class-based-views/generic-editing.txt
View
@@ -128,16 +128,15 @@ here; we don't have to write any logic ourselves::
We have to use :func:`~django.core.urlresolvers.reverse_lazy` here, not
just ``reverse`` as the urls are not loaded when the file is imported.
-.. versionchanged:: 1.6
+The ``fields`` attribute works the same way as the ``fields`` attribute on the
+inner ``Meta`` class on :class:`~django.forms.ModelForm`. Unless you define the
+form class in another way, the attribute is required and the view will raise
+an :exc:`~django.core.exceptions.ImproperlyConfigured` exception if it's not.
-In Django 1.6, the ``fields`` attribute was added, which works the same way as
-the ``fields`` attribute on the inner ``Meta`` class on
-:class:`~django.forms.ModelForm`.
-
-Omitting the fields attribute will work as previously, but is deprecated and
-this attribute will be required from 1.8 (unless you define the form class in
-another way).
+.. versionchanged:: 1.8
+ Omitting the ``fields`` attribute was previously allowed and resulted in a
+ form with all of the model's fields.
Finally, we hook these new views into the URLconf::
10 docs/topics/forms/modelforms.txt
View
@@ -430,13 +430,11 @@ In addition, Django applies the following rule: if you set ``editable=False`` on
the model field, *any* form created from the model via ``ModelForm`` will not
include that field.
-.. versionchanged:: 1.6
-
- Before version 1.6, the ``'__all__'`` shortcut did not exist, but omitting
- the ``fields`` attribute had the same effect. Omitting both ``fields`` and
- ``exclude`` is now deprecated, but will continue to work as before until
- version 1.8
+.. versionchanged:: 1.8
+ In older versions, omitting both ``fields`` and ``exclude`` resulted in
+ a form with all the model's fields. Doing this now raises an
+ :exc:`~django.core.exceptions.ImproperlyConfigured` exception.
.. note::
38 tests/generic_views/test_edit.py
View
@@ -1,6 +1,5 @@
from __future__ import unicode_literals
-import warnings
from unittest import expectedFailure
from django.core.exceptions import ImproperlyConfigured
@@ -8,7 +7,6 @@
from django import forms
from django.test import TestCase
from django.test.client import RequestFactory
-from django.utils.deprecation import RemovedInDjango18Warning
from django.views.generic.base import View
from django.views.generic.edit import FormMixin, ModelFormMixin, CreateView
@@ -151,33 +149,23 @@ class MyCreateView(CreateView):
['name'])
def test_create_view_all_fields(self):
+ class MyCreateView(CreateView):
+ model = Author
+ fields = '__all__'
- with warnings.catch_warnings(record=True) as w:
- warnings.simplefilter("always", RemovedInDjango18Warning)
-
- class MyCreateView(CreateView):
- model = Author
- fields = '__all__'
-
- self.assertEqual(list(MyCreateView().get_form_class().base_fields),
- ['name', 'slug'])
- self.assertEqual(len(w), 0)
+ self.assertEqual(list(MyCreateView().get_form_class().base_fields),
+ ['name', 'slug'])
def test_create_view_without_explicit_fields(self):
+ class MyCreateView(CreateView):
+ model = Author
- with warnings.catch_warnings(record=True) as w:
- warnings.simplefilter("always", RemovedInDjango18Warning)
-
- class MyCreateView(CreateView):
- model = Author
-
- # Until end of the deprecation cycle, should still create the form
- # as before:
- self.assertEqual(list(MyCreateView().get_form_class().base_fields),
- ['name', 'slug'])
-
- # but with a warning:
- self.assertEqual(w[0].category, RemovedInDjango18Warning)
+ message = (
+ "Using ModelFormMixin (base class of MyCreateView) without the "
+ "'fields' attribute is prohibited."
+ )
+ with self.assertRaisesMessage(ImproperlyConfigured, message):
+ MyCreateView().get_form_class()
class UpdateViewTests(TestCase):
33 tests/model_forms/tests.py
View
@@ -4,10 +4,9 @@
import os
from decimal import Decimal
from unittest import skipUnless
-import warnings
from django import forms
-from django.core.exceptions import FieldError, NON_FIELD_ERRORS
+from django.core.exceptions import FieldError, ImproperlyConfigured, NON_FIELD_ERRORS
from django.core.files.uploadedfile import SimpleUploadedFile
from django.core.validators import ValidationError
from django.db import connection
@@ -15,7 +14,6 @@
from django.forms.models import (construct_instance, fields_for_model,
model_to_dict, modelform_factory, ModelFormMetaclass)
from django.test import TestCase, skipUnlessDBFeature
-from django.utils.deprecation import RemovedInDjango18Warning
from django.utils._os import upath
from django.utils import six
@@ -202,24 +200,16 @@ def test_empty_fields_to_construct_instance(self):
self.assertEqual(instance.name, '')
def test_missing_fields_attribute(self):
- with warnings.catch_warnings(record=True):
- warnings.simplefilter("always", RemovedInDjango18Warning)
-
+ message = (
+ "Creating a ModelForm without either the 'fields' attribute "
+ "or the 'exclude' attribute is prohibited; form "
+ "MissingFieldsForm needs updating."
+ )
+ with self.assertRaisesMessage(ImproperlyConfigured, message):
class MissingFieldsForm(forms.ModelForm):
class Meta:
model = Category
- # There is some internal state in warnings module which means that
- # if a warning has been seen already, the catch_warnings won't
- # have recorded it. The following line therefore will not work reliably:
-
- # self.assertEqual(w[0].category, RemovedInDjango18Warning)
-
- # Until end of the deprecation cycle, should still create the
- # form as before:
- self.assertEqual(list(MissingFieldsForm.base_fields),
- ['name', 'slug', 'url'])
-
def test_extra_fields(self):
class ExtraFields(BaseCategoryForm):
some_extra_field = forms.BooleanField()
@@ -2329,11 +2319,12 @@ def test_factory_with_widget_argument(self):
def test_modelform_factory_without_fields(self):
""" Regression for #19733 """
- with warnings.catch_warnings(record=True) as w:
- warnings.simplefilter("always", RemovedInDjango18Warning)
- # This should become an error once deprecation cycle is complete.
+ message = (
+ "Calling modelform_factory without defining 'fields' or 'exclude' "
+ "explicitly is prohibited."
+ )
+ with self.assertRaisesMessage(ImproperlyConfigured, message):
modelform_factory(Person)
- self.assertEqual(w[0].category, RemovedInDjango18Warning)
def test_modelform_factory_with_all_fields(self):
""" Regression for #19733 """
10 tests/model_formsets/tests.py
View
@@ -6,6 +6,7 @@
from decimal import Decimal
from django import forms
+from django.core.exceptions import ImproperlyConfigured
from django.db import models
from django.forms.models import (_get_foreign_key, inlineformset_factory,
modelformset_factory, BaseModelFormSet)
@@ -131,6 +132,15 @@ def test_outdated_deletion(self):
class ModelFormsetTest(TestCase):
+ def test_modelformset_factory_without_fields(self):
+ """ Regression for #19733 """
+ message = (
+ "Calling modelformset_factory without defining 'fields' or 'exclude' "
+ "explicitly is prohibited."
+ )
+ with self.assertRaisesMessage(ImproperlyConfigured, message):
+ modelformset_factory(Author)
+
def test_simple_save(self):
qs = Author.objects.all()
AuthorFormSet = modelformset_factory(Author, fields="__all__", extra=3)
Please sign in to comment.
Something went wrong with that request. Please try again.