From ac3284986cf60459f7ee42053db44d0f2c756e8a Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Sun, 5 Mar 2023 16:19:56 -0500
Subject: [PATCH] fix: Replies to comments in threads only trigger
 notifications to document subscribers, closes #4984

---
 server/models/helpers/NotificationHelper.ts   | 21 +++++++++++--------
 .../processors/NotificationsProcessor.ts      |  6 ++++--
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/server/models/helpers/NotificationHelper.ts b/server/models/helpers/NotificationHelper.ts
index 3b41ceca3..0f65cfd81 100644
--- a/server/models/helpers/NotificationHelper.ts
+++ b/server/models/helpers/NotificationHelper.ts
@@ -51,8 +51,9 @@ export default class NotificationHelper {
   ): Promise<NotificationSetting[]> => {
     const recipients = await this.getDocumentNotificationRecipients(
       document,
-      "documents.update",
-      actorId
+      "comments.create",
+      actorId,
+      !comment.parentCommentId
     );
 
     if (recipients.length > 0 && comment.parentCommentId) {
@@ -76,15 +77,18 @@ export default class NotificationHelper {
   /**
    * Get the recipients of a notification for a document event.
    *
-   * @param document The document to get recipients for
-   * @param eventName The event name
-   * @param actorId The id of the user that performed the action
+   * @param document The document to get recipients for.
+   * @param eventName The event name.
+   * @param actorId The id of the user that performed the action.
+   * @param onlySubscribers Whether to only return recipients that are actively
+   * subscribed to the document.
    * @returns A list of recipients
    */
   public static getDocumentNotificationRecipients = async (
     document: Document,
     eventName: string,
-    actorId: string
+    actorId: string,
+    onlySubscribers: boolean
   ): Promise<NotificationSetting[]> => {
     // First find all the users that have notifications enabled for this event
     // type at all and aren't the one that performed the action.
@@ -98,9 +102,8 @@ export default class NotificationHelper {
       },
     });
 
-    // If the event is a revision creation we can filter further to only those
-    // that have a subscription to the document…
-    if (eventName === "documents.update") {
+    // Filter further to only those that have a subscription to the document…
+    if (onlySubscribers) {
       const subscriptions = await Subscription.findAll({
         attributes: ["userId"],
         where: {
diff --git a/server/queues/processors/NotificationsProcessor.ts b/server/queues/processors/NotificationsProcessor.ts
index 7ffb8cdb3..10a569e22 100644
--- a/server/queues/processors/NotificationsProcessor.ts
+++ b/server/queues/processors/NotificationsProcessor.ts
@@ -81,7 +81,8 @@ export default class NotificationsProcessor extends BaseProcessor {
     const recipients = await NotificationHelper.getDocumentNotificationRecipients(
       document,
       "documents.publish",
-      document.lastModifiedById
+      document.lastModifiedById,
+      false
     );
 
     for (const recipient of recipients) {
@@ -128,7 +129,8 @@ export default class NotificationsProcessor extends BaseProcessor {
     const recipients = await NotificationHelper.getDocumentNotificationRecipients(
       document,
       "documents.update",
-      document.lastModifiedById
+      document.lastModifiedById,
+      true
     );
     if (!recipients.length) {
       return;