Ubuntu Apps Directory

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

  1. 1015515-archive-dont-overwrite-sca
  2. Merge into trunk
Proposed by Michael Nelson
Status: Merged
Merged at revision: 152
Proposed branch: lp:~michael.nelson/ubuntu-webcatalog/1015515-archive-dont-overwrite-sca
Merge into: lp:ubuntu-webcatalog
Prerequisite: lp:~michael.nelson/ubuntu-webcatalog/1015515-only-one-app
Diff against target: 283 lines (+56/-30)
7 files modified
src/webcatalog/forms.py (+12/-8)
src/webcatalog/management/commands/import_all_app_install_data.py (+2/-0)
src/webcatalog/management/commands/import_app_install_data.py (+3/-2)
src/webcatalog/management/commands/import_sca_apps.py (+2/-2)
src/webcatalog/tests/test_commands.py (+4/-1)
src/webcatalog/tests/test_forms.py (+32/-16)
src/webcatalog/views.py (+1/-1)
To merge this branch: bzr merge lp:~michael.nelson/ubuntu-webcatalog/1015515-archive-dont-overwrite-sca
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Review via email: mp+112561@code.launchpad.net

Commit message

import_app_install_data should not overwrite apps imported from sca.

Description of the change

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`

To post a comment you must log in.
lp:~michael.nelson/ubuntu-webcatalog/1015515-archive-dont-overwrite-sca updated
180. By Michael Nelson

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

181. By Michael Nelson

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

182. By Michael Nelson

Indentation and small test fix.

183. By Michael Nelson

REFACTOR: Renamed application forms.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

I love it!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/webcatalog/forms.py'
2--- src/webcatalog/forms.py 2012-06-28 14:27:19 +0000
3+++ src/webcatalog/forms.py 2012-06-28 14:27:19 +0000
4@@ -38,7 +38,7 @@
5
6 __metaclass__ = type
7 __all__ = [
8- 'ApplicationForm',
9+ 'ArchiveApplicationForm',
10 'desktop_field_mappings',
11 ]
12
13@@ -56,7 +56,7 @@
14 }
15
16
17-class ApplicationForm(forms.ModelForm):
18+class ArchiveApplicationForm(forms.ModelForm):
19
20 screenshot_url = forms.URLField(required=False)
21
22@@ -82,11 +82,15 @@
23 return cls(data=data, instance=instance)
24
25 def clean(self):
26- cleaned_data = super(ApplicationForm, self).clean()
27+ cleaned_data = super(ArchiveApplicationForm, self).clean()
28
29- # For all fields, ensure we don't overwrite valid data on an
30- # associated instance with blank fields.
31 if self.instance.id:
32+ if self.instance.imported_from_sca:
33+ raise forms.ValidationError(
34+ u"Attempt to overwrite an sca-imported app.")
35+
36+ # For all fields, ensure we don't overwrite valid data on an
37+ # associated instance with blank fields.
38 for key, value in cleaned_data.items():
39 if not value:
40 instance_value = getattr(self.instance, key, False)
41@@ -97,7 +101,7 @@
42 return cleaned_data
43
44 def save(self, commit=True):
45- app = super(ApplicationForm, self).save(commit=commit)
46+ app = super(ArchiveApplicationForm, self).save(commit=commit)
47 screenshot_url = self.cleaned_data['screenshot_url']
48 if commit and screenshot_url:
49 if screenshot_url not in app.screenshots:
50@@ -124,7 +128,7 @@
51 validator(email)
52
53
54-class ForPurchaseApplicationForm(forms.ModelForm):
55+class SCAApplicationForm(forms.ModelForm):
56 screenshot_urls = MultiURLField(required=False)
57 video_embedded_html_urls = MultiURLField(required=False)
58
59@@ -165,7 +169,7 @@
60 return apt.apt_pkg.upstream_version(value)
61
62 def save(self, commit=True):
63- app = super(ForPurchaseApplicationForm, self).save(commit=commit)
64+ app = super(SCAApplicationForm, self).save(commit=commit)
65 if commit:
66 self.save_media_urls()
67 return app
68
69=== modified file 'src/webcatalog/management/commands/import_all_app_install_data.py'
70--- src/webcatalog/management/commands/import_all_app_install_data.py 2012-06-21 20:18:44 +0000
71+++ src/webcatalog/management/commands/import_all_app_install_data.py 2012-06-28 14:27:19 +0000
72@@ -38,6 +38,8 @@
73 self.output("Importing app-install-data for {0}\n".format(
74 distroseries), 1)
75 call_command('import_app_install_data', distroseries)
76+ self.output("Running check_all_latest across all distroseries.\n", 1)
77+ call_command('check_all_latest')
78
79 def output(self, message, level=None, flush=False):
80 if hasattr(self, 'stdout'):
81
82=== modified file 'src/webcatalog/management/commands/import_app_install_data.py'
83--- src/webcatalog/management/commands/import_app_install_data.py 2012-06-19 13:23:48 +0000
84+++ src/webcatalog/management/commands/import_app_install_data.py 2012-06-28 14:27:19 +0000
85@@ -40,7 +40,7 @@
86 from django.core.files.images import ImageFile
87 from django.core.management.base import LabelCommand
88
89-from webcatalog.forms import ApplicationForm
90+from webcatalog.forms import ArchiveApplicationForm
91 from webcatalog.models import (
92 Application,
93 DistroSeries,
94@@ -193,7 +193,8 @@
95 def process_desktop_file(self, desktop_file_path, icon_dir, distroseries):
96 with open(desktop_file_path, 'r') as desktop_file:
97 data = desktop_file.read()
98- form = ApplicationForm.get_form_from_desktop_data(data, distroseries)
99+ form = ArchiveApplicationForm.get_form_from_desktop_data(
100+ data, distroseries)
101
102 if form.is_valid():
103 app = form.save()
104
105=== added symlink 'src/webcatalog/management/commands/import_for_purchase_apps.py'
106=== target is u'import_sca_apps.py'
107=== renamed file 'src/webcatalog/management/commands/import_for_purchase_apps.py' => 'src/webcatalog/management/commands/import_sca_apps.py'
108--- src/webcatalog/management/commands/import_for_purchase_apps.py 2012-06-14 09:23:43 +0000
109+++ src/webcatalog/management/commands/import_sca_apps.py 2012-06-28 14:27:19 +0000
110@@ -34,7 +34,7 @@
111 from django.core.files.images import ImageFile
112 from django.core.management.base import BaseCommand
113
114-from webcatalog.forms import ForPurchaseApplicationForm
115+from webcatalog.forms import SCAApplicationForm
116 from webcatalog.models import (
117 Application,
118 DistroSeries,
119@@ -73,7 +73,7 @@
120 Application.objects.check_latest(app_data['package_name'])
121
122 def import_app_from_data(self, app_data, icon_data, distroseries):
123- form = ForPurchaseApplicationForm.from_api_data(
124+ form = SCAApplicationForm.from_api_data(
125 app_data, distroseries)
126 if form.is_valid():
127 app = form.save()
128
129=== modified file 'src/webcatalog/tests/test_commands.py'
130--- src/webcatalog/tests/test_commands.py 2012-06-28 14:27:19 +0000
131+++ src/webcatalog/tests/test_commands.py 2012-06-28 14:27:19 +0000
132@@ -1044,7 +1044,10 @@
133 with patch(patch_attr) as mock_call_command:
134 call_command('import_all_app_install_data')
135
136- self.assertEqual(mock_call_command.call_count, len(TEST_VERSIONS))
137+ # In addition to calling import for each series, a final
138+ # call_command does check_all_latest automatically.
139+ self.assertEqual(mock_call_command.call_count,
140+ len(TEST_VERSIONS) + 1)
141
142
143 class ImportAllRatingsStatsTestCase(TestCaseWithFactory):
144
145=== modified file 'src/webcatalog/tests/test_forms.py'
146--- src/webcatalog/tests/test_forms.py 2012-06-28 14:27:19 +0000
147+++ src/webcatalog/tests/test_forms.py 2012-06-28 14:27:19 +0000
148@@ -28,9 +28,9 @@
149 from textwrap import dedent
150
151 from webcatalog.forms import (
152- ApplicationForm,
153+ ArchiveApplicationForm,
154 desktop_field_mappings,
155- ForPurchaseApplicationForm,
156+ SCAApplicationForm,
157 MultiURLField,
158 )
159 from webcatalog.models import Application, ApplicationMedia
160@@ -38,13 +38,13 @@
161
162 __metaclass__ = type
163 __all__ = [
164- 'ApplicationFormTestCase',
165- 'ForPurchaseApplicationFormTestCase',
166+ 'ArchiveApplicationFormTestCase',
167+ 'SCAApplicationFormTestCase',
168 'MultiURLFieldTestCase',
169 ]
170
171
172-class ApplicationFormTestCase(TestCaseWithFactory):
173+class ArchiveApplicationFormTestCase(TestCaseWithFactory):
174
175 def get_desktop_data(self, overrides):
176 data = {
177@@ -87,7 +87,7 @@
178 }
179 desktop_entry = self.get_desktop_entry(self.get_desktop_data(data))
180
181- form = ApplicationForm.get_form_from_desktop_data(
182+ form = ArchiveApplicationForm.get_form_from_desktop_data(
183 desktop_entry, distroseries=self.factory.make_distroseries())
184
185 self.assertTrue(form.is_valid())
186@@ -105,7 +105,7 @@
187 }
188 desktop_entry = self.get_desktop_entry(self.get_desktop_data(data))
189
190- form = ApplicationForm.get_form_from_desktop_data(
191+ form = ArchiveApplicationForm.get_form_from_desktop_data(
192 desktop_entry, distroseries=app.distroseries)
193
194 self.assertTrue(form.is_valid())
195@@ -129,7 +129,7 @@
196 info = self.get_desktop_data(extra_desktop_info)
197 desktop_entry = self.get_desktop_entry(info)
198
199- form = ApplicationForm.get_form_from_desktop_data(
200+ form = ArchiveApplicationForm.get_form_from_desktop_data(
201 desktop_entry, distroseries=self.factory.make_distroseries())
202
203 self.assertTrue(form.is_valid())
204@@ -147,7 +147,7 @@
205 info = self.get_desktop_data(extra_desktop_info)
206 desktop_entry = self.get_desktop_entry(info)
207
208- form = ApplicationForm.get_form_from_desktop_data(
209+ form = ArchiveApplicationForm.get_form_from_desktop_data(
210 desktop_entry, distroseries=app.distroseries)
211 form.save()
212
213@@ -168,17 +168,33 @@
214 d = dict(screenshot_url='http://foo.com:42/broken',
215 section='required', name='required', package_name='required',
216 distroseries=distroseries.id)
217- form = ApplicationForm(d)
218+ form = ArchiveApplicationForm(d)
219 self.assertTrue(form.is_valid())
220
221-
222-class ForPurchaseApplicationFormTestCase(TestCaseWithFactory):
223+ def test_updating_an_sca_imported_app_fails_validation(self):
224+ app = self.factory.make_application(imported_from_sca=True)
225+ extra_desktop_info = {
226+ 'X-AppInstall-Package': app.package_name,
227+ }
228+ info = self.get_desktop_data(extra_desktop_info)
229+ desktop_entry = self.get_desktop_entry(info)
230+
231+ form = ArchiveApplicationForm.get_form_from_desktop_data(
232+ desktop_entry, distroseries=app.distroseries)
233+
234+ self.assertFalse(form.is_valid())
235+ self.assertEqual(
236+ {'__all__': [u'Attempt to overwrite an sca-imported app.']},
237+ form.errors)
238+
239+
240+class SCAApplicationFormTestCase(TestCaseWithFactory):
241
242 def test_from_api_data_shortens_debtag(self):
243 app = self.factory.make_application()
244 data = self.make_valid_data(debtags=['hardware::storage:cd-writer'])
245
246- form = ForPurchaseApplicationForm.from_api_data(data, app.distroseries)
247+ form = SCAApplicationForm.from_api_data(data, app.distroseries)
248
249 self.assertEqual(form.data['debtags'], '["CD burner"]')
250
251@@ -199,8 +215,8 @@
252 'name': app.name,
253 }
254
255- form = ForPurchaseApplicationForm.from_api_data(data,
256- app.distroseries)
257+ form = SCAApplicationForm.from_api_data(data,
258+ app.distroseries)
259 self.assertTrue(form.is_valid())
260 form.save_media_urls()
261
262@@ -259,7 +275,7 @@
263
264 data = self.make_valid_data(archive_id='')
265
266- form = ForPurchaseApplicationForm.from_api_data(data, precise)
267+ form = SCAApplicationForm.from_api_data(data, precise)
268
269 self.assertTrue(form.is_valid())
270
271
272=== modified file 'src/webcatalog/views.py'
273--- src/webcatalog/views.py 2012-06-28 14:27:19 +0000
274+++ src/webcatalog/views.py 2012-06-28 14:27:19 +0000
275@@ -232,7 +232,7 @@
276
277 def application_reviews(request, package_name, distro, ajax=False, page=1):
278 app = get_object_or_404(Application, package_name=package_name,
279- distroseries__code_name=distro)
280+ distroseries__code_name=distro)
281
282 # XXX michaeln 2011-09-15 bug=851662 Better review language options.
283 reviews = WebServices().get_reviews_for_package(

Subscribers

People subscribed via source and target branches