From 0a07e0d820f31f47c0052cf62a6b2b7110d5348a Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Mon, 6 Dec 2010 14:37:05 -0500 Subject: [PATCH] ModelAdmin validation: allow non-model fields in list_display_links (#11058) In addition to model field names, list_display accepts callables, ModelAdmin attributes or Model attributes. This patch reuses the list_display resolution for these values for list_display_links validation as well and adds a test for each of the three extra types. See http://code.djangoproject.com/ticket/11058 --- django/contrib/admin/validation.py | 18 +++++++++++-- docs/ref/contrib/admin/index.txt | 10 ++++---- tests/regressiontests/admin_validation/tests.py | 30 ++++++++++++++++++++++- tests/regressiontests/modeladmin/models.py | 2 +- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/django/contrib/admin/validation.py b/django/contrib/admin/validation.py index bee2891..9a653ea 100644 --- a/django/contrib/admin/validation.py +++ b/django/contrib/admin/validation.py @@ -22,35 +22,47 @@ def validate(cls, model): opts = model._meta validate_base(cls, model) + # To handle values other than pure strings, we'll maintain a list of + # possible values allowing the possibility of using ModelAdmin fields + # or arbitrary callables in list_display_links. + list_display_fields = set() + # list_display if hasattr(cls, 'list_display'): check_isseq(cls, 'list_display', cls.list_display) for idx, field in enumerate(cls.list_display): + list_display_fields.add(field) + if not callable(field): if not hasattr(cls, field): if not hasattr(model, field): try: - opts.get_field(field) + f = opts.get_field(field) except models.FieldDoesNotExist: raise ImproperlyConfigured("%s.list_display[%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)) else: # getattr(model, field) could be an X_RelatedObjectsDescriptor f = fetch_attr(cls, model, opts, "list_display[%d]" % idx, field) + if isinstance(f, models.ManyToManyField): raise ImproperlyConfigured("'%s.list_display[%d]', '%s' is a ManyToManyField which is not supported." % (cls.__name__, idx, field)) + list_display_fields.add(f) + # list_display_links if hasattr(cls, 'list_display_links'): check_isseq(cls, 'list_display_links', cls.list_display_links) + for idx, field in enumerate(cls.list_display_links): - fetch_attr(cls, model, opts, 'list_display_links[%d]' % idx, field) - if field not in cls.list_display: + if field not in list_display_fields: raise ImproperlyConfigured("'%s.list_display_links[%d]'" "refers to '%s' which is not defined in 'list_display'." % (cls.__name__, idx, field)) + del list_display_fields + # list_filter if hasattr(cls, 'list_filter'): check_isseq(cls, 'list_filter', cls.list_filter) diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 0550576..0cc6c3c 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -404,12 +404,12 @@ be linked to the "change" page for an object. By default, the change list page will link the first column -- the first field specified in ``list_display`` -- to the change page for each item. But ``list_display_links`` lets you change which columns are linked. Set -``list_display_links`` to a list or tuple of field names (in the same format as -``list_display``) to link. +``list_display_links`` to a list or tuple of fields in the same format as +``list_display`` to link. -``list_display_links`` can specify one or many field names. As long as the -field names appear in ``list_display``, Django doesn't care how many (or how -few) fields are linked. The only requirement is: If you want to use +``list_display_links`` can specify one or many fields. As long as the fields +appear in ``list_display``, Django doesn't care how many (or how few) fields +are linked. The only requirement is: If you want to use ``list_display_links``, you must define ``list_display``. In this example, the ``first_name`` and ``last_name`` fields will be linked on diff --git a/tests/regressiontests/admin_validation/tests.py b/tests/regressiontests/admin_validation/tests.py index 9166360..aa44e92 100644 --- a/tests/regressiontests/admin_validation/tests.py +++ b/tests/regressiontests/admin_validation/tests.py @@ -6,7 +6,9 @@ from models import Song class ValidationTestCase(TestCase): + def test_readonly_and_editable(self): + class SongAdmin(admin.ModelAdmin): readonly_fields = ["original_release"] fieldsets = [ @@ -14,5 +16,31 @@ class ValidationTestCase(TestCase): "fields": ["title", "original_release"], }), ] - + + validate(SongAdmin, Song) + + def test_list_display_links(self): + """ + Confirm that list_display_links supports model fields, model admin + methods and arbitrary callables + """ + + def fancy_formatter(obj): + return "Custom display of %s" % obj + + class SongAdmin(admin.ModelAdmin): + foo_date = fancy_formatter + + list_display = ("title", "album", "original_release", + "readonly_method_on_model", "short_title", + foo_date) + list_display_links = ("title", "album", "original_release", + "readonly_method_on_model", "short_title", + foo_date) + + def short_title(self, obj): + # In real code this would use truncatewords/chars: + return obj.title[:20] + short_title.short_description = "Short Title" + validate(SongAdmin, Song) diff --git a/tests/regressiontests/modeladmin/models.py b/tests/regressiontests/modeladmin/models.py index 36ea416..8ca2031 100644 --- a/tests/regressiontests/modeladmin/models.py +++ b/tests/regressiontests/modeladmin/models.py @@ -653,7 +653,7 @@ ImproperlyConfigured: 'ValidationTestModelAdmin.list_display_links' must be a li >>> validate(ValidationTestModelAdmin, ValidationTestModel) Traceback (most recent call last): ... -ImproperlyConfigured: 'ValidationTestModelAdmin.list_display_links[0]' refers to 'non_existent_field' that is neither a field, method or property of model 'ValidationTestModel'. +ImproperlyConfigured: 'ValidationTestModelAdmin.list_display_links[0]'refers to 'non_existent_field' which is not defined in 'list_display'. >>> class ValidationTestModelAdmin(ModelAdmin): ... list_display_links = ('name',) -- 1.7.0.2