launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/fix-enable-review-type into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #671665 review type field for merge proposal not always enabled
  https://bugs.launchpad.net/bugs/671665


= Summary =

As per bug 671665, when the popup selection widget is used to choose the reviewer for a mp, the review type field does not become enabled. 

= Implementation =

There are 2 parts to the fix. 
1. Enhance lazr-js so that focus is returned to the input field when the user finishes selecting the value from the popup
2. Change the mechanism to trigger the review_type field enable update from onchange to onblur

The enabled processing will occur when the user tabs out of the reviewer field, and will work in the same way whether the reviewer value is typed or entered using the popup.

This branch can be merged immediately but the issue will only be fully addressed once lazr-js is upgraded. The relvant branch there is lp:wallyworld/lazr-js/popup-selection-focus-fix

= Tests =

The code.windmill.test_branchmergeproposal_review.py test was enhanced to the that the correct behaviour occurred when using the popup selector. 

bin/test -vvt test_branchmergeproposal_review

= Lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/templates/branch-register-merge.pt
  lib/lp/code/windmill/tests/test_branchmergeproposal_review.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/fix-enable-review-type/+merge/40302
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/fix-enable-review-type into lp:launchpad.

=== modified file 'lib/lp/code/templates/branch-register-merge.pt'
--- lib/lp/code/templates/branch-register-merge.pt	2010-10-01 05:34:25 +0000
+++ lib/lp/code/templates/branch-register-merge.pt	2010-11-08 06:04:52 +0000
@@ -69,7 +69,7 @@
                 review_type = document.getElementById('field.review_type');
                 review_type.disabled = (reviewer == '');
               };
-              Y.on('change',
+              Y.on('blur',
                   function(e) {
                     reviewer_changed(e.target.get('value'))
                   },

=== modified file 'lib/lp/code/windmill/tests/test_branchmergeproposal_review.py'
--- lib/lp/code/windmill/tests/test_branchmergeproposal_review.py	2010-11-04 19:59:02 +0000
+++ lib/lp/code/windmill/tests/test_branchmergeproposal_review.py	2010-11-08 06:04:52 +0000
@@ -57,15 +57,24 @@
         # reviewer field is empty works.
         client.asserts.assertProperty(
             id=u"field.review_type", validator='disabled|true')
+        # User types into reviewer field manually.
         client.type(text=u'mark', id=u'field.reviewer')
         client.asserts.assertProperty(
             id=u"field.review_type", validator='disabled|false')
         client.type(text=u'', id=u'field.reviewer')
         client.asserts.assertProperty(
             id=u"field.review_type", validator='disabled|true')
+        # User selects reviewer using popup selector widget.
+        client.click(id=u'show-widget-field-reviewer')
+        search_and_select_picker_widget(client, u'name12', 1)
+        # Tab out of the field.
+        client.keyPress(
+            options='\\9,true,false,false,false,false',
+            id=u'field.reviewer')
+        client.asserts.assertProperty(
+            id=u"field.review_type", validator='disabled|false')
 
         client.click(id=u'field.actions.register')
-
         client.waits.forPageLoad(timeout=u'10000')
         client.click(id=u'request-review')