Update unread count when creating important local notifications

ts/groups.ts

@@ -83,6 +83,8 @@ import {
   conversationJobQueue,
   conversationQueueJobEnum,
 } from './jobs/conversationJobQueue';
+import { ReadStatus } from './messages/MessageReadStatus';
+import { SeenStatus } from './MessageSeenStatus';
 
 type AccessRequiredEnum = Proto.AccessControl.AccessRequired;
 
@@ -272,7 +274,10 @@ type UploadedAvatarType = {
   key: string;
 };
 
-type BasicMessageType = Pick<MessageAttributesType, 'id' | 'schemaVersion'>;
+type BasicMessageType = Pick<
+  MessageAttributesType,
+  'id' | 'schemaVersion' | 'readStatus' | 'seenStatus'
+>;
 
 type GroupV2ChangeMessageType = {
   type: 'group-v2-change';
@@ -1845,9 +1850,11 @@ export async function createGroupV2({
     type: 'group-v2-change',
     sourceUuid: ourUuid,
     conversationId: conversation.id,
+    readStatus: ReadStatus.Read,
     received_at: window.Signal.Util.incrementMessageCounter(),
     received_at_ms: timestamp,
     timestamp,
+    seenStatus: SeenStatus.Seen,
     sent_at: timestamp,
     groupV2Change: {
       from: ourUuid,
@@ -2289,6 +2296,8 @@ export async function initiateMigrationToGroupV2(
         type: 'group-v1-migration',
         invitedGV2Members: pendingMembersV2,
         droppedGV2MemberIds,
+        readStatus: ReadStatus.Read,
+        seenStatus: SeenStatus.Seen,
       });
 
       await updateGroup({
@@ -2634,7 +2643,11 @@ export async function respondToGroupV2Migration({
                   ),
                 },
                 groupChangeMessages: [
-                  getBasicMigrationBubble(),
+                  {
+                    ...getBasicMigrationBubble(),
+                    readStatus: ReadStatus.Read,
+                    seenStatus: SeenStatus.Seen,
+                  },
                   {
                     ...generateBasicMessage(),
                     type: 'group-v2-change',
@@ -2646,6 +2659,8 @@ export async function respondToGroupV2Migration({
                         },
                       ],
                     },
+                    readStatus: ReadStatus.Read,
+                    seenStatus: SeenStatus.Unseen,
                   },
                 ],
                 members: [],
@@ -2663,7 +2678,13 @@ export async function respondToGroupV2Migration({
             sentAt,
             updates: {
               newAttributes: attributes,
-              groupChangeMessages: [getBasicMigrationBubble()],
+              groupChangeMessages: [
+                {
+                  ...getBasicMigrationBubble(),
+                  readStatus: ReadStatus.Read,
+                  seenStatus: SeenStatus.Seen,
+                },
+              ],
               members: [],
             },
           });
@@ -2694,9 +2715,11 @@ export async function respondToGroupV2Migration({
   // Generate notifications into the timeline
   const groupChangeMessages: Array<GroupChangeMessageType> = [];
 
-  groupChangeMessages.push(
+  groupChangeMessages.push({
-    buildMigrationBubble(previousGroupV1MembersIds, newAttributes)
+    ...buildMigrationBubble(previousGroupV1MembersIds, newAttributes),
-  );
+    readStatus: ReadStatus.Read,
+    seenStatus: SeenStatus.Seen,
+  });
 
   const areWeInvited = (newAttributes.pendingMembersV2 || []).some(
     item => item.uuid === ourUuid
@@ -2717,6 +2740,8 @@ export async function respondToGroupV2Migration({
           },
         ],
       },
+      readStatus: ReadStatus.Read,
+      seenStatus: SeenStatus.Unseen,
     });
   }
 
@@ -3190,6 +3215,7 @@ async function appendChangeMessages(
   // We updated the message, but didn't add new ones - refresh left pane
   if (!newMessages && mergedMessages.length > 0) {
     await conversation.updateLastMessage();
+    conversation.updateUnread();
   }
 }
 
@@ -3519,6 +3545,8 @@ async function generateLeftGroupChanges(
         },
       ],
     },
+    readStatus: ReadStatus.Read,
+    seenStatus: SeenStatus.Unseen,
   };
 
   return {
@@ -4299,6 +4327,7 @@ function extractDiffs({
   let timerNotification: GroupChangeMessageType | undefined;
 
   const firstUpdate = !isNumber(old.revision);
+  const isFromUs = ourUuid === sourceUuid;
 
   // Here we hardcode initial messages if this is our first time processing data this
   //   group. Ideally we can collapse it down to just one of: 'you were added',
@@ -4317,6 +4346,8 @@ function extractDiffs({
           },
         ],
       },
+      readStatus: ReadStatus.Read,
+      seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
     };
   } else if (firstUpdate && dropInitialJoinMessage) {
     // None of the rest of the messages should be added if dropInitialJoinMessage = true
@@ -4333,6 +4364,8 @@ function extractDiffs({
           },
         ],
       },
+      readStatus: ReadStatus.Read,
+      seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
     };
   } else if (firstUpdate && areWeInGroup) {
     message = {
@@ -4347,6 +4380,8 @@ function extractDiffs({
           },
         ],
       },
+      readStatus: ReadStatus.Read,
+      seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
     };
   } else if (firstUpdate) {
     message = {
@@ -4360,6 +4395,8 @@ function extractDiffs({
           },
         ],
       },
+      readStatus: ReadStatus.Read,
+      seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
     };
   } else if (details.length > 0) {
     message = {
@@ -4370,6 +4407,8 @@ function extractDiffs({
         from: sourceUuid,
         details,
       },
+      readStatus: ReadStatus.Read,
+      seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
     };
   }
 

ts/models/conversations.ts

@@ -2909,6 +2909,7 @@ export class ConversationModel extends window.Backbone
     );
 
     this.trigger('newmessage', model);
+    this.updateUnread();
   }
 
   async addDeliveryIssue({
@@ -2954,6 +2955,7 @@ export class ConversationModel extends window.Backbone
     this.trigger('newmessage', model);
 
     await this.notify(model);
+    this.updateUnread();
   }
 
   async addKeyChange(keyChangedId: UUID): Promise<void> {
@@ -3050,6 +3052,7 @@ export class ConversationModel extends window.Backbone
     );
 
     this.trigger('newmessage', model);
+    this.updateUnread();
 
     const uuid = this.getUuid();
     if (isDirectConversation(this.attributes) && uuid) {
@@ -3115,6 +3118,7 @@ export class ConversationModel extends window.Backbone
     );
 
     this.trigger('newmessage', model);
+    this.updateUnread();
   }
 
   /**
@@ -4546,7 +4550,9 @@ export class ConversationModel extends window.Backbone
     model.set({ id });
 
     const message = window.MessageController.register(id, model);
+
     this.addSingleMessage(message);
+    this.updateUnread();
 
     return message;
   }

ts/sql/migrations/58-update-unread.ts

@@ -0,0 +1,132 @@
+// Copyright 2021-2022 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { Database } from 'better-sqlite3';
+import { ReadStatus } from '../../messages/MessageReadStatus';
+import { SeenStatus } from '../../MessageSeenStatus';
+
+import type { LoggerType } from '../../types/Logging';
+
+export default function updateToSchemaVersion58(
+  currentVersion: number,
+  db: Database,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 58) {
+    return;
+  }
+
+  db.transaction(() => {
+    db.exec(
+      `
+      --- Promote unread status in JSON to SQL column
+
+      UPDATE messages
+        SET
+          readStatus = ${ReadStatus.Unread},
+          seenStatus = ${SeenStatus.Unseen}
+        WHERE
+          json_extract(json, '$.unread') IS true OR
+          json_extract(json, '$.unread') IS 1;
+
+      --- Clean up all old messages that still have a null read status
+      ---   Note: we don't need to update seenStatus, because that was defaulted to zero
+
+      UPDATE messages
+        SET
+          readStatus = ${ReadStatus.Read}
+        WHERE
+          readStatus IS NULL;
+
+      --- Re-run unseen/unread queries from migration 56
+
+      UPDATE messages
+        SET
+          seenStatus = ${SeenStatus.Unseen}
+        WHERE
+          readStatus = ${ReadStatus.Unread} AND
+          (
+            type IS NULL
+            OR
+            type IN (
+              'call-history',
+              'change-number-notification',
+              'chat-session-refreshed',
+              'delivery-issue',
+              'group',
+              'incoming',
+              'keychange',
+              'timer-notification',
+              'verified-change'
+            )
+          );
+
+      UPDATE messages
+        SET
+          readStatus = ${ReadStatus.Read}
+        WHERE
+          readStatus = ${ReadStatus.Unread} AND
+          type IS NOT NULL AND
+          type NOT IN (
+            'call-history',
+            'change-number-notification',
+            'chat-session-refreshed',
+            'delivery-issue',
+            'group',
+            'incoming',
+            'keychange',
+            'timer-notification',
+            'verified-change'
+          );
+
+      --- (new) Ensure these message types are not unread, just unseen
+
+      UPDATE messages
+        SET
+          readStatus = ${ReadStatus.Read}
+        WHERE
+          readStatus = ${ReadStatus.Unread} AND
+          (
+            type IN (
+              'change-number-notification',
+              'keychange'
+            )
+          );
+
+      --- (new) Ensure that these message types are neither unseen nor unread
+
+      UPDATE messages
+        SET
+          readStatus = ${ReadStatus.Read},
+          seenStatus = ${SeenStatus.Seen}
+        WHERE
+          type IN (
+            'group-v1-migration',
+            'message-history-unsynced',
+            'outgoing',
+            'profile-change',
+            'universal-timer-notification'
+          );
+
+      --- Make sure JSON reflects SQL columns
+
+      UPDATE messages
+        SET
+          json = json_patch(
+            json,
+            json_object(
+              'readStatus', readStatus,
+              'seenStatus', seenStatus
+            )
+          )
+        WHERE
+          readStatus IS NOT NULL OR
+          seenStatus IS NOT 0;
+      `
+    );
+
+    db.pragma('user_version = 58');
+  })();
+
+  logger.info('updateToSchemaVersion58: success!');
+}

ts/sql/migrations/index.ts

@@ -33,6 +33,7 @@ import updateToSchemaVersion54 from './54-unprocessed-received-at-counter';
 import updateToSchemaVersion55 from './55-report-message-aci';
 import updateToSchemaVersion56 from './56-add-unseen-to-message';
 import updateToSchemaVersion57 from './57-rm-message-history-unsynced';
+import updateToSchemaVersion58 from './58-update-unread';
 
 function updateToSchemaVersion1(
   currentVersion: number,
@@ -1929,6 +1930,7 @@ export const SCHEMA_VERSIONS = [
   updateToSchemaVersion55,
   updateToSchemaVersion56,
   updateToSchemaVersion57,
+  updateToSchemaVersion58,
 ];
 
 export function updateSchema(db: Database, logger: LoggerType): void {

ts/test-node/sql_migrations_test.ts

@@ -2026,4 +2026,340 @@ describe('SQL migrations test', () => {
       assert.notInclude(second, 'SCAN', 'second');
     });
   });
+
+  describe('updateToSchemaVersion58', () => {
+    it('updates readStatus/seenStatus for messages with unread: true/1 in JSON', () => {
+      const MESSAGE_ID_1 = generateGuid();
+      const MESSAGE_ID_2 = generateGuid();
+      const MESSAGE_ID_3 = generateGuid();
+      const MESSAGE_ID_4 = generateGuid();
+      const CONVERSATION_ID = generateGuid();
+
+      updateToVersion(57);
+
+      // prettier-ignore
+      db.exec(
+        `
+        INSERT INTO messages
+          (id, conversationId, type, json)
+          VALUES
+          ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify(
+            { unread: true }
+          )}'),
+          ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify(
+            { unread: 1 }
+          )}'),
+          ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify(
+            { unread: undefined }
+           )}'),
+          ('${MESSAGE_ID_4}', '${CONVERSATION_ID}', 'incoming', '${JSON.stringify(
+            { unread: 0 }
+          )}');
+        `
+      );
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        4,
+        'starting total'
+      );
+
+      updateToVersion(58);
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        4,
+        'ending total'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};`
+          )
+          .pluck()
+          .get(),
+        2,
+        'ending unread count'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE seenStatus = ${SeenStatus.Unseen};`
+          )
+          .pluck()
+          .get(),
+        2,
+        'ending unread count'
+      );
+
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT readStatus FROM messages WHERE id = '${MESSAGE_ID_2}' LIMIT 1;`
+          )
+          .pluck()
+          .get(),
+        ReadStatus.Unread,
+        'checking read status for message2'
+      );
+    });
+
+    it('updates unseenStatus for previously-unread messages', () => {
+      const MESSAGE_ID_1 = generateGuid();
+      const MESSAGE_ID_2 = generateGuid();
+      const MESSAGE_ID_3 = generateGuid();
+      const MESSAGE_ID_4 = generateGuid();
+      const MESSAGE_ID_5 = generateGuid();
+      const MESSAGE_ID_6 = generateGuid();
+      const MESSAGE_ID_7 = generateGuid();
+      const MESSAGE_ID_8 = generateGuid();
+      const MESSAGE_ID_9 = generateGuid();
+      const MESSAGE_ID_10 = generateGuid();
+      const MESSAGE_ID_11 = generateGuid();
+      const CONVERSATION_ID = generateGuid();
+
+      updateToVersion(55);
+
+      db.exec(
+        `
+        INSERT INTO messages
+          (id, conversationId, type, readStatus)
+          VALUES
+          ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'call-history', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'change-number-notification', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'chat-session-refreshed', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_4}', '${CONVERSATION_ID}', 'delivery-issue', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_5}', '${CONVERSATION_ID}', 'group', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_6}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_7}', '${CONVERSATION_ID}', 'keychange', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_8}', '${CONVERSATION_ID}', 'timer-notification', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_9}', '${CONVERSATION_ID}', 'verified-change', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_10}', '${CONVERSATION_ID}', NULL, ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_11}', '${CONVERSATION_ID}', 'other', ${ReadStatus.Unread});
+        `
+      );
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        11,
+        'starting total'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};`
+          )
+          .pluck()
+          .get(),
+        11,
+        'starting unread count'
+      );
+
+      updateToVersion(56);
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        11,
+        'ending total'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};`
+          )
+          .pluck()
+          .get(),
+        10,
+        'ending unread count'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE seenStatus = ${SeenStatus.Unseen};`
+          )
+          .pluck()
+          .get(),
+        10,
+        'ending unseen count'
+      );
+
+      assert.strictEqual(
+        db
+          .prepare(
+            "SELECT readStatus FROM messages WHERE type = 'other' LIMIT 1;"
+          )
+          .pluck()
+          .get(),
+        ReadStatus.Read,
+        "checking read status for 'other' message"
+      );
+    });
+
+    it('Sets readStatus=Read for keychange and change-number-notification messages', () => {
+      const MESSAGE_ID_1 = generateGuid();
+      const MESSAGE_ID_2 = generateGuid();
+      const MESSAGE_ID_3 = generateGuid();
+      const CONVERSATION_ID = generateGuid();
+
+      updateToVersion(57);
+
+      db.exec(
+        `
+        INSERT INTO messages
+          (id, conversationId, type, readStatus)
+          VALUES
+          ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'change-number-notification', ${ReadStatus.Unread}),
+          ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'keychange', ${ReadStatus.Unread});
+        `
+      );
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        3,
+        'starting total'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};`
+          )
+          .pluck()
+          .get(),
+        3,
+        'starting unread count'
+      );
+
+      updateToVersion(58);
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        3,
+        'ending total'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT COUNT(*) FROM messages WHERE readStatus = ${ReadStatus.Unread};`
+          )
+          .pluck()
+          .get(),
+        1,
+        'ending unread count'
+      );
+
+      assert.strictEqual(
+        db
+          .prepare(
+            "SELECT readStatus FROM messages WHERE type = 'keychange' LIMIT 1;"
+          )
+          .pluck()
+          .get(),
+        ReadStatus.Read,
+        "checking read status for 'keychange' message"
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            "SELECT seenStatus FROM messages WHERE type = 'keychange' LIMIT 1;"
+          )
+          .pluck()
+          .get(),
+        SeenStatus.Unseen,
+        "checking seen status for 'keychange' message"
+      );
+    });
+
+    it('updates readStatus/seenStatus for messages with unread: true/1 in JSON', () => {
+      const MESSAGE_ID_1 = generateGuid();
+      const MESSAGE_ID_2 = generateGuid();
+      const MESSAGE_ID_3 = generateGuid();
+      const MESSAGE_ID_4 = generateGuid();
+      const CONVERSATION_ID = generateGuid();
+
+      updateToVersion(57);
+
+      // prettier-ignore
+      db.exec(
+        `
+        INSERT INTO messages
+          (id, conversationId, type, readStatus, seenStatus, json)
+          VALUES
+          ('${MESSAGE_ID_1}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Unread}, NULL, '${JSON.stringify(
+            { body: 'message1' }
+          )}'),
+          ('${MESSAGE_ID_2}', '${CONVERSATION_ID}', 'incoming', ${ReadStatus.Read}, NULL, '${JSON.stringify(
+            { body: 'message2' }
+          )}'),
+          ('${MESSAGE_ID_3}', '${CONVERSATION_ID}', 'incoming', NULL, ${SeenStatus.Unseen}, '${JSON.stringify(
+            { body: 'message3' }
+           )}'),
+          ('${MESSAGE_ID_4}', '${CONVERSATION_ID}', 'incoming', NULL, ${SeenStatus.Seen}, '${JSON.stringify(
+            { body: 'message4' }
+          )}');
+        `
+      );
+
+      assert.strictEqual(
+        db.prepare('SELECT COUNT(*) FROM messages;').pluck().get(),
+        4,
+        'starting total'
+      );
+
+      updateToVersion(58);
+
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT json FROM messages WHERE id = '${MESSAGE_ID_1}' LIMIT 1;`
+          )
+          .pluck()
+          .get(),
+        JSON.stringify({
+          body: 'message1',
+          readStatus: ReadStatus.Unread,
+          seenStatus: SeenStatus.Unseen,
+        }),
+        'checking JSON for message1'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT json FROM messages WHERE id = '${MESSAGE_ID_2}' LIMIT 1;`
+          )
+          .pluck()
+          .get(),
+        JSON.stringify({ body: 'message2', readStatus: ReadStatus.Read }),
+        'checking JSON for message2'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT json FROM messages WHERE id = '${MESSAGE_ID_3}' LIMIT 1;`
+          )
+          .pluck()
+          .get(),
+        JSON.stringify({
+          body: 'message3',
+          readStatus: ReadStatus.Read,
+          seenStatus: SeenStatus.Unseen,
+        }),
+        'checking JSON for message3'
+      );
+      assert.strictEqual(
+        db
+          .prepare(
+            `SELECT json FROM messages WHERE id = '${MESSAGE_ID_4}' LIMIT 1;`
+          )
+          .pluck()
+          .get(),
+        JSON.stringify({
+          body: 'message4',
+          readStatus: ReadStatus.Read,
+          seenStatus: SeenStatus.Seen,
+        }),
+        'checking JSON for message4'
+      );
+    });
+  });
 });