launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06053
[Merge] lp:~sinzui/launchpad/unassign-bugs-0 into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/unassign-bugs-0 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #783271 in Launchpad itself: "Can't unassign newly self-assigned bug without refreshing"
https://bugs.launchpad.net/launchpad/+bug/783271
Bug #910876 in Launchpad itself: ""Remove assignee" no longer shown in bugtask assignee AJAX popup unless logged in user is the assignee"
https://bugs.launchpad.net/launchpad/+bug/910876
Bug #910879 in Launchpad itself: "Actions available in bug assignee AJAX popup do not refresh appropriately without reloading whole page "
https://bugs.launchpad.net/launchpad/+bug/910879
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/unassign-bugs-0/+merge/87785
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
--
https://code.launchpad.net/~sinzui/launchpad/unassign-bugs-0/+merge/87785
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/unassign-bugs-0 into lp:launchpad.
=== 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('foo.bar@xxxxxxxxxxxxx')
- 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'
