Merge lp:~michael.nelson/ubuntu-webcatalog/788207-paginator into lp:ubuntu-webcatalog

[Michael Nelson]

Load reviews snippet via ajax and add to details page.

Overview
========
Contrary to the name, this doesn't add pagination for the reviews - that can wait :), instead it adds the ajax reviews to the app details page.

Some points:
 * I removed the caching on the reviews view and added it to the rnr api call - so that we can provide different responses with the one view for normal and json requests.
 * I'm pulling in the YUI components from the local server (STATIC_URL) rather than from yui (I'll land the YUI code in the following branch)[1]
 * Using just io-base and node-base seems to result in 14 requests - I'm assuming we should update to use a combo loader (actually, I'll check to see if there's standard combos already provided)

Test: `fab bootstrap && fab test`

[1] https://code.launchpad.net/~michael.nelson/ubuntu-webcatalog/788207-add-YUI-resources/+merge/75695

=== modified file 'src/webcatalog/static/css/webcatalog.css'
--- src/webcatalog/static/css/webcatalog.css	2011-09-15 10:53:40 +0000
+++ src/webcatalog/static/css/webcatalog.css	2011-09-16 09:53:23 +0000
@@ -220,6 +220,9 @@
     margin: 0;
     float: none;
 }
+.reviews {
+    padding: 16px;
+}
 .review {
     margin: 8px 8px 16px 8px;
 }
 
=== modified file 'src/webcatalog/templates/webcatalog/application_detail.html'
--- src/webcatalog/templates/webcatalog/application_detail.html	2011-09-15 10:53:40 +0000
+++ src/webcatalog/templates/webcatalog/application_detail.html	2011-09-16 09:53:23 +0000
@@ -4,6 +4,24 @@
 
 {% block title %}Application {{ application.name }}{% endblock %}
 {% block header %}Details for {{ application.name }}{% endblock %}
+{% block head_extra %}
+{{ block.super }}
+<script src="{{ STATIC_URL }}yui/3.4.0/build/yui/yui-min.js"></script>
+<script>
+YUI().use('io-base', 'node-base', function (Y) {
+    function complete(id, obj){
+        var reviews_html = obj.responseText;
+        var reviews_div = Y.one('#reviews_placeholder');
+        reviews_div.set("innerHTML", reviews_html);
+    };
+    Y.on('io:complete', complete, Y);
+
+    // XXX michaeln 2011-09-16 bug=851660 Autoload further batches of results.
+    var uri = "{% url wc-package-reviews application.distroseries.code_name application.package_name %}";
+    Y.io(uri);
+});
+</script>
+{% endblock %}
 
 {% block content %}
     {% include "webcatalog/breadcrumbs_snippet.html" %}
@@ -56,5 +74,12 @@
         </tr>
       </table>
     </div>
+    <div class="reviews">
+      <h2>{% trans "Reviews" %}</h2>
+      <div id="reviews_placeholder">
+      <a href="{% url wc-package-reviews application.distroseries.code_name application.package_name %}">
+          {% blocktrans %}View the reviews for this app{% endblocktrans %}</a>
+      </div>
+    </div>
   </div>
 {% endblock %}
 
=== modified file 'src/webcatalog/templates/webcatalog/application_review_list.html'
--- src/webcatalog/templates/webcatalog/application_review_list.html	2011-09-15 10:53:40 +0000
+++ src/webcatalog/templates/webcatalog/application_review_list.html	2011-09-16 09:53:23 +0000
@@ -19,21 +19,6 @@
       <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 %}
+    {% include "webcatalog/application_review_list_snippet.html" %}
   </div>
 {% endblock %}
 
=== added file 'src/webcatalog/templates/webcatalog/application_review_list_snippet.html'
--- src/webcatalog/templates/webcatalog/application_review_list_snippet.html	1970-01-01 00:00:00 +0000
+++ src/webcatalog/templates/webcatalog/application_review_list_snippet.html	2011-09-16 09:53:23 +0000
@@ -0,0 +1,18 @@
+{% load i18n %}
+{% load webcatalog %}
+{% 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 %}
 
=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py	2011-09-15 10:53:40 +0000
+++ src/webcatalog/tests/test_views.py	2011-09-16 09:53:23 +0000
@@ -116,6 +116,15 @@
 
         self.assertContains(response, 'action="apt://pkgfoo"')
 
+    def test_link_to_package_reviews(self):
+        response, app = self.get_app_and_response()
+
+        self.assertContains(response, '<a href="{0}"'.format(
+            reverse('wc-package-reviews', args=[
+                app.distroseries.code_name,
+                app.package_name,
+                ])))
+
     def test_button_for_non_puchase_app(self):
         response, app = self.get_app_and_response()
 
@@ -515,7 +524,10 @@
         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 = []
+        eg_review = ReviewDetails.from_dict(
+            dict(package_name='foo', summary='foo summary',
+                 rating='3.5'))
+        self.mock_get_reviews.return_value = [eg_review]
 
     def tearDown(self):
         self.get_reviews_patcher.stop()
@@ -566,5 +578,25 @@
 
         self.assertTemplateUsed(
             response, 'webcatalog/application_review_list.html')
+        self.assertTemplateUsed(
+            response, 'webcatalog/application_review_list_snippet.html')
+        self.assertContains(response, 'review_summary1')
+        self.assertContains(response, 'review_summary2')
+
+    def test_renders_reviews_only_for_json_request(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]),
+            HTTP_X_REQUESTED_WITH='XMLHttpRequest')
+
+        self.assertTemplateNotUsed(
+            response, 'webcatalog/application_review_list.html')
+        self.assertTemplateUsed(
+            response, 'webcatalog/application_review_list_snippet.html')
         self.assertContains(response, 'review_summary1')
         self.assertContains(response, 'review_summary2')
 
=== modified file 'src/webcatalog/utilities.py'
--- src/webcatalog/utilities.py	2011-09-15 08:58:19 +0000
+++ src/webcatalog/utilities.py	2011-09-16 09:53:23 +0000
@@ -34,6 +34,7 @@
 from httplib2 import ServerNotFoundError
 
 from django.conf import settings
+from django.core.cache import cache
 
 from lazr.restfulclient.authorize import BasicHttpAuthorizer
 from lazr.restfulclient.authorize.oauth import OAuthAuthorizer
@@ -95,8 +96,16 @@
         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, 
+        key = 'get_reviews_for_package-{0}-{1}-{2}'.format(
+            package_name, distroseries, page)
+        cached_reviews = cache.get(key)
+        if cached_reviews is not None:
+            return cached_reviews
+
+        fresh_reviews = self.rnr_api.get_reviews(packagename=package_name, 
             distroseries=distroseries, page=page)
+        cache.set(key, fresh_reviews, settings.REVIEWS_CACHE_TIMEOUT)
+        return fresh_reviews
 
     def _fake_validate_token(self, token, openid_identifier, signature):
         """ This is a version of validate_token that gets the
 
=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2011-09-15 11:25:35 +0000
+++ src/webcatalog/views.py	2011-09-16 09:53:23 +0000
@@ -35,7 +35,6 @@
     render_to_response,
     )
 from django.template import RequestContext
-from django.views.decorators.cache import cache_page
 
 from webcatalog.models import (
     Application,
@@ -179,15 +178,19 @@
             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.
+    # XXX michaeln 2011-09-15 bug=851662 Better review language options.
     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())))
+
+    context = dict(application=app, reviews=reviews)
+    if request.is_ajax():
+        template = 'webcatalog/application_review_list_snippet.html'
+    else:
+        context['distroseries'] = distro
+        context['breadcrumbs'] = app.crumbs()
+        template = 'webcatalog/application_review_list.html'
+
+    return render_to_response(template, RequestContext(request, context))