Merge lp:~elachuni/ubuntu-webcatalog/screenshots-and-reviews into lp:ubuntu-webcatalog

[Anthony Lenton]

Fix screenshots and reviews.

A fix for two issues uncovered during testing:

- The review page crashes for packages with colons in their names. Afaik this is an issue only for multiarch packages (that's why they picked that separator). See for example http://catalog.razorgirl.info/cat/applications/oneiric/skype:i386/reviews/
http://catalog.razorgirl.info/cat/applications/skype:i386/ is even funnier.

- The screenshots page fails when there's more than one app for a given package name (this includes the same app on two different distroseries). See for example https://catalog.razorgirl.info/cat/applications/screenshots/hedgewars/

=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py	2012-04-18 22:29:29 +0000
+++ src/webcatalog/tests/test_views.py	2012-04-19 03:12:18 +0000
@@ -897,7 +897,8 @@
         self.mock_get_reviews.return_value = [eg_review]
 
     def tearDown(self):
-        self.get_reviews_patcher.stop()
+        if hasattr(self, 'get_reviews_patcher'):
+            self.get_reviews_patcher.stop()
         super(ApplicationReviewsTestCase, self).tearDown()
 
     def test_only_valid_apps(self):
@@ -968,11 +969,21 @@
         self.assertContains(response, 'review_summary1')
         self.assertContains(response, 'review_summary2')
 
+    def test_doesnt_break_for_invalid_package_name(self):
+        self.get_reviews_patcher.stop()
+        del self.get_reviews_patcher
+        app = self.factory.make_application(package_name='invalid:name')
+
+        response = self.client.get(reverse('wc-package-reviews',
+            args=[app.distroseries.code_name, app.package_name]))
+
+        self.assertEqual(200, response.status_code)
+
 
 class ApplicationScreenshotsTestCase(TestCaseWithFactory):
     def setUp(self):
         super(ApplicationScreenshotsTestCase, self).setUp()
-        self.app = self.factory.make_application()
+        self.app = self.factory.make_application(is_latest=True)
         self.expected = ['http://example.com/foo', 'http://example.com/bar']
 
     def test_only_valid_apps(self):
@@ -1032,6 +1043,15 @@
 
         self.assertEqual('max-age=600', response['cache-control'])
 
+    @patch('webcatalog.utilities.urllib.urlopen')
+    def test_doesnt_break_with_multiple_apps(self, mock_urlopen):
+        self.factory.make_application(package_name=self.app.package_name)
+
+        response = self.client.get(reverse('wc-package-screenshots',
+            args=[self.app.package_name]))
+
+        self.assertEqual(200, response.status_code)
+
 
 class ComboViewTestCase(TestCase):
     """Tests for ComboView."""
 
=== modified file 'src/webcatalog/utilities.py'
--- src/webcatalog/utilities.py	2012-04-18 22:29:29 +0000
+++ src/webcatalog/utilities.py	2012-04-19 03:12:18 +0000
@@ -42,7 +42,7 @@
     BasicAuthorizer,
     OAuthAuthorizer,
     )
-
+from piston_mini_client.validators import ValidationException
 from rnrclient import RatingsAndReviewsAPI
 from ssoclient import SingleSignOnAPI
 
@@ -74,8 +74,11 @@
         if cached_reviews is not None:
             return cached_reviews
 
-        fresh_reviews = self.rnr_api.get_reviews(packagename=package_name,
-            distroseries=distroseries, page=page)
+        try:
+            fresh_reviews = self.rnr_api.get_reviews(packagename=package_name,
+                distroseries=distroseries, page=page)
+        except ValidationException:
+            fresh_reviews = []
         cache.set(key, fresh_reviews, settings.REVIEWS_CACHE_TIMEOUT)
         return fresh_reviews
 
 
=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2012-04-18 22:29:29 +0000
+++ src/webcatalog/views.py	2012-04-19 03:12:18 +0000
@@ -211,7 +211,8 @@
 
 
 def application_screenshots(request, package_name):
-    app = get_object_or_404(Application, package_name=package_name)
+    app = get_object_or_404(Application, package_name=package_name,
+        is_latest=True)
     screenshots = WebServices().get_screenshots_for_package(package_name)
     if request.is_ajax():
         return HttpResponse(json.dumps(screenshots),