tweaks/fixes as per jbalogh's comments b02fd (bug 581203)

apps/amo/tests/test_redirects.py

@@ -22,7 +22,6 @@ class TestRedirects(test.TestCase):
     def test_top_tags(self):
         """`/top-tags/?` should 301 to `/tags/top`."""
         response = self.client.get(u'top-tags/', follow=True)
-        eq_(response.redirect_chain[0][1], 301)
         self.assertRedirects(response, '/en-US/firefox/tags/top',
                              status_code=301)
 

apps/tags/helpers.py

@@ -1,5 +1,3 @@
-from math import ceil
-
 from jingo import register
 import jinja2
 from access import acl
@@ -26,17 +24,20 @@ def tag_list(context, addon, dev_tags=[], user_tags=[],
 
 
 def range_convert(value, old_min, old_max, new_min, new_max):
-    """utility to tranfer a value (preserving the relative value in
-       the range) from its current range to a new one"""
+    """
+    Utility to tranfer a value (preserving the relative value in
+    the range) from its current range to a new one.
+    """
     old_range = 1 if old_max - old_min == 0 else old_max - old_min
     new_range = new_max - new_min
     return int(((value - old_min) * new_range) / old_range) + new_min
 
+
 @register.function
 def tag_link(tag, min_count, max_count):
     """create the tag cloud link with the poper tagLevel class"""
     factor = range_convert(tag.tagstat.num_addons, min_count, max_count, 1, 10)
-    hyperlink = '<a class="tagLevel%d tag" href="%s">%s</a>'
+    hyperlink = u'<a class="tagLevel%d tag" href="%s">%s</a>'
     return hyperlink % (factor,
                         remora_url('/tag/' + tag.tag_text),
                         tag.tag_text)

apps/tags/templates/tags/top_cloud.html

@@ -1,25 +1,25 @@
 {% extends "base.html" %}
 
 {% block title %}
-  {{ page_title( _('Top {0} Tags')|f(num_tags)) }}
+  {{ page_title(_('Top {0} Tags')|f(num_tags)) }}
 {% endblock %}
 
 {% block content %}
 <div class="primary full" role="main">
-    {% cache top_tags %}
-      {% if top_tags %}
-      {% set max_tag_count = top_tags[0].popularity or 1 %}
-      {% set min_tag_count = (top_tags|last).popularity or 1 %}
-      {# L10n: {0} is the current application name #}
-      <h2>{{ _('{0} Top Tags')|f(request.APP.pretty) }}</h2>
-      <div id="tag-box">
-        <div>
-          {% for tag in top_tags|list|shuffle if tag.tagstat %}
-            {{ tag_link(tag, min_tag_count, max_tag_count)|safe }}
-          {% endfor %}
-        </div>
+  {% cache top_tags %}
+    {% if top_tags %}
+    {% set max_tag_count = top_tags[0].popularity or 1 %}
+    {% set min_tag_count = (top_tags|last).popularity or 1 %}
+    {# L10n: {0} is the current application name #}
+    <h2>{{ _('{0} Top Tags')|f(request.APP.pretty) }}</h2>
+    <div id="tag-box">
+      <div>
+        {% for tag in top_tags|list|shuffle if tag.tagstat %}
+          {{ tag_link(tag, min_tag_count, max_tag_count)|safe }}
+        {% endfor %}
       </div>
-      {% endif %}
-    {% endcache %}
+    </div>
+    {% endif %}
+  {% endcache %}
 </div>
 {% endblock content %}

apps/tags/urls.py

@@ -1,6 +1,6 @@
-from django.conf.urls.defaults import patterns, url, include
+from django.conf.urls.defaults import patterns, url
 from . import views
 
 urlpatterns = patterns('',
-    url('^tags/top/?$', views.top_cloud, name='tags.top_cloud'),
-)
+    url('^tags/top$', views.top_cloud, name='tags.top_cloud'),
+)

urls.py

@@ -74,7 +74,7 @@ urlpatterns = patterns('',
     ('^persona/(\d+)',
      lambda r, id: redirect('addons.detail', id, permanent=True)),
 
-    # Redirect persona/xxx
+    # Redirect top-tags to tags/top
     ('^top-tags/?',
      lambda r: redirect('tags.top_cloud', permanent=True)),