Merge lp:~sinzui/launchpad/unassign-bugs-0 into lp:launchpad

[Curtis Hovey]

Permit logged in users to unassign bugs.

    Launchpad bug: https://bugs.launchpad.net/bugs/910876
       Also fixes: https://bugs.launchpad.net/bugs/910879
       Also fixes: https://bugs.launchpad.net/bugs/783271
    Pre-implementation: flacoste

Summary of bug 910876:
Lp has some complex logic to determine when to show the "Remove assignee"
action in the picker. The rules are easily subverted by users by assigning
themselves, then they have permission to remove themselves. The rules
for unassignment mirror the rules for assignment, but there are no cases
where users abused unassignment. Lp forces contributors to take awkward
steps in the UI or they use API do complete their task.

--------------------------------------------------------------------

RULES

    * Revert userCanUnassign to the 2009 rules, any logged in user
      can unassign form a bug.
      * This fix closes one or more other bugs

QA

    * As a user not in a project role, eg
      Visit https://bugs.qastaging.launchpad.net/pyflakes/+bug/848467
    * verify the picker shows the "Remove assignee" action.

LINT

    lib/lp/bugs/interfaces/bugtask.py
    lib/lp/bugs/model/bugtask.py
    lib/lp/bugs/model/tests/test_bugtask.py
    lib/lp/bugs/stories/bugtask-management/xx-change-assignee.txt

TEST

    ./bin/test -vvc -t test_userCanUnassign lp.bugs.model.tests.test_bugtask
    ./bin/test -vvc -t xx-change-assignee lp.bugs.tests.test_doc

IMPLEMENTATION

Updated userCanUnassign to only check for a logged in user.
    lib/lp/bugs/interfaces/bugtask.py
    lib/lp/bugs/model/bugtask.py

Removed redundant tests...the tests only need to check an anonymous
case and a logged in user case. Project roles are unimportant.
    lib/lp/bugs/model/tests/test_bugtask.py
    lib/lp/bugs/stories/bugtask-management/xx-change-assignee.txt

[Brad Crittenden (bac)]

Branch looks good Curtis. Lots of nice red.

review: Approve (code)

=== modified file 'lib/lp/bugs/interfaces/bugtask.py'
--- lib/lp/bugs/interfaces/bugtask.py	2012-01-03 15:56:55 +0000
+++ lib/lp/bugs/interfaces/bugtask.py	2012-01-06 17:27:12 +0000
@@ -841,12 +841,7 @@
         """
 
     def userCanUnassign(user):
-        """Check if the current user can set assignee to None.
-
-        Project owner, project drivers, series drivers, bug supervisors
-        and Launchpad admins can do this always; other users can do this
-        only if they or their reams are the assignee.
-        """
+        """Check if the current user can set assignee to None."""
 
     @mutator_for(assignee)
     @operation_parameters(

=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py	2012-01-02 17:10:14 +0000
+++ lib/lp/bugs/model/bugtask.py	2012-01-06 17:27:12 +0000
@@ -1093,15 +1093,8 @@
             return self.userHasBugSupervisorPrivileges(user)
 
     def userCanUnassign(self, user):
-        """True if user can set the assignee to None.
-
-        This option not shown for regular users unless they or their teams
-        are the assignees. Project owners, drivers, bug supervisors and
-        Launchpad admins can always unassign.
-        """
-        return user is not None and (
-            user.inTeam(self.assignee) or
-            self.userHasBugSupervisorPrivileges(user))
+        """See `IBugTask`."""
+        return user is not None
 
     def canTransitionToAssignee(self, assignee):
         """See `IBugTask`."""

=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py	2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py	2012-01-06 17:27:12 +0000
@@ -769,24 +769,16 @@
             self.assertTrue(
                 self.series_bugtask.userCanSetAnyAssignee(self.regular_user))
 
-    def test_userCanUnassign_regular_user(self):
-        # Ordinary users can unassign themselves...
-        login_person(self.regular_user)
-        self.assertEqual(self.target_bugtask.assignee, self.regular_user)
-        self.assertEqual(self.series_bugtask.assignee, self.regular_user)
-        self.assertTrue(
-            self.target_bugtask.userCanUnassign(self.regular_user))
-        self.assertTrue(
-            self.series_bugtask.userCanUnassign(self.regular_user))
-        # ...but not other assignees.
+    def test_userCanUnassign_logged_in_user(self):
+        # Ordinary users can unassign any user or team.
         login_person(self.target_owner_member)
         other_user = self.factory.makePerson()
         self.series_bugtask.transitionToAssignee(other_user)
         self.target_bugtask.transitionToAssignee(other_user)
         login_person(self.regular_user)
-        self.assertFalse(
+        self.assertTrue(
             self.target_bugtask.userCanUnassign(self.regular_user))
-        self.assertFalse(
+        self.assertTrue(
             self.series_bugtask.userCanUnassign(self.regular_user))
 
     def test_userCanSetAnyAssignee_target_owner(self):
@@ -797,14 +789,6 @@
         self.assertTrue(
             self.series_bugtask.userCanSetAnyAssignee(self.target.owner))
 
-    def test_userCanUnassign_target_owner(self):
-        # The target owner can unassign anybody.
-        login_person(self.target_owner_member)
-        self.assertTrue(
-            self.target_bugtask.userCanUnassign(self.target_owner_member))
-        self.assertTrue(
-            self.series_bugtask.userCanUnassign(self.target_owner_member))
-
     def test_userCanSetAnyAssignee_bug_supervisor(self):
         # A bug supervisor can assign anybody.
         if self.supervisor_member is not None:
@@ -816,15 +800,6 @@
                 self.series_bugtask.userCanSetAnyAssignee(
                     self.supervisor_member))
 
-    def test_userCanUnassign_bug_supervisor(self):
-        # A bug supervisor can unassign anybody.
-        if self.supervisor_member is not None:
-            login_person(self.supervisor_member)
-            self.assertTrue(
-                self.target_bugtask.userCanUnassign(self.supervisor_member))
-            self.assertTrue(
-                self.series_bugtask.userCanUnassign(self.supervisor_member))
-
     def test_userCanSetAnyAssignee_driver(self):
         # A project driver can assign anybody.
         login_person(self.driver_member)
@@ -833,14 +808,6 @@
         self.assertTrue(
             self.series_bugtask.userCanSetAnyAssignee(self.driver_member))
 
-    def test_userCanUnassign_driver(self):
-        # A project driver can unassign anybody.
-        login_person(self.driver_member)
-        self.assertTrue(
-            self.target_bugtask.userCanUnassign(self.driver_member))
-        self.assertTrue(
-            self.series_bugtask.userCanUnassign(self.driver_member))
-
     def test_userCanSetAnyAssignee_series_driver(self):
         # A series driver can assign anybody to series bug tasks.
         login_person(self.driver_member)
@@ -858,15 +825,6 @@
                 self.target_bugtask.userCanSetAnyAssignee(
                     self.series_driver_member))
 
-    def test_userCanUnassign_series_driver(self):
-        # The target owner can unassign anybody from series bug tasks...
-        login_person(self.series_driver_member)
-        self.assertTrue(
-            self.series_bugtask.userCanUnassign(self.series_driver_member))
-        # ...but not from tasks of the main product/distribution.
-        self.assertFalse(
-            self.target_bugtask.userCanUnassign(self.series_driver_member))
-
     def test_userCanSetAnyAssignee_launchpad_admins(self):
         # Launchpad admins can assign anybody.
         login_person(self.target_owner_member)
@@ -875,14 +833,6 @@
         self.assertTrue(self.target_bugtask.userCanSetAnyAssignee(foo_bar))
         self.assertTrue(self.series_bugtask.userCanSetAnyAssignee(foo_bar))
 
-    def test_userCanUnassign_launchpad_admins(self):
-        # Launchpad admins can unassign anybody.
-        login_person(self.target_owner_member)
-        foo_bar = getUtility(IPersonSet).getByEmail('[e-mail]')
-        login_person(foo_bar)
-        self.assertTrue(self.target_bugtask.userCanUnassign(foo_bar))
-        self.assertTrue(self.series_bugtask.userCanUnassign(foo_bar))
-
     def test_userCanSetAnyAssignee_bug_importer(self):
         # The bug importer celebrity can assign anybody.
         login_person(self.target_owner_member)
@@ -893,14 +843,6 @@
         self.assertTrue(
             self.series_bugtask.userCanSetAnyAssignee(bug_importer))
 
-    def test_userCanUnassign_launchpad_bug_importer(self):
-        # The bug importer celebrity can unassign anybody.
-        login_person(self.target_owner_member)
-        bug_importer = getUtility(ILaunchpadCelebrities).bug_importer
-        login_person(bug_importer)
-        self.assertTrue(self.target_bugtask.userCanUnassign(bug_importer))
-        self.assertTrue(self.series_bugtask.userCanUnassign(bug_importer))
-
 
 class TestProductBugTaskPermissionsToSetAssignee(
     TestBugTaskPermissionsToSetAssigneeMixin, TestCaseWithFactory):

=== modified file 'lib/lp/bugs/stories/bugtask-management/xx-change-assignee.txt'
--- lib/lp/bugs/stories/bugtask-management/xx-change-assignee.txt	2011-12-24 15:18:32 +0000
+++ lib/lp/bugs/stories/bugtask-management/xx-change-assignee.txt	2012-01-06 17:27:12 +0000
@@ -157,35 +157,3 @@
     There is 1 error in the data you entered. Please fix it and try again.
     (Find…)
     Constraint not satisfied
-
-The unassign option is only shown if the current assignee is the user
-or one of his teams...
-
-    >>> user_browser.open("http://bugs.launchpad.dev/jokosher/+bug/11")
-    >>> assignee_control = user_browser.getControl(
-    ...     name="jokosher.assignee.option", index=0)
-    >>> assignee_control.value = ["jokosher.assignee.assign_to_nobody"]
-    >>> user_browser.getControl("Save Changes", index=0).click()
-
-...or if there is no assignee.
-
-    >>> assignee_control = user_browser.getControl(
-    ...     name="jokosher.assignee.option", index=0)
-    >>> assignee_control.value = ["jokosher.assignee.assign_to_nobody"]
-
-If the bugtask is assigned to anybody else, the unassign option is not
-shown.
-
-    >>> admin_browser.open("http://bugs.launchpad.dev/jokosher/+bug/11")
-    >>> assignee_control = admin_browser.getControl(
-    ...     name="jokosher.assignee.option", index=0)
-    >>> assignee_control.value = ["jokosher.assignee.assign_to_me"]
-    >>> admin_browser.getControl("Save Changes", index=0).click()
-    >>> user_browser.open("http://bugs.launchpad.dev/jokosher/+bug/11")
-    >>> assignee_control = user_browser.getControl(
-    ...     name="jokosher.assignee.option", index=0)
-    >>> assignee_control.value = ["jokosher.assignee.assign_to_nobody"]
-    Traceback (most recent call last):
-    ...
-    ItemNotFoundError: insufficient items with name
-    'jokosher.assignee.assign_to_nobody'