Merge pull request #22 from fusionbox/sortform_fixes

SortForm fixes

betterforms/changelist.py

@@ -18,8 +18,15 @@
 
 def construct_querystring(data, **kwargs):
     params = copy.copy(data)
-    params.update(kwargs)
-    return urlencode(params)
+
+    # We can't call update here because QueryDict extends rather than replaces.
+    for key, value in kwargs.items():
+        params[key] = value
+
+    if hasattr(params, 'urlencode'):
+        return params.urlencode()
+    else:
+        return urlencode(params)
 
 
 class IterDict(OrderedDict):
@@ -275,7 +282,26 @@ def __getitem__(self, key):
             return self.HeaderClass.BoundClass(self.form, self.headers[key])
 
 
-class SortForm(BaseChangeListForm):
+class SortFormBase(BetterForm):
+    """
+    A base class for writing your own SortForm. This form handles everything
+    except applying the sorts to the queryset, which is convenient if you
+    aren't working within the ChangeListForm paradigm.
+
+    Usage::
+
+        class MyForm(SortFormBase):
+            HEADERS = (
+                Header('name', label='Name'),
+            )
+
+            # fields ...
+
+            def get_results(self):
+                queryset = # ...
+                queryset = self.apply_sorting(queryset)
+                return queryset
+    """
     HeaderSetClass = HeaderSet
     error_messages = {
         'unknown_header': 'Invalid sort parameter',
@@ -285,8 +311,8 @@ class SortForm(BaseChangeListForm):
     sorts = forms.CharField(required=False, widget=forms.HiddenInput())
 
     def __init__(self, *args, **kwargs):
-        super(SortForm, self).__init__(*args, **kwargs)
-        self.headers = HeaderSet(self, self.HEADERS)
+        super(SortFormBase, self).__init__(*args, **kwargs)
+        self.headers = self.HeaderSetClass(self, self.HEADERS)
 
     def clean_sorts(self):
         cleaned_data = self.cleaned_data
@@ -317,15 +343,18 @@ def get_order_by(self):
             order_by.append(param)
         return order_by
 
+    def apply_sorting(self, qs):
+        order_by = self.get_order_by()
+        if order_by:
+            qs = qs.order_by(*order_by)
+        return qs
+
+
+class SortForm(BaseChangeListForm, SortFormBase):
     def get_queryset(self):
         """
         Returns an ordered queryset, sorted based on the values submitted in
         the sort parameter.
         """
         qs = super(SortForm, self).get_queryset()
-
-        order_by = self.get_order_by()
-        if order_by:
-            qs = qs.order_by(*order_by)
-
-        return qs
+        return self.apply_sorting(qs)

betterforms/tests.py

@@ -14,6 +14,7 @@
 from django.db import models
 from django.test import TestCase
 from django.template.loader import render_to_string
+from django.http import QueryDict
 
 from betterforms.changelist import (
     BaseChangeListForm, SearchForm, SortForm, HeaderSet, Header, BoundHeader
@@ -1001,6 +1002,41 @@ def test_bound_header_querystring_properties(self):
         self.assertEqual(header_set['field_c'].singular_querystring, 'sorts=3')
         self.assertEqual(header_set['field_c'].remove_querystring, 'sorts=1.-2')
 
+    def assertQueryStringEqual(self, a, b, *args, **kwargs):
+        """
+        We need this because QueryDicts are dicts and key-ordering is not
+        guaranteed on Python3. So we just convert query_strings back into
+        QueryDicts to compare them.
+        """
+        self.assertEqual(QueryDict(a), QueryDict(b), *args, **kwargs)
+
+    def test_bound_header_querystring_with_querydict(self):
+        HEADERS = (
+            Header('field_a'),
+        )
+        self.form.data = QueryDict('field=value')
+        self.form.cleaned_data = {'field': 'value'}
+        self.form.HEADERS = HEADERS
+        header_set = HeaderSet(self.form, HEADERS)
+
+        self.assertQueryStringEqual(header_set['field_a'].querystring, 'field=value&sorts=1')
+        self.assertQueryStringEqual(header_set['field_a'].singular_querystring, 'field=value&sorts=1')
+        self.assertQueryStringEqual(header_set['field_a'].remove_querystring, 'field=value&sorts=')
+
+    def test_bound_header_querystring_with_querydict_overwrites_instead_of_appending(self):
+        HEADERS = (
+            Header('field_a'),
+        )
+        self.form.data = QueryDict('field=value&sorts=1')
+        self.form.cleaned_data = {'field': 'value', 'sorts': [1]}
+        self.form.HEADERS = HEADERS
+        header_set = HeaderSet(self.form, HEADERS)
+
+        # It used to output 'field=value&sorts=1&sorts=-1'
+        self.assertQueryStringEqual(header_set['field_a'].querystring, 'field=value&sorts=-1')
+        self.assertQueryStringEqual(header_set['field_a'].singular_querystring, 'field=value&sorts=-1')
+        self.assertQueryStringEqual(header_set['field_a'].remove_querystring, 'field=value&sorts=')
+
 
 class TestSortFormAPI(TestCase):
     def setUp(self):