Skip to content

Commit

Permalink
js: Break cyclic dependecny between unread_ops and recent_topics_ui.
Browse files Browse the repository at this point in the history
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"
  • Loading branch information
sbansal1999 committed Apr 9, 2023
1 parent 946b4e7 commit 9d27119
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 33 deletions.
6 changes: 5 additions & 1 deletion web/src/compose_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
}

Expand Down
4 changes: 2 additions & 2 deletions web/src/message_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion web/src/message_flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 7 additions & 2 deletions web/src/message_scroll.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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),
);
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions web/src/narrow.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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},
Expand All @@ -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":
Expand Down
3 changes: 2 additions & 1 deletion web/src/notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 16 additions & 6 deletions web/src/server_events_dispatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
33 changes: 18 additions & 15 deletions web/src/unread_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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() &&
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions web/tests/example7.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9d27119

Please sign in to comment.