[lib] Propagate thread infos to the notifs generating code

Summary: When a user performs an action, we generate an operation that is used to update Redux, send DM messages, and send the notifs. Updating Redux happens first. Generating notifs requires access to the thread. These cause a problem when leaving a thread, which might result in the thread being deleted from the store, making us unable to send the notifs. Notif sending code needs threads for two main reasons: to determine the recipients and to create a notification text. The solution is to pass a thread info inside notifs creation data so that we can use it instead of selecting from the store. https://linear.app/comm/issue/ENG-9823/fix-sending-notifs-about-leaving-a-thick-thread Depends on D13845 Test Plan: Created a thick thread with three users and verified that added users receive a thread, a message, and a notif about thread creation. Left this thread as one of the users and checked that an exception isn't thrown. The notif wasn't generated because `leaveThreadMessageSpec` doesn't implement `generatesNotifs` function. Added the user to the thread, which also doesn't generate notif for the same reason. Sending a text message to that thread generated the notifs. Reviewers: kamil, angelika Reviewed By: kamil Subscribers: ashoat Differential Revision: https://phab.comm.dev/D13848
CommE2E · Nov 8, 2024 · 8eec4d2 · 8eec4d2
1 parent 58824ef
commit 8eec4d2
Show file tree

Hide file tree

Showing 18 changed files with 142 additions and 49 deletions.
diff --git a/lib/push/send-hooks.react.js b/lib/push/send-hooks.react.js
@@ -14,7 +14,6 @@ import {
 import { ENSCacheContext } from '../components/ens-cache-provider.react.js';
 import { NeynarClientContext } from '../components/neynar-client-provider.react.js';
 import { usePeerOlmSessionsCreatorContext } from '../components/peer-olm-session-creator-provider.react.js';
-import { thickRawThreadInfosSelector } from '../selectors/thread-selectors.js';
 import { IdentityClientContext } from '../shared/identity-client-context.js';
 import type { AuthMetadata } from '../shared/identity-client-context.js';
 import { useTunnelbroker } from '../tunnelbroker/tunnelbroker-context.js';
@@ -111,7 +110,6 @@ function useSendPushNotifs(): (
   invariant(client, 'Identity context should be set');
   const { getAuthMetadata } = client;
   const rawMessageInfos = useSelector(state => state.messageStore.messages);
-  const thickRawThreadInfos = useSelector(thickRawThreadInfosSelector);
   const auxUserInfos = useSelector(state => state.auxUserStore.auxUserInfos);
   const userInfos = useSelector(state => state.userStore.userInfos);
   const { getENSNames } = React.useContext(ENSCacheContext);
@@ -138,17 +136,26 @@ function useSendPushNotifs(): (
         senderUserID,
         senderDeviceDescriptor,
       };
-      const { messageDatasWithMessageInfos, rescindData, badgeUpdateData } =
-        notifCreationData;
+      const {
+        messageDatasWithMessageInfos,
+        thickRawThreadInfos,
+        rescindData,
+        badgeUpdateData,
+      } = notifCreationData;
 
       const pushNotifsPreparationInput = {
         encryptedNotifUtilsAPI,
         senderDeviceDescriptor,
         olmSessionCreator,
         messageInfos: rawMessageInfos,
-        thickRawThreadInfos,
+        notifCreationData:
+          messageDatasWithMessageInfos && thickRawThreadInfos
+            ? {
+                thickRawThreadInfos,
+                messageDatasWithMessageInfos,
+              }
+            : null,
         auxUserInfos,
-        messageDatasWithMessageInfos,
         userInfos,
         getENSNames,
         getFCNames,
@@ -250,7 +257,6 @@ function useSendPushNotifs(): (
       encryptedNotifUtilsAPI,
       olmSessionCreator,
       rawMessageInfos,
-      thickRawThreadInfos,
       auxUserInfos,
       userInfos,
       getENSNames,

diff --git a/lib/push/send-utils.js b/lib/push/send-utils.js
@@ -111,26 +111,34 @@ function identityPlatformDetailsToPlatformDetails(
 
 async function getPushUserInfo(
   messageInfos: { +[id: string]: RawMessageInfo },
-  thickRawThreadInfos: ThickRawThreadInfos,
   auxUserInfos: AuxUserInfos,
-  messageDataWithMessageInfos: ?$ReadOnlyArray<{
-    +messageData: MessageData,
-    +rawMessageInfo: RawMessageInfo,
-  }>,
+  notifCreationData: ?{
+    +messageDatasWithMessageInfos: $ReadOnlyArray<{
+      +messageData: MessageData,
+      +rawMessageInfo: RawMessageInfo,
+    }>,
+    +thickRawThreadInfos: ThickRawThreadInfos,
+  },
 ): Promise<{
-  +pushInfos: ?PushInfo,
+  +pushInfos: ?{
+    +infos: PushInfo,
+    +thickRawThreadInfos: ThickRawThreadInfos,
+  },
   +rescindInfos: ?PushInfo,
 }> {
-  if (!messageDataWithMessageInfos) {
+  if (!notifCreationData) {
     return { pushInfos: null, rescindInfos: null };
   }
 
+  const { messageDatasWithMessageInfos, thickRawThreadInfos } =
+    notifCreationData;
+
   const threadsToMessageIndices: Map<string, number[]> = new Map();
   const newMessageInfos: RawMessageInfo[] = [];
   const messageDatas: MessageData[] = [];
 
   let nextNewMessageIndex = 0;
-  for (const messageDataWithInfo of messageDataWithMessageInfos) {
+  for (const messageDataWithInfo of messageDatasWithMessageInfos) {
     const { messageData, rawMessageInfo } = messageDataWithInfo;
 
     const threadID = messageData.threadID;
@@ -255,7 +263,10 @@ async function getPushUserInfo(
   ]);
 
   return {
-    pushInfos: _pickBy(Boolean)(pushInfo),
+    pushInfos: {
+      infos: _pickBy(Boolean)(pushInfo),
+      thickRawThreadInfos,
+    },
     rescindInfos: _pickBy(Boolean)(rescindInfo),
   };
 }
@@ -1177,12 +1188,14 @@ type PreparePushNotifsInputData = {
     devices: $ReadOnlyArray<DeviceSessionCreationRequest>,
   ) => Promise<void>,
   +messageInfos: { +[id: string]: RawMessageInfo },
-  +thickRawThreadInfos: ThickRawThreadInfos,
   +auxUserInfos: AuxUserInfos,
-  +messageDatasWithMessageInfos: ?$ReadOnlyArray<{
-    +messageData: MessageData,
-    +rawMessageInfo: RawMessageInfo,
-  }>,
+  +notifCreationData: ?{
+    +messageDatasWithMessageInfos: $ReadOnlyArray<{
+      +messageData: MessageData,
+      +rawMessageInfo: RawMessageInfo,
+    }>,
+    +thickRawThreadInfos: ThickRawThreadInfos,
+  },
   +userInfos: UserInfos,
   +getENSNames: ?GetENSNames,
   +getFCNames: ?GetFCNames,
@@ -1195,27 +1208,26 @@ async function preparePushNotifs(
     encryptedNotifUtilsAPI,
     senderDeviceDescriptor,
     olmSessionCreator,
-    messageDatasWithMessageInfos,
+    notifCreationData,
     messageInfos,
     auxUserInfos,
-    thickRawThreadInfos,
     userInfos,
     getENSNames,
     getFCNames,
   } = inputData;
 
   const { pushInfos } = await getPushUserInfo(
     messageInfos,
-    thickRawThreadInfos,
     auxUserInfos,
-    messageDatasWithMessageInfos,
+    notifCreationData,
   );
 
   if (!pushInfos) {
     return null;
   }
 
-  const filteredPushInfos = filterDevicesSupportingDMNotifsForUsers(pushInfos);
+  const { infos, thickRawThreadInfos } = pushInfos;
+  const filteredPushInfos = filterDevicesSupportingDMNotifsForUsers(infos);
 
   const userDevices: {
     [userID: string]: $ReadOnlyArray<string>,

diff --git a/lib/shared/dm-ops/add-members-spec.js b/lib/shared/dm-ops/add-members-spec.js
@@ -153,6 +153,9 @@ const addMembersSpec: DMOperationSpec<DMAddMembersOperation> = Object.freeze({
 
     const notificationsCreationData = {
       messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+      thickRawThreadInfos: {
+        [threadID]: resultThreadInfo,
+      },
     };
 
     return {

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
@@ -89,10 +89,6 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
         }
       }
 
-      const notificationsCreationData = {
-        messageDatasWithMessageInfos: [messageDataWithMessageInfos],
-      };
-
       if (currentThreadInfo) {
         const { membershipPermissions, roleID } =
           createPermissionsForNewMembers(currentThreadInfo, utilities);
@@ -134,6 +130,13 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
           },
         ];
 
+        const notificationsCreationData = {
+          messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+          thickRawThreadInfos: {
+            [threadID]: resultThreadInfo,
+          },
+        };
+
         return {
           rawMessageInfos,
           updateInfos,
@@ -173,6 +176,12 @@ const addViewerToThreadMembersSpec: DMOperationSpec<DMAddViewerToThreadMembersOp
           rawEntryInfos: [],
         },
       ];
+      const notificationsCreationData = {
+        messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+        thickRawThreadInfos: {
+          [threadID]: resultThreadInfo,
+        },
+      };
       return {
         rawMessageInfos: [],
         updateInfos,

diff --git a/lib/shared/dm-ops/change-thread-settings-spec.js b/lib/shared/dm-ops/change-thread-settings-spec.js
@@ -181,6 +181,9 @@ const changeThreadSettingsSpec: DMOperationSpec<DMChangeThreadSettingsOperation>
 
       const notificationsCreationData = {
         messageDatasWithMessageInfos: values(fieldNameToMessageData),
+        thickRawThreadInfos: {
+          [threadID]: threadInfoToUpdate,
+        },
       };
 
       return {

diff --git a/lib/shared/dm-ops/create-entry-spec.js b/lib/shared/dm-ops/create-entry-spec.js
@@ -36,7 +36,10 @@ function createMessageDataWithInfoFromDMOperation(
 }
 
 const createEntrySpec: DMOperationSpec<DMCreateEntryOperation> = Object.freeze({
-  processDMOperation: async (dmOperation: DMCreateEntryOperation) => {
+  processDMOperation: async (
+    dmOperation: DMCreateEntryOperation,
+    utilities: ProcessDMOperationUtilities,
+  ) => {
     const { threadID, creatorID, time, entryID, entryDate, text } = dmOperation;
 
     const messageDataWithMessageInfos =
@@ -68,6 +71,9 @@ const createEntrySpec: DMOperationSpec<DMCreateEntryOperation> = Object.freeze({
 
     const notificationsCreationData = {
       messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+      thickRawThreadInfos: {
+        [threadID]: utilities.threadInfos[threadID],
+      },
     };
 
     return {

diff --git a/lib/shared/dm-ops/create-sidebar-spec.js b/lib/shared/dm-ops/create-sidebar-spec.js
@@ -170,6 +170,9 @@ const createSidebarSpec: DMOperationSpec<DMCreateSidebarOperation> =
             rawMessageInfo: createSidebarMessageInfo,
           },
         ],
+        thickRawThreadInfos: {
+          [threadID]: rawThreadInfo,
+        },
       };
 
       return {

diff --git a/lib/shared/dm-ops/create-thread-spec.js b/lib/shared/dm-ops/create-thread-spec.js
@@ -275,6 +275,9 @@ const createThreadSpec: DMOperationSpec<DMCreateThreadOperation> =
 
       const notificationsCreationData = {
         messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+        thickRawThreadInfos: {
+          [threadID]: rawThreadInfo,
+        },
       };
 
       return {

diff --git a/lib/shared/dm-ops/delete-entry-spec.js b/lib/shared/dm-ops/delete-entry-spec.js
@@ -61,6 +61,9 @@ const deleteEntrySpec: DMOperationSpec<DMDeleteEntryOperation> = Object.freeze({
 
     const notificationsCreationData = {
       messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+      thickRawThreadInfos: {
+        [threadID]: utilities.threadInfos[threadID],
+      },
     };
 
     if (timestamp > time) {

diff --git a/lib/shared/dm-ops/edit-entry-spec.js b/lib/shared/dm-ops/edit-entry-spec.js
@@ -62,6 +62,9 @@ const editEntrySpec: DMOperationSpec<DMEditEntryOperation> = Object.freeze({
 
     const notificationsCreationData = {
       messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+      thickRawThreadInfos: {
+        [threadID]: utilities.threadInfos[threadID],
+      },
     };
 
     if (timestamp > time) {

diff --git a/lib/shared/dm-ops/join-thread-spec.js b/lib/shared/dm-ops/join-thread-spec.js
@@ -65,11 +65,13 @@ const joinThreadSpec: DMOperationSpec<DMJoinThreadOperation> = Object.freeze({
       };
     }
 
-    const notificationsCreationData = {
-      messageDatasWithMessageInfos: [messageDataWithMessageInfos],
-    };
-
     if (memberTimestamps[joinerID].isMember > time) {
+      const notificationsCreationData = {
+        messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+        thickRawThreadInfos: {
+          [currentThreadInfo.id]: currentThreadInfo,
+        },
+      };
       return {
         rawMessageInfos: joinThreadMessageInfos,
         updateInfos: [],
@@ -86,22 +88,24 @@ const joinThreadSpec: DMOperationSpec<DMJoinThreadOperation> = Object.freeze({
     const updateInfos: Array<ClientUpdateInfo> = [];
     const rawMessageInfos: Array<RawMessageInfo> = [];
 
+    let resultThreadInfo = currentThreadInfo;
     if (userIsMember(currentThreadInfo, joinerID)) {
       rawMessageInfos.push(...joinThreadMessageInfos);
+      resultThreadInfo = {
+        ...currentThreadInfo,
+        timestamps: {
+          ...currentThreadInfo.timestamps,
+          members: memberTimestamps,
+        },
+      };
       updateInfos.push({
         type: updateTypes.UPDATE_THREAD,
         id: uuid.v4(),
         time,
-        threadInfo: {
-          ...currentThreadInfo,
-          timestamps: {
-            ...currentThreadInfo.timestamps,
-            members: memberTimestamps,
-          },
-        },
+        threadInfo: resultThreadInfo,
       });
     } else if (viewerID === joinerID) {
-      const newThreadInfo = createThickRawThreadInfo(
+      resultThreadInfo = createThickRawThreadInfo(
         {
           ...existingThreadDetails,
           allMemberIDsWithSubscriptions: [
@@ -119,7 +123,7 @@ const joinThreadSpec: DMOperationSpec<DMJoinThreadOperation> = Object.freeze({
         type: updateTypes.JOIN_THREAD,
         id: uuid.v4(),
         time,
-        threadInfo: newThreadInfo,
+        threadInfo: resultThreadInfo,
         rawMessageInfos: joinThreadMessageInfos,
         truncationStatus: messageTruncationStatus.EXHAUSTIVE,
         rawEntryInfos: [],
@@ -156,7 +160,7 @@ const joinThreadSpec: DMOperationSpec<DMJoinThreadOperation> = Object.freeze({
         isSender: joinerID === viewerID,
         subscription: joinThreadSubscription,
       });
-      const updatedThreadInfo = {
+      resultThreadInfo = {
         ...currentThreadInfo,
         members: [...currentThreadInfo.members, member],
         timestamps: {
@@ -168,9 +172,17 @@ const joinThreadSpec: DMOperationSpec<DMJoinThreadOperation> = Object.freeze({
         type: updateTypes.UPDATE_THREAD,
         id: uuid.v4(),
         time,
-        threadInfo: updatedThreadInfo,
+        threadInfo: resultThreadInfo,
       });
     }
+
+    const notificationsCreationData = {
+      messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+      thickRawThreadInfos: {
+        [resultThreadInfo.id]: resultThreadInfo,
+      },
+    };
+
     return {
       rawMessageInfos,
       updateInfos,

diff --git a/lib/shared/dm-ops/leave-thread-spec.js b/lib/shared/dm-ops/leave-thread-spec.js
@@ -135,6 +135,9 @@ const leaveThreadSpec: DMOperationSpec<DMLeaveThreadOperation> = Object.freeze({
 
     const notificationsCreationData = {
       messageDatasWithMessageInfos: [messageDataWithMessageInfos],
+      thickRawThreadInfos: {
+        [threadID]: threadInfo,
+      },
     };
 
     if (viewerID === editorID) {