From 292ab54e22b7ddfdcd85c4b48038d8f73696aae4 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
Date: Tue, 5 Nov 2024 18:45:05 -0800
Subject: [PATCH] Change attributes before queuing job

Co-authored-by: Scott Nonnenberg <scott@signal.org>
---
 ts/background.ts                |  4 ++-
 ts/models/conversations.ts      | 50 +++++++++++++++++++++++++--------
 ts/services/storageRecordOps.ts |  5 +++-
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/ts/background.ts b/ts/background.ts
index 09ce6dabd17a..46e9799b0f99 100644
--- a/ts/background.ts
+++ b/ts/background.ts
@@ -2631,7 +2631,9 @@ export async function startApp(): Promise<void> {
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const conversation = window.ConversationController.get(id)!;
 
-    conversation.enableProfileSharing();
+    conversation.enableProfileSharing({
+      reason: 'handleMessageSentProfileUpdate',
+    });
     await DataWriter.updateConversation(conversation.attributes);
 
     // Then we update our own profileKey if it's different from what we have
diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts
index bfefc8ce8a04..984bbb742645 100644
--- a/ts/models/conversations.ts
+++ b/ts/models/conversations.ts
@@ -1018,7 +1018,7 @@ export class ConversationModel extends window.Backbone
       this.captureChange('removeContact');
     }
 
-    this.disableProfileSharing({ viaStorageServiceSync });
+    this.disableProfileSharing({ reason: 'remove', viaStorageServiceSync });
 
     // Drop existing message request state to avoid sending receipts and
     // display MR actions.
@@ -1077,24 +1077,39 @@ export class ConversationModel extends window.Backbone
     }
   }
 
-  enableProfileSharing({ viaStorageServiceSync = false } = {}): void {
+  enableProfileSharing({
+    reason,
+    viaStorageServiceSync = false,
+  }: {
+    reason: string;
+    viaStorageServiceSync?: boolean;
+  }): void {
     log.info(
-      `enableProfileSharing: ${this.idForLogging()} storage? ${viaStorageServiceSync}`
+      `enableProfileSharing: ${this.idForLogging()} reason=${reason} ` +
+        `storage? ${viaStorageServiceSync}`
     );
     const before = this.get('profileSharing');
+    if (before === true) {
+      return;
+    }
 
     this.set({ profileSharing: true });
 
-    const after = this.get('profileSharing');
-
-    if (!viaStorageServiceSync && Boolean(before) !== Boolean(after)) {
-      this.captureChange('enableProfileSharing');
+    if (!viaStorageServiceSync) {
+      this.captureChange(`enableProfileSharing/${reason}`);
     }
   }
 
-  disableProfileSharing({ viaStorageServiceSync = false } = {}): void {
+  disableProfileSharing({
+    reason,
+    viaStorageServiceSync = false,
+  }: {
+    reason: string;
+    viaStorageServiceSync?: boolean;
+  }): void {
     log.info(
-      `disableProfileSharing: ${this.idForLogging()} storage? ${viaStorageServiceSync}`
+      `disableProfileSharing: ${this.idForLogging()} reason=${reason} ` +
+        `storage? ${viaStorageServiceSync}`
     );
     const before = this.get('profileSharing');
 
@@ -1103,7 +1118,7 @@ export class ConversationModel extends window.Backbone
     const after = this.get('profileSharing');
 
     if (!viaStorageServiceSync && Boolean(before) !== Boolean(after)) {
-      this.captureChange('disableProfileSharing');
+      this.captureChange(`disableProfileSharing/${reason}`);
     }
   }
 
@@ -2426,7 +2441,10 @@ export class ConversationModel extends window.Backbone
         }
 
         if (isBlock || isDelete) {
-          this.disableProfileSharing({ viaStorageServiceSync });
+          this.disableProfileSharing({
+            reason: isBlock ? 'block' : 'delete',
+            viaStorageServiceSync,
+          });
         }
 
         if (isDelete) {
@@ -2459,7 +2477,10 @@ export class ConversationModel extends window.Backbone
         if (!viaStorageServiceSync) {
           await this.restoreContact({ shouldSave: false });
         }
-        this.enableProfileSharing({ viaStorageServiceSync });
+        this.enableProfileSharing({
+          reason: 'ACCEPT Message Request',
+          viaStorageServiceSync,
+        });
 
         // We really don't want to call this if we don't have to. It can take a lot of
         //   time to go through old messages to download attachments.
@@ -4141,6 +4162,11 @@ export class ConversationModel extends window.Backbone
       'Expected a timestamp'
     );
 
+    // Make sure profile sharing is enabled before job is queued and run
+    this.enableProfileSharing({
+      reason: 'mandatoryProfileSharing',
+    });
+
     await conversationJobQueue.add(
       {
         type: conversationQueueJobEnum.enum.NormalMessage,
diff --git a/ts/services/storageRecordOps.ts b/ts/services/storageRecordOps.ts
index 882501bcb31c..397fef07026f 100644
--- a/ts/services/storageRecordOps.ts
+++ b/ts/services/storageRecordOps.ts
@@ -697,7 +697,10 @@ function applyMessageRequestState(
   }
 
   if (record.whitelisted === false) {
-    conversation.disableProfileSharing({ viaStorageServiceSync: true });
+    conversation.disableProfileSharing({
+      reason: 'storage record not whitelisted',
+      viaStorageServiceSync: true,
+    });
   }
 }