Pass pairs of message data and raw message info to notif generation code

Summary: This differential modifies notif generation code to take pairs of message data and raw message info instead of message data alone to avoid issue of duplicated raw message info with different message id. Test Plan: Ensure that [those steps](https://linear.app/comm/issue/ENG-9344/duplicated-messages-on-native-in-dm-threads) no longer introduce the bug. Reviewers: tomek, kamil, ashoat Reviewed By: ashoat Differential Revision: https://phab.comm.dev/D13417
CommE2E · Sep 20, 2024 · cc116bc · cc116bc
1 parent b74fa44
commit cc116bc
Show file tree

Hide file tree

Showing 19 changed files with 327 additions and 225 deletions.
diff --git a/lib/push/send-hooks.react.js b/lib/push/send-hooks.react.js
@@ -129,7 +129,8 @@ function useSendPushNotifs(): (
         senderUserID,
         senderDeviceDescriptor,
       };
-      const { messageDatas, rescindData, badgeUpdateData } = notifCreationData;
+      const { messageDatasWithMessageInfos, rescindData, badgeUpdateData } =
+        notifCreationData;
 
       const pushNotifsPreparationInput = {
         encryptedNotifUtilsAPI,
@@ -138,7 +139,7 @@ function useSendPushNotifs(): (
         messageInfos: rawMessageInfos,
         thickRawThreadInfos,
         auxUserInfos,
-        messageDatas,
+        messageDatasWithMessageInfos,
         userInfos,
         getENSNames,
         getFCNames,

diff --git a/lib/push/send-utils.js b/lib/push/send-utils.js
@@ -23,7 +23,6 @@ import { createWebNotification } from './web-notif-creators.js';
 import { createWNSNotification } from './wns-notif-creators.js';
 import { hasPermission } from '../permissions/minimally-encoded-thread-permissions.js';
 import {
-  rawMessageInfoFromMessageData,
   createMessageInfo,
   shimUnsupportedRawMessageInfos,
   sortMessageInfoList,
@@ -48,7 +47,6 @@ import {
 import {
   type MessageData,
   type RawMessageInfo,
-  messageDataLocalID,
 } from '../types/message-types.js';
 import type { ThreadInfo } from '../types/minimally-encoded-thread-permissions-types.js';
 import type {
@@ -114,21 +112,26 @@ async function getPushUserInfo(
   messageInfos: { +[id: string]: RawMessageInfo },
   thickRawThreadInfos: ThickRawThreadInfos,
   auxUserInfos: AuxUserInfos,
-  messageDatas: ?$ReadOnlyArray<MessageData>,
+  messageDataWithMessageInfos: ?$ReadOnlyArray<{
+    +messageData: MessageData,
+    +rawMessageInfo: RawMessageInfo,
+  }>,
 ): Promise<{
   +pushInfos: ?PushInfo,
   +rescindInfos: ?PushInfo,
 }> {
-  if (!messageDatas || messageDatas.length === 0) {
+  if (!messageDataWithMessageInfos) {
     return { pushInfos: null, rescindInfos: null };
   }
 
   const threadsToMessageIndices: Map<string, number[]> = new Map();
   const newMessageInfos: RawMessageInfo[] = [];
+  const messageDatas: MessageData[] = [];
 
   let nextNewMessageIndex = 0;
-  for (let i = 0; i < messageDatas.length; i++) {
-    const messageData = messageDatas[i];
+  for (const messageDataWithInfo of messageDataWithMessageInfos) {
+    const { messageData, rawMessageInfo } = messageDataWithInfo;
+
     const threadID = messageData.threadID;
 
     let messageIndices = threadsToMessageIndices.get(threadID);
@@ -139,12 +142,7 @@ async function getPushUserInfo(
 
     const newMessageIndex = nextNewMessageIndex++;
     messageIndices.push(newMessageIndex);
-
-    const messageID = messageDataLocalID(messageData) ?? uuidv4();
-    const rawMessageInfo = rawMessageInfoFromMessageData(
-      messageData,
-      messageID,
-    );
+    messageDatas.push(messageData);
     newMessageInfos.push(rawMessageInfo);
   }
 
@@ -1046,7 +1044,10 @@ type PreparePushNotifsInputData = {
   +messageInfos: { +[id: string]: RawMessageInfo },
   +thickRawThreadInfos: ThickRawThreadInfos,
   +auxUserInfos: AuxUserInfos,
-  +messageDatas: ?$ReadOnlyArray<MessageData>,
+  +messageDatasWithMessageInfos: ?$ReadOnlyArray<{
+    +messageData: MessageData,
+    +rawMessageInfo: RawMessageInfo,
+  }>,
   +userInfos: UserInfos,
   +getENSNames: ?GetENSNames,
   +getFCNames: ?GetFCNames,
@@ -1059,7 +1060,7 @@ async function preparePushNotifs(
     encryptedNotifUtilsAPI,
     senderDeviceDescriptor,
     olmSessionCreator,
-    messageDatas,
+    messageDatasWithMessageInfos,
     messageInfos,
     auxUserInfos,
     thickRawThreadInfos,
@@ -1072,7 +1073,7 @@ async function preparePushNotifs(
     messageInfos,
     thickRawThreadInfos,
     auxUserInfos,
-    messageDatas,
+    messageDatasWithMessageInfos,
   );
 
   if (!pushInfos) {

diff --git a/lib/shared/dm-ops/add-members-spec.js b/lib/shared/dm-ops/add-members-spec.js
@@ -10,6 +10,7 @@ import type {
 } from './dm-op-spec.js';
 import type { DMAddMembersOperation } from '../../types/dm-ops.js';
 import { messageTypes } from '../../types/message-types-enum.js';
+import type { RawMessageInfo } from '../../types/message-types.js';
 import type { AddMembersMessageData } from '../../types/messages/add-members.js';
 import { minimallyEncodeMemberInfo } from '../../types/minimally-encoded-thread-permissions-types.js';
 import { joinThreadSubscription } from '../../types/subscription-types.js';
@@ -19,37 +20,43 @@ import { values } from '../../utils/objects.js';
 import { rawMessageInfoFromMessageData } from '../message-utils.js';
 import { roleIsDefaultRole, userIsMember } from '../thread-utils.js';
 
-function createAddNewMembersMessageDataFromDMOperation(
+function createAddNewMembersMessageDataWithInfoFromDMOperation(
   dmOperation: DMAddMembersOperation,
-): AddMembersMessageData {
-  const { editorID, time, addedUserIDs, threadID } = dmOperation;
-  return {
+): {
+  +messageData: AddMembersMessageData,
+  +rawMessageInfo: RawMessageInfo,
+} {
+  const { editorID, time, addedUserIDs, threadID, messageID } = dmOperation;
+  const messageData = {
     type: messageTypes.ADD_MEMBERS,
     threadID,
     creatorID: editorID,
     time,
     addedUserIDs: [...addedUserIDs],
   };
+  const rawMessageInfo = rawMessageInfoFromMessageData(messageData, messageID);
+  return { messageData, rawMessageInfo };
 }
 
 const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
   notificationsCreationData: async (dmOperation: DMAddMembersOperation) => {
-    const messageData =
-      createAddNewMembersMessageDataFromDMOperation(dmOperation);
-    return { messageDatas: [messageData] };
+    return {
+      messageDatasWithMessageInfos: [
+        createAddNewMembersMessageDataWithInfoFromDMOperation(dmOperation),
+      ],
+    };
   },
   processDMOperation: async (
     dmOperation: DMAddMembersOperation,
     utilities: ProcessDMOperationUtilities,
   ) => {
-    const { editorID, time, messageID, addedUserIDs, threadID } = dmOperation;
+    const { editorID, time, addedUserIDs, threadID } = dmOperation;
     const { viewerID, threadInfos } = utilities;
 
-    const messageData =
-      createAddNewMembersMessageDataFromDMOperation(dmOperation);
-    const rawMessageInfos = [
-      rawMessageInfoFromMessageData(messageData, messageID),
-    ];
+    const { rawMessageInfo } =
+      createAddNewMembersMessageDataWithInfoFromDMOperation(dmOperation);
+    const rawMessageInfos = [rawMessageInfo];
+
     const currentThreadInfo = threadInfos[threadID];
     if (!currentThreadInfo.thick) {
       return {
@@ -142,4 +149,7 @@ const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
   supportsAutoRetry: true,
 });
 
-export { addMembersSpec, createAddNewMembersMessageDataFromDMOperation };
+export {
+  addMembersSpec,
+  createAddNewMembersMessageDataWithInfoFromDMOperation,
+};
diff --git a/lib/shared/dm-ops/add-viewer-to-thread-members-spec.js b/lib/shared/dm-ops/add-viewer-to-thread-members-spec.js
@@ -9,34 +9,45 @@ import type {
 } from './dm-op-spec.js';
 import type { DMAddViewerToThreadMembersOperation } from '../../types/dm-ops.js';
 import { messageTypes } from '../../types/message-types-enum.js';
+import type { RawMessageInfo } from '../../types/message-types.js';
 import { messageTruncationStatus } from '../../types/message-types.js';
 import type { AddMembersMessageData } from '../../types/messages/add-members.js';
 import { joinThreadSubscription } from '../../types/subscription-types.js';
 import { updateTypes } from '../../types/update-types-enum.js';
 import { rawMessageInfoFromMessageData } from '../message-utils.js';
 import { userIsMember } from '../thread-utils.js';
 
-function createAddViewerToThreadMembersMessageDataFromDMOp(
+function createAddViewerToThreadMembersMessageDataWithInfoFromDMOp(
   dmOperation: DMAddViewerToThreadMembersOperation,
-): AddMembersMessageData {
-  const { editorID, time, addedUserIDs, existingThreadDetails } = dmOperation;
-  return {
+): {
+  +messageData: AddMembersMessageData,
+  +rawMessageInfo: RawMessageInfo,
+} {
+  const { editorID, time, addedUserIDs, existingThreadDetails, messageID } =
+    dmOperation;
+  const messageData = {
     type: messageTypes.ADD_MEMBERS,
     threadID: existingThreadDetails.threadID,
     creatorID: editorID,
     time,
     addedUserIDs: [...addedUserIDs],
   };
+  const rawMessageInfo = rawMessageInfoFromMessageData(messageData, messageID);
+  return { messageData, rawMessageInfo };
 }
 
 const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOperation> =
   Object.freeze({
     notificationsCreationData: async (
       dmOperation: DMAddViewerToThreadMembersOperation,
     ) => {
-      const messageData =
-        createAddViewerToThreadMembersMessageDataFromDMOp(dmOperation);
-      return { messageDatas: [messageData] };
+      return {
+        messageDatasWithMessageInfos: [
+          createAddViewerToThreadMembersMessageDataWithInfoFromDMOp(
+            dmOperation,
+          ),
+        ],
+      };
     },
     processDMOperation: async (
       dmOperation: DMAddViewerToThreadMembersOperation,
@@ -46,11 +57,9 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
         dmOperation;
       const { viewerID, threadInfos } = utilities;
 
-      const messageData =
-        createAddViewerToThreadMembersMessageDataFromDMOp(dmOperation);
-      const rawMessageInfos = messageID
-        ? [rawMessageInfoFromMessageData(messageData, messageID)]
-        : [];
+      const { rawMessageInfo } =
+        createAddViewerToThreadMembersMessageDataWithInfoFromDMOp(dmOperation);
+      const rawMessageInfos = messageID ? [rawMessageInfo] : [];
 
       const threadID = existingThreadDetails.threadID;
       const currentThreadInfo = threadInfos[threadID];
@@ -147,5 +156,5 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
 
 export {
   addViewerToThreadMembersSpec,
-  createAddViewerToThreadMembersMessageDataFromDMOp,
+  createAddViewerToThreadMembersMessageDataWithInfoFromDMOp,
 };
diff --git a/lib/shared/dm-ops/change-thread-settings-spec.js b/lib/shared/dm-ops/change-thread-settings-spec.js
@@ -11,7 +11,7 @@ import type {
   DMChangeThreadSettingsOperation,
   DMThreadSettingsChanges,
 } from '../../types/dm-ops.js';
-import type { MessageData, RawMessageInfo } from '../../types/message-types';
+import type { RawMessageInfo } from '../../types/message-types';
 import { messageTypes } from '../../types/message-types-enum.js';
 import type { ChangeSettingsMessageData } from '../../types/messages/change-settings.js';
 import type {
@@ -34,10 +34,15 @@ function getThreadIDFromChangeThreadSettingsDMOp(
 function createChangeSettingsMessageDatasAndUpdate(
   dmOperation: DMChangeThreadSettingsOperation,
 ): {
-  +fieldNameToMessageData: { +[fieldName: string]: ChangeSettingsMessageData },
+  +fieldNameToMessageData: {
+    +[fieldName: string]: {
+      +messageData: ChangeSettingsMessageData,
+      +rawMessageInfo: RawMessageInfo,
+    },
+  },
   +threadInfoUpdate: DMThreadSettingsChanges,
 } {
-  const { changes, editorID, time } = dmOperation;
+  const { changes, editorID, time, messageIDsPrefix } = dmOperation;
   const { name, description, color, avatar } = changes;
   const threadID = getThreadIDFromChangeThreadSettingsDMOp(dmOperation);
 
@@ -60,7 +65,10 @@ function createChangeSettingsMessageDatasAndUpdate(
   }
 
   const fieldNameToMessageData: {
-    [fieldName: string]: ChangeSettingsMessageData,
+    [fieldName: string]: {
+      +messageData: ChangeSettingsMessageData,
+      +rawMessageInfo: RawMessageInfo,
+    },
   } = {};
 
   const { avatar: avatarObject, ...rest } = threadInfoUpdate;
@@ -79,14 +87,19 @@ function createChangeSettingsMessageDatasAndUpdate(
 
   for (const fieldName in normalizedThreadInfoUpdate) {
     const value = normalizedThreadInfoUpdate[fieldName];
-    fieldNameToMessageData[fieldName] = {
+    const messageData: ChangeSettingsMessageData = {
       type: messageTypes.CHANGE_SETTINGS,
       threadID,
       creatorID: editorID,
       time,
       field: fieldName,
       value: value,
     };
+    const rawMessageInfo = rawMessageInfoFromMessageData(
+      messageData,
+      `${messageIDsPrefix}/${fieldName}`,
+    );
+    fieldNameToMessageData[fieldName] = { messageData, rawMessageInfo };
   }
 
   return { fieldNameToMessageData, threadInfoUpdate };
@@ -97,37 +110,27 @@ const changeThreadSettingsSpec: DMOperationSpec<DMChangeThreadSettingsOperation>
     notificationsCreationData: async (
       dmOperation: DMChangeThreadSettingsOperation,
     ) => {
-      const messageDatas: Array<MessageData> = [];
-
       const { fieldNameToMessageData } =
         createChangeSettingsMessageDatasAndUpdate(dmOperation);
-      messageDatas.push(...values(fieldNameToMessageData));
-      return { messageDatas };
+
+      return { messageDatasWithMessageInfos: values(fieldNameToMessageData) };
     },
     processDMOperation: async (
       dmOperation: DMChangeThreadSettingsOperation,
       utilities: ProcessDMOperationUtilities,
     ) => {
-      const { time, messageIDsPrefix } = dmOperation;
+      const { time } = dmOperation;
       const threadID = getThreadIDFromChangeThreadSettingsDMOp(dmOperation);
 
       const threadInfo: ?RawThreadInfo = utilities.threadInfos[threadID];
       const updateInfos: Array<ClientUpdateInfo> = [];
-      const rawMessageInfos: Array<RawMessageInfo> = [];
 
       const { fieldNameToMessageData, threadInfoUpdate } =
         createChangeSettingsMessageDatasAndUpdate(dmOperation);
 
-      const fieldNameToMessageDataPairs = Object.entries(
-        fieldNameToMessageData,
-      );
-      rawMessageInfos.push(
-        ...fieldNameToMessageDataPairs.map(([fieldName, messageData]) =>
-          rawMessageInfoFromMessageData(
-            messageData,
-            `${messageIDsPrefix}/${fieldName}`,
-          ),
-        ),
+      const messageDataWithMessageInfoPairs = values(fieldNameToMessageData);
+      const rawMessageInfos = messageDataWithMessageInfoPairs.map(
+        ({ rawMessageInfo }) => rawMessageInfo,
       );
 
       invariant(threadInfo?.thick, 'Thread should be thick');
@@ -146,7 +149,7 @@ const changeThreadSettingsSpec: DMOperationSpec<DMChangeThreadSettingsOperation>
         }
       }
 
-      if (fieldNameToMessageDataPairs.length > 0) {
+      if (messageDataWithMessageInfoPairs.length > 0) {
         updateInfos.push({
           type: updateTypes.UPDATE_THREAD,
           id: uuid.v4(),