Merge lp:~michael.nelson/ubuntu-webcatalog/962140-display-video-demos-pt-2 into lp:ubuntu-webcatalog

[Michael Nelson]

Add videos to the application detail page.

Overview
========

Adds videos to the application detail page, to look like this:

http://people.canonical.com/~michaeln/tmp/962140-video-iframes.png

Note:
 * The video we have on staging is no longer allowed to be embedded (that's an option set by the author)
 * I've also tweaked the style for the sharing options so that the facebook options are aligned.

`fab test`

[Łukasz Czyżykowski (lukasz-czyzykowski)]

Why using setattr, gettattr, hasattr for media caching instead of just plain dict (self._media_cache = {}). Or other than doing per request cache, maybe it would be better to push that to django cache layer?

[lp:~michael.nelson/ubuntu-webcatalog/962140-display-video-demos-pt-2]

126. By Michael Nelson on 2012-05-04

REFACTOR: update media cache to use a single dict rather than multiple private attributes.

[Michael Nelson (michael.nelson)]

On Fri, May 4, 2012 at 12:47 PM, Łukasz Czyżykowski
<email address hidden> wrote:
> Why using setattr, gettattr, hasattr for media caching instead of just plain dict (self._media_cache = {}). Or other than doing per request cache, maybe it would be better to push that to django cache layer?

12:02 < noodles> lukasz: +1 for self._media_cache instead, but I think
we'd specifically not want to use the django cache layer as caching
over multiple requests would be bad?
12:02 * noodles updates
12:03 < lukasz> noodles, why?
12:06 < noodles> lukasz: run import_for_pay_apps, then view a specific
app, but it's screenshots haven't updated, or multiple points in the
code need to know about the cache (to clear it during import)
12:06 < noodles> lukasz: I guess the second option isn't so bad.
12:07 < lukasz> noodles, yeah, that would be my only concern, a need
for keeping the cache up to date
12:07 < lukasz> noodles, ok, let's leave it at dict
12:07 < noodles> k
12:12 < noodles> lukasz: pushed with r126

=== modified file 'src/webcatalog/models/applications.py'
--- src/webcatalog/models/applications.py	2012-05-03 09:59:18 +0000
+++ src/webcatalog/models/applications.py	2012-05-04 12:14:18 +0000
@@ -114,12 +114,23 @@
     def __unicode__(self):
         return u"{0} ({1})".format(self.name, self.package_name)
 
+    def _get_media(self, media_type):
+        if not hasattr(self, '_media_cache'):
+            self._media_cache = {}
+
+        if not media_type in self._media_cache:
+            medias = self.applicationmedia_set.filter(media_type=media_type)
+            self._media_cache[media_type] = [media.url for media in medias]
+
+        return self._media_cache[media_type]
+
     @property
     def screenshots(self):
-        if not hasattr(self, '_screenshots'):
-            qs = self.applicationmedia_set.filter(media_type='screenshot')
-            self._screenshots = [am.url for am in qs]
-        return self._screenshots
+        return self._get_media('screenshot')
+
+    @property
+    def video_iframe_urls(self):
+        return self._get_media('video')
 
     @property
     def categories_set(self):

=== modified file 'src/webcatalog/static/css/webcatalog.css'
--- src/webcatalog/static/css/webcatalog.css	2012-04-19 23:05:19 +0000
+++ src/webcatalog/static/css/webcatalog.css	2012-05-04 12:14:18 +0000
@@ -58,6 +58,9 @@
 .share {
     padding: 16px;
 }
+.share .fb-like {
+    height: 24px;
+}
 .description {
     padding: 42px 16px;
     font-size: 14px;
@@ -427,3 +430,9 @@
     margin: 32px;
     color: #888;
 }
+
+div.videos iframe {
+    width: 100%;
+    height: 225px;
+    margin: 16px;
+}

=== modified file 'src/webcatalog/templates/webcatalog/application_detail.html'
--- src/webcatalog/templates/webcatalog/application_detail.html	2012-04-20 18:22:42 +0000
+++ src/webcatalog/templates/webcatalog/application_detail.html	2012-05-04 12:14:18 +0000
@@ -119,6 +119,14 @@
         {% endif %}
       </table>
     </div>
+    {% if application.video_iframe_urls %}
+    <div class="videos">
+      <h3>Videos</h3>
+      {% for iframe_url in application.video_iframe_urls %}
+        <iframe src="{{ iframe_url }}"></iframe>
+      {% endfor %}
+    </div>
+    {% endif %}
     <div class="share">
       {% include "webcatalog/twitter_share_snippet.html" %}
       {% include "webcatalog/googleplus_share_snippet.html" %}

=== modified file 'src/webcatalog/tests/test_models.py'
--- src/webcatalog/tests/test_models.py	2012-04-05 01:09:37 +0000
+++ src/webcatalog/tests/test_models.py	2012-05-04 12:14:18 +0000
@@ -121,6 +121,79 @@
         self.assertTrue(icon_url.startswith('/site_media/'))
         self.assertTrue('ubuntu-cof' in icon_url)
 
+    def test_screenshots(self):
+        app = self.factory.make_application()
+        app.applicationmedia_set.create(
+            media_type='screenshot',
+            url='http://example.com/screenshot1.png')
+        app.applicationmedia_set.create(
+            media_type='screenshot',
+            url='http://example.com/screenshot2.png')
+        app.applicationmedia_set.create(
+            media_type='video',
+            url='http://example.com/video.m4v')
+
+        urls = app.screenshots
+
+        self.assertEqual([
+            'http://example.com/screenshot1.png',
+            'http://example.com/screenshot2.png',
+            ],
+            urls)
+
+    def test_screenshots_cached(self):
+        # The db isn't hit if _screenshots is already
+        # cached.
+        app = self.factory.make_application()
+        app.applicationmedia_set.create(
+            media_type='screenshot',
+            url='http://example.com/screenshot1.png')
+        app.screenshots
+        app.applicationmedia_set.create(
+            media_type='screenshot',
+            url='http://example.com/screenshot2.png')
+
+        self.assertEqual([
+            'http://example.com/screenshot1.png',
+            ], app.screenshots)
+
+    def test_video_iframes_urls(self):
+        app = self.factory.make_application()
+        app.applicationmedia_set.create(
+            media_type='video',
+            url='http://example.com/video1.m4v')
+        app.applicationmedia_set.create(
+            media_type='video',
+            url='http://example.com/video2.m4v')
+        app.applicationmedia_set.create(
+            media_type='screenshot',
+            url='http://example.com/screenshot.png')
+
+        urls = app.video_iframe_urls
+
+        self.assertEqual([
+            'http://example.com/video1.m4v',
+            'http://example.com/video2.m4v',
+            ],
+            urls)
+
+    def test_video_iframes_urls_cached(self):
+        app = self.factory.make_application()
+        app.applicationmedia_set.create(
+            media_type='video',
+            url='http://example.com/video1.m4v')
+        app.video_iframe_urls
+        app.applicationmedia_set.create(
+            media_type='video',
+            url='http://example.com/video2.m4v')
+
+        urls = app.video_iframe_urls
+
+        self.assertEqual([
+            'http://example.com/video1.m4v',
+            ],
+            urls)
+
 
 class DepartmentTestCase(TestCaseWithFactory):
     def test_crumbs_no_parent(self):

=== modified file 'src/webcatalog/tests/test_views.py'
--- src/webcatalog/tests/test_views.py	2012-04-20 18:22:42 +0000
+++ src/webcatalog/tests/test_views.py	2012-05-04 12:14:18 +0000
@@ -366,6 +366,17 @@
 
         self.assertNotContains(response, 'Hardware requirements')
 
+    def test_videos_displayed(self):
+        app = self.factory.make_application()
+        app.applicationmedia_set.create(
+            media_type="video",
+            url="http://example.com/video_iframe.html")
+
+        response = self.client.get(self.get_app_details_url(app))
+
+        self.assertContains(response,
+            '<iframe src="http://example.com/video_iframe.html"')
+
 
 class ApplicationDetailNoSeriesTestCase(TestCaseWithFactory):
     def test_renders_latest(self):