From 9d271190a2bd1903919e0ee20dcd41c06582a954 Mon Sep 17 00:00:00 2001 From: sbansal1999 Date: Sat, 8 Apr 2023 12:12:14 +0530 Subject: [PATCH] js: Break cyclic dependecny between unread_ops and recent_topics_ui. This commits breaks the cyclic dependency between "unread_ops.js" and "recent_topics_ui.js" by passing the "update_topic" function through the function calls in "server_events_dispatch.js", hence avoiding the need of importing "recent_topics_ui.js" --- web/src/compose_actions.js | 6 +++++- web/src/message_events.js | 4 ++-- web/src/message_flags.js | 3 ++- web/src/message_scroll.js | 9 +++++++-- web/src/narrow.js | 6 +++--- web/src/notifications.js | 3 ++- web/src/server_events_dispatch.js | 22 +++++++++++++++------ web/src/unread_ops.js | 33 +++++++++++++++++-------------- web/tests/example7.test.js | 5 +++-- 9 files changed, 58 insertions(+), 33 deletions(-) diff --git a/web/src/compose_actions.js b/web/src/compose_actions.js index 03997dc6e3776..ababf89a45c6a 100644 --- a/web/src/compose_actions.js +++ b/web/src/compose_actions.js @@ -436,7 +436,11 @@ export function respond_to_message(opts) { } if (message_lists.current.can_mark_messages_read()) { - unread_ops.notify_server_message_read(message); + unread_ops.notify_server_message_read( + message, + {}, + recent_topics_ui.update_topic_unread_count, + ); } } diff --git a/web/src/message_events.js b/web/src/message_events.js index 959a9777943c1..99eff77fafeee 100644 --- a/web/src/message_events.js +++ b/web/src/message_events.js @@ -77,7 +77,7 @@ function maybe_add_narrowed_messages(messages, msg_list, callback, attempt = 1) ); callback(new_messages, msg_list); - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); notifications.notify_messages_outside_current_search(elsewhere_messages); }, error(xhr) { @@ -159,7 +159,7 @@ export function insert_new_messages(messages, sent_by_this_client) { unread_ui.update_unread_counts(); } - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); notifications.received_messages(messages); stream_list.update_streams_sidebar(); pm_list.update_private_messages(); diff --git a/web/src/message_flags.js b/web/src/message_flags.js index 5b7fec0625656..c8f2913691792 100644 --- a/web/src/message_flags.js +++ b/web/src/message_flags.js @@ -2,6 +2,7 @@ import _ from "lodash"; import * as channel from "./channel"; import * as message_store from "./message_store"; +import * as recent_topics_ui from "./recent_topics_ui"; import * as starred_messages from "./starred_messages"; import * as ui from "./ui"; import * as unread_ops from "./unread_ops"; @@ -104,7 +105,7 @@ export function toggle_starred_and_update_server(message) { // msg_list.can_mark_messages_read, because starring a message is an // 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); + unread_ops.notify_server_message_read(message, {}, recent_topics_ui.update_topic_unread_count); ui.update_starred_view(message.id, message.starred); if (message.starred) { diff --git a/web/src/message_scroll.js b/web/src/message_scroll.js index ad9643e90c1df..579a16d62d230 100644 --- a/web/src/message_scroll.js +++ b/web/src/message_scroll.js @@ -9,6 +9,7 @@ import * as message_lists from "./message_lists"; import * as message_viewport from "./message_viewport"; import * as narrow_banner from "./narrow_banner"; import * as narrow_state from "./narrow_state"; +import * as recent_topics_ui from "./recent_topics_ui"; import * as recent_topics_util from "./recent_topics_util"; import * as unread from "./unread"; import * as unread_ops from "./unread_ops"; @@ -246,7 +247,7 @@ export function initialize() { message_viewport.$message_pane.on( "scroll", _.throttle(() => { - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); scroll_finish(); }, 50), ); @@ -266,7 +267,11 @@ export function initialize() { messages = event.msg_list.message_range(event.previously_selected_id, event.id); } if (event.msg_list.can_mark_messages_read()) { - unread_ops.notify_server_messages_read(messages, {from: "pointer"}); + unread_ops.notify_server_messages_read( + messages, + {from: "pointer"}, + recent_topics_ui.update_topic_unread_count, + ); } else if ( unread.get_unread_messages(messages).length !== 0 && // The below checks might seem redundant, but it's diff --git a/web/src/narrow.js b/web/src/narrow.js index a282e53807430..a3cb2e4d891f3 100644 --- a/web/src/narrow.js +++ b/web/src/narrow.js @@ -820,7 +820,7 @@ export function update_selection(opts) { // narrowing message_lists.current.view.set_message_offset(select_offset); } - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); } export function activate_stream_for_cycle_hotkey(stream_name) { @@ -923,7 +923,7 @@ export function by_topic(target_id, opts) { // We don't check msg_list.can_mark_messages_read here only because // the target msg_list isn't initialized yet; in any case, the // message is about to be marked read in the new view. - unread_ops.notify_server_message_read(original); + unread_ops.notify_server_message_read(original, {}, recent_topics_ui.update_topic_unread_count); const search_terms = [ {operator: "stream", operand: original.stream}, @@ -942,7 +942,7 @@ export function by_recipient(target_id, opts) { // We don't check msg_list.can_mark_messages_read here only because // the target msg_list isn't initialized yet; in any case, the // message is about to be marked read in the new view. - unread_ops.notify_server_message_read(message); + unread_ops.notify_server_message_read(message, {}, recent_topics_ui.update_topic_unread_count); switch (message.type) { case "private": diff --git a/web/src/notifications.js b/web/src/notifications.js index fd22e0e4c7dc4..3c734def9e046 100644 --- a/web/src/notifications.js +++ b/web/src/notifications.js @@ -17,6 +17,7 @@ import * as navigate from "./navigate"; import {page_params} from "./page_params"; import * as people from "./people"; import {realm_user_settings_defaults} from "./realm_user_settings_defaults"; +import * as recent_topics_ui from "./recent_topics_ui"; import * as settings_config from "./settings_config"; import * as spoilers from "./spoilers"; import * as stream_data from "./stream_data"; @@ -85,7 +86,7 @@ export function initialize() { // Update many places on the DOM to reflect unread // counts. - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); }) .on("blur", () => { window_focused = false; diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index 79cc68fa83930..56fe62d1cf4d1 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -38,6 +38,7 @@ import * as realm_icon from "./realm_icon"; import * as realm_logo from "./realm_logo"; import * as realm_playground from "./realm_playground"; import {realm_user_settings_defaults} from "./realm_user_settings_defaults"; +import * as recent_topics_ui from "./recent_topics_ui"; import * as reload from "./reload"; import * as scroll_bar from "./scroll_bar"; import * as settings_account from "./settings_account"; @@ -102,7 +103,10 @@ export function dispatch_normal_event(event) { // message is passed to unread.get_unread_messages, // which returns all the unread messages out of a given list. // So double marking something as read would not occur - unread_ops.process_read_messages_event(msg_ids); + unread_ops.process_read_messages_event( + msg_ids, + recent_topics_ui.update_topic_unread_count, + ); // This methods updates message_list too and since stream_topic_history relies on it // this method should be called first. message_events.remove_messages(msg_ids); @@ -758,12 +762,18 @@ export function dispatch_normal_event(event) { break; case "read": if (event.op === "add") { - unread_ops.process_read_messages_event(event.messages); + unread_ops.process_read_messages_event( + event.messages, + recent_topics_ui.update_topic_unread_count, + ); } else { - unread_ops.process_unread_messages_event({ - message_ids: event.messages, - message_details: event.message_details, - }); + unread_ops.process_unread_messages_event( + { + message_ids: event.messages, + message_details: event.message_details, + }, + recent_topics_ui.complete_rerender, + ); } break; } diff --git a/web/src/unread_ops.js b/web/src/unread_ops.js index b0949335e9229..659755a05d1ef 100644 --- a/web/src/unread_ops.js +++ b/web/src/unread_ops.js @@ -16,7 +16,6 @@ import * as message_viewport from "./message_viewport"; import * as narrow_state from "./narrow_state"; import * as notifications from "./notifications"; import * as people from "./people"; -import * as recent_topics_ui from "./recent_topics_ui"; import * as ui_report from "./ui_report"; import * as unread from "./unread"; import * as unread_ui from "./unread_ui"; @@ -148,12 +147,12 @@ export function mark_all_as_read(args = {}) { }); } -function process_newly_read_message(message, options) { +function process_newly_read_message(message, options, update_topic_unread_count) { for (const msg_list of message_lists.all_rendered_message_lists()) { msg_list.show_message_as_read(message, options); } notifications.close_notification(message); - recent_topics_ui.update_topic_unread_count(message); + update_topic_unread_count(message); } export function mark_as_unread_from_here( @@ -257,7 +256,7 @@ export function resume_reading() { message_lists.current.resume_reading(); } -export function process_read_messages_event(message_ids) { +export function process_read_messages_event(message_ids, update_topic_unread_count) { /* This code has a lot in common with notify_server_messages_read, but there are subtle differences due to the fact that the @@ -287,14 +286,14 @@ export function process_read_messages_event(message_ids) { // recent conversations per message, not a single global // rerender or one per conversation. if (message) { - process_newly_read_message(message, options); + process_newly_read_message(message, options, update_topic_unread_count); } } unread_ui.update_unread_counts(); } -export function process_unread_messages_event({message_ids, message_details}) { +export function process_unread_messages_event({message_ids, message_details}, complete_rerender) { // This is the reverse of process_unread_messages_event. message_ids = unread.get_read_message_ids(message_ids); if (message_ids.length === 0) { @@ -347,7 +346,7 @@ export function process_unread_messages_event({message_ids, message_details}) { user-visible in the form of users' avatars flickering. */ message_live_update.rerender_messages_view_by_message_ids(message_ids); - recent_topics_ui.complete_rerender(); + complete_rerender(); if ( !message_lists.current.can_mark_messages_read() && @@ -361,7 +360,7 @@ export function process_unread_messages_event({message_ids, message_details}) { // Takes a list of messages and marks them as read. // Skips any messages that are already marked as read. -export function notify_server_messages_read(messages, options = {}) { +export function notify_server_messages_read(messages, options = {}, update_topic_unread_count) { messages = unread.get_unread_messages(messages); if (messages.length === 0) { return; @@ -375,7 +374,7 @@ export function notify_server_messages_read(messages, options = {}) { } unread.mark_as_read(message.id); - process_newly_read_message(message, options); + process_newly_read_message(message, options, update_topic_unread_count); } unread_ui.update_unread_counts(); @@ -385,14 +384,14 @@ export function notify_server_message_read(message, options) { notify_server_messages_read([message], options); } -export function process_scrolled_to_bottom() { +export function process_scrolled_to_bottom(update_topic_unread_count) { if (!narrow_state.is_message_feed_visible()) { // First, verify the current message list is visible. return; } if (message_lists.current.can_mark_messages_read()) { - mark_current_list_as_read(); + mark_current_list_as_read({}, update_topic_unread_count); return; } @@ -406,14 +405,18 @@ export function process_scrolled_to_bottom() { // If we ever materially change the algorithm for this function, we // may need to update notifications.received_messages as well. -export function process_visible() { +export function process_visible(update_topic_unread_count) { if (message_viewport.is_visible_and_focused() && message_viewport.bottom_message_visible()) { - process_scrolled_to_bottom(); + process_scrolled_to_bottom(update_topic_unread_count); } } -export function mark_current_list_as_read(options) { - notify_server_messages_read(message_lists.current.all_messages(), options); +export function mark_current_list_as_read(options, update_topic_unread_count) { + notify_server_messages_read( + message_lists.current.all_messages(), + options, + update_topic_unread_count, + ); } export function mark_stream_as_read(stream_id, cont) { diff --git a/web/tests/example7.test.js b/web/tests/example7.test.js index 9e7c18697a0d5..4423b13fb310f 100644 --- a/web/tests/example7.test.js +++ b/web/tests/example7.test.js @@ -60,6 +60,7 @@ message_lists.home = {}; message_lists.all_rendered_message_lists = () => [message_lists.home, message_lists.current]; const message_store = zrequire("message_store"); +const recent_topics_ui = zrequire("recent_topics_ui"); const recent_topics_util = zrequire("recent_topics_util"); const stream_data = zrequire("stream_data"); const unread = zrequire("unread"); @@ -129,13 +130,13 @@ run_test("unread_ops", ({override}) => { // First, test for a message list that cannot read messages. can_mark_messages_read = false; - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); assert.deepEqual(channel_post_opts, undefined); // Now flip the boolean, and get to the main thing we are testing. can_mark_messages_read = true; - unread_ops.process_visible(); + unread_ops.process_visible(recent_topics_ui.update_topic_unread_count); // The most important side effect of the above call is that // we post info to the server. We can verify that the correct