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

[Michael Nelson]

Import video urls during import_for_purchase_apps.

Overview
========

Just a prep-branch for bug 962140 that:

* ensures we import the video urls when importing for-purchase apps
* removes unnecessary list serialise/unserialise and simplifies importer code by putting responsibility for saving media in the form.save() itself (matching the distro import command)

`fab test`

Tomorrow's first branch will simply use the imported data to display the iframe.

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

124. By Michael Nelson on 2012-05-03

REFACTOR: Added commit kwargs back to form.save() methods.

[Anthony Lenton (elachuni)]

Looks fine!

review: Approve

=== modified file 'src/webcatalog/forms.py'
--- src/webcatalog/forms.py	2012-04-20 06:41:56 +0000
+++ src/webcatalog/forms.py	2012-05-03 12:42:19 +0000
@@ -96,12 +96,10 @@
 
         return cleaned_data
 
-    def save(self):
-        # We don't provide a commit=False option as we have
-        # to commit to be able to add the screenshots.
-        app = super(ApplicationForm, self).save()
+    def save(self, commit=True):
+        app = super(ApplicationForm, self).save(commit=commit)
         screenshot_url = self.cleaned_data['screenshot_url']
-        if screenshot_url:
+        if commit and screenshot_url:
             if screenshot_url not in app.screenshots:
                 app.applicationmedia_set.create(
                     media_type='screenshot', url=screenshot_url)
@@ -114,7 +112,7 @@
         # Return an empty list if no input was given.
         if not value:
             return []
-        return value.split(',')
+        return value
 
     def validate(self, value):
         "Check if value consists only of valid urls."
@@ -128,20 +126,21 @@
 
 class ForPurchaseApplicationForm(forms.ModelForm):
     screenshot_urls = MultiURLField(required=False)
+    video_embedded_html_urls = MultiURLField(required=False)
 
     class Meta:
         model = Application
-        exclude = ('distroseries', 'section', 'popcon')
+        exclude = ('section', 'popcon')
 
     @classmethod
-    def from_json(cls, app_data, distroseries):
+    def from_api_data(cls, app_data, distroseries):
         app_data = app_data.copy()
+        app_data['distroseries'] = distroseries.id
+        app_data['for_purchase'] = True
         if 'description' in app_data:
             tagline, _, description = app_data['description'].partition('\n')
             app_data['comment'] = tagline
             app_data['description'] = description
-        if 'screenshot_urls' in app_data:
-            app_data['screenshot_urls'] = ",".join(app_data['screenshot_urls'])
         if 'debtags' in  app_data and app_data['debtags']:
             app_data['debtags'] = json.dumps([get_hw_short_description(x)
                                    for x in app_data['debtags']])
@@ -159,16 +158,20 @@
         return apt.apt_pkg.upstream_version(value)
 
     def save(self, commit=True):
-        app = super(ForPurchaseApplicationForm, self).save(commit)
+        app = super(ForPurchaseApplicationForm, self).save(commit=commit)
         if commit:
-            self.save_screenshot_urls()
+            self.save_media_urls()
         return app
 
-    def save_screenshot_urls(self):
+    def save_media_urls(self):
         for screenshot_url in self.cleaned_data['screenshot_urls']:
             self.instance.applicationmedia_set.get_or_create(
                 url=screenshot_url, media_type='screenshot')
 
+        for video_url in self.cleaned_data['video_embedded_html_urls']:
+            self.instance.applicationmedia_set.get_or_create(
+                url=video_url, media_type='video')
+
 
 class EmailDownloadLinkForm(forms.Form):
     email = forms.EmailField()

=== modified file 'src/webcatalog/management/commands/import_for_purchase_apps.py'
--- src/webcatalog/management/commands/import_for_purchase_apps.py	2012-03-19 14:38:02 +0000
+++ src/webcatalog/management/commands/import_for_purchase_apps.py	2012-05-03 12:42:19 +0000
@@ -70,13 +70,10 @@
             Application.objects.check_latest(app_data['package_name'])
 
     def import_app_from_data(self, app_data, icon_data, distroseries):
-        form = ForPurchaseApplicationForm.from_json(app_data, distroseries)
+        form = ForPurchaseApplicationForm.from_api_data(
+            app_data, distroseries)
         if form.is_valid():
-            app = form.save(commit=False)
-            app.distroseries = distroseries
-            app.for_purchase = True
-            app.save()
-            form.save_screenshot_urls()
+            app = form.save()
             app.update_departments()
             self.add_icon_to_app(app, data=icon_data)
             self.output(u"{0} created.\n".format(app.name).encode('utf-8'), 1)

=== modified file 'src/webcatalog/tests/test_forms.py'
--- src/webcatalog/tests/test_forms.py	2012-04-20 06:41:56 +0000
+++ src/webcatalog/tests/test_forms.py	2012-05-03 12:42:19 +0000
@@ -22,12 +22,16 @@
     with_statement,
     )
 
+from django.forms import ValidationError
+from django.test import TestCase
+
 from textwrap import dedent
 
 from webcatalog.forms import (
     ApplicationForm,
     desktop_field_mappings,
     ForPurchaseApplicationForm,
+    MultiURLField,
     )
 from webcatalog.models import Application, ApplicationMedia
 from webcatalog.tests.factory import TestCaseWithFactory
@@ -36,6 +40,7 @@
 __all__ = [
     'ApplicationFormTestCase',
     'ForPurchaseApplicationFormTestCase',
+    'MultiURLFieldTestCase',
     ]
 
 
@@ -169,10 +174,64 @@
 
 class ForPurchaseApplicationFormTestCase(TestCaseWithFactory):
 
-    def test_from_json_shortens_debtag(self):
+    def test_from_api_data_shortens_debtag(self):
         app = self.factory.make_application()
         data = {'debtags': ['hardware::storage:cd-writer']}
 
-        form = ForPurchaseApplicationForm.from_json(data, app.distroseries)
+        form = ForPurchaseApplicationForm.from_api_data(data, app.distroseries)
 
         self.assertEqual(form.data['debtags'], '["CD burner"]')
+
+    def test_save_media_urls(self):
+        app = self.factory.make_application(
+            screenshot_url="http://example.com/screenshot1.png")
+        self.assertEqual(1, app.applicationmedia_set.count())
+        data = {
+            'screenshot_urls': [
+                'http://example.com/screenshot1.png',
+                'http://example.com/screenshot2.png',
+                ],
+            'video_embedded_html_urls': [
+                'http://example.com/video1.mp4',
+                ],
+            'archive_id': app.archive_id,
+            'package_name': app.package_name,
+            'name': app.name,
+            }
+
+        form = ForPurchaseApplicationForm.from_api_data(data,
+            app.distroseries)
+        self.assertTrue(form.is_valid())
+        form.save_media_urls()
+        self.assertEqual(3, app.applicationmedia_set.count())
+        actual_urls = [media.url for media in app.applicationmedia_set.all()]
+        self.assertEqual([
+            'http://example.com/screenshot1.png',
+            'http://example.com/screenshot2.png',
+            'http://example.com/video1.mp4',
+            ], actual_urls)
+
+
+class MultiURLFieldTestCase(TestCase):
+
+    def test_normalize_empty_value(self):
+        self.assertEqual(
+            [], MultiURLField().to_python(''))
+        self.assertEqual(
+            [], MultiURLField().to_python(None))
+        self.assertEqual(
+            [], MultiURLField().to_python([]))
+
+    def test_normalize_expects_list_strings(self):
+        self.assertEqual(
+            ['foo', 'bar'],
+            MultiURLField().to_python(['foo', 'bar']))
+
+    def test_validates_multiple_URLs(self):
+        MultiURLField().validate(
+            ['http://example.com', 'https://example.com/two'])
+
+        self.assertRaises(
+            ValidationError,
+            MultiURLField().validate,
+            ['httpd://example.com', 'https://example.com/two'])