launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/nomination-page-oops into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #671002 in Launchpad itself: "+nomination page triggers NominationError OOPS"
  https://bugs.launchpad.net/launchpad/+bug/671002

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/nomination-page-oops/+merge/61208

= Implementation =

In the view initialize(), redirect back to the bug target page with an error message if the user does not have permission to nominate/target the bug task. I did this instead of raising a 403 because it's more user friendly.

= Tests =

Add a new test to TestBugNominationView: test_submit_action_unauthorised()

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/bugnomination.py
  lib/lp/bugs/browser/tests/test_bugnomination.py

-- 
https://code.launchpad.net/~wallyworld/launchpad/nomination-page-oops/+merge/61208
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/nomination-page-oops into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py	2011-05-12 04:02:15 +0000
+++ lib/lp/bugs/browser/bugnomination.py	2011-05-17 06:42:32 +0000
@@ -67,8 +67,13 @@
         submit_action = self.__class__.actions.byname['actions.submit']
         if self.userIsReleaseManager():
             submit_action.label = _("Target")
-        else:
+        elif self.userIsBugSupervisor():
             submit_action.label = _("Nominate")
+        else:
+            self.request.response.addErrorNotification(
+                "You do not have permission to nominate this bug.")
+            self.request.response.redirect(
+                canonical_url(self.current_bugtask))
 
     @property
     def label(self):
@@ -89,6 +94,12 @@
         return check_permission(
             "launchpad.Driver", 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)
+
     def userCanChangeDriver(self):
         """Can the current user set the release management team?"""
         return check_permission(

=== modified file 'lib/lp/bugs/browser/tests/test_bugnomination.py'
--- lib/lp/bugs/browser/tests/test_bugnomination.py	2011-05-12 05:05:07 +0000
+++ lib/lp/bugs/browser/tests/test_bugnomination.py	2011-05-17 06:42:32 +0000
@@ -9,7 +9,11 @@
 
 from canonical.testing.layers import DatabaseFunctionalLayer
 from canonical.launchpad.webapp.interaction import get_current_principal
-from canonical.launchpad.webapp.interfaces import ILaunchBag
+from canonical.launchpad.webapp.interfaces import (
+    BrowserNotificationLevel,
+    ILaunchBag,
+    )
+from canonical.launchpad.webapp.publisher import canonical_url
 from lp.testing import (
     login_person,
     person_logged_in,
@@ -53,6 +57,21 @@
         action = view.__class__.actions.byname['actions.submit']
         self.assertEqual('Target', action.label)
 
+    def test_submit_action_unauthorised(self):
+        # An unauthorised user sees an error on the bug target page.
+        login_person(None)
+        view = create_initialized_view(self.bug_task, name='+nominate')
+        self.assertEqual(
+            canonical_url(self.bug_task),
+            view.request.response.getHeader('Location'))
+        notifications = view.request.notifications
+        self.assertEqual(1, len(notifications))
+        self.assertEqual(
+            BrowserNotificationLevel.ERROR, notifications[0].level)
+        self.assertEqual(
+            "You do not have permission to nominate this bug.",
+            notifications[0].message)
+
 
 class TestBugNominationEditView(TestCaseWithFactory):
     """Tests for BugNominationEditView."""