launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/js-error-empty-bug-search-950418 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #950418 in Launchpad itself: "Javascript error for empty bug search"
  https://bugs.launchpad.net/launchpad/+bug/950418

For more details, see:
https://code.launchpad.net/~deryck/launchpad/js-error-empty-bug-search-950418/+merge/98479

This fixes a js console error. It's a simple fix to get rid of some console noise. I'm self reviewing to get this going on to landing, especially since it's such a safe change.

The one negative of this branch is that the change is not tested.  However, the object TableView was added and was not tested when added.  So it needs a bit of work to be testable and tested.  I filed bug 960476 about this, and added an XXX in the code.
-- 
https://code.launchpad.net/~deryck/launchpad/js-error-empty-bug-search-950418/+merge/98479
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/js-error-empty-bug-search-950418 into lp:launchpad.

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2012-03-01 19:29:01 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2012-03-20 18:29:22 +0000
@@ -352,6 +352,17 @@
          */
         render: function () {
             var that = this;
+
+            // Exit from render if we do not have a navigator.
+            // XXX: deryck 2012-03-20 Bug #960476
+            // This module is not tested, nor is it easily testable,
+            // so tests for this return were not added.  This code
+            // needs refactoring to be able to test this kind of stuff
+            // more easily.
+            if (!Y.Lang.isValue(this.navigator)) {
+                return;
+            }
+
             var field_visibility =
                 that.navigator.get('model').get_field_visibility();