Merge lp:~michael.nelson/ubuntu-webcatalog/788210-navigate-distroseries into lp:ubuntu-webcatalog

[Michael Nelson]

Redirect to series-specific app if series not provided (and refactor view code into template tag).

Overview
========
This branch does part of bug 788210 - redirecting to an app for a specific distroseries when we know the users distroseries, but is mostly moving existing code around to get ready for the next branch.

Details
=======
 * Moved the fat from the application_detail view into a template tag
 * Refactored the urls, removing the need for a separate application_detail_no_series view (and reordered the args for the view so the resource is consistent)
 * Added a context processor to include the user-agent in the request context (so it can be used in a template tag).

To test: follow the readme to bootstrap and then `fab test`

This branch is landable as is, but in the following branch I plan to:
 * Move the current view tests for the install options into template tag tests (and add some tests - it's telling me an app is not available for my distroseries while displaying the app for my distroseries)
 * Refactor the get_user_os method to hopefully make it a bit more readable (removing distro_is_a_guess if possible)
 * Add the list of other distroseries to the templates (second part of bug 788210)
 * Fix an issue where currently we blindly assume that apps exist for our default distroseries... we should instead be finding the app with the highest released distroseries version (if a specific default is not set)
 * Related to the last one, update the urls to use distroseries *versions* for released distroseries and codenames for unreleased series (consistent with ubuntu convention).

[Michael Foord (mfoord)]

review: Approve

=== modified file 'django_project/config/main.cfg'
--- django_project/config/main.cfg	2011-06-28 15:03:12 +0000
+++ django_project/config/main.cfg	2011-06-29 09:42:34 +0000
@@ -49,6 +49,7 @@
                               django.core.context_processors.static
                               django.contrib.messages.context_processors.messages
                               webcatalog.context_processors.google_analytics_id
+                              webcatalog.context_processors.user_agent
 
 template_dirs = django_project/templates/
 static_root = ./django_project/static/

=== modified file 'src/webcatalog/context_processors.py'
--- src/webcatalog/context_processors.py	2011-06-18 23:17:11 +0000
+++ src/webcatalog/context_processors.py	2011-06-29 09:42:34 +0000
@@ -24,3 +24,8 @@
     return {
         'google_analytics_id': getattr(settings, 'GOOGLE_ANALYTICS_ID', None),
         }
+
+
+def user_agent(request):
+    """Adds the browser user-agent string to the context if present."""
+    return dict(user_agent=request.META.get('HTTP_USER_AGENT', ''))

=== modified file 'src/webcatalog/templates/webcatalog/application_detail.html'
--- src/webcatalog/templates/webcatalog/application_detail.html	2011-06-28 15:03:12 +0000
+++ src/webcatalog/templates/webcatalog/application_detail.html	2011-06-29 09:42:34 +0000
@@ -1,5 +1,6 @@
 {% extends "webcatalog/base.html" %}
 {% load i18n %}
+{% load webcatalog %}
 
 {% block title %}Application {{ application.name }}{% endblock %}
 {% block header %}Details for {{ application.name }}{% endblock %}
@@ -27,16 +28,7 @@
         <img src="http://screenshots.ubuntu.com/thumbnail-with-version/{{ application.package_name }}/ignored" />
       </div>
       <p>{{ application.description }}</p>
-      {% if hide_button %}
-        {{ message_text }}
-      {% else %}
-        <a href="apt://{{ application.package_name }}" class="awesome">{{ button_text }} {{ application.name }}</a>
-      {% endif %}
-      {% if links %}
-        {% for text, link in links %}
-            <br><a href="{{link}}" class="awesome">{{ text }}</a>
-        {% endfor %}
-      {% endif %}
+      {% install_options application %}
     </div>
     <div class="license">
       <table>

=== added file 'src/webcatalog/templates/webcatalog/install_options_snippet.html'
--- src/webcatalog/templates/webcatalog/install_options_snippet.html	1970-01-01 00:00:00 +0000
+++ src/webcatalog/templates/webcatalog/install_options_snippet.html	2011-06-29 09:42:34 +0000
@@ -0,0 +1,11 @@
+{% if hide_button %}
+{{ message_text }}
+{% else %}
+<a href="apt://{{ application.package_name }}" class="awesome">{{ button_text }} {{ application.name }}</a>
+{% endif %}
+{% if links %}
+{% for text, link in links %}
+    <br><a href="{{link}}" class="awesome">{{ text }}</a>
+{% endfor %}
+{% endif %}
+

=== added directory 'src/webcatalog/templatetags'
=== added file 'src/webcatalog/templatetags/__init__.py'
=== added file 'src/webcatalog/templatetags/webcatalog.py'
--- src/webcatalog/templatetags/webcatalog.py	1970-01-01 00:00:00 +0000
+++ src/webcatalog/templatetags/webcatalog.py	2011-06-29 09:42:34 +0000
@@ -0,0 +1,81 @@
+#   -*- coding: utf-8 -*-
+#   This file is part of the Ubuntu Web Catalog
+#   Copyright (C) 2011 Canonical Ltd.
+#
+#   This program is free software: you can redistribute it and/or modify
+#   it under the terms of the GNU Affero General Public License as
+#   published by the Free Software Foundation, either version 3 of the
+#   License, or (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful,
+#   but WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#   GNU Affero General Public License for more details.
+#
+#   You should have received a copy of the GNU Affero General Public License
+#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Custom template tags for the webcatalog app."""
+
+from __future__ import (
+    absolute_import,
+    with_statement,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'install_options',
+    ]
+
+from django import template
+from django.core.urlresolvers import reverse
+
+from webcatalog.models import Application
+from webcatalog.views import get_user_os
+
+register = template.Library()
+
+
[e-mail]_tag('webcatalog/install_options_snippet.html',
+                        takes_context=True)
+def install_options(context, application):
+    app = application
+    os = get_user_os(context['user_agent'])
+    button_text = 'Purchase' if app.for_purchase else 'Install'
+    right_platform = os['is_linux']
+    right_arch = app.architectures.find(os['arch']) > -1
+    right_distro = os['distro'].find(app.distroseries.code_name) > -1
+    perfect_match = (right_platform and right_distro  and right_arch)
+    message_text = 'Not available for your version of Ubuntu'
+    links = []
+    if not perfect_match:
+        if not right_platform:
+            message_text = "Not available for your platform"
+            links.append(('Download Ubuntu', 'http://www.ubuntu.com/download'))
+        elif not right_distro and os['distro_is_a_guess']:
+            # see if we have this package for *other* distros
+            apps_found = Application.objects.filter(
+                package_name=application.package_name).exclude(
+                    distroseries__code_name=os['distro'])
+            if apps_found:
+                message_text = ('Not running %s?' % os['distro'])
+                for app_found in apps_found:
+                    args = [app.distroseries.code_name, app.package_name]
+                    url = reverse('wc-package-detail', args=args)
+                    text = "%s %s for %s" % (button_text, app.name,
+                                          app_found.distroseries.code_name)
+                    links.append((text, url))
+        elif not right_distro:
+            app_found = Application.objects.filter(
+                package_name=application.package_name,
+                distroseries__code_name=os['distro'])
+            if app_found:
+                args = [app.distroseries.code_name, app.package_name]
+                url = reverse('wc-package-detail', args=args)
+                message_text = ('%s is also available for your version '
+                                'of Ubuntu' % app.name)
+                links.append(("%s for os['distro']" % button_text, url))
+
+    return dict(application=application, button_text=button_text,
+        hide_button=not perfect_match, message_text=message_text,
+        links=links)

=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py	2011-06-28 18:17:25 +0000
+++ src/webcatalog/tests/test_views.py	2011-06-29 09:42:34 +0000
@@ -33,8 +33,9 @@
 
 __metaclass__ = type
 __all__ = [
+    'ApplicationDetailNoSeriesTestCase',
+    'ApplicationDetailTestCase',
     'OverviewTestCase',
-    'PackageDetailTestCase',
     'SearchTestCase',
     ]
 
@@ -46,7 +47,7 @@
                     'Gecko/20100101 Firefox/4.0.1')
 
 
-class PackageDetailTestCase(TestCaseWithFactory):
+class ApplicationDetailTestCase(TestCaseWithFactory):
 
     def get_app_and_response(self, code_name='natty', version='11.04',
                                         arch='x86_64', name=None, comment=None,
@@ -67,7 +68,7 @@
         if not detail_package:
             detail_package = app.package_name
 
-        url = reverse('wc-package-detail', args=[detail_distro, detail_package])
+        url = reverse('wc-package-detail', args=[detail_package, detail_distro])
 
         if useragent:
             response = self.client.get(url, HTTP_USER_AGENT=useragent)
@@ -138,7 +139,7 @@
         self.assertContains(response, 'Purchase %s for oneric' % app.name)
         self.assertContains(response, '/cat/applications/oneric/pkgfoo/')
 
-    @patch('webcatalog.views.get_user_os')
+    @patch('webcatalog.templatetags.webcatalog.get_user_os')
     def test_button_for_non_matching_distroseries_and_unavailable(self,
                                                         mock_get_user_os):
         # patch user agent check so we can have distro is a guess be False
@@ -190,6 +191,49 @@
                             settings.DEFAULT_DISTRO)
 
 
+class ApplicationDetailNoSeriesTestCase(TestCaseWithFactory):
+
+    def test_defaults_to_specified_series(self):
+        # If a distroseries is not included in the url,
+        # and we cannot determine it from the user agent,
+        # we redirect to the one specified in the configuration.
+        natty = self.factory.make_distroseries(code_name='natty')
+        lucid = self.factory.make_distroseries(code_name='lucid')
+        natty_app = self.factory.make_application(
+            package_name='pkgfoo', distroseries=natty)
+        lucid_app = self.factory.make_application(
+            package_name='pkgfoo', distroseries=lucid)
+
+        for default_distro in ('natty', 'lucid'):
+            with patch_settings(DEFAULT_DISTRO=default_distro):
+                url = reverse('wc-package-detail', args=['pkgfoo'])
+                response = self.client.get(url)
+
+                self.assertRedirects(
+                    response, reverse(
+                        'wc-package-detail', args=['pkgfoo', default_distro]))
+
+    def test_redirects_to_ua_distroseries(self):
+        # If a distroseries is not included in the url, but we
+        # know it from the user agent, and the app exists for that
+        # distro series also, we redirect there.
+        natty = self.factory.make_distroseries(code_name='natty')
+        lucid = self.factory.make_distroseries(code_name='lucid')
+        natty_app = self.factory.make_application(
+            package_name='pkgfoo', distroseries=natty)
+        lucid_app = self.factory.make_application(
+            package_name='pkgfoo', distroseries=lucid)
+
+        url = reverse('wc-package-detail', args=['pkgfoo'])
+        with patch_settings(DEFAULT_DISTRO='natty'):
+            response = self.client.get(
+                url, HTTP_USER_AGENT='blah X11; Linux Ubuntu/10.04 blah blah')
+
+        self.assertRedirects(
+            response, reverse(
+                'wc-package-detail', args=['pkgfoo', 'lucid']))
+
+
 class SearchTestCase(TestCaseWithFactory):
     def test_no_search_retrieves_no_apps(self):
         # Ensure there's some data in the DB

=== modified file 'src/webcatalog/urls.py'
--- src/webcatalog/urls.py	2011-06-28 15:03:12 +0000
+++ src/webcatalog/urls.py	2011-06-29 09:42:34 +0000
@@ -23,8 +23,6 @@
     )
 from django.conf.urls.defaults import patterns, url
 
-from webcatalog.views import application_detail, application_detail_no_distro
-
 __metaclass__ = type
 __all__ = [
     'urlpatterns',
@@ -35,9 +33,9 @@
     url(r'^$', 'index', name='wc-index'),
     url(r'^department/(?P<dept_id>\d+)/$', 'department_overview',
         name='wc-department'),
-    url(r'^applications/(?P<distro>[-.+\w]+)/(?P<slug>[-.+\w]+)/$',
-        application_detail, name="wc-package-detail"),
-    url(r'^applications/(?P<slug>[-.+\w]+)/$', application_detail_no_distro,
-        name="wc-package-detail-no-distro"),
+    url(r'^applications/(?P<package_name>[-.+\w]+)/(?P<distro>[-.+\w]+)/$',
+        'application_detail', name="wc-package-detail"),
+    url(r'^applications/(?P<package_name>[-.+\w]+)/$', 'application_detail',
+        name="wc-package-detail"),
     url(r'^search/$', 'search', name="wc-search"),
 )

=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2011-06-28 18:31:28 +0000
+++ src/webcatalog/views.py	2011-06-29 09:42:34 +0000
@@ -29,6 +29,7 @@
 from django.core.paginator import Paginator
 from django.core.urlresolvers import reverse
 from django.db.models import Q
+from django.http import HttpResponseRedirect
 from django.template import RequestContext
 from django.shortcuts import (
     get_object_or_404,
@@ -107,67 +108,42 @@
         context_instance=context)
 
 
-def application_detail_no_distro(request, slug):
-    default_distro = settings.DEFAULT_DISTRO
-    return application_detail(request, default_distro, slug)
-
-
-def application_detail(request, distro, slug):
-    os = get_user_os(request)
-    app = get_object_or_404(Application, package_name=slug,
+def application_detail(request, package_name, distro=None):
+    os = get_user_os(request.META.get('HTTP_USER_AGENT', ''))
+    if distro is None:
+        # Check for the distroseries in the useragent, if we have it,
+        # redirect there.
+        if not os['distro_is_a_guess'] and os['distro']:
+            distro = os['distro']
+        else:
+            distro = settings.DEFAULT_DISTRO
+        return HttpResponseRedirect(
+            reverse('wc-package-detail',
+                    args=[package_name, distro]))
+
+    app = get_object_or_404(Application, package_name=package_name,
                             distroseries__code_name=distro)
-    button_text = 'Purchase' if app.for_purchase else 'Install'
-    right_platform = os['is_linux']
-    right_arch = app.architectures.find(os['arch']) > -1
-    right_distro = os['distro'].find(app.distroseries.code_name) > -1
-    perfect_match = (right_platform and right_distro  and right_arch)
-    message_text = 'Not available for your version of Ubuntu'
-    links = []
-    if not perfect_match:
-        if not right_platform:
-            message_text = "Not available for your platform"
-            links.append(('Download Ubuntu', 'http://www.ubuntu.com/download'))
-        elif not right_distro and os['distro_is_a_guess']:
-            # see if we have this package for *other* distros
-            apps_found = Application.objects.filter(package_name=slug).exclude(
-                                        distroseries__code_name=os['distro'])
-            if apps_found:
-                message_text = ('Not running %s?' % os['distro'])
-                for app_found in apps_found:
-                    args = [app.distroseries.code_name, app.package_name]
-                    url = reverse('wc-package-detail', args=args)
-                    text = "%s %s for %s" % (button_text, app.name,
-                                          app_found.distroseries.code_name)
-                    links.append((text, url))
-        elif not right_distro:
-            app_found = Application.objects.filter(package_name=slug,
-                                        distroseries__code_name=os['distro'])
-            if app_found:
-                args = [app.distroseries.code_name, app.package_name]
-                url = reverse('wc-package-detail', args=args)
-                message_text = ('%s is also available for your version '
-                                'of Ubuntu' % app.name)
-                links.append(("%s for os['distro']" % button_text, url))
-
-    ctx = {'application': app, 'button_text': button_text,
-           'hide_button': not perfect_match, 'message_text': message_text,
-            'breadcrumbs': app.crumbs(), 'links': links}
-
-    context = RequestContext(request, dict=ctx)
-    return render_to_response('webcatalog/application_detail.html', context)
-
-
-def get_user_os(request):
+
+    return render_to_response(
+        'webcatalog/application_detail.html', RequestContext(
+            request, dict(application=app, breadcrumbs=app.crumbs())))
+
+
+def get_user_os(user_agent_string):
     distros = {
         '10.04': 'lucid',
         '10.10': 'maverick',
         '11.04': 'natty',
         '11.10': 'oneiric',
     }
-    os = {'is_linux': False, 'distro': 'unknown', 'arch': 'unknown'}
-    try:
-        ua = request.META['HTTP_USER_AGENT']
-    except KeyError:
+    os = {
+        'is_linux': False,
+        'distro': '',
+        'arch': '',
+        'distro_is_a_guess': False,
+        }
+    ua = user_agent_string
+    if not ua:
         return os
     os['is_linux'] = ua.find('X11; Linux') > -1
     if os['is_linux']: