Clean up current_query and preserve multiple args

When implementing search for multiple architectures or repositories, I
didn't update this method to accomidate the new query parameters. Clean
it up a bit by not appending/stripping the leading '?' anywhere but in
the template itself, and ensure we can handle multiple of any parameter
passed in. Fixes FS#23180.

Signed-off-by: Dan McGee <dan@archlinux.org>

3 files changed, 23 insertions, 21 deletions

diff --git a/packages/templatetags/package_extras.py b/packages/templatetags/package_extras.py
index d59a5562..24264af0 100644
--- a/packages/templatetags/package_extras.py
+++ b/packages/templatetags/package_extras.py
@@ -1,4 +1,6 @@
-import cgi, urllib
+import urllib
+import urlparse
+
 from django import template
 from django.utils.html import escape
 
@@ -9,15 +11,15 @@ class BuildQueryStringNode(template.Node):
         self.sortfield = sortfield
 
     def render(self, context):
-        qs = dict(cgi.parse_qsl(context['current_query'][1:]))
-        if qs.has_key('sort') and qs['sort'] == self.sortfield:
+        qs = urlparse.parse_qs(context['current_query'])
+        if qs.has_key('sort') and self.sortfield in qs['sort']:
             if self.sortfield.startswith('-'):
-                qs['sort'] = self.sortfield[1:]
+                qs['sort'] = [self.sortfield[1:]]
             else:
-                qs['sort'] = '-' + self.sortfield
+                qs['sort'] = ['-' + self.sortfield]
         else:
-            qs['sort'] = self.sortfield
-        return '?' + urllib.urlencode(qs)
+            qs['sort'] = [self.sortfield]
+        return urllib.urlencode(qs, True)
 
 @register.tag(name='buildsortqs')
 def do_buildsortqs(parser, token):
diff --git a/packages/views.py b/packages/views.py
index 59779fe4..3cef8226 100644
--- a/packages/views.py
+++ b/packages/views.py
@@ -168,12 +168,10 @@ class PackageSearchForm(forms.Form):
                 [(m.username, m.get_full_name()) for m in maints]
 
 def search(request, page=None):
-    current_query = '?'
     limit = 50
     packages = Package.objects.select_related('arch', 'repo')
 
     if request.GET:
-        current_query += request.GET.urlencode()
         form = PackageSearchForm(data=request.GET)
         if form.is_valid():
             if form.cleaned_data['repo']:
@@ -208,12 +206,14 @@ def search(request, page=None):
     else:
         form = PackageSearchForm()
 
-    page_dict = {'search_form': form,
-            'current_query': current_query
-            }
     if packages.count() == 1:
         return redirect(packages[0])
 
+    current_query = request.GET.urlencode()
+    page_dict = {
+            'search_form': form,
+            'current_query': current_query
+    }
     allowed_sort = ["arch", "repo", "pkgname", "last_update", "flag_date"]
     allowed_sort += ["-" + s for s in allowed_sort]
     sort = request.GET.get('sort', None)
diff --git a/templates/packages/search.html b/templates/packages/search.html
index dad05ffa..5d0de1f4 100644
--- a/templates/packages/search.html
+++ b/templates/packages/search.html
@@ -50,13 +50,13 @@
 
         <p class="pkglist-nav">
             {% if page_obj.has_previous %}
-            <a class="prev" href="/packages/{{page_obj.previous_page_number}}/{{current_query}}"
+            <a class="prev" href="/packages/{{page_obj.previous_page_number}}/?{{current_query}}"
                 title="Go to previous page">&lt; Prev</a>
             {% else %}
             <span class="prev">&lt; Prev</span>
             {% endif %}
             {% if page_obj.has_next %}
-            <a class="next" href="/packages/{{page_obj.next_page_number}}/{{current_query}}"
+            <a class="next" href="/packages/{{page_obj.next_page_number}}/?{{current_query}}"
                 title="Go to next page">Next &gt;</a>
             {% else %}
             <span class="next">Next &gt;</span>
@@ -74,17 +74,17 @@
                     {% if perms.main.change_package %}
                     <th>&nbsp;</th>
                     {% endif %}
-                    <th><a href="/packages/{% buildsortqs "arch" %}"
+                    <th><a href="/packages/?{% buildsortqs "arch" %}"
                             title="Sort packages by architecture">Arch</a></th>
-                    <th><a href="/packages/{% buildsortqs "repo" %}"
+                    <th><a href="/packages/?{% buildsortqs "repo" %}"
                             title="Sort packages by repository">Repo</a></th>
-                    <th><a href="/packages/{% buildsortqs "pkgname" %}"
+                    <th><a href="/packages/?{% buildsortqs "pkgname" %}"
                             title="Sort packages by package name">Name</a></th>
                     <th>Version</th>
                     <th>Description</th>
-                    <th><a href="/packages/{% buildsortqs "-last_update" %}"
+                    <th><a href="/packages/?{% buildsortqs "-last_update" %}"
                             title="Sort packages by last update">Last Updated</a></th>
-                    <th><a href="/packages/{% buildsortqs "-flag_date" %}"
+                    <th><a href="/packages/?{% buildsortqs "-flag_date" %}"
                             title="Sort packages by when marked-out of-date">Flag Date</a></th>
                 </tr>
             </thead>
@@ -118,13 +118,13 @@
 
                 <p class="pkglist-nav">
                     {% if page_obj.has_previous %}
-                    <a class="prev" href="/packages/{{page_obj.previous_page_number}}/{{current_query}}"
+                    <a class="prev" href="/packages/{{page_obj.previous_page_number}}/?{{current_query}}"
                         title="Go to previous page">&lt; Prev</a>
                     {% else %}
                     <span class="prev">&lt; Prev</span>
                     {% endif %}
                     {% if page_obj.has_next %}
-                    <a class="next" href="/packages/{{page_obj.next_page_number}}/{{current_query}}"
+                    <a class="next" href="/packages/{{page_obj.next_page_number}}/?{{current_query}}"
                         title="Go to next page">Next &gt;</a>
                     {% else %}
                     <span class="next">Next &gt;</span>