fixes bug 948704 - date changing links should keep query string intac…

…t, r=adrian

webapp-django/crashstats/base/helpers.py

@@ -0,0 +1,54 @@
+import cgi
+import urllib
+
+import jinja2
+from jingo import register
+
+
+@register.function
+@jinja2.contextfunction
+def change_query_string(context, **kwargs):
+    """
+    Template function for modifying the current URL by parameters.
+    You use it like this in a template:
+
+        <a href={{ change_query_string(foo='bar') }}>
+
+    And it takes the current request's URL (and query string) and modifies it
+    just by the parameters you pass in. So if the current URL is
+    `/page/?day=1` the output of this will become:
+
+        <a href=/page?day=1&foo=bar>
+
+    You can also pass lists like this:
+
+        <a href={{ change_query_string(thing=['bar','foo']) }}>
+
+    And you get this output:
+
+        <a href=/page?day=1&thing=bar&thing=foo>
+
+    And if you want to remove a parameter you can explicitely pass it `None`.
+    Like this for example:
+
+        <a href={{ change_query_string(day=None) }}>
+
+    And you get this output:
+
+        <a href=/page>
+
+    """
+    base = context['request'].META['PATH_INFO']
+    qs = cgi.parse_qs(context['request'].META['QUERY_STRING'])
+    for key, value in kwargs.items():
+        if value is None:
+            # delete the parameter
+            if key in qs:
+                del qs[key]
+        else:
+            # change it
+            qs[key] = value
+    new_qs = urllib.urlencode(qs, True)
+    if new_qs:
+        return '%s?%s' % (base, new_qs)
+    return base

webapp-django/crashstats/base/tests/test_helpers.py

@@ -0,0 +1,53 @@
+from nose.tools import eq_
+
+from django.test import TestCase
+from django.test.client import RequestFactory
+
+from crashstats.base.helpers import (
+    change_query_string
+)
+
+
+class TestChangeURL(TestCase):
+
+    def test_root_url_no_query_string(self):
+        context = {}
+        context['request'] = RequestFactory().get('/')
+        result = change_query_string(context)
+        eq_(result, '/')
+
+    def test_with_path_no_query_string(self):
+        context = {}
+        context['request'] = RequestFactory().get('/page/')
+        result = change_query_string(context)
+        eq_(result, '/page/')
+
+    def test_with_query_string(self):
+        context = {}
+        context['request'] = RequestFactory().get('/page/?foo=bar&bar=baz')
+        result = change_query_string(context)
+        eq_(result, '/page/?foo=bar&bar=baz')
+
+    def test_add_query_string(self):
+        context = {}
+        context['request'] = RequestFactory().get('/page/')
+        result = change_query_string(context, foo='bar')
+        eq_(result, '/page/?foo=bar')
+
+    def test_change_query_string(self):
+        context = {}
+        context['request'] = RequestFactory().get('/page/?foo=bar')
+        result = change_query_string(context, foo='else')
+        eq_(result, '/page/?foo=else')
+
+    def test_remove_query_string(self):
+        context = {}
+        context['request'] = RequestFactory().get('/page/?foo=bar')
+        result = change_query_string(context, foo=None)
+        eq_(result, '/page/')
+
+    def test_remove_leave_some(self):
+        context = {}
+        context['request'] = RequestFactory().get('/page/?foo=bar&other=thing')
+        result = change_query_string(context, foo=None)
+        eq_(result, '/page/?other=thing')

webapp-django/crashstats/crashstats/templates/crashstats/exploitability.html

@@ -7,30 +7,25 @@
     <div class="page-heading">
         <h2>Exploitable crashes for {{ product }}
         {% if version %}{{ version }}{% endif %}</h2>
-        <ul class="options">
-        {% for day in possible_days %}
-          <li><a href="?days={{ day }}" {% if days == day %}class="selected"{% endif %}>{{ day }} days</a></li>
-        {% endfor %}
-        </ul>
     </div>
 
     <div class="panel">
         <div class="body notitle">
-            
+
             {% if current_page == 1 %}
                 <strong>&lt;&lt;First</strong>
             {% else %}
-                <a href="{{ url('crashstats.exploitable_crashes') }}?page=1">&lt;&lt;First</a>
-                <a href="{{ url('crashstats.exploitable_crashes') }}?page={{ current_page-1 }}">&lt;Previous</a>
+                <a href="{{ change_query_string(page=None) }}">&lt;&lt;First</a>
+                <a href="{{ change_query_string(page=current_page - 1) }}">&lt;Previous</a>
             {% endif %}
-                        
+
             {% if current_page == pages %}
             <strong>Last&gt;&gt;</strong>
             {% else %}
-            <a href="{{ url('crashstats.exploitable_crashes') }}?page={{ current_page+1 }}">Next&gt;</a>
-            <a href="{{ url('crashstats.exploitable_crashes') }}?page={{ pages }}">Last&gt;&gt;</a>
+            <a href="{{ change_query_string(page=current_page + 1) }}">Next&gt;</a>
+            <a href="{{ change_query_string(page=pages) }}">Last&gt;&gt;</a>
             {% endif %}
-            
+
             <table class="builds data-table">
                 <thead>
                     <tr>

webapp-django/crashstats/crashstats/templates/crashstats/home.html

@@ -10,15 +10,15 @@
         <h2 id="homepage-heading">{{ product }} {% if version %}{{ version }}{% endif %} Crash Data</h2>
         <ul id="duration" class="options">
         {% for day in possible_days %}
-            <li><a href="?days={{ day }}" {% if days == day %} class="selected" {% endif %}>{{ day }} days</a></li>
+            <li><a href="{{ change_query_string(days=day) }}" {% if days == day %} class="selected" {% endif %}>{{ day }} days</a></li>
         {% endfor %}
         </ul>
 
         {% if has_builds %}
         <ul id="date-range-type" class="options">
             <li>Date Range:</li>
-            <li><a href="?date_range_type=report"{% if default_date_range_type == 'report' %} class="selected"{% endif %}>By Crash Date</a></li>
-            <li><a href="?date_range_type=build"{% if default_date_range_type == 'build' %} class="selected"{% endif %}>By Build Date</a></li>
+            <li><a href="{{ change_query_string(date_range_type='report') }}"{% if default_date_range_type == 'report' %} class="selected"{% endif %}>By Crash Date</a></li>
+            <li><a href="{{ change_query_string(date_range_type='build') }}"{% if default_date_range_type == 'build' %} class="selected"{% endif %}>By Build Date</a></li>
         </ul>
         {% endif %}
     </div>

webapp-django/crashstats/crashstats/templates/crashstats/report_list_base.html

@@ -13,7 +13,7 @@ <h2>Crash Reports for {{ signature }}</h2>
     <ul class="options">
       {% for day in [3, 7, 14, 28] %}
       <li>
-        <a href="{{ url('crashstats.report_list') }}?product={{ product }}{% for product_version in product_versions %}&amp;version={{ product_version }}{% endfor %}&amp;query_search=signature&amp;query_type=contains&amp;reason_type=contains&amp;date={{ end_date }}&amp;range_value={{ day }}&amp;range_unit=days&amp;hang_type=any&amp;process_type=any&amp;signature={{ signature|urlencode }}" {% if day == current_day %} class="selected" {% endif %}>{{ day }} days</a>
+        <a href="{{ change_query_string(range_value=day) }}" {% if day == current_day %} class="selected" {% endif %}>{{ day }} days</a>
       </li>
       {% endfor %}
     </ul>
@@ -36,4 +36,3 @@ <h2>Crash Reports for {{ signature }}</h2>
 <!-- end content -->
 </div>
 {% endblock %}
-

webapp-django/crashstats/crashstats/templates/crashstats/topchangers.html

@@ -10,7 +10,7 @@
     <h2>Top Changers for {{ product }} {{ versions|join(', ') }}</h2>
     <ul class="options">
     {% for day in possible_days %}
-      <li><a href="?days={{ day }}" {% if days == day %}class="selected"{% endif %}>{{ day }} days</a></li>
+      <li><a href="{{ change_query_string(days=day) }}" {% if days == day %}class="selected"{% endif %}>{{ day }} days</a></li>
     {% endfor %}
     </ul>
   </div>

webapp-django/crashstats/crashstats/templates/crashstats/topcrasher.html

@@ -53,7 +53,7 @@ <h2>Top Crashers for <span id="current-product">{{ product }}</span> <span id="c
             <ul class="tc-duration-days tc-filter">
                 <li class="tc-selector-heading">Days:</li>
                 {% for day in possible_days %}
-                <li><a href="{{ url('crashstats.topcrasher', product, version, date_range_type, crash_type, os_name, result_count) }}?days={{ day }}" {% if days == day %} class="selected" {% endif %}>{{ day }}</a></li>
+                <li><a href="{{ change_query_string(days=day) }}" {% if days == day %} class="selected" {% endif %}>{{ day }}</a></li>
                 {% endfor %}
             </ul>
             <ul class="tc-per-platform tc-filter">