Improve message targeting for incoming reactions

ts/ConversationController.ts

@@ -17,7 +17,7 @@ import type { ConversationModel } from './models/conversations';
 import dataInterface from './sql/Client';
 import * as log from './logging/log';
 import * as Errors from './types/errors';
-import { getContactId } from './messages/helpers';
+import { getAuthorId } from './messages/helpers';
 import { maybeDeriveGroupV2Id } from './groups';
 import { assertDev, strictAssert } from './util/assert';
 import { drop } from './util/drop';
@@ -1236,7 +1236,7 @@ export class ConversationController {
     targetTimestamp: number
   ): Promise<ConversationModel | null | undefined> {
     const messages = await getMessagesBySentAt(targetTimestamp);
-    const targetMessage = messages.find(m => getContactId(m) === targetFromId);
+    const targetMessage = messages.find(m => getAuthorId(m) === targetFromId);
 
     if (targetMessage) {
       return this.get(targetMessage.conversationId);

ts/background.ts

@@ -59,7 +59,7 @@ import { RoutineProfileRefresher } from './routineProfileRefresh';
 import { isOlderThan } from './util/timestamp';
 import { isValidReactionEmoji } from './reactions/isValidReactionEmoji';
 import type { ConversationModel } from './models/conversations';
-import { getContact, isIncoming } from './messages/helpers';
+import { getAuthor, isIncoming } from './messages/helpers';
 import { migrateMessageData } from './messages/migrateMessageData';
 import { createBatcher } from './util/batcher';
 import {
@@ -2329,7 +2329,7 @@ export async function startApp(): Promise<void> {
     const message = initIncomingMessage(data, messageDescriptor);
 
     if (isIncoming(message.attributes)) {
-      const sender = getContact(message.attributes);
+      const sender = getAuthor(message.attributes);
       strictAssert(sender, 'MessageModel has no sender');
 
       const serviceIdKind = window.textsecure.storage.user.getOurServiceIdKind(
@@ -2395,7 +2395,7 @@ export async function startApp(): Promise<void> {
         fromId: fromConversation.id,
         remove: reaction.remove,
         source: ReactionSource.FromSomeoneElse,
-        storyReactionMessage: message,
+        generatedMessageForStoryReaction: message,
         targetAuthorAci,
         targetTimestamp: reaction.targetTimestamp,
         receivedAtDate: data.receivedAtDate,
@@ -2784,7 +2784,7 @@ export async function startApp(): Promise<void> {
         fromId: window.ConversationController.getOurConversationIdOrThrow(),
         remove: reaction.remove,
         source: ReactionSource.FromSync,
-        storyReactionMessage: message,
+        generatedMessageForStoryReaction: message,
         targetAuthorAci,
         targetTimestamp: reaction.targetTimestamp,
         receivedAtDate: data.receivedAtDate,

ts/messageModifiers/Deletes.ts

@@ -2,7 +2,7 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import type { MessageAttributesType } from '../model-types.d';
-import { getContactId } from '../messages/helpers';
+import { getAuthorId } from '../messages/helpers';
 import * as log from '../logging/log';
 import * as Errors from '../types/errors';
 import { deleteForEveryone } from '../util/deleteForEveryone';
@@ -32,7 +32,7 @@ export function forMessage(
 
   const matchingDeletes = deleteValues.filter(item => {
     return (
-      item.fromId === getContactId(messageAttributes) &&
+      item.fromId === getAuthorId(messageAttributes) &&
       sentTimestamps.has(item.targetSentTimestamp)
     );
   });
@@ -77,7 +77,7 @@ export async function onDelete(del: DeleteAttributesType): Promise<void> {
         );
 
         const targetMessage = messages.find(
-          m => del.fromId === getContactId(m) && !m.deletedForEveryone
+          m => del.fromId === getAuthorId(m) && !m.deletedForEveryone
         );
 
         if (!targetMessage) {

ts/messageModifiers/Edits.ts

@@ -5,7 +5,7 @@ import type { MessageAttributesType } from '../model-types.d';
 import * as Errors from '../types/errors';
 import * as log from '../logging/log';
 import { drop } from '../util/drop';
-import { getContactId } from '../messages/helpers';
+import { getAuthorId } from '../messages/helpers';
 import { handleEditMessage } from '../util/handleEditMessage';
 import { getMessageSentTimestamp } from '../util/getMessageSentTimestamp';
 import {
@@ -55,7 +55,7 @@ export function forMessage(
   const matchingEdits = editValues.filter(item => {
     return (
       item.targetSentTimestamp === sentAt &&
-      item.fromId === getContactId(messageAttributes)
+      item.fromId === getAuthorId(messageAttributes)
     );
   });
 
@@ -125,7 +125,7 @@ export async function onEdit(edit: EditAttributesType): Promise<void> {
         const targetMessage = messages.find(
           m =>
             edit.conversationId === m.conversationId &&
-            edit.fromId === getContactId(m)
+            edit.fromId === getAuthorId(m)
         );
 
         if (!targetMessage) {

ts/messageModifiers/Reactions.ts

@@ -2,17 +2,17 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import type { AciString } from '../types/ServiceId';
-import type { ConversationModel } from '../models/conversations';
 import type { MessageAttributesType } from '../model-types.d';
 import type { MessageModel } from '../models/messages';
 import type { ReactionSource } from '../reactions/ReactionSource';
 import * as Errors from '../types/errors';
 import * as log from '../logging/log';
-import { getContactId, getContact } from '../messages/helpers';
+import { getAuthor } from '../messages/helpers';
-import { getMessageIdForLogging } from '../util/idForLogging';
 import { getMessageSentTimestampSet } from '../util/getMessageSentTimestampSet';
-import { isDirectConversation, isMe } from '../util/whatTypeOfConversation';
+import { isMe } from '../util/whatTypeOfConversation';
-import { isOutgoing, isStory } from '../state/selectors/message';
+import { isStory } from '../state/selectors/message';
+import { getPropForTimestamp } from '../util/editHelpers';
+import { isSent } from '../messages/MessageSendState';
 import { strictAssert } from '../util/assert';
 
 export type ReactionAttributesType = {
@@ -22,9 +22,10 @@ export type ReactionAttributesType = {
   remove?: boolean;
   removeFromMessageReceiverCache: () => unknown;
   source: ReactionSource;
-  // Necessary to put 1:1 story replies into the right conversation - not the same
+  // If this is a reaction to a 1:1 story, we can use this message, generated from the
-  //   conversation as the target message!
+  //   reaction message itself. Necessary to put 1:1 story replies into the right
-  storyReactionMessage?: MessageModel;
+  //   conversation - not the same conversation as the target message!
+  generatedMessageForStoryReaction?: MessageModel;
   targetAuthorAci: AciString;
   targetTimestamp: number;
   timestamp: number;
@@ -38,70 +39,133 @@ function remove(reaction: ReactionAttributesType): void {
   reaction.removeFromMessageReceiverCache();
 }
 
-export function forMessage(
+export function findReactionsForMessage(
   message: MessageModel
 ): Array<ReactionAttributesType> {
-  const logId = `Reactions.forMessage(${getMessageIdForLogging(
+  const matchingReactions = Array.from(reactions.values()).filter(reaction => {
-    message.attributes
+    return isMessageAMatchForReaction({
-  )})`;
+      message: message.attributes,
-
+      targetTimestamp: reaction.targetTimestamp,
-  const reactionValues = Array.from(reactions.values());
+      targetAuthorAci: reaction.targetAuthorAci,
-  const sentTimestamps = getMessageSentTimestampSet(message.attributes);
+      reactionSenderConversationId: reaction.fromId,
-  if (isOutgoing(message.attributes)) {
-    const outgoingReactions = reactionValues.filter(item =>
-      sentTimestamps.has(item.targetTimestamp)
-    );
-
-    if (outgoingReactions.length > 0) {
-      log.info(`${logId}: Found early reaction for outgoing message`);
-      outgoingReactions.forEach(item => {
-        remove(item);
-      });
-      return outgoingReactions;
-    }
-  }
-
-  const senderId = getContactId(message.attributes);
-  const reactionsBySource = reactionValues.filter(re => {
-    const targetSender = window.ConversationController.lookupOrCreate({
-      serviceId: re.targetAuthorAci,
-      reason: logId,
     });
-    return (
-      targetSender?.id === senderId && sentTimestamps.has(re.targetTimestamp)
-    );
   });
 
-  if (reactionsBySource.length > 0) {
+  matchingReactions.forEach(reaction => remove(reaction));
-    log.info(`${logId}: Found early reaction for message`);
+  return matchingReactions;
-    reactionsBySource.forEach(item => {
-      remove(item);
-      item.removeFromMessageReceiverCache();
-    });
-    return reactionsBySource;
-  }
-
-  return [];
 }
 
-async function findMessage(
+async function findMessageForReaction({
-  targetTimestamp: number,
+  targetTimestamp,
-  targetConversationId: string
+  targetAuthorAci,
-): Promise<MessageAttributesType | undefined> {
+  reactionSenderConversationId,
+  logId,
+}: {
+  targetTimestamp: number;
+  targetAuthorAci: string;
+  reactionSenderConversationId: string;
+  logId: string;
+}): Promise<MessageAttributesType | undefined> {
   const messages = await window.Signal.Data.getMessagesBySentAt(
     targetTimestamp
   );
 
-  return messages.find(m => {
+  const matchingMessages = messages.filter(message =>
-    const contact = getContact(m);
+    isMessageAMatchForReaction({
+      message,
+      targetTimestamp,
+      targetAuthorAci,
+      reactionSenderConversationId,
+    })
+  );
 
-    if (!contact) {
+  if (!matchingMessages.length) {
+    return undefined;
+  }
+
+  if (matchingMessages.length > 1) {
+    // This could theoretically happen given limitations in the reaction proto but
+    // is very unlikely
+    log.warn(
+      `${logId}/findMessageForReaction: found ${matchingMessages.length} matching messages for the reaction!`
+    );
+  }
+
+  return matchingMessages[0];
+}
+
+function isMessageAMatchForReaction({
+  message,
+  targetTimestamp,
+  targetAuthorAci,
+  reactionSenderConversationId,
+}: {
+  message: MessageAttributesType;
+  targetTimestamp: number;
+  targetAuthorAci: string;
+  reactionSenderConversationId: string;
+}): boolean {
+  if (!getMessageSentTimestampSet(message).has(targetTimestamp)) {
+    return false;
+  }
+
+  const targetAuthorConversation =
+    window.ConversationController.get(targetAuthorAci);
+  const reactionSenderConversation = window.ConversationController.get(
+    reactionSenderConversationId
+  );
+
+  if (!targetAuthorConversation || !reactionSenderConversation) {
+    return false;
+  }
+
+  const author = getAuthor(message);
+  if (!author) {
+    return false;
+  }
+
+  if (author.id !== targetAuthorConversation.id) {
+    return false;
+  }
+
+  if (isMe(reactionSenderConversation.attributes)) {
+    // I am either the recipient or sender of all the messages I know about!
+    return true;
+  }
+
+  if (message.type === 'outgoing') {
+    const sendStateByConversationId = getPropForTimestamp({
+      log,
+      message,
+      prop: 'sendStateByConversationId',
+      targetTimestamp,
+    });
+
+    const sendState =
+      sendStateByConversationId?.[reactionSenderConversation.id];
+    if (!sendState) {
       return false;
     }
 
-    const mcid = contact.get('id');
+    return isSent(sendState.status);
-    return mcid === targetConversationId;
+  }
-  });
+
+  if (message.type === 'incoming') {
+    const messageConversation = window.ConversationController.get(
+      message.conversationId
+    );
+    if (!messageConversation) {
+      return false;
+    }
+
+    const reactionSenderServiceId = reactionSenderConversation.getServiceId();
+    return (
+      reactionSenderServiceId != null &&
+      messageConversation.hasMember(reactionSenderServiceId)
+    );
+  }
+
+  return true;
 }
 
 export async function onReaction(
@@ -112,36 +176,14 @@ export async function onReaction(
   const logId = `Reactions.onReaction(timestamp=${reaction.timestamp};target=${reaction.targetTimestamp})`;
 
   try {
-    // The conversation the target message was in; we have to find it in the database
+    const matchingMessage = await findMessageForReaction({
-    //   to to figure that out.
+      targetTimestamp: reaction.targetTimestamp,
-    const targetAuthorConversation =
+      targetAuthorAci: reaction.targetAuthorAci,
-      window.ConversationController.lookupOrCreate({
+      reactionSenderConversationId: reaction.fromId,
-        serviceId: reaction.targetAuthorAci,
+      logId,
-        reason: logId,
+    });
-      });
-    const targetConversationId = targetAuthorConversation?.id;
-    if (!targetConversationId) {
-      throw new Error(
-        `${logId} Error: No conversationId returned from lookupOrCreate!`
-      );
-    }
 
-    const generatedMessage = reaction.storyReactionMessage;
+    if (!matchingMessage) {
-    strictAssert(
-      generatedMessage,
-      `${logId} strictAssert: Story reactions must provide storyReactionMessage`
-    );
-    const fromConversation = window.ConversationController.get(
-      generatedMessage.get('conversationId')
-    );
-
-    let targetConversation: ConversationModel | undefined | null;
-
-    const targetMessageCheck = await findMessage(
-      reaction.targetTimestamp,
-      targetConversationId
-    );
-    if (!targetMessageCheck) {
       log.info(
         `${logId}: No message for reaction`,
         'targeting',
@@ -150,22 +192,11 @@ export async function onReaction(
       return;
     }
 
-    if (
+    const matchingMessageConversation = window.ConversationController.get(
-      fromConversation &&
+      matchingMessage.conversationId
-      isStory(targetMessageCheck) &&
+    );
-      isDirectConversation(fromConversation.attributes) &&
-      !isMe(fromConversation.attributes)
-    ) {
-      targetConversation = fromConversation;
-    } else {
-      targetConversation =
-        await window.ConversationController.getConversationForTargetMessage(
-          targetConversationId,
-          reaction.targetTimestamp
-        );
-    }
 
-    if (!targetConversation) {
+    if (!matchingMessageConversation) {
       log.info(
         `${logId}: No target conversation for reaction`,
         reaction.targetAuthorAci,
@@ -176,45 +207,52 @@ export async function onReaction(
     }
 
     // awaiting is safe since `onReaction` is never called from inside the queue
-    await targetConversation.queueJob('Reactions.onReaction', async () => {
+    await matchingMessageConversation.queueJob(
-      log.info(`${logId}: handling`);
+      'Reactions.onReaction',
+      async () => {
+        log.info(`${logId}: handling`);
 
-      // Thanks TS.
+        // Message is fetched inside the conversation queue so we have the
-      if (!targetConversation) {
+        // most recent data
-        remove(reaction);
+        const targetMessage = await findMessageForReaction({
-        return;
+          targetTimestamp: reaction.targetTimestamp,
-      }
+          targetAuthorAci: reaction.targetAuthorAci,
-
+          reactionSenderConversationId: reaction.fromId,
-      // Message is fetched inside the conversation queue so we have the
+          logId: `${logId}/conversationQueue`,
-      // most recent data
-      const targetMessage = await findMessage(
-        reaction.targetTimestamp,
-        targetConversationId
-      );
-
-      if (!targetMessage) {
-        remove(reaction);
-        return;
-      }
-
-      const message = window.MessageCache.__DEPRECATED$register(
-        targetMessage.id,
-        targetMessage,
-        'Reactions.onReaction'
-      );
-
-      // Use the generated message in ts/background.ts to create a message
-      // if the reaction is targeted at a story.
-      if (!isStory(targetMessage)) {
-        await message.handleReaction(reaction);
-      } else {
-        await generatedMessage.handleReaction(reaction, {
-          storyMessage: targetMessage,
         });
-      }
 
-      remove(reaction);
+        if (!targetMessage || targetMessage.id !== matchingMessage.id) {
-    });
+          log.warn(
+            `${logId}: message no longer a match for reaction! Maybe it's been deleted?`
+          );
+          remove(reaction);
+          return;
+        }
+
+        const targetMessageModel = window.MessageCache.__DEPRECATED$register(
+          targetMessage.id,
+          targetMessage,
+          'Reactions.onReaction'
+        );
+
+        // Use the generated message in ts/background.ts to create a message
+        // if the reaction is targeted at a story.
+        if (!isStory(targetMessage)) {
+          await targetMessageModel.handleReaction(reaction);
+        } else {
+          const generatedMessage = reaction.generatedMessageForStoryReaction;
+          strictAssert(
+            generatedMessage,
+            'Generated message must exist for story reaction'
+          );
+          await generatedMessage.handleReaction(reaction, {
+            storyMessage: targetMessage,
+          });
+        }
+
+        remove(reaction);
+      }
+    );
   } catch (error) {
     remove(reaction);
     log.error(`${logId} error:`, Errors.toLogFormat(error));

ts/messages/helpers.ts

@@ -132,11 +132,11 @@ export function isQuoteAMatch(
   return (
     isSameTimestamp &&
     message.conversationId === conversationId &&
-    getContactId(message) === authorConversation?.id
+    getAuthorId(message) === authorConversation?.id
   );
 }
 
-export function getContactId(
+export function getAuthorId(
   message: Pick<MessageAttributesType, 'type' | 'source' | 'sourceServiceId'>
 ): string | undefined {
   const source = getSource(message);
@@ -149,15 +149,15 @@ export function getContactId(
   const conversation = window.ConversationController.lookupOrCreate({
     e164: source,
     serviceId: sourceServiceId,
-    reason: 'helpers.getContactId',
+    reason: 'helpers.getAuthorId',
   });
   return conversation?.id;
 }
 
-export function getContact(
+export function getAuthor(
   message: MessageAttributesType
 ): ConversationModel | undefined {
-  const id = getContactId(message);
+  const id = getAuthorId(message);
   return window.ConversationController.get(id);
 }
 

ts/models/conversations.ts

@@ -51,7 +51,7 @@ import type {
   CustomColorType,
 } from '../types/Colors';
 import type { MessageModel } from './messages';
-import { getContact } from '../messages/helpers';
+import { getAuthor } from '../messages/helpers';
 import { strictAssert } from '../util/assert';
 import { isConversationMuted } from '../util/isConversationMuted';
 import { isConversationSMSOnly } from '../util/isConversationSMSOnly';
@@ -5153,7 +5153,7 @@ export class ConversationModel extends window.Backbone
 
     const sender = reaction
       ? window.ConversationController.get(reaction.fromId)
-      : getContact(message.attributes);
+      : getAuthor(message.attributes);
     const senderName = sender
       ? sender.getTitle()
       : window.i18n('icu:unknownContact');

ts/models/messages.ts

@@ -113,12 +113,12 @@ import type {
 import * as log from '../logging/log';
 import { cleanupMessage, deleteMessageData } from '../util/cleanup';
 import {
-  getContact,
   getSource,
   getSourceServiceId,
   isCustomError,
   messageHasPaymentEvent,
   isQuoteAMatch,
+  getAuthor,
 } from '../messages/helpers';
 import { viewOnceOpenJobQueue } from '../jobs/viewOnceOpenJobQueue';
 import { getMessageIdForLogging } from '../util/idForLogging';
@@ -1625,7 +1625,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     const type = message.get('type');
     const conversationId = message.get('conversationId');
 
-    const fromContact = getContact(this.attributes);
+    const fromContact = getAuthor(this.attributes);
     if (fromContact) {
       fromContact.setRegistered();
     }
@@ -1751,6 +1751,8 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           return;
         }
         if (existingMessage) {
+          // TODO: (DESKTOP-7301): improve this check in case previous message is not yet
+          // registered in memory
           log.warn(
             `${idLog}: Received duplicate transcript for message ${message.idForLogging()}, but it was not an update transcript. Dropping.`
           );
@@ -2477,7 +2479,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           );
         }
 
-        const generatedMessage = reaction.storyReactionMessage;
+        const generatedMessage = reaction.generatedMessageForStoryReaction;
         strictAssert(
           generatedMessage,
           'Story reactions must provide storyReactionMessage'
@@ -2668,7 +2670,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
           'New story reaction must have an emoji'
         );
 
-        const generatedMessage = reaction.storyReactionMessage;
+        const generatedMessage = reaction.generatedMessageForStoryReaction;
         strictAssert(
           generatedMessage,
           'Story reactions must provide storyReactionmessage'

ts/reactions/enqueueReactionForSend.ts

@@ -121,7 +121,7 @@ export async function enqueueReactionForSend({
     fromId: window.ConversationController.getOurConversationIdOrThrow(),
     remove,
     source: ReactionSource.FromThisDevice,
-    storyReactionMessage,
+    generatedMessageForStoryReaction: storyReactionMessage,
     targetAuthorAci,
     targetTimestamp,
     receivedAtDate: timestamp,

ts/state/ducks/composer.ts

@@ -70,7 +70,7 @@ import { shouldShowInvalidMessageToast } from '../../util/shouldShowInvalidMessa
 import { writeDraftAttachment } from '../../util/writeDraftAttachment';
 import { __DEPRECATED$getMessageById } from '../../messages/getMessageById';
 import { canReply, isNormalBubble } from '../selectors/message';
-import { getContactId } from '../../messages/helpers';
+import { getAuthorId } from '../../messages/helpers';
 import { getConversationSelector } from '../selectors/conversations';
 import { enqueueReactionForSend } from '../../reactions/enqueueReactionForSend';
 import { useBoundActions } from '../../hooks/useBoundActions';
@@ -341,7 +341,7 @@ function scrollToQuotedMessage({
       Boolean(
         item.conversationId === conversationId &&
           authorId &&
-          getContactId(item) === authorId
+          getAuthorId(item) === authorId
       )
     );
 

ts/state/selectors/message.ts

@@ -244,7 +244,7 @@ export type GetContactOptions = Pick<
   'conversationSelector' | 'ourConversationId' | 'ourNumber' | 'ourAci'
 >;
 
-export function getContactId(
+export function getAuthorId(
   message: MessageWithUIFieldsType,
   {
     conversationSelector,
@@ -704,7 +704,7 @@ export const getPropsForMessage = (
     (message.reactions || []).find(re => re.fromId === ourConversationId) || {}
   ).emoji;
 
-  const authorId = getContactId(message, {
+  const authorId = getAuthorId(message, {
     conversationSelector,
     ourConversationId,
     ourNumber,
@@ -2096,7 +2096,7 @@ export const getMessageDetails = createSelector(
     let conversationIds: Array<string>;
     if (isIncoming(message)) {
       conversationIds = [
-        getContactId(message, {
+        getAuthorId(message, {
           conversationSelector,
           ourConversationId,
           ourNumber,

ts/test-electron/models/messages_test.ts

@@ -18,7 +18,7 @@ import enMessages from '../../../_locales/en/messages.json';
 import { SendStatus } from '../../messages/MessageSendState';
 import { SignalService as Proto } from '../../protobuf';
 import { generateAci } from '../../types/ServiceId';
-import { getContact } from '../../messages/helpers';
+import { getAuthor } from '../../messages/helpers';
 import { setupI18n } from '../../util/setupI18n';
 import {
   APPLICATION_JSON,
@@ -237,7 +237,7 @@ describe('Message', () => {
   describe('getContact', () => {
     it('gets outgoing contact', () => {
       const message = createMessage(attributes);
-      assert.exists(getContact(message.attributes));
+      assert.exists(getAuthor(message.attributes));
     });
 
     it('gets incoming contact', () => {
@@ -245,7 +245,7 @@ describe('Message', () => {
         type: 'incoming',
         source,
       });
-      assert.exists(getContact(message.attributes));
+      assert.exists(getAuthor(message.attributes));
     });
   });
 

ts/test-mock/helpers.ts

@@ -1,7 +1,15 @@
 // Copyright 2023 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
+import {
+  type Device,
+  type Group,
+  PrimaryDevice,
+  type Proto,
+  StorageState,
+} from '@signalapp/mock-server';
 import { assert } from 'chai';
+import Long from 'long';
 import type { Locator, Page } from 'playwright';
 import { expect } from 'playwright/test';
 
@@ -85,3 +93,187 @@ export async function expectSystemMessages(
     expected
   );
 }
+
+function getDevice(author: PrimaryDevice | Device): Device {
+  return author instanceof PrimaryDevice ? author.device : author;
+}
+
+type GroupInfo = {
+  group: Group;
+  members: Array<PrimaryDevice>;
+};
+
+function maybeWrapInSyncMessage({
+  isSync,
+  to,
+  sentTo,
+  dataMessage,
+}: {
+  isSync: boolean;
+  to: PrimaryDevice | Device;
+  sentTo?: Array<PrimaryDevice | Device>;
+  dataMessage: Proto.IDataMessage;
+}): Proto.IContent {
+  return isSync
+    ? {
+        syncMessage: {
+          sent: {
+            destinationServiceId: getDevice(to).aci,
+            message: dataMessage,
+            timestamp: dataMessage.timestamp,
+            unidentifiedStatus: (sentTo ?? [to]).map(contact => ({
+              destinationServiceId: getDevice(contact).aci,
+              destination: getDevice(contact).number,
+            })),
+          },
+        },
+      }
+    : { dataMessage };
+}
+
+function isToGroup(to: Device | PrimaryDevice | GroupInfo): to is GroupInfo {
+  return 'group' in to;
+}
+
+export function sendTextMessage({
+  from,
+  to,
+  text,
+  desktop,
+  timestamp = Date.now(),
+}: {
+  from: PrimaryDevice;
+  to: PrimaryDevice | Device | GroupInfo;
+  text: string;
+  desktop: Device;
+  timestamp?: number;
+}): Promise<void> {
+  const isSync = from.secondaryDevices.includes(desktop);
+  const toDevice = isSync || isToGroup(to) ? desktop : getDevice(to);
+  const groupInfo = isToGroup(to) ? to : undefined;
+  return from.sendRaw(
+    toDevice,
+    maybeWrapInSyncMessage({
+      isSync,
+      to: to as PrimaryDevice,
+      dataMessage: {
+        body: text,
+        timestamp: Long.fromNumber(timestamp),
+        groupV2: groupInfo
+          ? {
+              masterKey: groupInfo.group.masterKey,
+              revision: groupInfo.group.revision,
+            }
+          : undefined,
+      },
+      sentTo: groupInfo ? groupInfo.members : [to as PrimaryDevice | Device],
+    }),
+    { timestamp }
+  );
+}
+
+export function sendReaction({
+  from,
+  to,
+  targetAuthor,
+  targetMessageTimestamp,
+  emoji = '👍',
+  reactionTimestamp = Date.now(),
+  desktop,
+}: {
+  from: PrimaryDevice;
+  to: PrimaryDevice | Device;
+  targetAuthor: PrimaryDevice | Device;
+  targetMessageTimestamp: number;
+  emoji: string;
+  reactionTimestamp?: number;
+  desktop: Device;
+}): Promise<void> {
+  const isSync = from.secondaryDevices.includes(desktop);
+  return from.sendRaw(
+    isSync ? desktop : getDevice(to),
+    maybeWrapInSyncMessage({
+      isSync,
+      to,
+      dataMessage: {
+        timestamp: Long.fromNumber(reactionTimestamp),
+        reaction: {
+          emoji,
+          targetAuthorAci: getDevice(targetAuthor).aci,
+          targetTimestamp: Long.fromNumber(targetMessageTimestamp),
+        },
+      },
+    }),
+    {
+      timestamp: reactionTimestamp,
+    }
+  );
+}
+
+async function getStorageState(phone: PrimaryDevice) {
+  return (await phone.getStorageState()) ?? StorageState.getEmpty();
+}
+
+export async function createGroup(
+  phone: PrimaryDevice,
+  otherMembers: Array<PrimaryDevice>,
+  groupTitle: string
+): Promise<Group> {
+  const group = await phone.createGroup({
+    title: groupTitle,
+    members: [phone, ...otherMembers],
+  });
+  let state = await getStorageState(phone);
+
+  state = state
+    .addGroup(group, {
+      whitelisted: true,
+    })
+    .pinGroup(group);
+
+  // Finally whitelist and pin contacts
+  for (const member of otherMembers) {
+    state = state.addContact(member, {
+      whitelisted: true,
+      serviceE164: member.device.number,
+      identityKey: member.publicKey.serialize(),
+      profileKey: member.profileKey.serialize(),
+      givenName: member.profileName,
+    });
+  }
+  await phone.setStorageState(state);
+  return group;
+}
+
+export async function clickOnConversation(
+  page: Page,
+  contact: PrimaryDevice
+): Promise<void> {
+  const leftPane = page.locator('#LeftPane');
+  await leftPane.getByTestId(contact.device.aci).click();
+}
+export async function pinContact(
+  phone: PrimaryDevice,
+  contact: PrimaryDevice
+): Promise<void> {
+  const state = await getStorageState(phone);
+  state.pin(contact);
+  await phone.setStorageState(state);
+}
+
+export function acceptConversation(page: Page): Promise<void> {
+  return page
+    .locator('.module-message-request-actions button >> "Accept"')
+    .click();
+}
+
+export function getTimeline(page: Page): Locator {
+  return page.locator('.module-timeline__messages__container');
+}
+
+export function getMessageInTimelineByTimestamp(
+  page: Page,
+  timestamp: number
+): Locator {
+  return getTimeline(page).getByTestId(`${timestamp}`);
+}

ts/test-mock/messaging/reaction_test.ts

@@ -0,0 +1,353 @@
+// Copyright 2024 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import createDebug from 'debug';
+import { StorageState } from '@signalapp/mock-server';
+import { type Page } from 'playwright';
+import { expect } from 'playwright/test';
+import { assert } from 'chai';
+
+import type { App } from '../playwright';
+import { Bootstrap } from '../bootstrap';
+import { MINUTE } from '../../util/durations';
+import { strictAssert } from '../../util/assert';
+import {
+  clickOnConversation,
+  getMessageInTimelineByTimestamp,
+  sendTextMessage,
+  sendReaction,
+  createGroup,
+} from '../helpers';
+
+export const debug = createDebug('mock:test:reactions');
+
+async function getReactionsForMessage(page: Page, timestamp: number) {
+  const reactionsByEmoji: Record<string, Array<string>> = {};
+
+  try {
+    const message = await getMessageInTimelineByTimestamp(page, timestamp);
+
+    await message.locator('.module-message__reactions').click();
+
+    const reactionRows = await page
+      .locator('.module-reaction-viewer__body__row')
+      .all();
+
+    for (const row of reactionRows) {
+      // eslint-disable-next-line no-await-in-loop
+      const emoji = await row.locator('img').getAttribute('title');
+      // eslint-disable-next-line no-await-in-loop
+      const reactor = await row
+        .locator('.module-reaction-viewer__body__row__name')
+        .innerText();
+
+      strictAssert(emoji, 'emoji must exist');
+      reactionsByEmoji[emoji] = (reactionsByEmoji[emoji] ?? []).concat([
+        reactor,
+      ]);
+    }
+    // click away
+    await page.getByText("chat history isn't transferred").click();
+  } catch {
+    // pass
+  }
+  return reactionsByEmoji;
+}
+
+async function expectMessageToHaveReactions(
+  page: Page,
+  timestamp: number,
+  reactionsBySender: Record<string, Array<string>>,
+  options?: { timeout: number }
+): Promise<void> {
+  return expect(async () => {
+    assert.deepEqual(
+      await getReactionsForMessage(page, timestamp),
+      reactionsBySender
+    );
+  }).toPass({ timeout: options?.timeout ?? 10000 });
+}
+
+describe('reactions', function (this: Mocha.Suite) {
+  let bootstrap: Bootstrap;
+  let app: App;
+
+  this.timeout(MINUTE);
+  beforeEach(async () => {
+    bootstrap = new Bootstrap();
+    await bootstrap.init();
+
+    const { phone, contacts } = bootstrap;
+    const [alice, bob, charlie] = contacts;
+    let state = StorageState.getEmpty();
+
+    state = state.addContact(alice, {
+      identityKey: alice.publicKey.serialize(),
+      profileKey: alice.profileKey.serialize(),
+    });
+    state = state.addContact(bob, {
+      identityKey: bob.publicKey.serialize(),
+      profileKey: bob.profileKey.serialize(),
+    });
+    state = state.addContact(charlie, {
+      identityKey: charlie.publicKey.serialize(),
+      profileKey: charlie.profileKey.serialize(),
+    });
+
+    await phone.setStorageState(state);
+
+    app = await bootstrap.link();
+  });
+
+  afterEach(async function (this: Mocha.Context) {
+    if (!bootstrap) {
+      return;
+    }
+
+    await bootstrap.maybeSaveLogs(this.currentTest, app);
+    await app.close();
+    await bootstrap.teardown();
+  });
+
+  it('should correctly match on participant, timestamp, and author in 1:1 conversation', async () => {
+    this.timeout(10000);
+    const { contacts, phone, desktop } = bootstrap;
+    const [alice, bob, charlie] = contacts;
+
+    const window = await app.getWindow();
+
+    const alice1on1Timestamp = Date.now();
+    const outgoingTimestamp = alice1on1Timestamp;
+
+    await sendTextMessage({
+      from: alice,
+      to: desktop,
+      text: 'hi from alice',
+      timestamp: alice1on1Timestamp,
+      desktop,
+    });
+
+    // To test the case where we have different outgoing messages with the same
+    // timestamps, we need to send these without awaiting since otherwise desktop will
+    // drop them since they have the same timestamp (DESKTOP-7301)
+    await Promise.all([
+      sendTextMessage({
+        from: phone,
+        to: bob,
+        text: 'hi bob',
+        timestamp: outgoingTimestamp,
+        desktop,
+      }),
+
+      sendTextMessage({
+        from: phone,
+        to: charlie,
+        text: 'hi charlie',
+        timestamp: outgoingTimestamp,
+        desktop,
+      }),
+    ]);
+
+    // [❌ invalid reaction] bob trying to trick us by reacting to a message in a
+    // conversation he's not a part of
+    await sendReaction({
+      from: bob,
+      to: desktop,
+      emoji: '👻',
+      targetAuthor: alice,
+      targetMessageTimestamp: alice1on1Timestamp,
+      desktop,
+    });
+
+    // [❌ invalid reaction] phone sending message with wrong author but right timestamp
+    await sendReaction({
+      from: phone,
+      to: desktop,
+      emoji: '💀',
+      targetAuthor: bob,
+      targetMessageTimestamp: alice1on1Timestamp,
+      desktop,
+    });
+
+    // [✅ incoming message] alice reacting to her own message
+    await sendReaction({
+      from: alice,
+      to: desktop,
+      emoji: '👍',
+      targetAuthor: alice,
+      targetMessageTimestamp: alice1on1Timestamp,
+      desktop,
+    });
+
+    await clickOnConversation(window, alice);
+    await expectMessageToHaveReactions(window, alice1on1Timestamp, {
+      '👍': [alice.profileName],
+    });
+
+    // [✅ incoming message] phone sending message with right author
+    await sendReaction({
+      from: phone,
+      to: alice,
+      emoji: '👋',
+      targetAuthor: alice,
+      targetMessageTimestamp: alice1on1Timestamp,
+      desktop,
+    });
+
+    await expectMessageToHaveReactions(window, alice1on1Timestamp, {
+      '👍': [alice.profileName],
+      '👋': ['You'],
+    });
+
+    // now, receive reactions from those messages with same timestamp
+    // [✅ outgoing message] bob reacting to our message
+    await sendReaction({
+      from: bob,
+      to: desktop,
+      emoji: '👋',
+      targetAuthor: phone,
+      targetMessageTimestamp: outgoingTimestamp,
+      desktop,
+    });
+
+    // [✅ outgoing message] alice reacting to our message
+    await sendReaction({
+      from: charlie,
+      to: desktop,
+      emoji: '👋',
+      targetAuthor: phone,
+      targetMessageTimestamp: outgoingTimestamp,
+      desktop,
+    });
+
+    await clickOnConversation(window, bob);
+    await expectMessageToHaveReactions(window, outgoingTimestamp, {
+      '👋': [bob.profileName],
+    });
+
+    await clickOnConversation(window, charlie);
+    await expectMessageToHaveReactions(window, outgoingTimestamp, {
+      '👋': [charlie.profileName],
+    });
+  });
+
+  it('should correctly match on participant, timestamp, and author in group conversation', async () => {
+    this.timeout(10000);
+
+    const { contacts, phone, desktop } = bootstrap;
+    const [alice, bob, charlie, danielle] = contacts;
+
+    const groupMembers = [alice, bob, charlie];
+    const groupForSending = {
+      group: await createGroup(phone, groupMembers, 'ReactionGroup'),
+      members: groupMembers,
+    };
+
+    const window = await app.getWindow();
+    const leftPane = window.locator('#LeftPane');
+
+    const now = Date.now();
+    const myGroupTimestamp = now;
+    const aliceGroupTimestamp = now + 1;
+    const bobGroupTimestamp = now + 2;
+    const charlieGroupTimestamp = now + 3;
+
+    // [✅ outgoing message]: charlie reacting to bob's group message, early
+    await sendReaction({
+      from: charlie,
+      to: desktop,
+      emoji: '👋',
+      targetAuthor: bob,
+      targetMessageTimestamp: bobGroupTimestamp,
+      desktop,
+    });
+
+    // Send a bunch of messages in the group
+    await sendTextMessage({
+      from: phone,
+      to: groupForSending,
+      text: "hello group, it's me",
+      timestamp: myGroupTimestamp,
+      desktop,
+    });
+
+    await sendTextMessage({
+      from: alice,
+      to: groupForSending,
+      text: "hello group, it's alice",
+      timestamp: aliceGroupTimestamp,
+      desktop,
+    });
+
+    await sendTextMessage({
+      from: bob,
+      to: groupForSending,
+      text: "hello group, it's bob",
+      timestamp: bobGroupTimestamp,
+      desktop,
+    });
+
+    await sendTextMessage({
+      from: charlie,
+      to: groupForSending,
+      text: "hello group, it's charlie",
+      timestamp: charlieGroupTimestamp,
+      desktop,
+    });
+
+    await leftPane.getByText('ReactionGroup').click();
+
+    // [❌ invalid reaction] danielle reacting to our group message, but she's not in the
+    // group!
+    await sendReaction({
+      from: danielle,
+      to: desktop,
+      emoji: '👻',
+      targetAuthor: phone,
+      targetMessageTimestamp: myGroupTimestamp,
+      desktop,
+    });
+
+    // [✅ outgoing message]: alice reacting to our group message
+    await sendReaction({
+      from: alice,
+      to: desktop,
+      emoji: '👍',
+      targetAuthor: phone,
+      targetMessageTimestamp: myGroupTimestamp,
+      desktop,
+    });
+
+    // [✅ outgoing message]: bob reacting to our group message
+    await sendReaction({
+      from: bob,
+      to: desktop,
+      emoji: '👍',
+      targetAuthor: phone,
+      targetMessageTimestamp: myGroupTimestamp,
+      desktop,
+    });
+
+    // [✅ outgoing message]: charlie reacting to alice's group message
+    await sendReaction({
+      from: charlie,
+      to: desktop,
+      emoji: '😛',
+      targetAuthor: alice,
+      targetMessageTimestamp: aliceGroupTimestamp,
+      desktop,
+    });
+
+    await expectMessageToHaveReactions(window, myGroupTimestamp, {
+      '👍': [bob.profileName, alice.profileName],
+    });
+
+    await expectMessageToHaveReactions(window, aliceGroupTimestamp, {
+      '😛': [charlie.profileName],
+    });
+
+    await expectMessageToHaveReactions(window, bobGroupTimestamp, {
+      '👋': [charlie.profileName],
+    });
+  });
+});

ts/util/deleteForEveryone.ts

@@ -5,7 +5,7 @@ import type { DeleteAttributesType } from '../messageModifiers/Deletes';
 import type { MessageModel } from '../models/messages';
 import * as log from '../logging/log';
 import { isMe } from './whatTypeOfConversation';
-import { getContactId } from '../messages/helpers';
+import { getAuthorId } from '../messages/helpers';
 import { isStory } from '../state/selectors/message';
 import { isTooOldToModifyMessage } from './isTooOldToModifyMessage';
 
@@ -54,7 +54,7 @@ function isDeletionByMe(
   const ourConversationId =
     window.ConversationController.getOurConversationIdOrThrow();
   return (
-    getContactId(message.attributes) === ourConversationId &&
+    getAuthorId(message.attributes) === ourConversationId &&
     doe.fromId === ourConversationId
   );
 }

ts/util/findStoryMessage.ts

@@ -8,7 +8,7 @@ import type { AciString } from '../types/ServiceId';
 import * as log from '../logging/log';
 import { normalizeAci } from './normalizeAci';
 import { filter } from './iterables';
-import { getContactId } from '../messages/helpers';
+import { getAuthorId } from '../messages/helpers';
 import { getTimestampFromLong } from './timestampLongUtils';
 
 export async function findStoryMessages(
@@ -89,7 +89,7 @@ function isStoryAMatch(
 
   return (
     message.sent_at === sentTimestamp &&
-    getContactId(message) === authorConversation?.id &&
+    getAuthorId(message) === authorConversation?.id &&
     (message.conversationId === conversationId ||
       message.conversationId === ourConversationId)
   );

ts/util/getNotificationDataForMessage.ts

@@ -48,7 +48,7 @@ import {
   isMessageRequestResponse,
 } from '../state/selectors/message';
 import {
-  getContact,
+  getAuthor,
   messageHasPaymentEvent,
   getPaymentEventNotificationText,
 } from '../messages/helpers';
@@ -260,7 +260,7 @@ export function getNotificationDataForMessage(
 
   if (isGroupUpdate(attributes)) {
     const { group_update: groupUpdate } = attributes;
-    const fromContact = getContact(attributes);
+    const fromContact = getAuthor(attributes);
     const messages = [];
     if (!groupUpdate) {
       throw new Error('getNotificationData: Missing group_update');
@@ -499,7 +499,7 @@ export function getNotificationDataForMessage(
       };
     }
 
-    const fromContact = getContact(attributes);
+    const fromContact = getAuthor(attributes);
     const sender = fromContact?.getTitle() ?? window.i18n('icu:unknownContact');
     return {
       emoji,

ts/util/makeQuote.ts

@@ -10,7 +10,7 @@ import type { MIMEType } from '../types/MIME';
 import type { LinkPreviewType } from '../types/message/LinkPreviews';
 import type { StickerType } from '../types/Stickers';
 import { IMAGE_JPEG, IMAGE_GIF } from '../types/MIME';
-import { getContact } from '../messages/helpers';
+import { getAuthor } from '../messages/helpers';
 import { getQuoteBodyText } from './getQuoteBodyText';
 import { isGIF } from '../types/Attachment';
 import { isGiftBadge, isTapToView } from '../state/selectors/message';
@@ -22,7 +22,7 @@ import { getMessageSentTimestamp } from './getMessageSentTimestamp';
 export async function makeQuote(
   quotedMessage: MessageAttributesType
 ): Promise<QuotedMessageType> {
-  const contact = getContact(quotedMessage);
+  const contact = getAuthor(quotedMessage);
 
   strictAssert(contact, 'makeQuote: no contact');
 

ts/util/modifyTargetMessage.ts

@@ -220,12 +220,16 @@ export async function modifyTargetMessage(
   }
 
   // Does message message have any pending, previously-received associated reactions?
-  const reactions = Reactions.forMessage(message);
+  const reactions = Reactions.findReactionsForMessage(message);
+
+  log.info(
+    `${logId}: Found ${reactions.length} early reaction(s) for ${message.attributes.type} message`
+  );
   await Promise.all(
     reactions.map(async reaction => {
       if (isStory(message.attributes)) {
         // We don't set changed = true here, because we don't modify the original story
-        const generatedMessage = reaction.storyReactionMessage;
+        const generatedMessage = reaction.generatedMessageForStoryReaction;
         strictAssert(
           generatedMessage,
           'Story reactions must provide storyReactionMessage'