Fixed #15606 -- Ensured that boolean fields always use the Boolean filterspec. Thanks to Martin Tiršel for the report

git-svn-id: http://code.djangoproject.com/svn/django/trunk@15817 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/admin/filterspecs.py

@@ -121,6 +121,38 @@ FilterSpec.register(lambda f: (
         hasattr(f, 'rel') and bool(f.rel) or
         isinstance(f, models.related.RelatedObject)), RelatedFilterSpec)
 
+class BooleanFieldFilterSpec(FilterSpec):
+    def __init__(self, f, request, params, model, model_admin,
+                 field_path=None):
+        super(BooleanFieldFilterSpec, self).__init__(f, request, params, model,
+                                                     model_admin,
+                                                     field_path=field_path)
+        self.lookup_kwarg = '%s__exact' % self.field_path
+        self.lookup_kwarg2 = '%s__isnull' % self.field_path
+        self.lookup_val = request.GET.get(self.lookup_kwarg, None)
+        self.lookup_val2 = request.GET.get(self.lookup_kwarg2, None)
+
+    def title(self):
+        return self.field.verbose_name
+
+    def choices(self, cl):
+        for k, v in ((_('All'), None), (_('Yes'), '1'), (_('No'), '0')):
+            yield {'selected': self.lookup_val == v and not self.lookup_val2,
+                   'query_string': cl.get_query_string(
+                                   {self.lookup_kwarg: v},
+                                   [self.lookup_kwarg2]),
+                   'display': k}
+        if isinstance(self.field, models.NullBooleanField):
+            yield {'selected': self.lookup_val2 == 'True',
+                   'query_string': cl.get_query_string(
+                                   {self.lookup_kwarg2: 'True'},
+                                   [self.lookup_kwarg]),
+                   'display': _('Unknown')}
+
+FilterSpec.register(lambda f: isinstance(f, models.BooleanField)
+                              or isinstance(f, models.NullBooleanField),
+                                 BooleanFieldFilterSpec)
+
 class ChoicesFilterSpec(FilterSpec):
     def __init__(self, f, request, params, model, model_admin,
                  field_path=None):
@@ -187,37 +219,6 @@ class DateFieldFilterSpec(FilterSpec):
 FilterSpec.register(lambda f: isinstance(f, models.DateField),
                               DateFieldFilterSpec)
 
-class BooleanFieldFilterSpec(FilterSpec):
-    def __init__(self, f, request, params, model, model_admin,
-                 field_path=None):
-        super(BooleanFieldFilterSpec, self).__init__(f, request, params, model,
-                                                     model_admin,
-                                                     field_path=field_path)
-        self.lookup_kwarg = '%s__exact' % self.field_path
-        self.lookup_kwarg2 = '%s__isnull' % self.field_path
-        self.lookup_val = request.GET.get(self.lookup_kwarg, None)
-        self.lookup_val2 = request.GET.get(self.lookup_kwarg2, None)
-
-    def title(self):
-        return self.field.verbose_name
-
-    def choices(self, cl):
-        for k, v in ((_('All'), None), (_('Yes'), '1'), (_('No'), '0')):
-            yield {'selected': self.lookup_val == v and not self.lookup_val2,
-                   'query_string': cl.get_query_string(
-                                   {self.lookup_kwarg: v},
-                                   [self.lookup_kwarg2]),
-                   'display': k}
-        if isinstance(self.field, models.NullBooleanField):
-            yield {'selected': self.lookup_val2 == 'True',
-                   'query_string': cl.get_query_string(
-                                   {self.lookup_kwarg2: 'True'},
-                                   [self.lookup_kwarg]),
-                   'display': _('Unknown')}
-
-FilterSpec.register(lambda f: isinstance(f, models.BooleanField)
-                              or isinstance(f, models.NullBooleanField),
-                                 BooleanFieldFilterSpec)
 
 # This should be registered last, because it's a last resort. For example,
 # if a field is eligible to use the BooleanFieldFilterSpec, that'd be much

tests/regressiontests/admin_filterspecs/models.py

@@ -9,3 +9,15 @@ class Book(models.Model):
 
     def __unicode__(self):
         return self.title
+
+class BoolTest(models.Model):
+    NO = False
+    YES = True
+    YES_NO_CHOICES = (
+        (NO, 'no'),
+        (YES, 'yes')
+    )
+    completed = models.BooleanField(
+        default=NO,
+        choices=YES_NO_CHOICES
+    )

tests/regressiontests/admin_filterspecs/tests.py

@@ -6,7 +6,7 @@ from django.contrib import admin
 from django.contrib.admin.views.main import ChangeList
 from django.utils.encoding import force_unicode
 
-from models import Book
+from models import Book, BoolTest
 
 def select_by(dictlist, key, value):
     return [x for x in dictlist if x[key] == value][0]
@@ -26,6 +26,10 @@ class FilterSpecsTests(TestCase):
         gipsy_book.contributors = [self.bob, lisa]
         gipsy_book.save()
 
+        # BoolTests
+        self.trueTest = BoolTest.objects.create(completed=True)
+        self.falseTest = BoolTest.objects.create(completed=False)
+
         self.request_factory = RequestFactory()
 
 
@@ -167,9 +171,41 @@ class FilterSpecsTests(TestCase):
         self.assertEqual(choice['selected'], True)
         self.assertEqual(choice['query_string'], '?books_contributed__id__exact=%d' % self.django_book.pk)
 
+    def test_BooleanFilterSpec(self):
+        modeladmin = BoolTestAdmin(BoolTest, admin.site)
+
+        request = self.request_factory.get('/')
+        changelist = ChangeList(request, BoolTest, modeladmin.list_display, modeladmin.list_display_links,
+            modeladmin.list_filter, modeladmin.date_hierarchy, modeladmin.search_fields,
+            modeladmin.list_select_related, modeladmin.list_per_page, modeladmin.list_editable, modeladmin)
+
+        # Make sure changelist.get_query_set() does not raise IncorrectLookupParameters
+        queryset = changelist.get_query_set()
+
+        # Make sure the last choice is None and is selected
+        filterspec = changelist.get_filters(request)[0][0]
+        self.assertEqual(force_unicode(filterspec.title()), u'completed')
+        choices = list(filterspec.choices(changelist))
+        self.assertEqual(choices[-1]['selected'], False)
+        self.assertEqual(choices[-1]['query_string'], '?completed__exact=0')
+
+        request = self.request_factory.get('/', {'completed__exact': 1})
+        changelist = self.get_changelist(request, BoolTest, modeladmin)
+
+        # Make sure the correct choice is selected
+        filterspec = changelist.get_filters(request)[0][0]
+        self.assertEqual(force_unicode(filterspec.title()), u'completed')
+        # order of choices depends on User model, which has no order
+        choice = select_by(filterspec.choices(changelist), "display", "Yes")
+        self.assertEqual(choice['selected'], True)
+        self.assertEqual(choice['query_string'], '?completed__exact=1')
+
 class CustomUserAdmin(UserAdmin):
     list_filter = ('books_authored', 'books_contributed')
 
 class BookAdmin(admin.ModelAdmin):
     list_filter = ('year', 'author', 'contributors')
     order_by = '-id'
+
+class BoolTestAdmin(admin.ModelAdmin):
+    list_filter = ('completed',)