Ensure that seenStatus is always updated along with readStatus

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/models/conversations.ts

@@ -3383,6 +3383,7 @@ export class ConversationModel extends window.Backbone
     return this.queueJob('onReadMessage', () =>
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       this.markRead(message.get('received_at')!, {
+        newestSentAt: message.get('sent_at'),
         sendReadReceipts: false,
         readAt,
       })

ts/models/messages.ts

@@ -209,7 +209,16 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
 
     const readStatus = migrateLegacyReadStatus(this.attributes);
     if (readStatus !== undefined) {
-      this.set('readStatus', readStatus, { silent: true });
+      this.set(
+        {
+          readStatus,
+          seenStatus:
+            readStatus === ReadStatus.Unread
+              ? SeenStatus.Unseen
+              : SeenStatus.Seen,
+        },
+        { silent: true }
+      );
     }
 
     const sendStateByConversationId = migrateLegacySendAttributes(

ts/services/MessageUpdater.ts

@@ -4,6 +4,7 @@
 import type { MessageAttributesType } from '../model-types.d';
 import { ReadStatus, maxReadStatus } from '../messages/MessageReadStatus';
 import { notificationService } from './notifications';
+import { SeenStatus } from '../MessageSeenStatus';
 
 function markReadOrViewed(
   messageAttrs: Readonly<MessageAttributesType>,
@@ -17,6 +18,7 @@ function markReadOrViewed(
   const nextMessageAttributes: MessageAttributesType = {
     ...messageAttrs,
     readStatus: newReadStatus,
+    seenStatus: SeenStatus.Seen,
   };
 
   const { id: messageId, expireTimer, expirationStartTimestamp } = messageAttrs;

ts/sql/Interface.ts

@@ -21,6 +21,7 @@ import type { UUIDStringType } from '../types/UUID';
 import type { BadgeType } from '../badges/types';
 import type { RemoveAllConfiguration } from '../types/RemoveAllConfiguration';
 import type { LoggerType } from '../types/Logging';
+import type { ReadStatus } from '../messages/MessageReadStatus';
 
 export type AttachmentDownloadJobTypeType =
   | 'long-message'
@@ -397,7 +398,16 @@ export type DataInterface = {
     storyId?: UUIDStringType;
   }) => Promise<
     Array<
-      Pick<MessageType, 'id' | 'source' | 'sourceUuid' | 'sent_at' | 'type'>
+      { originalReadStatus: ReadStatus | undefined } & Pick<
+        MessageType,
+        | 'id'
+        | 'readStatus'
+        | 'seenStatus'
+        | 'sent_at'
+        | 'source'
+        | 'sourceUuid'
+        | 'type'
+      >
     >
   >;
   getUnreadReactionsAndMarkRead: (options: {

ts/sql/Server.ts

@@ -2075,7 +2075,18 @@ async function getUnreadByConversationAndMarkRead({
   storyId?: UUIDStringType;
   readAt?: number;
 }): Promise<
-  Array<Pick<MessageType, 'id' | 'source' | 'sourceUuid' | 'sent_at' | 'type'>>
+  Array<
+    { originalReadStatus: ReadStatus | undefined } & Pick<
+      MessageType,
+      | 'id'
+      | 'source'
+      | 'sourceUuid'
+      | 'sent_at'
+      | 'type'
+      | 'readStatus'
+      | 'seenStatus'
+    >
+  >
 > {
   const db = getInstance();
   return db.transaction(() => {
@@ -2109,10 +2120,10 @@ async function getUnreadByConversationAndMarkRead({
       .prepare<Query>(
         `
         SELECT id, json FROM messages
-        INDEXED BY messages_unread
         WHERE
-          readStatus = ${ReadStatus.Unread} AND
           conversationId = $conversationId AND
+          seenStatus = ${SeenStatus.Unseen} AND
+          isStory = 0 AND
           (${_storyIdPredicate(storyId, isGroup)}) AND
           received_at <= $newestUnreadAt
         ORDER BY received_at DESC, sent_at DESC;
@@ -2151,7 +2162,9 @@ async function getUnreadByConversationAndMarkRead({
     return rows.map(row => {
       const json = jsonToObject<MessageType>(row.json);
       return {
+        originalReadStatus: json.readStatus,
         readStatus: ReadStatus.Read,
+        seenStatus: SeenStatus.Seen,
         ...pick(json, [
           'expirationStartTimestamp',
           'id',

ts/test-electron/sql/markRead_test.ts

@@ -166,7 +166,7 @@ describe('sql/markRead', () => {
       readAt,
     });
 
-    assert.lengthOf(markedRead2, 3, 'three messages marked read');
+    assert.lengthOf(markedRead2, 2, 'two messages marked read');
     assert.strictEqual(markedRead2[0].id, message7.id, 'should be message7');
 
     assert.strictEqual(

ts/util/markConversationRead.ts

@@ -1,6 +1,8 @@
 // Copyright 2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
+import { omit } from 'lodash';
+
 import type { ConversationAttributesType } from '../model-types.d';
 import { hasErrors } from '../state/selectors/message';
 import { readReceiptsJobQueue } from '../jobs/readReceiptsJobQueue';
@@ -9,6 +11,7 @@ import { notificationService } from '../services/notifications';
 import { isGroup } from './whatTypeOfConversation';
 import * as log from '../logging/log';
 import { getConversationIdForLogging } from './idForLogging';
+import { ReadStatus } from '../messages/MessageReadStatus';
 
 export async function markConversationRead(
   conversationAttrs: ConversationAttributesType,
@@ -76,11 +79,12 @@ export async function markConversationRead(
     const message = window.MessageController.getById(messageSyncData.id);
     // we update the in-memory MessageModel with the fresh database call data
     if (message) {
-      message.set(messageSyncData);
+      message.set(omit(messageSyncData, 'originalReadStatus'));
     }
 
     return {
       messageId: messageSyncData.id,
+      originalReadStatus: messageSyncData.originalReadStatus,
       senderE164: messageSyncData.source,
       senderUuid: messageSyncData.sourceUuid,
       senderId: window.ConversationController.ensureContactIds({
@@ -92,14 +96,18 @@ export async function markConversationRead(
     };
   });
 
-  // Some messages we're marking read are local notifications with no sender
-  // If a message has errors, we don't want to send anything out about it.
+  // Some messages we're marking read are local notifications with no sender or were just
+  //   unseen and not unread.
+  // Also, if a message has errors, we don't want to send anything out about it:
   //   read syncs - let's wait for a client that really understands the message
   //      to mark it read. we'll mark our local error read locally, though.
   //   read receipts - here we can run into infinite loops, where each time the
   //      conversation is viewed, another error message shows up for the contact
   const unreadMessagesSyncData = allReadMessagesSync.filter(
-    item => Boolean(item.senderId) && !item.hasErrors
+    item =>
+      Boolean(item.senderId) &&
+      item.originalReadStatus === ReadStatus.Unread &&
+      !item.hasErrors
   );
 
   const readSyncs: Array<{