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

[Brian Murray]

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

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

[Edwin Grubbs (edwin-grubbs)]

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)

[Brian Murray (brian-murray)]

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

[Edwin Grubbs (edwin-grubbs)]

Brian,

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

-Edwin

review: Approve (code)

=== 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-21 20:19:52 +0000
@@ -176,21 +176,27 @@
 
     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)
 
+    @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)

=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml	2010-10-14 20:20:47 +0000
+++ lib/lp/bugs/browser/configure.zcml	2010-10-21 20:19:52 +0000
@@ -397,7 +397,7 @@
     <browser:page
         for="lp.registry.interfaces.product.IProduct"
         facet="bugs"
-        permission="launchpad.Edit"
+        permission="launchpad.BugSupervisor"
         name="+configure-bugtracker"
         template="../../app/templates/generic-edit.pt"
         class="lp.bugs.browser.bugtarget.ProductConfigureBugTrackerView"/>

=== modified file 'lib/lp/bugs/browser/tests/test_bugtarget_configure.py'
--- lib/lp/bugs/browser/tests/test_bugtarget_configure.py	2010-10-04 19:50:45 +0000
+++ lib/lp/bugs/browser/tests/test_bugtarget_configure.py	2010-10-21 20:19:52 +0000
@@ -9,6 +9,7 @@
 from lp.app.enums import ServiceUsage
 from lp.testing import (
     login_person,
+    logout,
     TestCaseWithFactory,
     )
 from lp.testing.views import create_initialized_view
@@ -21,8 +22,11 @@
     def setUp(self):
         super(TestProductBugConfigurationView, self).setUp()
         self.owner = self.factory.makePerson(name='boing-owner')
+        self.bug_supervisor = self.factory.makePerson(
+            name='boing-bug-supervisor')
         self.product = self.factory.makeProduct(
-            name='boing', owner=self.owner)
+            name='boing', owner=self.owner,
+            bug_supervisor=self.bug_supervisor)
         login_person(self.owner)
 
     def _makeForm(self):
@@ -37,14 +41,28 @@
             'field.actions.change': 'Change',
             }
 
-    def test_view_attributes(self):
-        view = create_initialized_view(
-            self.product, name='+configure-bugtracker')
-        label = 'Configure bug tracker'
-        self.assertEqual(label, view.label)
-        fields = [
-            'bug_supervisor', 'security_contact', 'bugtracker',
-            'enable_bug_expiration', 'remote_product',
+    def test_owner_view_attributes(self):
+        view = create_initialized_view(
+            self.product, name='+configure-bugtracker')
+        label = 'Configure bug tracker'
+        self.assertEqual(label, view.label)
+        fields = [
+            'bugtracker', 'enable_bug_expiration', 'remote_product',
+            'bug_reporting_guidelines', 'bug_reported_acknowledgement',
+            'bug_supervisor', 'security_contact']
+        self.assertEqual(fields, view.field_names)
+        self.assertEqual('http://launchpad.dev/boing', view.next_url)
+        self.assertEqual('http://launchpad.dev/boing', view.cancel_url)
+
+    def test_bug_supervisor_view_attributes(self):
+        logout()
+        login_person(self.bug_supervisor)
+        view = create_initialized_view(
+            self.product, name='+configure-bugtracker')
+        label = 'Configure bug tracker'
+        self.assertEqual(label, view.label)
+        fields = [
+            'bugtracker', 'enable_bug_expiration', 'remote_product',
             'bug_reporting_guidelines', 'bug_reported_acknowledgement']
         self.assertEqual(fields, view.field_names)
         self.assertEqual('http://launchpad.dev/boing', view.next_url)

=== modified file 'lib/lp/bugs/tests/test_bugnotification.py'
--- lib/lp/bugs/tests/test_bugnotification.py	2010-10-14 15:22:46 +0000
+++ lib/lp/bugs/tests/test_bugnotification.py	2010-10-21 20:19:52 +0000
@@ -15,7 +15,7 @@
 from canonical.config import config
 from canonical.launchpad.database.message import MessageSet
 from canonical.launchpad.ftests import login
-from canonical.testing.layers import (
+from canonical.testing import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
     LaunchpadZopelessLayer,

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2010-10-06 15:00:24 +0000
+++ lib/lp/registry/browser/product.py	2010-10-21 20:19:52 +0000
@@ -511,7 +511,7 @@
         text = 'Change details'
         return Link('+edit', text, icon='edit')
 
-    @enabled_with_permission('launchpad.Edit')
+    @enabled_with_permission('launchpad.BugSupervisor')
     def configure_bugtracker(self):
         text = 'Configure bug tracker'
         summary = 'Specify where bugs are tracked for this project'