launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/nomination-limitation into lp:launchpad.

Commit message:
Add a feature flag to open up bug targeting to bug supervisors.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/nomination-limitation/+merge/307672

Add a feature flag to open up bug targeting to bug supervisors.

In the beginning, bug nomination was open to everyone, and targeting was
restricted to drivers. Over time, targeting was opened up to uploaders,
and nomination locked down to bug supervisors.

Now we have requests to open targeting up further, but it would be
lovely to avoid adding yet another hardcoded role. Since the vast
majority of recent nominations are approved, let's try opening targeting
up to the (usually semi-vetted) bug supervisor and see what happens.

Setting this feature flag effectively disables the creation of new
unapproved nominations by extending targeting privileges to everyone who
can nominate.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/nomination-limitation into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2015-10-15 14:09:50 +0000
+++ lib/lp/bugs/browser/bug.py	2016-10-05 09:04:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """IBug related view classes."""
@@ -104,6 +104,7 @@
     get_structural_subscriptions_for_bug,
     )
 from lp.registry.interfaces.person import IPersonSet
+from lp.services.features import getFeatureFlag
 from lp.services.fields import DuplicateBug
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.mail.mailwrapper import MailWrapper
@@ -304,7 +305,9 @@
         """Return the 'Target/Nominate for series' Link."""
         launchbag = getUtility(ILaunchBag)
         target = launchbag.product or launchbag.distribution
-        if check_permission("launchpad.Driver", target):
+        if check_permission("launchpad.Driver", target) or (
+                getFeatureFlag('bugs.nominations.bug_supervisors_can_target')
+                and check_permission("launchpad.BugSupervisor", target)):
             text = "Target to series"
             return Link('+nominate', text, icon='milestone')
         elif (check_permission("launchpad.BugSupervisor", target) or

=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py	2014-11-28 22:07:05 +0000
+++ lib/lp/bugs/browser/bugnomination.py	2016-10-05 09:04:15 +0000
@@ -30,6 +30,7 @@
     IBugNominationForm,
     )
 from lp.bugs.interfaces.cve import ICveSet
+from lp.services.features import getFeatureFlag
 from lp.services.webapp import (
     canonical_url,
     LaunchpadView,
@@ -52,9 +53,9 @@
         LaunchpadFormView.initialize(self)
         # Update the submit label based on the user's permission.
         submit_action = self.__class__.actions.byname['actions.submit']
-        if self.userIsReleaseManager():
+        if self.userCanTarget():
             submit_action.label = _("Target")
-        elif self.userIsBugSupervisor():
+        elif self.userCanNominate():
             submit_action.label = _("Nominate")
         else:
             self.request.response.addErrorNotification(
@@ -68,19 +69,23 @@
 
         The label returned depends on the user's privileges.
         """
-        if self.userIsReleaseManager():
+        if self.userCanTarget():
             return "Target bug #%d to series" % self.context.bug.id
         else:
             return "Nominate bug #%d for series" % self.context.bug.id
 
     page_title = label
 
-    def userIsReleaseManager(self):
-        """Does the current user have release management privileges?"""
-        return self.current_bugtask.userHasDriverPrivileges(self.user)
+    def userCanTarget(self):
+        """Can the current user target the bug to a series?"""
+        return (
+            self.current_bugtask.userHasDriverPrivileges(self.user)
+            or (getFeatureFlag('bugs.nominations.bug_supervisors_can_target')
+                and self.current_bugtask.userHasBugSupervisorPrivileges(
+                    self.user)))
 
-    def userIsBugSupervisor(self):
-        """Is the current user the bug supervisor?"""
+    def userCanNominate(self):
+        """Can the current user nominate the bug for a series?"""
         return self.current_bugtask.userHasBugSupervisorPrivileges(
             self.user)
 

=== modified file 'lib/lp/bugs/browser/tests/test_bugnomination.py'
--- lib/lp/bugs/browser/tests/test_bugnomination.py	2012-10-11 14:57:32 +0000
+++ lib/lp/bugs/browser/tests/test_bugnomination.py	2016-10-05 09:04:15 +0000
@@ -12,6 +12,7 @@
 from zope.component import getUtility
 
 from lp.registry.interfaces.series import SeriesStatus
+from lp.services.features.testing import FeatureFixture
 from lp.services.webapp.interaction import get_current_principal
 from lp.services.webapp.interfaces import (
     BrowserNotificationLevel,
@@ -63,6 +64,16 @@
         action = view.__class__.actions.byname['actions.submit']
         self.assertEqual('Nominate', action.label)
 
+    def test_submit_action_bug_supervisor_feature_flag(self):
+        # A bug supervisor sees the Target action label when the feature
+        # flag is enabled.
+        self.useFixture(FeatureFixture(
+            {'bugs.nominations.bug_supervisors_can_target': 'on'}))
+        login_person(self.bug_worker)
+        view = create_initialized_view(self.bug_task, name='+nominate')
+        action = view.__class__.actions.byname['actions.submit']
+        self.assertEqual('Target', action.label)
+
     def test_submit_action_driver(self):
         # A driver sees the Target action label.
         login_person(self.distribution.driver)

=== modified file 'lib/lp/bugs/model/bugnomination.py'
--- lib/lp/bugs/model/bugnomination.py	2015-07-08 16:05:11 +0000
+++ lib/lp/bugs/model/bugnomination.py	2016-10-05 09:04:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes related to bug nomination.
@@ -37,6 +37,7 @@
 from lp.services.database.datetimecol import UtcDateTimeCol
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.sqlbase import SQLBase
+from lp.services.features import getFeatureFlag
 
 
 @implementer(IBugNomination)
@@ -122,6 +123,12 @@
         # Use the class method to check permissions because there is not
         # yet a bugtask instance with the this target.
         BugTask = self.bug.bugtasks[0].__class__
+
+        if (getFeatureFlag('bugs.nominations.bug_supervisors_can_target') and
+                BugTask.userHasBugSupervisorPrivilegesContext(
+                    self.target, person)):
+            return True
+
         if BugTask.userHasDriverPrivilegesContext(self.target, person):
             return True
 

=== modified file 'lib/lp/bugs/templates/bug-nominate-for-series.pt'
--- lib/lp/bugs/templates/bug-nominate-for-series.pt	2010-12-21 19:11:49 +0000
+++ lib/lp/bugs/templates/bug-nominate-for-series.pt	2016-10-05 09:04:15 +0000
@@ -14,14 +14,14 @@
     <div metal:use-macro="context/@@launchpad_form/form">
 
       <div metal:fill-slot="extra_info">
-        <tal:targeting-mode condition="view/userIsReleaseManager">
+        <tal:targeting-mode condition="view/userCanTarget">
           <p>
           Series targeting allows you to track the status, assignee, and
           importance of a bug in one or more official series.
           </p>
         </tal:targeting-mode>
 
-        <tal:nomination-mode condition="not:view/userIsReleaseManager">
+        <tal:nomination-mode condition="not:view/userCanTarget">
           <p>
           Bug nominations are evaluated by release managers and accepted
           or declined for fixing in a series.

=== modified file 'lib/lp/bugs/tests/test_bugnomination.py'
--- lib/lp/bugs/tests/test_bugnomination.py	2012-11-26 08:33:03 +0000
+++ lib/lp/bugs/tests/test_bugnomination.py	2016-10-05 09:04:15 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests related to bug nominations."""
@@ -14,6 +14,7 @@
     IBugNomination,
     IBugNominationSet,
     )
+from lp.services.features.testing import FeatureFixture
 from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 from lp.testing import (
     celebrity_logged_in,
@@ -308,6 +309,26 @@
         self.assertTrue(nomination.canApprove(product.driver))
         self.assertTrue(nomination.canApprove(series.driver))
 
+    def test_bug_supervisors_can_approve_with_feature_flag(self):
+        product = self.factory.makeProduct(driver=self.factory.makePerson())
+        series = self.factory.makeProductSeries(product=product)
+        with celebrity_logged_in('admin'):
+            product.bug_supervisor = self.factory.makePerson()
+            product.driver = self.factory.makePerson()
+            series.driver = self.factory.makePerson()
+
+        nomination = self.factory.makeBugNomination(target=series)
+
+        self.assertFalse(nomination.canApprove(product.bug_supervisor))
+        self.assertTrue(nomination.canApprove(product.driver))
+        self.assertTrue(nomination.canApprove(series.driver))
+
+        with FeatureFixture({
+                'bugs.nominations.bug_supervisors_can_target': 'on'}):
+            self.assertTrue(nomination.canApprove(product.bug_supervisor))
+            self.assertTrue(nomination.canApprove(product.driver))
+            self.assertTrue(nomination.canApprove(series.driver))
+
     def publishSource(self, series, sourcepackagename, component):
         return self.factory.makeSourcePackagePublishingHistory(
             archive=series.main_archive,