launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/filebug-unpriviliged-1012912 into lp:launchpad.

Requested reviews:
  Ian Booth (wallyworld)
Related bugs:
  Bug #1012912 in Launchpad itself: "+filebug choice popups break when the user is unprivileged"
  https://bugs.launchpad.net/launchpad/+bug/1012912

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/filebug-unpriviliged-1012912/+merge/110442

== Implementation ==

The filebug page safely ignores missing fields like status and importance if they are not rendered. Previously the javascript crashed because it assumed the fields were there.

== Tests ==

Add a new yui test to test_filebug

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/choice.js
  lib/lp/bugs/javascript/tests/test_filebug.html
  lib/lp/bugs/javascript/tests/test_filebug.js

-- 
https://code.launchpad.net/~wallyworld/launchpad/filebug-unpriviliged-1012912/+merge/110442
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js	2012-06-13 01:16:09 +0000
+++ lib/lp/app/javascript/choice.js	2012-06-15 01:21:21 +0000
@@ -126,6 +126,9 @@
  */
 namespace.addPopupChoice = function(field_name, choices, show_description) {
     var legacy_node = Y.one('[id=field.' + field_name + ']');
+    if (!Y.Lang.isValue(legacy_node)) {
+        return;
+    }
     var get_fn = function(node) {
         return node.get('value');
     };
@@ -147,6 +150,9 @@
 
     var legacy_node = Y.one('[name=field.' + field_name + ']')
         .ancestor('table.radio-button-widget');
+    if (!Y.Lang.isValue(legacy_node)) {
+        return;
+    }
     var get_fn = function(node) {
         var field_value = choices[0].value;
         node.all('input[name=field.' + field_name + ']').each(function(node) {

=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.html'
--- lib/lp/bugs/javascript/tests/test_filebug.html	2012-06-06 04:32:05 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.html	2012-06-15 01:21:21 +0000
@@ -110,7 +110,7 @@
             </select>
         </div>
         <div class="value">
-            <select size="1" name="field.status" id="field.importance">
+            <select size="1" name="field.importance" id="field.importance">
             <option value="Undecided" selected="selected">Undecided</option>
             <option value="High">High</option>
             </select>

=== modified file 'lib/lp/bugs/javascript/tests/test_filebug.js'
--- lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-13 15:52:31 +0000
+++ lib/lp/bugs/javascript/tests/test_filebug.js	2012-06-15 01:21:21 +0000
@@ -149,6 +149,14 @@
             Y.Assert.isTrue(legacy_dropdown.hasClass('unseen'));
         },
 
+        // The choice popup wiring works even if the field is missing.
+        // This is so fields which the user does not have permission to see
+        // can be missing and everything still works as expected.
+        test_missing_fields: function () {
+            Y.one('[id=field.status]').remove(true);
+            this.test_importance_setup();
+        },
+
         // The bugtask status choice popup updates the form.
         test_status_selection: function() {
             Y.lp.bugs.filebug.setup_filebug(true);