Commit b5c0b3d1 authored by Jannis Leidel's avatar Jannis Leidel
Browse files

Merge pull request #1000 from bmispelon/ticket-15126 

Fix #15126: Better error message when passing invalid options to ModelFo...
parents 5ab66dea f9dc1379

django/forms/models.py

+15 −0
Original line number Diff line number Diff line
@@ -205,6 +205,21 @@ class ModelFormMetaclass(type): 
        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`,

        # which is likely to be a mistake where the user typed ('foo') instead

        # of ('foo',)

        for opt in ['fields', 'exclude']:

            value = getattr(opts, opt)

            if isinstance(value, six.string_types):

                msg = ("%(model)s.Meta.%(opt)s cannot be a string. "

                       "Did you mean to type: ('%(value)s',)?" % {

                           'model': new_class.__name__,

                           'opt': opt,

                           'value': value,

                       })

                raise TypeError(msg)



        if opts.model:

            # If a model is defined, extract form fields from it.

            fields = fields_for_model(opts.model, opts.fields,

tests/model_forms/tests.py

+34 −0
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ import os 
from decimal import Decimal



from django import forms

from django.core.exceptions import FieldError

from django.core.files.uploadedfile import SimpleUploadedFile

from django.core.validators import ValidationError

from django.db import connection
@@ -228,6 +229,22 @@ class ModelFormBaseTest(TestCase): 
        self.assertEqual(list(LimitFields.base_fields),

                         ['url'])



    def test_limit_nonexistent_field(self):

        expected_msg = 'Unknown field(s) (nonexistent) specified for Category'

        with self.assertRaisesMessage(FieldError, expected_msg):

            class InvalidCategoryForm(forms.ModelForm):

                class Meta:

                    model = Category

                    fields = ['nonexistent']



    def test_limit_fields_with_string(self):

        expected_msg = "CategoryForm.Meta.fields cannot be a string. Did you mean to type: ('url',)?"

        with self.assertRaisesMessage(TypeError, expected_msg):

            class CategoryForm(forms.ModelForm):

                class Meta:

                    model = Category

                    fields = ('url') # note the missing comma



    def test_exclude_fields(self):

        class ExcludeFields(forms.ModelForm):

            class Meta:
@@ -237,6 +254,23 @@ class ModelFormBaseTest(TestCase): 
        self.assertEqual(list(ExcludeFields.base_fields),

                         ['name', 'slug'])



    def test_exclude_nonexistent_field(self):

        class ExcludeFields(forms.ModelForm):

            class Meta:

                model = Category

                exclude = ['nonexistent']



        self.assertEqual(list(ExcludeFields.base_fields),

                         ['name', 'slug', 'url'])



    def test_exclude_fields_with_string(self):

        expected_msg = "CategoryForm.Meta.exclude cannot be a string. Did you mean to type: ('url',)?"

        with self.assertRaisesMessage(TypeError, expected_msg):

            class CategoryForm(forms.ModelForm):

                class Meta:

                    model = Category

                    exclude = ('url') # note the missing comma



    def test_confused_form(self):

        class ConfusedForm(forms.ModelForm):

            """ Using 'fields' *and* 'exclude'. Not sure why you'd want to do