refactor: Avoid update_calculated_fields() calls.

This change should make live-update code less brittle,
or at least less cumbersome.

Instead of having to re-compute calculated fields for
every change to a stream message, we now just compute
the fields right before we render stream settings UI.

frontend_tests/node_tests/buddy_data.js

@@ -16,7 +16,6 @@ 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");
 
@@ -180,7 +179,6 @@ test("compose fade interactions (streams)", () => {
     };
     stream_data.add_sub(sub);
     stream_data.subscribe_myself(sub);
-    stream_settings_data.update_calculated_fields(sub);
 
     people.add_active_user(fred);
 
@@ -226,7 +224,6 @@ test("compose fade interactions (missing topic)", () => {
     };
     stream_data.add_sub(sub);
     stream_data.subscribe_myself(sub);
-    stream_settings_data.update_calculated_fields(sub);
 
     people.add_active_user(fred);
 

frontend_tests/node_tests/composebox_typeahead.js

@@ -49,7 +49,6 @@ const settings_config = zrequire("settings_config");
 const pygments_data = zrequire("../generated/pygments_data.json");
 
 // To be eliminated in next commit:
-stream_data.__Rewire__("update_calculated_fields", () => {});
 stream_data.__Rewire__("set_filter_out_inactives", () => false);
 
 const ct = composebox_typeahead;

frontend_tests/node_tests/stream_data.js

@@ -251,12 +251,19 @@ test("admin_options", () => {
         return sub;
     }
 
+    function is_realm_admin(sub) {
+        return stream_settings_data.get_sub_for_settings(sub).is_realm_admin;
+    }
+
+    function can_change_stream_permissions(sub) {
+        return stream_settings_data.get_sub_for_settings(sub).can_change_stream_permissions;
+    }
+
     // non-admins can't do anything
     page_params.is_admin = false;
     let sub = make_sub();
-    stream_settings_data.update_calculated_fields(sub);
-    assert(!sub.is_realm_admin);
-    assert(!sub.can_change_stream_permissions);
+    assert(!is_realm_admin(sub));
+    assert(!can_change_stream_permissions(sub));
 
     // just a sanity check that we leave "normal" fields alone
     assert.equal(sub.color, "blue");
@@ -266,25 +273,22 @@ test("admin_options", () => {
 
     // admins can make public streams become private
     sub = make_sub();
-    stream_settings_data.update_calculated_fields(sub);
-    assert(sub.is_realm_admin);
-    assert(sub.can_change_stream_permissions);
+    assert(is_realm_admin(sub));
+    assert(can_change_stream_permissions(sub));
 
     // admins can only make private streams become public
     // if they are subscribed
     sub = make_sub();
     sub.invite_only = true;
     sub.subscribed = false;
-    stream_settings_data.update_calculated_fields(sub);
-    assert(sub.is_realm_admin);
-    assert(!sub.can_change_stream_permissions);
+    assert(is_realm_admin(sub));
+    assert(!can_change_stream_permissions(sub));
 
     sub = make_sub();
     sub.invite_only = true;
     sub.subscribed = true;
-    stream_settings_data.update_calculated_fields(sub);
-    assert(sub.is_realm_admin);
-    assert(sub.can_change_stream_permissions);
+    assert(is_realm_admin(sub));
+    assert(can_change_stream_permissions(sub));
 });
 
 test("stream_settings", () => {
@@ -345,7 +349,6 @@ test("stream_settings", () => {
     });
     stream_data.update_stream_post_policy(sub, 1);
     stream_data.update_message_retention_setting(sub, -1);
-    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);

frontend_tests/node_tests/stream_events.js

@@ -46,7 +46,6 @@ 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",
@@ -267,8 +266,6 @@ test("marked_subscribed (normal)", (override) => {
     const sub = {...frontend};
     stream_data.add_sub(sub);
     override(stream_data, "subscribe_myself", noop);
-    override(stream_settings_data, "update_calculated_fields", noop);
-
     override(stream_color, "update_stream_color", noop);
 
     narrow_to_frontend();
@@ -304,7 +301,6 @@ test("marked_subscribed (normal)", (override) => {
 
 test("marked_subscribed (color)", (override) => {
     override(stream_data, "subscribe_myself", noop);
-    override(stream_settings_data, "update_calculated_fields", noop);
     override(message_util, "do_unread_count_updates", noop);
     override(stream_list, "add_sidebar_row", noop);
 
@@ -335,7 +331,6 @@ test("marked_subscribed (color)", (override) => {
 test("marked_subscribed (emails)", (override) => {
     const sub = {...frontend};
     stream_data.add_sub(sub);
-    override(stream_settings_data, "update_calculated_fields", noop);
     override(stream_color, "update_stream_color", noop);
 
     // Test assigning subscriber emails
@@ -358,8 +353,6 @@ test("marked_subscribed (emails)", (override) => {
 });
 
 test("mark_unsubscribed (update_settings_for_unsubscribed)", (override) => {
-    override(stream_settings_data, "update_calculated_fields", noop);
-
     // Test unsubscribe
     const sub = {...dev_help};
     assert(sub.subscribed);
@@ -379,8 +372,6 @@ test("mark_unsubscribed (render_title_area)", (override) => {
     const sub = {...frontend, subscribed: true};
     stream_data.add_sub(sub);
 
-    override(stream_settings_data, "update_calculated_fields", noop);
-
     // Test update bookend and remove done event
     narrow_to_frontend();
     const message_view_header_stub = make_stub();

static/js/stream_data.js

@@ -6,7 +6,6 @@ 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";
 
@@ -684,8 +683,6 @@ export function create_sub_from_server_data(attrs) {
         sub.color = color_data.pick_color();
     }
 
-    // TODO: Let stream settings code add these fields.
-    stream_settings_data.update_calculated_fields(sub);
     clean_up_description(sub);
 
     stream_info.set(sub.name, sub);

static/js/stream_edit.js

@@ -439,10 +439,10 @@ export function stream_settings(sub) {
 
 export function show_settings_for(node) {
     const stream_id = get_stream_id(node);
-    const sub = stream_data.get_sub_by_id(stream_id);
+    const slim_sub = stream_data.get_sub_by_id(stream_id);
+    stream_data.clean_up_description(slim_sub);
+    const sub = stream_settings_data.get_sub_for_settings(slim_sub);
 
-    stream_settings_data.update_calculated_fields(sub);
-    stream_data.clean_up_description(sub);
     const html = render_subscription_settings({
         sub,
         settings: stream_settings(sub),

static/js/stream_events.js

@@ -15,7 +15,6 @@ 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,
@@ -118,7 +117,6 @@ export function mark_subscribed(sub, subscribers, color) {
     if (subscribers) {
         peer_data.set_subscribers(sub.stream_id, subscribers);
     }
-    stream_settings_data.update_calculated_fields(sub);
 
     if (overlays.streams_open()) {
         subs.update_settings_for_subscribed(sub);
@@ -144,7 +142,6 @@ export function mark_unsubscribed(sub) {
         return;
     } else if (sub.subscribed) {
         stream_data.unsubscribe_myself(sub);
-        stream_settings_data.update_calculated_fields(sub);
         if (overlays.streams_open()) {
             subs.update_settings_for_unsubscribed(sub);
         }

static/js/stream_settings_data.js

@@ -6,14 +6,10 @@ 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,
-    };
+    const settings_sub = {...sub};
+    add_settings_fields(settings_sub);
+    settings_sub.subscriber_count = peer_data.get_subscriber_count(sub.stream_id);
+    return settings_sub;
 }
 
 function get_subs_for_settings(subs) {
@@ -25,16 +21,8 @@ function get_subs_for_settings(subs) {
 }
 
 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);
@@ -43,7 +31,7 @@ export function get_updated_unsorted_subs() {
     return get_subs_for_settings(all_subs);
 }
 
-export function update_calculated_fields(sub) {
+export function add_settings_fields(sub) {
     // Note that we don't calculate subscriber counts here.
 
     sub.is_realm_admin = page_params.is_admin;
@@ -115,11 +103,6 @@ export function get_streams_for_settings_page() {
     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);
 }
 

static/js/subs.js

@@ -225,7 +225,6 @@ export function update_stream_description(sub, description, rendered_description
 
 export function update_stream_privacy(sub, values) {
     stream_data.update_stream_privacy(sub, values);
-    stream_settings_data.update_calculated_fields(sub);
 
     // Update UI elements
     update_left_panel_row(sub);
@@ -241,8 +240,6 @@ 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_settings_data.update_calculated_fields(sub);
-
     stream_ui_updates.update_stream_subscription_type_text(sub);
 }