diff --git a/web/src/message_fetch.js b/web/src/message_fetch.js index d8e0a513fb..211b3196de 100644 --- a/web/src/message_fetch.js +++ b/web/src/message_fetch.js @@ -19,6 +19,8 @@ import * as stream_data from "./stream_data"; import * as stream_list from "./stream_list"; import * as ui_report from "./ui_report"; +let is_all_messages_data_loaded = false; + const consts = { backfill_idle_time: 10 * 1000, backfill_batch_size: 1000, @@ -59,21 +61,24 @@ function process_result(data, opts) { } else { opts.msg_list_data.add_messages(messages); } - } - if (messages.length > 0 && opts.msg_list === message_lists.home) { - // We keep track of how far back we've fetched messages for, for messaging in - // the recent view. This assumes `data.messages` is already sorted. - const oldest_timestamp = all_messages_data.first().timestamp; - recent_view_ui.set_oldest_message_date( - oldest_timestamp, - has_found_oldest, - has_found_newest, - ); + // To avoid non-contiguous blocks of data in recent view from + // message_lists.home and recent_view_message_list_data, we + // only process data from message_lists.home if we have found + // the newest message in message_lists.home. We check this via + // is_all_messages_data_loaded, to avoid unnecessary + // double-processing of the last batch of messages; + // is_all_messages_data_loaded is set via opts.cont, below. + if ( + opts.is_recent_view_data || + (opts.msg_list === message_lists.home && is_all_messages_data_loaded) + ) { + const msg_list_data = opts.msg_list_data ?? opts.msg_list.data; + recent_view_ui.process_messages(messages, msg_list_data); + } } huddle_data.process_loaded_messages(messages); - recent_view_ui.process_messages(messages); stream_list.update_streams_sidebar(); stream_list.maybe_scroll_narrow_into_view(); @@ -501,6 +506,13 @@ export function initialize(home_view_loaded) { } if (data.found_newest) { + // Mark that we've finishing loading all the way to the + // present in the all_messages_data data set. At this + // time, it's safe to call recent_view_ui.process_messages + // with all the messages in our cache. + is_all_messages_data_loaded = true; + recent_view_ui.process_messages(all_messages_data.all_messages(), all_messages_data); + if (page_params.is_spectator) { // Since for spectators, this is the main fetch, we // hide the Recent Conversations loading indicator here. @@ -509,6 +521,7 @@ export function initialize(home_view_loaded) { // See server_events.js for this callback. home_view_loaded(); + start_backfilling_messages(); return; } @@ -591,6 +604,7 @@ export function initialize(home_view_loaded) { num_before: consts.recent_view_initial_fetch_size, num_after: 0, msg_list_data: recent_view_message_list_data, + is_recent_view_data: true, cont: recent_view_ui.hide_loading_indicator, }); } diff --git a/web/src/recent_view_data.ts b/web/src/recent_view_data.ts index 8f3d6bda78..e344ea1a13 100644 --- a/web/src/recent_view_data.ts +++ b/web/src/recent_view_data.ts @@ -12,6 +12,11 @@ export const topics = new Map(); // For pms, key is the user IDs to whom the message is being sent. export function process_message(msg: Message): boolean { + // Important: This function must correctly handle processing a + // given message more than once; this happens during the loading + // process because of how recent_view_message_list_data duplicates + // all_messages_data. + // Return whether any conversation data is updated. let conversation_data_updated = false; diff --git a/web/src/recent_view_ui.js b/web/src/recent_view_ui.js index d8322dba10..bcd2feb391 100644 --- a/web/src/recent_view_ui.js +++ b/web/src/recent_view_ui.js @@ -130,9 +130,14 @@ const SOME_MESSAGES_LOADED_INCLUDING_NEWEST = 2; const ALL_MESSAGES_LOADED = 3; let loading_state = NO_MESSAGES_LOADED; -let oldest_message_timestamp; +let oldest_message_timestamp = Number.POSITIVE_INFINITY; + +export function set_oldest_message_date(msg_list_data) { + const has_found_oldest = msg_list_data.fetch_status.has_found_oldest(); + const has_found_newest = msg_list_data.fetch_status.has_found_newest(); + + oldest_message_timestamp = Math.min(msg_list_data.first().timestamp, oldest_message_timestamp); -export function set_oldest_message_date(timestamp, has_found_oldest, has_found_newest) { if (has_found_oldest) { loading_state = ALL_MESSAGES_LOADED; } else if (has_found_newest) { @@ -140,7 +145,6 @@ export function set_oldest_message_date(timestamp, has_found_oldest, has_found_n } else { loading_state = SOME_MESSAGES_LOADED; } - oldest_message_timestamp = timestamp; // We might be loading messages in another narrow before the recent view // is shown, so we keep the state updated and update the banner only @@ -369,11 +373,16 @@ export function hide_loading_indicator() { }); } -export function process_messages(messages) { - // While this is inexpensive and handles all the cases itself, - // the UX can be bad if user wants to scroll down the list as - // the UI will be returned to the beginning of the list on every - // update. +export function process_messages(messages, msg_list_data) { + // This code path processes messages from 3 sources: + // 1. Newly sent messages from the server_events system. This is safe to + // process because we always will have the latest previously sent messages. + // 2. Messages in all_messages_data, the main cache of contiguous + // message history that the client maintains. + // 3. Latest messages fetched specifically for Recent view when + // the browser first loads. We will be able to remove this once + // all_messages_data is fetched in a better order. + let conversation_data_updated = false; if (messages.length > 0) { for (const msg of messages) { @@ -383,6 +392,12 @@ export function process_messages(messages) { } } + if (msg_list_data) { + // Update the recent view UI's understanding of which messages + // we have available for the recent view. + set_oldest_message_date(msg_list_data); + } + // Only rerender if conversation data actually changed. if (conversation_data_updated) { complete_rerender();