From 049b3ff8a25a3907e7791091cb6a87910cff95df Mon Sep 17 00:00:00 2001 From: Ramiro Morales Date: Sat, 26 Feb 2011 02:00:18 +0000 Subject: [PATCH] [1.2.X] Fixed #15424 -- Corrected lookup of callables listed in admin inlines' `readonly_fields` by passing the right ModelAdmin (sub)class instance when instantiating inline forms admin wrappers. Also, added early validation of its elements. Thanks kmike for the report and Karen for the patch fixing the issue. Backport of [15650] from trunk. git-svn-id: http://code.djangoproject.com/svn/django/branches/releases/1.2.X@15651 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/admin/helpers.py | 8 ++-- django/contrib/admin/util.py | 2 +- django/contrib/admin/validation.py | 26 +++++++----- tests/regressiontests/admin_inlines/models.py | 40 +++++++++++++++++++ tests/regressiontests/admin_inlines/tests.py | 19 +++++++++ .../admin_validation/models.py | 8 ++++ .../regressiontests/admin_validation/tests.py | 16 +++++--- 7 files changed, 99 insertions(+), 20 deletions(-) diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 101c1e7de3..83af5a1803 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -207,14 +207,14 @@ class InlineAdminFormSet(object): for form, original in zip(self.formset.initial_forms, self.formset.get_queryset()): yield InlineAdminForm(self.formset, form, self.fieldsets, self.opts.prepopulated_fields, original, self.readonly_fields, - model_admin=self.model_admin) + model_admin=self.opts) for form in self.formset.extra_forms: yield InlineAdminForm(self.formset, form, self.fieldsets, self.opts.prepopulated_fields, None, self.readonly_fields, - model_admin=self.model_admin) + model_admin=self.opts) yield InlineAdminForm(self.formset, self.formset.empty_form, self.fieldsets, self.opts.prepopulated_fields, None, - self.readonly_fields, model_admin=self.model_admin) + self.readonly_fields, model_admin=self.opts) def fields(self): fk = getattr(self.formset, "fk", None) @@ -223,7 +223,7 @@ class InlineAdminFormSet(object): continue if field in self.readonly_fields: yield { - 'label': label_for_field(field, self.opts.model, self.model_admin), + 'label': label_for_field(field, self.opts.model, self.opts), 'widget': { 'is_hidden': False }, diff --git a/django/contrib/admin/util.py b/django/contrib/admin/util.py index ba86dba769..607916f3c7 100644 --- a/django/contrib/admin/util.py +++ b/django/contrib/admin/util.py @@ -300,7 +300,7 @@ def label_for_field(name, model, model_admin=None, return_attr=False): else: message = "Unable to lookup '%s' on %s" % (name, model._meta.object_name) if model_admin: - message += " or %s" % (model_admin.__name__,) + message += " or %s" % (model_admin.__class__.__name__,) raise AttributeError(message) if hasattr(attr, "short_description"): diff --git a/django/contrib/admin/validation.py b/django/contrib/admin/validation.py index 64cafde812..ee92b88d68 100644 --- a/django/contrib/admin/validation.py +++ b/django/contrib/admin/validation.py @@ -121,16 +121,7 @@ def validate(cls, model): get_field(cls, model, opts, 'ordering[%d]' % idx, field) if hasattr(cls, "readonly_fields"): - check_isseq(cls, "readonly_fields", cls.readonly_fields) - for idx, field in enumerate(cls.readonly_fields): - if not callable(field): - if not hasattr(cls, field): - if not hasattr(model, field): - try: - opts.get_field(field) - except models.FieldDoesNotExist: - raise ImproperlyConfigured("%s.readonly_fields[%d], %r is not a callable or an attribute of %r or found in the model %r." - % (cls.__name__, idx, field, cls.__name__, model._meta.object_name)) + check_readonly_fields(cls, model, opts) # list_select_related = False # save_as = False @@ -191,6 +182,9 @@ def validate_inline(cls, parent, parent_model): "'%s' - this is the foreign key to the parent model " "%s." % (cls.__name__, fk.name, parent_model.__name__)) + if hasattr(cls, "readonly_fields"): + check_readonly_fields(cls, cls.model, cls.model._meta) + def validate_base(cls, model): opts = model._meta @@ -376,3 +370,15 @@ def fetch_attr(cls, model, opts, label, field): except AttributeError: raise ImproperlyConfigured("'%s.%s' refers to '%s' that is neither a field, method or property of model '%s'." % (cls.__name__, label, field, model.__name__)) + +def check_readonly_fields(cls, model, opts): + check_isseq(cls, "readonly_fields", cls.readonly_fields) + for idx, field in enumerate(cls.readonly_fields): + if not callable(field): + if not hasattr(cls, field): + if not hasattr(model, field): + try: + opts.get_field(field) + except models.FieldDoesNotExist: + raise ImproperlyConfigured("%s.readonly_fields[%d], %r is not a callable or an attribute of %r or found in the model %r." + % (cls.__name__, idx, field, cls.__name__, model._meta.object_name)) diff --git a/tests/regressiontests/admin_inlines/models.py b/tests/regressiontests/admin_inlines/models.py index bb299f32b8..ee0abd1d3d 100644 --- a/tests/regressiontests/admin_inlines/models.py +++ b/tests/regressiontests/admin_inlines/models.py @@ -151,3 +151,43 @@ class TitleInline(admin.TabularInline): extra = 1 admin.site.register(TitleCollection, inlines=[TitleInline]) + +# Models for #15424 + +class Poll(models.Model): + name = models.CharField(max_length=40) + +class Question(models.Model): + poll = models.ForeignKey(Poll) + +class QuestionInline(admin.TabularInline): + model = Question + readonly_fields=['call_me'] + + def call_me(self, obj): + return 'Callable in QuestionInline' + +class PollAdmin(admin.ModelAdmin): + inlines = [QuestionInline] + + def call_me(self, obj): + return 'Callable in PollAdmin' + +class Novel(models.Model): + name = models.CharField(max_length=40) + +class Chapter(models.Model): + novel = models.ForeignKey(Novel) + +class ChapterInline(admin.TabularInline): + model = Chapter + readonly_fields=['call_me'] + + def call_me(self, obj): + return 'Callable in ChapterInline' + +class NovelAdmin(admin.ModelAdmin): + inlines = [ChapterInline] + +admin.site.register(Poll, PollAdmin) +admin.site.register(Novel, NovelAdmin) diff --git a/tests/regressiontests/admin_inlines/tests.py b/tests/regressiontests/admin_inlines/tests.py index 915c6fac8d..067b3c5eaf 100644 --- a/tests/regressiontests/admin_inlines/tests.py +++ b/tests/regressiontests/admin_inlines/tests.py @@ -84,6 +84,25 @@ class TestInline(TestCase): # Here colspan is "4": two fields (title1 and title2), one hidden field and the delete checkbock. self.assertContains(response, '') + def test_no_parent_callable_lookup(self): + """Admin inline `readonly_field` shouldn't invoke parent ModelAdmin callable""" + # Identically named callable isn't present in the parent ModelAdmin, + # rendering of the add view shouldn't explode + response = self.client.get('/test_admin/admin/admin_inlines/novel/add/') + self.assertEqual(response.status_code, 200) + # View should have the child inlines section + self.assertContains(response, '
') + + def test_callable_lookup(self): + """Admin inline should invoke local callable when its name is listed in readonly_fields""" + response = self.client.get('/test_admin/admin/admin_inlines/poll/add/') + self.assertEqual(response.status_code, 200) + # Add parent object view should have the child inlines section + self.assertContains(response, '
') + # The right callabe should be used for the inline readonly_fields + # column cells + self.assertContains(response, '

Callable in QuestionInline

') + class TestInlineMedia(TestCase): fixtures = ['admin-views-users.xml'] diff --git a/tests/regressiontests/admin_validation/models.py b/tests/regressiontests/admin_validation/models.py index 24387cc363..5e080a9232 100644 --- a/tests/regressiontests/admin_validation/models.py +++ b/tests/regressiontests/admin_validation/models.py @@ -45,3 +45,11 @@ class Book(models.Model): class AuthorsBooks(models.Model): author = models.ForeignKey(Author) book = models.ForeignKey(Book) + + +class State(models.Model): + name = models.CharField(max_length=15) + + +class City(models.Model): + state = models.ForeignKey(State) diff --git a/tests/regressiontests/admin_validation/tests.py b/tests/regressiontests/admin_validation/tests.py index 1872ca55e2..6fbdc8040e 100644 --- a/tests/regressiontests/admin_validation/tests.py +++ b/tests/regressiontests/admin_validation/tests.py @@ -4,7 +4,7 @@ from django.contrib.admin.validation import validate, validate_inline, \ ImproperlyConfigured from django.test import TestCase -from models import Song, Book, Album, TwoAlbumFKAndAnE +from models import Song, Book, Album, TwoAlbumFKAndAnE, State, City class SongForm(forms.ModelForm): pass @@ -162,6 +162,16 @@ class ValidationTestCase(TestCase): validate, SongAdmin, Song) + def test_nonexistant_field_on_inline(self): + class CityInline(admin.TabularInline): + model = City + readonly_fields=['i_dont_exist'] # Missing attribute + + self.assertRaisesMessage(ImproperlyConfigured, + "CityInline.readonly_fields[0], 'i_dont_exist' is not a callable or an attribute of 'CityInline' or found in the model 'City'.", + validate_inline, + CityInline, None, State) + def test_extra(self): class SongAdmin(admin.ModelAdmin): def awesome_song(self, instance): @@ -241,7 +251,3 @@ class ValidationTestCase(TestCase): fields = ['title', 'extra_data'] validate(FieldsOnFormOnlyAdmin, Song) - - - -