Merge lp:~michael.nelson/ubuntu-webcatalog/fix-import-crash into lp:ubuntu-webcatalog

Proposed by Michael Nelson
Status: Merged
Approved by: Łukasz Czyżykowski
Approved revision: 111
Merged at revision: 109
Proposed branch: lp:~michael.nelson/ubuntu-webcatalog/fix-import-crash
Merge into: lp:ubuntu-webcatalog
Diff against target: 60 lines (+9/-6)
3 files modified
src/webcatalog/forms.py (+6/-3)
src/webcatalog/management/commands/import_app_install_data.py (+0/-2)
src/webcatalog/tests/test_forms.py (+3/-1)
To merge this branch: bzr merge lp:~michael.nelson/ubuntu-webcatalog/fix-import-crash
Reviewer Review Type Date Requested Status
Canonical Consumer Applications Hackers Pending
Review via email: mp+102795@code.launchpad.net

Commit message

Don't save an application without a distroseries during import.

Description of the change

When the initial error appeared:

https://bugs.launchpad.net/ubuntu-webcatalog/+bug/985901

the fix was to ensure the app was saved so that when the ApplicationForm.save() method tried to add the related screenshots, it didn't error. But the app cannot be saved without a distroseries either.

This branch updates the ApplicationForm so that it includes the distroseries in its cleaned data, then during save, it saves the app *with* the distroseries before trying to add any related screenshots (and removes the option of commit=False)

`fab test`

I'll test this on the vps once it's landed.

To post a comment you must log in.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/webcatalog/forms.py'
--- src/webcatalog/forms.py 2012-03-26 20:48:43 +0000
+++ src/webcatalog/forms.py 2012-04-20 06:45:22 +0000
@@ -62,7 +62,7 @@
6262
63 class Meta:63 class Meta:
64 model = Application64 model = Application
65 exclude = ('distroseries', 'for_purchase', 'archive_id', 'price')65 exclude = ('for_purchase', 'archive_id', 'price')
6666
67 @classmethod67 @classmethod
68 def get_form_from_desktop_data(cls, str_data, distroseries):68 def get_form_from_desktop_data(cls, str_data, distroseries):
@@ -78,6 +78,7 @@
78 package_name=data['package_name'], distroseries=distroseries)78 package_name=data['package_name'], distroseries=distroseries)
79 except Application.DoesNotExist:79 except Application.DoesNotExist:
80 instance = None80 instance = None
81 data['distroseries'] = distroseries.id
81 return cls(data=data, instance=instance)82 return cls(data=data, instance=instance)
8283
83 def clean(self):84 def clean(self):
@@ -95,8 +96,10 @@
9596
96 return cleaned_data97 return cleaned_data
9798
98 def save(self, commit=True):99 def save(self):
99 app = super(ApplicationForm, self).save(commit)100 # We don't provide a commit=False option as we have
101 # to commit to be able to add the screenshots.
102 app = super(ApplicationForm, self).save()
100 screenshot_url = self.cleaned_data['screenshot_url']103 screenshot_url = self.cleaned_data['screenshot_url']
101 if screenshot_url:104 if screenshot_url:
102 if screenshot_url not in app.screenshots:105 if screenshot_url not in app.screenshots:
103106
=== modified file 'src/webcatalog/management/commands/import_app_install_data.py'
--- src/webcatalog/management/commands/import_app_install_data.py 2012-04-19 21:20:32 +0000
+++ src/webcatalog/management/commands/import_app_install_data.py 2012-04-20 06:45:22 +0000
@@ -190,8 +190,6 @@
190190
191 if form.is_valid():191 if form.is_valid():
192 app = form.save()192 app = form.save()
193 app.distroseries = distroseries
194 app.save()
195 app.update_departments()193 app.update_departments()
196 self.add_icon_to_app(app, icon_dir)194 self.add_icon_to_app(app, icon_dir)
197 if self.verbosity > 1:195 if self.verbosity > 1:
198196
=== modified file 'src/webcatalog/tests/test_forms.py'
--- src/webcatalog/tests/test_forms.py 2012-03-26 20:48:43 +0000
+++ src/webcatalog/tests/test_forms.py 2012-04-20 06:45:22 +0000
@@ -160,8 +160,10 @@
160 # - It adds a significant delay to tests unless carefully mocked160 # - It adds a significant delay to tests unless carefully mocked
161 # - It doesn't provide a real benefit as 99% of the time the field161 # - It doesn't provide a real benefit as 99% of the time the field
162 # is populated by a script that doesn't have typo issues.162 # is populated by a script that doesn't have typo issues.
163 distroseries = self.factory.make_distroseries()
163 form = ApplicationForm(dict(screenshot_url='http://foo.com:42/broken',164 form = ApplicationForm(dict(screenshot_url='http://foo.com:42/broken',
164 section='required', name='required', package_name='required'))165 section='required', name='required', package_name='required',
166 distroseries=distroseries.id))
165 self.assertTrue(form.is_valid())167 self.assertTrue(form.is_valid())
166168
167169

Subscribers

People subscribed via source and target branches