id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
6337	ModelForm inheritance logic is broken	Ilya Semenov	nobody	"A simple example:

{{{
#!python
class TicketForm(forms.ModelForm):
  class Meta:
    model = Ticket
    fields = ('priority','summary')

class EditTicketForm(TicketForm):
  pass
}}}

Supposedly, the program behaviour generally should not change if ``TicketForm`` usage is replaced with ``EditTicketForm``. However, it changes in a very unexpected way (instance saving still works fine, but the ""fields"" filter is gone!). Actually it brings some more ""gotchas"", but they're harder to explain with a simple example.

When I looked closer, I discovered that ``django.newforms.models.ModelFormMetaclass`` is not very well thought when it comes to the inheritance.

First, regarding the simple problem mentioned above, it says:
{{{
#!python
class ModelFormMetaclass(type):
  ...
  opts = ModelFormOptions(attrs.get('Meta', None))
  attrs['_meta'] = opts
}}}
It can be easily seen that for ``EditTicketForm``, as there's no Meta in it, opts becomes ""empty"" ModelFormOptions with ``model=None, fields=None, and exclude=None``. Therefore, the further save() call doesn't use proper ""fields"" filter, since it is lost in the parent class'es Meta.

Another related issue is that ``ModelFormMetaclass`` (according to the code and comments) is supposed to allow only one base class with the defined Meta attribute. I can see the rationale behind this limitation - it wouldn't be easy to deal with several Metas with contradictory fields= or exclude=. However, the current implementation is broken in that case, as it actually allows to have two Metas (one in the base classes, and one in the child class), with unpredictable results. We need either to disallow that completely, or come up with some logic which handles these cases accurately.

The proposed patch fixes the arisen problems. With inherited Metas, the following algorithm is imposed:[[br]]
- If a child class defines a Meta class, it is taken as an argument to ModelFormOptions.[[br]]
- If a child class doesn't define a Meta, it is taken from its parents. The parents are obliged to define no more than one Meta.
- If neither a child class nor its parents define any Meta class, an ""empty"" ModelFormOptions is created.

Also, I extracted a function from DeclarativeFieldsMetaclass to reuse it (and get rid of copy-pasted code) in ModelFormMetaclass.

The patch includes documentation describing the proposed behaviour (previously, the model form inheritance was not covered in the documentation at all)."		closed	Forms	dev		fixed	ModelForm inheritance		Accepted	1	0	1	0	0	0