From 88e17156393be86df64d44554de8ab77a6d3ed7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastia=CC=81n=20Magri=CC=81?= Date: Sun, 18 Nov 2012 20:51:05 -0430 Subject: [PATCH] Fixed #19318 -- Ensured that the admin's SimpleListFilter options can be displayed as selected even if the lookup's first element is not a string. --- django/contrib/admin/filters.py | 4 +- tests/regressiontests/admin_filters/tests.py | 58 +++++++++++++++++--- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py index 0a34f807b3..ff66d3e3f3 100644 --- a/django/contrib/admin/filters.py +++ b/django/contrib/admin/filters.py @@ -9,7 +9,7 @@ import datetime from django.db import models from django.core.exceptions import ImproperlyConfigured, ValidationError -from django.utils.encoding import smart_text +from django.utils.encoding import smart_text, force_text from django.utils.translation import ugettext_lazy as _ from django.utils import timezone from django.contrib.admin.util import (get_model_from_relation, @@ -102,7 +102,7 @@ class SimpleListFilter(ListFilter): } for lookup, title in self.lookup_choices: yield { - 'selected': self.value() == lookup, + 'selected': self.value() == force_text(lookup), 'query_string': cl.get_query_string({ self.parameter_name: lookup, }, []), diff --git a/tests/regressiontests/admin_filters/tests.py b/tests/regressiontests/admin_filters/tests.py index 6a36f62cea..d00490038d 100644 --- a/tests/regressiontests/admin_filters/tests.py +++ b/tests/regressiontests/admin_filters/tests.py @@ -78,6 +78,21 @@ class DecadeListFilterParameterEndsWith__Isnull(DecadeListFilter): parameter_name = 'decade__isnull' # Ends with '__isnull" +class DepartmentListFilterLookupWithNonStringValue(SimpleListFilter): + title = 'department' + parameter_name = 'department' + + def lookups(self, request, model_admin): + return set([ + (employee.department.id, # Intentionally not a string (Refs #19318) + employee.department.code) + for employee in model_admin.queryset(request).all() + ]) + + def queryset(self, request, queryset): + if self.value(): + return queryset.filter(department__id=self.value()) + class CustomUserAdmin(UserAdmin): list_filter = ('books_authored', 'books_contributed') @@ -118,6 +133,10 @@ class EmployeeAdmin(ModelAdmin): list_filter = ['department'] +class DepartmentFilterEmployeeAdmin(EmployeeAdmin): + list_filter = [DepartmentListFilterLookupWithNonStringValue, ] + + class ListFiltersTests(TestCase): def setUp(self): @@ -140,6 +159,14 @@ class ListFiltersTests(TestCase): self.gipsy_book.contributors = [self.bob, self.lisa] self.gipsy_book.save() + # Departments + self.dev = Department.objects.create(code='DEV', description='Development') + self.design = Department.objects.create(code='DSN', description='Design') + + # Employees + self.john = Employee.objects.create(name='John Blue', department=self.dev) + self.jack = Employee.objects.create(name='Jack Red', department=self.design) + def get_changelist(self, request, model, modeladmin): return ChangeList(request, model, modeladmin.list_display, modeladmin.list_display_links, modeladmin.list_filter, modeladmin.date_hierarchy, modeladmin.search_fields, @@ -638,6 +665,28 @@ class ListFiltersTests(TestCase): self.assertEqual(choices[2]['selected'], True) self.assertEqual(choices[2]['query_string'], '?decade__isnull=the+90s') + def test_lookup_with_non_string_value(self): + """ + Ensure choices are set the selected class when using non-string values + for lookups in SimpleListFilters. + Refs #19318 + """ + + modeladmin = DepartmentFilterEmployeeAdmin(Employee, site) + request = self.request_factory.get('/', {'department': '1'}) + changelist = self.get_changelist(request, Employee, modeladmin) + + queryset = changelist.get_query_set(request) + + self.assertEqual(list(queryset), [self.john]) + + filterspec = changelist.get_filters(request)[0][-1] + self.assertEqual(force_text(filterspec.title), 'department') + choices = list(filterspec.choices(changelist)) + self.assertEqual(choices[2]['display'], 'DEV') + self.assertEqual(choices[2]['selected'], True) + self.assertEqual(choices[2]['query_string'], '?department=1') + def test_fk_with_to_field(self): """ Ensure that a filter on a FK respects the FK's to_field attribute. @@ -645,17 +694,12 @@ class ListFiltersTests(TestCase): """ modeladmin = EmployeeAdmin(Employee, site) - dev = Department.objects.create(code='DEV', description='Development') - design = Department.objects.create(code='DSN', description='Design') - john = Employee.objects.create(name='John Blue', department=dev) - jack = Employee.objects.create(name='Jack Red', department=design) - request = self.request_factory.get('/', {}) changelist = self.get_changelist(request, Employee, modeladmin) # Make sure the correct queryset is returned queryset = changelist.get_query_set(request) - self.assertEqual(list(queryset), [jack, john]) + self.assertEqual(list(queryset), [self.jack, self.john]) filterspec = changelist.get_filters(request)[0][-1] self.assertEqual(force_text(filterspec.title), 'department') @@ -680,7 +724,7 @@ class ListFiltersTests(TestCase): # Make sure the correct queryset is returned queryset = changelist.get_query_set(request) - self.assertEqual(list(queryset), [john]) + self.assertEqual(list(queryset), [self.john]) filterspec = changelist.get_filters(request)[0][-1] self.assertEqual(force_text(filterspec.title), 'department')