fix: Email notifications not sent when mention added to edited comment (#5145

* WIP: Need new email template

* New emails

server/commands/commentUpdater.ts

@@ -1,5 +1,6 @@
 import { Transaction } from "sequelize";
 import { Event, Comment, User } from "@server/models";
+import ProsemirrorHelper from "@server/models/helpers/ProsemirrorHelper";
 
 type Props = {
   /** The user updating the comment */
@@ -29,6 +30,10 @@ export default async function commentUpdater({
   ip,
   transaction,
 }: Props): Promise<Comment> {
+  const mentionIdsBefore = ProsemirrorHelper.parseMentions(
+    ProsemirrorHelper.toProsemirror(comment.data)
+  ).map((mention) => mention.id);
+
   if (resolvedBy !== undefined) {
     comment.resolvedBy = resolvedBy;
   }
@@ -36,6 +41,14 @@ export default async function commentUpdater({
     comment.data = data;
   }
 
+  const mentionsAfter = ProsemirrorHelper.parseMentions(
+    ProsemirrorHelper.toProsemirror(comment.data)
+  );
+
+  const newMentionIds = mentionsAfter
+    .filter((mention) => !mentionIdsBefore.includes(mention.id))
+    .map((mention) => mention.id);
+
   await comment.save({ transaction });
 
   await Event.create(
@@ -46,6 +59,9 @@ export default async function commentUpdater({
       actorId: user.id,
       documentId: comment.documentId,
       ip,
+      data: {
+        newMentionIds,
+      },
     },
     { transaction }
   );

server/emails/templates/CommentMentionedEmail.tsx

@@ -0,0 +1,143 @@
+import inlineCss from "inline-css";
+import * as React from "react";
+import { NotificationEventType } from "@shared/types";
+import env from "@server/env";
+import { Document, User } from "@server/models";
+import NotificationSettingsHelper from "@server/models/helpers/NotificationSettingsHelper";
+import BaseEmail, { EmailProps } from "./BaseEmail";
+import Body from "./components/Body";
+import Button from "./components/Button";
+import Diff from "./components/Diff";
+import EmailTemplate from "./components/EmailLayout";
+import EmptySpace from "./components/EmptySpace";
+import Footer from "./components/Footer";
+import Header from "./components/Header";
+import Heading from "./components/Heading";
+
+type InputProps = EmailProps & {
+  userId: string;
+  documentId: string;
+  actorName: string;
+  commentId: string;
+  collectionName: string | undefined;
+  teamUrl: string;
+  content: string;
+};
+
+type BeforeSend = {
+  document: Document;
+  body: string | undefined;
+  unsubscribeUrl: string;
+};
+
+type Props = InputProps & BeforeSend;
+
+/**
+ * Email sent to a user when a new comment is created in a document they are
+ * subscribed to.
+ */
+export default class CommentMentionedEmail extends BaseEmail<
+  InputProps,
+  BeforeSend
+> {
+  protected async beforeSend({ documentId, userId, content }: InputProps) {
+    const document = await Document.unscoped().findByPk(documentId);
+    if (!document) {
+      return false;
+    }
+
+    const user = await User.findByPk(userId);
+    if (!user) {
+      return false;
+    }
+
+    // inline all css so that it works in as many email providers as possible.
+    let body;
+    if (content) {
+      body = await inlineCss(content, {
+        url: env.URL,
+        applyStyleTags: true,
+        applyLinkTags: false,
+        removeStyleTags: true,
+      });
+    }
+
+    return {
+      document,
+      body,
+      unsubscribeUrl: NotificationSettingsHelper.unsubscribeUrl(
+        user,
+        NotificationEventType.Mentioned
+      ),
+    };
+  }
+
+  protected subject({ actorName, document }: Props) {
+    return `${actorName} mentioned you in “${document.title}”`;
+  }
+
+  protected preview({ actorName }: Props): string {
+    return `${actorName} mentioned you in a thread`;
+  }
+
+  protected fromName({ actorName }: Props): string {
+    return actorName;
+  }
+
+  protected renderAsText({
+    actorName,
+    teamUrl,
+    document,
+    commentId,
+    collectionName,
+  }: Props): string {
+    return `
+${actorName} mentioned you in a comment on "${document.title}"${
+      collectionName ? `in the ${collectionName} collection` : ""
+    }.
+
+Open Thread: ${teamUrl}${document.url}?commentId=${commentId}
+`;
+  }
+
+  protected render({
+    document,
+    actorName,
+    collectionName,
+    teamUrl,
+    commentId,
+    unsubscribeUrl,
+    body,
+  }: Props) {
+    const link = `${teamUrl}${document.url}?commentId=${commentId}&ref=notification-email`;
+
+    return (
+      <EmailTemplate>
+        <Header />
+
+        <Body>
+          <Heading>{document.title}</Heading>
+          <p>
+            {actorName} mentioned you in a comment on{" "}
+            <a href={link}>{document.title}</a>{" "}
+            {collectionName ? `in the ${collectionName} collection` : ""}.
+          </p>
+          {body && (
+            <>
+              <EmptySpace height={20} />
+              <Diff>
+                <div dangerouslySetInnerHTML={{ __html: body }} />
+              </Diff>
+              <EmptySpace height={20} />
+            </>
+          )}
+          <p>
+            <Button href={link}>Open Thread</Button>
+          </p>
+        </Body>
+
+        <Footer unsubscribeUrl={unsubscribeUrl} />
+      </EmailTemplate>
+    );
+  }
+}

server/emails/templates/DocumentMentionedEmail.tsx

@@ -23,7 +23,7 @@ type Props = InputProps & BeforeSend;
 /**
  * Email sent to a user when someone mentions them in a document.
  */
-export default class MentionNotificationEmail extends BaseEmail<
+export default class DocumentMentionedEmail extends BaseEmail<
   InputProps,
   BeforeSend
 > {

server/emails/templates/DocumentPublishedOrUpdatedEmail.tsx

@@ -38,7 +38,7 @@ type Props = InputProps & BeforeSend;
  * Email sent to a user when they have enabled document notifications, the event
  * may be published or updated.
  */
-export default class DocumentNotificationEmail extends BaseEmail<
+export default class DocumentPublishedOrUpdatedEmail extends BaseEmail<
   InputProps,
   BeforeSend
 > {

server/queues/processors/NotificationsProcessor.test.ts

@@ -1,5 +1,5 @@
 import { NotificationEventType } from "@shared/types";
-import DocumentNotificationEmail from "@server/emails/templates/DocumentNotificationEmail";
+import DocumentPublishedOrUpdatedEmail from "@server/emails/templates/DocumentPublishedOrUpdatedEmail";
 import {
   View,
   Subscription,
@@ -26,7 +26,7 @@ beforeEach(async () => {
 describe("documents.publish", () => {
   test("should not send a notification to author", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
 
@@ -55,7 +55,7 @@ describe("documents.publish", () => {
 
   test("should send a notification to other users in team", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const user = await buildUser();
@@ -82,7 +82,7 @@ describe("documents.publish", () => {
 
   test("should send only one notification in a 12-hour window", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const user = await buildUser();
@@ -125,7 +125,7 @@ describe("documents.publish", () => {
 
   test("should not send a notification to users without collection access", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const user = await buildUser();
@@ -159,7 +159,7 @@ describe("documents.publish", () => {
 describe("revisions.create", () => {
   test("should send a notification to other collaborators", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -187,7 +187,7 @@ describe("revisions.create", () => {
 
   test("should not send a notification if viewed since update", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -219,7 +219,7 @@ describe("revisions.create", () => {
 
   test("should not send a notification to last editor", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const user = await buildUser();
@@ -247,7 +247,7 @@ describe("revisions.create", () => {
 
   test("should send a notification for subscriptions, even to collaborator", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -331,7 +331,7 @@ describe("revisions.create", () => {
 
   test("should not send multiple emails", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const collaborator0 = await buildUser();
@@ -382,7 +382,7 @@ describe("revisions.create", () => {
 
   test("should not create subscriptions if previously unsubscribed", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const collaborator0 = await buildUser();
@@ -445,7 +445,7 @@ describe("revisions.create", () => {
 
   test("should send a notification for subscriptions to non-collaborators", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -487,7 +487,7 @@ describe("revisions.create", () => {
 
   test("should not send a notification for subscriptions to collaborators if unsubscribed", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -532,7 +532,7 @@ describe("revisions.create", () => {
 
   test("should not send a notification for subscriptions to members outside of the team", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -580,7 +580,7 @@ describe("revisions.create", () => {
 
   test("should not send a notification if viewed since update", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const document = await buildDocument();
@@ -610,7 +610,7 @@ describe("revisions.create", () => {
 
   test("should not send a notification to last editor", async () => {
     const schedule = jest.spyOn(
-      DocumentNotificationEmail.prototype,
+      DocumentPublishedOrUpdatedEmail.prototype,
       "schedule"
     );
     const user = await buildUser();

server/queues/processors/NotificationsProcessor.ts

@@ -6,8 +6,8 @@ import { Minute } from "@shared/utils/time";
 import subscriptionCreator from "@server/commands/subscriptionCreator";
 import { sequelize } from "@server/database/sequelize";
 import CollectionCreatedEmail from "@server/emails/templates/CollectionCreatedEmail";
-import DocumentNotificationEmail from "@server/emails/templates/DocumentNotificationEmail";
+import DocumentMentionedEmail from "@server/emails/templates/DocumentMentionedEmail";
-import MentionNotificationEmail from "@server/emails/templates/MentionNotificationEmail";
+import DocumentPublishedOrUpdatedEmail from "@server/emails/templates/DocumentPublishedOrUpdatedEmail";
 import env from "@server/env";
 import Logger from "@server/logging/Logger";
 import {
@@ -29,6 +29,7 @@ import {
   CommentEvent,
 } from "@server/types";
 import CommentCreatedNotificationTask from "../tasks/CommentCreatedNotificationTask";
+import CommentUpdatedNotificationTask from "../tasks/CommentUpdatedNotificationTask";
 import BaseProcessor from "./BaseProcessor";
 
 export default class NotificationsProcessor extends BaseProcessor {
@@ -37,6 +38,7 @@ export default class NotificationsProcessor extends BaseProcessor {
     "revisions.create",
     "collections.create",
     "comments.create",
+    "comments.update",
   ];
 
   async perform(event: Event) {
@@ -49,6 +51,8 @@ export default class NotificationsProcessor extends BaseProcessor {
         return this.collectionCreated(event);
       case "comments.create":
         return this.commentCreated(event);
+      case "comments.update":
+        return this.commentUpdated(event);
       default:
     }
   }
@@ -59,6 +63,12 @@ export default class NotificationsProcessor extends BaseProcessor {
     });
   }
 
+  async commentUpdated(event: CommentEvent) {
+    await CommentUpdatedNotificationTask.schedule(event, {
+      delay: Minute,
+    });
+  }
+
   async documentPublished(event: DocumentEvent) {
     // never send notifications when batch importing documents
     if (
@@ -104,7 +114,7 @@ export default class NotificationsProcessor extends BaseProcessor {
           documentId: document.id,
         });
         userIdsSentNotifications.push(recipient.id);
-        await new MentionNotificationEmail(
+        await new DocumentMentionedEmail(
           {
             to: recipient.email,
             documentId: event.documentId,
@@ -137,7 +147,7 @@ export default class NotificationsProcessor extends BaseProcessor {
           teamId: team.id,
           documentId: document.id,
         });
-        await new DocumentNotificationEmail(
+        await new DocumentPublishedOrUpdatedEmail(
           {
             to: recipient.email,
             userId: recipient.id,
@@ -193,7 +203,7 @@ export default class NotificationsProcessor extends BaseProcessor {
           documentId: document.id,
         });
         userIdsSentNotifications.push(recipient.id);
-        await new MentionNotificationEmail(
+        await new DocumentMentionedEmail(
           {
             to: recipient.email,
             documentId: event.documentId,
@@ -241,7 +251,7 @@ export default class NotificationsProcessor extends BaseProcessor {
           documentId: document.id,
         });
 
-        await new DocumentNotificationEmail(
+        await new DocumentPublishedOrUpdatedEmail(
           {
             to: recipient.email,
             userId: recipient.id,

server/queues/tasks/CommentCreatedNotificationTask.ts

@@ -4,6 +4,7 @@ import subscriptionCreator from "@server/commands/subscriptionCreator";
 import { sequelize } from "@server/database/sequelize";
 import { schema } from "@server/editor";
 import CommentCreatedEmail from "@server/emails/templates/CommentCreatedEmail";
+import CommentMentionedEmail from "@server/emails/templates/CommentMentionedEmail";
 import { Comment, Document, Notification, Team, User } from "@server/models";
 import DocumentHelper from "@server/models/helpers/DocumentHelper";
 import NotificationHelper from "@server/models/helpers/NotificationHelper";
@@ -82,13 +83,12 @@ export default class CommentCreatedNotificationTask extends BaseTask<
         });
         userIdsSentNotifications.push(recipient.id);
 
-        await new CommentCreatedEmail(
+        await new CommentMentionedEmail(
           {
             to: recipient.email,
             userId: recipient.id,
             documentId: document.id,
             teamUrl: team.url,
-            isReply: !!comment.parentCommentId,
             actorName: comment.createdBy.name,
             commentId: comment.id,
             content,

server/queues/tasks/CommentUpdatedNotificationTask.ts

@@ -0,0 +1,91 @@
+import { NotificationEventType } from "@shared/types";
+import CommentMentionedEmail from "@server/emails/templates/CommentMentionedEmail";
+import { Comment, Document, Notification, Team, User } from "@server/models";
+import DocumentHelper from "@server/models/helpers/DocumentHelper";
+import ProsemirrorHelper from "@server/models/helpers/ProsemirrorHelper";
+import { CommentEvent, CommentUpdateEvent } from "@server/types";
+import BaseTask, { TaskPriority } from "./BaseTask";
+
+export default class CommentUpdatedNotificationTask extends BaseTask<
+  CommentEvent
+> {
+  public async perform(event: CommentUpdateEvent) {
+    const [document, comment, team] = await Promise.all([
+      Document.scope("withCollection").findOne({
+        where: {
+          id: event.documentId,
+        },
+      }),
+      Comment.findByPk(event.modelId),
+      Team.findByPk(event.teamId),
+    ]);
+    if (!document || !comment || !team) {
+      return;
+    }
+
+    const mentions = ProsemirrorHelper.parseMentions(
+      ProsemirrorHelper.toProsemirror(comment.data)
+    ).filter((mention) => event.data.newMentionIds.includes(mention.id));
+    if (mentions.length === 0) {
+      return;
+    }
+
+    let content = ProsemirrorHelper.toHTML(
+      ProsemirrorHelper.toProsemirror(comment.data),
+      {
+        centered: false,
+      }
+    );
+    if (!content) {
+      return;
+    }
+
+    content = await DocumentHelper.attachmentsToSignedUrls(
+      content,
+      event.teamId,
+      86400 * 4
+    );
+
+    for (const mention of mentions) {
+      const [recipient, actor] = await Promise.all([
+        User.findByPk(mention.modelId),
+        User.findByPk(mention.actorId),
+      ]);
+      if (
+        recipient &&
+        actor &&
+        recipient.id !== actor.id &&
+        recipient.subscribedToEventType(NotificationEventType.Mentioned)
+      ) {
+        const notification = await Notification.create({
+          event: event.name,
+          userId: recipient.id,
+          actorId: actor.id,
+          teamId: team.id,
+          documentId: document.id,
+        });
+
+        await new CommentMentionedEmail(
+          {
+            to: recipient.email,
+            userId: recipient.id,
+            documentId: document.id,
+            teamUrl: team.url,
+            actorName: comment.createdBy.name,
+            commentId: comment.id,
+            content,
+            collectionName: document.collection?.name,
+          },
+          { notificationId: notification.id }
+        ).schedule();
+      }
+    }
+  }
+
+  public get options() {
+    return {
+      attempts: 1,
+      priority: TaskPriority.Background,
+    };
+  }
+}

server/types.ts

@@ -279,13 +279,24 @@ export type PinEvent = BaseEvent & {
   collectionId?: string;
 };
 
+export type CommentUpdateEvent = BaseEvent & {
+  name: "comments.update";
+  modelId: string;
+  documentId: string;
+  actorId: string;
+  data: {
+    newMentionIds: string[];
+  };
+};
+
 export type CommentEvent =
   | (BaseEvent & {
-      name: "comments.create" | "comments.update";
+      name: "comments.create";
       modelId: string;
       documentId: string;
       actorId: string;
     })
+  | CommentUpdateEvent
   | (BaseEvent & {
       name: "comments.delete";
       modelId: string;