Merge pull request #2560 from Bilb/mark-read-do-not-return-messages-if-not-needed

fix: do not return updated messages from markAllRead if not needed

ts/data/data.ts

@@ -493,9 +493,17 @@ async function getUnreadByConversation(conversationId: string): Promise<MessageC
 }
 
 async function markAllAsReadByConversationNoExpiration(
-  conversationId: string
+  conversationId: string,
+  returnMessagesUpdated: boolean // for performance reason we do not return them because usually they are not needed
 ): Promise<Array<number>> {
-  const messagesIds = await channels.markAllAsReadByConversationNoExpiration(conversationId);
+  // tslint:disable-next-line: no-console
+  console.time('markAllAsReadByConversationNoExpiration');
+  const messagesIds = await channels.markAllAsReadByConversationNoExpiration(
+    conversationId,
+    returnMessagesUpdated
+  );
+  // tslint:disable-next-line: no-console
+  console.timeEnd('markAllAsReadByConversationNoExpiration');
   return messagesIds;
 }
 

ts/interactions/conversationInteractions.ts

@@ -254,11 +254,7 @@ export function showLeaveGroupByConvoId(conversationId: string) {
 export function showInviteContactByConvoId(conversationId: string) {
   window.inboxStore?.dispatch(updateInviteContactModal({ conversationId }));
 }
-export async function onMarkAllReadByConvoId(conversationId: string) {
-  const conversation = getConversationController().get(conversationId);
 
-  await conversation.markReadBouncy(Date.now());
-}
 
 export function showAddModeratorsByConvoId(conversationId: string) {
   window.inboxStore?.dispatch(updateAddModeratorsModal({ conversationId }));

ts/models/conversation.ts

@@ -32,6 +32,7 @@ import {
   actions as conversationActions,
   conversationChanged,
   conversationsChanged,
+  markConversationFullyRead,
   MessageModelPropsWithoutConvoProps,
   ReduxConversationType,
 } from '../state/ducks/conversations';
@@ -1231,27 +1232,38 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
    * Send read receipt if needed.
    */
   public async markAllAsRead() {
-    if (this.isOpenGroupV2()) {
+    /**
+     *  when marking all as read, there is a bunch of things we need to do.
+     *   - we need to update all the messages in the DB not read yet for that conversation
+     *   - we need to send the read receipts if there is one needed for those messages
+     *   - we need to trigger a change on the redux store, so those messages are read AND mark the whole convo as read.
+     *   - we need to remove any notifications related to this conversation ID.
+     *
+     *
+     * (if there is an expireTimer, we do it the slow way, handling each message separately)
+     */
+    const expireTimerSet = !!this.get('expireTimer');
+    if (this.isOpenGroupV2() || !expireTimerSet) {
       // for opengroups, we batch everything as there is no expiration timer to take care (and potentially a lot of messages)
 
-      await Data.markAllAsReadByConversationNoExpiration(this.id);
+      const isOpenGroup = this.isOpenGroupV2();
+      // if this is an opengroup there is no need to send read receipt, and so no need to fetch messages updated.
+      const allReadMessages = await Data.markAllAsReadByConversationNoExpiration(
+        this.id,
+        !isOpenGroup
+      );
       this.set({ mentionedUs: false, unreadCount: 0 });
 
       await this.commit();
-      return;
+      if (!this.isOpenGroupV2() && allReadMessages.length) {
-    }
-
-    // if the conversation has no expiration timer, we can also batch everything, but we also need to send read receipts potentially
-    // so we grab them from the db
-    if (!this.get('expireTimer')) {
-      const allReadMessages = await Data.markAllAsReadByConversationNoExpiration(this.id);
-      this.set({ mentionedUs: false, unreadCount: 0 });
-      await this.commit();
-      if (allReadMessages.length) {
         await this.sendReadReceiptsIfNeeded(uniq(allReadMessages));
       }
+      Notifications.clearByConversationID(this.id);
+      window.inboxStore?.dispatch(markConversationFullyRead(this.id));
+
       return;
     }
+    // otherwise, do it the slow way
     await this.markReadBouncy(Date.now());
   }
 

ts/node/database_utility.ts

@@ -264,7 +264,7 @@ export function rebuildFtsTable(db: BetterSqlite3.Database) {
           CREATE TRIGGER messages_on_delete AFTER DELETE ON ${MESSAGES_TABLE} BEGIN
             DELETE FROM ${MESSAGES_FTS_TABLE} WHERE id = old.id;
           END;
-          CREATE TRIGGER messages_on_update AFTER UPDATE ON ${MESSAGES_TABLE} BEGIN
+          CREATE TRIGGER messages_on_update AFTER UPDATE ON ${MESSAGES_TABLE} WHEN new.body <> old.body BEGIN
             DELETE FROM ${MESSAGES_FTS_TABLE} WHERE id = old.id;
             INSERT INTO ${MESSAGES_FTS_TABLE}(
               id,

ts/node/migration/sessionMigrations.ts

@@ -78,6 +78,7 @@ const LOKI_SCHEMA_VERSIONS = [
   updateToSessionSchemaVersion26,
   updateToSessionSchemaVersion27,
   updateToSessionSchemaVersion28,
+  updateToSessionSchemaVersion29,
 ];
 
 function updateToSessionSchemaVersion1(currentVersion: number, db: BetterSqlite3.Database) {
@@ -1181,6 +1182,28 @@ function updateToSessionSchemaVersion28(currentVersion: number, db: BetterSqlite
   console.log(`updateToSessionSchemaVersion${targetVersion}: success!`);
 }
 
+function updateToSessionSchemaVersion29(currentVersion: number, db: BetterSqlite3.Database) {
+  const targetVersion = 29;
+  if (currentVersion >= targetVersion) {
+    return;
+  }
+
+  console.log(`updateToSessionSchemaVersion${targetVersion}: starting...`);
+
+  db.transaction(() => {
+    dropFtsAndTriggers(db);
+    db.exec(`CREATE INDEX messages_unread_by_conversation ON ${MESSAGES_TABLE} (
+      unread,
+      conversationId
+    );`);
+    rebuildFtsTable(db);
+    // Keeping this empty migration because some people updated to this already, even if it is not needed anymore
+    writeSessionSchemaVersion(targetVersion, db);
+  })();
+
+  console.log(`updateToSessionSchemaVersion${targetVersion}: success!`);
+}
+
 // function printTableColumns(table: string, db: BetterSqlite3.Database) {
 //   console.info(db.pragma(`table_info('${table}');`));
 // }

ts/node/sql.ts

@@ -1117,8 +1117,11 @@ function getUnreadByConversation(conversationId: string) {
  * Warning: This does not start expiration timer
  */
 function markAllAsReadByConversationNoExpiration(
-  conversationId: string
+  conversationId: string,
-): Array<{ id: string; timestamp: number }> {
+  returnMessagesUpdated: boolean
+): Array<number> {
+  let toReturn: Array<number> = [];
+  if (returnMessagesUpdated) {
     const messagesUnreadBefore = assertGlobalInstance()
       .prepare(
         `SELECT json FROM ${MESSAGES_TABLE} WHERE
@@ -1129,6 +1132,8 @@ function markAllAsReadByConversationNoExpiration(
         unread: 1,
         conversationId,
       });
+    toReturn = compact(messagesUnreadBefore.map(row => jsonToObject(row.json).sent_at));
+  }
 
   assertGlobalInstance()
     .prepare(
@@ -1142,7 +1147,7 @@ function markAllAsReadByConversationNoExpiration(
       conversationId,
     });
 
-  return compact(messagesUnreadBefore.map(row => jsonToObject(row.json).sent_at));
+  return toReturn;
 }
 
 function getUnreadCountByConversation(conversationId: string) {

ts/state/ducks/conversations.ts

@@ -699,11 +699,23 @@ const conversationsSlice = createSlice({
         return state;
       }
 
+      let updatedMessages = state.messages;
+
+      // if some are unread, mark them as read
+      if (state.messages.some(m => m.propsForMessage.isUnread)) {
+        updatedMessages = state.messages.map(m => ({
+          ...m,
+          propsForMessage: { ...m.propsForMessage, isUnread: false },
+        }));
+      }
+
       // keep the unread visible just like in other apps. It will be shown until the user changes convo
       return {
         ...state,
         shouldHighlightMessage: false,
         firstUnreadMessageId: undefined,
+
+        messages: updatedMessages,
       };
     },
     /**