Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #30449 -- Fixed RelatedFieldListFilter/RelatedOnlyFieldListFilter to respect model's Meta.ordering. #11400

Merged
merged 2 commits into from Aug 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 11 additions & 4 deletions django/contrib/admin/filters.py
Expand Up @@ -193,11 +193,17 @@ def has_output(self):
def expected_parameters(self):
return [self.lookup_kwarg, self.lookup_kwarg_isnull]

def field_choices(self, field, request, model_admin):
ordering = ()
def field_admin_ordering(self, field, request, model_admin):
"""
Return the model admin's ordering for related field, if provided.
"""
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 related_admin.get_ordering(request)
return ()

def field_choices(self, field, request, model_admin):
ordering = self.field_admin_ordering(field, request, model_admin)
return field.get_choices(include_blank=False, ordering=ordering)

def choices(self, changelist):
Expand Down Expand Up @@ -419,4 +425,5 @@ def choices(self, changelist):
class RelatedOnlyFieldListFilter(RelatedFieldListFilter):
def field_choices(self, field, request, model_admin):
pk_qs = model_admin.get_queryset(request).distinct().values_list('%s__pk' % self.field_path, flat=True)
return field.get_choices(include_blank=False, limit_choices_to={'pk__in': pk_qs})
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)
6 changes: 4 additions & 2 deletions django/db/models/fields/__init__.py
Expand Up @@ -825,9 +825,11 @@ def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, limit_
if hasattr(self.remote_field, 'get_related_field')
else 'pk'
)
qs = rel_model._default_manager.complex_filter(limit_choices_to)
if ordering:
felixxm marked this conversation as resolved.
Show resolved Hide resolved
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):
Expand Down
5 changes: 4 additions & 1 deletion django/db/models/fields/reverse_related.py
Expand Up @@ -122,8 +122,11 @@ def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, orderi
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
felixxm marked this conversation as resolved.
Show resolved Hide resolved
]

def is_hidden(self):
Expand Down
5 changes: 5 additions & 0 deletions docs/releases/2.2.5.txt
Expand Up @@ -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 <django.contrib.admin.ModelAdmin.list_filter>`
choices to foreign objects don't respect a model's ``Meta.ordering``
(:ticket:`30449`).
84 changes: 84 additions & 0 deletions tests/admin_filters/tests.py
Expand Up @@ -591,6 +591,22 @@ class BookAdmin(ModelAdmin):
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)

Expand Down Expand Up @@ -696,6 +712,23 @@ def test_relatedfieldlistfilter_reverse_relationships(self):
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)

Expand All @@ -708,6 +741,57 @@ def test_relatedonlyfieldlistfilter_foreignkey(self):
expected = [(self.alfred.pk, 'alfred'), (self.bob.pk, 'bob')]
self.assertEqual(sorted(filterspec.lookup_choices), sorted(expected))

def test_relatedonlyfieldlistfilter_foreignkey_ordering(self):
"""RelatedOnlyFieldListFilter ordering respects ModelAdmin.ordering."""
class EmployeeAdminWithOrdering(ModelAdmin):
ordering = ('name',)

felixxm marked this conversation as resolved.
Show resolved Hide resolved
class BookAdmin(ModelAdmin):
list_filter = (
felixxm marked this conversation as resolved.
Show resolved Hide resolved
('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()

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 = [(albert.pk, 'Albert Green'), (self.jack.pk, 'Jack Red')]
self.assertEqual(filterspec.lookup_choices, expected)

def test_relatedonlyfieldlistfilter_foreignkey_default_ordering(self):
"""RelatedOnlyFieldListFilter ordering respects Meta.ordering."""
class BookAdmin(ModelAdmin):
list_filter = (
felixxm marked this conversation as resolved.
Show resolved Hide resolved
('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
Expand Down
20 changes: 18 additions & 2 deletions tests/model_fields/tests.py
Expand Up @@ -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')

Expand All @@ -241,6 +241,14 @@ def test_get_choices(self):
[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',)),
Expand All @@ -250,3 +258,11 @@ def test_get_choices_reverse_related_field(self):
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]
)