From 6c15b5db6014a7dadb0c237e689bdaed4761fb16 Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Wed, 22 Apr 2009 15:48:51 +0000 Subject: [PATCH] Fixed #10208: `ModelAdmin` now respects the `exclude` and `field` atributes of custom `ModelForm`s. Thanks, Alex Gaynor. git-svn-id: http://code.djangoproject.com/svn/django/trunk@10619 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/options.py | 8 +++- django/forms/models.py | 43 ++++++++++++++-------- tests/regressiontests/modeladmin/models.py | 42 ++++++++++++++++++++- 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index c87d903ec5..7a40446847 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -336,10 +336,12 @@ class ModelAdmin(BaseModelAdmin): exclude = [] else: exclude = list(self.exclude) + # if exclude is an empty list we pass None to be consistant with the + # default on modelform_factory defaults = { "form": self.form, "fields": fields, - "exclude": exclude + kwargs.get("exclude", []), + "exclude": (exclude + kwargs.get("exclude", [])) or None, "formfield_callback": curry(self.formfield_for_dbfield, request=request), } defaults.update(kwargs) @@ -1138,12 +1140,14 @@ class InlineModelAdmin(BaseModelAdmin): exclude = [] else: exclude = list(self.exclude) + # if exclude is an empty list we use None, since that's the actual + # default defaults = { "form": self.form, "formset": self.formset, "fk_name": self.fk_name, "fields": fields, - "exclude": exclude + kwargs.get("exclude", []), + "exclude": (exclude + kwargs.get("exclude", [])) or None, "formfield_callback": curry(self.formfield_for_dbfield, request=request), "extra": self.extra, "max_num": self.max_num, diff --git a/django/forms/models.py b/django/forms/models.py index d40a1ee55e..626e727e3a 100644 --- a/django/forms/models.py +++ b/django/forms/models.py @@ -344,16 +344,34 @@ class ModelForm(BaseModelForm): def modelform_factory(model, form=ModelForm, fields=None, exclude=None, formfield_callback=lambda f: f.formfield()): - # HACK: we should be able to construct a ModelForm without creating - # and passing in a temporary inner class - class Meta: - pass - setattr(Meta, 'model', model) - setattr(Meta, 'fields', fields) - setattr(Meta, 'exclude', exclude) + # Create the inner Meta class. FIXME: ideally, we should be able to + # construct a ModelForm without creating and passing in a temporary + # inner class. + + # Build up a list of attributes that the Meta object will have. + attrs = {'model': model} + if fields is not None: + attrs['fields'] = fields + if exclude is not None: + attrs['exclude'] = exclude + + # If parent form class already has an inner Meta, the Meta we're + # creating needs to inherit from the parent's inner meta. + parent = (object,) + if hasattr(form, 'Meta'): + parent = (form.Meta, object) + Meta = type('Meta', parent, attrs) + + # Give this new form class a reasonable name. class_name = model.__name__ + 'Form' - return ModelFormMetaclass(class_name, (form,), {'Meta': Meta, - 'formfield_callback': formfield_callback}) + + # Class attributes for the new form class. + form_class_attrs = { + 'Meta': Meta, + 'formfield_callback': formfield_callback + } + + return ModelFormMetaclass(class_name, (form,), form_class_attrs) # ModelFormSets ############################################################## @@ -617,13 +635,6 @@ def inlineformset_factory(parent_model, model, form=ModelForm, # enforce a max_num=1 when the foreign key to the parent model is unique. if fk.unique: max_num = 1 - if fields is not None: - fields = list(fields) - fields.append(fk.name) - else: - # get all the fields for this model that will be generated. - fields = fields_for_model(model, fields, exclude, formfield_callback).keys() - fields.append(fk.name) kwargs = { 'form': form, 'formfield_callback': formfield_callback, diff --git a/tests/regressiontests/modeladmin/models.py b/tests/regressiontests/modeladmin/models.py index 3a7d3f031f..ef05b5a048 100644 --- a/tests/regressiontests/modeladmin/models.py +++ b/tests/regressiontests/modeladmin/models.py @@ -37,7 +37,7 @@ class ValidationTestInlineModel(models.Model): __test__ = {'API_TESTS': """ ->>> from django.contrib.admin.options import ModelAdmin, HORIZONTAL, VERTICAL +>>> from django.contrib.admin.options import ModelAdmin, TabularInline, HORIZONTAL, VERTICAL >>> from django.contrib.admin.sites import AdminSite None of the following tests really depend on the content of the request, so @@ -262,6 +262,46 @@ blank=True for the model field. Finally, the widget should have the >>> list(cmafa.base_fields['transport'].widget.choices) [('', u'None'), (1, 'Plane'), (2, 'Train'), (3, 'Bus')] +>>> class AdminConcertForm(forms.ModelForm): +... class Meta: +... model = Concert +... exclude = ('transport',) + +>>> class ConcertAdmin(ModelAdmin): +... form = AdminConcertForm + +>>> ma = ConcertAdmin(Concert, site) +>>> ma.get_form(request).base_fields.keys() +['main_band', 'opening_band', 'day'] + +>>> class AdminConcertForm(forms.ModelForm): +... extra = forms.CharField() +... class Meta: +... model = Concert +... fields = ['extra', 'transport'] + +>>> class ConcertAdmin(ModelAdmin): +... form = AdminConcertForm + +>>> ma = ConcertAdmin(Concert, site) +>>> ma.get_form(request).base_fields.keys() +['extra', 'transport'] + +>>> class ConcertInline(TabularInline): +... form = AdminConcertForm +... model = Concert +... fk_name = 'main_band' + +>>> class BandAdmin(ModelAdmin): +... inlines = [ +... ConcertInline +... ] + +>>> ma = BandAdmin(Band, site) +>>> list(ma.get_formsets(request))[0]().forms[0].fields.keys() +['extra', 'transport', 'id', 'DELETE', 'main_band'] + + >>> band.delete() # ModelAdmin Option Validation ################################################