Launchpad itself

Merge lp:~bac/launchpad/bug-266890 into lp:launchpad

  1. bug-266890
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Aaron Bentley
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-266890
Merge into: lp:launchpad
Diff against target: 112 lines (+50/-12)
2 files modified
lib/lp/registry/browser/product.py (+21/-12)
lib/lp/registry/browser/tests/product-views.txt (+29/-0)
To merge this branch: bzr merge lp:~bac/launchpad/bug-266890
Reviewer Review Type Date Requested Status
Aaron Bentley (community) code Approve
Review via email: mp+15153@code.launchpad.net

Commit message

Prevent setting private bugs from causing an OOPS when constraints aren't satisfied.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Setting private bugs on the +review-license page for a project with no
bug supervisor set causes an OOPS due to a db constraint.

== Proposed fix ==

Add a test in the form validation to flag the attempt to set private
bugs. I found that the +admin page already does the same thing so the
code was refactored.

== Pre-implementation notes ==

None

== Implementation details ==

As above.

== Tests ==

bin/test -vvt product-views.txt

== Demo and Q/A ==

As mark, go to https://launchpad.dev/firefox/+review-license and turn on
private bugs. Note the nice error message.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/tests/product-views.txt

== Pylint notices ==

lib/lp/registry/browser/product.py
    54: [F0401] Unable to import 'lazr.delegates' (No module named
delegates)

Revision history for this message
Aaron Bentley (abentley) wrote :

Makes sense to me, and it has tests.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2009-11-20 07:32:32 +0000
+++ lib/lp/registry/browser/product.py 2009-11-23 15:20:31 +0000
@@ -1147,7 +1147,18 @@
1147 return self.next_url1147 return self.next_url
11481148
11491149
1150class ProductAdminView(ProductEditView):1150class EditPrivateBugsMixin:
1151
1152 def validate_private_bugs(self, data):
1153 """Perform validation for the private bugs setting."""
1154 if data.get('private_bugs') and self.context.bug_supervisor is None:
1155 self.setFieldError('private_bugs',
1156 structured(
1157 'Set a <a href="%s/+bugsupervisor">bug supervisor</a> '
1158 'for this project first.',
1159 canonical_url(self.context, rootsite="bugs")))
1160
1161class ProductAdminView(ProductEditView, EditPrivateBugsMixin):
1151 label = "Administer project details"1162 label = "Administer project details"
1152 field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]1163 field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]
11531164
@@ -1201,12 +1212,7 @@
12011212
1202 def validate(self, data):1213 def validate(self, data):
1203 """See `LaunchpadFormView`."""1214 """See `LaunchpadFormView`."""
1204 if data.get('private_bugs') and self.context.bug_supervisor is None:1215 self.validate_private_bugs(data)
1205 self.setFieldError('private_bugs',
1206 structured(
1207 'Set a <a href="%s/+bugsupervisor">bug supervisor</a> '
1208 'for this project first.',
1209 canonical_url(self.context, rootsite="bugs")))
12101216
1211 @property1217 @property
1212 def cancel_url(self):1218 def cancel_url(self):
@@ -1214,7 +1220,7 @@
1214 return canonical_url(self.context)1220 return canonical_url(self.context)
12151221
12161222
1217class ProductReviewLicenseView(ProductEditView):1223class ProductReviewLicenseView(ProductEditView, EditPrivateBugsMixin):
1218 """A view to review a project and change project privileges."""1224 """A view to review a project and change project privileges."""
1219 label = "Review project"1225 label = "Review project"
1220 field_names = [1226 field_names = [
@@ -1231,11 +1237,10 @@
1231 return 'Review %s' % self.context.title1237 return 'Review %s' % self.context.title
12321238
1233 def validate(self, data):1239 def validate(self, data):
1234 """See `LaunchpadFormView`.1240 """See `LaunchpadFormView`."""
12351241
1236 A project can only be approved if it has OTHER_OPEN_SOURCE as one of1242 # A project can only be approved if it has OTHER_OPEN_SOURCE as one of
1237 its licenses and not OTHER_PROPRIETARY.1243 # its licenses and not OTHER_PROPRIETARY.
1238 """
1239 licenses = self.context.licenses1244 licenses = self.context.licenses
1240 license_approved = data.get('license_approved', False)1245 license_approved = data.get('license_approved', False)
1241 if license_approved:1246 if license_approved:
@@ -1251,6 +1256,10 @@
1251 # approved.1256 # approved.
1252 pass1257 pass
12531258
1259 # Private bugs can only be enabled if the product has a bug
1260 # supervisor.
1261 self.validate_private_bugs(data)
1262
1254 @property1263 @property
1255 def next_url(self):1264 def next_url(self):
1256 """See `LaunchpadFormView`."""1265 """See `LaunchpadFormView`."""
12571266
=== modified file 'lib/lp/registry/browser/tests/product-views.txt'
--- lib/lp/registry/browser/tests/product-views.txt 2009-11-19 04:31:41 +0000
+++ lib/lp/registry/browser/tests/product-views.txt 2009-11-23 15:20:31 +0000
@@ -188,6 +188,35 @@
188 ... }188 ... }
189 >>> view = create_initialized_view(189 >>> view = create_initialized_view(
190 ... firefox, name='+review-license', form=form)190 ... firefox, name='+review-license', form=form)
191
192He encounters an error, though, as the project does not have a bug
193supervisor set, which is required for turning on private bugs.
194
195 >>> from canonical.launchpad.testing.pages import extract_text
196 >>> for error in view.errors:
197 ... print extract_text(error)
198 Set a bug supervisor for this project first.
199
200 >>> firefox.bug_supervisor is None
201 True
202
203After setting the bug supervisor the project can have private bugs enabled.
204
205 >>> login('mark@example.com')
206 >>> firefox.setBugSupervisor(cprov, mark)
207 >>> login('commercial-member@canonical.com')
208
209 >>> firefox.private_bugs
210 False
211
212 >>> form = {
213 ... 'field.active': 'on',
214 ... 'field.private_bugs': 'on',
215 ... 'field.reviewer_whiteboard': 'Reinstated old project',
216 ... 'field.actions.change': 'Change',
217 ... }
218 >>> view = create_initialized_view(
219 ... firefox, name='+review-license', form=form)
191 >>> view.errors220 >>> view.errors
192 []221 []
193 >>> firefox.active222 >>> firefox.active