launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/daily-build-form into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #999842 in Launchpad itself: "TypeError +recipe Y.one('#request-daily-build-form') is null"
  https://bugs.launchpad.net/launchpad/+bug/999842

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/daily-build-form/+merge/105884

Pre-implementation: no one

When visiting a recipe page such as
https://code.qastaging.launchpad.net/~sinzui/+recipe/pocket-lint-daily I
see an error:
    Uncaught TypeError: Cannot call method 'addClass' of null
The browser just executed.
    Y.one('#request-daily-build-form').addClass('unseen')

The request-daily-build-form form is only rendered when the link is
available. This regression was introduced when I replaced the noscript
with a call to hide the form.

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

RULES

    * Do not call addClass() if the form was not rendered.


QA

    * Visit https://code.qastaging.launchpad.net/~sinzui/+recipe/pocket-lint-daily
    * Verify there is not a TypeError when the page loads


LINT

    lib/lp/code/javascript/requestbuild_overlay.js
    lib/lp/code/javascript/tests/test_requestbuild_overlay.js


TEST

    ./bin/test -vvc --layer=YUITest lp.code


IMPLEMENTATION

Check that there is a node befor adding a class.
    lib/lp/code/javascript/requestbuild_overlay.js
    lib/lp/code/javascript/tests/test_requestbuild_overlay.js
-- 
https://code.launchpad.net/~sinzui/launchpad/daily-build-form/+merge/105884
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/daily-build-form into lp:launchpad.

=== modified file 'lib/lp/code/javascript/requestbuild_overlay.js'
--- lib/lp/code/javascript/requestbuild_overlay.js	2012-04-27 23:34:27 +0000
+++ lib/lp/code/javascript/requestbuild_overlay.js	2012-05-15 19:38:19 +0000
@@ -59,7 +59,10 @@
         }
         Y.lp.code.requestbuild_overlay.connect_requestbuilds();
     }
-    Y.one('#request-daily-build-form').addClass('unseen');
+    var form = Y.one('#request-daily-build-form');
+    if (form) {
+        form.addClass('unseen');
+    }
 };
 
 

=== modified file 'lib/lp/code/javascript/tests/test_requestbuild_overlay.js'
--- lib/lp/code/javascript/tests/test_requestbuild_overlay.js	2012-04-27 23:39:20 +0000
+++ lib/lp/code/javascript/tests/test_requestbuild_overlay.js	2012-05-15 19:38:19 +0000
@@ -264,6 +264,14 @@
             Y.one('#request-daily-build-form').hasClass('unseen'));
         Y.Assert.isFalse(
             Y.one('#request-daily-build').hasClass('unseen'));
+    },
+
+    test_hookUpDailyBuildsSchedule_without_form: function() {
+        var form = Y.one('#request-daily-build-form');
+        form.get('parentNode').removeChild(form);
+        module.hookUpDailyBuildsSchedule();
+        Y.Assert.isFalse(
+            Y.one('#request-daily-build').hasClass('unseen'));
     }
 }));