Merge lp:~michael.nelson/ubuntu-webcatalog/1015515-archive-dont-overwrite-sca into lp:ubuntu-webcatalog

[Michael Nelson]

import_app_install_data should not overwrite apps imported from sca.

Overview
========

Builds on the prerequisite, ensuring that when archive apps are imported, they are not able to overwrite app instances imported from sca.

I also renamed the forms to be (hopefully) clearer, renamed import_for_purchase_apps (but symlinked with the old name), and added a check_all_latest to the end of import_all_app_install_data.

`fab test`

[lp:~michael.nelson/ubuntu-webcatalog/1015515-archive-dont-overwrite-sca]

180. By Michael Nelson on 2012-06-28

Merged 1015515-only-one-app into 1015515-archive-dont-overwrite-sca.

181. By Michael Nelson on 2012-06-28

Merged 1015515-only-one-app into 1015515-archive-dont-overwrite-sca.

182. By Michael Nelson on 2012-06-28

Indentation and small test fix.

183. By Michael Nelson on 2012-06-28

REFACTOR: Renamed application forms.

[Natalia Bidart (nataliabidart)]

I love it!

review: Approve

=== modified file 'src/webcatalog/forms.py'
--- src/webcatalog/forms.py	2012-06-28 14:27:19 +0000
+++ src/webcatalog/forms.py	2012-06-28 14:27:19 +0000
@@ -38,7 +38,7 @@
 
 __metaclass__ = type
 __all__ = [
-    'ApplicationForm',
+    'ArchiveApplicationForm',
     'desktop_field_mappings',
 ]
 
@@ -56,7 +56,7 @@
 }
 
 
-class ApplicationForm(forms.ModelForm):
+class ArchiveApplicationForm(forms.ModelForm):
 
     screenshot_url = forms.URLField(required=False)
 
@@ -82,11 +82,15 @@
         return cls(data=data, instance=instance)
 
     def clean(self):
-        cleaned_data = super(ApplicationForm, self).clean()
+        cleaned_data = super(ArchiveApplicationForm, self).clean()
 
-        # For all fields, ensure we don't overwrite valid data on an
-        # associated instance with blank fields.
         if self.instance.id:
+            if self.instance.imported_from_sca:
+                raise forms.ValidationError(
+                    u"Attempt to overwrite an sca-imported app.")
+
+            # For all fields, ensure we don't overwrite valid data on an
+            # associated instance with blank fields.
             for key, value in cleaned_data.items():
                 if not value:
                     instance_value = getattr(self.instance, key, False)
@@ -97,7 +101,7 @@
         return cleaned_data
 
     def save(self, commit=True):
-        app = super(ApplicationForm, self).save(commit=commit)
+        app = super(ArchiveApplicationForm, self).save(commit=commit)
         screenshot_url = self.cleaned_data['screenshot_url']
         if commit and screenshot_url:
             if screenshot_url not in app.screenshots:
@@ -124,7 +128,7 @@
             validator(email)
 
 
-class ForPurchaseApplicationForm(forms.ModelForm):
+class SCAApplicationForm(forms.ModelForm):
     screenshot_urls = MultiURLField(required=False)
     video_embedded_html_urls = MultiURLField(required=False)
 
@@ -165,7 +169,7 @@
         return apt.apt_pkg.upstream_version(value)
 
     def save(self, commit=True):
-        app = super(ForPurchaseApplicationForm, self).save(commit=commit)
+        app = super(SCAApplicationForm, self).save(commit=commit)
         if commit:
             self.save_media_urls()
         return app
 
=== modified file 'src/webcatalog/management/commands/import_all_app_install_data.py'
--- src/webcatalog/management/commands/import_all_app_install_data.py	2012-06-21 20:18:44 +0000
+++ src/webcatalog/management/commands/import_all_app_install_data.py	2012-06-28 14:27:19 +0000
@@ -38,6 +38,8 @@
             self.output("Importing app-install-data for {0}\n".format(
                 distroseries), 1)
             call_command('import_app_install_data', distroseries)
+        self.output("Running check_all_latest across all distroseries.\n", 1)
+        call_command('check_all_latest')
 
     def output(self, message, level=None, flush=False):
         if hasattr(self, 'stdout'):
 
=== modified file 'src/webcatalog/management/commands/import_app_install_data.py'
--- src/webcatalog/management/commands/import_app_install_data.py	2012-06-19 13:23:48 +0000
+++ src/webcatalog/management/commands/import_app_install_data.py	2012-06-28 14:27:19 +0000
@@ -40,7 +40,7 @@
 from django.core.files.images import ImageFile
 from django.core.management.base import LabelCommand
 
-from webcatalog.forms import ApplicationForm
+from webcatalog.forms import ArchiveApplicationForm
 from webcatalog.models import (
     Application,
     DistroSeries,
@@ -193,7 +193,8 @@
     def process_desktop_file(self, desktop_file_path, icon_dir, distroseries):
         with open(desktop_file_path, 'r') as desktop_file:
             data = desktop_file.read()
-        form = ApplicationForm.get_form_from_desktop_data(data, distroseries)
+        form = ArchiveApplicationForm.get_form_from_desktop_data(
+            data, distroseries)
 
         if form.is_valid():
             app = form.save()
 
=== added symlink 'src/webcatalog/management/commands/import_for_purchase_apps.py'
=== target is u'import_sca_apps.py'
=== renamed file 'src/webcatalog/management/commands/import_for_purchase_apps.py' => 'src/webcatalog/management/commands/import_sca_apps.py'
--- src/webcatalog/management/commands/import_for_purchase_apps.py	2012-06-14 09:23:43 +0000
+++ src/webcatalog/management/commands/import_sca_apps.py	2012-06-28 14:27:19 +0000
@@ -34,7 +34,7 @@
 from django.core.files.images import ImageFile
 from django.core.management.base import BaseCommand
 
-from webcatalog.forms import ForPurchaseApplicationForm
+from webcatalog.forms import SCAApplicationForm
 from webcatalog.models import (
     Application,
     DistroSeries,
@@ -73,7 +73,7 @@
             Application.objects.check_latest(app_data['package_name'])
 
     def import_app_from_data(self, app_data, icon_data, distroseries):
-        form = ForPurchaseApplicationForm.from_api_data(
+        form = SCAApplicationForm.from_api_data(
             app_data, distroseries)
         if form.is_valid():
             app = form.save()
 
=== modified file 'src/webcatalog/tests/test_commands.py'
--- src/webcatalog/tests/test_commands.py	2012-06-28 14:27:19 +0000
+++ src/webcatalog/tests/test_commands.py	2012-06-28 14:27:19 +0000
@@ -1044,7 +1044,10 @@
             with patch(patch_attr) as mock_call_command:
                 call_command('import_all_app_install_data')
 
-            self.assertEqual(mock_call_command.call_count, len(TEST_VERSIONS))
+            # In addition to calling import for each series, a final
+            # call_command does check_all_latest automatically.
+            self.assertEqual(mock_call_command.call_count,
+                             len(TEST_VERSIONS) + 1)
 
 
 class ImportAllRatingsStatsTestCase(TestCaseWithFactory):
 
=== modified file 'src/webcatalog/tests/test_forms.py'
--- src/webcatalog/tests/test_forms.py	2012-06-28 14:27:19 +0000
+++ src/webcatalog/tests/test_forms.py	2012-06-28 14:27:19 +0000
@@ -28,9 +28,9 @@
 from textwrap import dedent
 
 from webcatalog.forms import (
-    ApplicationForm,
+    ArchiveApplicationForm,
     desktop_field_mappings,
-    ForPurchaseApplicationForm,
+    SCAApplicationForm,
     MultiURLField,
 )
 from webcatalog.models import Application, ApplicationMedia
@@ -38,13 +38,13 @@
 
 __metaclass__ = type
 __all__ = [
-    'ApplicationFormTestCase',
-    'ForPurchaseApplicationFormTestCase',
+    'ArchiveApplicationFormTestCase',
+    'SCAApplicationFormTestCase',
     'MultiURLFieldTestCase',
 ]
 
 
-class ApplicationFormTestCase(TestCaseWithFactory):
+class ArchiveApplicationFormTestCase(TestCaseWithFactory):
 
     def get_desktop_data(self, overrides):
         data = {
@@ -87,7 +87,7 @@
         }
         desktop_entry = self.get_desktop_entry(self.get_desktop_data(data))
 
-        form = ApplicationForm.get_form_from_desktop_data(
+        form = ArchiveApplicationForm.get_form_from_desktop_data(
             desktop_entry, distroseries=self.factory.make_distroseries())
 
         self.assertTrue(form.is_valid())
@@ -105,7 +105,7 @@
         }
         desktop_entry = self.get_desktop_entry(self.get_desktop_data(data))
 
-        form = ApplicationForm.get_form_from_desktop_data(
+        form = ArchiveApplicationForm.get_form_from_desktop_data(
             desktop_entry, distroseries=app.distroseries)
 
         self.assertTrue(form.is_valid())
@@ -129,7 +129,7 @@
         info = self.get_desktop_data(extra_desktop_info)
         desktop_entry = self.get_desktop_entry(info)
 
-        form = ApplicationForm.get_form_from_desktop_data(
+        form = ArchiveApplicationForm.get_form_from_desktop_data(
             desktop_entry, distroseries=self.factory.make_distroseries())
 
         self.assertTrue(form.is_valid())
@@ -147,7 +147,7 @@
         info = self.get_desktop_data(extra_desktop_info)
         desktop_entry = self.get_desktop_entry(info)
 
-        form = ApplicationForm.get_form_from_desktop_data(
+        form = ArchiveApplicationForm.get_form_from_desktop_data(
             desktop_entry, distroseries=app.distroseries)
         form.save()
 
@@ -168,17 +168,33 @@
         d = dict(screenshot_url='http://foo.com:42/broken',
                  section='required', name='required', package_name='required',
                  distroseries=distroseries.id)
-        form = ApplicationForm(d)
+        form = ArchiveApplicationForm(d)
         self.assertTrue(form.is_valid())
 
-
-class ForPurchaseApplicationFormTestCase(TestCaseWithFactory):
+    def test_updating_an_sca_imported_app_fails_validation(self):
+        app = self.factory.make_application(imported_from_sca=True)
+        extra_desktop_info = {
+            'X-AppInstall-Package': app.package_name,
+        }
+        info = self.get_desktop_data(extra_desktop_info)
+        desktop_entry = self.get_desktop_entry(info)
+
+        form = ArchiveApplicationForm.get_form_from_desktop_data(
+            desktop_entry, distroseries=app.distroseries)
+
+        self.assertFalse(form.is_valid())
+        self.assertEqual(
+            {'__all__': [u'Attempt to overwrite an sca-imported app.']},
+            form.errors)
+
+
+class SCAApplicationFormTestCase(TestCaseWithFactory):
 
     def test_from_api_data_shortens_debtag(self):
         app = self.factory.make_application()
         data = self.make_valid_data(debtags=['hardware::storage:cd-writer'])
 
-        form = ForPurchaseApplicationForm.from_api_data(data, app.distroseries)
+        form = SCAApplicationForm.from_api_data(data, app.distroseries)
 
         self.assertEqual(form.data['debtags'], '["CD burner"]')
 
@@ -199,8 +215,8 @@
             'name': app.name,
         }
 
-        form = ForPurchaseApplicationForm.from_api_data(data,
-                                                        app.distroseries)
+        form = SCAApplicationForm.from_api_data(data,
+                                                app.distroseries)
         self.assertTrue(form.is_valid())
         form.save_media_urls()
 
@@ -259,7 +275,7 @@
 
         data = self.make_valid_data(archive_id='')
 
-        form = ForPurchaseApplicationForm.from_api_data(data, precise)
+        form = SCAApplicationForm.from_api_data(data, precise)
 
         self.assertTrue(form.is_valid())
 
 
=== modified file 'src/webcatalog/views.py'
--- src/webcatalog/views.py	2012-06-28 14:27:19 +0000
+++ src/webcatalog/views.py	2012-06-28 14:27:19 +0000
@@ -232,7 +232,7 @@
 
 def application_reviews(request, package_name, distro, ajax=False, page=1):
     app = get_object_or_404(Application, package_name=package_name,
-                           distroseries__code_name=distro)
+                            distroseries__code_name=distro)
 
     # XXX michaeln 2011-09-15 bug=851662 Better review language options.
     reviews = WebServices().get_reviews_for_package(