From bec667ebee22f9324f690adb64fb7b8f5db915f2 Mon Sep 17 00:00:00 2001
From: Audric Ackermann <audric@loki.network>
Date: Thu, 22 Jun 2023 14:12:30 +0200
Subject: [PATCH] fix: remove row_number ordering when looking up message

---
 ts/components/avatar/Avatar.tsx               |  1 +
 .../leftpane/LeftPaneMessageSection.tsx       |  3 +-
 ts/node/sql.ts                                | 65 ++++++++++++-------
 ts/state/selectors/conversations.ts           | 10 ++-
 ts/state/selectors/messages.ts                |  2 +-
 5 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/ts/components/avatar/Avatar.tsx b/ts/components/avatar/Avatar.tsx
index 0318dfedc..c441b759e 100644
--- a/ts/components/avatar/Avatar.tsx
+++ b/ts/components/avatar/Avatar.tsx
@@ -98,6 +98,7 @@ const AvatarImage = (props: {
   }
   const dataToDisplay = base64Data ? `data:image/jpeg;base64,${base64Data}` : avatarPath;
 
+  // tslint:disable: react-a11y-img-has-alt
   return (
     <img
       onError={handleImageError}
diff --git a/ts/components/leftpane/LeftPaneMessageSection.tsx b/ts/components/leftpane/LeftPaneMessageSection.tsx
index fbc72ca61..ec0f529fa 100644
--- a/ts/components/leftpane/LeftPaneMessageSection.tsx
+++ b/ts/components/leftpane/LeftPaneMessageSection.tsx
@@ -100,14 +100,13 @@ export class LeftPaneMessageSection extends React.Component<Props> {
     // Note: conversations is not a known prop for List, but it is required to ensure that
     //   it re-renders when our conversations data changes. Otherwise it would just render
     //   on startup and scroll.
-
+    // TODO do need that `conversations` prop? I again don't see why it is needed. Especially because the list item use hook to fetch their details.
     return (
       <StyledLeftPaneList key={0}>
         <AutoSizer>
           {({ height, width }) => (
             <List
               className="module-left-pane__virtual-list"
-              conversations={conversations}
               height={height}
               rowCount={length}
               rowHeight={64}
diff --git a/ts/node/sql.ts b/ts/node/sql.ts
index 5772dcd58..7078cd289 100644
--- a/ts/node/sql.ts
+++ b/ts/node/sql.ts
@@ -1241,34 +1241,49 @@ function getMessagesByConversation(conversationId: string, { messageId = null }
     const messageFound = getMessageById(messageId || firstUnread);
 
     if (messageFound && messageFound.conversationId === conversationId) {
-      // tslint:disable-next-line: no-shadowed-variable
-      const rows = assertGlobalInstance()
-        .prepare(
-          `WITH cte AS (
-            SELECT id, conversationId, json, row_number() OVER (${orderByClause}) as row_number
-              FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId
-          ), current AS (
-          SELECT row_number
-            FROM cte
-          WHERE id = $messageId
+      // tslint:disable-next-li ne: no-shadowed-variable
+      const start = Date.now();
+      const msgTimestamp =
+        messageFound.serverTimestamp || messageFound.sent_at || messageFound.received_at;
 
+      const commonArgs = {
+        conversationId,
+        msgTimestamp,
+        limit:
+          numberOfMessagesInConvo < floorLoadAllMessagesInConvo
+            ? floorLoadAllMessagesInConvo
+            : absLimit,
+      };
+
+      const messagesBefore = assertGlobalInstance()
+        .prepare(
+          `SELECT id, conversationId, json
+            FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId AND COALESCE(serverTimestamp, sent_at, received_at) <= $msgTimestamp
+            ${orderByClause}
+            LIMIT $limit`
         )
-        SELECT cte.*
-          FROM cte, current
-            WHERE ABS(cte.row_number - current.row_number) <= $limit
-          ORDER BY cte.row_number;
-          `
+        .all(commonArgs);
+
+      const messagesAfter = assertGlobalInstance()
+        .prepare(
+          `SELECT id, conversationId, json
+            FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId AND COALESCE(serverTimestamp, sent_at, received_at) > $msgTimestamp
+            ${orderByClauseASC}
+            LIMIT $limit`
         )
-        .all({
-          conversationId,
-          messageId: messageId || firstUnread,
-          limit:
-            numberOfMessagesInConvo < floorLoadAllMessagesInConvo
-              ? floorLoadAllMessagesInConvo
-              : absLimit,
-        });
-
-      return map(rows, row => jsonToObject(row.json));
+        .all(commonArgs);
+
+      console.info(`getMessagesByConversation around took ${Date.now() - start}ms `);
+
+      // sorting is made in redux already when rendered, but some things are made outside of redux, so let's make sure the order is right
+      return map([...messagesBefore, ...messagesAfter], row => jsonToObject(row.json)).sort(
+        (a, b) => {
+          return (
+            (b.serverTimestamp || b.sent_at || b.received_at) -
+            (a.serverTimestamp || a.sent_at || a.received_at)
+          );
+        }
+      );
     }
     console.info(
       `getMessagesByConversation: Could not find messageId ${messageId} in db with conversationId: ${conversationId}. Just fetching the convo as usual.`
diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts
index 68de73d4b..b6c4dfab1 100644
--- a/ts/state/selectors/conversations.ts
+++ b/ts/state/selectors/conversations.ts
@@ -250,7 +250,9 @@ const _getLeftPaneLists = (
   sortedConversations: Array<ReduxConversationType>
 ): Array<ReduxConversationType> => {
   return sortedConversations.filter(conversation => {
-    if (conversation.isBlocked) return false;
+    if (conversation.isBlocked) {
+      return false;
+    }
 
     // a private conversation not approved is a message request. Exclude them from the left pane lists
 
@@ -285,12 +287,14 @@ const _getPrivateFriendsConversations = (
   sortedConversations: Array<ReduxConversationType>
 ): Array<ReduxConversationType> => {
   return sortedConversations.filter(convo => {
-    convo.isPrivate &&
+    return (
+      convo.isPrivate &&
       !convo.isMe &&
       !convo.isBlocked &&
       convo.isApproved &&
       convo.didApproveMe &&
-      convo.activeAt !== undefined;
+      convo.activeAt !== undefined
+    );
   });
 };
 
diff --git a/ts/state/selectors/messages.ts b/ts/state/selectors/messages.ts
index 5f4618f8b..2c3fe68c4 100644
--- a/ts/state/selectors/messages.ts
+++ b/ts/state/selectors/messages.ts
@@ -83,7 +83,7 @@ export const useMessageDirection = (messageId: string | undefined): string | und
   return useMessageIdProps(messageId)?.propsForMessage.direction;
 };
 
-export const useMessageLinkPreview = (messageId: string | undefined): any[] | undefined => {
+export const useMessageLinkPreview = (messageId: string | undefined): Array<any> | undefined => {
   return useMessageIdProps(messageId)?.propsForMessage.previews;
 };