From f9dc1379b874f80694a80e95f0f266a3d42f7368 Mon Sep 17 00:00:00 2001 From: Baptiste Mispelon Date: Sun, 7 Apr 2013 18:41:35 +0200 Subject: [PATCH] Fix #15126: Better error message when passing invalid options to ModelForm.Meta. --- django/forms/models.py | 15 +++++++++++++++ tests/model_forms/tests.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/django/forms/models.py b/django/forms/models.py index 5185e17c32..6a4b8b80e4 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -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, diff --git a/tests/model_forms/tests.py b/tests/model_forms/tests.py index e8c638389b..96c3ecbdce 100644 --- a/tests/model_forms/tests.py +++ b/tests/model_forms/tests.py @@ -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