From 625b07c010c191273328620938a42bc6cee5b6fc Mon Sep 17 00:00:00 2001 From: Aman Agrawal Date: Sat, 11 May 2024 10:50:13 +0000 Subject: [PATCH] recent_view: Complete TODO to use first unmuted unread message id. We already had the data at initial request, so we just add some plumbing to make it available. --- web/src/recent_view_ui.ts | 4 ++-- web/src/ui_init.js | 7 ++----- web/src/unread.ts | 41 +++++++++++++++++++++++++++++++++------ 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/web/src/recent_view_ui.ts b/web/src/recent_view_ui.ts index 7fee96485c..1f7b96e5e8 100644 --- a/web/src/recent_view_ui.ts +++ b/web/src/recent_view_ui.ts @@ -1632,7 +1632,7 @@ export function initialize({ on_click_participant: (avatar_element: Element, participant_user_id: number) => void; on_mark_pm_as_read: (user_ids_string: string) => void; on_mark_topic_as_read: (stream_id: number, topic: string) => void; - maybe_load_older_messages: () => void; + maybe_load_older_messages: (first_unread_unmuted_message_id: number) => void; }): void { load_filters(); @@ -1813,7 +1813,7 @@ export function initialize({ $(".recent-view-load-more-container .fetch-messages-button .loading-indicator"), {width: 20}, ); - maybe_load_older_messages(); + maybe_load_older_messages(unread.first_unread_unmuted_message_id); }); $(document).on("compose_canceled.zulip", () => { diff --git a/web/src/ui_init.js b/web/src/ui_init.js index 95712b2393..261d2c3845 100644 --- a/web/src/ui_init.js +++ b/web/src/ui_init.js @@ -733,17 +733,14 @@ export function initialize_everything(state_data) { }, on_mark_pm_as_read: unread_ops.mark_pm_as_read, on_mark_topic_as_read: unread_ops.mark_topic_as_read, - maybe_load_older_messages() { + maybe_load_older_messages(first_unread_message_id) { recent_view_ui.set_backfill_in_progress(true); message_fetch.maybe_load_older_messages({ msg_list_data: all_messages_data, recent_view: true, // To have a hard anchor on our target of first unread message id, // we pass it from here, otherwise it might get updated and lead to confusion. - // - // TODO: Ideally, muted unread messages would not - // count for this purpose. - first_unread_message_id: unread.get_all_msg_ids()[0], + first_unread_message_id, }); }, }); diff --git a/web/src/unread.ts b/web/src/unread.ts index c4d8b5ff04..dfb0cd479c 100644 --- a/web/src/unread.ts +++ b/web/src/unread.ts @@ -26,6 +26,7 @@ import * as util from "./util"; // for more details on how this system is designed. export let old_unreads_missing = false; +export let first_unread_unmuted_message_id = Number.POSITIVE_INFINITY; export function clear_old_unreads_missing(): void { old_unreads_missing = false; @@ -112,13 +113,19 @@ class UnreadDirectMessageCounter { this.reverse_lookup.delete(message_id); } - get_counts(): DirectMessageCountInfo { + get_counts(update_first_unmuted_message_id = false): DirectMessageCountInfo { const pm_dict = new Map(); // Hash by user_ids_string -> count Optional[, max_id] let total_count = 0; for (const [user_ids_string, id_set] of this.bucketer) { const count = id_set.size; pm_dict.set(user_ids_string, count); total_count += count; + if (update_first_unmuted_message_id) { + first_unread_unmuted_message_id = Math.min( + first_unread_unmuted_message_id, + ...id_set, + ); + } } return { total_count, @@ -300,7 +307,7 @@ class UnreadTopicCounter { }; } - get_counts(): UnreadStreamCounts { + get_counts(update_first_unmuted_message_id = false): UnreadStreamCounts { let stream_unread_messages = 0; let followed_topic_unread_messages = 0; const stream_counts_by_id = new Map(); // hash by stream_id -> count @@ -317,7 +324,10 @@ class UnreadTopicCounter { // get_stream_count_info calculates both the number of // unmuted unread as well as the number of muted // unreads. - const stream_count_info = this.get_stream_count_info(stream_id); + const stream_count_info = this.get_stream_count_info( + stream_id, + update_first_unmuted_message_id, + ); stream_counts_by_id.set(stream_id, stream_count_info); stream_unread_messages += stream_count_info.unmuted_count; followed_topic_unread_messages += stream_count_info.followed_count; @@ -371,7 +381,10 @@ class UnreadTopicCounter { return result; } - get_stream_count_info(stream_id: number): StreamCountInfo { + get_stream_count_info( + stream_id: number, + update_first_unmuted_message_id = false, + ): StreamCountInfo { const per_stream_bucketer = this.bucketer.get(stream_id); if (!per_stream_bucketer) { @@ -396,12 +409,24 @@ class UnreadTopicCounter { if (user_topics.is_topic_unmuted_or_followed(stream_id, topic)) { unmuted_count += topic_count; + if (update_first_unmuted_message_id) { + first_unread_unmuted_message_id = Math.min( + first_unread_unmuted_message_id, + ...msgs, + ); + } } else if (user_topics.is_topic_muted(stream_id, topic)) { muted_count += topic_count; } else if (sub?.is_muted) { muted_count += topic_count; } else { unmuted_count += topic_count; + if (update_first_unmuted_message_id) { + first_unread_unmuted_message_id = Math.min( + first_unread_unmuted_message_id, + ...msgs, + ); + } } } const stream_count = { @@ -877,8 +902,12 @@ export type FullUnreadCountsData = { // pretty cheap, even if you don't care about all the counts, and you // should strive to keep it free of side effects on globals or DOM. export function get_counts(): FullUnreadCountsData { - const topic_res = unread_topic_counter.get_counts(); - const pm_res = unread_direct_message_counter.get_counts(); + const update_first_unmuted_message_id = true; + // Reset the first_unread_unmuted_message_id, to ensure it is always capture the + // minimum of current unread messages between topics and DMs. + first_unread_unmuted_message_id = Number.POSITIVE_INFINITY; + const topic_res = unread_topic_counter.get_counts(update_first_unmuted_message_id); + const pm_res = unread_direct_message_counter.get_counts(update_first_unmuted_message_id); return { direct_message_count: pm_res.total_count,