Merge pull request carltongibson#110 from timheap/fix_ordering

Fix regression in order_by
Ogreman · Aug 10, 2013 · 36180d5 · 36180d5
2 parents ab79407 + 1b6c89f
commit 36180d5
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 10 deletions.
diff --git a/django_filters/filterset.py b/django_filters/filterset.py
@@ -4,6 +4,7 @@
 from copy import deepcopy
 
 from django import forms
+from django.core.validators import EMPTY_VALUES
 from django.db import models
 from django.db.models.fields import FieldDoesNotExist
 from django.db.models.related import RelatedObject
@@ -263,7 +264,6 @@ def qs(self):
 
             # start with all the results and filter from there
             qs = self.queryset.all()
-            ordered_value = None
             for name, filter_ in six.iteritems(self.filters):
                 value = None
                 if valid:
@@ -284,16 +284,20 @@ def qs(self):
                 if value is not None:  # valid & clean data
                     qs = filter_.filter(qs, value)
 
-                    # check if we are ordering and if this field is the order_by field
-                    # In the future, maybe collect multiple order_by fields in a dict???
-                    if self._meta.order_by and name == self.order_by_field:
-                        ordered_value = value
-
             if self._meta.order_by:
-                if ordered_value is None:
+                order_field = self.form.fields[self.order_by_field]
+                data = self.form[self.order_by_field].data
+                ordered_value = None
+                try:
+                    ordered_value = order_field.clean(data)
+                except forms.ValidationError:
+                    pass
+
+                if ordered_value in EMPTY_VALUES and self.strict:
                     ordered_value = self.form.fields[self.order_by_field].choices[0][0]
 
-                qs = qs.order_by(ordered_value)
+                if ordered_value:
+                    qs = qs.order_by(ordered_value)
 
             self._qs = qs
 

diff --git a/tests/test_filterset.py b/tests/test_filterset.py
@@ -453,7 +453,11 @@ class F(FilterSet):
             class Meta:
                 model = User
                 fields = ['username', 'status']
-                order_by = ['status']
+                order_by = ['username', 'status']
+
+        f = F({'o': 'username'}, queryset=self.qs)
+        self.assertQuerysetEqual(
+            f.qs, ['aaron', 'alex', 'carl', 'jacob'], lambda o: o.username)
 
         f = F({'o': 'status'}, queryset=self.qs)
         self.assertQuerysetEqual(
@@ -481,7 +485,7 @@ class Meta:
 
         f = F({'o': 'username'}, queryset=self.qs)
         self.assertQuerysetEqual(
-            f.qs, ['carl', 'alex', 'jacob', 'aaron'], lambda o: o.username)
+            f.qs, ['alex', 'jacob', 'aaron', 'carl'], lambda o: o.username)
 
     def test_ordering_on_different_field(self):
         class F(FilterSet):
@@ -494,6 +498,10 @@ class Meta:
         self.assertQuerysetEqual(
             f.qs, ['aaron', 'alex', 'carl', 'jacob'], lambda o: o.username)
 
+        f = F({'o': 'status'}, queryset=self.qs)
+        self.assertQuerysetEqual(
+            f.qs, ['carl', 'alex', 'jacob', 'aaron'], lambda o: o.username)
+
     @unittest.skip('todo')
     def test_ordering_uses_filter_name(self):
         class F(FilterSet):