launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/target-series-bug-834082 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #834082 in Launchpad itself: "Cannot target a bug to a series from an existing series task"
  https://bugs.launchpad.net/launchpad/+bug/834082

For more details, see:
https://code.launchpad.net/~deryck/launchpad/target-series-bug-834082/+merge/77966

r13069 introduced a bug where people could no longer target a bug to a series.  The issue was that BugNominationView was changed to rely on self.userIsBugSupervisor and self.userIsReleaseManager to check if the error message should be added or not.  Unfortunately, these methods were getting the bugtask from ILaunchBug, rather that the context.

The branch I have here for review fixes these methods to use self.current_bugtask which is set from the current context.  Test coverage is added ass well, to ensure that a bug supervisor doesn't get an error notification.
-- 
https://code.launchpad.net/~deryck/launchpad/target-series-bug-834082/+merge/77966
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/target-series-bug-834082 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py	2011-05-17 06:36:31 +0000
+++ lib/lp/bugs/browser/bugnomination.py	2011-10-03 15:28:45 +0000
@@ -90,15 +90,13 @@
 
     def userIsReleaseManager(self):
         """Does the current user have release management privileges?"""
-        current_bugtask = getUtility(ILaunchBag).bugtask
         return check_permission(
-            "launchpad.Driver", current_bugtask.target)
+            "launchpad.Driver", self.current_bugtask.target)
 
     def userIsBugSupervisor(self):
         """Is the current user the bug supervisor?"""
-        current_bugtask = getUtility(ILaunchBag).bugtask
         return check_permission(
-            "launchpad.BugSupervisor", current_bugtask.target)
+            "launchpad.BugSupervisor", self.current_bugtask.target)
 
     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	2011-05-17 06:36:31 +0000
+++ lib/lp/bugs/browser/tests/test_bugnomination.py	2011-10-03 15:28:45 +0000
@@ -14,6 +14,7 @@
     ILaunchBag,
     )
 from canonical.launchpad.webapp.publisher import canonical_url
+from lp.registry.interfaces.series import SeriesStatus
 from lp.testing import (
     login_person,
     person_logged_in,
@@ -43,6 +44,14 @@
         launchbag.add(self.distribution)
         launchbag.add(self.bug_task)
 
+    def _makeBugSupervisorTeam(self, person, owner, target):
+        """Create a bug supervisor team which includes the person argument."""
+        members = [self.factory.makePerson() for i in range(2)]
+        members.append(person)
+        bug_supervisor = self.factory.makeTeam(members=members, owner=owner)
+        with person_logged_in(owner):
+            target.setBugSupervisor(bug_supervisor, owner)
+
     def test_submit_action_bug_supervisor(self):
         # A bug supervisor sees the Nominate action label.
         login_person(self.bug_worker)
@@ -72,6 +81,69 @@
             "You do not have permission to nominate this bug.",
             notifications[0].message)
 
+    def test_bug_supervisor_nominate_distribution_does_not_error(self):
+        # A bug supervisor should not receive error notifications
+        # from the BugNominationView for a distro series.
+        person = self.factory.makePerson(
+            name='main-person-test', password='test')
+        distro = self.factory.makeDistribution()
+        owner = distro.owner
+        self._makeBugSupervisorTeam(person, owner, distro)
+        current_series = self.factory.makeDistroSeries(
+            distribution=distro, status=SeriesStatus.CURRENT)
+        # Ensure we have some older series so test data better reflects
+        # actual usage.
+        for index in range(3):
+            self.factory.makeDistroSeries(distribution=distro)
+        bug = self.factory.makeBug(distribution=distro, series=current_series)
+        series_bugtask = bug.bugtasks[1]
+        login_person(person)
+        view = create_initialized_view(series_bugtask, name='+nominate')
+        self.assertEqual(0, len(view.request.notifications))
+
+    def test_bug_supervisor_nominate_source_package_does_not_error(self):
+        # A bug supervisor should not receive error notifications
+        # from the BugNominationView for a source package distro series.
+        person = self.factory.makePerson(
+            name='main-person-test', password='test')
+        distro = self.factory.makeDistribution()
+        owner = distro.owner
+        self._makeBugSupervisorTeam(person, owner, distro)
+        current_series = self.factory.makeDistroSeries(
+            distribution=distro, status=SeriesStatus.CURRENT)
+        # Ensure we have some older series so test data better reflects
+        # actual usage.
+        for index in range(3):
+            self.factory.makeDistroSeries(distribution=distro)
+        package = self.factory.makeDistributionSourcePackage(
+            distribution=distro)
+        bug = self.factory.makeBug(
+            distribution=distro, series=current_series,
+            sourcepackagename=package.name)
+        series_bugtask = bug.bugtasks[1]
+        login_person(person)
+        view = create_initialized_view(series_bugtask, name='+nominate')
+        self.assertEqual(0, len(view.request.notifications))
+
+    def test_bug_supervisor_nominate_product_does_not_error(self):
+        # A bug supervisor should not receive error notifications
+        # from the BugNominationView for a product series.
+        person = self.factory.makePerson(
+            name='main-person-test-product', password='test')
+        product = self.factory.makeProduct()
+        owner = product.owner
+        self._makeBugSupervisorTeam(person, owner, product)
+        current_series = self.factory.makeProductSeries(product=product)
+        # Ensure we have some older series so test data better reflects
+        # actual usage.
+        for index in range(3):
+            self.factory.makeProductSeries(product=product)
+        bug = self.factory.makeBug(product=product, series=current_series)
+        series_bugtask = bug.bugtasks[1]
+        login_person(person)
+        view = create_initialized_view(series_bugtask, name='+nominate')
+        self.assertEqual(0, len(view.request.notifications))
+
 
 class TestBugNominationEditView(TestCaseWithFactory):
     """Tests for BugNominationEditView."""