Merge lp:~abentley/launchpad/distro-structural-subscription into lp:launchpad

[Aaron Bentley]

No exception when structural subscriptions disabled.

= Summary =
Fix bug #902252: sort buttons that are missing on the distribution dynamic bugs listing

== Proposed fix ==
Change the structural subscription code so that it does not raise an exception if there is no structural subscription link. There are cases, specifically related to distros, where it is reasonable for the link to be missing, even if a user is logged in.

== Pre-implementation notes ==
Discussed with bac

== Implementation details ==
Changed Y.error to Y.log and updated the tests accordingly.

Fixed some lint.

== Tests ==
xvfb-run bin/test -t test_structural_subscription.html

== Demo and Q/A ==
Ensure you are member of custom-buglisting-demo and not a member of charm-contributors on qastaging. Go to https://bugs.qastaging.launchpad.net/charm

The order-by bar should be present, the next link should be green, and there should be no "Subscribe to bug mail" link.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/javascript/tests/test_structural_subscription.js
  lib/lp/registry/javascript/structural-subscription.js

[Deryck Hodge (deryck)]

The code looks great and the change is minimal, so r=me. As we discussed on IRC, we should try to avoid log output for pages when the behavior is normal and not exceptional, but I take the point that this helps people debug what's happening here. I'm fine for it to stand if we log at a lower priority, i.e. to debug. We can turn it on if we need, but it's not noising on normal page requests.

Thanks for being flexible and being willing to fix this up. Approved with that change.

review: Approve

=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js	2011-11-26 04:03:29 +0000
+++ lib/lp/registry/javascript/structural-subscription.js	2011-12-12 16:51:25 +0000
@@ -575,7 +575,9 @@
                 .set('size', '50'))
             .append(Y.Node.create('<a/>')
                 .set('target', 'help')
-                .set('href', '/+help-registry/structural-subscription-tags.html')
+                .set(
+                    'href', '/+help-registry/structural-subscription-tags'
+                    + '.html')
                 .addClass('sprite')
                 .addClass('maybe')
                 .append(Y.Node.create('<span/>')
@@ -779,7 +781,8 @@
         '  <dd>' +
         '    <input type="text" name="name">' +
         '    <a target="help" class="sprite maybe"' +
-        '          href="/+help-registry/structural-subscription-name.html">&nbsp;' +
+        '          href="/+help-registry/structural-subscription-name' +
+        '.html">&nbsp;' +
         '      <span class="invisible-link">Structural subscription' +
         '        description help</span></a> ' +
         '  </dd>' +
@@ -1836,7 +1839,8 @@
     // Modify the menu-link-subscribe-to-bug-mail link to be visible.
     var link = Y.one(link_id);
     if (!Y.Lang.isValue(link)) {
-        Y.error('Link to set as the pop-up link not found.');
+        Y.log('No structural subscription link found.', 'debug');
+        return;
     }
     link.removeClass('invisible-link');
     link.addClass('visible-link');

=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js	2011-07-08 05:12:39 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js	2011-12-12 16:51:25 +0000
@@ -1472,16 +1472,17 @@
             delete this.content_box;
         },
 
-        _should: {
-            error: {
-                test_setup_subscription_link_none: new Error(
-                    'Link to set as the pop-up link not found.')
-            }
-        },
-
         // Setting up a subscription link with no link in the DOM should fail.
         test_setup_subscription_link_none: function() {
+            var logged_missing_link = false;
+            Y.on('yui:log', function(e){
+                if (e.msg === "No structural subscription link found."){
+                    logged_missing_link = true;
+                }
+            });
             module.setup_subscription_link(this.config, "#link");
+            Y.Assert.isTrue(
+                logged_missing_link, "Missing link not reported.");
         },
 
         // Setting up a subscription link should unset the 'invisible-link',
@@ -1849,5 +1850,5 @@
     }));
 
     Y.lp.testing.Runner.run(suite);
-        
+
 });