launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/live-mp-longpoll-filter into lp:launchpad with lp:~rvb/launchpad/live-mp-longpoll as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/launchpad/live-mp-longpoll-filter/+merge/77175

This branches fixes the usage of the LongPoll mechanism on the MP page in two ways:

- The event name is read from LP.cache.merge_proposal_event_key instead of LP.cache.new_mp_diff_event.

- Filter events to only update the diff when the event means that the preview_diff field of the MP has been updated.

= Tests =

lib/lp/code/javascript/tests/test_branchmergeproposal.updater.html
-- 
https://code.launchpad.net/~rvb/launchpad/live-mp-longpoll-filter/+merge/77175
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/live-mp-longpoll-filter into lp:launchpad.

=== modified file 'lib/lp/code/javascript/branchmergeproposal.updater.js'
--- lib/lp/code/javascript/branchmergeproposal.updater.js	2011-09-27 14:49:29 +0000
+++ lib/lp/code/javascript/branchmergeproposal.updater.js	2011-09-27 14:49:30 +0000
@@ -162,10 +162,19 @@
 
 });
 
-
 /*
  * Export UpdaterWidget.
  */
 namespace.UpdaterWidget = UpdaterWidget;
 
+/*
+ * Returns true if the event fired means that the preview_diff field of the
+ * MP has been updated.
+ *
+ */
+namespace.is_mp_diff_updated = function(event_data) {
+    return (event_data.what === "modified" &&
+        event_data.edited_fields.indexOf("preview_diff") >= 0);
+};
+
 }, '0.1', {requires: ['node', 'lp.client', 'lp.anim']});

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.updater.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.updater.js	2011-09-27 14:49:29 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.updater.js	2011-09-27 14:49:30 +0000
@@ -165,6 +165,27 @@
 
 }));
 
+suite.add(new Y.Test.Case({
+
+    name: 'branchmergeproposal-updater-utilities',
+
+    test_is_mp_diff_updated_modified: function() {
+        var data = {what: 'modified', edited_fields: ['preview_diff']};
+        Y.Assert.isTrue(module.is_mp_diff_updated(data));
+    },
+
+    test_is_mp_diff_updater_deleted: function() {
+        var data = {what: 'deleted'};
+        Y.Assert.isFalse(module.is_mp_diff_updated(data));
+    },
+
+    test_is_mp_diff_updated_title_changed: function() {
+        var data = {what: 'modified', edited_fields: ['title']};
+        Y.Assert.isFalse(module.is_mp_diff_updated(data));
+    }
+
+}));
+
 Y.lp.testing.Runner.run(suite);
 
 });

=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt	2011-09-27 14:49:29 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt	2011-09-27 14:49:30 +0000
@@ -225,10 +225,14 @@
         (new Y.lp.code.branchmergeproposal.reviewcomment.NumberToggle()
             ).render();
 
-        if (Y.Lang.isValue(LP.cache.new_mp_diff_event)) {
+        if (Y.Lang.isValue(LP.cache.merge_proposal_event_key)) {
             var updater = new Y.lp.code.branchmergeproposal.updater.UpdaterWidget(
                 {srcNode: Y.one('#diff-area')});
-            Y.on(LP.cache.new_mp_diff_event, updater.update, updater);
+            Y.on(LP.cache.merge_proposal_event_key, function(data) {
+                if (Y.lp.code.branchmergeproposal.updater.is_mp_diff_updated(data)) {
+		    updater.update();
+                }
+            });
             updater.on(updater.NAME + '.updated', function() {
                 (new Y.lp.code.branchmergeproposal.reviewcomment.NumberToggle()
                     ).render();