launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/notification-already-targetted into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #907840 in Launchpad itself: "Permissions on +nominate are messed up"
  https://bugs.launchpad.net/launchpad/+bug/907840

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/notification-already-targetted/+merge/89826

This branch is badly named, but it solves the bug where a distribution source package targetted bug has been nominated to a series and then a user attempts to +nominate using the series task. BugNominationView.userIsBugSupervisor() was checking the target only and ignoring the distribution-wide rules. As such, I have ripped out the check_permission() calls and replaced them with IBugTask.userHas{Driver,BugSupervisor}Privileges() calls.
-- 
https://code.launchpad.net/~stevenk/launchpad/notification-already-targetted/+merge/89826
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/notification-already-targetted into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py	2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/browser/bugnomination.py	2012-01-24 05:16:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser view classes related to bug nominations."""
@@ -90,13 +90,12 @@
 
     def userIsReleaseManager(self):
         """Does the current user have release management privileges?"""
-        return check_permission(
-            "launchpad.Driver", self.current_bugtask.target)
+        return self.current_bugtask.userHasDriverPrivileges(self.user)
 
     def userIsBugSupervisor(self):
         """Is the current user the bug supervisor?"""
-        return check_permission(
-            "launchpad.BugSupervisor", self.current_bugtask.target)
+        return self.current_bugtask.userHasBugSupervisorPrivileges(
+            self.user)
 
     def userCanChangeDriver(self):
         """Can the current user set the release management team?"""

=== modified file 'lib/lp/bugs/browser/tests/test_bugnomination.py'
--- lib/lp/bugs/browser/tests/test_bugnomination.py	2012-01-15 13:32:27 +0000
+++ lib/lp/bugs/browser/tests/test_bugnomination.py	2012-01-24 05:16:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for bug nomination views."""
@@ -145,6 +145,25 @@
         view = create_initialized_view(series_bugtask, name='+nominate')
         self.assertEqual(0, len(view.request.notifications))
 
+    def test_series_targets_allow_nomination(self):
+        # When a bug is already nominated for a series, the view checks
+        # for bug supervisor permission on the series correctly.
+        person = self.factory.makePerson()
+        dsp = self.factory.makeDistributionSourcePackage()
+        series = self.factory.makeDistroSeries(distribution=dsp.distribution)
+        self._makeBugSupervisorTeam(
+            person, dsp.distribution.owner, dsp.distribution)
+        bug = self.factory.makeBug(
+            distribution=dsp.distribution,
+            sourcepackagename=dsp.sourcepackagename)
+        with person_logged_in(dsp.distribution.owner):
+            nomination = bug.addNomination(dsp.distribution.owner, series)
+            nomination.approve(person)
+        series_bugtask = bug.bugtasks[1]
+        with person_logged_in(person):
+            view = create_initialized_view(series_bugtask, name='+nominate')
+            self.assertEqual(0, len(view.request.notifications))
+
 
 class TestBugEditLinks(TestCaseWithFactory):