From 4380fe4eafdb159ceee79b678e7054bcda5b6cb4 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Sun, 4 Apr 2021 13:15:18 +0000 Subject: [PATCH] refactor: Extract stream_settings_data. This is mostly a pure code move. In passing I remove an unneeded call to update_calculated_fields in the dispatch code, plus some tests that don't need them. --- frontend_tests/node_tests/buddy_data.js | 5 +- frontend_tests/node_tests/dispatch_subs.js | 1 - frontend_tests/node_tests/narrow.js | 3 +- frontend_tests/node_tests/peer_data.js | 7 +- frontend_tests/node_tests/settings_org.js | 5 +- frontend_tests/node_tests/stream_data.js | 21 +- frontend_tests/node_tests/stream_events.js | 11 +- frontend_tests/node_tests/typeahead_helper.js | 6 - static/js/server_events_dispatch.js | 1 - static/js/settings_notifications.js | 4 +- static/js/settings_org.js | 4 +- static/js/stream_create.js | 3 +- static/js/stream_data.js | 191 +---------------- static/js/stream_edit.js | 3 +- static/js/stream_events.js | 5 +- static/js/stream_settings_data.js | 195 ++++++++++++++++++ static/js/subs.js | 15 +- 17 files changed, 246 insertions(+), 234 deletions(-) create mode 100644 static/js/stream_settings_data.js diff --git a/frontend_tests/node_tests/buddy_data.js b/frontend_tests/node_tests/buddy_data.js index 205930d647..0dfc0f48fa 100644 --- a/frontend_tests/node_tests/buddy_data.js +++ b/frontend_tests/node_tests/buddy_data.js @@ -16,6 +16,7 @@ const peer_data = zrequire("peer_data"); const people = zrequire("people"); const presence = zrequire("presence"); const stream_data = zrequire("stream_data"); +const stream_settings_data = zrequire("stream_settings_data"); const user_status = zrequire("user_status"); const buddy_data = zrequire("buddy_data"); @@ -179,7 +180,7 @@ test("compose fade interactions (streams)", () => { }; stream_data.add_sub(sub); stream_data.subscribe_myself(sub); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); people.add_active_user(fred); @@ -225,7 +226,7 @@ test("compose fade interactions (missing topic)", () => { }; stream_data.add_sub(sub); stream_data.subscribe_myself(sub); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); people.add_active_user(fred); diff --git a/frontend_tests/node_tests/dispatch_subs.js b/frontend_tests/node_tests/dispatch_subs.js index 90532771b6..7ee732f54c 100644 --- a/frontend_tests/node_tests/dispatch_subs.js +++ b/frontend_tests/node_tests/dispatch_subs.js @@ -187,7 +187,6 @@ test("stream create", (override) => { const stub = make_stub(); override(stream_data, "create_streams", stub.f); override(stream_data, "get_sub_by_id", noop); - override(stream_data, "update_calculated_fields", noop); override(subs, "add_sub_to_table", noop); override(overlays, "streams_open", () => true); dispatch(event); diff --git a/frontend_tests/node_tests/narrow.js b/frontend_tests/node_tests/narrow.js index d630242b25..7a597b2422 100644 --- a/frontend_tests/node_tests/narrow.js +++ b/frontend_tests/node_tests/narrow.js @@ -16,6 +16,7 @@ const narrow_banner = zrequire("narrow_banner"); const narrow_state = zrequire("narrow_state"); const people = zrequire("people"); const stream_data = zrequire("stream_data"); +const stream_settings_data = zrequire("stream_settings_data"); const {Filter} = zrequire("../js/filter"); const narrow = zrequire("narrow"); @@ -117,7 +118,7 @@ run_test("show_empty_narrow_message", () => { // for non sub public stream stream_data.add_sub({name: "ROME", stream_id: 99}); - stream_data.update_calculated_fields(stream_data.get_sub("ROME")); + stream_settings_data.update_calculated_fields(stream_data.get_sub("ROME")); set_filter([["stream", "Rome"]]); hide_all_empty_narrow_messages(); narrow_banner.show_empty_narrow_message(); diff --git a/frontend_tests/node_tests/peer_data.js b/frontend_tests/node_tests/peer_data.js index cd72fe5ede..fcee5354fa 100644 --- a/frontend_tests/node_tests/peer_data.js +++ b/frontend_tests/node_tests/peer_data.js @@ -16,6 +16,7 @@ const {page_params} = require("../zjsunit/zpage_params"); const peer_data = zrequire("peer_data"); const people = zrequire("people"); const stream_data = zrequire("stream_data"); +const stream_settings_data = zrequire("stream_settings_data"); page_params.is_admin = false; page_params.realm_users = []; @@ -113,7 +114,7 @@ test("subscribers", () => { ]); peer_data.set_subscribers(stream_id, [me.user_id, fred.user_id, george.user_id]); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert(stream_data.is_user_subscribed(stream_id, me.user_id)); assert(stream_data.is_user_subscribed(stream_id, fred.user_id)); assert(stream_data.is_user_subscribed(stream_id, george.user_id)); @@ -181,7 +182,7 @@ test("subscribers", () => { // Verify that we noop and don't crash when unsubscribed. sub.subscribed = false; - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); peer_data.add_subscriber(stream_id, brutus.user_id); assert.equal(stream_data.is_user_subscribed(stream_id, brutus.user_id), true); peer_data.remove_subscriber(stream_id, brutus.user_id); @@ -195,7 +196,7 @@ test("subscribers", () => { 2, ); sub.invite_only = true; - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert.equal(stream_data.is_user_subscribed(stream_id, brutus.user_id), undefined); peer_data.remove_subscriber(stream_id, brutus.user_id); assert.equal(stream_data.is_user_subscribed(stream_id, brutus.user_id), undefined); diff --git a/frontend_tests/node_tests/settings_org.js b/frontend_tests/node_tests/settings_org.js index 3660de777e..7695e9b754 100644 --- a/frontend_tests/node_tests/settings_org.js +++ b/frontend_tests/node_tests/settings_org.js @@ -55,6 +55,7 @@ set_global("FormData", _FormData); const settings_config = zrequire("settings_config"); const settings_bots = zrequire("settings_bots"); const stream_data = zrequire("stream_data"); +const stream_settings_data = zrequire("stream_settings_data"); const settings_account = zrequire("settings_account"); const settings_org = zrequire("settings_org"); const dropdown_list_widget = zrequire("dropdown_list_widget"); @@ -901,7 +902,7 @@ test("test get_sorted_options_list", () => { assert.deepEqual(settings_org.get_sorted_options_list(option_values_2), expected_option_values); }); -test("misc", () => { +test("misc", (override) => { page_params.is_admin = false; const stub_notification_disable_parent = $.create(" { settings_account.update_email_change_display(); assert(!$("#change_email .button").prop("disabled")); - stream_data.__Rewire__("get_streams_for_settings_page", () => [ + override(stream_settings_data, "get_streams_for_settings_page", () => [ {name: "some_stream", stream_id: 75}, {name: "some_stream", stream_id: 42}, ]); diff --git a/frontend_tests/node_tests/stream_data.js b/frontend_tests/node_tests/stream_data.js index 6f6cf9bbde..0ac5edea27 100644 --- a/frontend_tests/node_tests/stream_data.js +++ b/frontend_tests/node_tests/stream_data.js @@ -13,6 +13,7 @@ const color_data = zrequire("color_data"); const stream_topic_history = zrequire("stream_topic_history"); const people = zrequire("people"); const stream_data = zrequire("stream_data"); +const stream_settings_data = zrequire("stream_settings_data"); const settings_config = zrequire("settings_config"); const me = { @@ -253,7 +254,7 @@ test("admin_options", () => { // non-admins can't do anything page_params.is_admin = false; let sub = make_sub(); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert(!sub.is_realm_admin); assert(!sub.can_change_stream_permissions); @@ -265,7 +266,7 @@ test("admin_options", () => { // admins can make public streams become private sub = make_sub(); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert(sub.is_realm_admin); assert(sub.can_change_stream_permissions); @@ -274,14 +275,14 @@ test("admin_options", () => { sub = make_sub(); sub.invite_only = true; sub.subscribed = false; - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert(sub.is_realm_admin); assert(!sub.can_change_stream_permissions); sub = make_sub(); sub.invite_only = true; sub.subscribed = true; - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert(sub.is_realm_admin); assert(sub.can_change_stream_permissions); }); @@ -317,7 +318,7 @@ test("stream_settings", () => { stream_data.add_sub(amber); stream_data.add_sub(blue); - let sub_rows = stream_data.get_streams_for_settings_page(); + let sub_rows = stream_settings_data.get_streams_for_settings_page(); assert.equal(sub_rows[0].color, "blue"); assert.equal(sub_rows[1].color, "amber"); assert.equal(sub_rows[2].color, "cinnamon"); @@ -344,17 +345,17 @@ test("stream_settings", () => { }); stream_data.update_stream_post_policy(sub, 1); stream_data.update_message_retention_setting(sub, -1); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); assert.equal(sub.invite_only, false); assert.equal(sub.history_public_to_subscribers, false); assert.equal(sub.stream_post_policy, stream_data.stream_post_policy_values.everyone.code); assert.equal(sub.message_retention_days, -1); // For guest user only retrieve subscribed streams - sub_rows = stream_data.get_updated_unsorted_subs(); + sub_rows = stream_settings_data.get_updated_unsorted_subs(); assert.equal(sub_rows.length, 3); page_params.is_guest = true; - sub_rows = stream_data.get_updated_unsorted_subs(); + sub_rows = stream_settings_data.get_updated_unsorted_subs(); assert.equal(sub_rows[0].name, "c"); assert.equal(sub_rows[1].name, "a"); assert.equal(sub_rows.length, 2); @@ -542,7 +543,7 @@ test("notifications", () => { antarctica.push_notifications = null; antarctica.wildcard_mentions_notify = null; - const unmatched_streams = stream_data.get_unmatched_streams_for_notification_settings(); + const unmatched_streams = stream_settings_data.get_unmatched_streams_for_notification_settings(); const expected_streams = [ { desktop_notifications: true, @@ -765,7 +766,7 @@ test("edge_cases", () => { const bad_stream_ids = [555555, 99999]; // just make sure we don't explode - stream_data.sort_for_stream_settings(bad_stream_ids); + stream_settings_data.sort_for_stream_settings(bad_stream_ids); }); test("get_invite_stream_data", () => { diff --git a/frontend_tests/node_tests/stream_events.js b/frontend_tests/node_tests/stream_events.js index aa621c9878..166dc6da3e 100644 --- a/frontend_tests/node_tests/stream_events.js +++ b/frontend_tests/node_tests/stream_events.js @@ -46,6 +46,7 @@ const peer_data = zrequire("peer_data"); const people = zrequire("people"); const stream_data = zrequire("stream_data"); const stream_events = zrequire("stream_events"); +const stream_settings_data = zrequire("stream_settings_data"); const george = { email: "george@zulip.com", @@ -266,7 +267,7 @@ test("marked_subscribed (normal)", (override) => { const sub = {...frontend}; stream_data.add_sub(sub); override(stream_data, "subscribe_myself", noop); - override(stream_data, "update_calculated_fields", noop); + override(stream_settings_data, "update_calculated_fields", noop); override(stream_color, "update_stream_color", noop); @@ -303,7 +304,7 @@ test("marked_subscribed (normal)", (override) => { test("marked_subscribed (color)", (override) => { override(stream_data, "subscribe_myself", noop); - override(stream_data, "update_calculated_fields", noop); + override(stream_settings_data, "update_calculated_fields", noop); override(message_util, "do_unread_count_updates", noop); override(stream_list, "add_sidebar_row", noop); @@ -334,7 +335,7 @@ test("marked_subscribed (color)", (override) => { test("marked_subscribed (emails)", (override) => { const sub = {...frontend}; stream_data.add_sub(sub); - override(stream_data, "update_calculated_fields", noop); + override(stream_settings_data, "update_calculated_fields", noop); override(stream_color, "update_stream_color", noop); // Test assigning subscriber emails @@ -357,7 +358,7 @@ test("marked_subscribed (emails)", (override) => { }); test("mark_unsubscribed (update_settings_for_unsubscribed)", (override) => { - override(stream_data, "update_calculated_fields", noop); + override(stream_settings_data, "update_calculated_fields", noop); // Test unsubscribe const sub = {...dev_help}; @@ -378,7 +379,7 @@ test("mark_unsubscribed (render_title_area)", (override) => { const sub = {...frontend, subscribed: true}; stream_data.add_sub(sub); - override(stream_data, "update_calculated_fields", noop); + override(stream_settings_data, "update_calculated_fields", noop); // Test update bookend and remove done event narrow_to_frontend(); diff --git a/frontend_tests/node_tests/typeahead_helper.js b/frontend_tests/node_tests/typeahead_helper.js index 20304d3e3d..4ff3481b09 100644 --- a/frontend_tests/node_tests/typeahead_helper.js +++ b/frontend_tests/node_tests/typeahead_helper.js @@ -305,9 +305,6 @@ function get_typeahead_result(query, current_stream, current_topic) { } test("sort_recipients", () => { - const dev_sub = stream_data.get_sub("Dev"); - const linux_sub = stream_data.get_sub("Linux"); - // Typeahead for recipientbox [query, "", undefined] assert.deepEqual(get_typeahead_result("b", ""), [ "b_user_1@zulip.net", @@ -337,9 +334,6 @@ test("sort_recipients", () => { peer_data.add_subscriber(1, people.get_user_id(subscriber_email_2)); peer_data.add_subscriber(1, people.get_user_id(subscriber_email_3)); - stream_data.update_calculated_fields(dev_sub); - stream_data.update_calculated_fields(linux_sub); - // For splitting based on whether a PM was sent pm_conversations.set_partner(5); pm_conversations.set_partner(6); diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js index 1af1065a43..9548a3875c 100644 --- a/static/js/server_events_dispatch.js +++ b/static/js/server_events_dispatch.js @@ -406,7 +406,6 @@ export function dispatch_normal_event(event) { for (const stream of event.streams) { const sub = stream_data.get_sub_by_id(stream.stream_id); - stream_data.update_calculated_fields(sub); if (overlays.streams_open()) { subs.add_sub_to_table(sub); } diff --git a/static/js/settings_notifications.js b/static/js/settings_notifications.js index 339e4ba13c..fcca8de077 100644 --- a/static/js/settings_notifications.js +++ b/static/js/settings_notifications.js @@ -9,8 +9,8 @@ import {page_params} from "./page_params"; import * as settings_config from "./settings_config"; import * as settings_org from "./settings_org"; import * as settings_ui from "./settings_ui"; -import * as stream_data from "./stream_data"; import * as stream_edit from "./stream_edit"; +import * as stream_settings_data from "./stream_settings_data"; import * as unread_ui from "./unread_ui"; function rerender_ui() { @@ -20,7 +20,7 @@ function rerender_ui() { return; } - const unmatched_streams = stream_data.get_unmatched_streams_for_notification_settings(); + const unmatched_streams = stream_settings_data.get_unmatched_streams_for_notification_settings(); unmatched_streams_table.find(".stream-row").remove(); diff --git a/static/js/settings_org.js b/static/js/settings_org.js index 5b46e238f6..064d59c913 100644 --- a/static/js/settings_org.js +++ b/static/js/settings_org.js @@ -17,7 +17,7 @@ import * as realm_logo from "./realm_logo"; import * as settings_config from "./settings_config"; import * as settings_notifications from "./settings_notifications"; import * as settings_ui from "./settings_ui"; -import * as stream_data from "./stream_data"; +import * as stream_settings_data from "./stream_settings_data"; import * as ui_report from "./ui_report"; export let parse_time_limit; @@ -613,7 +613,7 @@ export function save_discard_widget_status_handler(subsection) { } export function init_dropdown_widgets() { - const streams = stream_data.get_streams_for_settings_page(); + const streams = stream_settings_data.get_streams_for_settings_page(); const notification_stream_options = { data: streams.map((x) => ({ name: x.name, diff --git a/static/js/stream_create.js b/static/js/stream_create.js index 6953d984e9..c89d4830a1 100644 --- a/static/js/stream_create.js +++ b/static/js/stream_create.js @@ -12,6 +12,7 @@ import {page_params} from "./page_params"; import * as peer_data from "./peer_data"; import * as people from "./people"; import * as stream_data from "./stream_data"; +import * as stream_settings_data from "./stream_settings_data"; import * as subs from "./subs"; import * as ui_report from "./ui_report"; @@ -271,7 +272,7 @@ export function show_new_stream_modal() { all_users.unshift(people.get_by_user_id(page_params.user_id)); return render_new_stream_users({ users: all_users, - streams: stream_data.get_streams_for_settings_page(), + streams: stream_settings_data.get_streams_for_settings_page(), is_admin: page_params.is_admin, }); }); diff --git a/static/js/stream_data.js b/static/js/stream_data.js index b162ae213f..b913ff5c62 100644 --- a/static/js/stream_data.js +++ b/static/js/stream_data.js @@ -1,12 +1,12 @@ import * as blueslip from "./blueslip"; import * as color_data from "./color_data"; import {FoldDict} from "./fold_dict"; -import * as hash_util from "./hash_util"; import {i18n} from "./i18n"; import {page_params} from "./page_params"; import * as peer_data from "./peer_data"; import * as people from "./people"; import * as settings_config from "./settings_config"; +import * as stream_settings_data from "./stream_settings_data"; import * as stream_topic_history from "./stream_topic_history"; import * as util from "./util"; @@ -374,44 +374,6 @@ export function get_unsorted_subs() { return Array.from(stream_info.values()); } -export function get_sub_for_settings(sub) { - // Since we make a copy of the sub here, it may eventually - // make sense to get the other calculated fields here as - // well, instead of using update_calculated_fields everywhere. - const sub_count = peer_data.get_subscriber_count(sub.stream_id); - return { - ...sub, - subscriber_count: sub_count, - }; -} - -function get_subs_for_settings(subs) { - // We may eventually add subscribers to the subs here, rather than - // delegating, so that we can more efficiently compute subscriber counts - // (in bulk). If that plan appears to have been aborted, feel free to - // inline this. - return subs.map((sub) => get_sub_for_settings(sub)); -} - -export function get_updated_unsorted_subs() { - // This function is expensive in terms of calculating - // some values (particularly stream counts) but avoids - // prematurely sorting subs. - let all_subs = Array.from(stream_info.values()); - - // Add in admin options and stream counts. - for (const sub of all_subs) { - update_calculated_fields(sub); - } - - // We don't display unsubscribed streams to guest users. - if (page_params.is_guest) { - all_subs = all_subs.filter((sub) => sub.subscribed); - } - - return get_subs_for_settings(all_subs); -} - export function num_subscribed_subs() { return stream_info.num_true_items(); } @@ -495,39 +457,6 @@ export function receives_notifications(stream_id, notification_name) { return page_params["enable_stream_" + notification_name]; } -export function update_calculated_fields(sub) { - // Note that we don't calculate subscriber counts here. - - sub.is_realm_admin = page_params.is_admin; - // Admin can change any stream's name & description either stream is public or - // private, subscribed or unsubscribed. - sub.can_change_name_description = page_params.is_admin; - // If stream is public then any user can subscribe. If stream is private then only - // subscribed users can unsubscribe. - // Guest users can't subscribe themselves to any stream. - sub.should_display_subscription_button = - sub.subscribed || (!page_params.is_guest && !sub.invite_only); - sub.should_display_preview_button = - sub.subscribed || !sub.invite_only || sub.previously_subscribed; - sub.can_change_stream_permissions = - page_params.is_admin && (!sub.invite_only || sub.subscribed); - // User can add other users to stream if stream is public or user is subscribed to stream. - // Guest users can't access subscribers of any(public or private) non-subscribed streams. - sub.can_access_subscribers = - page_params.is_admin || sub.subscribed || (!page_params.is_guest && !sub.invite_only); - sub.preview_url = hash_util.by_stream_uri(sub.stream_id); - sub.can_add_subscribers = !page_params.is_guest && (!sub.invite_only || sub.subscribed); - sub.is_old_stream = sub.stream_weekly_traffic !== null; - if (sub.rendered_description !== undefined) { - sub.rendered_description = sub.rendered_description.replace("

", "").replace("

", ""); - } - - // Apply the defaults for our notification settings for rendering. - for (const setting of settings_config.stream_specific_notification_settings) { - sub[setting + "_display"] = receives_notifications(sub.stream_id, setting); - } -} - export function all_subscribed_streams_are_in_home_view() { return subscribed_subs().every((sub) => !sub.is_muted); } @@ -724,7 +653,8 @@ export function create_sub_from_server_data(attrs) { sub.color = color_data.pick_color(); } - update_calculated_fields(sub); + // TODO: Let stream settings code add these fields. + stream_settings_data.update_calculated_fields(sub); stream_info.set(sub.name, sub); subs_by_stream_id.set(sub.stream_id, sub); @@ -732,121 +662,6 @@ export function create_sub_from_server_data(attrs) { return sub; } -export function get_unmatched_streams_for_notification_settings() { - const subscribed_rows = subscribed_subs(); - subscribed_rows.sort((a, b) => util.strcmp(a.name, b.name)); - - const notification_settings = []; - for (const row of subscribed_rows) { - const settings_values = {}; - let make_table_row = false; - for (const notification_name of settings_config.stream_specific_notification_settings) { - const prepend = - notification_name === "wildcard_mentions_notify" ? "" : "enable_stream_"; - const default_setting = page_params[prepend + notification_name]; - const stream_setting = receives_notifications(row.stream_id, notification_name); - - settings_values[notification_name] = stream_setting; - if (stream_setting !== default_setting) { - make_table_row = true; - } - } - // We do not need to display the streams whose settings - // match with the global settings defined by the user. - if (make_table_row) { - settings_values.stream_name = row.name; - settings_values.stream_id = row.stream_id; - settings_values.invite_only = row.invite_only; - settings_values.is_web_public = row.is_web_public; - - notification_settings.push(settings_values); - } - } - return notification_settings; -} - -export function get_streams_for_settings_page() { - // TODO: This function is only used for copy-from-stream, so - // the current name is slightly misleading now, plus - // it's not entirely clear we need unsubscribed streams - // for that. Also we may be revisiting that UI. - - // Build up our list of subscribed streams from the data we already have. - const subscribed_rows = subscribed_subs(); - const unsubscribed_rows = unsubscribed_subs(); - - // Sort and combine all our streams. - function by_name(a, b) { - return util.strcmp(a.name, b.name); - } - subscribed_rows.sort(by_name); - unsubscribed_rows.sort(by_name); - const all_subs = unsubscribed_rows.concat(subscribed_rows); - - // Add in admin options and stream counts. - for (const sub of all_subs) { - update_calculated_fields(sub); - } - - return get_subs_for_settings(all_subs); -} - -export function sort_for_stream_settings(stream_ids, order) { - // TODO: We may want to simply use util.strcmp here, - // which uses Intl.Collator() when possible. - - function name(stream_id) { - const sub = get_sub_by_id(stream_id); - if (!sub) { - return ""; - } - return sub.name.toLocaleLowerCase(); - } - - function weekly_traffic(stream_id) { - const sub = get_sub_by_id(stream_id); - if (sub && sub.is_old_stream) { - return sub.stream_weekly_traffic; - } - // don't intersperse new streams with zero-traffic existing streams - return -1; - } - - function by_stream_name(id_a, id_b) { - const stream_a_name = name(id_a); - const stream_b_name = name(id_b); - return String.prototype.localeCompare.call(stream_a_name, stream_b_name); - } - - function by_subscriber_count(id_a, id_b) { - const out = peer_data.get_subscriber_count(id_b) - peer_data.get_subscriber_count(id_a); - if (out === 0) { - return by_stream_name(id_a, id_b); - } - return out; - } - - function by_weekly_traffic(id_a, id_b) { - const out = weekly_traffic(id_b) - weekly_traffic(id_a); - if (out === 0) { - return by_stream_name(id_a, id_b); - } - return out; - } - - const orders = new Map([ - ["by-stream-name", by_stream_name], - ["by-subscriber-count", by_subscriber_count], - ["by-weekly-traffic", by_weekly_traffic], - ]); - - if (order === undefined || !orders.has(order)) { - order = "by-stream-name"; - } - - stream_ids.sort(orders.get(order)); -} - export function get_streams_for_admin() { // Sort and combine all our streams. function by_name(a, b) { diff --git a/static/js/stream_edit.js b/static/js/stream_edit.js index 58c957a29f..df90fda5b9 100644 --- a/static/js/stream_edit.js +++ b/static/js/stream_edit.js @@ -27,6 +27,7 @@ import * as settings_ui from "./settings_ui"; import * as stream_color from "./stream_color"; import * as stream_data from "./stream_data"; import * as stream_pill from "./stream_pill"; +import * as stream_settings_data from "./stream_settings_data"; import * as stream_ui_updates from "./stream_ui_updates"; import * as subs from "./subs"; import * as ui from "./ui"; @@ -434,7 +435,7 @@ export function show_settings_for(node) { const stream_id = get_stream_id(node); const sub = stream_data.get_sub_by_id(stream_id); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); const html = render_subscription_settings({ sub, settings: stream_settings(sub), diff --git a/static/js/stream_events.js b/static/js/stream_events.js index b837dd82bb..5e7de406e8 100644 --- a/static/js/stream_events.js +++ b/static/js/stream_events.js @@ -15,6 +15,7 @@ import * as stream_color from "./stream_color"; import * as stream_data from "./stream_data"; import * as stream_list from "./stream_list"; import * as stream_muting from "./stream_muting"; +import * as stream_settings_data from "./stream_settings_data"; import * as subs from "./subs"; // In theory, this function should apply the account-level defaults, @@ -117,7 +118,7 @@ export function mark_subscribed(sub, subscribers, color) { if (subscribers) { peer_data.set_subscribers(sub.stream_id, subscribers); } - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); if (overlays.streams_open()) { subs.update_settings_for_subscribed(sub); @@ -143,7 +144,7 @@ export function mark_unsubscribed(sub) { return; } else if (sub.subscribed) { stream_data.unsubscribe_myself(sub); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); if (overlays.streams_open()) { subs.update_settings_for_unsubscribed(sub); } diff --git a/static/js/stream_settings_data.js b/static/js/stream_settings_data.js new file mode 100644 index 0000000000..908735269b --- /dev/null +++ b/static/js/stream_settings_data.js @@ -0,0 +1,195 @@ +import * as hash_util from "./hash_util"; +import {page_params} from "./page_params"; +import * as peer_data from "./peer_data"; +import * as settings_config from "./settings_config"; +import * as stream_data from "./stream_data"; +import * as util from "./util"; + +export function get_sub_for_settings(sub) { + // Since we make a copy of the sub here, it may eventually + // make sense to get the other calculated fields here as + // well, instead of using update_calculated_fields everywhere. + const sub_count = peer_data.get_subscriber_count(sub.stream_id); + return { + ...sub, + subscriber_count: sub_count, + }; +} + +function get_subs_for_settings(subs) { + // We may eventually add subscribers to the subs here, rather than + // delegating, so that we can more efficiently compute subscriber counts + // (in bulk). If that plan appears to have been aborted, feel free to + // inline this. + return subs.map((sub) => get_sub_for_settings(sub)); +} + +export function get_updated_unsorted_subs() { + // This function is expensive in terms of calculating + // some values (particularly stream counts) but avoids + // prematurely sorting subs. + let all_subs = stream_data.get_unsorted_subs(); + + // Add in admin options and stream counts. + for (const sub of all_subs) { + update_calculated_fields(sub); + } + + // We don't display unsubscribed streams to guest users. + if (page_params.is_guest) { + all_subs = all_subs.filter((sub) => sub.subscribed); + } + + return get_subs_for_settings(all_subs); +} + +export function update_calculated_fields(sub) { + // Note that we don't calculate subscriber counts here. + + sub.is_realm_admin = page_params.is_admin; + // Admin can change any stream's name & description either stream is public or + // private, subscribed or unsubscribed. + sub.can_change_name_description = page_params.is_admin; + // If stream is public then any user can subscribe. If stream is private then only + // subscribed users can unsubscribe. + // Guest users can't subscribe themselves to any stream. + sub.should_display_subscription_button = + sub.subscribed || (!page_params.is_guest && !sub.invite_only); + sub.should_display_preview_button = + sub.subscribed || !sub.invite_only || sub.previously_subscribed; + sub.can_change_stream_permissions = + page_params.is_admin && (!sub.invite_only || sub.subscribed); + // User can add other users to stream if stream is public or user is subscribed to stream. + // Guest users can't access subscribers of any(public or private) non-subscribed streams. + sub.can_access_subscribers = + page_params.is_admin || sub.subscribed || (!page_params.is_guest && !sub.invite_only); + sub.preview_url = hash_util.by_stream_uri(sub.stream_id); + sub.can_add_subscribers = !page_params.is_guest && (!sub.invite_only || sub.subscribed); + sub.is_old_stream = sub.stream_weekly_traffic !== null; + if (sub.rendered_description !== undefined) { + sub.rendered_description = sub.rendered_description.replace("

", "").replace("

", ""); + } + + // Apply the defaults for our notification settings for rendering. + for (const setting of settings_config.stream_specific_notification_settings) { + sub[setting + "_display"] = stream_data.receives_notifications(sub.stream_id, setting); + } +} + +export function get_unmatched_streams_for_notification_settings() { + const subscribed_rows = stream_data.subscribed_subs(); + subscribed_rows.sort((a, b) => util.strcmp(a.name, b.name)); + + const notification_settings = []; + for (const row of subscribed_rows) { + const settings_values = {}; + let make_table_row = false; + for (const notification_name of settings_config.stream_specific_notification_settings) { + const prepend = + notification_name === "wildcard_mentions_notify" ? "" : "enable_stream_"; + const default_setting = page_params[prepend + notification_name]; + const stream_setting = stream_data.receives_notifications( + row.stream_id, + notification_name, + ); + + settings_values[notification_name] = stream_setting; + if (stream_setting !== default_setting) { + make_table_row = true; + } + } + // We do not need to display the streams whose settings + // match with the global settings defined by the user. + if (make_table_row) { + settings_values.stream_name = row.name; + settings_values.stream_id = row.stream_id; + settings_values.invite_only = row.invite_only; + settings_values.is_web_public = row.is_web_public; + + notification_settings.push(settings_values); + } + } + return notification_settings; +} + +export function get_streams_for_settings_page() { + // TODO: This function is only used for copy-from-stream, so + // the current name is slightly misleading now, plus + // it's not entirely clear we need unsubscribed streams + // for that. Also we may be revisiting that UI. + + // Build up our list of subscribed streams from the data we already have. + const subscribed_rows = stream_data.subscribed_subs(); + const unsubscribed_rows = stream_data.unsubscribed_subs(); + + // Sort and combine all our streams. + function by_name(a, b) { + return util.strcmp(a.name, b.name); + } + subscribed_rows.sort(by_name); + unsubscribed_rows.sort(by_name); + const all_subs = unsubscribed_rows.concat(subscribed_rows); + + // Add in admin options and stream counts. + for (const sub of all_subs) { + update_calculated_fields(sub); + } + + return get_subs_for_settings(all_subs); +} + +export function sort_for_stream_settings(stream_ids, order) { + // TODO: We may want to simply use util.strcmp here, + // which uses Intl.Collator() when possible. + + function name(stream_id) { + const sub = stream_data.get_sub_by_id(stream_id); + if (!sub) { + return ""; + } + return sub.name.toLocaleLowerCase(); + } + + function weekly_traffic(stream_id) { + const sub = stream_data.get_sub_by_id(stream_id); + if (sub && sub.is_old_stream) { + return sub.stream_weekly_traffic; + } + // don't intersperse new streams with zero-traffic existing streams + return -1; + } + + function by_stream_name(id_a, id_b) { + const stream_a_name = name(id_a); + const stream_b_name = name(id_b); + return String.prototype.localeCompare.call(stream_a_name, stream_b_name); + } + + function by_subscriber_count(id_a, id_b) { + const out = peer_data.get_subscriber_count(id_b) - peer_data.get_subscriber_count(id_a); + if (out === 0) { + return by_stream_name(id_a, id_b); + } + return out; + } + + function by_weekly_traffic(id_a, id_b) { + const out = weekly_traffic(id_b) - weekly_traffic(id_a); + if (out === 0) { + return by_stream_name(id_a, id_b); + } + return out; + } + + const orders = new Map([ + ["by-stream-name", by_stream_name], + ["by-subscriber-count", by_subscriber_count], + ["by-weekly-traffic", by_weekly_traffic], + ]); + + if (order === undefined || !orders.has(order)) { + order = "by-stream-name"; + } + + stream_ids.sort(orders.get(order)); +} diff --git a/static/js/subs.js b/static/js/subs.js index 7b5e3d8c38..5c8fd170ea 100644 --- a/static/js/subs.js +++ b/static/js/subs.js @@ -28,6 +28,7 @@ import * as stream_data from "./stream_data"; import * as stream_edit from "./stream_edit"; import * as stream_list from "./stream_list"; import * as stream_muting from "./stream_muting"; +import * as stream_settings_data from "./stream_settings_data"; import * as stream_ui_updates from "./stream_ui_updates"; import * as ui from "./ui"; import * as ui_report from "./ui_report"; @@ -64,7 +65,7 @@ export function update_left_panel_row(sub) { } blueslip.debug(`Updating row in left panel of stream settings for: ${sub.name}`); - const setting_sub = stream_data.get_sub_for_settings(sub); + const setting_sub = stream_settings_data.get_sub_for_settings(sub); const html = render_subscription(setting_sub); const new_row = $(html); @@ -223,7 +224,7 @@ export function update_stream_description(sub, description, rendered_description export function update_stream_privacy(sub, values) { stream_data.update_stream_privacy(sub, values); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); // Update UI elements update_left_panel_row(sub); @@ -239,7 +240,7 @@ export function update_stream_privacy(sub, values) { export function update_stream_post_policy(sub, new_value) { stream_data.update_stream_post_policy(sub, new_value); - stream_data.update_calculated_fields(sub); + stream_settings_data.update_calculated_fields(sub); stream_ui_updates.update_stream_subscription_type_text(sub); } @@ -272,7 +273,7 @@ export function add_sub_to_table(sub) { return; } - const setting_sub = stream_data.get_sub_for_settings(sub); + const setting_sub = stream_settings_data.get_sub_for_settings(sub); const html = render_subscription(setting_sub); const new_row = $(html); add_tooltip_to_left_panel_row(new_row); @@ -421,8 +422,8 @@ function get_stream_id_buckets(stream_ids, left_panel_params) { } } - stream_data.sort_for_stream_settings(buckets.name, left_panel_params.sort_order); - stream_data.sort_for_stream_settings(buckets.desc, left_panel_params.sort_order); + stream_settings_data.sort_for_stream_settings(buckets.name, left_panel_params.sort_order); + stream_settings_data.sort_for_stream_settings(buckets.desc, left_panel_params.sort_order); return buckets; } @@ -432,7 +433,7 @@ export function render_left_panel_superset() { // allowed to know about and put them in the DOM, then we do // a second pass where we filter/sort them. const html = blueslip.measure_time("render left panel", () => { - const sub_rows = stream_data.get_updated_unsorted_subs(); + const sub_rows = stream_settings_data.get_updated_unsorted_subs(); const template_data = { subscriptions: sub_rows,