From 0207bdd2d4157c542c981264c86706b78ca246e9 Mon Sep 17 00:00:00 2001 From: Loek van Gent Date: Fri, 13 Mar 2015 11:08:03 +0100 Subject: [PATCH] Fixed #24474 -- Allowed configuring the admin's empty change list value. --- django/contrib/admin/filters.py | 8 +- django/contrib/admin/helpers.py | 6 +- django/contrib/admin/options.py | 9 +++ django/contrib/admin/sites.py | 10 +++ .../contrib/admin/templatetags/admin_list.py | 14 ++-- django/contrib/admin/utils.py | 12 ++- django/contrib/admin/views/main.py | 3 - docs/ref/contrib/admin/index.txt | 65 +++++++++++++++- docs/releases/1.9.txt | 6 ++ tests/admin_changelist/admin.py | 9 +++ tests/admin_changelist/tests.py | 74 ++++++++++++++++--- tests/admin_utils/tests.py | 42 ++++++----- 12 files changed, 205 insertions(+), 53 deletions(-) diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index a5704812cf..d234d28e3c 100644 --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -174,6 +174,7 @@ class RelatedFieldListFilter(FieldListFilter): else: self.lookup_title = other_model._meta.verbose_name self.title = self.lookup_title + self.empty_value_display = model_admin.get_empty_value_display() def has_output(self): if self.field.null: @@ -189,7 +190,6 @@ class RelatedFieldListFilter(FieldListFilter): return field.get_choices(include_blank=False) def choices(self, cl): - from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE yield { 'selected': self.lookup_val is None and not self.lookup_val_isnull, 'query_string': cl.get_query_string({}, @@ -210,7 +210,7 @@ class RelatedFieldListFilter(FieldListFilter): 'query_string': cl.get_query_string({ self.lookup_kwarg_isnull: 'True', }, [self.lookup_kwarg]), - 'display': EMPTY_CHANGELIST_VALUE, + 'display': self.empty_value_display, } FieldListFilter.register(lambda f: f.remote_field, RelatedFieldListFilter) @@ -352,6 +352,7 @@ class AllValuesFieldListFilter(FieldListFilter): self.lookup_kwarg_isnull = '%s__isnull' % field_path self.lookup_val = request.GET.get(self.lookup_kwarg) self.lookup_val_isnull = request.GET.get(self.lookup_kwarg_isnull) + self.empty_value_display = model_admin.get_empty_value_display() parent_model, reverse_path = reverse_field_path(model, field_path) # Obey parent ModelAdmin queryset when deciding which options to show if model == parent_model: @@ -369,7 +370,6 @@ class AllValuesFieldListFilter(FieldListFilter): return [self.lookup_kwarg, self.lookup_kwarg_isnull] def choices(self, cl): - from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE yield { 'selected': (self.lookup_val is None and self.lookup_val_isnull is None), @@ -396,7 +396,7 @@ class AllValuesFieldListFilter(FieldListFilter): 'query_string': cl.get_query_string({ self.lookup_kwarg_isnull: 'True', }, [self.lookup_kwarg]), - 'display': EMPTY_CHANGELIST_VALUE, + 'display': self.empty_value_display, } FieldListFilter.register(lambda f: True, AllValuesFieldListFilter) diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index 1168474e20..57c74434f5 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -174,6 +174,7 @@ class AdminReadonlyField(object): self.is_first = is_first self.is_checkbox = False self.is_readonly = True + self.empty_value_display = model_admin.get_empty_value_display() def label_tag(self): attrs = {} @@ -186,12 +187,11 @@ class AdminReadonlyField(object): def contents(self): from django.contrib.admin.templatetags.admin_list import _boolean_icon - from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE field, obj, model_admin = self.field['field'], self.form.instance, self.model_admin try: f, attr, value = lookup_field(field, obj, model_admin) except (AttributeError, ValueError, ObjectDoesNotExist): - result_repr = EMPTY_CHANGELIST_VALUE + result_repr = self.empty_value_display else: if f is None: boolean = getattr(attr, "boolean", False) @@ -207,7 +207,7 @@ class AdminReadonlyField(object): if isinstance(f.remote_field, ManyToManyRel) and value is not None: result_repr = ", ".join(map(six.text_type, value.all())) else: - result_repr = display_for_field(value, f) + result_repr = display_for_field(value, f, self.empty_value_display) return conditional_escape(result_repr) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index ece061490f..cc20872049 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -273,6 +273,15 @@ class BaseModelAdmin(six.with_metaclass(forms.MediaDefiningClass)): 'object_id': obj.pk }) + def get_empty_value_display(self): + """ + Return the empty_value_display set on ModelAdmin or AdminSite. + """ + try: + return mark_safe(self.empty_value_display) + except AttributeError: + return mark_safe(self.admin_site.empty_value_display) + def get_fields(self, request, obj=None): """ Hook for specifying fields. diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 3dc5075443..37cb5f2f7e 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -48,6 +48,8 @@ class AdminSite(object): # URL for the "View site" link at the top of each admin page. site_url = '/' + _empty_value_display = '-' + login_form = None index_template = None app_index_template = None @@ -154,6 +156,14 @@ class AdminSite(object): """ return six.iteritems(self._actions) + @property + def empty_value_display(self): + return self._empty_value_display + + @empty_value_display.setter + def empty_value_display(self, empty_value_display): + self._empty_value_display = empty_value_display + def has_permission(self, request): """ Returns True if the given HttpRequest has permission to view diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index fe1dee0b0f..f3fbaa7ac1 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -8,7 +8,7 @@ from django.contrib.admin.utils import ( display_for_field, display_for_value, label_for_field, lookup_field, ) from django.contrib.admin.views.main import ( - ALL_VAR, EMPTY_CHANGELIST_VALUE, ORDER_VAR, PAGE_VAR, SEARCH_VAR, + ALL_VAR, ORDER_VAR, PAGE_VAR, SEARCH_VAR, ) from django.core.exceptions import ObjectDoesNotExist from django.core.urlresolvers import NoReverseMatch @@ -194,20 +194,22 @@ def items_for_result(cl, result, form): first = True pk = cl.lookup_opts.pk.attname for field_name in cl.list_display: + empty_value_display = cl.model_admin.get_empty_value_display() row_classes = ['field-%s' % field_name] try: f, attr, value = lookup_field(field_name, result, cl.model_admin) except ObjectDoesNotExist: - result_repr = EMPTY_CHANGELIST_VALUE + result_repr = empty_value_display else: + empty_value_display = getattr(attr, 'empty_value_display', empty_value_display) if f is None or f.auto_created: if field_name == 'action_checkbox': row_classes = ['action-checkbox'] allow_tags = getattr(attr, 'allow_tags', False) boolean = getattr(attr, 'boolean', False) - if boolean: + if boolean or not value: allow_tags = True - result_repr = display_for_value(value, boolean) + result_repr = display_for_value(value, empty_value_display, boolean) # Strip HTML tags in the resulting text, except if the # function has an "allow_tags" attribute set to True. if allow_tags: @@ -218,11 +220,11 @@ def items_for_result(cl, result, form): if isinstance(f.remote_field, models.ManyToOneRel): field_val = getattr(result, f.name) if field_val is None: - result_repr = EMPTY_CHANGELIST_VALUE + result_repr = empty_value_display else: result_repr = field_val else: - result_repr = display_for_field(value, f) + result_repr = display_for_field(value, f, empty_value_display) if isinstance(f, (models.DateField, models.TimeField, models.ForeignKey)): row_classes.append('nowrap') if force_text(result_repr) == '': diff --git a/django/contrib/admin/utils.py b/django/contrib/admin/utils.py index 1a4f76edfb..2d8c4b9d00 100644 --- a/django/contrib/admin/utils.py +++ b/django/contrib/admin/utils.py @@ -378,18 +378,17 @@ def help_text_for_field(name, model): return smart_text(help_text) -def display_for_field(value, field): +def display_for_field(value, field, empty_value_display): from django.contrib.admin.templatetags.admin_list import _boolean_icon - from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE if field.flatchoices: - return dict(field.flatchoices).get(value, EMPTY_CHANGELIST_VALUE) + return dict(field.flatchoices).get(value, empty_value_display) # NullBooleanField needs special-case null-handling, so it comes # before the general null test. elif isinstance(field, models.BooleanField) or isinstance(field, models.NullBooleanField): return _boolean_icon(value) elif value is None: - return EMPTY_CHANGELIST_VALUE + return empty_value_display elif isinstance(field, models.DateTimeField): return formats.localize(timezone.template_localtime(value)) elif isinstance(field, (models.DateField, models.TimeField)): @@ -404,14 +403,13 @@ def display_for_field(value, field): return smart_text(value) -def display_for_value(value, boolean=False): +def display_for_value(value, empty_value_display, boolean=False): from django.contrib.admin.templatetags.admin_list import _boolean_icon - from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE if boolean: return _boolean_icon(value) elif value is None: - return EMPTY_CHANGELIST_VALUE + return empty_value_display elif isinstance(value, datetime.datetime): return formats.localize(timezone.template_localtime(value)) elif isinstance(value, (datetime.date, datetime.time)): diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py index 0b38f7b027..5da031db9d 100644 --- a/django/contrib/admin/views/main.py +++ b/django/contrib/admin/views/main.py @@ -33,9 +33,6 @@ ERROR_FLAG = 'e' IGNORED_PARAMS = ( ALL_VAR, ORDER_VAR, ORDER_TYPE_VAR, SEARCH_VAR, IS_POPUP_VAR, TO_FIELD_VAR) -# Text to display within change-list table cells if the value is blank. -EMPTY_CHANGELIST_VALUE = '-' - class ChangeList(object): def __init__(self, request, model, list_display, list_display_links, diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 540ed42e12..091967c796 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -205,6 +205,33 @@ subclass:: to its documentation for some caveats when time zone support is enabled (:setting:`USE_TZ = True `). +.. attribute:: ModelAdmin.empty_value_display + + .. versionadded:: 1.9 + + This attribute overrides the default display value for record's fields that + are empty (``None``, empty string, etc.). The default value is ``-`` (a + dash). For example:: + + from django.contrib import admin + + class AuthorAdmin(admin.ModelAdmin): + empty_value_display = '-empty-' + + You can also override ``empty_value_display`` for all admin pages with + :attr:`AdminSite.empty_value_display`, or for specific fields like this:: + + from django.contrib import admin + + class AuthorAdmin(admin.ModelAdmin): + fields = ('name', 'title', 'view_birth_date') + + def view_birth_date(self, obj): + return obj.birth_date + + view_birth_date.short_name = 'birth_date' + view_birth_date.empty_value_display = '???' + .. attribute:: ModelAdmin.exclude This attribute, if given, should be a list of field names to exclude from @@ -583,6 +610,33 @@ subclass:: class PersonAdmin(admin.ModelAdmin): list_display = ('first_name', 'last_name', 'colored_name') + * If the value of a field is ``None``, an empty string, or an iterable + without elements, Django will display ``-`` (a dash). You can override + this with :attr:`AdminSite.empty_value_display`:: + + from django.contrib import admin + + admin.site.empty_value_display = '(None)' + + You can also use :attr:`AdminSite.empty_value_display`:: + + class PersonAdmin(admin.ModelAdmin): + empty_value_display = 'unknown' + + Or on a field level:: + + class PersonAdmin(admin.ModelAdmin): + list_display = ('name', 'birth_date_view') + + def birth_date_view(self, obj): + return obj.birth_date + + birth_date_view.empty_value_display = 'unknown' + + .. versionadded:: 1.9 + + The ability to customize ``empty_value_display`` was added. + * If the string given is a method of the model, ``ModelAdmin`` or a callable that returns True or False Django will display a pretty "on" or "off" icon if you give the method a ``boolean`` attribute @@ -604,7 +658,6 @@ subclass:: class PersonAdmin(admin.ModelAdmin): list_display = ('name', 'born_in_fifties') - * The ``__str__()`` (``__unicode__()`` on Python 2) method is just as valid in ``list_display`` as any other model method, so it's perfectly OK to do this:: @@ -2474,6 +2527,16 @@ Templates can override or extend base admin templates as described in Path to a custom template that will be used by the admin site app index view. +.. attribute:: AdminSite.empty_value_display + + .. versionadded:: 1.9 + + The string to use for displaying empty values in the admin site's change + list. Defaults to a dash. The value can also be overridden on a per + ``ModelAdmin`` basis and on a custom field within a ``ModelAdmin`` by + setting an ``empty_value_display`` attribute on the field. See + :attr:`ModelAdmin.empty_value_display` for examples. + .. attribute:: AdminSite.login_template Path to a custom template that will be used by the admin site login view. diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 4047785ef1..ecd2af6ddc 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -52,6 +52,12 @@ Minor features applications for the current user, has been added to the :meth:`AdminSite.each_context() ` method. +* :attr:`AdminSite.empty_value_display + ` and + :attr:`ModelAdmin.empty_value_display + ` were added to override + the display of empty values in admin change list. You can also customize the + value for each field. :mod:`django.contrib.auth` ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/admin_changelist/admin.py b/tests/admin_changelist/admin.py index 99deafcc62..c32082f78a 100644 --- a/tests/admin_changelist/admin.py +++ b/tests/admin_changelist/admin.py @@ -130,3 +130,12 @@ class DynamicSearchFieldsChildAdmin(admin.ModelAdmin): search_fields = super(DynamicSearchFieldsChildAdmin, self).get_search_fields(request) search_fields += ('age',) return search_fields + + +class EmptyValueChildAdmin(admin.ModelAdmin): + empty_value_display = '-empty-' + list_display = ('name', 'age_display', 'age') + + def age_display(self, obj): + return obj.age + age_display.empty_value_display = '†' diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py index ded3bfc154..86682581b9 100644 --- a/tests/admin_changelist/tests.py +++ b/tests/admin_changelist/tests.py @@ -20,9 +20,9 @@ from .admin import ( BandAdmin, ChildAdmin, ChordsBandAdmin, ConcertAdmin, CustomPaginationAdmin, CustomPaginator, DynamicListDisplayChildAdmin, DynamicListDisplayLinksChildAdmin, DynamicListFilterChildAdmin, - DynamicSearchFieldsChildAdmin, FilteredChildAdmin, GroupAdmin, - InvitationAdmin, NoListDisplayLinksParentAdmin, ParentAdmin, QuartetAdmin, - SwallowAdmin, site as custom_site, + DynamicSearchFieldsChildAdmin, EmptyValueChildAdmin, FilteredChildAdmin, + GroupAdmin, InvitationAdmin, NoListDisplayLinksParentAdmin, ParentAdmin, + QuartetAdmin, SwallowAdmin, site as custom_site, ) from .models import ( Band, Child, ChordsBand, ChordsMusician, Concert, CustomIdUser, Event, @@ -109,14 +109,67 @@ class ChangeListTests(TestCase): list_display = m.get_list_display(request) list_display_links = m.get_list_display_links(request, list_display) cl = ChangeList(request, Child, list_display, list_display_links, - m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) cl.formset = None template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) table_output = template.render(context) link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) - row_html = 'name-' % link + row_html = ( + 'name' + '-' % link + ) + self.assertNotEqual(table_output.find(row_html), -1, + 'Failed to find expected row element: %s' % table_output) + + def test_result_list_set_empty_value_display_on_admin_site(self): + """ + Test that empty value display can be set on AdminSite + """ + new_child = Child.objects.create(name='name', parent=None) + request = self.factory.get('/child/') + # Set a new empty display value on AdminSite. + admin.site.empty_value_display = '???' + m = ChildAdmin(Child, admin.site) + list_display = m.get_list_display(request) + list_display_links = m.get_list_display_links(request, list_display) + cl = ChangeList(request, Child, list_display, list_display_links, + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) + cl.formset = None + template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') + context = Context({'cl': cl}) + table_output = template.render(context) + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = ( + 'name' + '???' % link + ) + self.assertNotEqual(table_output.find(row_html), -1, + 'Failed to find expected row element: %s' % table_output) + + def test_result_list_set_empty_value_display_in_model_admin(self): + """ + Test that empty value display can be set in ModelAdmin or individual fields. + """ + new_child = Child.objects.create(name='name', parent=None) + request = self.factory.get('/child/') + m = EmptyValueChildAdmin(Child, admin.site) + list_display = m.get_list_display(request) + list_display_links = m.get_list_display_links(request, list_display) + cl = ChangeList(request, Child, list_display, list_display_links, + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) + cl.formset = None + template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') + context = Context({'cl': cl}) + table_output = template.render(context) + link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) + row_html = ( + 'name' + '†-empty-' % link + ) self.assertNotEqual(table_output.find(row_html), -1, 'Failed to find expected row element: %s' % table_output) @@ -132,14 +185,17 @@ class ChangeListTests(TestCase): list_display = m.get_list_display(request) list_display_links = m.get_list_display_links(request, list_display) cl = ChangeList(request, Child, list_display, list_display_links, - m.list_filter, m.date_hierarchy, m.search_fields, - m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m) cl.formset = None template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') context = Context({'cl': cl}) table_output = template.render(context) link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) - row_html = 'nameParent object' % link + row_html = ( + 'name' + 'Parent object' % link + ) self.assertNotEqual(table_output.find(row_html), -1, 'Failed to find expected row element: %s' % table_output) diff --git a/tests/admin_utils/tests.py b/tests/admin_utils/tests.py index 499732cbbb..7296af7eb2 100644 --- a/tests/admin_utils/tests.py +++ b/tests/admin_utils/tests.py @@ -11,7 +11,6 @@ from django.contrib.admin.utils import ( NestedObjects, display_for_field, flatten, flatten_fieldsets, label_for_field, lookup_field, ) -from django.contrib.admin.views.main import EMPTY_CHANGELIST_VALUE from django.db import DEFAULT_DB_ALIAS, models from django.test import TestCase, override_settings from django.utils import six @@ -96,6 +95,9 @@ class NestedObjectsTests(TestCase): class UtilsTests(TestCase): + + empty_value = '-empty-' + def test_values_from_lookup_field(self): """ Regression test for #12654: lookup_field @@ -136,7 +138,7 @@ class UtilsTests(TestCase): field, attr, resolved_value = lookup_field(name, article, mock_admin) if field is not None: - resolved_value = display_for_field(resolved_value, field) + resolved_value = display_for_field(resolved_value, field, self.empty_value) self.assertEqual(value, resolved_value) @@ -145,53 +147,53 @@ class UtilsTests(TestCase): Regression test for #12550: display_for_field should handle None value. """ - display_value = display_for_field(None, models.CharField()) - self.assertEqual(display_value, EMPTY_CHANGELIST_VALUE) + display_value = display_for_field(None, models.CharField(), self.empty_value) + self.assertEqual(display_value, self.empty_value) display_value = display_for_field(None, models.CharField( choices=( (None, "test_none"), ) - )) + ), self.empty_value) self.assertEqual(display_value, "test_none") - display_value = display_for_field(None, models.DateField()) - self.assertEqual(display_value, EMPTY_CHANGELIST_VALUE) + display_value = display_for_field(None, models.DateField(), self.empty_value) + self.assertEqual(display_value, self.empty_value) - display_value = display_for_field(None, models.TimeField()) - self.assertEqual(display_value, EMPTY_CHANGELIST_VALUE) + display_value = display_for_field(None, models.TimeField(), self.empty_value) + self.assertEqual(display_value, self.empty_value) # Regression test for #13071: NullBooleanField has special # handling. - display_value = display_for_field(None, models.NullBooleanField()) + display_value = display_for_field(None, models.NullBooleanField(), self.empty_value) expected = 'None' % settings.STATIC_URL self.assertHTMLEqual(display_value, expected) - display_value = display_for_field(None, models.DecimalField()) - self.assertEqual(display_value, EMPTY_CHANGELIST_VALUE) + display_value = display_for_field(None, models.DecimalField(), self.empty_value) + self.assertEqual(display_value, self.empty_value) - display_value = display_for_field(None, models.FloatField()) - self.assertEqual(display_value, EMPTY_CHANGELIST_VALUE) + display_value = display_for_field(None, models.FloatField(), self.empty_value) + self.assertEqual(display_value, self.empty_value) def test_number_formats_display_for_field(self): - display_value = display_for_field(12345.6789, models.FloatField()) + display_value = display_for_field(12345.6789, models.FloatField(), self.empty_value) self.assertEqual(display_value, '12345.6789') - display_value = display_for_field(Decimal('12345.6789'), models.DecimalField()) + display_value = display_for_field(Decimal('12345.6789'), models.DecimalField(), self.empty_value) self.assertEqual(display_value, '12345.6789') - display_value = display_for_field(12345, models.IntegerField()) + display_value = display_for_field(12345, models.IntegerField(), self.empty_value) self.assertEqual(display_value, '12345') @override_settings(USE_L10N=True, USE_THOUSAND_SEPARATOR=True) def test_number_formats_with_thousand_seperator_display_for_field(self): - display_value = display_for_field(12345.6789, models.FloatField()) + display_value = display_for_field(12345.6789, models.FloatField(), self.empty_value) self.assertEqual(display_value, '12,345.6789') - display_value = display_for_field(Decimal('12345.6789'), models.DecimalField()) + display_value = display_for_field(Decimal('12345.6789'), models.DecimalField(), self.empty_value) self.assertEqual(display_value, '12,345.6789') - display_value = display_for_field(12345, models.IntegerField()) + display_value = display_for_field(12345, models.IntegerField(), self.empty_value) self.assertEqual(display_value, '12,345') def test_label_for_field(self):