From d233f617dd93908e212d3ce5029c5a9fd351e575 Mon Sep 17 00:00:00 2001 From: Elliott Jin Date: Sat, 11 Feb 2017 00:17:19 -0800 Subject: [PATCH] muting: Refactor to move side effects into UI layer. --- frontend_tests/node_tests/muting.js | 44 +++++------------------------ frontend_tests/node_tests/unread.js | 2 +- static/js/muting.js | 13 ++------- static/js/muting_ui.js | 19 +++++++++++-- static/js/popovers.js | 6 ++-- 5 files changed, 31 insertions(+), 53 deletions(-) diff --git a/frontend_tests/node_tests/muting.js b/frontend_tests/node_tests/muting.js index e27e315140..0613b503e7 100644 --- a/frontend_tests/node_tests/muting.js +++ b/frontend_tests/node_tests/muting.js @@ -17,29 +17,29 @@ var muting = require('js/muting.js'); (function test_basics() { assert(!muting.is_topic_muted('devel', 'java')); - muting.mute_topic('devel', 'java'); + muting.add_muted_topic('devel', 'java'); assert(muting.is_topic_muted('devel', 'java')); // test idempotentcy - muting.mute_topic('devel', 'java'); + muting.add_muted_topic('devel', 'java'); assert(muting.is_topic_muted('devel', 'java')); - muting.unmute_topic('devel', 'java'); + muting.remove_muted_topic('devel', 'java'); assert(!muting.is_topic_muted('devel', 'java')); // test idempotentcy - muting.unmute_topic('devel', 'java'); + muting.remove_muted_topic('devel', 'java'); assert(!muting.is_topic_muted('devel', 'java')); // test unknown stream is harmless too - muting.unmute_topic('unknown', 'java'); + muting.remove_muted_topic('unknown', 'java'); assert(!muting.is_topic_muted('unknown', 'java')); }()); (function test_get_and_set_muted_topics() { assert.deepEqual(muting.get_muted_topics(), []); - muting.mute_topic('office', 'gossip'); - muting.mute_topic('devel', 'java'); + muting.add_muted_topic('office', 'gossip'); + muting.add_muted_topic('devel', 'java'); assert.deepEqual(muting.get_muted_topics().sort(), [ ['devel', 'java'], ['office', 'gossip'], @@ -55,36 +55,6 @@ var muting = require('js/muting.js'); ]); }()); -(function test_muting_performance() { - // This test ensures that each call to mute_topic and set_muted_topics only - // results in one call to unread.update_unread_counts. - - // We replace (for the duration of this test) the real update_unread_counts - // with a test version that just counts the number of calls. - var saved = unread.update_unread_counts; - var num_calls = 0; - unread.update_unread_counts = function () { - num_calls += 1; - }; - - muting.mute_topic('office', 'gossip'); - assert.equal(num_calls, 1); - - muting.set_muted_topics([ - ['social', 'breakfast'], - ]); - assert.equal(num_calls, 2); - - muting.set_muted_topics([ - ['social', 'breakfast'], - ['design', 'typography'], - ['devel', 'java'], - ]); - assert.equal(num_calls, 3); - - unread.update_unread_counts = saved; -}()); - (function test_case_insensitivity() { muting.set_muted_topics([]); assert(!muting.is_topic_muted('SOCial', 'breakfast')); diff --git a/frontend_tests/node_tests/unread.js b/frontend_tests/node_tests/unread.js index a64899096c..84eb0b0bf4 100644 --- a/frontend_tests/node_tests/unread.js +++ b/frontend_tests/node_tests/unread.js @@ -164,7 +164,7 @@ var zero_counts = { assert.equal(counts.home_unread_messages, 1); assert.equal(unread.num_unread_for_stream('social'), 1); - muting.mute_topic('social', 'test_muting'); + muting.add_muted_topic('social', 'test_muting'); counts = unread.get_counts(); assert.equal(counts.stream_count.get('social'), 0); assert.equal(counts.home_unread_messages, 0); diff --git a/static/js/muting.js b/static/js/muting.js index ae708850f7..617f302ab5 100644 --- a/static/js/muting.js +++ b/static/js/muting.js @@ -4,26 +4,20 @@ var exports = {}; var muted_topics = new Dict({fold_case: true}); -function set_muted_topic(stream, topic) { +exports.add_muted_topic = function (stream, topic) { var sub_dict = muted_topics.get(stream); if (!sub_dict) { sub_dict = new Dict({fold_case: true}); muted_topics.set(stream, sub_dict); } sub_dict.set(topic, true); -} - -exports.mute_topic = function (stream, topic) { - set_muted_topic(stream, topic); - unread.update_unread_counts(); }; -exports.unmute_topic = function (stream, topic) { +exports.remove_muted_topic = function (stream, topic) { var sub_dict = muted_topics.get(stream); if (sub_dict) { sub_dict.del(topic); } - unread.update_unread_counts(); }; exports.is_topic_muted = function (stream, topic) { @@ -51,9 +45,8 @@ exports.set_muted_topics = function (tuples) { var stream = tuple[0]; var topic = tuple[1]; - set_muted_topic(stream, topic); + exports.add_muted_topic(stream, topic); }); - unread.update_unread_counts(); }; return exports; diff --git a/static/js/muting_ui.js b/static/js/muting_ui.js index a06f677bf4..f11df7730b 100644 --- a/static/js/muting_ui.js +++ b/static/js/muting_ui.js @@ -110,12 +110,27 @@ exports.handle_updates = function (muted_topics) { return; } - muting.set_muted_topics(muted_topics); + exports.update_muted_topics(muted_topics); exports.rerender(); }; +exports.mute_topic = function (stream, topic) { + muting.add_muted_topic(stream, topic); + unread.update_unread_counts(); +}; + +exports.unmute_topic = function (stream, topic) { + muting.remove_muted_topic(stream, topic); + unread.update_unread_counts(); +}; + +exports.update_muted_topics = function (muted_topics) { + muting.set_muted_topics(muted_topics); + unread.update_unread_counts(); +}; + $(function () { - muting.set_muted_topics(page_params.muted_topics); + exports.update_muted_topics(page_params.muted_topics); }); return exports; diff --git a/static/js/popovers.js b/static/js/popovers.js index d6eff0b31a..4b3eaf6166 100644 --- a/static/js/popovers.js +++ b/static/js/popovers.js @@ -266,7 +266,7 @@ exports.hide_actions_popover = function () { exports.topic_ops = { mute: function (stream, topic) { popovers.hide_topic_sidebar_popover(); - muting.mute_topic(stream, topic); + muting_ui.mute_topic(stream, topic); muting_ui.persist_and_rerender(); muting_ui.notify_with_undo_option(stream, topic); }, @@ -275,7 +275,7 @@ exports.topic_ops = { // and miss out on info. unmute: function (stream, topic) { popovers.hide_topic_sidebar_popover(); - muting.unmute_topic(stream, topic); + muting_ui.unmute_topic(stream, topic); muting_ui.persist_and_rerender(); }, }; @@ -822,7 +822,7 @@ exports.register_click_handlers = function () { var stream = $(e.currentTarget).data('msg-stream'); var topic = $(e.currentTarget).data('msg-topic'); popovers.hide_actions_popover(); - muting.unmute_topic(stream, topic); + muting_ui.unmute_topic(stream, topic); muting_ui.persist_and_rerender(); e.stopPropagation(); e.preventDefault();