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
1=== modified file 'lib/lp/registry/browser/product.py'
2--- lib/lp/registry/browser/product.py 2009-11-20 07:32:32 +0000
3+++ lib/lp/registry/browser/product.py 2009-11-23 15:20:31 +0000
4@@ -1147,7 +1147,18 @@
5 return self.next_url
6
7
8-class ProductAdminView(ProductEditView):
9+class EditPrivateBugsMixin:
10+
11+ def validate_private_bugs(self, data):
12+ """Perform validation for the private bugs setting."""
13+ if data.get('private_bugs') and self.context.bug_supervisor is None:
14+ self.setFieldError('private_bugs',
15+ structured(
16+ 'Set a <a href="%s/+bugsupervisor">bug supervisor</a> '
17+ 'for this project first.',
18+ canonical_url(self.context, rootsite="bugs")))
19+
20+class ProductAdminView(ProductEditView, EditPrivateBugsMixin):
21 label = "Administer project details"
22 field_names = ["name", "owner", "active", "autoupdate", "private_bugs"]
23
24@@ -1201,12 +1212,7 @@
25
26 def validate(self, data):
27 """See `LaunchpadFormView`."""
28- if data.get('private_bugs') and self.context.bug_supervisor is None:
29- self.setFieldError('private_bugs',
30- structured(
31- 'Set a <a href="%s/+bugsupervisor">bug supervisor</a> '
32- 'for this project first.',
33- canonical_url(self.context, rootsite="bugs")))
34+ self.validate_private_bugs(data)
35
36 @property
37 def cancel_url(self):
38@@ -1214,7 +1220,7 @@
39 return canonical_url(self.context)
40
41
42-class ProductReviewLicenseView(ProductEditView):
43+class ProductReviewLicenseView(ProductEditView, EditPrivateBugsMixin):
44 """A view to review a project and change project privileges."""
45 label = "Review project"
46 field_names = [
47@@ -1231,11 +1237,10 @@
48 return 'Review %s' % self.context.title
49
50 def validate(self, data):
51- """See `LaunchpadFormView`.
52+ """See `LaunchpadFormView`."""
53
54- A project can only be approved if it has OTHER_OPEN_SOURCE as one of
55- its licenses and not OTHER_PROPRIETARY.
56- """
57+ # A project can only be approved if it has OTHER_OPEN_SOURCE as one of
58+ # its licenses and not OTHER_PROPRIETARY.
59 licenses = self.context.licenses
60 license_approved = data.get('license_approved', False)
61 if license_approved:
62@@ -1251,6 +1256,10 @@
63 # approved.
64 pass
65
66+ # Private bugs can only be enabled if the product has a bug
67+ # supervisor.
68+ self.validate_private_bugs(data)
69+
70 @property
71 def next_url(self):
72 """See `LaunchpadFormView`."""
73
74=== modified file 'lib/lp/registry/browser/tests/product-views.txt'
75--- lib/lp/registry/browser/tests/product-views.txt 2009-11-19 04:31:41 +0000
76+++ lib/lp/registry/browser/tests/product-views.txt 2009-11-23 15:20:31 +0000
77@@ -188,6 +188,35 @@
78 ... }
79 >>> view = create_initialized_view(
80 ... firefox, name='+review-license', form=form)
81+
82+He encounters an error, though, as the project does not have a bug
83+supervisor set, which is required for turning on private bugs.
84+
85+ >>> from canonical.launchpad.testing.pages import extract_text
86+ >>> for error in view.errors:
87+ ... print extract_text(error)
88+ Set a bug supervisor for this project first.
89+
90+ >>> firefox.bug_supervisor is None
91+ True
92+
93+After setting the bug supervisor the project can have private bugs enabled.
94+
95+ >>> login('mark@example.com')
96+ >>> firefox.setBugSupervisor(cprov, mark)
97+ >>> login('commercial-member@canonical.com')
98+
99+ >>> firefox.private_bugs
100+ False
101+
102+ >>> form = {
103+ ... 'field.active': 'on',
104+ ... 'field.private_bugs': 'on',
105+ ... 'field.reviewer_whiteboard': 'Reinstated old project',
106+ ... 'field.actions.change': 'Change',
107+ ... }
108+ >>> view = create_initialized_view(
109+ ... firefox, name='+review-license', form=form)
110 >>> view.errors
111 []
112 >>> firefox.active