compose_recipient: Move selected_recipient_id to compose_state.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

web/src/compose_actions.js

@@ -234,9 +234,11 @@ export function start(msg_type, opts) {
     stream_bar.decorate(opts.stream_id, $stream_header_colorblock);
 
     if (msg_type === "private") {
-        compose_state.set_compose_recipient_id(compose_recipient.DIRECT_MESSAGE_ID);
+        compose_state.set_compose_recipient_id(compose_state.DIRECT_MESSAGE_ID);
+        compose_recipient.on_compose_select_recipient_update();
     } else if (opts.stream_id) {
         compose_state.set_stream_id(opts.stream_id);
+        compose_recipient.on_compose_select_recipient_update();
     } else {
         // Open stream selection dropdown if no stream is selected.
         compose_recipient.open_compose_recipient_dropdown();

web/src/compose_recipient.js

@@ -23,18 +23,6 @@ import * as sub_store from "./sub_store";
 import * as ui_util from "./ui_util";
 import * as util from "./util";
 
-// selected_recipient_id is the current state for the stream picker widget:
-// "" -> stream message but no stream is selected
-// integer -> stream id of the selected stream.
-// "direct" -> Direct message is selected.
-export let selected_recipient_id = "";
-export const DIRECT_MESSAGE_ID = "direct";
-
-export function set_selected_recipient_id(recipient_id) {
-    selected_recipient_id = recipient_id;
-    on_compose_select_recipient_update();
-}
-
 function composing_to_current_topic_narrow() {
     return (
         util.lower_same(compose_state.stream_name(), narrow_state.stream_name() || "") &&
@@ -110,7 +98,7 @@ export function update_on_recipient_change() {
 }
 
 export function get_posting_policy_error_message() {
-    if (selected_recipient_id === "direct") {
+    if (compose_state.selected_recipient_id === "direct") {
         const recipients = compose_pm_pill.get_user_ids_string();
         if (!people.user_can_direct_message(recipients)) {
             return $t({
@@ -120,7 +108,7 @@ export function get_posting_policy_error_message() {
         return "";
     }
 
-    const stream = sub_store.get(selected_recipient_id);
+    const stream = sub_store.get(compose_state.selected_recipient_id);
     if (stream && !stream_data.can_post_messages_in_stream(stream)) {
         return $t({
             defaultMessage: "You do not have permission to post in this stream.",
@@ -138,7 +126,7 @@ export function check_posting_policy_for_compose_box() {
     }
 
     let banner_classname = compose_banner.CLASSNAMES.no_post_permissions;
-    if (selected_recipient_id === "direct") {
+    if (compose_state.selected_recipient_id === "direct") {
         banner_classname = compose_banner.CLASSNAMES.private_messages_disabled;
     }
     $(".compose_right_float_container").addClass("disabled-compose-send-button-container");
@@ -206,7 +194,7 @@ export function on_compose_select_recipient_update() {
     const prev_message_type = compose_state.get_message_type();
 
     let curr_message_type = "stream";
-    if (selected_recipient_id === DIRECT_MESSAGE_ID) {
+    if (compose_state.selected_recipient_id === compose_state.DIRECT_MESSAGE_ID) {
         curr_message_type = "private";
     }
 
@@ -229,17 +217,18 @@ export function on_compose_select_recipient_update() {
 }
 
 export function possibly_update_stream_name_in_compose(stream_id) {
-    if (selected_recipient_id === stream_id) {
+    if (compose_state.selected_recipient_id === stream_id) {
         on_compose_select_recipient_update();
     }
 }
 
 function item_click_callback(event, dropdown) {
     let recipient_id = $(event.currentTarget).attr("data-unique-id");
-    if (recipient_id !== DIRECT_MESSAGE_ID) {
+    if (recipient_id !== compose_state.DIRECT_MESSAGE_ID) {
         recipient_id = Number.parseInt(recipient_id, 10);
     }
-    set_selected_recipient_id(recipient_id);
+    compose_state.set_selected_recipient_id(recipient_id);
+    on_compose_select_recipient_update();
     dropdown.hide();
     event.preventDefault();
     event.stopPropagation();
@@ -250,7 +239,7 @@ function get_options_for_recipient_widget() {
 
     const direct_messages_option = {
         is_direct_message: true,
-        unique_id: DIRECT_MESSAGE_ID,
+        unique_id: compose_state.DIRECT_MESSAGE_ID,
         name: $t({defaultMessage: "Direct message"}),
     };
 

web/src/compose_state.js

@@ -1,7 +1,6 @@
 import $ from "jquery";
 
 import * as compose_pm_pill from "./compose_pm_pill";
-import * as compose_recipient from "./compose_recipient";
 import {$t} from "./i18n";
 import * as sub_store from "./sub_store";
 
@@ -69,10 +68,19 @@ function get_or_set(fieldname, keep_leading_whitespace, no_trim) {
     };
 }
 
-// NOTE: See `selected_recipient_id` in compose_recipient to for
+// selected_recipient_id is the current state for the stream picker widget:
-// documentation on the variable and how it is used.
+// "" -> stream message but no stream is selected
+// integer -> stream id of the selected stream.
+// "direct" -> Direct message is selected.
+export let selected_recipient_id = "";
+export const DIRECT_MESSAGE_ID = "direct";
+
+export function set_selected_recipient_id(recipient_id) {
+    selected_recipient_id = recipient_id;
+}
+
 export function stream_id() {
-    const stream_id = compose_recipient.selected_recipient_id;
+    const stream_id = selected_recipient_id;
     if (typeof stream_id === "number") {
         return stream_id;
     }
@@ -80,7 +88,7 @@ export function stream_id() {
 }
 
 export function stream_name() {
-    const stream_id = compose_recipient.selected_recipient_id;
+    const stream_id = selected_recipient_id;
     if (typeof stream_id === "number") {
         return sub_store.maybe_get_stream_name(stream_id) || "";
     }
@@ -88,14 +96,14 @@ export function stream_name() {
 }
 
 export function set_stream_id(stream_id) {
-    compose_recipient.set_selected_recipient_id(stream_id);
+    set_selected_recipient_id(stream_id);
 }
 
 export function set_compose_recipient_id(recipient_id) {
     if (typeof recipient_id !== "number") {
-        recipient_id = compose_recipient.DIRECT_MESSAGE_ID;
+        recipient_id = DIRECT_MESSAGE_ID;
     }
-    compose_recipient.set_selected_recipient_id(recipient_id);
+    set_selected_recipient_id(recipient_id);
 }
 
 // TODO: Break out setter and getter into their own functions.

web/src/server_events_dispatch.js

@@ -12,6 +12,7 @@ import * as compose from "./compose";
 import * as compose_call from "./compose_call";
 import * as compose_pm_pill from "./compose_pm_pill";
 import * as compose_recipient from "./compose_recipient";
+import * as compose_state from "./compose_state";
 import * as composebox_typeahead from "./composebox_typeahead";
 import * as dark_theme from "./dark_theme";
 import * as emoji from "./emoji";
@@ -552,8 +553,9 @@ export function dispatch_normal_event(event) {
                         stream_settings_ui.remove_stream(stream.stream_id);
                         if (was_subscribed) {
                             stream_list.remove_sidebar_row(stream.stream_id);
-                            if (stream.stream_id === compose_recipient.selected_recipient_id) {
+                            if (stream.stream_id === compose_state.selected_recipient_id) {
-                                compose_recipient.set_selected_recipient_id("");
+                                compose_state.set_selected_recipient_id("");
+                                compose_recipient.on_compose_select_recipient_update();
                             }
                         }
                         settings_streams.update_default_streams_table();

web/tests/compose.test.js

@@ -303,7 +303,6 @@ test_ui("enter_with_preview_open", ({override, override_rewire}) => {
     mock_banners();
     $("#compose-textarea").toggleClass = noop;
     mock_stream_header_colorblock();
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", noop);
     override_rewire(compose_banner, "clear_message_sent_banners", () => {});
     override(document, "to_$", () => $("document-stub"));
     let show_button_spinner_called = false;
@@ -725,7 +724,6 @@ test_ui("on_events", ({override, override_rewire}) => {
 test_ui("create_message_object", ({override, override_rewire}) => {
     mock_stream_header_colorblock();
     mock_banners();
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", noop);
 
     compose_state.set_stream_id(social.stream_id);
     $("#stream_message_recipient_topic").val("lunch");

web/tests/compose_fade.test.js

@@ -55,7 +55,6 @@ run_test("set_focused_recipient", ({override_rewire}) => {
     override_rewire(compose_recipient, "selected_stream_name", "social");
     override_rewire(compose_recipient, "is_direct_message_selected", false);
 
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     const sub = {
         stream_id: 101,
         name: "social",

web/tests/compose_state.test.js

@@ -10,7 +10,6 @@ const $ = require("./lib/zjquery");
 const compose_pm_pill = mock_esm("../src/compose_pm_pill");
 
 const compose_state = zrequire("compose_state");
-const compose_recipient = zrequire("compose_recipient");
 const stream_data = zrequire("stream_data");
 
 const noop = () => {};
@@ -27,11 +26,10 @@ run_test("private_message_recipient", ({override}) => {
     assert.equal(compose_state.private_message_recipient(), "fred@fred.org");
 });
 
-run_test("has_full_recipient", ({override, override_rewire}) => {
+run_test("has_full_recipient", ({override}) => {
     mock_stream_header_colorblock();
     $(`#compose_banners .topic_resolved`).remove = noop;
     $(".narrow_to_compose_recipients").toggleClass = noop;
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
 
     let emails;
     override(compose_pm_pill, "set_from_emails", (value) => {

web/tests/compose_validate.test.js

@@ -152,9 +152,7 @@ test_ui("validate_stream_message_address_info", ({mock_template}) => {
     assert.ok(subscription_error_rendered);
 });
 
-test_ui("validate", ({override_rewire, mock_template}) => {
+test_ui("validate", ({mock_template}) => {
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
-
     function initialize_pm_pill() {
         $.clear_all_elements();
 
@@ -384,7 +382,6 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
     // we are separating it up in different test. Though their relative position
     // of execution should not be changed.
     mock_banners();
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     page_params.user_id = me.user_id;
     page_params.realm_mandatory_topics = false;
 
@@ -433,129 +430,117 @@ test_ui("validate_stream_message", ({override_rewire, mock_template}) => {
     assert.ok(wildcards_not_allowed_rendered);
 });
 
-test_ui(
+test_ui("test_validate_stream_message_post_policy_admin_only", ({mock_template}) => {
-    "test_validate_stream_message_post_policy_admin_only",
+    // This test is in continuation with test_validate but it has been separated out
-    ({override_rewire, mock_template}) => {
+    // for better readability. Their relative position of execution should not be changed.
-        // This test is in continuation with test_validate but it has been separated out
+    // Although the position with respect to test_validate_stream_message does not matter
-        // for better readability. Their relative position of execution should not be changed.
+    // as different stream is used for this test.
-        // Although the position with respect to test_validate_stream_message does not matter
+    mock_banners();
-        // as different stream is used for this test.
+    page_params.is_admin = false;
-        mock_banners();
+    const sub_stream_102 = {
-        override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
+        stream_id: 102,
-        page_params.is_admin = false;
+        name: "stream102",
-        const sub_stream_102 = {
+        subscribed: true,
-            stream_id: 102,
+        stream_post_policy: settings_config.stream_post_policy_values.admins.code,
-            name: "stream102",
+    };
-            subscribed: true,
-            stream_post_policy: settings_config.stream_post_policy_values.admins.code,
-        };
 
-        stream_data.add_sub(sub_stream_102);
+    stream_data.add_sub(sub_stream_102);
-        compose_state.topic("topic102");
+    compose_state.topic("topic102");
-        compose_state.set_stream_id(sub_stream_102.stream_id);
+    compose_state.set_stream_id(sub_stream_102.stream_id);
 
-        let banner_rendered = false;
+    let banner_rendered = false;
-        mock_template("compose_banner/compose_banner.hbs", false, (data) => {
+    mock_template("compose_banner/compose_banner.hbs", false, (data) => {
-            assert.equal(data.classname, compose_banner.CLASSNAMES.no_post_permissions);
+        assert.equal(data.classname, compose_banner.CLASSNAMES.no_post_permissions);
-            assert.equal(
+        assert.equal(
-                data.banner_text,
+            data.banner_text,
-                $t({
+            $t({
-                    defaultMessage: "You do not have permission to post in this stream.",
+                defaultMessage: "You do not have permission to post in this stream.",
-                }),
+            }),
-            );
+        );
-            banner_rendered = true;
+        banner_rendered = true;
-        });
+    });
-        assert.ok(!compose_validate.validate());
+    assert.ok(!compose_validate.validate());
-        assert.ok(banner_rendered);
+    assert.ok(banner_rendered);
 
-        // Reset error message.
+    // Reset error message.
-        compose_state.set_stream_id(social_sub.stream_id);
+    compose_state.set_stream_id(social_sub.stream_id);
 
-        page_params.is_admin = false;
+    page_params.is_admin = false;
-        page_params.is_guest = true;
+    page_params.is_guest = true;
 
-        compose_state.topic("topic102");
+    compose_state.topic("topic102");
-        compose_state.set_stream_id(sub_stream_102.stream_id);
+    compose_state.set_stream_id(sub_stream_102.stream_id);
-        banner_rendered = false;
+    banner_rendered = false;
-        assert.ok(!compose_validate.validate());
+    assert.ok(!compose_validate.validate());
-        assert.ok(banner_rendered);
+    assert.ok(banner_rendered);
-    },
+});
-);
 
-test_ui(
+test_ui("test_validate_stream_message_post_policy_moderators_only", ({mock_template}) => {
-    "test_validate_stream_message_post_policy_moderators_only",
+    mock_banners();
-    ({override_rewire, mock_template}) => {
-        mock_banners();
-        override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
 
-        page_params.is_admin = false;
+    page_params.is_admin = false;
-        page_params.is_moderator = false;
+    page_params.is_moderator = false;
-        page_params.is_guest = false;
+    page_params.is_guest = false;
 
-        const sub = {
+    const sub = {
-            stream_id: 104,
+        stream_id: 104,
-            name: "stream104",
+        name: "stream104",
-            subscribed: true,
+        subscribed: true,
-            stream_post_policy: settings_config.stream_post_policy_values.moderators.code,
+        stream_post_policy: settings_config.stream_post_policy_values.moderators.code,
-        };
+    };
 
-        stream_data.add_sub(sub);
+    stream_data.add_sub(sub);
-        compose_state.topic("topic104");
+    compose_state.topic("topic104");
-        compose_state.set_stream_id(sub.stream_id);
+    compose_state.set_stream_id(sub.stream_id);
-        let banner_rendered = false;
+    let banner_rendered = false;
-        mock_template("compose_banner/compose_banner.hbs", false, (data) => {
+    mock_template("compose_banner/compose_banner.hbs", false, (data) => {
-            assert.equal(data.classname, compose_banner.CLASSNAMES.no_post_permissions);
+        assert.equal(data.classname, compose_banner.CLASSNAMES.no_post_permissions);
-            assert.equal(
+        assert.equal(
-                data.banner_text,
+            data.banner_text,
-                $t({
+            $t({
-                    defaultMessage: "You do not have permission to post in this stream.",
+                defaultMessage: "You do not have permission to post in this stream.",
-                }),
+            }),
-            );
+        );
-            banner_rendered = true;
+        banner_rendered = true;
-        });
+    });
-        assert.ok(!compose_validate.validate());
+    assert.ok(!compose_validate.validate());
-        assert.ok(banner_rendered);
+    assert.ok(banner_rendered);
-        // Reset error message.
+    // Reset error message.
-        compose_state.set_stream_id(social_sub.stream_id);
+    compose_state.set_stream_id(social_sub.stream_id);
 
-        page_params.is_guest = true;
+    page_params.is_guest = true;
-        assert.ok(!compose_validate.validate());
+    assert.ok(!compose_validate.validate());
-        assert.ok(banner_rendered);
+    assert.ok(banner_rendered);
-    },
+});
-);
 
-test_ui(
+test_ui("test_validate_stream_message_post_policy_full_members_only", ({mock_template}) => {
-    "test_validate_stream_message_post_policy_full_members_only",
+    mock_banners();
-    ({override_rewire, mock_template}) => {
+    page_params.is_admin = false;
-        mock_banners();
+    page_params.is_guest = true;
-        override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
+    const sub = {
-        page_params.is_admin = false;
+        stream_id: 103,
-        page_params.is_guest = true;
+        name: "stream103",
-        const sub = {
+        subscribed: true,
-            stream_id: 103,
+        stream_post_policy: settings_config.stream_post_policy_values.non_new_members.code,
-            name: "stream103",
+    };
-            subscribed: true,
-            stream_post_policy: settings_config.stream_post_policy_values.non_new_members.code,
-        };
 
-        stream_data.add_sub(sub);
+    stream_data.add_sub(sub);
-        compose_state.topic("topic103");
+    compose_state.topic("topic103");
-        compose_state.set_stream_id(sub.stream_id);
+    compose_state.set_stream_id(sub.stream_id);
-        let banner_rendered = false;
+    let banner_rendered = false;
-        mock_template("compose_banner/compose_banner.hbs", false, (data) => {
+    mock_template("compose_banner/compose_banner.hbs", false, (data) => {
-            assert.equal(data.classname, compose_banner.CLASSNAMES.no_post_permissions);
+        assert.equal(data.classname, compose_banner.CLASSNAMES.no_post_permissions);
-            assert.equal(
+        assert.equal(
-                data.banner_text,
+            data.banner_text,
-                $t({
+            $t({
-                    defaultMessage: "You do not have permission to post in this stream.",
+                defaultMessage: "You do not have permission to post in this stream.",
-                }),
+            }),
-            );
+        );
-            banner_rendered = true;
+        banner_rendered = true;
-        });
+    });
-        assert.ok(!compose_validate.validate());
+    assert.ok(!compose_validate.validate());
-        assert.ok(banner_rendered);
+    assert.ok(banner_rendered);
-    },
+});
-);
 
 test_ui("test_check_overflow_text", ({mock_template}) => {
     mock_banners();
@@ -639,9 +624,7 @@ test_ui("needs_subscribe_warning", () => {
     assert.equal(compose_validate.needs_subscribe_warning(bob.user_id, sub.stream_id), true);
 });
 
-test_ui("warn_if_private_stream_is_linked", ({mock_template, override_rewire}) => {
+test_ui("warn_if_private_stream_is_linked", ({mock_template}) => {
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
-
     const $textarea = $("<textarea>").attr("id", "compose-textarea");
     stub_message_row($textarea);
     const test_sub = {
@@ -686,7 +669,7 @@ test_ui("warn_if_private_stream_is_linked", ({mock_template, override_rewire}) =
 
     // Not everyone is subscribed to secret_stream in denmark, so the
     // warning is rendered.
-    compose_recipient.set_selected_recipient_id(denmark.stream_id);
+    compose_state.set_selected_recipient_id(denmark.stream_id);
     const secret_stream = {
         invite_only: true,
         name: "Denmark",
@@ -698,8 +681,7 @@ test_ui("warn_if_private_stream_is_linked", ({mock_template, override_rewire}) =
     assert.ok(banner_rendered);
 });
 
-test_ui("warn_if_mentioning_unsubscribed_user", ({override, override_rewire, mock_template}) => {
+test_ui("warn_if_mentioning_unsubscribed_user", ({override, mock_template}) => {
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     const $textarea = $("<textarea>").attr("id", "compose-textarea");
     stub_message_row($textarea);
     compose_state.set_stream_id("");
@@ -787,9 +769,8 @@ test_ui("warn_if_mentioning_unsubscribed_user", ({override, override_rewire, moc
     assert.ok(!new_banner_rendered);
 });
 
-test_ui("test warn_if_topic_resolved", ({override, override_rewire, mock_template}) => {
+test_ui("test warn_if_topic_resolved", ({override, mock_template}) => {
     mock_banners();
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     $("#compose_banners .topic_resolved").length = 0;
     override(settings_data, "user_can_move_messages_to_another_topic", () => true);
 

web/tests/composebox_typeahead.test.js

@@ -704,7 +704,6 @@ const sweden_topics_to_show = ["<&>", "even more ice", "furniture", "ice", "kron
 test("initialize", ({override, override_rewire, mock_template}) => {
     mock_stream_header_colorblock();
     mock_banners();
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", noop);
 
     let pill_items = [];
     let cleared = false;
@@ -1862,7 +1861,6 @@ test("direct message recipients sorted according to stream / topic being viewed"
     );
     mock_stream_header_colorblock();
     mock_banners();
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
 
     // When viewing no stream, sorting is alphabetical
     compose_state.set_stream_id("");

web/tests/drafts.test.js

@@ -155,7 +155,6 @@ test("draft_model delete", ({override}) => {
 test("snapshot_message", ({override_rewire}) => {
     override_rewire(user_pill, "get_user_ids", () => [aaron.user_id]);
     override_rewire(compose_pm_pill, "set_from_emails", noop);
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     mock_banners();
 
     $(".narrow_to_compose_recipients").toggleClass = noop;

web/tests/narrow.test.js

@@ -17,7 +17,6 @@ const stream_data = zrequire("stream_data");
 const {Filter} = zrequire("../src/filter");
 const narrow = zrequire("narrow");
 const settings_config = zrequire("settings_config");
-const compose_recipient = zrequire("compose_recipient");
 
 const compose_pm_pill = mock_esm("../src/compose_pm_pill");
 mock_esm("../src/spectators", {
@@ -652,8 +651,7 @@ run_test("show_invalid_narrow_message", ({mock_template}) => {
     );
 });
 
-run_test("narrow_to_compose_target errors", ({override_rewire, disallow_rewire}) => {
+run_test("narrow_to_compose_target errors", ({disallow_rewire}) => {
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     disallow_rewire(narrow, "activate");
 
     // No-op when not composing.
@@ -667,7 +665,6 @@ run_test("narrow_to_compose_target errors", ({override_rewire, disallow_rewire})
 });
 
 run_test("narrow_to_compose_target streams", ({override_rewire}) => {
-    override_rewire(compose_recipient, "on_compose_select_recipient_update", () => {});
     const args = {called: false};
     override_rewire(narrow, "activate", (operators, opts) => {
         args.operators = operators;