fix: preserve unread state for remote deletions

ts/interactions/conversations/deleteOrMarkAsDeletedMessages.ts

@@ -30,7 +30,7 @@ export async function deleteOrMarkAsDeletedMessages({
       await conversation.removeMessage(message.id);
     } else {
       // just mark the message as deleted but still show in conversation
-      await message.markAsDeleted(deletionType);
+      await message.markAsDeleted(deletionType, { shouldMarkAsRead: actionContextIsUI });
     }
   }
 }

ts/models/message.ts

@@ -111,7 +111,6 @@ import { privateSet, privateSetKey } from './modelFriends';
 import { getFeatureFlag } from '../state/ducks/types/releasedFeaturesReduxTypes';
 import type { OutgoingProMessageDetails } from '../types/message/OutgoingProMessageDetails';
 import { longOrNumberToBigInt } from '../types/Bigint';
-import { toSqliteBoolean } from '../node/database_utility';
 import type { WithLocalMessageDeletionType } from '../session/types/with';
 
 // tslint:disable: cyclomatic-complexity
@@ -967,7 +966,8 @@ export class MessageModel extends Model<MessageAttributes> {
     requestedDeleteType: Extract<
       WithLocalMessageDeletionType['deletionType'],
       'markDeletedGlobally' | 'markDeletedThisDevice'
-    >
+    >,
+    { shouldMarkAsRead }: { shouldMarkAsRead: boolean }
   ) {
     const isDeletedType = this.get('isDeleted');
     const requestedDeleteLocallyOnly = requestedDeleteType === 'markDeletedThisDevice';
@@ -1006,16 +1006,13 @@ export class MessageModel extends Model<MessageAttributes> {
       reaction: undefined,
       messageRequestResponse: undefined,
       errors: undefined,
-      unread: toSqliteBoolean(false),
+      ...(shouldMarkAsRead ? { unread: READ_MESSAGE_STATE.read } : {}),
     });
     // Only overwrite the messageHash when we are deleting globally.
     // This is because a locally deleted message should be able to be marked as deleted globally
     if (requestedDeleteGlobally) {
       this.set({ messageHash: undefined });
     }
-    // we can ignore the result of that markMessageReadNoCommit as it would only be used
-    // to refresh the expiry of it(but it is already marked as "deleted", so we don't care)
-    this.markMessageReadNoCommit(Date.now());
     await this.commit();
     // the line below makes sure that getNextExpiringMessage will find this message as expiring.
     // getNextExpiringMessage is used on app start to clean already expired messages which should have been removed already, but are not

ts/test/session/unit/models/MessageModel_test.ts

@@ -0,0 +1,85 @@
+import { expect } from 'chai';
+import Sinon from 'sinon';
+
+import { Data } from '../../../../data/data';
+import { deleteOrMarkAsDeletedMessages } from '../../../../interactions/conversations/deleteOrMarkAsDeletedMessages';
+import type { ConversationModel } from '../../../../models/conversation';
+import { READ_MESSAGE_STATE } from '../../../../models/conversationAttributes';
+import { MessageDeletedType } from '../../../../models/messageType';
+import { TestUtils } from '../../../test-utils';
+
+describe('deleteOrMarkAsDeletedMessages', () => {
+  beforeEach(() => {
+    TestUtils.stubWindowLog();
+  });
+
+  afterEach(() => {
+    Sinon.restore();
+  });
+
+  function makeUnreadMessage() {
+    const message = TestUtils.generateFakeIncomingPrivateMessage();
+    message.set({ unread: READ_MESSAGE_STATE.unread });
+
+    const refreshInMemoryDetails = Sinon.stub().resolves();
+    const updateLastMessage = Sinon.stub();
+    const saveMessage = Sinon.stub(Data, 'saveMessage').resolves(message.id);
+
+    Sinon.stub(message, 'getConversation').returns({
+      refreshInMemoryDetails,
+      updateLastMessage,
+    } as unknown as ConversationModel);
+
+    return { message, refreshInMemoryDetails, saveMessage, updateLastMessage };
+  }
+
+  it('preserves unread state when a message is deleted remotely', async () => {
+    const { message, refreshInMemoryDetails, saveMessage, updateLastMessage } = makeUnreadMessage();
+
+    await deleteOrMarkAsDeletedMessages({
+      conversation: {} as ConversationModel,
+      messages: [message],
+      deletionType: 'markDeletedGlobally',
+      actionContextIsUI: false,
+    });
+
+    expect(message.get('isDeleted')).to.equal(MessageDeletedType.deletedGlobally);
+    expect(message.get('unread')).to.equal(READ_MESSAGE_STATE.unread);
+    expect(saveMessage.calledOnce).to.equal(true);
+    expect(saveMessage.firstCall.firstArg.unread).to.equal(READ_MESSAGE_STATE.unread);
+    expect(refreshInMemoryDetails.calledOnce).to.equal(true);
+    expect(updateLastMessage.calledOnce).to.equal(true);
+  });
+
+  it('marks an unread message as read when it is deleted from the UI', async () => {
+    const { message, saveMessage } = makeUnreadMessage();
+
+    await deleteOrMarkAsDeletedMessages({
+      conversation: {} as ConversationModel,
+      messages: [message],
+      deletionType: 'markDeletedThisDevice',
+      actionContextIsUI: true,
+    });
+
+    expect(message.get('isDeleted')).to.equal(MessageDeletedType.deletedLocally);
+    expect(message.get('unread')).to.equal(READ_MESSAGE_STATE.read);
+    expect(saveMessage.calledOnce).to.equal(true);
+    expect(saveMessage.firstCall.firstArg.unread).to.equal(READ_MESSAGE_STATE.read);
+  });
+
+  it('does not start delete-after-read expiry when a message is deleted remotely', async () => {
+    const { message } = makeUnreadMessage();
+    message.set({ expirationType: 'deleteAfterRead', expireTimer: 60 });
+
+    await deleteOrMarkAsDeletedMessages({
+      conversation: {} as ConversationModel,
+      messages: [message],
+      deletionType: 'markDeletedGlobally',
+      actionContextIsUI: false,
+    });
+
+    expect(message.get('unread')).to.equal(READ_MESSAGE_STATE.unread);
+    expect(message.getExpirationStartTimestamp()).to.equal(undefined);
+    expect(message.getExpiresAt()).to.equal(undefined);
+  });
+});