From 6d4e5feb79f7eabe8a0c7c4b87f25b1a7f87ca0b Mon Sep 17 00:00:00 2001 From: Hasan Ramezani Date: Fri, 19 Oct 2018 15:41:29 +0200 Subject: [PATCH] Fixed #29835 -- Made RelatedFieldListFilter respect ModelAdmin.ordering. --- django/contrib/admin/filters.py | 6 +++- django/db/models/fields/__init__.py | 4 +-- django/db/models/fields/reverse_related.py | 4 +-- tests/admin_filters/tests.py | 37 ++++++++++++++++++++++ tests/model_fields/tests.py | 36 ++++++++++++++++++++- 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index 7cf75bcd0d..d65e01d5e2 100644 --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -194,7 +194,11 @@ class RelatedFieldListFilter(FieldListFilter): return [self.lookup_kwarg, self.lookup_kwarg_isnull] def field_choices(self, field, request, model_admin): - return field.get_choices(include_blank=False) + ordering = () + related_admin = model_admin.admin_site._registry.get(field.remote_field.model) + if related_admin is not None: + ordering = related_admin.get_ordering(request) + return field.get_choices(include_blank=False, ordering=ordering) def choices(self, changelist): yield { diff --git a/django/db/models/fields/__init__.py b/django/db/models/fields/__init__.py index 04c7f002bc..fc4966c8b7 100644 --- a/django/db/models/fields/__init__.py +++ b/django/db/models/fields/__init__.py @@ -807,7 +807,7 @@ class Field(RegisterLookupMixin): return return_None return str # return empty string - def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, limit_choices_to=None): + def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, limit_choices_to=None, ordering=()): """ Return choices with a default blank choices included, for use as choices for this field. @@ -123,7 +123,7 @@ class ForeignObjectRel(FieldCacheMixin): initially for utilization by RelatedFieldListFilter. """ return (blank_choice if include_blank else []) + [ - (x.pk, str(x)) for x in self.related_model._default_manager.all() + (x.pk, str(x)) for x in self.related_model._default_manager.order_by(*ordering) ] def is_hidden(self): diff --git a/tests/admin_filters/tests.py b/tests/admin_filters/tests.py index 2e16909dc3..f901d32532 100644 --- a/tests/admin_filters/tests.py +++ b/tests/admin_filters/tests.py @@ -548,6 +548,43 @@ class ListFiltersTests(TestCase): self.assertIs(choice['selected'], True) self.assertEqual(choice['query_string'], '?author__id__exact=%d' % self.alfred.pk) + def test_relatedfieldlistfilter_foreignkey_ordering(self): + """RelatedFieldListFilter ordering respects ModelAdmin.ordering.""" + class EmployeeAdminWithOrdering(ModelAdmin): + ordering = ('name',) + + class BookAdmin(ModelAdmin): + list_filter = ('employee',) + + site.register(Employee, EmployeeAdminWithOrdering) + self.addCleanup(lambda: site.unregister(Employee)) + 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_foreignkey_ordering_reverse(self): + class EmployeeAdminWithOrdering(ModelAdmin): + ordering = ('-name',) + + class BookAdmin(ModelAdmin): + list_filter = ('employee',) + + site.register(Employee, EmployeeAdminWithOrdering) + self.addCleanup(lambda: site.unregister(Employee)) + 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.john.pk, 'John Blue'), (self.jack.pk, 'Jack Red')] + self.assertEqual(filterspec.lookup_choices, expected) + def test_relatedfieldlistfilter_manytomany(self): modeladmin = BookAdmin(Book, site) diff --git a/tests/model_fields/tests.py b/tests/model_fields/tests.py index 45f61a0034..d638acb488 100644 --- a/tests/model_fields/tests.py +++ b/tests/model_fields/tests.py @@ -6,7 +6,7 @@ from django.test import SimpleTestCase, TestCase from django.utils.functional import lazy from .models import ( - Foo, RenamedField, VerboseNameField, Whiz, WhizIter, WhizIterEmpty, + Bar, Foo, RenamedField, VerboseNameField, Whiz, WhizIter, WhizIterEmpty, ) @@ -157,3 +157,37 @@ class GetChoicesTests(SimpleTestCase): lazy_func = lazy(lambda x: 0 / 0, int) # raises ZeroDivisionError if evaluated. f = models.CharField(choices=[(lazy_func('group'), (('a', 'A'), ('b', 'B')))]) self.assertEqual(f.get_choices(include_blank=True)[0], ('', '---------')) + + +class GetChoicesOrderingTests(TestCase): + + @classmethod + def setUpTestData(cls): + cls.foo1 = Foo.objects.create(a='a', d='12.34') + cls.foo2 = Foo.objects.create(a='b', d='12.34') + cls.bar1 = Bar.objects.create(a=cls.foo1, b='a') + cls.bar2 = Bar.objects.create(a=cls.foo2, b='a') + cls.field = Bar._meta.get_field('a') + + def assertChoicesEqual(self, choices, objs): + self.assertEqual(choices, [(obj.pk, str(obj)) for obj in objs]) + + def test_get_choices(self): + self.assertChoicesEqual( + self.field.get_choices(include_blank=False, ordering=('a',)), + [self.foo1, self.foo2] + ) + self.assertChoicesEqual( + self.field.get_choices(include_blank=False, ordering=('-a',)), + [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',)), + [self.bar1, self.bar2] + ) + self.assertChoicesEqual( + self.field.remote_field.get_choices(include_blank=False, ordering=('-a',)), + [self.bar2, self.bar1] + )