p31829507
/
django
mirror of https://github.com/django/django.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Made ModelForms raise ImproperlyConfigured if the list of fields is not specified. 

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

refs #19733.
This commit is contained in:
Tim Graham 2014-03-21 20:44:34 -04:00
parent 1c8dbb0cc2
commit ee4edb1eda
9 changed files with 77 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
django/forms/models.py
View File

@ -6,10 +6,9 @@ and database field objects.
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.utils import ErrorList
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 @@ class ModelFormMetaclass(DeclarativeFieldsMetaclass):
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 File

@ -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 @@ class ModelFormMixin(FormMixin, SingleObjectMixin):
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 File

@ -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 File

@ -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.
The ``localized_fields``, ``labels``, ``help_texts``, and
``error_messages`` parameters were added.
.. versionchanged:: 1.8
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 File

@ -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 File

@ -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::

24
tests/generic_views/test_edit.py
View File

@ -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.core.urlresolvers import reverse
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 CreateViewTests(TestCase):
['name'])
def test_create_view_all_fields(self):
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)
def test_create_view_without_explicit_fields(self):
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 File

@ -4,10 +4,9 @@ import datetime
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.db.models.query import EmptyQuerySet
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 @@ class ModelFormBaseTest(TestCase):
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 @@ class FormFieldCallbackTests(TestCase):
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 File

@ -6,6 +6,7 @@ from datetime import date
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 @@ class DeletionTests(TestCase):
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)