From 91d8d27f2d06b513643ffe366f6f45618109019f Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 24 Sep 2022 23:29:11 +0200 Subject: [PATCH] feat: Render diffs in email notifications (#4164) * deps * diffCompact * Diffs in email * test * fix: Fade deleted images fix: Don't include empty paragraphs as context fix: Allow for same image multiple times and refactor * Remove target _blank * fix: Table heading incorrect color --- app/typings/styled-components.d.ts | 1 - server/commands/revisionCreator.ts | 14 +- .../templates/DocumentNotificationEmail.tsx | 27 +++- server/emails/templates/InviteEmail.tsx | 2 +- .../emails/templates/InviteReminderEmail.tsx | 4 +- server/emails/templates/WelcomeEmail.tsx | 4 +- server/emails/templates/components/Diff.tsx | 25 +++ server/models/helpers/DocumentHelper.test.ts | 83 ++++++++++ server/models/helpers/DocumentHelper.tsx | 147 ++++++++++++++++-- server/presenters/document.ts | 30 +--- server/queues/processors/DebounceProcessor.ts | 2 +- .../processors/NotificationsProcessor.test.ts | 38 +++-- .../processors/NotificationsProcessor.ts | 96 ++++++++++-- shared/editor/components/Styles.ts | 6 +- shared/styles/theme.ts | 2 - 15 files changed, 396 insertions(+), 85 deletions(-) create mode 100644 server/emails/templates/components/Diff.tsx create mode 100644 server/models/helpers/DocumentHelper.test.ts diff --git a/app/typings/styled-components.d.ts b/app/typings/styled-components.d.ts index c8e571d58..a51e8e5d0 100644 --- a/app/typings/styled-components.d.ts +++ b/app/typings/styled-components.d.ts @@ -16,7 +16,6 @@ declare module "styled-components" { tableDivider: string; tableSelected: string; tableSelectedBackground: string; - tableHeaderBackground: string; quote: string; codeBackground: string; codeBorder: string; diff --git a/server/commands/revisionCreator.ts b/server/commands/revisionCreator.ts index 9ce4db287..962b8fe7f 100644 --- a/server/commands/revisionCreator.ts +++ b/server/commands/revisionCreator.ts @@ -10,10 +10,7 @@ export default async function revisionCreator({ user: User; ip?: string; }) { - let transaction; - - try { - transaction = await sequelize.transaction(); + return sequelize.transaction(async (transaction) => { const revision = await Revision.createFromDocument(document, { transaction, }); @@ -32,13 +29,6 @@ export default async function revisionCreator({ transaction, } ); - await transaction.commit(); return revision; - } catch (err) { - if (transaction) { - await transaction.rollback(); - } - - throw err; - } + }); } diff --git a/server/emails/templates/DocumentNotificationEmail.tsx b/server/emails/templates/DocumentNotificationEmail.tsx index f7c7a54f4..d99a1693b 100644 --- a/server/emails/templates/DocumentNotificationEmail.tsx +++ b/server/emails/templates/DocumentNotificationEmail.tsx @@ -3,6 +3,7 @@ import { Document } from "@server/models"; import BaseEmail 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"; @@ -17,6 +18,7 @@ type InputProps = { eventName: string; teamUrl: string; unsubscribeUrl: string; + content: string; }; type BeforeSend = { @@ -73,25 +75,34 @@ Open Document: ${teamUrl}${document.url} eventName = "published", teamUrl, unsubscribeUrl, + content, }: Props) { + const link = `${teamUrl}${document.url}?ref=notification-email`; + return (
- "{document.title}" {eventName} + “{document.title}” {eventName}

- {actorName} {eventName} the document "{document.title}", in the{" "} - {collectionName} collection. + {actorName} {eventName} the document{" "} + {document.title}, in the {collectionName}{" "} + collection.

-
- -

{document.getSummary()}

- + {content && ( + <> + + +
+ + + + )}

- +

diff --git a/server/emails/templates/InviteEmail.tsx b/server/emails/templates/InviteEmail.tsx index c7259fa56..a80f0fffe 100644 --- a/server/emails/templates/InviteEmail.tsx +++ b/server/emails/templates/InviteEmail.tsx @@ -57,7 +57,7 @@ Join now: ${teamUrl}

- +

diff --git a/server/emails/templates/InviteReminderEmail.tsx b/server/emails/templates/InviteReminderEmail.tsx index 6dc6a6f61..b06f83ecb 100644 --- a/server/emails/templates/InviteReminderEmail.tsx +++ b/server/emails/templates/InviteReminderEmail.tsx @@ -59,7 +59,9 @@ If you haven't signed up yet, you can do so here: ${teamUrl}

If you haven't signed up yet, you can do so here:

- +

diff --git a/server/emails/templates/WelcomeEmail.tsx b/server/emails/templates/WelcomeEmail.tsx index 1c787f8c6..d8fe13fba 100644 --- a/server/emails/templates/WelcomeEmail.tsx +++ b/server/emails/templates/WelcomeEmail.tsx @@ -59,7 +59,9 @@ ${teamUrl}/home

- +

diff --git a/server/emails/templates/components/Diff.tsx b/server/emails/templates/components/Diff.tsx new file mode 100644 index 000000000..ad73ae906 --- /dev/null +++ b/server/emails/templates/components/Diff.tsx @@ -0,0 +1,25 @@ +import * as React from "react"; +import theme from "@shared/styles/theme"; + +type Props = { + children: React.ReactNode; + href?: string; +}; + +export default ({ children, ...rest }: Props) => { + const style = { + borderRadius: "4px", + background: theme.secondaryBackground, + padding: ".75em 1em", + color: theme.text, + display: "block", + textDecoration: "none", + width: "100%", + }; + + return ( +
+ {children} +
+ ); +}; diff --git a/server/models/helpers/DocumentHelper.test.ts b/server/models/helpers/DocumentHelper.test.ts new file mode 100644 index 000000000..4fe08379d --- /dev/null +++ b/server/models/helpers/DocumentHelper.test.ts @@ -0,0 +1,83 @@ +import Revision from "@server/models/Revision"; +import DocumentHelper from "./DocumentHelper"; + +describe("toEmailDiff", () => { + test("toEmailDiff", () => { + const before = new Revision({ + title: "Title", + text: ` +This is a test paragraph + +- list item 1 +- list item 2 + +:::info +Content in an info block +::: + +!!This is a placeholder!! + +==this is a highlight== + +- [ ] checklist item 1 +- [ ] checklist item 2 +- [x] checklist item 3 + +same on both sides + +same on both sides + +same on both sides`, + }); + + const after = new Revision({ + title: "Title", + text: ` +This is a test paragraph + +A new paragraph + +- list item 1 + +This is a new paragraph. + +!!This is a placeholder!! + +==this is a highlight== + +- [x] checklist item 1 +- [x] checklist item 2 +- [ ] checklist item 3 +- [ ] checklist item 4 +- [x] checklist item 5 + +same on both sides + +same on both sides + +same on both sides`, + }); + + const html = DocumentHelper.toEmailDiff(before, after); + + // marks breaks in diff + expect(html).toContain("diff-context-break"); + + // changed list + expect(html).toContain("checklist item 1"); + expect(html).toContain("checklist item 5"); + + // added + expect(html).toContain("A new paragraph"); + + // Retained for context above added paragraph + expect(html).toContain("This is a test paragraph"); + + // removed + expect(html).toContain("Content in an info block"); + + // unchanged + expect(html).not.toContain("same on both sides"); + expect(html).not.toContain("this is a highlight"); + }); +}); diff --git a/server/models/helpers/DocumentHelper.tsx b/server/models/helpers/DocumentHelper.tsx index efc0688eb..3c3eda484 100644 --- a/server/models/helpers/DocumentHelper.tsx +++ b/server/models/helpers/DocumentHelper.tsx @@ -3,6 +3,7 @@ import { yDocToProsemirrorJSON, } from "@getoutline/y-prosemirror"; import { JSDOM } from "jsdom"; +import { escapeRegExp } from "lodash"; import diff from "node-htmldiff"; import { Node, DOMSerializer } from "prosemirror-model"; import * as React from "react"; @@ -18,12 +19,17 @@ import { parser, schema } from "@server/editor"; import Logger from "@server/logging/Logger"; import Document from "@server/models/Document"; import type Revision from "@server/models/Revision"; +import parseAttachmentIds from "@server/utils/parseAttachmentIds"; +import { getSignedUrl } from "@server/utils/s3"; +import Attachment from "../Attachment"; type HTMLOptions = { /** Whether to include the document title in the generated HTML (defaults to true) */ includeTitle?: boolean; /** Whether to include style tags in the generated HTML (defaults to true) */ includeStyles?: boolean; + /** Whether to include styles to center diff (defaults to true) */ + centered?: boolean; }; export default class DocumentHelper { @@ -73,11 +79,13 @@ export default class DocumentHelper { const sheet = new ServerStyleSheet(); let html, styleTags; - const Centered = styled.article` - max-width: 46em; - margin: 0 auto; - padding: 0 1em; - `; + const Centered = options?.centered + ? styled.article` + max-width: 46em; + margin: 0 auto; + padding: 0 1em; + ` + : "article"; const rtl = isRTL(document.title); const children = ( @@ -142,7 +150,7 @@ export default class DocumentHelper { } /** - * Generates a HTML diff between after documents or revisions. + * Generates a HTML diff between documents or revisions. * * @param before The before document * @param after The after document @@ -172,12 +180,133 @@ export default class DocumentHelper { // Inject the diffed content into the original document with styling and // serialize back to a string. - beforeDOM.window.document.getElementsByTagName( - "article" - )[0].innerHTML = diffedContentAsHTML; + const article = beforeDOM.window.document.querySelector("article"); + if (article) { + article.innerHTML = diffedContentAsHTML; + } return beforeDOM.serialize(); } + /** + * Generates a compact HTML diff between documents or revisions, the + * diff is reduced up to show only the parts of the document that changed and + * the immediate context. Breaks in the diff are denoted with + * "div.diff-context-break" nodes. + * + * @param before The before document + * @param after The after document + * @param options Options passed to HTML generation + * @returns The diff as a HTML string + */ + static toEmailDiff( + before: Document | Revision | null, + after: Revision, + options?: HTMLOptions + ) { + if (!before) { + return ""; + } + + const html = DocumentHelper.diff(before, after, options); + const dom = new JSDOM(html); + const doc = dom.window.document; + + const containsDiffElement = (node: Element | null) => { + return node && node.innerHTML.includes("data-operation-index"); + }; + + // We use querySelectorAll to get a static NodeList as we'll be modifying + // it as we iterate, rather than getting content.childNodes. + const contents = doc.querySelectorAll("#content > *"); + let previousNodeRemoved = false; + let previousDiffClipped = false; + + const br = doc.createElement("div"); + br.innerHTML = "…"; + br.className = "diff-context-break"; + + for (const childNode of contents) { + // If the block node contains a diff tag then we want to keep it + if (containsDiffElement(childNode as Element)) { + if (previousNodeRemoved && previousDiffClipped) { + childNode.parentElement?.insertBefore(br.cloneNode(true), childNode); + } + previousNodeRemoved = false; + previousDiffClipped = true; + + // If the block node does not contain a diff tag and the previous + // block node did not contain a diff tag then remove the previous. + } else { + if ( + childNode.nodeName === "P" && + childNode.textContent && + childNode.nextElementSibling?.nodeName === "P" && + containsDiffElement(childNode.nextElementSibling) + ) { + if (previousDiffClipped) { + childNode.parentElement?.insertBefore( + br.cloneNode(true), + childNode + ); + } + previousNodeRemoved = false; + continue; + } + if ( + childNode.nodeName === "P" && + childNode.textContent && + childNode.previousElementSibling?.nodeName === "P" && + containsDiffElement(childNode.previousElementSibling) + ) { + previousNodeRemoved = false; + continue; + } + previousNodeRemoved = true; + childNode.remove(); + } + } + + const head = doc.querySelector("head"); + const body = doc.querySelector("body"); + return `${head?.innerHTML} ${body?.innerHTML}`; + } + + /** + * Converts attachment urls in documents to signed equivalents that allow + * direct access without a session cookie + * + * @param text The text either html or markdown which contains urls to be converted + * @param teamId The team context + * @param expiresIn The time that signed urls should expire in (ms) + * @returns The replaced text + */ + static async attachmentsToSignedUrls( + text: string, + teamId: string, + expiresIn = 3000 + ) { + const attachmentIds = parseAttachmentIds(text); + await Promise.all( + attachmentIds.map(async (id) => { + const attachment = await Attachment.findOne({ + where: { + id, + teamId, + }, + }); + + if (attachment) { + const signedUrl = await getSignedUrl(attachment.key, expiresIn); + text = text.replace( + new RegExp(escapeRegExp(attachment.redirectUrl), "g"), + signedUrl + ); + } + }) + ); + return text; + } + /** * Applies the given Markdown to the document, this essentially creates a * single change in the collaborative state that makes all the edits to get diff --git a/server/presenters/document.ts b/server/presenters/document.ts index 59d9d734b..67402206b 100644 --- a/server/presenters/document.ts +++ b/server/presenters/document.ts @@ -1,35 +1,12 @@ -import { escapeRegExp } from "lodash"; import { APM } from "@server/logging/tracing"; import { Document } from "@server/models"; -import Attachment from "@server/models/Attachment"; -import parseAttachmentIds from "@server/utils/parseAttachmentIds"; -import { getSignedUrl } from "@server/utils/s3"; +import DocumentHelper from "@server/models/helpers/DocumentHelper"; import presentUser from "./user"; type Options = { isPublic?: boolean; }; -// replaces attachments.redirect urls with signed/authenticated url equivalents -async function replaceImageAttachments(text: string) { - const attachmentIds = parseAttachmentIds(text); - await Promise.all( - attachmentIds.map(async (id) => { - const attachment = await Attachment.findByPk(id); - - if (attachment) { - const signedUrl = await getSignedUrl(attachment.key, 3600); - text = text.replace( - new RegExp(escapeRegExp(attachment.redirectUrl), "g"), - signedUrl - ); - } - }) - ); - - return text; -} - async function present( document: Document, options: Options | null | undefined = {} @@ -40,7 +17,10 @@ async function present( }; await document.migrateVersion(); const text = options.isPublic - ? await replaceImageAttachments(document.text) + ? await DocumentHelper.attachmentsToSignedUrls( + document.text, + document.teamId + ) : document.text; const data: Record = { diff --git a/server/queues/processors/DebounceProcessor.ts b/server/queues/processors/DebounceProcessor.ts index 393636a54..13f2dc75b 100644 --- a/server/queues/processors/DebounceProcessor.ts +++ b/server/queues/processors/DebounceProcessor.ts @@ -18,7 +18,7 @@ export default class DebounceProcessor extends BaseProcessor { { // speed up revision creation in development, we don't have all the // time in the world. - delay: (env.ENVIRONMENT === "development" ? 1 : 5) * 60 * 1000, + delay: (env.ENVIRONMENT === "development" ? 0.5 : 5) * 60 * 1000, } ); break; diff --git a/server/queues/processors/NotificationsProcessor.test.ts b/server/queues/processors/NotificationsProcessor.test.ts index f040a57e9..582690eb9 100644 --- a/server/queues/processors/NotificationsProcessor.test.ts +++ b/server/queues/processors/NotificationsProcessor.test.ts @@ -5,6 +5,7 @@ import { Subscription, Event, Notification, + Revision, } from "@server/models"; import { buildDocument, @@ -156,6 +157,7 @@ describe("documents.publish", () => { describe("revisions.create", () => { test("should send a notification to other collaborators", async () => { const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); const collaborator = await buildUser({ teamId: document.teamId }); document.collaboratorIds = [collaborator.id]; await document.save(); @@ -171,7 +173,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); expect(DocumentNotificationEmail.schedule).toHaveBeenCalled(); @@ -179,6 +181,7 @@ describe("revisions.create", () => { test("should not send a notification if viewed since update", async () => { const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); const collaborator = await buildUser({ teamId: document.teamId }); document.collaboratorIds = [collaborator.id]; await document.save(); @@ -196,7 +199,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled(); @@ -208,6 +211,7 @@ describe("revisions.create", () => { teamId: user.teamId, lastModifiedById: user.id, }); + const revision = await Revision.createFromDocument(document); await NotificationSetting.create({ userId: user.id, teamId: user.teamId, @@ -220,7 +224,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: user.id, - modelId: document.id, + modelId: revision.id, ip, }); expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled(); @@ -228,6 +232,7 @@ describe("revisions.create", () => { test("should send a notification for subscriptions, even to collaborator", async () => { const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); const collaborator = await buildUser({ teamId: document.teamId }); const subscriber = await buildUser({ teamId: document.teamId }); @@ -256,7 +261,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -268,6 +273,7 @@ describe("revisions.create", () => { const collaborator1 = await buildUser({ teamId: collaborator0.teamId }); const collaborator2 = await buildUser({ teamId: collaborator0.teamId }); const document = await buildDocument({ userId: collaborator0.id }); + const revision = await Revision.createFromDocument(document); await document.update({ collaboratorIds: [collaborator0.id, collaborator1.id, collaborator2.id], @@ -281,7 +287,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator0.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -312,6 +318,7 @@ describe("revisions.create", () => { teamId: collaborator0.teamId, userId: collaborator0.id, }); + const revision = await Revision.createFromDocument(document); await document.update({ collaboratorIds: [collaborator0.id, collaborator1.id, collaborator2.id], @@ -338,7 +345,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator0.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -355,6 +362,7 @@ describe("revisions.create", () => { teamId: collaborator0.teamId, userId: collaborator0.id, }); + const revision = await Revision.createFromDocument(document); await document.update({ collaboratorIds: [collaborator0.id, collaborator1.id, collaborator2.id], @@ -378,7 +386,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator0.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -406,6 +414,7 @@ describe("revisions.create", () => { const document = await buildDocument(); const collaborator = await buildUser({ teamId: document.teamId }); const subscriber = await buildUser({ teamId: document.teamId }); + const revision = await Revision.createFromDocument(document); // `subscriber` hasn't collaborated on `document`. document.collaboratorIds = [collaborator.id]; @@ -435,7 +444,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -444,6 +453,7 @@ describe("revisions.create", () => { test("should not send a notification for subscriptions to collaborators if unsubscribed", async () => { const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); const collaborator = await buildUser({ teamId: document.teamId }); const subscriber = await buildUser({ teamId: document.teamId }); @@ -477,7 +487,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -487,6 +497,7 @@ describe("revisions.create", () => { test("should not send a notification for subscriptions to members outside of the team", async () => { const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); const collaborator = await buildUser({ teamId: document.teamId }); // `subscriber` *does not* belong @@ -523,7 +534,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); @@ -533,6 +544,7 @@ describe("revisions.create", () => { test("should not send a notification if viewed since update", async () => { const document = await buildDocument(); + const revision = await Revision.createFromDocument(document); const collaborator = await buildUser({ teamId: document.teamId }); document.collaboratorIds = [collaborator.id]; await document.save(); @@ -551,7 +563,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: collaborator.id, - modelId: document.id, + modelId: revision.id, ip, }); expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled(); @@ -563,6 +575,8 @@ describe("revisions.create", () => { teamId: user.teamId, lastModifiedById: user.id, }); + const revision = await Revision.createFromDocument(document); + await NotificationSetting.create({ userId: user.id, teamId: user.teamId, @@ -575,7 +589,7 @@ describe("revisions.create", () => { collectionId: document.collectionId, teamId: document.teamId, actorId: user.id, - modelId: document.id, + modelId: revision.id, ip, }); expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled(); diff --git a/server/queues/processors/NotificationsProcessor.ts b/server/queues/processors/NotificationsProcessor.ts index 1be7728bc..2f7df8c18 100644 --- a/server/queues/processors/NotificationsProcessor.ts +++ b/server/queues/processors/NotificationsProcessor.ts @@ -5,6 +5,7 @@ import subscriptionCreator from "@server/commands/subscriptionCreator"; import { sequelize } from "@server/database/sequelize"; import CollectionNotificationEmail from "@server/emails/templates/CollectionNotificationEmail"; import DocumentNotificationEmail from "@server/emails/templates/DocumentNotificationEmail"; +import env from "@server/env"; import Logger from "@server/logging/Logger"; import { View, @@ -15,7 +16,9 @@ import { NotificationSetting, Subscription, Notification, + Revision, } from "@server/models"; +import DocumentHelper from "@server/models/helpers/DocumentHelper"; import { CollectionEvent, RevisionEvent, @@ -34,9 +37,9 @@ export default class NotificationsProcessor extends BaseProcessor { async perform(event: Event) { switch (event.name) { case "documents.publish": + return this.documentPublished(event); case "revisions.create": - return this.documentUpdated(event); - + return this.revisionCreated(event); case "collections.create": return this.collectionCreated(event); @@ -44,10 +47,13 @@ export default class NotificationsProcessor extends BaseProcessor { } } - async documentUpdated(event: DocumentEvent | RevisionEvent) { + async documentPublished(event: DocumentEvent) { // never send notifications when batch importing documents - // @ts-expect-error ts-migrate(2339) FIXME: Property 'data' does not exist on type 'DocumentEv... Remove this comment to see the full error message - if (event.data?.source === "import") { + if ( + "data" in event && + "source" in event.data && + event.data.source === "import" + ) { return; } @@ -65,9 +71,7 @@ export default class NotificationsProcessor extends BaseProcessor { const recipients = await this.getDocumentNotificationRecipients( document, - event.name === "documents.publish" - ? "documents.publish" - : "documents.update" + "documents.publish" ); for (const recipient of recipients) { @@ -84,8 +88,7 @@ export default class NotificationsProcessor extends BaseProcessor { await DocumentNotificationEmail.schedule( { to: recipient.user.email, - eventName: - event.name === "documents.publish" ? "published" : "updated", + eventName: "published", documentId: document.id, teamUrl: team.url, actorName: document.updatedBy.name, @@ -98,6 +101,66 @@ export default class NotificationsProcessor extends BaseProcessor { } } + async revisionCreated(event: RevisionEvent) { + const [collection, document, revision, team] = await Promise.all([ + Collection.findByPk(event.collectionId), + Document.findByPk(event.documentId), + Revision.findByPk(event.modelId), + Team.findByPk(event.teamId), + ]); + + if (!document || !team || !revision || !collection) { + return; + } + + await this.createDocumentSubscriptions(document, event); + + const recipients = await this.getDocumentNotificationRecipients( + document, + "documents.update" + ); + + // generate the diff html for the email + const before = await revision.previous(); + let content = DocumentHelper.toEmailDiff(before, revision, { + includeTitle: false, + centered: false, + }); + content = await DocumentHelper.attachmentsToSignedUrls( + content, + event.teamId, + 86400 * 4 + ); + + for (const recipient of recipients) { + const notify = await this.shouldNotify(document, recipient.user); + + if (notify) { + const notification = await Notification.create({ + event: event.name, + userId: recipient.user.id, + actorId: document.updatedBy.id, + teamId: team.id, + documentId: document.id, + }); + + await DocumentNotificationEmail.schedule( + { + to: recipient.user.email, + eventName: "updated", + documentId: document.id, + teamUrl: team.url, + actorName: document.updatedBy.name, + collectionName: collection.name, + unsubscribeUrl: recipient.unsubscribeUrl, + content, + }, + { notificationId: notification.id } + ); + } + } + } + async collectionCreated(event: CollectionEvent) { const collection = await Collection.scope("withUser").findByPk( event.collectionId @@ -263,7 +326,18 @@ export default class NotificationsProcessor extends BaseProcessor { }); if (notification) { - return false; + if (env.ENVIRONMENT === "development") { + Logger.info( + "processor", + `would have suppressed notification to ${user.id}, but not in development` + ); + } else { + Logger.info( + "processor", + `suppressing notification to ${user.id} as recently notified` + ); + return false; + } } // If this recipient has viewed the document since the last update was made diff --git a/shared/editor/components/Styles.ts b/shared/editor/components/Styles.ts index 908894dd6..3d6f1adf0 100644 --- a/shared/editor/components/Styles.ts +++ b/shared/editor/components/Styles.ts @@ -1035,7 +1035,7 @@ table { } th { - background: ${props.theme.tableHeaderBackground}; + background: transparent; } td, @@ -1284,6 +1284,10 @@ del { text-decoration: strikethrough; } +del img { + opacity: .5; +} + @media print { .placeholder:before, .block-menu-trigger, diff --git a/shared/styles/theme.ts b/shared/styles/theme.ts index 3ff5c7cf5..129f726e3 100644 --- a/shared/styles/theme.ts +++ b/shared/styles/theme.ts @@ -130,7 +130,6 @@ export const light = { tableDivider: colors.smokeDark, tableSelected: colors.primary, tableSelectedBackground: "#E5F7FF", - tableHeaderBackground: colors.white, buttonNeutralBackground: colors.white, buttonNeutralText: colors.almostBlack, buttonNeutralBorder: darken(0.15, colors.white), @@ -188,7 +187,6 @@ export const dark = { tableDivider: colors.lightBlack, tableSelected: colors.primary, tableSelectedBackground: "#002333", - tableHeaderBackground: colors.almostBlack, buttonNeutralBackground: colors.almostBlack, buttonNeutralText: colors.white, buttonNeutralBorder: colors.slateDark,