Skip to content

Commit

Permalink
Fixed #29835 -- Made RelatedFieldListFilter respect ModelAdmin.ordering.
Browse files Browse the repository at this point in the history
  • Loading branch information
hramezani authored and timgraham committed Nov 14, 2018
1 parent 35a08b8 commit 6d4e5fe
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 6 deletions.
6 changes: 5 additions & 1 deletion django/contrib/admin/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,11 @@ def expected_parameters(self):
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)

This comment has been minimized.

Copy link
@dacotagh

dacotagh Feb 9, 2021

The same code should be used in RelatedOnlyFieldListFilter also.
It was however refactored and fixed in django 3.x versions.
Shouldn't it be fixed in 2.2.x version?

This comment has been minimized.

Copy link
@timgraham

timgraham Feb 9, 2021

Member

Per our supported versions policy, 2.2.x is only receiving fixes for security and data loss issues.


def choices(self, changelist):
yield {
Expand Down
4 changes: 2 additions & 2 deletions django/db/models/fields/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,7 @@ def _get_default(self):
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 <select> choices for this field.
Expand All @@ -828,7 +828,7 @@ def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, limit_
)
return (blank_choice if include_blank else []) + [
(choice_func(x), str(x))
for x in rel_model._default_manager.complex_filter(limit_choices_to)
for x in rel_model._default_manager.complex_filter(limit_choices_to).order_by(*ordering)
]

def value_to_string(self, obj):
Expand Down
4 changes: 2 additions & 2 deletions django/db/models/fields/reverse_related.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def __repr__(self):
self.related_model._meta.model_name,
)

def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH):
def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH, ordering=()):
"""
Return choices with a default blank choices included, for use
as <select> choices for this field.
Expand All @@ -123,7 +123,7 @@ def get_choices(self, include_blank=True, blank_choice=BLANK_CHOICE_DASH):
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):
Expand Down
37 changes: 37 additions & 0 deletions tests/admin_filters/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,43 @@ def test_relatedfieldlistfilter_foreignkey(self):
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)

Expand Down
36 changes: 35 additions & 1 deletion tests/model_fields/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from django.utils.functional import lazy

from .models import (
Foo, RenamedField, VerboseNameField, Whiz, WhizIter, WhizIterEmpty,
Bar, Foo, RenamedField, VerboseNameField, Whiz, WhizIter, WhizIterEmpty,
)


Expand Down Expand Up @@ -157,3 +157,37 @@ def test_lazy_strings_not_evaluated(self):
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]
)

0 comments on commit 6d4e5fe

Please sign in to comment.