Preserve existing query params in PaginatorMixin

Previously, generation of next/previous links would discard any existing
query parameters. This commit introduces a dependency on URLObject, which
is used to intelligently parse and modify URLs to ensure existing params
are preserved.

Addresses issues #107

djangorestframework/mixins.py

@@ -7,6 +7,7 @@ from django.contrib.auth.models import AnonymousUser
 from django.core.paginator import Paginator
 from django.db.models.fields.related import ForeignKey
 from django.http import HttpResponse
+from urlobject import URLObject
 
 from djangorestframework import status
 from djangorestframework.renderers import BaseRenderer
@@ -659,11 +660,12 @@ class PaginatorMixin(object):
 
     def url_with_page_number(self, page_number):
         """ Constructs a url used for getting the next/previous urls """
-        url = "%s?page=%d" % (self.request.path, page_number)
+        url = URLObject.parse(self.request.get_full_path())
+        url = url.add_query_param('page', page_number)
 
         limit = self.get_limit()
         if limit != self.limit:
-            url = "%s&limit=%d" % (url, limit)
+            url = url.add_query_param('limit', limit)
 
         return url
 

djangorestframework/tests/mixins.py

@@ -237,3 +237,14 @@ class TestPagination(TestCase):
         response = MockPaginatorView.as_view()(request)
         content = json.loads(response.content)
         self.assertEqual(response.status_code, status.NOT_FOUND)
+
+    def test_existing_query_parameters_are_preserved(self):
+        """ Tests that existing query parameters are preserved when
+        generating next/previous page links """
+        request = self.req.get('/paginator/?foo=bar&another=something')
+        response = MockPaginatorView.as_view()(request)
+        content = json.loads(response.content)
+        self.assertEqual(response.status_code, status.OK)
+        self.assertTrue('foo=bar' in content['next'])
+        self.assertTrue('another=something' in content['next'])
+        self.assertTrue('page=2' in content['next'])

requirements.txt

@@ -3,3 +3,4 @@
 
 Django>=1.2
 coverage>=3.4
+URLObject>=0.6.0

setup.py

@@ -26,6 +26,7 @@ setup(
     package_dir={'djangorestframework': 'djangorestframework'},
     package_data = {'djangorestframework': ['templates/*', 'static/*']},
     test_suite = 'djangorestframework.runtests.runcoverage.main',
+    install_requires=['URLObject>=0.6.0'],
     classifiers = [
         'Development Status :: 4 - Beta',
         'Environment :: Web Environment',