Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixed #19617 -- Refactored Form metaclasses to support more inheritan…

…ce scenarios.

Thanks apollo13, funkybob and mjtamlyn for the reviews.
  • Loading branch information...
commit ac5ec7b8bcc5623cc05d4df900c39e194f5affcf 1 parent 54cd930
@loic loic authored
View
48 django/forms/forms.py
@@ -11,7 +11,7 @@
from django.core.exceptions import ValidationError
from django.forms.fields import Field, FileField
from django.forms.utils import flatatt, ErrorDict, ErrorList
-from django.forms.widgets import Media, media_property, TextInput, Textarea
+from django.forms.widgets import Media, MediaDefiningClass, TextInput, Textarea
from django.utils.html import conditional_escape, format_html
from django.utils.encoding import smart_text, force_text, python_2_unicode_compatible
from django.utils.safestring import mark_safe
@@ -29,6 +29,7 @@ def pretty_name(name):
return ''
return name.replace('_', ' ').capitalize()
+
def get_declared_fields(bases, attrs, with_base_fields=True):
"""
Create a list of form field instances from the passed in 'attrs', plus any
@@ -40,6 +41,13 @@ def get_declared_fields(bases, attrs, with_base_fields=True):
used. The distinction is useful in ModelForm subclassing.
Also integrates any additional media definitions.
"""
+
+ warnings.warn(
+ "get_declared_fields is deprecated and will be removed in Django 1.9.",
+ PendingDeprecationWarning,
+ stacklevel=2,
+ )
+
fields = [(field_name, attrs.pop(field_name)) for field_name, obj in list(six.iteritems(attrs)) if isinstance(obj, Field)]
fields.sort(key=lambda x: x[1].creation_counter)
@@ -57,19 +65,37 @@ def get_declared_fields(bases, attrs, with_base_fields=True):
return OrderedDict(fields)
-class DeclarativeFieldsMetaclass(type):
+
+class DeclarativeFieldsMetaclass(MediaDefiningClass):
"""
- Metaclass that converts Field attributes to a dictionary called
- 'base_fields', taking into account parent class 'base_fields' as well.
+ Metaclass that collects Fields declared on the base classes.
"""
- def __new__(cls, name, bases, attrs):
- attrs['base_fields'] = get_declared_fields(bases, attrs)
- new_class = super(DeclarativeFieldsMetaclass,
- cls).__new__(cls, name, bases, attrs)
- if 'media' not in attrs:
- new_class.media = media_property(new_class)
+ def __new__(mcs, name, bases, attrs):
+ # Collect fields from current class.
+ current_fields = []
+ for key, value in list(attrs.items()):
+ if isinstance(value, Field):
+ current_fields.append((key, value))
+ attrs.pop(key)
+ current_fields.sort(key=lambda x: x[1].creation_counter)
+ attrs['declared_fields'] = OrderedDict(current_fields)
+
+ new_class = (super(DeclarativeFieldsMetaclass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
+ # Walk through the MRO.
+ declared_fields = OrderedDict()
+ for base in reversed(new_class.__mro__):
+ # Collect fields from base class.
+ if hasattr(base, 'declared_fields'):
+ declared_fields.update(base.declared_fields)

This changes the public api. The call to get_declared_fields that was removed (old line 66) included base.base_fields, not base.declared_fields. The behavior of the following is now different:

class ABaseForm(ModelForm):
    class Meta:
        model = AModel
        fields = ('some', 'fields')


class AChildForm(ABaseForm):
    pass

Previously AChildForm.declared_fields would include the fields given in the Meta of ABaseForm, now it does not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
+ new_class.base_fields = declared_fields
+ new_class.declared_fields = declared_fields
+
return new_class
+
@python_2_unicode_compatible
class BaseForm(object):
# This is the main implementation of all the Form logic. Note that this
@@ -398,6 +424,7 @@ def visible_fields(self):
"""
return [field for field in self if not field.is_hidden]
+
class Form(six.with_metaclass(DeclarativeFieldsMetaclass, BaseForm)):
"A collection of Fields, plus their associated data."
# This is a separate class from BaseForm in order to abstract the way
@@ -406,6 +433,7 @@ class Form(six.with_metaclass(DeclarativeFieldsMetaclass, BaseForm)):
# to define a form using declarative syntax.
# BaseForm itself has no way of designating self.fields.
+
@python_2_unicode_compatible
class BoundField(object):
"A Field plus data"
View
48 django/forms/models.py
@@ -10,11 +10,11 @@
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
+from django.forms.forms import DeclarativeFieldsMetaclass, BaseForm
from django.forms.formsets import BaseFormSet, formset_factory
from django.forms.utils import ErrorList
from django.forms.widgets import (SelectMultiple, HiddenInput,
- MultipleHiddenInput, media_property, CheckboxSelectMultiple)
+ MultipleHiddenInput, CheckboxSelectMultiple)
from django.utils.encoding import smart_text, force_text
from django.utils import six
from django.utils.text import get_text_list, capfirst
@@ -61,6 +61,7 @@ def construct_instance(form, instance, fields=None, exclude=None):
return instance
+
def save_instance(form, instance, fields=None, fail_message='saved',
commit=True, exclude=None, construct=True):
"""
@@ -138,6 +139,7 @@ def model_to_dict(instance, fields=None, exclude=None):
data[f.name] = f.value_from_object(instance)
return data
+
def fields_for_model(model, fields=None, exclude=None, widgets=None,
formfield_callback=None, localized_fields=None,
labels=None, help_texts=None, error_messages=None):
@@ -207,6 +209,7 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None,
)
return field_dict
+
class ModelFormOptions(object):
def __init__(self, options=None):
self.model = getattr(options, 'model', None)
@@ -219,22 +222,16 @@ def __init__(self, options=None):
self.error_messages = getattr(options, 'error_messages', None)
-class ModelFormMetaclass(type):
- def __new__(cls, name, bases, attrs):
+class ModelFormMetaclass(DeclarativeFieldsMetaclass):
+ def __new__(mcs, name, bases, attrs):
formfield_callback = attrs.pop('formfield_callback', None)
- try:
- parents = [b for b in bases if issubclass(b, ModelForm)]
- except NameError:
- # We are defining ModelForm itself.
- parents = None
- declared_fields = get_declared_fields(bases, attrs, False)
- new_class = super(ModelFormMetaclass, cls).__new__(cls, name, bases,
- attrs)
- if not parents:
+
+ new_class = (super(ModelFormMetaclass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
+ if bases == (BaseModelForm,):
return new_class
- if 'media' not in attrs:
- new_class.media = media_property(new_class)
opts = new_class._meta = ModelFormOptions(getattr(new_class, 'Meta', None))
# We check if a string was passed to `fields` or `exclude`,
@@ -253,7 +250,6 @@ 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.
@@ -263,7 +259,7 @@ def __new__(cls, name, bases, attrs):
DeprecationWarning, stacklevel=2)
if opts.fields == ALL_FIELDS:
- # sentinel for fields_for_model to indicate "get the list of
+ # Sentinel for fields_for_model to indicate "get the list of
# fields from the model"
opts.fields = None
@@ -274,8 +270,8 @@ def __new__(cls, name, bases, attrs):
# make sure opts.fields doesn't specify an invalid field
none_model_fields = [k for k, v in six.iteritems(fields) if not v]
- missing_fields = set(none_model_fields) - \
- set(declared_fields.keys())
+ missing_fields = (set(none_model_fields) -
+ set(new_class.declared_fields.keys()))
if missing_fields:
message = 'Unknown field(s) (%s) specified for %s'
message = message % (', '.join(missing_fields),
@@ -283,13 +279,15 @@ def __new__(cls, name, bases, attrs):
raise FieldError(message)
# Override default model fields with any custom declared ones
# (plus, include all the other declared fields).
- fields.update(declared_fields)
+ fields.update(new_class.declared_fields)
else:
- fields = declared_fields
- new_class.declared_fields = declared_fields
+ fields = new_class.declared_fields
+
new_class.base_fields = fields
+
return new_class
+
class BaseModelForm(BaseForm):
def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=None,
@@ -438,9 +436,11 @@ def save(self, commit=True):
save.alters_data = True
+
class ModelForm(six.with_metaclass(ModelFormMetaclass, BaseModelForm)):
pass
+
def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
formfield_callback=None, widgets=None, localized_fields=None,
labels=None, help_texts=None, error_messages=None):
@@ -780,6 +780,7 @@ def pk_is_not_editable(pk):
form.fields[self._pk_field.name] = ModelChoiceField(qs, initial=pk_value, required=False, widget=widget)
super(BaseModelFormSet, self).add_fields(form, index)
+
def 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,
@@ -1021,6 +1022,7 @@ def clean(self, value):
def _has_changed(self, initial, data):
return False
+
class ModelChoiceIterator(object):
def __init__(self, field):
self.field = field
@@ -1047,6 +1049,7 @@ def __len__(self):
def choice(self, obj):
return (self.field.prepare_value(obj), self.field.label_from_instance(obj))
+
class ModelChoiceField(ChoiceField):
"""A ChoiceField whose choices are a model QuerySet."""
# This class is a subclass of ChoiceField for purity, but it doesn't
@@ -1141,6 +1144,7 @@ def _has_changed(self, initial, data):
data_value = data if data is not None else ''
return force_text(self.prepare_value(initial_value)) != force_text(data_value)
+
class ModelMultipleChoiceField(ModelChoiceField):
"""A MultipleChoiceField whose choices are a model QuerySet."""
widget = SelectMultiple
View
12 django/forms/widgets.py
@@ -131,12 +131,16 @@ def _media(self):
return property(_media)
class MediaDefiningClass(type):
- "Metaclass for classes that can have media definitions"
- def __new__(cls, name, bases, attrs):
- new_class = super(MediaDefiningClass, cls).__new__(cls, name, bases,
- attrs)
+ """
+ Metaclass for classes that can have media definitions.
+ """
+ def __new__(mcs, name, bases, attrs):
+ new_class = (super(MediaDefiningClass, mcs)
+ .__new__(mcs, name, bases, attrs))
+
if 'media' not in attrs:
new_class.media = media_property(new_class)
+
return new_class
@python_2_unicode_compatible
View
2  docs/internals/deprecation.txt
@@ -467,6 +467,8 @@ these changes.
* The ``use_natural_keys`` argument for ``serializers.serialize()`` will be
removed. Use ``use_natural_foreign_keys`` instead.
+* ``django.forms.get_declared_fields`` will be removed.
+
2.0
---
View
11 docs/releases/1.7.txt
@@ -288,6 +288,11 @@ Forms
:func:`~django.forms.formsets.formset_factory` to allow validating
a minimum number of submitted forms.
+* The metaclasses used by ``Form`` and ``ModelForm`` have been reworked to
+ support more inheritance scenarios. The previous limitation that prevented
+ inheriting from both ``Form`` and ``ModelForm`` simultaneously have been
+ removed as long as ``ModelForm`` appears first in the MRO.
+
Internationalization
^^^^^^^^^^^^^^^^^^^^
@@ -513,6 +518,12 @@ Miscellaneous
still worked, even though it was not documented or officially supported. If
you're still using it, please update to the current :setting:`CACHES` syntax.
+* The default ordering of ``Form`` fields in case of inheritance has changed to
+ follow normal Python MRO. Fields are now discovered by iterating through the
+ MRO in reverse with the topmost class coming last. This only affects you if
+ you relied on the default field ordering while having fields defined on both
+ the current class *and* on a parent ``Form``.
+
Features deprecated in 1.7
==========================
View
9 docs/topics/forms/modelforms.txt
@@ -644,11 +644,12 @@ There are a couple of things to note, however.
used. This means the child's ``Meta``, if it exists, otherwise the
``Meta`` of the first parent, etc.
-* For technical reasons, a subclass cannot inherit from both a ``ModelForm``
- and a ``Form`` simultaneously.
+.. versionchanged:: 1.7
-Chances are these notes won't affect you unless you're trying to do something
-tricky with subclassing.
+* It's possible to inherit from both ``Form`` and ``ModelForm`` simultaneuosly,
+ however, you must ensure that ``ModelForm`` appears first in the MRO. This is
+ because these classes rely on different metaclasses and a class can only have
+ one metaclass.
.. _modelforms-factory:
View
4 tests/forms_tests/tests/test_forms.py
@@ -1302,10 +1302,10 @@ class Beatle(Person, Instrument):
haircut_type = CharField()
b = Beatle(auto_id=False)
- self.assertHTMLEqual(b.as_ul(), """<li>First name: <input type="text" name="first_name" /></li>
+ self.assertHTMLEqual(b.as_ul(), """<li>Instrument: <input type="text" name="instrument" /></li>
+<li>First name: <input type="text" name="first_name" /></li>
<li>Last name: <input type="text" name="last_name" /></li>
<li>Birthday: <input type="text" name="birthday" /></li>
-<li>Instrument: <input type="text" name="instrument" /></li>
<li>Haircut type: <input type="text" name="haircut_type" /></li>""")
def test_forms_with_prefixes(self):
View
13 tests/model_forms/tests.py
@@ -1802,3 +1802,16 @@ def test_multiple_widgets(self):
html = form.as_p()
self.assertInHTML('<ul id="id_status">', html)
self.assertInHTML(dreaded_help_text, html, count=0)
+
+
+class ModelFormInheritanceTests(TestCase):
+ def test_form_subclass_inheritance(self):
+ class Form(forms.Form):
+ age = forms.IntegerField()
+
+ class ModelForm(forms.ModelForm, Form):
+ class Meta:
+ model = Writer
+ fields = '__all__'
+
+ self.assertEqual(list(ModelForm().fields.keys()), ['name', 'age'])
Please sign in to comment.
Something went wrong with that request. Please try again.