modelform_factory returns a broken form when given wrong value for fields

[ben]

modelform_factory is breaking where you ask for an invalid fields option (even if the field exists):

Here's a doctest that demonstrates:

>>> from django.forms.models import modelform_factory, form_for_fields
>>> from django.contrib.auth.models import User
>>> Form = modelform_factory(User, fields=['id', 'username'])
>>> Form.base_fields
{'id': None, 'username': <django.forms.fields.CharField object at 0xd4e4ad0>}
>>> print Form()
------------------------------------------------------------
Traceback (most recent call last):
  ...
    if self.field.label is None:
AttributeError: 'NoneType' object has no attribute 'label'

Likewise you can specify fields that don't even exist on the model:

>>> Form = modelform_factory(User, fields=['no-field', 'username'])
>>> Form.base_fields
{'no-field': None, 'username': <django.forms.fields.CharField object at 0xd4eec10>}
>>> print Form()
------------------------------------------------------------
Traceback (most recent call last):
  ...
    if self.field.label is None:
AttributeError: 'NoneType' object has no attribute 'label'

This appears to be due to logic in form_for_fields (line 102 in my checkout) in the first case, and fields_for_model (line 170) in the second.

I'd propose some error handling be put in place to give meaningfull exception in either of these cases rather than return a broken form to the user.

I'm happy to submit a patch (I can't do it from here as I'm at work and don't have direct access to django's svn repo) but I can do it from home.

Ben

[ben]

Sorry screwed up the formatting, try:

>>> from django.forms.models import modelform_factory, form_for_fields
>>> from django.contrib.auth.models import User
>>> Form = modelform_factory(User, fields=['id', 'username'])
>>> Form.base_fields
{'id': None, 'username': <django.forms.fields.CharField object at 0xd4e4ad0>}
>>> print Form()
Traceback (most recent call last):
    ....
    if self.field.label is None:
AttributeError: 'NoneType' object has no attribute 'label'

>>> Form = modelform_factory(User, fields=['no-field', 'username'])
>>> Form.base_fields
{'no-field': None, 'username': <django.forms.fields.CharField object at 0xd4eec10>}
>>> print Form()
Traceback (most recent call last):
    ....
AttributeError: 'NoneType' object has no attribute 'label'

[Rémy Hubscher]

I wish this could have a patch as mentionned.

I get the same error on the SVN version and I don't know what is wrong.
​http://paste.pocoo.org/show/146130/

Thank you.

[Johnny82]

I have the same error

Caught an exception while rendering: 'NoneType' object has no attribute 'label'

it's occurs on Admin page when i want to use editor to add data to a Model
if comment such models field, and remove it from Admin class for that model, all works fine

techspec = models.ManyToManyField('TechSpec', through = 'ModelTechSpec')

[James Bennett]

While better error reporting is always nice, it's not release-critical and can be bumped to 1.3.

[Colin Copeland]

Checking for unknown fields in ​fields_for_model will break some existing functionality:

1. django.contrib.auth uses a custom ​form for user creation that passes invalid fields (password1 and password2) to fields_for_model.

2. If a declarative field is listed in ModelForm._meta.fields (as described ​here), but is not an actual field of the Model, an error will be raised with this change when it previously succeeded. Current tests (like this ​one) will fail with this issue.

To test this, I decided to raise a FieldError in fields_for_model when a passed field is not part of the related model. The diff can be found ​here. I'd be happy to attempt a fix at both the auth and declarative field issues, but I'd like to get some feedback first. Any thoughts?

[Colin Copeland]

After talking with Karen, I decided to look for a different spot to validate the field list. I ended up modifying ModelFormMetaclass to cross check the fields generated from fields_for_model() against the form's declared fields. fields_for_model returns a mapping of field_name->field, but field will be None if field_name is not found on the specified model. If the missing field name is not found in the declared field list, then the field is considered invalid and will raise a FieldError. With this change, the exception is raised when creating the factory, rather than when rending the form. Patch with tests is attached.

[Karen Tracey]

(In [13739]) Fixed #11905: Raise an error on model form creation if a non-existent field was listed in fields. Thanks ben and copelco.

[Jacob]

Milestone 1.3 deleted