launchpad-reviewers team mailing list archive

[Richard Harding <rick.harding@xxxxxxxxxxxxx>]

Richard Harding has proposed merging lp:~rharding/launchpad/1024866_buglink into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1024866 in Launchpad itself: ""report a bug" link in the involvement portlet on bug listings is broken"
  https://bugs.launchpad.net/launchpad/+bug/1024866

For more details, see:
https://code.launchpad.net/~rharding/launchpad/1024866_buglink/+merge/115117

= Summary =

There was an update to the portlet html that the bug listing code ended up
grabbing html from the portlet and trying to turn it into nex/prev links. This
corrects that.

== Implementation Notes ==

This adds helpers to the listing navigator to add a parent node to limit work
within replacing usage of Y.one/all with it's own version.

== Tests ==

test_listing_navigator.js


== LoC Qualification ==

I've got a small credit from recent YUI 3.5 work see branch:

https://code.launchpad.net/~rharding/launchpad/soyuz_yui35
-- 
https://code.launchpad.net/~rharding/launchpad/1024866_buglink/+merge/115117
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/1024866_buglink into lp:launchpad.

=== modified file 'lib/lp/app/javascript/listing_navigator.js'
--- lib/lp/app/javascript/listing_navigator.js	2012-06-27 14:09:43 +0000
+++ lib/lp/app/javascript/listing_navigator.js	2012-07-16 12:07:22 +0000
@@ -28,6 +28,22 @@
     return url.substr(0, cut_at - 1);
 }
 
+
+/**
+ * Generate selector helpers so that we only find nodes within our own
+ * buglisting UI.
+ */
+module.one = function (parent, selector) {
+    var updated_selector = parent + " " + selector;
+    return Y.one(updated_selector);
+};
+
+module.all = function (parent, selector) {
+    var updated_selector = parent + " " + selector;
+    return Y.all(updated_selector);
+};
+
+
 /**
  * Constructor.
  *
@@ -520,9 +536,9 @@
  * Rewrite all nodes with navigation classes so that they are hyperlinks.
  * Content is retained.
  */
-module.linkify_navigation = function() {
+module.linkify_navigation = function(parent_selector) {
     Y.each(['previous', 'next', 'first', 'last'], function(class_name) {
-        Y.all('.' + class_name).each(function(node) {
+        module.all(parent_selector, '.' + class_name).each(function(node) {
             new_node = Y.Node.create('<a href="#"></a>');
             new_node.addClass(class_name);
             new_node.setContent(node.getContent());

=== modified file 'lib/lp/app/javascript/tests/test_listing_navigator.js'
--- lib/lp/app/javascript/tests/test_listing_navigator.js	2012-06-27 20:04:57 +0000
+++ lib/lp/app/javascript/tests/test_listing_navigator.js	2012-07-16 12:07:22 +0000
@@ -181,22 +181,36 @@
          */
         test_linkify_navigation: function() {
             Y.one('#fixture').setContent(
+                '<span id="notme" class="first"></span>' +
+                '<div id="bugs-table-listing">' +
                 '<span class="previous">PreVious</span>' +
                 '<span class="next">NeXt</span>' +
                 '<span class="first">FiRST</span>' +
-                '<span class="last">lAst</span>');
-            module.linkify_navigation();
+                '<span class="last">lAst</span>' +
+                '</div>');
+            module.linkify_navigation('#bugs-table-listing');
             function checkNav(selector, content) {
-                var node = Y.one(selector);
-                Y.Assert.areEqual('a', node.get('tagName').toLowerCase());
-                Y.Assert.areEqual(content, node.getContent());
-                Y.Assert.areEqual('#', node.get('href').substr(-1, 1));
+                var nodelist = Y.all(selector);
+                nodelist.each(function (node) {
+                    if (node.get('id') != 'notme') {
+                        Y.Assert.areEqual('a',
+                            node.get('tagName').toLowerCase());
+                        Y.Assert.areEqual(content, node.getContent());
+                        Y.Assert.areEqual('#',
+                            node.get('href').substr(-1, 1));
+                    } else {
+                        Y.Assert.areEqual('span',
+                            node.get('tagName').toLowerCase(),
+                            'Ignore nodes outside of the bug listing table');
+                    }
+                });
             }
             checkNav('.previous', 'PreVious');
             checkNav('.next', 'NeXt');
             checkNav('.first', 'FiRST');
             checkNav('.last', 'lAst');
         },
+
         /**
          * Render should update the navigation_indices with the result info.
          */

=== modified file 'lib/lp/bugs/javascript/buglisting.js'
--- lib/lp/bugs/javascript/buglisting.js	2012-03-22 19:08:39 +0000
+++ lib/lp/bugs/javascript/buglisting.js	2012-07-16 12:07:22 +0000
@@ -176,7 +176,7 @@
         var navigation_indices = Y.all('.batch-navigation-index');
         var pre_fetch = Y.lp.app.listing_navigator.get_feature_flag(
             'bugs.dynamic_bug_listings.pre_fetch');
-        Y.lp.app.listing_navigator.linkify_navigation();
+        Y.lp.app.listing_navigator.linkify_navigation('#bugs-table-listing');
         var navigator = new module.BugListingNavigator({
             current_url: window.location,
             cache: LP.cache,

=== modified file 'lib/lp/registry/javascript/sharing/shareetable.js'
--- lib/lp/registry/javascript/sharing/shareetable.js	2012-07-07 14:00:30 +0000
+++ lib/lp/registry/javascript/sharing/shareetable.js	2012-07-16 12:07:22 +0000
@@ -100,7 +100,7 @@
 
     make_navigator: function() {
         var navigation_indices = Y.all('.batch-navigation-index');
-        Y.lp.app.listing_navigator.linkify_navigation();
+        Y.lp.app.listing_navigator.linkify_navigation('#sharee-table-listing');
         var ns = Y.lp.registry.sharing.shareelisting_navigator;
         var cache = LP.cache;
         cache.total = this.get('sharees').length;