launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-449561 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #449561 in Launchpad itself: "Bug task assignee picker shown to unauthenticated users"
  https://bugs.launchpad.net/launchpad/+bug/449561
  Bug #781460 in Launchpad itself: "bugttask_index.js hide_assignee_team_selection handling broken"
  https://bugs.launchpad.net/launchpad/+bug/781460

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-449561/+merge/60729

This branch fixes two small bugs in the AJAX bugtask assignee picker.

Firstly, it was being loaded even when the user was unauthenticated, which is pointless and just causes the JS to choke when it can't find me_link. I've fixed that by exposing a user_can_edit_assignee config item along with the rest, and only enabling the picker if that's set.

Secondly, the handling of hide_assignee_team_selection was broken. It looked for .yui-picker-search-box, but that changed to .yui3-picker-search-box some time ago. Single character fixes FTW.

Tests are fairly simple Windmill abominations containing nauseating XPath. I need to check test_no_picker_for_anonymous_users with someone JavaScripty to see if there's a better way to ensure the load has finished without sleeping for a predetermined interval.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-449561/+merge/60729
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-449561 into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-05-10 16:16:21 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-05-12 06:35:54 +0000
@@ -3561,6 +3561,14 @@
         return self.context.userCanEditImportance(self.user)
 
     @property
+    def user_can_edit_assignee(self):
+        """Can the user edit the Milestone field?
+
+        If yes, return True, otherwise return False.
+        """
+        return self.user is not None
+
+    @property
     def user_can_edit_milestone(self):
         """Can the user edit the Milestone field?
 
@@ -3611,6 +3619,7 @@
                                     request=IWebServiceClientRequest(
                                         self.request)) or
                                 None),
+            'user_can_edit_assignee': self.user_can_edit_assignee,
             'user_can_edit_milestone': self.user_can_edit_milestone,
             'user_can_edit_status': not self.context.bugwatch,
             'user_can_edit_importance': (

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-04-20 13:15:38 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-05-12 06:35:54 +0000
@@ -833,7 +833,7 @@
             milestone_choice_edit);
         milestone_choice_edit.render();
     }
-    if (Y.Lang.isValue(assignee_content)) {
+    if (Y.Lang.isValue(assignee_content) && conf.user_can_edit_assignee) {
         // A validation callback called by the picker when the user selects
         // an assignee. We check to see if an assignee is a contributor and if
         // they are not, the user is asked to confirm their selection.
@@ -906,7 +906,7 @@
         // of any team,
         if (conf.hide_assignee_team_selection) {
             content_box = assignee_picker.get('contentBox');
-            search_box = content_box.one('.yui-picker-search-box');
+            search_box = content_box.one('.yui3-picker-search-box');
             search_box.setStyle('display', 'none');
             step_title = content_box.one('.contains-steptitle');
             step_title.setStyle('display', 'none');

=== modified file 'lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py'
--- lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py	2011-04-18 04:12:20 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_inline_assignment.py	2011-05-12 06:35:54 +0000
@@ -1,6 +1,9 @@
 # Copyright 2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import transaction
+
+from canonical.launchpad.webapp import canonical_url
 from lp.bugs.windmill.testing import BugsWindmillLayer
 from lp.testing import WindmillTestCase
 from lp.testing.windmill import lpuser
@@ -8,15 +11,33 @@
     FOR_ELEMENT,
     )
 
+ASSIGN_BUTTON = (u'//*[@id="affected-software"]//tr//td[5]' +
+    '//button[contains(@class, "yui3-activator-act")]')
+VISIBLE_PICKER_OVERLAY = (
+    u'//div[contains(@class, "yui3-picker ") and '
+        'not(contains(@class, "yui3-picker-hidden"))]')
+VISIBLE_PICKER_SEARCH = (
+    u"//input[@class='yui3-picker-search' and "
+    "not(ancestor::*[contains(@style,'display: none')])]")
+
+
+def full_picker_element_xpath(element_path):
+    return VISIBLE_PICKER_OVERLAY + element_path
+
 
 class TestInlineAssignment(WindmillTestCase):
 
     layer = BugsWindmillLayer
 
+    def openAssigneePicker(self, client):
+        client.waits.forElement(xpath=ASSIGN_BUTTON, timeout=FOR_ELEMENT)
+        client.click(xpath=ASSIGN_BUTTON)
+        client.waits.forElement(
+            xpath=VISIBLE_PICKER_OVERLAY, timeout=FOR_ELEMENT)
+
     def test_inline_assignment_non_contributor(self):
         """Test assigning bug to a non contributor displays a confirmation."""
 
-        import transaction
         # Create a person who has not contributed
         self.factory.makePerson(name="fred")
         transaction.commit()
@@ -24,23 +45,11 @@
         client, start_url = self.getClientFor(
             "/firefox/+bug/1", lpuser.SAMPLE_PERSON)
 
-        ASSIGN_BUTTON = (u'//*[@id="affected-software"]//tr//td[5]' +
-            '//button[contains(@class, "yui3-activator-act")]')
-        client.waits.forElement(xpath=ASSIGN_BUTTON, timeout=FOR_ELEMENT)
-        client.click(xpath=ASSIGN_BUTTON)
-
-        VISIBLE_PICKER_OVERLAY = (
-            u'//div[contains(@class, "yui3-picker ") and '
-             'not(contains(@class, "yui3-picker-hidden"))]')
-
-        client.waits.forElement(
-            xpath=VISIBLE_PICKER_OVERLAY, timeout=FOR_ELEMENT)
-
-        def full_picker_element_xpath(element_path):
-            return VISIBLE_PICKER_OVERLAY + element_path
-
-        client.type(xpath=full_picker_element_xpath(
-            "//input[@class='yui3-picker-search']"), text='fred')
+        self.openAssigneePicker(client)
+
+        client.type(
+            xpath=full_picker_element_xpath(VISIBLE_PICKER_SEARCH),
+            text='fred')
         client.click(xpath=full_picker_element_xpath(
             "//div[@class='yui3-picker-search-box']/button"))
 
@@ -55,3 +64,31 @@
         self.client.asserts.assertTextIn(
             xpath=CONFIRMATION,
             validator="Fred did not previously have any assigned bugs")
+
+    def test_no_picker_for_anonymous_users(self):
+        # No assignee picker is shown for an anonymous user.
+
+        client, start_url = self.getClientForAnonymous("/firefox/+bug/1")
+
+        HIDDEN_ASSIGN_BUTTON = (u"//*[@id='affected-software']//tr//td[5]" +
+            "//button[contains(@class,'yui3-activator-act') and "
+            "contains(@class,'yui3-activator-hidden')]")
+        client.waits.sleep(milliseconds=1000)
+        client.asserts.assertNode(xpath=HIDDEN_ASSIGN_BUTTON)
+
+    def test_no_search_widget_for_teamless_users(self):
+        # Teamless unprivileged users can only assign themselves, so no
+        # search widget is shown.
+
+        product = self.factory.makeProduct(
+            bug_supervisor=self.factory.makePerson())
+        bug = self.factory.makeBug(product=product)
+        transaction.commit()
+        client, start_url = self.getClientFor(
+            canonical_url(bug, force_local_path=True), lpuser.NO_PRIV)
+
+        self.openAssigneePicker(client)
+
+        # Ensure that there is no non-hidden picker search widget.
+        client.asserts.assertNotNode(
+            xpath=full_picker_element_xpath(VISIBLE_PICKER_SEARCH))

=== modified file 'lib/lp/bugs/windmill/tests/test_bug_tags_entry.py'
--- lib/lp/bugs/windmill/tests/test_bug_tags_entry.py	2011-03-29 05:59:29 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_tags_entry.py	2011-05-12 06:35:54 +0000
@@ -60,7 +60,7 @@
 
         # Test that anonymous users are prompted to log in.
 
-        client, start_url = self.getClientForAnomymous(bug)
+        client, start_url = self.getClientForAnonymous(bug)
         client.waits.sleep(milliseconds=constants.SLEEP)
         client.click(id=u'edit-tags-trigger')
         client.waits.forPageLoad(timeout=constants.PAGE_LOAD)

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2011-05-09 17:46:54 +0000
+++ lib/lp/testing/__init__.py	2011-05-12 06:35:54 +0000
@@ -840,7 +840,7 @@
             person.displayname, naked_person.preferredemail.email, password)
         return self.getClientFor(url, user=user)
 
-    def getClientForAnomymous(self, obj, view_name=None):
+    def getClientForAnonymous(self, obj, view_name=None):
         """Return a new client, and the url that it has loaded."""
         client = WindmillTestClient(self.suite_name)
         if isinstance(obj, basestring):