Fixed #13095 -- `formfield_callback` keyword argument is now more sane and works…

Fixed #13095 -- `formfield_callback` keyword argument is now more sane and works with widgets defined in `ModelForm.Meta.widgets`.  Thanks, hvdklauw for bug report, vung for initial patch, and carljm for review.


git-svn-id: http://code.djangoproject.com/svn/django/trunk@13730 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/forms/models.py

@@ -150,7 +150,7 @@ def model_to_dict(instance, fields=None, exclude=None):
             data[f.name] = f.value_from_object(instance)
     return data
 
-def fields_for_model(model, fields=None, exclude=None, widgets=None, formfield_callback=lambda f, **kwargs: f.formfield(**kwargs)):
+def fields_for_model(model, fields=None, exclude=None, widgets=None, formfield_callback=None):
     """
     Returns a ``SortedDict`` containing form fields for the given model.
 
@@ -175,7 +175,14 @@ def fields_for_model(model, fields=None, exclude=None, widgets=None, formfield_c
             kwargs = {'widget': widgets[f.name]}
         else:
             kwargs = {}
-        formfield = formfield_callback(f, **kwargs)
+
+        if formfield_callback is None:
+            formfield = f.formfield(**kwargs)
+        elif not callable(formfield_callback):
+            raise TypeError('formfield_callback must be a function or callable')
+        else:
+            formfield = formfield_callback(f, **kwargs)
+
         if formfield:
             field_list.append((f.name, formfield))
         else:
@@ -198,8 +205,7 @@ class ModelFormOptions(object):
 
 class ModelFormMetaclass(type):
     def __new__(cls, name, bases, attrs):
-        formfield_callback = attrs.pop('formfield_callback',
-                lambda f, **kwargs: f.formfield(**kwargs))
+        formfield_callback = attrs.pop('formfield_callback', None)
         try:
             parents = [b for b in bases if issubclass(b, ModelForm)]
         except NameError:
@@ -376,7 +382,7 @@ class ModelForm(BaseModelForm):
     __metaclass__ = ModelFormMetaclass
 
 def modelform_factory(model, form=ModelForm, fields=None, exclude=None,
-                       formfield_callback=lambda f: f.formfield()):
+                       formfield_callback=None):
     # Create the inner Meta class. FIXME: ideally, we should be able to
     # construct a ModelForm without creating and passing in a temporary
     # inner class.
@@ -658,7 +664,7 @@ class BaseModelFormSet(BaseFormSet):
             form.fields[self._pk_field.name] = ModelChoiceField(qs, initial=pk_value, required=False, widget=HiddenInput)
         super(BaseModelFormSet, self).add_fields(form, index)
 
-def modelformset_factory(model, form=ModelForm, formfield_callback=lambda f: f.formfield(),
+def modelformset_factory(model, form=ModelForm, formfield_callback=None,
                          formset=BaseModelFormSet,
                          extra=1, can_delete=False, can_order=False,
                          max_num=None, fields=None, exclude=None):
@@ -813,7 +819,7 @@ def inlineformset_factory(parent_model, model, form=ModelForm,
                           formset=BaseInlineFormSet, fk_name=None,
                           fields=None, exclude=None,
                           extra=3, can_order=False, can_delete=True, max_num=None,
-                          formfield_callback=lambda f: f.formfield()):
+                          formfield_callback=None):
     """
     Returns an ``InlineFormSet`` for the given kwargs.
 

tests/regressiontests/model_forms_regress/tests.py

@@ -250,3 +250,47 @@ class URLFieldTests(TestCase):
         form.is_valid()
         # self.assertTrue(form.is_valid())
         # self.assertEquals(form.cleaned_data['url'], 'http://example.com/test')
+
+
+class FormFieldCallbackTests(TestCase):
+
+    def test_baseform_with_widgets_in_meta(self):
+        """Regression for #13095: Using base forms with widgets defined in Meta should not raise errors."""
+        widget = forms.Textarea()
+
+        class BaseForm(forms.ModelForm):
+            class Meta:
+                model = Person
+                widgets = {'name': widget}
+
+        Form = modelform_factory(Person, form=BaseForm)
+        self.assertTrue(Form.base_fields['name'].widget is widget)
+
+    def test_custom_callback(self):
+        """Test that a custom formfield_callback is used if provided"""
+
+        callback_args = []
+
+        def callback(db_field, **kwargs):
+            callback_args.append((db_field, kwargs))
+            return db_field.formfield(**kwargs)
+
+        widget = forms.Textarea()
+
+        class BaseForm(forms.ModelForm):
+            class Meta:
+                model = Person
+                widgets = {'name': widget}
+
+        _ = modelform_factory(Person, form=BaseForm,
+                              formfield_callback=callback)
+        id_field, name_field = Person._meta.fields
+
+        self.assertEqual(callback_args,
+                         [(id_field, {}), (name_field, {'widget': widget})])
+
+    def test_bad_callback(self):
+        # A bad callback provided by user still gives an error
+        self.assertRaises(TypeError, modelform_factory, Person,
+                          formfield_callback='not a function or callable')
+

tests/regressiontests/model_formsets_regress/tests.py

-from django.forms.models import modelform_factory, inlineformset_factory
+from django import forms
+from django.forms.models import modelform_factory, inlineformset_factory, modelformset_factory
 from django.test import TestCase
 
 from models import User, UserSite, Restaurant, Manager
 
+
 class InlineFormsetTests(TestCase):
     def test_formset_over_to_field(self):
         "A formset over a ForeignKey with a to_field can be saved. Regression for #10243"
@@ -156,3 +158,61 @@ class InlineFormsetTests(TestCase):
         # you can create a formset with an instance of None
         form = Form(instance=None)
         formset = FormSet(instance=None)
+
+
+class CustomWidget(forms.CharField):
+    pass
+
+
+class UserSiteForm(forms.ModelForm):
+    class Meta:
+        model = UserSite
+        widgets = {'data': CustomWidget}
+
+
+class Callback(object):
+
+    def __init__(self):
+        self.log = []
+
+    def __call__(self, db_field, **kwargs):
+        self.log.append((db_field, kwargs))
+        return db_field.formfield(**kwargs)
+
+
+class FormfieldCallbackTests(TestCase):
+    """
+    Regression for #13095: Using base forms with widgets
+    defined in Meta should not raise errors.
+    """
+
+    def test_inlineformset_factory_default(self):
+        Formset = inlineformset_factory(User, UserSite, form=UserSiteForm)
+        form = Formset({}).forms[0]
+        self.assertTrue(isinstance(form['data'].field.widget, CustomWidget))
+
+    def test_modelformset_factory_default(self):
+        Formset = modelformset_factory(UserSite, form=UserSiteForm)
+        form = Formset({}).forms[0]
+        self.assertTrue(isinstance(form['data'].field.widget, CustomWidget))
+
+    def assertCallbackCalled(self, callback):
+        id_field, user_field, data_field = UserSite._meta.fields
+        expected_log = [
+            (id_field, {}),
+            (user_field, {}),
+            (data_field, {'widget': CustomWidget}),
+        ]
+        self.assertEqual(callback.log, expected_log)
+
+    def test_inlineformset_custom_callback(self):
+        callback = Callback()
+        inlineformset_factory(User, UserSite, form=UserSiteForm,
+                              formfield_callback=callback)
+        self.assertCallbackCalled(callback)
+
+    def test_modelformset_custom_callback(self):
+        callback = Callback()
+        modelformset_factory(UserSite, form=UserSiteForm,
+                             formfield_callback=callback)
+        self.assertCallbackCalled(callback)