Launchpad itself

Merge lp:~brian-murray/launchpad/bug-supervisor-bug-expiry into lp:launchpad

  1. bug-supervisor-bug-expiry
  2. Merge into devel
Proposed by Brian Murray
Status: Merged
Approved by: Edwin Grubbs
Approved revision: no longer in the source branch.
Merged at revision: 11809
Proposed branch: lp:~brian-murray/launchpad/bug-supervisor-bug-expiry
Merge into: lp:launchpad
Diff against target: 141 lines (+45/-21)
5 files modified
lib/lp/bugs/browser/bugtarget.py (+15/-9)
lib/lp/bugs/browser/configure.zcml (+1/-1)
lib/lp/bugs/browser/tests/test_bugtarget_configure.py (+27/-9)
lib/lp/bugs/tests/test_bugnotification.py (+1/-1)
lib/lp/registry/browser/product.py (+1/-1)
To merge this branch: bzr merge lp:~brian-murray/launchpad/bug-supervisor-bug-expiry
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+38958@code.launchpad.net

Commit message

Allow bug supervisors to configure the bug tracker section of their project.

Description of the change

Recently Deryck was trying to communicate with people about enabling bug expiry and sent an email to bug supervisors for projects indicating how they could enable bug expiry. However, bug supervisor's don't actually have access to +configure-bugtracker so were unable to act on this. After discussion with Deryck this branch was written to allow bug supervisors of projects to the link to +configure-bugtracker and modifies the fields that appear on +configure-bugtracker so that modifying the bug_supervisor and security_contact of the project is only possible by the owner and driver of the project.

I added a test to test_bugtarget_configure.py that ensures that the correct fields appear for the right people.

bin/test -cvvt test_bugtarget_configure.py

To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Brian,

This is a nice improvement, but I'm worried about the "Configure bugtracker" link appearing on bugs.lp.net but not on lp.net. Apparently, this is caused by bugtarget-bugs.pt not using pillar-involvement-portlet.pt, and pillar-involvement-portlet.pt wraps all the links in a context/required:launchpad.Edit.

Unless there is a good reason to have the portlet appear differently on those two pages, I think it would be beneficial to make bugtarget-bugs.pt use the portlet. This will definitely take some more sophisticated permission checks to make it look write for launchpad.Edit and for launchpad.BugSupervisor.

I have one comment below.

-Edwin

>=== modified file 'lib/lp/bugs/browser/bugtarget.py'
>--- lib/lp/bugs/browser/bugtarget.py 2010-10-06 16:12:16 +0000
>+++ lib/lp/bugs/browser/bugtarget.py 2010-10-20 19:13:30 +0000
>@@ -176,21 +176,30 @@
>
> label = "Configure bug tracker"
> schema = IProductBugConfiguration
>- field_names = [
>- "bug_supervisor",
>- "security_contact",
>- "bugtracker",
>- "enable_bug_expiration",
>- "remote_product",
>- "bug_reporting_guidelines",
>- "bug_reported_acknowledgement",
>- ]
> # This ProductBugTrackerWidget renders enable_bug_expiration and
> # remote_product as subordinate fields, so this view suppresses them.
> custom_widget('bugtracker', ProductBugTrackerWidget)
> custom_widget('enable_bug_expiration', GhostCheckBoxWidget)
> custom_widget('remote_product', GhostWidget)
>
>+ def __init__(self, context, request):
>+ ProductConfigureBase.__init__(self, context, request)
>+

You can remove this method override.

>+ @property
>+ def field_names(self):
>+ """Return the list of field names to display."""
>+ field_names = [
>+ "bugtracker",
>+ "enable_bug_expiration",
>+ "remote_product",
>+ "bug_reporting_guidelines",
>+ "bug_reported_acknowledgement",
>+ ]
>+ if check_permission("launchpad.Edit", self.context):
>+ field_names.extend(["bug_supervisor", "security_contact"])
>+
>+ return field_names
>+
> def validate(self, data):
> """Constrain bug expiration to Launchpad Bugs tracker."""
> self.validateBugSupervisor(data)

review: Needs Fixing (code)
Revision history for this message
Brian Murray (brian-murray) wrote :

On Wed, Oct 20, 2010 at 07:36:53PM -0000, Edwin Grubbs wrote:
> Review: Needs Fixing code
> Hi Brian,
>
> This is a nice improvement, but I'm worried about the "Configure
> bugtracker" link appearing on bugs.lp.net but not on lp.net.
> Apparently, this is caused by bugtarget-bugs.pt not using
> pillar-involvement-portlet.pt, and pillar-involvement-portlet.pt wraps
> all the links in a context/required:launchpad.Edit.
>
> Unless there is a good reason to have the portlet appear differently
> on those two pages, I think it would be beneficial to make
> bugtarget-bugs.pt use the portlet. This will definitely take some more
> sophisticated permission checks to make it look write for
> launchpad.Edit and for launchpad.BugSupervisor.

I was working on this branch partially during my off work hours as a
favor to Deryck and due to a promise of free beer. I agree that having
the "Configure bug tracker" link appearing on one page and not another
is odd so I briefly looked at modifying the branch to meet your
requirements. However, as much as I like beer, it looks like more work
than I'm willing to commit to at this point in time.

> I have one comment below.

I'll modify the branch fixing your comment regarding the code in it.

--
Brian Murray

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Brian,

Can you go ahead and open a bug for the remaining work? You can land this then.

-Edwin

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/bugs/browser/bugtarget.py'
2--- lib/lp/bugs/browser/bugtarget.py 2010-10-06 16:12:16 +0000
3+++ lib/lp/bugs/browser/bugtarget.py 2010-10-21 20:19:52 +0000
4@@ -176,21 +176,27 @@
5
6 label = "Configure bug tracker"
7 schema = IProductBugConfiguration
8- field_names = [
9- "bug_supervisor",
10- "security_contact",
11- "bugtracker",
12- "enable_bug_expiration",
13- "remote_product",
14- "bug_reporting_guidelines",
15- "bug_reported_acknowledgement",
16- ]
17 # This ProductBugTrackerWidget renders enable_bug_expiration and
18 # remote_product as subordinate fields, so this view suppresses them.
19 custom_widget('bugtracker', ProductBugTrackerWidget)
20 custom_widget('enable_bug_expiration', GhostCheckBoxWidget)
21 custom_widget('remote_product', GhostWidget)
22
23+ @property
24+ def field_names(self):
25+ """Return the list of field names to display."""
26+ field_names = [
27+ "bugtracker",
28+ "enable_bug_expiration",
29+ "remote_product",
30+ "bug_reporting_guidelines",
31+ "bug_reported_acknowledgement",
32+ ]
33+ if check_permission("launchpad.Edit", self.context):
34+ field_names.extend(["bug_supervisor", "security_contact"])
35+
36+ return field_names
37+
38 def validate(self, data):
39 """Constrain bug expiration to Launchpad Bugs tracker."""
40 self.validateBugSupervisor(data)
41
42=== modified file 'lib/lp/bugs/browser/configure.zcml'
43--- lib/lp/bugs/browser/configure.zcml 2010-10-14 20:20:47 +0000
44+++ lib/lp/bugs/browser/configure.zcml 2010-10-21 20:19:52 +0000
45@@ -397,7 +397,7 @@
46 <browser:page
47 for="lp.registry.interfaces.product.IProduct"
48 facet="bugs"
49- permission="launchpad.Edit"
50+ permission="launchpad.BugSupervisor"
51 name="+configure-bugtracker"
52 template="../../app/templates/generic-edit.pt"
53 class="lp.bugs.browser.bugtarget.ProductConfigureBugTrackerView"/>
54
55=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_configure.py'
56--- lib/lp/bugs/browser/tests/test_bugtarget_configure.py 2010-10-04 19:50:45 +0000
57+++ lib/lp/bugs/browser/tests/test_bugtarget_configure.py 2010-10-21 20:19:52 +0000
58@@ -9,6 +9,7 @@
59 from lp.app.enums import ServiceUsage
60 from lp.testing import (
61 login_person,
62+ logout,
63 TestCaseWithFactory,
64 )
65 from lp.testing.views import create_initialized_view
66@@ -21,8 +22,11 @@
67 def setUp(self):
68 super(TestProductBugConfigurationView, self).setUp()
69 self.owner = self.factory.makePerson(name='boing-owner')
70+ self.bug_supervisor = self.factory.makePerson(
71+ name='boing-bug-supervisor')
72 self.product = self.factory.makeProduct(
73- name='boing', owner=self.owner)
74+ name='boing', owner=self.owner,
75+ bug_supervisor=self.bug_supervisor)
76 login_person(self.owner)
77
78 def _makeForm(self):
79@@ -37,14 +41,28 @@
80 'field.actions.change': 'Change',
81 }
82
83- def test_view_attributes(self):
84- view = create_initialized_view(
85- self.product, name='+configure-bugtracker')
86- label = 'Configure bug tracker'
87- self.assertEqual(label, view.label)
88- fields = [
89- 'bug_supervisor', 'security_contact', 'bugtracker',
90- 'enable_bug_expiration', 'remote_product',
91+ def test_owner_view_attributes(self):
92+ view = create_initialized_view(
93+ self.product, name='+configure-bugtracker')
94+ label = 'Configure bug tracker'
95+ self.assertEqual(label, view.label)
96+ fields = [
97+ 'bugtracker', 'enable_bug_expiration', 'remote_product',
98+ 'bug_reporting_guidelines', 'bug_reported_acknowledgement',
99+ 'bug_supervisor', 'security_contact']
100+ self.assertEqual(fields, view.field_names)
101+ self.assertEqual('http://launchpad.dev/boing', view.next_url)
102+ self.assertEqual('http://launchpad.dev/boing', view.cancel_url)
103+
104+ def test_bug_supervisor_view_attributes(self):
105+ logout()
106+ login_person(self.bug_supervisor)
107+ view = create_initialized_view(
108+ self.product, name='+configure-bugtracker')
109+ label = 'Configure bug tracker'
110+ self.assertEqual(label, view.label)
111+ fields = [
112+ 'bugtracker', 'enable_bug_expiration', 'remote_product',
113 'bug_reporting_guidelines', 'bug_reported_acknowledgement']
114 self.assertEqual(fields, view.field_names)
115 self.assertEqual('http://launchpad.dev/boing', view.next_url)
116
117=== modified file 'lib/lp/bugs/tests/test_bugnotification.py'
118--- lib/lp/bugs/tests/test_bugnotification.py 2010-10-14 15:22:46 +0000
119+++ lib/lp/bugs/tests/test_bugnotification.py 2010-10-21 20:19:52 +0000
120@@ -15,7 +15,7 @@
121 from canonical.config import config
122 from canonical.launchpad.database.message import MessageSet
123 from canonical.launchpad.ftests import login
124-from canonical.testing.layers import (
125+from canonical.testing import (
126 DatabaseFunctionalLayer,
127 LaunchpadFunctionalLayer,
128 LaunchpadZopelessLayer,
129
130=== modified file 'lib/lp/registry/browser/product.py'
131--- lib/lp/registry/browser/product.py 2010-10-06 15:00:24 +0000
132+++ lib/lp/registry/browser/product.py 2010-10-21 20:19:52 +0000
133@@ -511,7 +511,7 @@
134 text = 'Change details'
135 return Link('+edit', text, icon='edit')
136
137- @enabled_with_permission('launchpad.Edit')
138+ @enabled_with_permission('launchpad.BugSupervisor')
139 def configure_bugtracker(self):
140 text = 'Configure bug tracker'
141 summary = 'Specify where bugs are tracked for this project'