Ticket #14467: AllValuesFilterSpec_Null.3.diff

django/contrib/admin/filterspecs.py

@@ -161 +161 @@
 class AllValuesFilterSpec(FilterSpec):
     def __init__(self, f, request, params, model, model_admin):
         super(AllValuesFilterSpec, self).__init__(f, request, params, model, model_admin)
-        self.lookup_val = request.GET.get(f.name, None)
+        self.lookup_kwarg = f.name
+        self.lookup_kwarg_isnull = '%s__isnull' % f.name
+        self.lookup_val = request.GET.get(self.lookup_kwarg, None)
+        self.lookup_val_isnull = request.GET.get(self.lookup_kwarg_isnull, None)
         self.lookup_choices = model_admin.queryset(request).distinct().order_by(f.name).values(f.name)
 
     def title(self):
         return self.field.verbose_name
 
     def choices(self, cl):
-        yield {'selected': self.lookup_val is None,
-               'query_string': cl.get_query_string({}, [self.field.name]),
+        # Turn the choices into a list. If changing self.lookup_choices query
+        # to values_list(f.name, flat=True) is not considered backwards
+        # incompatible, that is of course the proper solution...
+        lookup_choice_vals = [val[self.field.name] for val in self.lookup_choices]
+        yield {'selected': self.lookup_val is None and self.lookup_val_isnull is None,
+               'query_string': cl.get_query_string({}, [self.lookup_kwarg, self.lookup_kwarg_isnull]),
                'display': _('All')}
-        for val in self.lookup_choices:
-            val = smart_unicode(val[self.field.name])
+
+        for val in lookup_choice_vals:
+            if val is None:
+                continue
+            val = smart_unicode(val)
             yield {'selected': self.lookup_val == val,
-                   'query_string': cl.get_query_string({self.field.name: val}),
+                   'query_string': cl.get_query_string({self.lookup_kwarg: val}, [self.lookup_kwarg_isnull]),
                    'display': val}
+        if None in lookup_choice_vals:
+            yield {'selected': self.lookup_val_isnull is not None,
+                    'query_string': cl.get_query_string({self.lookup_kwarg_isnull: 'True'}, [self.lookup_kwarg]),
+                    'display': _('None')}
+
 FilterSpec.register(lambda f: True, AllValuesFilterSpec)

new file tests/regressiontests/admin_filterspecs/models.py

@@ +1 @@
+from django.db import models
+
+class OptionalAge(models.Model):
+    name = models.CharField(max_length=25)
+    age = models.PositiveIntegerField(null=True, blank=True)

new file tests/regressiontests/admin_filterspecs/tests.py

@@ +1 @@
+from django.test import TestCase
+
+from django.contrib import admin
+from django.contrib.admin.views.main import ChangeList
+from django.contrib.admin.options import IncorrectLookupParameters
+
+from models import OptionalAge
+
+class AllValuesFilterSpecTest(TestCase):
+    def setUp(self):
+        OptionalAge.objects.create(name='Sam', age=35)
+        OptionalAge.objects.create(name='Tilk', age=135)
+        OptionalAge.objects.create(name='Oneill', age=None)
+
+    def testAllValuesFilterSpecTest(self):
+        m = OptionalAgeAdmin(OptionalAge, admin.site)
+        r = MockRequest()
+        r.GET = {'age__isnull': 'True'}
+        cl = ChangeList(r, OptionalAge, m.list_display, m.list_display_links,
+            m.list_filter, m.date_hierarchy, m.search_fields,
+            m.list_select_related, m.list_per_page, m.list_editable, m)
+        """ make sure cl.get_query_set does not raise IncorrectLookupParameters """
+        queryset = cl.get_query_set()
+
+        """ make sure the last choice is None and is selected """
+        filterspec = cl.get_filters(r)[0][0]
+        choices = list(filterspec.choices(cl))
+        self.assertEquals(choices[3]['selected'], True)
+        self.assertEquals(choices[3]['query_string'], '?age__isnull=True')
+
+        """ make sure correct choice is selected """
+        r = MockRequest()
+        r.GET = {'age': '135'}
+        cl = ChangeList(r, OptionalAge, m.list_display, m.list_display_links,
+            m.list_filter, m.date_hierarchy, m.search_fields,
+            m.list_select_related, m.list_per_page, m.list_editable, m)
+        filterspec = cl.get_filters(r)[0][0]
+        choices = list(filterspec.choices(cl))
+        self.assertEquals(choices[2]['selected'], True)
+        self.assertEquals(choices[2]['query_string'], '?age=135')
+
+    def tearDown(self):
+        OptionalAge.objects.all().delete()
+
+class OptionalAgeAdmin(admin.ModelAdmin):
+    list_filter = ('age',)
+    order_by = '-id'
+
+class MockRequest(object):
+    GET = {}