Merge lp:~deryck/launchpad/lock-fix-released-status-126516 into lp:launchpad

[Deryck Hodge]

Lock FIXRELEASED bugtask status so that only project maintainers and bug supervisors can transition away from that status.

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!

[Jelmer Vernooij (jelmer)]

The line breaks for line 46 and 76 seem unnecessary, please consider removing them.

As pointed out on IRC, the linked bug report requests a slightly different change (transitioning to FIXRELEASED vs transitioning away from it).

review: Approve (code)

[Deryck Hodge (deryck)]

Hi, Jelmer.

I've got a new bug linked now. Thanks for catching that!

Also, I'd like to keep the line breaks. That phrasing is used throughout the tests, and longer status names require the line break. Even though these lines here don't require it, in the context of the entire test, I felt it was more readable to be consistent.

Thanks for the review!

Cheers,
deryck

=== modified file 'lib/lp/bugs/browser/tests/bugs-views.txt'
--- lib/lp/bugs/browser/tests/bugs-views.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/browser/tests/bugs-views.txt	2010-10-21 20:54:52 +0000
@@ -37,7 +37,7 @@
     >>> len(bug_eight.bugtasks)
     1
     >>> bug_eight.bugtasks[0].transitionToStatus(
-    ...     BugTaskStatus.CONFIRMED, getUtility(ILaunchBag).user)
+    ...     BugTaskStatus.CONFIRMED, bug_eight.bugtasks[0].distribution.owner)
     >>> def fix_bug(bug_id, bugtask_index=0):
     ...     bugtask = getUtility(IBugSet).get(bug_id).bugtasks[bugtask_index]
     ...     bugtask.transitionToStatus(

=== modified file 'lib/lp/bugs/doc/bugtask.txt'
--- lib/lp/bugs/doc/bugtask.txt	2010-10-19 18:44:31 +0000
+++ lib/lp/bugs/doc/bugtask.txt	2010-10-21 20:54:52 +0000
@@ -1022,7 +1022,7 @@
 And attribute setting:
 
     >>> bug_upstream_firefox_no_svg_support.transitionToStatus(
-    ...     BugTaskStatus.FIXRELEASED, getUtility(ILaunchBag).user)
+    ...     BugTaskStatus.CONFIRMED, getUtility(ILaunchBag).user)
     >>> bug_upstream_firefox_no_svg_support.transitionToStatus(
     ...     BugTaskStatus.NEW, getUtility(ILaunchBag).user)
 

=== modified file 'lib/lp/bugs/doc/displaying-bugs-and-tasks.txt'
--- lib/lp/bugs/doc/displaying-bugs-and-tasks.txt	2010-10-19 18:44:31 +0000
+++ lib/lp/bugs/doc/displaying-bugs-and-tasks.txt	2010-10-21 20:54:52 +0000
@@ -164,7 +164,7 @@
 Lastly, some cleanup:
 
   >>> test_task.transitionToStatus(
-  ...   ORIGINAL_STATUS, getUtility(ILaunchBag).user)
+  ...   ORIGINAL_STATUS, test_task.distribution.owner)
   >>> test_task.transitionToAssignee(ORIGINAL_ASSIGNEE)
 
 

=== modified file 'lib/lp/bugs/doc/malone-karma.txt'
--- lib/lp/bugs/doc/malone-karma.txt	2010-10-19 18:44:31 +0000
+++ lib/lp/bugs/doc/malone-karma.txt	2010-10-21 20:54:52 +0000
@@ -105,7 +105,7 @@
 
     >>> old_bugtask = Snapshot(bugtask, providing=IDistroBugTask)
     >>> bugtask.transitionToStatus(
-    ...     BugTaskStatus.INVALID, getUtility(ILaunchBag).user)
+    ...     BugTaskStatus.INVALID, bugtask.target.owner)
     >>> notify(ObjectModifiedEvent(bugtask, old_bugtask, ['status']))
     Karma added: action=bugrejected, distribution=debian
 

=== 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-21 20:54:52 +0000
@@ -891,12 +891,13 @@
             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`."""
-        if not new_status:
+        if not new_status or user is None:
             # This is mainly to facilitate tests which, unlike the
             # normal status form, don't always submit a status when
             # testing the edit form.

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt	2010-10-09 16:36:22 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-text-pages.txt	2010-10-21 20:54:52 +0000
@@ -28,7 +28,8 @@
     ...      "bug-patch.diff", is_patch=True, content_type='text/plain',
     ...      description="a patch")
 
-Next, we'll cycle through all statuses so the dates are present:
+Next, we'll cycle through all statuses so the dates are present (to
+toggle away from Fix Released we must be the target owner):
 
     >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
     >>> t0 = bug.bugtasks[0]
@@ -36,7 +37,7 @@
     >>> t0.transitionToStatus(BugTaskStatus.CONFIRMED, mark)
     >>> t0.transitionToStatus(BugTaskStatus.INPROGRESS, mark)
     >>> t0.transitionToStatus(BugTaskStatus.FIXRELEASED, mark)
-    >>> t0.transitionToStatus(BugTaskStatus.NEW, mark)
+    >>> t0.transitionToStatus(BugTaskStatus.NEW, t0.target.owner)
     >>> t0.transitionToStatus(BugTaskStatus.FIXRELEASED, mark)
     >>> logout()
     >>> flush_database_updates()

=== modified file 'lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt'
--- lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/bugs/stories/bugs/xx-incomplete-bugs.txt	2010-10-21 20:54:52 +0000
@@ -234,6 +234,16 @@
 cannot expire. No Privileges Person sets a Debian bug to Incomplete,
 and does not see the expiration notice.
 
+In order for this to work, the bug cannot be FIXRELEASED, which
+it is by default.  So we set the bug back to NEW.
+
+    >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
+    >>> login('[e-mail]')
+    >>> bug_8 = getUtility(IBugSet).get(8)
+    >>> bug_8.bugtasks[0].transitionToStatus(
+    ...     BugTaskStatus.NEW, bug_8.bugtasks[0].distribution.owner)
+    >>> logout()
+
     >>> user_browser.open(
     ...     'http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/8')
     >>> user_browser.getControl('Status').value = ['Incomplete']

=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
--- lib/lp/bugs/tests/bugs-emailinterface.txt	2010-10-21 01:42:14 +0000
+++ lib/lp/bugs/tests/bugs-emailinterface.txt	2010-10-21 20:54:52 +0000
@@ -1620,20 +1620,20 @@
 edited:
 
     >>> login('[e-mail]')
-    >>> bug_eigth = getUtility(IBugSet).get(8)
-    >>> len(bug_eigth.bugtasks)
+    >>> bug_ten = getUtility(IBugSet).get(10)
+    >>> len(bug_ten.bugtasks)
     1
-    >>> submit_commands(bug_eigth, 'status confirmed')
-    >>> mozilla_task = bug_eigth.bugtasks[0]
-    >>> print mozilla_task.status.name
+    >>> submit_commands(bug_ten, 'status confirmed')
+    >>> linux_task = bug_ten.bugtasks[0]
+    >>> print linux_task.status.name
     CONFIRMED
 
     >>> bug_notification = BugNotification.selectFirst(orderBy='-id')
     >>> print bug_notification.bug.id
-    8
+    10
     >>> print bug_notification.message.text_contents
-    ** Changed in: mozilla-firefox (Debian)
-        Status: Fix Released => Confirmed
+    ** Changed in: linux-source-2.6.15 (Ubuntu)
+        Status: New => Confirmed
 
 If the bug has more than one bug task, we try to guess which bug task
 the user wanted to edit. We apply the following heuristics for choosing

=== modified file 'lib/lp/bugs/tests/test_bugtask_1.py'
--- lib/lp/bugs/tests/test_bugtask_1.py	2010-10-04 19:50:45 +0000
+++ lib/lp/bugs/tests/test_bugtask_1.py	2010-10-21 20:54:52 +0000
@@ -116,9 +116,11 @@
     def tearDownBugsElsewhereTests(self):
         """Resets the modified bugtasks to their original statuses."""
         self.firefox_upstream.transitionToStatus(
-            self.old_firefox_status, getUtility(ILaunchBag).user)
+            self.old_firefox_status,
+            self.firefox_upstream.target.bug_supervisor)
         self.thunderbird_upstream.transitionToStatus(
-            self.old_thunderbird_status, getUtility(ILaunchBag).user)
+            self.old_thunderbird_status,
+            self.firefox_upstream.target.bug_supervisor)
         flush_database_updates()
 
     def assertBugTaskIsPendingBugWatchElsewhere(self, bugtask):

=== 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-21 20:54:52 +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):

=== modified file 'lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt'
--- lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt	2010-10-17 15:44:08 +0000
+++ lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt	2010-10-21 20:54:52 +0000
@@ -300,12 +300,12 @@
     >>> pmount_bug = getUtility(IBugSet).get(pmount_bug_id)
     >>> [pmount_task] = pmount_bug.bugtasks
     >>> pmount_task.transitionToStatus(
-    ...     BugTaskStatus.CONFIRMED, getUtility(ILaunchBag).user)
+    ...     BugTaskStatus.CONFIRMED, pmount_task.distribution.owner)
 
     >>> another_pmount_bug = getUtility(IBugSet).get(another_pmount_bug_id)
     >>> [another_pmount_task] = another_pmount_bug.bugtasks
     >>> another_pmount_task.transitionToStatus(
-    ...     BugTaskStatus.CONFIRMED, getUtility(ILaunchBag).user)
+    ...     BugTaskStatus.CONFIRMED, another_pmount_task.distribution.owner)
 
 
     >>> print_single_task_status(pmount_bug_id)