From 00035672a460b6eb5442d2837bc783f8af28c6f3 Mon Sep 17 00:00:00 2001 From: zeyneloz Date: Thu, 15 Aug 2019 06:54:41 +0200 Subject: [PATCH] Fixed #30449 -- Fixed RelatedFieldListFilter/RelatedOnlyFieldListFilter to respect model's Meta.ordering. Regression in 6d4e5feb79f7eabe8a0c7c4b87f25b1a7f87ca0b. Co-Authored-By: Mariusz Felisiak --- django/db/models/fields/__init__.py | 6 ++- django/db/models/fields/reverse_related.py | 5 +- docs/releases/2.2.5.txt | 5 ++ tests/admin_filters/tests.py | 57 ++++++++++++++++++++++ tests/model_fields/tests.py | 20 +++++++- 5 files changed, 88 insertions(+), 5 deletions(-) diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 194514c900..1aad845470 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -825,9 +825,11 @@ class Field(RegisterLookupMixin): if hasattr(self.remote_field, 'get_related_field') else 'pk' ) + qs = rel_model._default_manager.complex_filter(limit_choices_to) + if ordering: + qs = qs.order_by(*ordering) return (blank_choice if include_blank else []) + [ - (choice_func(x), str(x)) - for x in rel_model._default_manager.complex_filter(limit_choices_to).order_by(*ordering) + (choice_func(x), str(x)) for x in qs ] def value_to_string(self, obj): diff --git a/django/db/models/fields/reverse_related.py b/django/db/models/fields/reverse_related.py index eb6b934259..700410a086 100644 --- a/django/db/models/fields/reverse_related.py +++ b/django/db/models/fields/reverse_related.py @@ -122,8 +122,11 @@ class ForeignObjectRel(FieldCacheMixin): Analog of django.db.models.fields.Field.get_choices(), provided initially for utilization by RelatedFieldListFilter. """ + qs = self.related_model._default_manager.all() + if ordering: + qs = qs.order_by(*ordering) return (blank_choice if include_blank else []) + [ - (x.pk, str(x)) for x in self.related_model._default_manager.order_by(*ordering) + (x.pk, str(x)) for x in qs ] def is_hidden(self): diff --git a/docs/releases/2.2.5.txt b/docs/releases/2.2.5.txt index 0b9310a344..9f143c8d96 100644 --- a/docs/releases/2.2.5.txt +++ b/docs/releases/2.2.5.txt @@ -17,3 +17,8 @@ Bugfixes :class:`~django.contrib.postgres.fields.JSONField` and :class:`~django.contrib.postgres.fields.HStoreField` when using on expressions with params (:ticket:`30672`). + +* Fixed a regression in Django 2.2 where + :attr:`ModelAdmin.list_filter ` + choices to foreign objects don't respect a model's ``Meta.ordering`` + (:ticket:`30449`). diff --git a/tests/admin_filters/tests.py b/tests/admin_filters/tests.py index 4ff7d012e5..75563bbaaf 100644 --- a/tests/admin_filters/tests.py +++ b/tests/admin_filters/tests.py @@ -591,6 +591,22 @@ class ListFiltersTests(TestCase): expected = [(self.john.pk, 'John Blue'), (self.jack.pk, 'Jack Red')] self.assertEqual(filterspec.lookup_choices, expected) + def test_relatedfieldlistfilter_foreignkey_default_ordering(self): + """RelatedFieldListFilter ordering respects Model.ordering.""" + class BookAdmin(ModelAdmin): + list_filter = ('employee',) + + self.addCleanup(setattr, Employee._meta, 'ordering', Employee._meta.ordering) + Employee._meta.ordering = ('name',) + modeladmin = BookAdmin(Book, site) + + request = self.request_factory.get('/') + request.user = self.alfred + changelist = modeladmin.get_changelist_instance(request) + filterspec = changelist.get_filters(request)[0][0] + expected = [(self.jack.pk, 'Jack Red'), (self.john.pk, 'John Blue')] + self.assertEqual(filterspec.lookup_choices, expected) + def test_relatedfieldlistfilter_manytomany(self): modeladmin = BookAdmin(Book, site) @@ -696,6 +712,23 @@ class ListFiltersTests(TestCase): filterspec = changelist.get_filters(request)[0] self.assertEqual(len(filterspec), 0) + def test_relatedfieldlistfilter_reverse_relationships_default_ordering(self): + self.addCleanup(setattr, Book._meta, 'ordering', Book._meta.ordering) + Book._meta.ordering = ('title',) + modeladmin = CustomUserAdmin(User, site) + + request = self.request_factory.get('/') + request.user = self.alfred + changelist = modeladmin.get_changelist_instance(request) + filterspec = changelist.get_filters(request)[0][0] + expected = [ + (self.bio_book.pk, 'Django: a biography'), + (self.djangonaut_book.pk, 'Djangonaut: an art of living'), + (self.guitar_book.pk, 'Guitar for dummies'), + (self.django_book.pk, 'The Django Book') + ] + self.assertEqual(filterspec.lookup_choices, expected) + def test_relatedonlyfieldlistfilter_foreignkey(self): modeladmin = BookAdminRelatedOnlyFilter(Book, site) @@ -708,6 +741,30 @@ class ListFiltersTests(TestCase): expected = [(self.alfred.pk, 'alfred'), (self.bob.pk, 'bob')] self.assertEqual(sorted(filterspec.lookup_choices), sorted(expected)) + def test_relatedonlyfieldlistfilter_foreignkey_default_ordering(self): + """RelatedOnlyFieldListFilter ordering respects Meta.ordering.""" + class BookAdmin(ModelAdmin): + list_filter = ( + ('employee', RelatedOnlyFieldListFilter), + ) + + albert = Employee.objects.create(name='Albert Green', department=self.dev) + self.djangonaut_book.employee = albert + self.djangonaut_book.save() + self.bio_book.employee = self.jack + self.bio_book.save() + + self.addCleanup(setattr, Employee._meta, 'ordering', Employee._meta.ordering) + Employee._meta.ordering = ('name',) + modeladmin = BookAdmin(Book, site) + + request = self.request_factory.get('/') + request.user = self.alfred + changelist = modeladmin.get_changelist_instance(request) + filterspec = changelist.get_filters(request)[0][0] + expected = [(albert.pk, 'Albert Green'), (self.jack.pk, 'Jack Red')] + self.assertEqual(filterspec.lookup_choices, expected) + def test_relatedonlyfieldlistfilter_underscorelookup_foreignkey(self): Department.objects.create(code='TEST', description='Testing') self.djangonaut_book.employee = self.john diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index bb82c7b93d..abc5273d90 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -222,9 +222,9 @@ class GetChoicesOrderingTests(TestCase): @classmethod def setUpTestData(cls): - cls.foo1 = Foo.objects.create(a='a', d='12.34') + cls.foo1 = Foo.objects.create(a='a', d='12.35') cls.foo2 = Foo.objects.create(a='b', d='12.34') - cls.bar1 = Bar.objects.create(a=cls.foo1, b='a') + cls.bar1 = Bar.objects.create(a=cls.foo1, b='b') cls.bar2 = Bar.objects.create(a=cls.foo2, b='a') cls.field = Bar._meta.get_field('a') @@ -241,6 +241,14 @@ class GetChoicesOrderingTests(TestCase): [self.foo2, self.foo1] ) + def test_get_choices_default_ordering(self): + self.addCleanup(setattr, Foo._meta, 'ordering', Foo._meta.ordering) + Foo._meta.ordering = ('d',) + self.assertChoicesEqual( + self.field.get_choices(include_blank=False), + [self.foo2, self.foo1] + ) + def test_get_choices_reverse_related_field(self): self.assertChoicesEqual( self.field.remote_field.get_choices(include_blank=False, ordering=('a',)), @@ -250,3 +258,11 @@ class GetChoicesOrderingTests(TestCase): self.field.remote_field.get_choices(include_blank=False, ordering=('-a',)), [self.bar2, self.bar1] ) + + def test_get_choices_reverse_related_field_default_ordering(self): + self.addCleanup(setattr, Bar._meta, 'ordering', Bar._meta.ordering) + Bar._meta.ordering = ('b',) + self.assertChoicesEqual( + self.field.remote_field.get_choices(include_blank=False), + [self.bar2, self.bar1] + )