Handle messages with the same received_at

js/views/conversation_view.js

@@ -567,6 +567,7 @@
           const receivedAt = message.get('received_at');
           const models = await getOlderMessagesByConversation(conversationId, {
             receivedAt,
+            messageId: oldestMessageId,
             limit: 500,
             MessageCollection: Whisper.MessageCollection,
           });
@@ -796,6 +797,7 @@
         const older = await getOlderMessagesByConversation(conversationId, {
           limit: 250,
           receivedAt,
+          messageId,
           MessageCollection: Whisper.MessageCollection,
         });
         const newer = await getNewerMessagesByConversation(conversationId, {
@@ -874,11 +876,18 @@
         const scrollToMessageId =
           setFocus && metrics.newest ? metrics.newest.id : undefined;
 
+        // Because our `getOlderMessages` fetch above didn't specify a receivedAt, we got
+        //   the most recent 500 messages in the conversation. If it has a conflict with
+        //   metrics, fetched a bit before, that's likely a race condition. So we tell our
+        //   reducer to trust the message set we just fetched for determining if we have
+        //   the newest message loaded.
+        const unboundedFetch = true;
         messagesReset(
           conversationId,
           cleaned.map(model => model.getReduxData()),
           metrics,
-          scrollToMessageId
+          scrollToMessageId,
+          unboundedFetch
         );
       } catch (error) {
         setMessagesLoading(conversationId, false);
@@ -1735,12 +1744,8 @@
         window.log.warn(`onOpened: Did not find message ${messageId}`);
       }
 
-      // Incoming messages may still be processing, so we wait until those are
-      //   complete to pull the 500 most-recent messages in this conversation.
-      this.model.queueJob(() => {
-        this.loadNewestMessages();
-        this.model.updateLastMessage();
-      });
+      this.loadNewestMessages();
+      this.model.updateLastMessage();
 
       this.focusMessageField();
 

ts/sql/Client.ts

@@ -969,10 +969,12 @@ async function getOlderMessagesByConversation(
   {
     limit = 100,
     receivedAt = Number.MAX_VALUE,
+    messageId,
     MessageCollection,
   }: {
     limit?: number;
     receivedAt?: number;
+    messageId?: string;
     MessageCollection: BackboneMessageCollectionType;
   }
 ) {
@@ -981,6 +983,7 @@ async function getOlderMessagesByConversation(
     {
       limit,
       receivedAt,
+      messageId,
     }
   );
 

ts/sql/Interface.ts

@@ -197,7 +197,7 @@ export type ServerInterface = DataInterface & {
   getMessagesBySentAt: (sentAt: number) => Promise<Array<MessageType>>;
   getOlderMessagesByConversation: (
     conversationId: string,
-    options?: { limit?: number; receivedAt?: number }
+    options?: { limit?: number; receivedAt?: number; messageId?: string }
   ) => Promise<Array<MessageTypeUnhydrated>>;
   getNewerMessagesByConversation: (
     conversationId: string,

ts/sql/Server.ts

@@ -2598,21 +2598,45 @@ async function getOlderMessagesByConversation(
   {
     limit = 100,
     receivedAt = Number.MAX_VALUE,
-  }: { limit?: number; receivedAt?: number } = {}
+    messageId,
+  }: { limit?: number; receivedAt?: number; messageId?: string } = {}
 ) {
+  if (receivedAt !== Number.MAX_VALUE && !messageId) {
+    throw new Error('If receivedAt is supplied, messageId should be as well');
+  }
+
   const db = getInstance();
-  const rows = await db.all(
-    `SELECT json FROM messages WHERE
+  let rows;
+
+  if (messageId) {
+    rows = await db.all(
+      `SELECT json FROM messages WHERE
+       conversationId = $conversationId AND
+       received_at <= $received_at AND
+       id != $messageId
+     ORDER BY received_at DESC
+     LIMIT $limit;`,
+      {
+        $conversationId: conversationId,
+        $received_at: receivedAt,
+        $limit: limit,
+        $messageId: messageId,
+      }
+    );
+  } else {
+    rows = await db.all(
+      `SELECT json FROM messages WHERE
        conversationId = $conversationId AND
        received_at < $received_at
      ORDER BY received_at DESC
      LIMIT $limit;`,
-    {
-      $conversationId: conversationId,
-      $received_at: receivedAt,
-      $limit: limit,
-    }
-  );
+      {
+        $conversationId: conversationId,
+        $received_at: receivedAt,
+        $limit: limit,
+      }
+    );
+  }
 
   return rows.reverse();
 }

ts/state/ducks/conversations.ts

@@ -209,6 +209,9 @@ export type MessagesResetActionType = {
     messages: Array<MessageType>;
     metrics: MessageMetricsType;
     scrollToMessageId?: string;
+    // The set of provided messages should be trusted, even if it conflicts with metrics,
+    //   because we weren't looking for a specific time window of messages with our query.
+    unboundedFetch: boolean;
   };
 };
 export type SetMessagesLoadingActionType = {
@@ -424,11 +427,13 @@ function messagesReset(
   conversationId: string,
   messages: Array<MessageType>,
   metrics: MessageMetricsType,
-  scrollToMessageId?: string
+  scrollToMessageId?: string,
+  unboundedFetch?: boolean
 ): MessagesResetActionType {
   return {
     type: 'MESSAGES_RESET',
     payload: {
+      unboundedFetch: Boolean(unboundedFetch),
       conversationId,
       messages,
       metrics,
@@ -784,6 +789,7 @@ export function reducer(
       messages,
       metrics,
       scrollToMessageId,
+      unboundedFetch,
     } = action.payload;
     const { messagesByConversation, messagesLookup } = state;
 
@@ -807,7 +813,10 @@ export function reducer(
       }
 
       const last = messages[messages.length - 1];
-      if (last && (!newest || last.received_at >= newest.received_at)) {
+      if (
+        last &&
+        (!newest || unboundedFetch || last.received_at >= newest.received_at)
+      ) {
         newest = pick(last, ['id', 'received_at']);
       }
     }