From 7e52509ee7f3a10375b1449e1dfb4f26ea0e5e2c Mon Sep 17 00:00:00 2001
From: Lalit <lalitkumarsingh3716@gmail.com>
Date: Tue, 25 Apr 2023 09:06:10 +0530
Subject: [PATCH] ui: Move update_message_lists related functions to other
 related modules.

This commit moves mainly two functions from `ui.js` to `message_live_update`,
`update_message_in_all_views` and `update_starred_view`. This is done in favor of
eliminating `ui.js` and also these functions are more closely related to
`message_live_update` module than to `ui` module.

We also move `show_message_failed` and `show_failed_message_success` to `echo.js`
for cleaner seperation of responsibilities.
---
 web/src/echo.js                 | 22 ++++++++++++--
 web/src/message_flags.js        |  6 ++--
 web/src/message_live_update.js  | 34 +++++++++++++++++++++
 web/src/ui.js                   | 52 ---------------------------------
 web/tests/dispatch.test.js      | 19 +++---------
 web/tests/echo.test.js          |  4 +--
 web/tests/message_flags.test.js |  6 ++--
 7 files changed, 65 insertions(+), 78 deletions(-)

diff --git a/web/src/echo.js b/web/src/echo.js
index 53c24c137c..c1fd24efe9 100644
--- a/web/src/echo.js
+++ b/web/src/echo.js
@@ -10,6 +10,7 @@ import * as local_message from "./local_message";
 import * as markdown from "./markdown";
 import * as message_events from "./message_events";
 import * as message_lists from "./message_lists";
+import * as message_live_update from "./message_live_update";
 import * as message_store from "./message_store";
 import * as narrow_state from "./narrow_state";
 import * as notifications from "./notifications";
@@ -23,7 +24,6 @@ import * as sent_messages from "./sent_messages";
 import * as stream_list from "./stream_list";
 import * as stream_topic_history from "./stream_topic_history";
 import * as transmit from "./transmit";
-import * as ui from "./ui";
 import * as util from "./util";
 
 // Docs: https://zulip.readthedocs.io/en/latest/subsystems/sending-messages.html
@@ -61,9 +61,25 @@ function insert_message(message) {
     message_events.insert_new_messages([message], true);
 }
 
+function show_message_failed(message_id, failed_msg) {
+    // Failed to send message, so display inline retry/cancel
+    message_live_update.update_message_in_all_views(message_id, ($row) => {
+        const $failed_div = $row.find(".message_failed");
+        $failed_div.toggleClass("hide", false);
+        $failed_div.find(".failed_text").attr("title", failed_msg);
+    });
+}
+
+function show_failed_message_success(message_id) {
+    // Previously failed message succeeded
+    message_live_update.update_message_in_all_views(message_id, ($row) => {
+        $row.find(".message_failed").toggleClass("hide", true);
+    });
+}
+
 function failed_message_success(message_id) {
     message_store.get(message_id).failed_request = false;
-    ui.show_failed_message_success(message_id);
+    show_failed_message_success(message_id);
 }
 
 function resend_message(message, $row) {
@@ -442,7 +458,7 @@ export function _patch_waiting_for_ack(data) {
 export function message_send_error(message_id, error_response) {
     // Error sending message, show inline
     message_store.get(message_id).failed_request = true;
-    ui.show_message_failed(message_id, error_response);
+    show_message_failed(message_id, error_response);
 }
 
 function abort_message(message) {
diff --git a/web/src/message_flags.js b/web/src/message_flags.js
index 5b7fec0625..d34e5e9494 100644
--- a/web/src/message_flags.js
+++ b/web/src/message_flags.js
@@ -1,9 +1,9 @@
 import _ from "lodash";
 
 import * as channel from "./channel";
+import * as message_live_update from "./message_live_update";
 import * as message_store from "./message_store";
 import * as starred_messages from "./starred_messages";
-import * as ui from "./ui";
 import * as unread_ops from "./unread_ops";
 
 function send_flag_update_for_messages(msg_ids, flag, op) {
@@ -86,7 +86,7 @@ export function update_starred_flag(message_id, new_value) {
         return;
     }
     message.starred = new_value;
-    ui.update_starred_view(message_id, new_value);
+    message_live_update.update_starred_view(message_id, new_value);
 }
 
 export function toggle_starred_and_update_server(message) {
@@ -105,7 +105,7 @@ export function toggle_starred_and_update_server(message) {
     // explicit interaction and we'd like to preserve the user
     // expectation invariant that all starred messages are read.
     unread_ops.notify_server_message_read(message);
-    ui.update_starred_view(message.id, message.starred);
+    message_live_update.update_starred_view(message.id, message.starred);
 
     if (message.starred) {
         send_flag_update_for_messages([message.id], "starred", "add");
diff --git a/web/src/message_live_update.js b/web/src/message_live_update.js
index 54baebe00d..d5033dd69a 100644
--- a/web/src/message_live_update.js
+++ b/web/src/message_live_update.js
@@ -31,6 +31,40 @@ function rerender_messages_view_for_user(user_id) {
     }
 }
 
+export function update_message_in_all_views(message_id, callback) {
+    for (const msg_list of message_lists.all_rendered_message_lists()) {
+        const $row = msg_list.get_row(message_id);
+        if ($row === undefined) {
+            // The row may not exist, e.g. if you do an action on a message in
+            // a narrowed view
+            continue;
+        }
+        callback($row);
+    }
+}
+
+export function update_starred_view(message_id, new_value) {
+    const starred = new_value;
+
+    // Avoid a full re-render, but update the star in each message
+    // table in which it is visible.
+    update_message_in_all_views(message_id, ($row) => {
+        const $elt = $row.find(".star");
+        const $star_container = $row.find(".star_container");
+        if (starred) {
+            $elt.addClass("fa-star").removeClass("fa-star-o");
+            $star_container.removeClass("empty-star");
+        } else {
+            $elt.removeClass("fa-star").addClass("fa-star-o");
+            $star_container.addClass("empty-star");
+        }
+        const data_template_id = starred
+            ? "unstar-message-tooltip-template"
+            : "star-message-tooltip-template";
+        $star_container.attr("data-tooltip-template-id", data_template_id);
+    });
+}
+
 export function update_stream_name(stream_id, new_name) {
     message_store.update_property("stream_name", new_name, {stream_id});
     rerender_messages_view();
diff --git a/web/src/ui.js b/web/src/ui.js
index ae7d8cec30..c7620cc427 100644
--- a/web/src/ui.js
+++ b/web/src/ui.js
@@ -1,8 +1,6 @@
 import $ from "jquery";
 import SimpleBar from "simplebar";
 
-import * as message_lists from "./message_lists";
-
 // What, if anything, obscures the home tab?
 
 export function replace_emoji_with_text($element) {
@@ -46,56 +44,6 @@ export function reset_scrollbar($element) {
     }
 }
 
-function update_message_in_all_views(message_id, callback) {
-    for (const msg_list of message_lists.all_rendered_message_lists()) {
-        const $row = msg_list.get_row(message_id);
-        if ($row === undefined) {
-            // The row may not exist, e.g. if you do an action on a message in
-            // a narrowed view
-            continue;
-        }
-        callback($row);
-    }
-}
-
-export function update_starred_view(message_id, new_value) {
-    const starred = new_value;
-
-    // Avoid a full re-render, but update the star in each message
-    // table in which it is visible.
-    update_message_in_all_views(message_id, ($row) => {
-        const $elt = $row.find(".star");
-        const $star_container = $row.find(".star_container");
-        if (starred) {
-            $elt.addClass("fa-star").removeClass("fa-star-o");
-            $star_container.removeClass("empty-star");
-        } else {
-            $elt.removeClass("fa-star").addClass("fa-star-o");
-            $star_container.addClass("empty-star");
-        }
-        const data_template_id = starred
-            ? "unstar-message-tooltip-template"
-            : "star-message-tooltip-template";
-        $star_container.attr("data-tooltip-template-id", data_template_id);
-    });
-}
-
-export function show_message_failed(message_id, failed_msg) {
-    // Failed to send message, so display inline retry/cancel
-    update_message_in_all_views(message_id, ($row) => {
-        const $failed_div = $row.find(".message_failed");
-        $failed_div.toggleClass("hide", false);
-        $failed_div.find(".failed_text").attr("title", failed_msg);
-    });
-}
-
-export function show_failed_message_success(message_id) {
-    // Previously failed message succeeded
-    update_message_in_all_views(message_id, ($row) => {
-        $row.find(".message_failed").toggleClass("hide", true);
-    });
-}
-
 // Save the compose content cursor position and restore when we
 // shift-tab back in (see hotkey.js).
 let saved_compose_cursor = 0;
diff --git a/web/tests/dispatch.test.js b/web/tests/dispatch.test.js
index 9c98a7760a..2366ce5c54 100644
--- a/web/tests/dispatch.test.js
+++ b/web/tests/dispatch.test.js
@@ -70,7 +70,6 @@ mock_esm("../src/top_left_corner", {
     update_starred_count() {},
 });
 const typing_events = mock_esm("../src/typing_events");
-const ui = mock_esm("../src/ui");
 const unread_ops = mock_esm("../src/unread_ops");
 const unread_ui = mock_esm("../src/unread_ui");
 const user_events = mock_esm("../src/user_events");
@@ -84,12 +83,14 @@ const electron_bridge = set_global("electron_bridge", {});
 
 message_lists.update_recipient_bar_background_color = noop;
 message_lists.current = {
+    get_row: noop,
     rerender_view: noop,
     data: {
         get_messages_sent_by_user: () => [],
     },
 };
 message_lists.home = {
+    get_row: noop,
     rerender_view: noop,
     data: {
         get_messages_sent_by_user: () => [],
@@ -996,28 +997,16 @@ run_test("update_message (unread)", ({override}) => {
     });
 });
 
-run_test("update_message (add star)", ({override}) => {
+run_test("update_message (add star)", () => {
     const event = event_fixtures.update_message_flags__starred_add;
-    const stub = make_stub();
-    override(ui, "update_starred_view", stub.f);
     dispatch(event);
-    assert.equal(stub.num_calls, 1);
-    const args = stub.get_args("message_id", "new_value");
-    assert_same(args.message_id, test_message.id);
-    assert_same(args.new_value, true); // for 'add'
     const msg = message_store.get(test_message.id);
     assert.equal(msg.starred, true);
 });
 
-run_test("update_message (remove star)", ({override}) => {
+run_test("update_message (remove star)", () => {
     const event = event_fixtures.update_message_flags__starred_remove;
-    const stub = make_stub();
-    override(ui, "update_starred_view", stub.f);
     dispatch(event);
-    assert.equal(stub.num_calls, 1);
-    const args = stub.get_args("message_id", "new_value");
-    assert_same(args.message_id, test_message.id);
-    assert_same(args.new_value, false);
     const msg = message_store.get(test_message.id);
     assert.equal(msg.starred, false);
 });
diff --git a/web/tests/echo.test.js b/web/tests/echo.test.js
index a033b048b7..5e99c558fd 100644
--- a/web/tests/echo.test.js
+++ b/web/tests/echo.test.js
@@ -15,8 +15,8 @@ const notifications = mock_esm("../src/notifications");
 
 let disparities = [];
 
-mock_esm("../src/ui", {
-    show_failed_message_success() {},
+mock_esm("../src/message_live_update", {
+    update_message_in_all_views() {},
 });
 
 mock_esm("../src/sent_messages", {
diff --git a/web/tests/message_flags.test.js b/web/tests/message_flags.test.js
index cc6ea80e74..9f9f2f1778 100644
--- a/web/tests/message_flags.test.js
+++ b/web/tests/message_flags.test.js
@@ -6,7 +6,7 @@ const {mock_esm, set_global, with_overrides, zrequire} = require("./lib/namespac
 const {run_test} = require("./lib/test");
 
 const channel = mock_esm("../src/channel");
-const ui = mock_esm("../src/ui");
+const message_live_update = mock_esm("../src/message_live_update");
 
 mock_esm("../src/starred_messages", {
     add() {},
@@ -22,7 +22,7 @@ run_test("starred", ({override}) => {
     };
     let ui_updated;
 
-    override(ui, "update_starred_view", () => {
+    override(message_live_update, "update_starred_view", () => {
         ui_updated = true;
     });
 
@@ -76,7 +76,7 @@ run_test("starring local echo", () => {
 
     message_flags.toggle_starred_and_update_server(locally_echoed_message);
 
-    // ui.update_starred_view not called
+    // message_live_update.update_starred_view not called
 
     // channel post request not made