From f9dc1379b874f80694a80e95f0f266a3d42f7368 Mon Sep 17 00:00:00 2001
From: Baptiste Mispelon <bmispelon@gmail.com>
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