new collection pages, redirects from old pages (bug 574272)

apps/bandwagon/fixtures/bandwagon/test_models.json

@@ -40,7 +40,8 @@
             "default_locale": "ru",
             "up_votes": 0,
             "icontype": "",
-            "nickname": null,
+            "nickname": "wut",
+            "slug": "wut-slug",
             "addon_count": 4,
             "addon_index": null,
             "description": null

apps/bandwagon/models.py

@@ -10,6 +10,7 @@ from django.db import models, connection
 import amo
 import amo.models
 from amo.utils import sorted_groupby
+from amo.urlresolvers import reverse
 from addons.models import Addon, AddonCategory, AddonRecommendation
 from applications.models import Application
 from users.models import UserProfile
@@ -76,6 +77,8 @@ class Collection(amo.models.ModelBase):
     def save(self, **kw):
         if not self.uuid:
             self.uuid = unicode(uuid.uuid4())
+        if not self.slug:
+            self.slug = self.uuid[:30]
 
         # Maintain our index of add-on ids.
         if self.id:
@@ -85,8 +88,11 @@ class Collection(amo.models.ModelBase):
         super(Collection, self).save(**kw)
 
     def get_url_path(self):
-        # TODO(jbalogh): reverse
-        return '/collection/%s' % self.url_slug
+        if settings.NEW_COLLECTIONS:
+            nick = self.author.nickname if self.author else 'anonymous'
+            return reverse('collections.detail', args=[nick, self.slug])
+        else:
+            return '/collection/%s' % self.url_slug
 
     @classmethod
     def get_fallback(cls):

apps/bandwagon/templates/bandwagon/collection_detail.html

@@ -0,0 +1,95 @@
+{% extends "base.html" %}
+
+{% set c = collection %}
+
+{% block title %}{{ page_title(_('{0} :: Collections')|f(c.name)) }}{% endblock %}
+
+{% block bodyclass %}inverse{% endblock %}
+
+{% block content %}
+<div class="primary">
+<header>
+  {% with crumbs = [(url('collections.list'), _('Collections')), (None, c.name)] %}
+    {% if c.author %}
+      {% do crumbs.insert(1, (url('collections.user', c.author.nickname), c.author.display_name)) %}
+    {% endif %}
+    {{ breadcrumbs(crumbs) }}
+  {% endwith %}
+  <hgroup>
+    <h2>
+      <img src="{{ c.icon_url }}" class="icon">
+      <span>{{ c.name }}</span>
+    </h2>
+    <h4 class="author">
+      {% trans users=users_list(collection.listed_authors) %}
+        by {{ users }}
+      {% endtrans %}
+    </h4>
+  </hgroup>
+</header>
+
+<div class="featured">
+  <div class="featured-inner object-lead">
+    <div class="meta">
+      <ul>
+        <li>{{ barometer(collection) }}</li>
+        <li class="followers">
+          {% trans p=c.subscribers, num=c.subscribers|numberfmt %}
+            {{ num }} follower {% pluralize %} {{ num }} followers
+          {% endtrans %}
+        </li>
+        <li>{{ _('Updated {0}')|f(c.modified) }}</li>
+      </ul>
+    </div>
+    <h3>{{ _('About this Collection') }}</h3>
+    <p>{{ c.description }}</p>
+  </div>
+</div>
+
+<div class="separated-listing">
+  <h3>
+    {% trans num=c.addon_count %}
+      {{ num }} Add-on in this Collection
+    {% pluralize %}
+      {{ num }} Add-ons in this Collection
+    {% endtrans %}
+  </h3>
+  <form class="item-sort go" action="">
+    <label for="sortby">{{ _('Sort by:') }}</label>
+    <select id="sortby" name="{{ filter.key }}">
+      {% for value, title in filter.opts %}
+      <option value="{{ value }}" {{ value|ifeq(filter.field, 'selected') }}>
+        {{ title }}</option>
+      {% endfor %}
+    </select>
+    <button type="submit">{{ _('Go') }}</button>
+  </form>
+  {% cache addons.object_list %}
+    {{ addon_listing_items(addons.object_list, notes=notes.next()) }}
+    {{ addons|paginator }}
+  {% endcache %}
+</div>
+</div> {# primary #}
+
+<div class="secondary">
+<div class="highlight"></div>
+  <h3>{{ _('What are Collections?') }}</h3>
+  <p>{% trans %}
+  Collections are groups of related add-ons that anyone can create and share.
+  {% endtrans %}</p>
+  <a class="more-info" href="{{ url('collections.list') }}">
+    {{ _('Explore Collections') }}</a>
+</div>
+
+{% if author_collections %}
+  <div>
+    <h3>{{ _('More by this User') }}</h3>
+    {% for ac in author_collections %}
+      <a class="collectionitem" href="{{ ac.get_url_path() }}">
+        {{ ac.name }}</a>
+    {% endfor %}
+    <a class="more-info" href="{{ url('collections.user', c.author.nickname) }}">
+      {{ _('See all collections by this user') }}</a>
+  </div>
+{% endif %}
+{% endblock %}

apps/bandwagon/tests/test_helpers.py

@@ -60,9 +60,11 @@ class TestHelpers(test.TestCase):
 
 
     def test_user_collection_list(self):
-        c1 = Collection(uuid='eb4e3cd8-5cf1-4832-86fb-a90fc6d3765c')
-        c2 = Collection(uuid='61780943-e159-4206-8acd-0ae9f63f294c',
-                        nickname='my_collection')
+        c1 = Collection.objects.create(
+            uuid='eb4e3cd8-5cf1-4832-86fb-a90fc6d3765c')
+        c2 = Collection.objects.create(
+            uuid='61780943-e159-4206-8acd-0ae9f63f294c',
+            nickname='my_collection')
         heading = 'My Heading'
         response = unicode(user_collection_list([c1, c2], heading))
 
@@ -71,12 +73,10 @@ class TestHelpers(test.TestCase):
                             'collection list heading missing')
         # both items
         # TODO reverse URLs
-        self.assert_(response.find(u'/collection/%s' % c1.uuid) >= 0,
+        self.assert_(response.find(c1.get_url_path()) >= 0,
                             'collection UUID link missing')
-        self.assert_(response.find(u'/collection/%s' % c2.nickname) >= 0,
+        self.assert_(response.find(c2.get_url_path()) >= 0,
                             'collection nickname link missing')
-        self.assert_(response.find(u'/collection/%s' % c2.uuid) == -1,
-                         'collection with nickname should not have UUID link')
 
         # empty collection, empty response
         response = unicode(user_collection_list([], heading))

apps/bandwagon/tests/test_views.py

@@ -0,0 +1,29 @@
+from nose.tools import eq_, with_setup
+import test_utils
+
+from bandwagon.models import Collection
+
+
+class TestViews(test_utils.TestCase):
+    fixtures = ['bandwagon/test_models.json']
+
+    def check_response(self, url, code, to=None):
+        response = self.client.get(url, follow=True)
+        if code == 404:
+            eq_(response.status_code, 404)
+        elif code in (301, 302):
+            self.assertRedirects(response, to, status_code=code)
+        else:
+            assert code in (301, 302, 404), code
+
+    def test_legacy_redirects(self):
+        collection = Collection.objects.get(nickname='wut')
+        url = collection.get_url_path()
+        tests = [
+            ('/collection/wut', 301, url),
+            ('/collection/wut/', 301, url),
+            ('/collection/f94d08c7-794d-3ce4-4634-99caa09f9ef4', 301, url),
+            ('/collection/f94d08c7-794d-3ce4-4634-99caa09f9ef4/', 301, url),
+            ('/collection/404', 404)]
+        for test in tests:
+            self.check_response(*test)

apps/bandwagon/urls.py

@@ -0,0 +1,14 @@
+from django.conf.urls.defaults import patterns, url
+
+from . import views
+
+
+urlpatterns = patterns('',
+    url('^collection/(?P<uuid>[^/]+)/?$', views.legacy_redirect),
+
+    url('^collections/$', views.collection_listing, name='collections.list'),
+    url('^collections/(?P<user>[^/]+)/$', views.user_listing,
+        name='collections.user'),
+    url('^collections/(?P<user>[^/]+)/(?P<slug>[^/]+)$',
+        views.collection_detail, name='collections.detail'),
+)

apps/bandwagon/views.py

@@ -0,0 +1,66 @@
+from django import http
+from django.shortcuts import get_object_or_404, redirect
+
+import jingo
+from tower import ugettext_lazy as _lazy
+
+import amo.utils
+from addons.models import Addon
+from addons.views import BaseFilter
+from translations.query import order_by_translation
+from .models import Collection, CollectionAddon
+
+
+def legacy_redirect(self, uuid):
+    # Nicknames have a limit of 30, so len == 36 implies a uuid.
+    key = 'uuid' if len(uuid) == 36 else 'nickname'
+    c = get_object_or_404(Collection.objects, **{key: uuid})
+    return redirect(c.get_url_path())
+
+
+def collection_listing(request):
+    return http.HttpResponse()
+
+
+def user_listing(request, user):
+    return http.HttpResponse()
+
+
+class CollectionAddonFilter(BaseFilter):
+    opts = (('added', _lazy('Added')),
+            ('popular', _lazy('Popularity')),
+            ('name', _lazy('Name')))
+
+    def filter(self, field):
+        if field == 'added':
+            return self.base_queryset.order_by('collectionaddon__created')
+        elif field == 'name':
+            return order_by_translation(self.base_queryset, 'name')
+        elif field == 'popular':
+            return (self.base_queryset.order_by('-weekly_downloads')
+                    .with_index(addons='downloads_type_idx'))
+
+
+def collection_detail(request, user, slug):
+    # TODO: owner=user when dd adds owner to collections
+    cn = get_object_or_404(Collection.objects, slug=slug)
+    base = cn.addons.all() & Addon.objects.listed(request.APP)
+    filter = CollectionAddonFilter(request, base,
+                                   key='sort', default='popular')
+    notes = get_notes(cn)
+    count = base.with_index(addons='type_status_inactive_idx').count()
+    addons = amo.utils.paginate(request, filter.qs, count=count)
+    return jingo.render(request, 'bandwagon/collection_detail.html',
+                        {'collection': cn, 'filter': filter,
+                         'addons': addons, 'notes': notes})
+
+
+def get_notes(collection):
+    # This might hurt in a big collection with lots of notes.
+    # It's a generator so we don't evaluate anything by default.
+    notes = CollectionAddon.objects.filter(collection=collection,
+                                           comments__isnull=False)
+    rv = {}
+    for note in notes:
+        rv[note.addon_id] = note.comments
+    yield rv

migrations/53-collections-addon-idx.sql

@@ -0,0 +1,2 @@
+-- Holy crap this takes forever.
+CREATE INDEX created_idx ON addons_collections (collection_id, created)

settings.py

@@ -165,6 +165,7 @@ def JINJA_CONFIG():
     from django.conf import settings
     from caching.base import cache
     config = {'extensions': ['tower.template.i18n', 'amo.ext.cache',
+                             'jinja2.ext.do',
                              'jinja2.ext.with_', 'jinja2.ext.loopcontrols'],
               'finalize': lambda x: x if x is not None else ''}
     if 'memcached' in cache.scheme and not settings.DEBUG:
@@ -513,3 +514,9 @@ def read_only_mode(env):
     m = list(env['MIDDLEWARE_CLASSES'])
     m.insert(m.index(before), extra)
     env['MIDDLEWARE_CLASSES'] = tuple(m)
+
+
+## Feature switches
+# Use this to keep collections compatible with remora before we're ready to
+# switch to zamboni/bandwagon3.
+NEW_COLLECTIONS = True

urls.py

@@ -20,6 +20,9 @@ urlpatterns = patterns('',
     # Browse pages.
     ('', include('browse.urls')),
 
+    # Collections.
+    ('', include('bandwagon.urls')),
+
     # Users
     ('', include('users.urls')),