Merge lp:~canonical-isd-hackers/ubuntu-webcatalog/useragent_778562 into lp:ubuntu-webcatalog

[Danny Tamez]

Enhances the application details page to show more helpful messages based on the useragent of the request.

This branch adds support for the application details page to display more helpful messages. In particular, the useragent in the request is examined to determine the user's OS and distroseries. Based on this information the app details page will display alternative links to other distros or a link to the ubuntu download page if the OS detected is not Linux. At this point there is no way to actually determine the distro the user is running so a default guess is used (which is configured in the settings).

[David Owen (dsowen)]

I really like how you factored the tests!

Approved, with recommendation that Linux fields not be set in get_user_os() when not a Linux browser.

review: Approve

[ISD Branch Mangler (isd-branches-mangler)]

Attempt to merge into lp:ubuntu-webcatalog failed due to conflicts:

text conflict in src/webcatalog/models.py
text conflict in src/webcatalog/templates/webcatalog/department_overview.html
text conflict in src/webcatalog/templates/webcatalog/search_results.html
text conflict in src/webcatalog/urls.py
text conflict in src/webcatalog/views.py

[Michael Nelson (michael.nelson)]

Hey Danny! I was just pulling this up to find out why it hadn't landed yet (argh, conflicts), but noticed that you're not grabbing the distroseries from the user-agent when present... just wondering why?

Here's a screenshot showing Ubuntu/11.04 for me with chromium:
http://people.canonical.com/~michaeln/tmp/778562-headers.png

It won't be available in all browsers, but those that are patched by ubuntu should have it.

I really don't think we should be guessing at all, as it could provide a very confusing user experience. Either we know their distroseries and take advantage of that, or we don't and do some baseline functionality.

[Michael Nelson (michael.nelson)]

> Here's a screenshot showing Ubuntu/11.04 for me with chromium:
> http://people.canonical.com/~michaeln/tmp/778562-headers.png
>
> It won't be available in all browsers, but those that are patched by ubuntu
> should have it.

I just found bug 709125 which says that it'll no longer be supported in FF, but that ubufox package can add it for certain websites (as they have done for apt.ubuntu.com.

If we did ask them to also include (eventually) the web catalog, would it be SRUd to older series? Hrm.

[lp:~canonical-isd-hackers/ubuntu-webcatalog/useragent_778562]

26. By Danny Tamez on 2011-06-28

Merged trunk and resolved conflicts.

=== modified file 'django_project/config/main.cfg'
--- django_project/config/main.cfg	2011-06-23 12:38:12 +0000
+++ django_project/config/main.cfg	2011-06-28 15:06:35 +0000
@@ -93,6 +93,7 @@
 serve_site_media = True
 sca_api_url = https://sc.staging.ubuntu.com/api/2.0/
 disk_apt_cache_location = /tmp/webcat_cache
+default_distro = natty
 
 [google]
 google_analytics_id = UA-1018242-24
 
=== modified file 'src/webcatalog/models.py'
--- src/webcatalog/models.py	2011-06-23 14:20:52 +0000
+++ src/webcatalog/models.py	2011-06-28 15:06:35 +0000
@@ -62,7 +62,7 @@
     popcon = models.IntegerField()
     channel = models.CharField(max_length=255, blank=True)
     screenshot_url = models.URLField(blank=True,
-        help_text="Only use this if it is other than the normal screenshot url.")
+      help_text="Only use this if it is other than the normal screenshot url.")
     mimetype = models.CharField(max_length=2048, blank=True)
     architectures = models.CharField(max_length=255, blank=True)
     keywords = models.CharField(max_length=255, blank=True)
@@ -73,6 +73,9 @@
     icon_name = models.CharField(max_length=255, blank=True)
     icon = models.ImageField(upload_to='icons/%Y/%m', max_length=200,
         null=True, blank=True)
+    for_purchase = models.BooleanField(default=False)
+    archive_id = models.CharField(max_length=64, null=True,
+         db_index=True, blank=True, unique=True)
 
     # Other desktop fields used by s-c
     # x-gnome-fullname
@@ -83,9 +86,6 @@
     # (using python-apt - as above we'll need access to info from different
     # series etc.)
     description = models.TextField(blank=True)
-    for_purchase = models.BooleanField(default=False)
-    archive_id = models.CharField(max_length=64, null=True,
-         db_index=True, blank=True, unique=True)
 
     def __unicode__(self):
         return u"{0} ({1})".format(self.name, self.package_name)
@@ -127,7 +127,7 @@
         else:
             crumbs = [{'name': 'Get Software', 'url': reverse('wc-index')}]
         crumbs.append({'name': self.name, 'url': reverse('wc-package-detail',
-            args=[self.package_name])})
+            args=[self.distroseries.code_name, self.package_name])})
         return crumbs
 
 class Department(models.Model):
 
=== modified file 'src/webcatalog/schema.py'
--- src/webcatalog/schema.py	2011-06-27 12:07:30 +0000
+++ src/webcatalog/schema.py	2011-06-28 15:06:35 +0000
@@ -47,6 +47,7 @@
     webcatalog.serve_site_media = BoolConfigOption(default=True)
     webcatalog.sca_api_url = StringConfigOption()
     webcatalog.disk_apt_cache_location = StringConfigOption()
+    webcatalog.default_distro = StringConfigOption()
     webcatalog.page_batch_size = IntConfigOption(default=20)
 
     google = ConfigSection()
 
=== modified file 'src/webcatalog/templates/webcatalog/application_detail.html'
--- src/webcatalog/templates/webcatalog/application_detail.html	2011-06-23 14:20:52 +0000
+++ src/webcatalog/templates/webcatalog/application_detail.html	2011-06-28 15:06:35 +0000
@@ -27,7 +27,16 @@
         <img src="http://screenshots.ubuntu.com/thumbnail-with-version/{{ application.package_name }}/ignored" />
       </div>
       <p>{{ application.description }}</p>
-      <a href="apt://{{ application.package_name }}" class="awesome">{%if application.for_purchase %}Install{% else %}Download{% endif %} {{ application.name }}</a>
+      {% 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 %}
     </div>
     <div class="license">
       <table>
 
=== modified file 'src/webcatalog/templates/webcatalog/application_overview_snippet.html'
--- src/webcatalog/templates/webcatalog/application_overview_snippet.html	2011-06-23 14:20:52 +0000
+++ src/webcatalog/templates/webcatalog/application_overview_snippet.html	2011-06-28 15:06:35 +0000
@@ -5,7 +5,7 @@
         <img src="{{ STATIC_URL }}images/noicon_32.png"/>
     {% endif %}
     <h3>
-    <a href="{% url wc-package-detail app.package_name %}">{{ app.name }}</a>
+    <a href="{% url wc-package-detail app.distroseries.code_name app.package_name %}">{{ app.name }}</a>
     </h3>
     <p>{{ app.comment }}</p>
 </div>
 
=== modified file 'src/webcatalog/tests/factory.py'
--- src/webcatalog/tests/factory.py	2011-05-06 09:46:17 +0000
+++ src/webcatalog/tests/factory.py	2011-06-28 15:06:35 +0000
@@ -24,7 +24,7 @@
 import os
 from itertools import count
 
-from django.core.files.images import ImageFile
+from django.contrib.auth.models import User
 from django.test import TestCase
 
 from webcatalog.models import (
@@ -76,7 +76,7 @@
 
     def make_application(self, package_name=None, name=None,
         comment=None, description=None, icon_name='', icon=None,
-        distroseries=None):
+        distroseries=None, arch='i686'):
         if name is None:
             name = self.get_unique_string(prefix='Readable Name')
         if package_name is None:
@@ -91,7 +91,7 @@
         return Application.objects.create(
             package_name=package_name, name=name, comment=comment,
             description=description, popcon=999, icon=icon,
-            icon_name=icon_name, distroseries=distroseries)
+            icon_name=icon_name, distroseries=distroseries, architectures=arch)
 
     def make_department(self, name, parent=None):
         return Department.objects.create(name=name, parent=parent)
 
=== modified file 'src/webcatalog/tests/test_models.py'
--- src/webcatalog/tests/test_models.py	2011-06-23 14:49:53 +0000
+++ src/webcatalog/tests/test_models.py	2011-06-28 15:06:35 +0000
@@ -65,7 +65,7 @@
         self.assertEquals([], list(app.departments.all()))
         expected = [{'name': 'Get Software', 'url': reverse('wc-index')},
             {'name': app.name, 'url': reverse('wc-package-detail',
-            args=[app.package_name])}]
+            args=[app.distroseries.code_name, app.package_name])}]
 
         self.assertEquals(expected, app.crumbs())
 
@@ -78,7 +78,7 @@
             {'name': dept.name, 'url': reverse('wc-department',
                 args=[dept.id])},
             {'name': app.name, 'url': reverse('wc-package-detail',
-                args=[app.package_name])}]
+                args=[app.distroseries.code_name, app.package_name])}]
 
         self.assertEquals(expected, app.crumbs())
 
@@ -93,7 +93,7 @@
             {'name': dept.name, 'url': reverse('wc-department',
                 args=[dept.id])},
             {'name': app.name, 'url': reverse('wc-package-detail',
-                args=[app.package_name])}]
+                args=[app.distroseries.code_name, app.package_name])}]
 
         self.assertEquals(expected, app.crumbs())
 
 
=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py	2011-06-27 13:39:39 +0000
+++ src/webcatalog/tests/test_views.py	2011-06-28 15:06:35 +0000
@@ -22,8 +22,12 @@
     with_statement,
     )
 
+from django.conf import settings
 from django.core.urlresolvers import reverse
 
+from mock import patch
+
+from webcatalog.models import DistroSeries
 from webcatalog.tests.factory import TestCaseWithFactory
 from webcatalog.tests.helpers import patch_settings
 
@@ -35,23 +39,52 @@
     ]
 
 
+WINDOWS_USERAGENT = ('Mozilla/5.0 (Windows NT 5.1; rv:2.0) '
+                        'Gecko/20100101 Firefox/4.0')
+
+UBUNTU_USERAGENT = ('Mozilla/5.0 (X11; Linux x86_64; rv:2.0.1) '
+                    'Gecko/20100101 Firefox/4.0.1')
+
+
 class PackageDetailTestCase(TestCaseWithFactory):
 
+    def get_app_and_response(self, code_name='natty', version='11.04',
+                                        arch='x86_64', name=None, comment=None,
+                                        description=None, detail_distro=None,
+                                        detail_package=None, for_purchase=False,
+                                        useragent=UBUNTU_USERAGENT):
+        series = DistroSeries(code_name=code_name, version=version)
+        series.save()
+        app = self.factory.make_application(package_name='pkgfoo',
+                                            distroseries=series, arch=arch,
+                                            name=name, comment=comment,
+                                            description=description)
+        if for_purchase:
+            app.for_purchase = True
+            app.save()
+        if not detail_distro:
+            detail_distro = app.distroseries.code_name
+        if not detail_package:
+            detail_package = app.package_name
+
+        url = reverse('wc-package-detail', args=[detail_distro, detail_package])
+
+        if useragent:
+            response = self.client.get(url, HTTP_USER_AGENT=useragent)
+        else:
+            response = self.client.get(url)
+        return response, app
+
     def test_renders_correct_template(self):
-        self.factory.make_application(package_name='pkgfoo')
-        url = reverse('wc-package-detail', args=['pkgfoo'])
-
-        response = self.client.get(url)
+        response, app = self.get_app_and_response()
 
         self.assertTemplateUsed(response, 'webcatalog/application_detail.html')
 
     def test_includes_application_details(self):
         # The details of an application are included in the rendered html.
-        self.factory.make_application(package_name='pkgfoo', name="My app foo",
-            comment="The best app eva.", description="A long description.")
-        url = reverse('wc-package-detail', args=['pkgfoo'])
-
-        response = self.client.get(url)
+        response, app = self.get_app_and_response(name='My app foo',
+                                            comment='The best app eva.',
+                                            description='A long description.')
 
         self.assertContains(response, "My app foo")
         self.assertContains(response, "The best app eva.")
@@ -59,40 +92,83 @@
 
     def test_screenshot_thumbnail_url(self):
         # The thumbnail link to screenshots is included.
-        self.factory.make_application(package_name='pkgfoo')
-        url = reverse('wc-package-detail', args=['pkgfoo'])
-
-        response = self.client.get(url)
+        response, app = self.get_app_and_response()
 
         self.assertContains(
             response, '<img src="http://screenshots.ubuntu.com/'
                       'thumbnail-with-version/pkgfoo/ignored"')
 
     def test_link_to_apt_package(self):
-        self.factory.make_application(package_name='pkgfoo')
-        url = reverse('wc-package-detail', args=['pkgfoo'])
-
-        response = self.client.get(url)
-
-        self.assertContains( response, '<a href="apt://pkgfoo"')
+        response, app = self.get_app_and_response()
+
+        self.assertContains(response, '<a href="apt://pkgfoo"')
 
     def test_button_for_non_puchase_app(self):
-        app = self.factory.make_application(package_name='pkgfoo')
-        url = reverse('wc-package-detail', args=['pkgfoo'])
-
-        response = self.client.get(url)
-
-        self.assertContains( response, 'Download %s' % app.name)
+        response, app = self.get_app_and_response()
+
+        self.assertContains(response, 'Install %s' % app.name)
+        self.assertNotContains(response, 'Purcchase %s' % app.name)
 
     def test_button_for_for_puchase_app(self):
-        app = self.factory.make_application(package_name='pkgfoo')
-        app.for_purchase = True
-        app.save()
-        url = reverse('wc-package-detail', args=['pkgfoo'])
-
-        response = self.client.get(url)
-
-        self.assertContains( response, 'Install %s' % app.name)
+        response, app = self.get_app_and_response(for_purchase=True)
+
+        self.assertContains(response, 'Purchase %s' % app.name)
+        self.assertNotContains(response, 'Install %s' % app.name)
+
+    def test_button_for_matching_distroseries(self):
+        response, app = self.get_app_and_response(for_purchase=True)
+
+        self.assertContains(response, 'Purchase %s' % app.name)
+        self.assertNotContains(response, 'Install %s' % app.name)
+
+    def test_button_for_non_matching_distroseries_but_available(self):
+        series = DistroSeries(code_name='natty', version='11.04')
+        series.save()
+        app2 = self.factory.make_application(package_name='pkgfoo',
+                                            distroseries=series,
+                                            arch='x86_64')
+        app2.save()
+        response, app = self.get_app_and_response(code_name='oneric',
+                                                  version='11.10',
+                                                  for_purchase=True,
+                                                  detail_distro='oneric')
+
+        self.assertContains(response, 'Not running %s?' %
+                            settings.DEFAULT_DISTRO)
+        self.assertContains(response, 'Purchase %s for oneric' % app.name)
+        self.assertContains(response, '/cat/applications/oneric/pkgfoo/')
+
+    @patch('webcatalog.views.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
+        # otherwise we'll get the 'Not using natty?' message
+        fake = {'is_linux': True, 'distro': 'natty', 'distro_is_a_guess': False,
+                'arch': 'x86_64'}
+        mock_get_user_os.return_value = fake
+        response, app = self.get_app_and_response(code_name='oneric',
+                                                  version='11.10',
+                                                  for_purchase=True,
+                                                  detail_distro='oneric')
+
+        self.assertNotContains(response, 'Purchase %s' % app.name)
+        not_avail_msg = 'Not available for your version of Ubuntu'
+        self.assertContains(response, not_avail_msg)
+
+    def test_button_for_non_matching_platform(self):
+        response, app = self.get_app_and_response(useragent=WINDOWS_USERAGENT)
+
+        self.assertNotContains(response, 'Purchase %s' % app.name)
+        self.assertContains(response, 'Not available for your platform')
+        self.assertContains(response, 'http://www.ubuntu.com/download')
+        self.assertContains(response, 'Download Ubuntu')
+
+    def test_app_detail_with_no_useragent(self):
+        # don't set the useragent
+        response, app = self.get_app_and_response(useragent=None)
+
+        self.assertNotContains(response, 'Purchase %s' % app.name)
+
 
 
 class SearchTestCase(TestCaseWithFactory):
@@ -201,6 +277,7 @@
         self.assertEqual(1, page.number)
 
 
+
 class OverviewTestCase(TestCaseWithFactory):
     def test_index_contains_links_to_departments(self):
         dept = self.factory.make_department('foo')
@@ -227,7 +304,7 @@
         response = self.client.get(reverse('wc-department', args=[dept.id]))
 
         self.assertContains(response, reverse('wc-package-detail',
-            args=[app.package_name]))
+            args=[app.distroseries.code_name, app.package_name]))
 
     def test_department_with_no_subdepts_doesnt_contain_header(self):
         dept = self.factory.make_department('bar')
 
=== modified file 'src/webcatalog/urls.py'
--- src/webcatalog/urls.py	2011-06-23 14:20:52 +0000
+++ src/webcatalog/urls.py	2011-06-28 15:06:35 +0000
@@ -22,9 +22,8 @@
     with_statement,
     )
 from django.conf.urls.defaults import patterns, url
-from django.views.generic.detail import DetailView
 
-from webcatalog.models import Application
+from webcatalog.views import application_detail, application_detail_no_distro
 
 __metaclass__ = type
 __all__ = [
@@ -36,7 +35,9 @@
     url(r'^$', 'index', name='wc-index'),
     url(r'^department/(?P<dept_id>\d+)/$', 'department_overview',
         name='wc-department'),
-    url(r'^applications/(?P<package_name>[-.+\w]+)/$', 'application_detail',
-        name="wc-package-detail"),
+    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'^search/$', 'search', name="wc-search"),
 )
 
=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2011-06-27 14:08:41 +0000
+++ src/webcatalog/views.py	2011-06-28 15:06:35 +0000
@@ -55,6 +55,7 @@
         return int(page_num)
     return 1
 
+
 def search(request):
     apps = []
     query = ''
@@ -82,12 +83,14 @@
     return render_to_response('webcatalog/search_results.html',
         context_instance=context)
 
+
 def index(request):
     depts = Department.objects.filter(parent=None).order_by('name')
     context = RequestContext(request, dict={'depts': depts})
     return render_to_response('webcatalog/index.html',
         context_instance=context)
 
+
 def department_overview(request, dept_id):
     dept = get_object_or_404(Department, pk=dept_id)
     subdepts = Department.objects.filter(parent=dept)
@@ -104,12 +107,77 @@
         context_instance=context)
 
 
-def application_detail(request, package_name):
-    app = get_object_or_404(Application, package_name=package_name)
-
-    context = RequestContext(request, {
-        'application': app,
-        'breadcrumbs': app.crumbs(),
-        })
-    return render_to_response('webcatalog/application_detail.html',
-        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,
+                            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):
+    os = {'is_linux': False, 'distro': 'unknown', 'arch': 'unknown'}
+    try:
+        ua = request.META['HTTP_USER_AGENT']
+    except KeyError:
+        return os
+    os['is_linux'] = ua.find('X11; Linux') > -1
+    # Until we can figure out a way to actually get the distro we'll
+    # assume a default distro
+    if os['is_linux']:
+        os['distro'] = settings.DEFAULT_DISTRO
+        os['distro_is_a_guess'] = True
+    else:
+        os['distro'] = ''
+        os['distro_is_a_guess'] = False
+
+    is_x86_64 = ua.find('x86_64')
+    is_686 = ua.find('i686')
+    if is_x86_64:
+        os['arch'] = 'x86_64'
+    elif is_686:
+        os['arch'] = 'i686'
+    # TODO: DT Get other architectures
+    return os