Merge lp:~michael.nelson/ubuntu-webcatalog/788207-display-reviews-non-js into lp:ubuntu-webcatalog

[Michael Nelson]

Provide a non-js view displaying reviews for an app.

Overview
========

This branch is the first step towards loading reviews live into the app details page (bug 788207).

It provides a non-js page displaying the first page of reviews for an app.

The next branches will:
 * Add a custom paginator object for use on the reviews page,
 * Extract just the snippet with the reviews to a separate template, which can then be rendered on its own when requested via JS.
 * Add the JS to progressively enhance the details page (without js, just a link to reviews, but js will replace that with actual reviews).

To test: `fab bootstrap && fab test`

Screenshot on the bug.

I also updated the import ratings stats management command so that we pull stats for all origins (as currenly we only support ubuntu and commercial PPAs afaik), as otherwise we don't get ratings for commercial apps.

=== modified file '.bzrignore'
--- .bzrignore	2011-08-15 11:13:35 +0000
+++ .bzrignore	2011-09-15 11:41:49 +0000
@@ -12,3 +12,4 @@
 sourcecode/django/
 django_project/rnrclient.py
 django_project/adminaudit
+django_project/local.cfg

=== modified file 'django_project/config/main.cfg'
--- django_project/config/main.cfg	2011-09-12 18:51:47 +0000
+++ django_project/config/main.cfg	2011-09-15 11:41:49 +0000
@@ -99,6 +99,7 @@
 
 [rnr]
 rnr_service_root = http://reviews.ubuntu.com/reviews/api/1.0
+reviews_cache_timeout = 900
 
 [sso_api]
 sso_api_service_root = https://login.staging.ubuntu.com/api/1.0

=== modified file 'src/webcatalog/management/commands/import_for_purchase_apps.py'
--- src/webcatalog/management/commands/import_for_purchase_apps.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/management/commands/import_for_purchase_apps.py	2011-09-15 11:41:49 +0000
@@ -99,5 +99,3 @@
                 self.stdout.write(message)
             if flush:
                 self.stdout.flush()
-
-    
\ No newline at end of file

=== modified file 'src/webcatalog/management/commands/import_ratings_stats.py'
--- src/webcatalog/management/commands/import_ratings_stats.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/management/commands/import_ratings_stats.py	2011-09-15 11:41:49 +0000
@@ -30,13 +30,12 @@
 from django.conf import settings
 from django.core.management.base import LabelCommand
 
-from rnrclient import RatingsAndReviewsAPI
-
 from webcatalog.models import (
     Application,
     DistroSeries,
     ReviewStatsImport,
     )
+from webcatalog.utilities import WebServices
 
 
 class Command(LabelCommand):
@@ -93,8 +92,10 @@
             elif time_since_last_import < timedelta(days=7):
                 num_days = 7
 
-        rnr_api = RatingsAndReviewsAPI(service_root=settings.RNR_SERVICE_ROOT)
-        kwargs = dict(origin='ubuntu', distroseries=distroseries.code_name)
+        # Currently we have ratings in rnr for ubuntu and for-purchase
+        # PPAs only, so we set origin to any. If we later support reviews
+        # for general PPAs, we will need to update this.
+        kwargs = dict(origin='any', distroseries=distroseries.code_name)
         if num_days:
             kwargs['days'] = num_days
-        return rnr_api.review_stats(**kwargs)
+        return WebServices().rnr_api.review_stats(**kwargs)

=== modified file 'src/webcatalog/schema.py'
--- src/webcatalog/schema.py	2011-09-12 18:44:56 +0000
+++ src/webcatalog/schema.py	2011-09-15 11:41:49 +0000
@@ -75,6 +75,7 @@
     rnr = ConfigSection()
     rnr.rnr_service_root = StringConfigOption(
         default="http://reviews.ubuntu.com/reviews/api/1.0")
+    rnr.reviews_cache_timeout = IntConfigOption(default=60*15)
 
     sso_api = ConfigSection()
     sso_api.sso_api_service_root = StringConfigOption()

=== modified file 'src/webcatalog/static/css/webcatalog.css'
--- src/webcatalog/static/css/webcatalog.css	2011-08-15 11:34:36 +0000
+++ src/webcatalog/static/css/webcatalog.css	2011-09-15 11:41:49 +0000
@@ -206,9 +206,12 @@
     width: 64px;
     height: 64px;
 }
-.star-rating {
+.star-rating.small {
     width: 100px;
+}
+.star-rating.average {
     float: right;
+    display: block;
 }
 .star-rating.large {
     width: 175px;
@@ -217,6 +220,24 @@
     margin: 0;
     float: none;
 }
+.review {
+    margin: 8px 8px 16px 8px;
+}
+.review .star-rating {
+    display: inline;
+    margin-right: 8px; 
+}
+.review p {
+    margin-top: 10px;
+    font-size: 14px;
+}
+.review .meta {
+    float: right;
+    color: #adaaa6;
+}
+.review p.usefulness {
+    font-size: 12px;
+}
 
 /* Breadcrumbs styles */
 #breadcrumbs {

=== modified file 'src/webcatalog/templates/webcatalog/application_detail.html'
--- src/webcatalog/templates/webcatalog/application_detail.html	2011-08-15 11:28:26 +0000
+++ src/webcatalog/templates/webcatalog/application_detail.html	2011-09-15 11:41:49 +0000
@@ -24,7 +24,7 @@
     </div>
   <div id="sc-mockup">
     <div class="header">
-      {% rating_summary application 'large' %}
+      {% rating_summary application.ratings_average 'large' application.ratings_total %}
       {% if application.icon %}
           <img class="icon64" src="{{ application.icon.url }}"/>
       {% else %}

=== modified file 'src/webcatalog/templates/webcatalog/application_overview_snippet.html'
--- src/webcatalog/templates/webcatalog/application_overview_snippet.html	2011-07-21 11:31:10 +0000
+++ src/webcatalog/templates/webcatalog/application_overview_snippet.html	2011-09-15 11:41:49 +0000
@@ -16,7 +16,7 @@
         <p>{{ app.comment }}</p>
     </td>
     <td class="app-stats">
-      {% rating_summary app %}
+      {% rating_summary app.ratings_average 'small' app.ratings_total %}
     </td>
   </tr>
 {% endfor %}

=== added file 'src/webcatalog/templates/webcatalog/application_review_list.html'
--- src/webcatalog/templates/webcatalog/application_review_list.html	1970-01-01 00:00:00 +0000
+++ src/webcatalog/templates/webcatalog/application_review_list.html	2011-09-15 11:41:49 +0000
@@ -0,0 +1,39 @@
+{% extends "webcatalog/base.html" %}
+{% load i18n %}
+{% load webcatalog %}
+
+{% block title %}Reviews for {{ application.name }}{% endblock %}
+{% block header %}Reviews for for {{ application.name }}{% endblock %}
+
+{% block content %}
+    {% include "webcatalog/breadcrumbs_snippet.html" %}
+  <div id="sc-mockup">
+    <div class="header">
+      {% rating_summary application.ratings_average 'large' application.ratings_total %}
+      {% if application.icon %}
+          <img class="icon64" src="{{ application.icon.url }}"/>
+      {% else %}
+          <img class="icon64" src="{{ STATIC_URL }}images/applications-other-64.png"/>
+      {% endif %}
+
+      <h2>{{ application.name }}</h2>
+      <p>{{ application.comment }}</p>
+    </div>
+    {% for review in reviews %}
+      <div class="review">
+        <div class="meta">
+            <strong>{{ review.reviewer_displayname }}</strong>,
+            {{ review.date_created|slice:":10" }}
+        </div>
+          {% rating_summary review.rating %}
+          <strong>{{ review.summary }}</strong>
+        <p>{{ review.review_text }}</p>
+        <p class="usefulness">
+          {% blocktrans with favorable=review.usefulness_favorable total=review.usefulness_total %}
+          {{ favorable }} out of {{ total }} found this review helpful.
+          {% endblocktrans %}
+        </p>
+      </div>
+    {% endfor %}
+  </div>
+{% endblock %}

=== modified file 'src/webcatalog/templates/webcatalog/breadcrumbs_snippet.html'
--- src/webcatalog/templates/webcatalog/breadcrumbs_snippet.html	2011-06-23 14:20:52 +0000
+++ src/webcatalog/templates/webcatalog/breadcrumbs_snippet.html	2011-09-15 11:41:49 +0000
@@ -3,4 +3,4 @@
     <li class="crumb {% if forloop.first %} first{% endif %}{% if forloop.last %} last{% endif %}">
         <a href="{{ crumb.url }}">{{ crumb.name }}</a></li>
 {% endfor %}
-</ul>
\ No newline at end of file
+</ul>

=== modified file 'src/webcatalog/templates/webcatalog/rating_summary.html'
--- src/webcatalog/templates/webcatalog/rating_summary.html	2011-08-15 11:34:36 +0000
+++ src/webcatalog/templates/webcatalog/rating_summary.html	2011-09-15 11:41:49 +0000
@@ -1,9 +1,11 @@
 {% load i18n %}
-{% if total %}
-<div class="star-rating {{ size }}">
+{% if stars %}
+<div class="star-rating {{ size }}{% if total %} average{% endif %}">
   {% for star in stars %}
   <img src="{{ STATIC_URL }}images/star-{{ size }}-{{ star }}.png" />
   {% endfor %}
+  {% if total %}
   <p>{% blocktrans %}{{ total }} Ratings{% endblocktrans %}</p>
+  {% endif %}
 </div>
 {% endif %}

=== modified file 'src/webcatalog/templatetags/webcatalog.py'
--- src/webcatalog/templatetags/webcatalog.py	2011-09-12 13:58:47 +0000
+++ src/webcatalog/templatetags/webcatalog.py	2011-09-15 11:41:49 +0000
@@ -183,19 +183,19 @@
 
 
 @register.inclusion_tag("webcatalog/rating_summary.html")
-def rating_summary(app, size='small'):
+def rating_summary(num_stars, size='small', num_ratings=None):
     stars = []
-    if not app.ratings_average:
+    if not num_stars:
         return {}
 
-    ratings_average = float(app.ratings_average)
+    num_stars = float(num_stars)
     for count in range(1, 6):
-        if ratings_average >= count:
+        if num_stars >= count:
             stars.append(1)
-        elif 0.5 >= count - ratings_average > 0:
+        elif 0.5 >= count - num_stars > 0:
             stars.append(0.5)
         else:
             stars.append(0)
 
-    return dict(stars=stars, total=app.ratings_total,
+    return dict(stars=stars, total=num_ratings,
         STATIC_URL=settings.STATIC_URL, size=size)

=== modified file 'src/webcatalog/tests/test_commands.py'
--- src/webcatalog/tests/test_commands.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/tests/test_commands.py	2011-09-15 11:41:49 +0000
@@ -492,7 +492,7 @@
         call_command('import_ratings_stats', 'onion')
 
         self.mock_review_stats.assert_called_with(
-            origin='ubuntu', distroseries='onion')
+            origin='any', distroseries='onion')
 
     def test_download_review_stats_with_recent(self):
         # If there has been a recent import, we'll only grab the
@@ -505,7 +505,7 @@
         call_command('import_ratings_stats', 'onion')
 
         self.mock_review_stats.assert_called_with(
-            origin='ubuntu', distroseries='onion', days=3)
+            origin='any', distroseries='onion', days=3)
 
     def test_update_app_stats(self):
         # Any stats provided in the api call will be used to update our
@@ -541,4 +541,4 @@
         # update_apps_with_stats returns None on success:
         self.assertIsNone(command.update_apps_with_stats(natty, stats))
 
-        
\ No newline at end of file
+        

=== modified file 'src/webcatalog/tests/test_templatetags.py'
--- src/webcatalog/tests/test_templatetags.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/tests/test_templatetags.py	2011-09-15 11:41:49 +0000
@@ -296,10 +296,7 @@
 
 class RatingSummaryTestCase(TestCaseWithFactory):
     def test_rating_summary(self):
-        app = self.factory.make_application(ratings_average=Decimal('3.5'),
-            ratings_total=234)
-
-        result = rating_summary(app)
+        result = rating_summary('3.5', num_ratings=234)
 
         self.assertEqual([1, 1, 1, 0.5, 0], result['stars'])
         self.assertEqual(234, result['total'])

=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/tests/test_views.py	2011-09-15 11:41:49 +0000
@@ -28,6 +28,7 @@
 from django.core.urlresolvers import reverse
 
 from mock import patch
+from rnrclient import ReviewDetails
 
 from webcatalog.models import DistroSeries
 from webcatalog.tests.factory import TestCaseWithFactory
@@ -37,6 +38,7 @@
 __all__ = [
     'ApplicationDetailNoSeriesTestCase',
     'ApplicationDetailTestCase',
+    'ApplicationReviewsTestCase',
     'OverviewTestCase',
     'SearchTestCase',
     ]
@@ -504,3 +506,65 @@
         url = reverse('wc-department', args=['maverick', dept.id])
         maver_pos = response.content.find('<a href="{0}">Ubuntu'.format(url))
         self.assertTrue(lucid_pos > maver_pos)
+
+
+class ApplicationReviewsTestCase(TestCaseWithFactory):
+
+    def setUp(self):
+        super(ApplicationReviewsTestCase, self).setUp()
+        get_reviews_fn = 'rnrclient.RatingsAndReviewsAPI.get_reviews'
+        self.get_reviews_patcher = patch(get_reviews_fn)
+        self.mock_get_reviews = self.get_reviews_patcher.start()
+        self.mock_get_reviews.return_value = []
+
+    def tearDown(self):
+        self.get_reviews_patcher.stop()
+        super(ApplicationReviewsTestCase, self).tearDown()
+
+    def test_only_valid_apps(self):
+        response = self.client.get(reverse('wc-package-reviews',
+            args=['jaunty', 'doesntexist']))
+
+        self.assertEqual(404, response.status_code)
+
+    def test_uncached_calls_to_rnr_api(self):
+        app = self.factory.make_application()
+
+        response = self.client.get(reverse('wc-package-reviews',
+            args=[app.distroseries.code_name, app.package_name]))
+
+        self.assertEqual(1, self.mock_get_reviews.call_count)
+
+    def test_second_call_cached(self):
+        app = self.factory.make_application()
+
+        for count in range(2):
+            response = self.client.get(reverse('wc-package-reviews',
+                args=[app.distroseries.code_name, app.package_name]))
+
+        self.assertEqual(1, self.mock_get_reviews.call_count)
+
+    def make_review_details(self, package_name=None, summary=None,
+        rating='3.5'):
+        if package_name is None:
+            package_name = self.factory.get_unique_string(prefix='pkg-')
+        if summary is None:
+            summary = self.factory.make_unique_string(prefix='summary-')
+        return ReviewDetails.from_dict(
+            dict(package_name=package_name, summary=summary,
+                 rating=rating))
+
+    def test_renders_reviews(self):
+        app = self.factory.make_application()
+        self.mock_get_reviews.return_value = [
+            self.make_review_details(summary='review_summary1'),
+            self.make_review_details(summary='review_summary2'),
+            ]
+
+        response = self.client.get(reverse('wc-package-reviews',
+            args=[app.distroseries.code_name, app.package_name]))
+
+        self.assertTemplateUsed(
+            response, 'webcatalog/application_review_list.html')
+        self.assertContains(response, 'review_summary1')
+        self.assertContains(response, 'review_summary2')

=== modified file 'src/webcatalog/urls.py'
--- src/webcatalog/urls.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/urls.py	2011-09-15 11:41:49 +0000
@@ -37,8 +37,12 @@
         name='wc-department'),
     url(r'^applications/(?P<distro>[-.+\w]+)/(?P<package_name>[-.+\w]+)/$',
         'application_detail', name="wc-package-detail"),
+    # The following URL redirects to the series-specific URL above.
     url(r'^applications/(?P<package_name>[-.+\w]+)/$', 'application_detail',
         name="wc-package-detail"),
+    url(r'^applications/(?P<distro>[-.+\w]+)/(?P<package_name>[-.+\w]+)/'
+        r'reviews/$',
+        'application_reviews', name="wc-package-reviews"),
     url(r'^search/$', 'search', name="wc-search"),
     url(r'^search/(?P<distro>[-.+\w]+)/$', 'search', name="wc-search"),
 

=== modified file 'src/webcatalog/utilities.py'
--- src/webcatalog/utilities.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/utilities.py	2011-09-15 11:41:49 +0000
@@ -41,6 +41,9 @@
 from lazr.restfulclient.resource import ServiceRoot
 from oauth.oauth import OAuthToken
 
+from rnrclient import RatingsAndReviewsAPI
+
+
 class WebServiceError(Exception):
     """Raised when we cannot connect to a web service."""
 
@@ -87,6 +90,14 @@
 
         return self._identity_provider
 
+    @property
+    def rnr_api(self):
+        return RatingsAndReviewsAPI(service_root=settings.RNR_SERVICE_ROOT)
+
+    def get_reviews_for_package(self, package_name, distroseries, page=1):
+        return self.rnr_api.get_reviews(packagename=package_name, 
+            distroseries=distroseries, page=page)
+
     def _fake_validate_token(self, token, openid_identifier, signature):
         """ This is a version of validate_token that gets the
             token_secret, consumer_secret from the plaintext signature.

=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2011-09-12 13:37:24 +0000
+++ src/webcatalog/views.py	2011-09-15 11:41:49 +0000
@@ -30,22 +30,27 @@
 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,
     render_to_response,
     )
+from django.template import RequestContext
+from django.views.decorators.cache import cache_page
 
 from webcatalog.models import (
     Application,
     Department,
     DistroSeries,
     )
-from webcatalog.utilities import UserAgentString
+from webcatalog.utilities import (
+    UserAgentString,
+    WebServices,
+    )
 
 __metaclass__ = type
 __all__ = [
     'application_detail',
+    'application_reviews',
     'index',
     'department_overview',
     'search',
@@ -129,7 +134,7 @@
     dept = get_object_or_404(Department, pk=dept_id)
     subdepts = Department.objects.filter(parent=dept)
     subdepts = subdepts.order_by('name')
-    
+
     apps = Application.objects.filter(departments=dept,
         distroseries__code_name=distro)
     apps = apps.order_by('name')
@@ -172,3 +177,17 @@
     return render_to_response(
         'webcatalog/application_detail.html', RequestContext(
             request, atts))
+
+
+@cache_page(settings.REVIEWS_CACHE_TIMEOUT)
+def application_reviews(request, package_name, distro, page=1):
+    app = get_object_or_404(Application, package_name=package_name,
+                            distroseries__code_name=distro)
+    # XXX michaeln 2011-09-15 Don't request reviews for all languages.
+    reviews = WebServices().get_reviews_for_package(package_name, 
+        distroseries=distro, page=page)
+    return render_to_response(
+        'webcatalog/application_review_list.html', RequestContext(
+            request, dict(
+                application=app, distroseries=distro, reviews=reviews,
+                breadcrumbs=app.crumbs())))