Centralize notification logic

ts/messageModifiers/Reactions.ts

@@ -34,7 +34,6 @@ import { strictAssert } from '../util/assert';
 import { repeat, zipObject } from '../util/iterables';
 import { getMessageIdForLogging } from '../util/idForLogging';
 import { hydrateStoryContext } from '../util/hydrateStoryContext';
-import { shouldReplyNotifyUser } from '../util/shouldReplyNotifyUser';
 import { drop } from '../util/drop';
 import * as reactionUtil from '../reactions/util';
 import { isNewReactionReplacingPrevious } from '../reactions/util';
@@ -45,6 +44,7 @@ import {
   conversationJobQueue,
   conversationQueueJobEnum,
 } from '../jobs/conversationJobQueue';
+import { maybeNotify } from '../messages/maybeNotify';
 
 export type ReactionAttributesType = {
   emoji: string;
@@ -431,18 +431,12 @@ export async function handleReaction(
       }
 
       if (isFromSomeoneElse) {
-        log.info(
+        drop(
-          'handleReaction: notifying for story reaction to ' +
+          maybeNotify({
-            `${getMessageIdForLogging(storyMessage)} from someone else`
+            message: generatedMessage.attributes,
+            conversation: targetConversation,
+          })
         );
-        if (
-          await shouldReplyNotifyUser(
-            generatedMessage.attributes,
-            targetConversation
-          )
-        ) {
-          drop(targetConversation.notify(generatedMessage.attributes));
-        }
       }
     }
   } else {
@@ -515,7 +509,13 @@ export async function handleReaction(
         message.set({ reactions });
 
         if (isOutgoing(message.attributes) && isFromSomeoneElse) {
-          void conversation.notify(message.attributes, reaction);
+          drop(
+            maybeNotify({
+              targetMessage: message.attributes,
+              conversation,
+              reaction,
+            })
+          );
         }
       }
     }

ts/messages/maybeNotify.ts

@@ -0,0 +1,173 @@
+// Copyright 2025 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import * as log from '../logging/log';
+
+import { getAuthor, isIncoming, isOutgoing } from './helpers';
+
+import type { ConversationModel } from '../models/conversations';
+import { getActiveProfile } from '../state/selectors/notificationProfiles';
+import { shouldNotify as shouldNotifyDuringNotificationProfile } from '../types/NotificationProfile';
+import { isMessageUnread } from '../util/isMessageUnread';
+import { isDirectConversation } from '../util/whatTypeOfConversation';
+import { hasExpiration } from '../types/Message2';
+import {
+  notificationService,
+  NotificationType,
+} from '../services/notifications';
+import { getNotificationTextForMessage } from '../util/getNotificationTextForMessage';
+import type { MessageAttributesType } from '../model-types';
+import type { ReactionAttributesType } from '../messageModifiers/Reactions';
+import { shouldStoryReplyNotifyUser } from '../util/shouldStoryReplyNotifyUser';
+import { ReactionSource } from '../reactions/ReactionSource';
+
+type MaybeNotifyArgs = {
+  conversation: ConversationModel;
+} & (
+  | {
+      reaction: Readonly<ReactionAttributesType>;
+      targetMessage: Readonly<MessageAttributesType>;
+    }
+  | { message: Readonly<MessageAttributesType>; reaction?: never }
+);
+
+export async function maybeNotify(args: MaybeNotifyArgs): Promise<void> {
+  if (!notificationService.isEnabled) {
+    return;
+  }
+
+  const { conversation, reaction } = args;
+
+  let warrantsNotification: boolean;
+  if (reaction) {
+    warrantsNotification = doesReactionWarrantNotification(args);
+  } else {
+    warrantsNotification = await doesMessageWarrantNotification(args);
+  }
+  if (!warrantsNotification) {
+    return;
+  }
+
+  if (!isAllowedByConversation(args)) {
+    return;
+  }
+
+  const activeProfile = getActiveProfile(window.reduxStore.getState());
+  if (
+    !shouldNotifyDuringNotificationProfile({
+      activeProfile,
+      conversationId: conversation.id,
+      isCall: false,
+      isMention: args.reaction ? false : Boolean(args.message.mentionsMe),
+    })
+  ) {
+    log.info(
+      'maybeNotify: Would notify for message, but notification profile prevented it'
+    );
+    return;
+  }
+
+  const conversationId = conversation.get('id');
+  const messageForNotification = args.reaction
+    ? args.targetMessage
+    : args.message;
+  const isMessageInDirectConversation = isDirectConversation(
+    conversation.attributes
+  );
+
+  const sender = reaction
+    ? window.ConversationController.get(reaction.fromId)
+    : getAuthor(args.message);
+  const senderName = sender
+    ? sender.getTitle()
+    : window.i18n('icu:unknownContact');
+  const senderTitle = isMessageInDirectConversation
+    ? senderName
+    : window.i18n('icu:notificationSenderInGroup', {
+        sender: senderName,
+        group: conversation.getTitle(),
+      });
+
+  const { url, absolutePath } = await conversation.getAvatarOrIdenticon();
+
+  const messageId = messageForNotification.id;
+  const isExpiringMessage = hasExpiration(messageForNotification);
+
+  notificationService.add({
+    senderTitle,
+    conversationId,
+    storyId: isMessageInDirectConversation
+      ? undefined
+      : messageForNotification.storyId,
+    notificationIconUrl: url,
+    notificationIconAbsolutePath: absolutePath,
+    isExpiringMessage,
+    message: getNotificationTextForMessage(messageForNotification),
+    messageId,
+    reaction: reaction
+      ? {
+          emoji: reaction.emoji,
+          targetAuthorAci: reaction.targetAuthorAci,
+          targetTimestamp: reaction.targetTimestamp,
+        }
+      : undefined,
+    sentAt: messageForNotification.timestamp,
+    type: reaction ? NotificationType.Reaction : NotificationType.Message,
+  });
+}
+
+function doesReactionWarrantNotification({
+  reaction,
+  targetMessage,
+}: {
+  targetMessage: MessageAttributesType;
+  reaction: ReactionAttributesType;
+}): boolean {
+  return (
+    reaction.source === ReactionSource.FromSomeoneElse &&
+    isOutgoing(targetMessage)
+  );
+}
+
+async function doesMessageWarrantNotification({
+  message,
+  conversation,
+}: {
+  message: MessageAttributesType;
+  conversation: ConversationModel;
+}): Promise<boolean> {
+  if (!isIncoming(message)) {
+    return false;
+  }
+
+  if (!isMessageUnread(message)) {
+    return false;
+  }
+
+  if (
+    message.storyId &&
+    !(await shouldStoryReplyNotifyUser(message, conversation))
+  ) {
+    return false;
+  }
+
+  return true;
+}
+
+function isAllowedByConversation(args: MaybeNotifyArgs): boolean {
+  const { conversation, reaction } = args;
+
+  if (!conversation.isMuted()) {
+    return true;
+  }
+
+  if (reaction) {
+    return false;
+  }
+
+  if (conversation.get('dontNotifyForMentionsIfMuted')) {
+    return false;
+  }
+
+  return args.message.mentionsMe === true;
+}

ts/messages/saveAndNotify.ts

@@ -11,17 +11,12 @@ import {
   modifyTargetMessage,
   ModifyTargetMessageResult,
 } from '../util/modifyTargetMessage';
-import { shouldReplyNotifyUser } from '../util/shouldReplyNotifyUser';
 import { isStory } from './helpers';
 import { drop } from '../util/drop';
 
 import type { ConversationModel } from '../models/conversations';
 import type { MessageModel } from '../models/messages';
-import { getActiveProfile } from '../state/selectors/notificationProfiles';
+import { maybeNotify } from './maybeNotify';
-import {
-  redactNotificationProfileId,
-  shouldNotify,
-} from '../types/NotificationProfile';
 
 export async function saveAndNotify(
   message: MessageModel,
@@ -53,30 +48,7 @@ export async function saveAndNotify(
 
     drop(conversation.onNewMessage(message));
 
-    const activeProfile = getActiveProfile(window.reduxStore.getState());
+    drop(maybeNotify({ message: message.attributes, conversation }));
-    const doesProfileAllowNotify = shouldNotify({
-      activeProfile,
-      conversationId: conversation.id,
-      isCall: false,
-      isMention: Boolean(message.get('mentionsMe')),
-    });
-    const shouldStoryReplyNotify = await shouldReplyNotifyUser(
-      message.attributes,
-      conversation
-    );
-
-    if (!shouldStoryReplyNotify) {
-      log.info(
-        `saveAndNotify: Not notifying for story reply ${message.get('sent_at')}`
-      );
-    } else if (!doesProfileAllowNotify) {
-      const redactedId = redactNotificationProfileId(activeProfile?.id ?? '');
-      log.info(
-        `saveAndNotify: Would notify for message ${message.get('sent_at')}, but notification profile ${redactedId} prevented it`
-      );
-    } else {
-      await conversation.notify(message.attributes);
-    }
 
     // Increment the sent message count if this is an outgoing message
     if (message.get('type') === 'outgoing') {

ts/models/conversations.ts

@@ -59,7 +59,6 @@ import type {
   ConversationColorType,
   CustomColorType,
 } from '../types/Colors';
-import { getAuthor } from '../messages/helpers';
 import { strictAssert } from '../util/assert';
 import { isConversationMuted } from '../util/isConversationMuted';
 import { isConversationSMSOnly } from '../util/isConversationSMSOnly';
@@ -89,14 +88,9 @@ import {
 import { decryptAttachmentV2 } from '../AttachmentCrypto';
 import * as Bytes from '../Bytes';
 import type { DraftBodyRanges } from '../types/BodyRange';
-import { BodyRange } from '../types/BodyRange';
 import { migrateColor } from '../util/migrateColor';
 import { isNotNil } from '../util/isNotNil';
-import {
+import { shouldSaveNotificationAvatarToDisk } from '../services/notifications';
-  NotificationType,
-  notificationService,
-  shouldSaveNotificationAvatarToDisk,
-} from '../services/notifications';
 import { storageServiceUploadJob } from '../services/storage';
 import { getSendOptions } from '../util/getSendOptions';
 import type { IsConversationAcceptedOptionsType } from '../util/isConversationAccepted';
@@ -143,7 +137,6 @@ import {
   conversationJobQueue,
   conversationQueueJobEnum,
 } from '../jobs/conversationJobQueue';
-import type { ReactionAttributesType } from '../messageModifiers/Reactions';
 import { getProfile } from '../util/getProfile';
 import { SEALED_SENDER } from '../types/SealedSender';
 import { createIdenticon } from '../util/createIdenticon';
@@ -177,7 +170,6 @@ import { generateMessageId } from '../util/generateMessageId';
 import { getMessageAuthorText } from '../util/getMessageAuthorText';
 import { downscaleOutgoingAttachment } from '../util/attachments';
 import { MessageRequestResponseEvent } from '../types/MessageRequestResponseEvent';
-import { hasExpiration } from '../types/Message2';
 import type { AddressableMessage } from '../textsecure/messageReceiverEvents';
 import {
   getConversationIdentifier,
@@ -195,6 +187,7 @@ import { applyNewAvatar } from '../groups';
 import { safeSetTimeout } from '../util/timeout';
 import { getTypingIndicatorSetting } from '../types/Util';
 import { INITIAL_EXPIRE_TIMER_VERSION } from '../util/expirationTimer';
+import { maybeNotify } from '../messages/maybeNotify';
 
 /* eslint-disable more/no-then */
 window.Whisper = window.Whisper || {};
@@ -3196,7 +3189,7 @@ export class ConversationModel extends window.Backbone
     drop(this.onNewMessage(message));
     this.throttledUpdateUnread();
 
-    await this.notify(message.attributes);
+    await maybeNotify({ message: message.attributes, conversation: this });
   }
 
   async addKeyChange(
@@ -5476,84 +5469,6 @@ export class ConversationModel extends window.Backbone
     return isConversationMuted(this.attributes);
   }
 
-  async notify(
-    message: Readonly<MessageAttributesType>,
-    reaction?: Readonly<ReactionAttributesType>
-  ): Promise<void> {
-    // As a performance optimization don't perform any work if notifications are
-    // disabled.
-    if (!notificationService.isEnabled) {
-      return;
-    }
-
-    if (this.isMuted()) {
-      if (this.get('dontNotifyForMentionsIfMuted')) {
-        return;
-      }
-
-      const ourAci = window.textsecure.storage.user.getCheckedAci();
-      const ourPni = window.textsecure.storage.user.getCheckedPni();
-      const ourServiceIds: Set<ServiceIdString> = new Set([ourAci, ourPni]);
-
-      const mentionsMe = (message.bodyRanges || []).some(bodyRange => {
-        if (!BodyRange.isMention(bodyRange)) {
-          return false;
-        }
-        return ourServiceIds.has(
-          normalizeServiceId(bodyRange.mentionAci, 'notify: mentionsMe check')
-        );
-      });
-      if (!mentionsMe) {
-        return;
-      }
-    }
-
-    if (!isIncoming(message) && !reaction) {
-      return;
-    }
-
-    const conversationId = this.id;
-    const isMessageInDirectConversation = isDirectConversation(this.attributes);
-
-    const sender = reaction
-      ? window.ConversationController.get(reaction.fromId)
-      : getAuthor(message);
-    const senderName = sender
-      ? sender.getTitle()
-      : window.i18n('icu:unknownContact');
-    const senderTitle = isMessageInDirectConversation
-      ? senderName
-      : window.i18n('icu:notificationSenderInGroup', {
-          sender: senderName,
-          group: this.getTitle(),
-        });
-
-    const { url, absolutePath } = await this.getAvatarOrIdenticon();
-
-    const messageId = message.id;
-    const isExpiringMessage = hasExpiration(message);
-
-    notificationService.add({
-      senderTitle,
-      conversationId,
-      storyId: isMessageInDirectConversation ? undefined : message.storyId,
-      notificationIconUrl: url,
-      notificationIconAbsolutePath: absolutePath,
-      isExpiringMessage,
-      message: getNotificationTextForMessage(message),
-      messageId,
-      reaction: reaction
-        ? {
-            emoji: reaction.emoji,
-            targetAuthorAci: reaction.targetAuthorAci,
-            targetTimestamp: reaction.targetTimestamp,
-          }
-        : undefined,
-      sentAt: message.timestamp,
-      type: reaction ? NotificationType.Reaction : NotificationType.Message,
-    });
-  }
-
   async getAvatarOrIdenticon(): Promise<{
     url: string;
     absolutePath?: string;

ts/util/shouldStoryReplyNotifyUser.ts

@@ -8,7 +8,7 @@ import { DataReader } from '../sql/Client';
 import { isGroup } from './whatTypeOfConversation';
 import { isMessageUnread } from './isMessageUnread';
 
-export async function shouldReplyNotifyUser(
+export async function shouldStoryReplyNotifyUser(
   messageAttributes: Pick<
     ReadonlyMessageAttributesType,
     'readStatus' | 'storyId'
@@ -24,6 +24,7 @@ export async function shouldReplyNotifyUser(
 
   // If this is not a reply to a story, always notify.
   if (storyId == null) {
+    log.error('shouldStoryReplyNotifyUser: called with a non-story-reply');
     return true;
   }