Ignore delivery receipts for outgoing reactions

ts/messageModifiers/MessageReceipts.ts

@@ -105,6 +105,28 @@ const processReceiptBatcher = createWaitBatcher({
         // eslint-disable-next-line no-await-in-loop
         await window.Signal.Data.getMessagesBySentAt(sentAt);
 
+      if (messagesMatchingTimestamp.length === 0) {
+        // eslint-disable-next-line no-await-in-loop
+        const reaction = await window.Signal.Data.getReactionByTimestamp(
+          window.ConversationController.getOurConversationIdOrThrow(),
+          sentAt
+        );
+
+        if (reaction) {
+          for (const receipt of receiptsForMessageSentAt) {
+            log.info(
+              'MesageReceipts.processReceiptBatcher: Got receipt for reaction',
+              receipt.messageSentAt,
+              receipt.type,
+              receipt.sourceConversationId,
+              receipt.sourceServiceId
+            );
+            remove(receipt);
+          }
+          continue;
+        }
+      }
+
       for (const receipt of receiptsForMessageSentAt) {
         const targetMessage = getTargetMessage({
           sourceConversationId: receipt.sourceConversationId,

ts/models/messages.ts

@@ -42,6 +42,7 @@ import { getUserLanguages } from '../util/userLanguages';
 import { copyCdnFields } from '../util/attachments';
 
 import type { ReactionType } from '../types/Reactions';
+import { ReactionReadStatus } from '../types/Reactions';
 import type { ServiceIdString } from '../types/ServiceId';
 import { normalizeServiceId } from '../types/ServiceId';
 import { isAciString } from '../util/isAciString';
@@ -2615,13 +2616,6 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
             );
           }
           this.set({ reactions });
-
-          await window.Signal.Data.removeReactionFromConversation({
-            emoji: reaction.emoji,
-            fromId: reaction.fromId,
-            targetAuthorServiceId: reaction.targetAuthorAci,
-            targetTimestamp: reaction.targetTimestamp,
-          });
         } else {
           log.info(
             'handleReaction: adding reaction for message',
@@ -2648,8 +2642,19 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           if (isOutgoing(this.attributes) && isFromSomeoneElse) {
             void conversation.notify(this, reaction);
           }
+        }
+      }
 
-          await window.Signal.Data.addReaction({
+      if (reaction.remove) {
+        await window.Signal.Data.removeReactionFromConversation({
+          emoji: reaction.emoji,
+          fromId: reaction.fromId,
+          targetAuthorServiceId: reaction.targetAuthorAci,
+          targetTimestamp: reaction.targetTimestamp,
+        });
+      } else {
+        await window.Signal.Data.addReaction(
+          {
             conversationId: this.get('conversationId'),
             emoji: reaction.emoji,
             fromId: reaction.fromId,
@@ -2657,8 +2662,14 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
             messageReceivedAt: this.get('received_at'),
             targetAuthorAci: reaction.targetAuthorAci,
             targetTimestamp: reaction.targetTimestamp,
-          });
+            timestamp: reaction.timestamp,
-        }
+          },
+          {
+            readStatus: isFromThisDevice
+              ? ReactionReadStatus.Read
+              : ReactionReadStatus.Unread,
+          }
+        );
       }
 
       const currentLength = (this.get('reactions') || []).length;

ts/sql/Interface.ts

@@ -7,7 +7,7 @@ import type {
   SenderKeyInfoType,
 } from '../model-types.d';
 import type { StoredJob } from '../jobs/types';
-import type { ReactionType } from '../types/Reactions';
+import type { ReactionType, ReactionReadStatus } from '../types/Reactions';
 import type { ConversationColorType, CustomColorType } from '../types/Colors';
 import type { StorageAccessType } from '../types/Storage.d';
 import type { AttachmentType } from '../types/Attachment';
@@ -586,7 +586,16 @@ export type DataInterface = {
     targetAuthorServiceId: ServiceIdString;
     targetTimestamp: number;
   }) => Promise<void>;
-  addReaction: (reactionObj: ReactionType) => Promise<void>;
+  getReactionByTimestamp: (
+    fromId: string,
+    timestamp: number
+  ) => Promise<ReactionType | undefined>;
+  addReaction: (
+    reactionObj: ReactionType,
+    options: {
+      readStatus: ReactionReadStatus;
+    }
+  ) => Promise<void>;
   _getAllReactions: () => Promise<Array<ReactionType>>;
   _removeAllReactions: () => Promise<void>;
   getMessageBySender: (options: {

ts/sql/Server.ts

@@ -33,6 +33,7 @@ import * as Errors from '../types/errors';
 import { ReadStatus } from '../messages/MessageReadStatus';
 import type { GroupV2MemberType } from '../model-types.d';
 import type { ReactionType } from '../types/Reactions';
+import { ReactionReadStatus } from '../types/Reactions';
 import { STORAGE_UI_KEYS } from '../types/StorageUIKeys';
 import type { StoryDistributionIdString } from '../types/StoryDistributionId';
 import type { ServiceIdString, AciString } from '../types/ServiceId';
@@ -274,6 +275,7 @@ const dataInterface: ServerInterface = {
   getUnreadByConversationAndMarkRead,
   getUnreadReactionsAndMarkRead,
   markReactionAsRead,
+  getReactionByTimestamp,
   addReaction,
   removeReactionFromConversation,
   _getAllReactions,
@@ -2537,15 +2539,32 @@ async function markReactionAsRead(
   })();
 }
 
-async function addReaction({
+async function getReactionByTimestamp(
-  conversationId,
+  fromId: string,
-  emoji,
+  timestamp: number
-  fromId,
+): Promise<ReactionType | undefined> {
-  messageId,
+  const db = getReadonlyInstance();
-  messageReceivedAt,
+  const [query, params] = sql`
-  targetAuthorAci,
+    SELECT * FROM reactions
-  targetTimestamp,
+    WHERE fromId IS ${fromId} AND timestamp IS ${timestamp}
-}: ReactionType): Promise<void> {
+  `;
+
+  return db.prepare(query).get(params);
+}
+
+async function addReaction(
+  {
+    conversationId,
+    emoji,
+    fromId,
+    messageId,
+    messageReceivedAt,
+    targetAuthorAci,
+    targetTimestamp,
+    timestamp,
+  }: ReactionType,
+  { readStatus }: { readStatus: ReactionReadStatus }
+): Promise<void> {
   const db = await getWritableInstance();
   await db
     .prepare(
@@ -2557,6 +2576,7 @@ async function addReaction({
       messageReceivedAt,
       targetAuthorAci,
       targetTimestamp,
+      timestamp,
       unread
     ) VALUES (
       $conversationId,
@@ -2566,6 +2586,7 @@ async function addReaction({
       $messageReceivedAt,
       $targetAuthorAci,
       $targetTimestamp,
+      $timestamp,
       $unread
     );`
     )
@@ -2577,7 +2598,8 @@ async function addReaction({
       messageReceivedAt,
       targetAuthorAci,
       targetTimestamp,
-      unread: 1,
+      timestamp,
+      unread: readStatus === ReactionReadStatus.Unread ? 1 : 0,
     });
 }
 

ts/sql/migrations/980-reaction-timestamp.ts

@@ -0,0 +1,32 @@
+// Copyright 2023 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import type { Database } from '@signalapp/better-sqlite3';
+
+import type { LoggerType } from '../../types/Logging';
+
+export const version = 980;
+
+export function updateToSchemaVersion980(
+  currentVersion: number,
+  db: Database,
+  logger: LoggerType
+): void {
+  if (currentVersion >= 980) {
+    return;
+  }
+
+  db.transaction(() => {
+    db.exec(`
+      ALTER TABLE reactions ADD COLUMN timestamp NUMBER;
+
+      CREATE INDEX reactions_byTimestamp
+      ON reactions
+      (fromId, timestamp);
+    `);
+  })();
+
+  db.pragma('user_version = 980');
+
+  logger.info('updateToSchemaVersion980: success!');
+}

ts/sql/migrations/index.ts

@@ -72,10 +72,11 @@ import { updateToSchemaVersion930 } from './930-fts5-secure-delete';
 import { updateToSchemaVersion940 } from './940-fts5-revert';
 import { updateToSchemaVersion950 } from './950-fts5-secure-delete';
 import { updateToSchemaVersion960 } from './960-untag-pni';
+import { updateToSchemaVersion970 } from './970-fts5-optimize';
 import {
   version as MAX_VERSION,
-  updateToSchemaVersion970,
+  updateToSchemaVersion980,
-} from './970-fts5-optimize';
+} from './980-reaction-timestamp';
 
 function updateToSchemaVersion1(
   currentVersion: number,
@@ -2015,6 +2016,7 @@ export const SCHEMA_VERSIONS = [
   updateToSchemaVersion950,
   updateToSchemaVersion960,
   updateToSchemaVersion970,
+  updateToSchemaVersion980,
 ];
 
 export class DBVersionFromFutureError extends Error {

ts/test-electron/sql/markRead_test.ts

@@ -8,6 +8,7 @@ import dataInterface from '../../sql/Client';
 import { generateAci } from '../../types/ServiceId';
 
 import type { ReactionType } from '../../types/Reactions';
+import { ReactionReadStatus } from '../../types/Reactions';
 import { DurationInSeconds } from '../../util/durations';
 import type { MessageAttributesType } from '../../model-types.d';
 import { ReadStatus } from '../../messages/MessageReadStatus';
@@ -24,6 +25,8 @@ const {
   getUnreadReactionsAndMarkRead,
 } = dataInterface;
 
+const UNREAD_REACTION = { readStatus: ReactionReadStatus.Unread };
+
 describe('sql/markRead', () => {
   beforeEach(async () => {
     await _removeAllMessages();
@@ -528,6 +531,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message1.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction2: ReactionType = {
       conversationId,
@@ -537,6 +541,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message2.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction3: ReactionType = {
       conversationId: generateUuid(),
@@ -546,6 +551,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message3.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction4: ReactionType = {
       conversationId,
@@ -555,6 +561,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message4.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction5: ReactionType = {
       conversationId,
@@ -564,13 +571,14 @@ describe('sql/markRead', () => {
       messageReceivedAt: message5.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
 
-    await addReaction(reaction1);
+    await addReaction(reaction1, UNREAD_REACTION);
-    await addReaction(reaction2);
+    await addReaction(reaction2, UNREAD_REACTION);
-    await addReaction(reaction3);
+    await addReaction(reaction3, UNREAD_REACTION);
-    await addReaction(reaction4);
+    await addReaction(reaction4, UNREAD_REACTION);
-    await addReaction(reaction5);
+    await addReaction(reaction5, UNREAD_REACTION);
 
     assert.lengthOf(await _getAllReactions(), 5);
     const markedRead = await getUnreadReactionsAndMarkRead({
@@ -677,6 +685,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message1.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction2: ReactionType = {
       conversationId,
@@ -686,6 +695,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message2.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction3: ReactionType = {
       conversationId: generateUuid(),
@@ -695,6 +705,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message3.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction4: ReactionType = {
       conversationId,
@@ -704,6 +715,7 @@ describe('sql/markRead', () => {
       messageReceivedAt: message4.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
     const reaction5: ReactionType = {
       conversationId,
@@ -713,13 +725,14 @@ describe('sql/markRead', () => {
       messageReceivedAt: message5.received_at,
       targetAuthorAci: generateAci(),
       targetTimestamp: start,
+      timestamp: start,
     };
 
-    await addReaction(reaction1);
+    await addReaction(reaction1, UNREAD_REACTION);
-    await addReaction(reaction2);
+    await addReaction(reaction2, UNREAD_REACTION);
-    await addReaction(reaction3);
+    await addReaction(reaction3, UNREAD_REACTION);
-    await addReaction(reaction4);
+    await addReaction(reaction4, UNREAD_REACTION);
-    await addReaction(reaction5);
+    await addReaction(reaction5, UNREAD_REACTION);
 
     assert.lengthOf(await _getAllReactions(), 5);
     const markedRead = await getUnreadReactionsAndMarkRead({

ts/types/Reactions.ts

@@ -11,4 +11,10 @@ export type ReactionType = Readonly<{
   messageReceivedAt: number;
   targetAuthorAci: AciString;
   targetTimestamp: number;
+  timestamp: number;
 }>;
+
+export enum ReactionReadStatus {
+  Unread = 'Unread',
+  Read = 'Read',
+}