launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/lock-fix-released-status-126516 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #126516 would like limited access control for status field changes
  https://bugs.launchpad.net/bugs/126516


This branch locks the status FIXRELEASED so that only project
maintainers and bug supervisors can transition away from the status.
Status transitions are well tested.

./bin/test -cvvt test_bugtask_status

The widget which allows toggling status on the bug page uses
IBugTask.canTransitionToStatus to determine which statuses are
clickable.  So no changes are needed to the UI.

Thanks for the review!
-- 
https://code.launchpad.net/~deryck/launchpad/lock-fix-released-status-126516/+merge/38931
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/lock-fix-released-status-126516 into lp:launchpad/devel.

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2010-09-23 10:27:11 +0000
+++ lib/lp/bugs/model/bugtask.py	2010-10-20 13:05:36 +0000
@@ -891,8 +891,9 @@
             user.id == celebrities.janitor.id):
             return True
         else:
-            return (self.status is not BugTaskStatus.WONTFIX and
-                    new_status not in BUG_SUPERVISOR_BUGTASK_STATUSES)
+            return (self.status not in (
+                        BugTaskStatus.WONTFIX, BugTaskStatus.FIXRELEASED)
+                    and new_status not in BUG_SUPERVISOR_BUGTASK_STATUSES)
 
     def transitionToStatus(self, new_status, user, when=None):
         """See `IBugTask`."""

=== modified file 'lib/lp/bugs/tests/test_bugtask_status.py'
--- lib/lp/bugs/tests/test_bugtask_status.py	2010-10-18 17:29:49 +0000
+++ lib/lp/bugs/tests/test_bugtask_status.py	2010-10-20 13:05:36 +0000
@@ -73,6 +73,15 @@
                 UserCannotEditBugTaskStatus, self.task.transitionToStatus,
                 BugTaskStatus.CONFIRMED, self.user)
 
+    def test_user_cannot_unset_fix_released_status(self):
+        # A regular user should not be able to transition a bug away
+        # from Fix Released.
+        removeSecurityProxy(self.task).status = BugTaskStatus.FIXRELEASED
+        with person_logged_in(self.user):
+            self.assertRaises(
+                UserCannotEditBugTaskStatus, self.task.transitionToStatus,
+                BugTaskStatus.FIXRELEASED, self.user)
+
     def test_user_canTransitionToStatus(self):
         # Regular user cannot transition to BUG_SUPERVISOR_BUGTASK_STATUSES,
         # but can transition to any other status.
@@ -129,6 +138,15 @@
                 BugTaskStatus.NEW, self.user),
             False)
 
+    def test_user_canTransitionToStatus_from_fixreleased(self):
+        # A regular user cannot transition away from Fix Released,
+        # so canTransitionToStatus should return False.
+        removeSecurityProxy(self.task).status = BugTaskStatus.FIXRELEASED
+        self.assertEqual(
+            self.task.canTransitionToStatus(
+                BugTaskStatus.NEW, self.user),
+            False)
+
 
 class TestBugTaskStatusTransitionForPrivilegedUserBase:
     """Base class used to test privileged users and status transitions."""
@@ -188,6 +206,13 @@
             self.task.transitionToStatus(BugTaskStatus.CONFIRMED, self.person)
             self.assertEqual(self.task.status, BugTaskStatus.CONFIRMED)
 
+    def test_privileged_user_can_unset_wont_fix_released(self):
+        # Privileged users can transition away from Fix Released.
+        removeSecurityProxy(self.task).status = BugTaskStatus.FIXRELEASED
+        with person_logged_in(self.person):
+            self.task.transitionToStatus(BugTaskStatus.CONFIRMED, self.person)
+            self.assertEqual(self.task.status, BugTaskStatus.CONFIRMED)
+
     def test_privileged_user_canTransitionToStatus(self):
         # Privileged users (like owner or bug supervisor) should
         # be able to set any status, so canTransitionToStatus should
@@ -246,6 +271,15 @@
                 BugTaskStatus.NEW, self.person),
             True)
 
+    def test_privileged_user_canTransitionToStatus_from_fixreleased(self):
+        # A privileged user can transition away from Fix Released, so
+        # canTransitionToStatus should return True.
+        removeSecurityProxy(self.task).status = BugTaskStatus.FIXRELEASED
+        self.assertEqual(
+            self.task.canTransitionToStatus(
+                BugTaskStatus.NEW, self.person),
+            True)
+
 
 class TestBugTaskStatusTransitionOwnerPerson(
     TestBugTaskStatusTransitionForPrivilegedUserBase, TestCaseWithFactory):