From ecb3879d0c40508b810e1efa519638a8ef801fbb Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sat, 4 Aug 2018 17:44:31 +0000 Subject: [PATCH] refactor: Break subs dependency in stream_data. We move remove_deactivated_user_from_all_streams into stream_events.js. There were some minor changes to rename variables and also to not rely on using `stream_info`. --- frontend_tests/node_tests/dispatch.js | 2 +- frontend_tests/node_tests/stream_data.js | 6 --- frontend_tests/node_tests/stream_events.js | 48 ++++++++++++++++++---- static/js/server_events_dispatch.js | 2 +- static/js/stream_data.js | 10 ----- static/js/stream_events.js | 11 +++++ 6 files changed, 54 insertions(+), 25 deletions(-) diff --git a/frontend_tests/node_tests/dispatch.js b/frontend_tests/node_tests/dispatch.js index 479346d037..0c761c8cf0 100644 --- a/frontend_tests/node_tests/dispatch.js +++ b/frontend_tests/node_tests/dispatch.js @@ -820,7 +820,7 @@ with_overrides(function (override) { event = event_fixtures.realm_user__remove; global.with_stub(function (stub) { override('people.deactivate', stub.f); - override('stream_data.remove_deactivated_user_from_all_streams', noop); + override('stream_events.remove_deactivated_user_from_all_streams', noop); dispatch(event); var args = stub.get_args('person'); assert_same(args.person, event.person); diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js index 0e9d18142e..2ef0d913be 100644 --- a/frontend_tests/node_tests/stream_data.js +++ b/frontend_tests/node_tests/stream_data.js @@ -212,12 +212,6 @@ run_test('subscribers', () => { stream_data.update_subscribers_count(sub); assert.equal(sub.subscriber_count, 0); - // verify that deactivating user should unsubscribe user from all streams - assert(stream_data.add_subscriber('Rome', george.user_id)); - set_global('subs', { rerender_subscriptions_settings: function () {} }); - stream_data.remove_deactivated_user_from_all_streams(george.user_id); - assert(!stream_data.is_user_subscribed('Rome', george.user_id)); - // verify that checking subscription with undefined user id blueslip.set_test_data('warn', 'Undefined user_id passed to function is_user_subscribed'); diff --git a/frontend_tests/node_tests/stream_events.js b/frontend_tests/node_tests/stream_events.js index 8110b65c7f..6811f4dbf7 100644 --- a/frontend_tests/node_tests/stream_events.js +++ b/frontend_tests/node_tests/stream_events.js @@ -10,10 +10,18 @@ set_global('colorspace', { }, }); +zrequire('people'); zrequire('stream_data'); zrequire('stream_events'); var with_overrides = global.with_overrides; +var george = { + email: 'george@zulip.com', + full_name: 'George', + user_id: 103, +}; +people.add(george); + var frontend = { subscribed: false, color: 'yellow', @@ -22,7 +30,8 @@ var frontend = { in_home_view: false, invite_only: false, }; -stream_data.add_sub('Frontend', frontend); + +stream_data.add_sub(frontend.name, frontend); run_test('update_property', () => { // Invoke error for non-existent stream/property @@ -149,12 +158,11 @@ run_test('marked_subscribed', () => { }, }); - set_global('stream_data', { - subscribe_myself: noop, - set_subscribers: noop, - get_colors: noop, - update_calculated_fields: noop, - }); + stream_data.subscribe_myself = noop; + stream_data.set_subscribers = noop; + stream_data.get_colors = noop; + stream_data.update_calculated_fields = noop; + set_global('subs', { update_settings_for_subscribed: noop }); set_global('narrow_state', { is_for_stream_id: noop }); set_global('overlays', { streams_open: return_true }); @@ -312,3 +320,29 @@ run_test('mark_unsubscribed', () => { assert.equal(event_triggered, true); }); }); + +stream_data.clear_subscriptions(); +var dev_help = { + subscribed: true, + color: 'blue', + name: 'dev help', + stream_id: 2, + in_home_view: false, + invite_only: false, +}; +stream_data.add_sub(dev_help.name, dev_help); + +run_test('remove_deactivated_user_from_all_streams', () => { + subs.rerender_subscriptions_settings = () => {}; + + dev_help.can_access_subscribers = true; + + // verify that deactivating user should unsubscribe user from all streams + assert(stream_data.add_subscriber(dev_help.name, george.user_id)); + assert(dev_help.subscribers.has(george.user_id)); + + stream_events.remove_deactivated_user_from_all_streams(george.user_id); + + assert(!dev_help.subscribers.has(george.user_id)); +}); + diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index a2be230e2c..d50ca0a11b 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -212,7 +212,7 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) { people.add_in_realm(event.person); } else if (event.op === 'remove') { people.deactivate(event.person); - stream_data.remove_deactivated_user_from_all_streams(event.person.user_id); + stream_events.remove_deactivated_user_from_all_streams(event.person.user_id); } else if (event.op === 'update') { user_events.update_person(event.person); } diff --git a/static/js/stream_data.js b/static/js/stream_data.js index 30395b2f59..3c68d2384d 100644 --- a/static/js/stream_data.js +++ b/static/js/stream_data.js @@ -381,16 +381,6 @@ exports.add_subscriber = function (stream_name, user_id) { return true; }; -exports.remove_deactivated_user_from_all_streams = function (user_id) { - stream_info.values().forEach(function (stream) { - if (exports.is_user_subscribed(stream.name, user_id)) { - exports.remove_subscriber(stream.name, user_id); - var sub = exports.get_sub(stream.name); - subs.rerender_subscriptions_settings(sub); - } - }); -}; - exports.remove_subscriber = function (stream_name, user_id) { var sub = exports.get_sub(stream_name); if (typeof sub === 'undefined') { diff --git a/static/js/stream_events.js b/static/js/stream_events.js index 850592a902..d04f06efd6 100644 --- a/static/js/stream_events.js +++ b/static/js/stream_events.js @@ -153,6 +153,17 @@ exports.mark_unsubscribed = function (sub) { $(document).trigger($.Event('subscription_remove_done.zulip', {sub: sub})); }; +exports.remove_deactivated_user_from_all_streams = function (user_id) { + var all_subs = stream_data.get_unsorted_subs(); + + _.each(all_subs, function (sub) { + if (stream_data.is_user_subscribed(sub.name, user_id)) { + stream_data.remove_subscriber(sub.name, user_id); + subs.rerender_subscriptions_settings(sub); + } + }); +}; + return exports;