Merge lp:~rvb/launchpad/dds-req-packagediff-fix-flashing into lp:launchpad

[Raphaël Badin]

[r=jcsackett][bug=746277] Fix the retry link to request package diffs on localpackagediffs page.

This branch fixes the retry link to request package diffs so that removing the link removes all the js listeners.

== Summary ==
On the +localpackagediffs page, when a package diff fails, a retry link is displayed. The listener was not properly cleared so it seemed that the clickable zone of this retry link was far too wide. This branch refactors the way the initial link is added so that removing it clears all the js listeners.

== Tests ==
lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html

== QA ==
On a failing package diff request (https://dogfood.launchpad.net/deribuntu/dangerous/+localpackagediffs), make sure than the Retry click listener is limited to the word "Retry".

[j.c.sackett (jcsackett)]

As I said in IRC, it could be worth getting a UI person to validate your decision not to have the red flash animation anymore. I think you're notion of already getting instant feedback is correct--it's your call if you think you should get UI checks on it too.

Otherwise, this looks good. Thanks.

review: Approve

=== modified file 'lib/lp/registry/javascript/distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/distroseriesdifferences_details.js	2011-03-28 16:33:48 +0000
+++ lib/lp/registry/javascript/distroseriesdifferences_details.js	2011-03-31 15:43:56 +0000
@@ -457,7 +457,7 @@
 */
 namespace.setup_request_derived_diff = function(container, dsd_link) {
     // Setup handler for diff requests.
-    container.one('.package-diff-placeholder').on('click', function(e) {
+    container.one('.package-diff-compute-request').on('click', function(e) {
         e.preventDefault();
         compute_package_diff(container, dsd_link);
     });
@@ -511,7 +511,7 @@
     };
     var failure_cb = function(transaction_id, response, args) {
         container.one('p.update-in-progress-message').remove();
-        var retry_handler = function(e) {
+        var recompute = function(e) {
             e.preventDefault();
             compute_package_diff(container, dsd_link);
         };
@@ -519,20 +519,15 @@
         var msg = response.responseText;
 
         // If the error is not of the type raised by an error properly
-        // raisedby python then set a standard error message.
+        // raised by python then set a standard error message.
         if (response.status !== 400) {
             msg = 'Failed to compute package diff.';
         }
         var failure_msg = Y.lp.soyuz.base.makeFailureNode(
-            Y.Escape.html(msg), retry_handler);
-        var placeholder = container.one('.package-diff-placeholder')
+            Y.Escape.html(msg), recompute);
+        var placeholder = container.one('.package-diff-placeholder');
         placeholder.set('innerHTML','');
         placeholder.appendChild(failure_msg);
-
-        var anim = Y.lazr.anim.red_flash({
-            node: placeholder
-        });
-        anim.run();
     };
     var config = {
         on: {

=== modified file 'lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html'
--- lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html	2011-03-28 20:45:09 +0000
+++ lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html	2011-03-31 15:43:56 +0000
@@ -42,9 +42,11 @@
             evolution</a>
           <span class="package-diff-button"></span>
           <dt class="package-diff-placeholder">
-          <a class="js-action sprite add" href="#">
-            Compute differences from last common version:
-          </a></dt>
+          <span class="package-diff-compute-request">
+            <a class="js-action sprite add" href="">
+              Compute differences from last common version:
+            </a>
+          </span></dt>
           <dd>
             <ul class="package-diff-status">
               <li>

=== modified file 'lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js	2011-03-28 21:32:35 +0000
+++ lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js	2011-03-31 15:43:56 +0000
@@ -75,10 +75,37 @@
                 config.on.success(completed_voc);
             }
         };
-
         dsd_details.poll_interval = 2000;
-
-   },
+    },
+
+    test_request_wrong_click: function() {
+        // Click on the placeholder has no effect.
+        // The listeners are on the the link with class
+        // '.package-diff-compute-request'.
+        // bug=746277.
+        var placeholder = Y.one('#placeholder');
+        placeholder.set('innerHTML', placeholder_content);
+        placeholder
+            .one('#derived')
+                .removeClass('PENDING')
+                    .addClass('FAILED');
+        dsd_details.setup_packages_diff_states(
+            placeholder.one('.diff-extra-container'), dsd_uri);
+        var func_req = undefined;
+        var lp_prot = Y.lp.client.Launchpad.prototype;
+        lp_prot.named_post = function(url, func, config) {
+            func_req = func;
+            config.on.success();
+        };
+
+        var wrong_button = placeholder.one('.package-diff-placeholder');
+
+        wrong_button.simulate('click');
+        var package_diff = Y.one('#parent');
+
+        // The request has not been triggered.
+        Assert.isTrue(package_diff.hasClass('request-derived-diff'));
+    },
 
     test_request_package_diff_computation: function() {
         // A click on the button changes the package diff status and requests
@@ -98,7 +125,7 @@
             config.on.success();
         };
 
-        var button = Y.one('.package-diff-placeholder');
+        var button = placeholder.one('.package-diff-compute-request');
 
         button.simulate('click');
         var package_diff = Y.one('#parent');

=== modified file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt	2011-03-31 05:54:22 +0000
+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt	2011-03-31 15:43:56 +0000
@@ -42,7 +42,10 @@
     <dt
       tal:condition="python: not(context.package_diff) or not(context.parent_package_diff)"
       class="package-diff-placeholder">
-        <a class="js-action sprite add" href="#">Compute differences from last common version</a>:
+        <span class="package-diff-compute-request">
+          <a class="js-action sprite add" href="">
+            Compute differences from last common version</a>:
+        </span>
     </dt>
     <dt
       tal:condition="python: context.package_diff and context.parent_package_diff">