message_fetch: Avoid non-contiguous conversations in recent view.

Fixes #27208. Fixes #27207.

We only process messages from all_messages_data and special data
fetched for recent view only.

This avoids us having a conversation present in recent view which is
not contiguous due to loading a random old conversation.

Also, to ensure displayed conversations are contiguous while we are
loading data, we don't show message list data until we have found the
newest message in the list.

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,
     });
 }

web/src/recent_view_data.ts

@@ -12,6 +12,11 @@ export const topics = new Map<string, TopicData>();
 // 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;
 

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();