From 372eaa395f167c038e546c6be554c96014d40109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Jaramillo=20Marti=CC=81nez?= Date: Fri, 5 Jan 2018 23:53:51 +0100 Subject: [PATCH] Fixed #28991 -- Added EmptyFieldListFilter class in admin.filters. Thanks Simon Charette and Carlton Gibson for reviews. Co-Authored-By: Jonas Haag Co-Authored-By: Christophe Baldy --- django/contrib/admin/__init__.py | 7 ++- django/contrib/admin/filters.py | 45 +++++++++++++ docs/ref/contrib/admin/index.txt | 13 ++++ docs/releases/3.1.txt | 4 +- tests/admin_filters/tests.py | 104 ++++++++++++++++++++++++++++++- 5 files changed, 167 insertions(+), 6 deletions(-) diff --git a/django/contrib/admin/__init__.py b/django/contrib/admin/__init__.py index f67995c63a..f9bf6543f9 100644 --- a/django/contrib/admin/__init__.py +++ b/django/contrib/admin/__init__.py @@ -1,8 +1,8 @@ from django.contrib.admin.decorators import register from django.contrib.admin.filters import ( AllValuesFieldListFilter, BooleanFieldListFilter, ChoicesFieldListFilter, - DateFieldListFilter, FieldListFilter, ListFilter, RelatedFieldListFilter, - RelatedOnlyFieldListFilter, SimpleListFilter, + DateFieldListFilter, EmptyFieldListFilter, FieldListFilter, ListFilter, + RelatedFieldListFilter, RelatedOnlyFieldListFilter, SimpleListFilter, ) from django.contrib.admin.options import ( HORIZONTAL, VERTICAL, ModelAdmin, StackedInline, TabularInline, @@ -15,7 +15,8 @@ __all__ = [ "TabularInline", "AdminSite", "site", "ListFilter", "SimpleListFilter", "FieldListFilter", "BooleanFieldListFilter", "RelatedFieldListFilter", "ChoicesFieldListFilter", "DateFieldListFilter", - "AllValuesFieldListFilter", "RelatedOnlyFieldListFilter", "autodiscover", + "AllValuesFieldListFilter", "EmptyFieldListFilter", + "RelatedOnlyFieldListFilter", "autodiscover", ] diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index a9e5563c6c..3e02cd89d7 100644 --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -427,3 +427,48 @@ class RelatedOnlyFieldListFilter(RelatedFieldListFilter): pk_qs = model_admin.get_queryset(request).distinct().values_list('%s__pk' % self.field_path, flat=True) ordering = self.field_admin_ordering(field, request, model_admin) return field.get_choices(include_blank=False, limit_choices_to={'pk__in': pk_qs}, ordering=ordering) + + +class EmptyFieldListFilter(FieldListFilter): + def __init__(self, field, request, params, model, model_admin, field_path): + if not field.empty_strings_allowed and not field.null: + raise ImproperlyConfigured( + "The list filter '%s' cannot be used with field '%s' which " + "doesn't allow empty strings and nulls." % ( + self.__class__.__name__, + field.name, + ) + ) + self.lookup_kwarg = '%s__isempty' % field_path + self.lookup_val = params.get(self.lookup_kwarg) + super().__init__(field, request, params, model, model_admin, field_path) + + def queryset(self, request, queryset): + if self.lookup_kwarg not in self.used_parameters: + return queryset + if self.lookup_val not in ('0', '1'): + raise IncorrectLookupParameters + + lookup_condition = models.Q() + if self.field.empty_strings_allowed: + lookup_condition |= models.Q(**{self.field_path: ''}) + if self.field.null: + lookup_condition |= models.Q(**{'%s__isnull' % self.field_path: True}) + if self.lookup_val == '1': + return queryset.filter(lookup_condition) + return queryset.exclude(lookup_condition) + + def expected_parameters(self): + return [self.lookup_kwarg] + + def choices(self, changelist): + for lookup, title in ( + (None, _('All')), + ('1', _('Empty')), + ('0', _('Not empty')), + ): + yield { + 'selected': self.lookup_val == lookup, + 'query_string': changelist.get_query_string({self.lookup_kwarg: lookup}), + 'display': title, + } diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 669fc185a4..6bffa6850c 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -971,11 +971,24 @@ subclass:: limit the ``list_filter`` choices to the users who have written a book instead of listing all users. + You can filter empty values using ``EmptyFieldListFilter``, which can + filter on both empty strings and nulls, depending on what the field + allows to store:: + + class BookAdmin(admin.ModelAdmin): + list_filter = ( + ('title', admin.EmptyFieldListFilter), + ) + .. note:: The ``FieldListFilter`` API is considered internal and might be changed. + .. versionadded:: 3.1 + + The ``EmptyFieldListFilter`` class was added. + List filter's typically appear only if the filter has more than one choice. A filter's ``has_output()`` method controls whether or not it appears. diff --git a/docs/releases/3.1.txt b/docs/releases/3.1.txt index 51a0c69ac6..6b7e73c431 100644 --- a/docs/releases/3.1.txt +++ b/docs/releases/3.1.txt @@ -33,7 +33,9 @@ Minor features :mod:`django.contrib.admin` ~~~~~~~~~~~~~~~~~~~~~~~~~~~ -* ... +* The new ``django.contrib.admin.EmptyFieldListFilter`` for + :attr:`.ModelAdmin.list_filter` allows filtering on empty values (empty + strings and nulls) in the admin changelist view. :mod:`django.contrib.admindocs` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/admin_filters/tests.py b/tests/admin_filters/tests.py index 6db2147e51..75769085e0 100644 --- a/tests/admin_filters/tests.py +++ b/tests/admin_filters/tests.py @@ -3,8 +3,8 @@ import sys import unittest from django.contrib.admin import ( - AllValuesFieldListFilter, BooleanFieldListFilter, ModelAdmin, - RelatedOnlyFieldListFilter, SimpleListFilter, site, + AllValuesFieldListFilter, BooleanFieldListFilter, EmptyFieldListFilter, + ModelAdmin, RelatedOnlyFieldListFilter, SimpleListFilter, site, ) from django.contrib.admin.options import IncorrectLookupParameters from django.contrib.auth.admin import UserAdmin @@ -248,6 +248,17 @@ class BookmarkAdminGenericRelation(ModelAdmin): list_filter = ['tags__tag'] +class BookAdminWithEmptyFieldListFilter(ModelAdmin): + list_filter = [ + ('author', EmptyFieldListFilter), + ('title', EmptyFieldListFilter), + ] + + +class DepartmentAdminWithEmptyFieldListFilter(ModelAdmin): + list_filter = [('description', EmptyFieldListFilter)] + + class ListFiltersTests(TestCase): request_factory = RequestFactory() @@ -1374,3 +1385,92 @@ class ListFiltersTests(TestCase): changelist = modeladmin.get_changelist_instance(request) changelist.get_results(request) self.assertEqual(changelist.full_result_count, 4) + + def test_emptylistfieldfilter(self): + empty_description = Department.objects.create(code='EMPT', description='') + none_description = Department.objects.create(code='NONE', description=None) + empty_title = Book.objects.create(title='', author=self.alfred) + + department_admin = DepartmentAdminWithEmptyFieldListFilter(Department, site) + book_admin = BookAdminWithEmptyFieldListFilter(Book, site) + + tests = [ + # Allows nulls and empty strings. + ( + department_admin, + {'description__isempty': '1'}, + [empty_description, none_description], + ), + ( + department_admin, + {'description__isempty': '0'}, + [self.dev, self.design], + ), + # Allows nulls. + (book_admin, {'author__isempty': '1'}, [self.guitar_book]), + ( + book_admin, + {'author__isempty': '0'}, + [self.django_book, self.bio_book, self.djangonaut_book, empty_title], + ), + # Allows empty strings. + (book_admin, {'title__isempty': '1'}, [empty_title]), + ( + book_admin, + {'title__isempty': '0'}, + [self.django_book, self.bio_book, self.djangonaut_book, self.guitar_book], + ), + ] + for modeladmin, query_string, expected_result in tests: + with self.subTest( + modeladmin=modeladmin.__class__.__name__, + query_string=query_string, + ): + request = self.request_factory.get('/', query_string) + request.user = self.alfred + changelist = modeladmin.get_changelist_instance(request) + queryset = changelist.get_queryset(request) + self.assertCountEqual(queryset, expected_result) + + def test_emptylistfieldfilter_choices(self): + modeladmin = BookAdminWithEmptyFieldListFilter(Book, site) + request = self.request_factory.get('/') + request.user = self.alfred + changelist = modeladmin.get_changelist_instance(request) + filterspec = changelist.get_filters(request)[0][0] + self.assertEqual(filterspec.title, 'Verbose Author') + choices = list(filterspec.choices(changelist)) + self.assertEqual(len(choices), 3) + + self.assertEqual(choices[0]['display'], 'All') + self.assertIs(choices[0]['selected'], True) + self.assertEqual(choices[0]['query_string'], '?') + + self.assertEqual(choices[1]['display'], 'Empty') + self.assertIs(choices[1]['selected'], False) + self.assertEqual(choices[1]['query_string'], '?author__isempty=1') + + self.assertEqual(choices[2]['display'], 'Not empty') + self.assertIs(choices[2]['selected'], False) + self.assertEqual(choices[2]['query_string'], '?author__isempty=0') + + def test_emptylistfieldfilter_non_empty_field(self): + class EmployeeAdminWithEmptyFieldListFilter(ModelAdmin): + list_filter = [('department', EmptyFieldListFilter)] + + modeladmin = EmployeeAdminWithEmptyFieldListFilter(Employee, site) + request = self.request_factory.get('/') + request.user = self.alfred + msg = ( + "The list filter 'EmptyFieldListFilter' cannot be used with field " + "'department' which doesn't allow empty strings and nulls." + ) + with self.assertRaisesMessage(ImproperlyConfigured, msg): + modeladmin.get_changelist_instance(request) + + def test_emptylistfieldfilter_invalid_lookup_parameters(self): + modeladmin = BookAdminWithEmptyFieldListFilter(Book, site) + request = self.request_factory.get('/', {'author__isempty': 42}) + request.user = self.alfred + with self.assertRaises(IncorrectLookupParameters): + modeladmin.get_changelist_instance(request)