launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/compute-differences-wiring-bug-795448 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #795448 in Launchpad itself: "The JS event for "compute differences" is not installed"
  https://bugs.launchpad.net/launchpad/+bug/795448

For more details, see:
https://code.launchpad.net/~allenap/launchpad/compute-differences-wiring-bug-795448/+merge/64194

This ensures that the "Compute differences ..." link is connected up with Javascript. I'm not sure - or my brain is too lazy/addled - how to test this in a non-silly way, so I present it as it is.
-- 
https://code.launchpad.net/~allenap/launchpad/compute-differences-wiring-bug-795448/+merge/64194
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/compute-differences-wiring-bug-795448 into lp:launchpad.

=== modified file 'lib/lp/registry/javascript/distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/distroseriesdifferences_details.js	2011-05-12 12:09:17 +0000
+++ lib/lp/registry/javascript/distroseriesdifferences_details.js	2011-06-10 14:47:31 +0000
@@ -233,16 +233,21 @@
                 parent_distro_name,
                 parent_series_name
                 ].join('/');
-            blacklist_slot = args.container.one('div.blacklist-options');
-            // The blacklist slot can be null if the user's not allowed
-            // to edit this difference.
+            // The blacklist slot is only available when the user has
+            // the right to blacklist.
+            var blacklist_slot = args.container.one('div.blacklist-options');
             if (blacklist_slot !== null) {
                 setup_blacklist_options(blacklist_slot, source_name, api_uri);
-                setup_add_comment(
-                    args.container.one('div.add-comment-placeholder'),
-                    api_uri);
-                namespace.setup_packages_diff_states(args.container, api_uri);
-          }
+            }
+            // The add comment slot is only available when the user has
+            // the right to add comments.
+            var add_comment_placeholder =
+                args.container.one('div.add-comment-placeholder');
+            if (add_comment_placeholder !== null) {
+                setup_add_comment(add_comment_placeholder, api_uri);
+            }
+            // Set-up diffs and the means to request them.
+            namespace.setup_packages_diff_states(args.container, api_uri);
         };
 
         var failure_cb = function(transaction_id, response, args) {
@@ -268,7 +273,7 @@
                 'success': success_cb,
                 'failure': failure_cb
             },
-            arguments: {
+            "arguments": {
                 'container': container,
                 'uri': uri,
                 'source_name': source_name
@@ -287,7 +292,7 @@
         // Only insert if there isn't already a container row there.
         var next_row = row.next();
         var details_row;
-        if (next_row == null || !next_row.hasClass('diff-extra')) {
+        if (next_row === null || !next_row.hasClass('diff-extra')) {
             var source_name = row.one('a.toggle-extra').get('text');
             var rev_link = row
                 .one('a.toggle-extra').get('href').split('/').reverse();
@@ -398,7 +403,7 @@
     var parent = container.hasClass('parent');
     var package_diff_uri = [
         dsd_link,
-        parent ? 'parent_package_diff_status' : 'package_diff_status',
+        parent ? 'parent_package_diff_status' : 'package_diff_status'
         ].join('/');
     var build_package_diff_update_config = {
         uri: package_diff_uri,
@@ -415,10 +420,7 @@
             var name = state_and_name.title;
             if (state === 'FAILED') {
                 set_package_diff_status(container, 'FAILED', name);
-                var anim = Y.lazr.anim.red_flash({
-                    node: container
-                });
-                anim.run();
+                Y.lazr.anim.red_flash({node: container}).run();
              }
             else if (state === 'COMPLETED') {
                 set_package_diff_status(container, 'COMPLETED');
@@ -427,10 +429,7 @@
                     parent ? 'parent_package_diff_url' : 'package_diff_url'
                     ].join('/');
                 namespace.add_link_to_package_diff(container, url_uri);
-                var anim = Y.lazr.anim.green_flash({
-                    node: container
-                });
-                anim.run();
+                Y.lazr.anim.green_flash({node: container}).run();
              }
         },
 
@@ -468,8 +467,9 @@
 * @dsd_link {string} The uri for the distroseriesdifference object.
 */
 namespace.setup_request_derived_diff = function(container, dsd_link) {
-    // Setup handler for diff requests.
-    container.one('.package-diff-compute-request').on('click', function(e) {
+    // Setup handler for diff requests. There should either zero or
+    // one clickable node.
+    container.all('.package-diff-compute-request').on('click', function(e) {
         e.preventDefault();
         compute_package_diff(container, dsd_link);
     });
@@ -488,11 +488,8 @@
     container.all('.PENDING').each(function(sub_container){
         namespace.setup_pending_package_diff(sub_container, dsd_link);
     });
-    // Find out if there is one package diff that can be requested.
-    request_node = container.one('span.request-derived-diff');
-    if (request_node !== null) {
-        namespace.setup_request_derived_diff(container, dsd_link);
-    }
+    // Set-up the means to request a diff.
+    namespace.setup_request_derived_diff(container, dsd_link);
 };
 
 /**
@@ -557,7 +554,7 @@
             'success': success_cb,
             'failure': failure_cb
         },
-        arguments: {
+        "arguments": {
             'container': container,
             'dsd_link': dsd_link
         }