From 45831e946990a172023b1efd83892b25ea29acb5 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 18 Mar 2023 09:32:41 -0400 Subject: [PATCH] Remove `NotificationSettings` table (#5036 * helper * Add script to move notification settings * wip, removal of NotificationSettings * event name * iteration * test * test * Remove last of NotificationSettings model * refactor * More fixes * snapshots * Change emails to class instances for type safety * test * docs * Update migration for self-hosted * tsc --- app/components/ExportDialog.tsx | 12 +- app/models/NotificationSetting.ts | 15 -- app/models/User.ts | 58 +++++- app/scenes/Settings/Notifications.tsx | 59 +++--- app/stores/NotificationSettingsStore.ts | 20 -- app/stores/RootStore.ts | 4 - plugins/email/server/auth/email.test.ts | 18 +- plugins/email/server/auth/email.ts | 17 +- .../server/tasks/DeliverWebhookTask.ts | 4 +- server/commands/accountProvisioner.test.ts | 8 +- server/commands/accountProvisioner.ts | 4 +- server/commands/teamPermanentDeleter.ts | 8 - server/commands/userInviter.ts | 4 +- server/commands/userProvisioner.ts | 4 +- server/emails/templates/BaseEmail.tsx | 26 ++- ...onEmail.tsx => CollectionCreatedEmail.tsx} | 46 +++-- .../emails/templates/CommentCreatedEmail.tsx | 46 +++-- .../templates/ConfirmUserDeleteEmail.tsx | 5 +- .../templates/DocumentNotificationEmail.tsx | 65 +++++-- .../emails/templates/ExportFailureEmail.tsx | 37 ++-- .../emails/templates/ExportSuccessEmail.tsx | 37 ++-- .../emails/templates/InviteAcceptedEmail.tsx | 25 +-- server/emails/templates/InviteEmail.tsx | 16 +- .../emails/templates/InviteReminderEmail.tsx | 16 +- .../templates/MentionNotificationEmail.tsx | 7 +- server/emails/templates/SigninEmail.tsx | 5 +- .../emails/templates/WebhookDisabledEmail.tsx | 5 +- server/emails/templates/WelcomeEmail.tsx | 5 +- ...230314013103-move-notification-settings.js | 66 +++++++ server/models/NotificationSetting.ts | 90 --------- server/models/User.ts | 99 ++++------ server/models/helpers/NotificationHelper.ts | 72 +++---- .../helpers/NotificationSettingsHelper.ts | 45 +++++ server/models/index.ts | 2 - server/policies/index.ts | 1 - server/policies/notificationSetting.ts | 16 -- server/presenters/index.ts | 2 - server/presenters/notificationSetting.ts | 10 - server/presenters/user.ts | 4 +- .../processors/NotificationsProcessor.test.ts | 176 +++++++++--------- .../processors/NotificationsProcessor.ts | 78 ++++---- .../tasks/CommentCreatedNotificationTask.ts | 31 +-- server/queues/tasks/ExportTask.ts | 32 ++-- .../queues/tasks/InviteReminderTask.test.ts | 2 +- server/queues/tasks/InviteReminderTask.ts | 4 +- server/routes/api/index.ts | 4 +- server/routes/api/notificationSettings.ts | 91 --------- server/routes/api/notifications/index.ts | 1 + .../routes/api/notifications/notifications.ts | 52 ++++++ server/routes/api/notifications/schema.ts | 44 +++++ .../__snapshots__/users.test.ts.snap | 6 + server/routes/api/users/index.ts | 1 + server/routes/api/users/schema.ts | 22 +++ server/routes/api/{ => users}/users.test.ts | 0 server/routes/api/{ => users}/users.ts | 50 ++++- ...313000000-migrate-notification-settings.ts | 62 ++++++ shared/i18n/locales/en_US/translation.json | 6 +- shared/types.ts | 38 ++++ 58 files changed, 972 insertions(+), 711 deletions(-) delete mode 100644 app/models/NotificationSetting.ts delete mode 100644 app/stores/NotificationSettingsStore.ts rename server/emails/templates/{CollectionNotificationEmail.tsx => CollectionCreatedEmail.tsx} (57%) create mode 100644 server/migrations/20230314013103-move-notification-settings.js delete mode 100644 server/models/NotificationSetting.ts create mode 100644 server/models/helpers/NotificationSettingsHelper.ts delete mode 100644 server/policies/notificationSetting.ts delete mode 100644 server/presenters/notificationSetting.ts delete mode 100644 server/routes/api/notificationSettings.ts create mode 100644 server/routes/api/notifications/index.ts create mode 100644 server/routes/api/notifications/notifications.ts create mode 100644 server/routes/api/notifications/schema.ts rename server/routes/api/{ => users}/__snapshots__/users.test.ts.snap (97%) create mode 100644 server/routes/api/users/index.ts create mode 100644 server/routes/api/users/schema.ts rename server/routes/api/{ => users}/users.test.ts (100%) rename server/routes/api/{ => users}/users.ts (90%) create mode 100644 server/scripts/20230313000000-migrate-notification-settings.ts diff --git a/app/components/ExportDialog.tsx b/app/components/ExportDialog.tsx index 061afb010..7b58210b9 100644 --- a/app/components/ExportDialog.tsx +++ b/app/components/ExportDialog.tsx @@ -2,12 +2,13 @@ import { observer } from "mobx-react"; import * as React from "react"; import { Trans, useTranslation } from "react-i18next"; import styled from "styled-components"; -import { FileOperationFormat } from "@shared/types"; +import { FileOperationFormat, NotificationEventType } from "@shared/types"; import Collection from "~/models/Collection"; import ConfirmationDialog from "~/components/ConfirmationDialog"; import Flex from "~/components/Flex"; import Text from "~/components/Text"; import env from "~/env"; +import useCurrentUser from "~/hooks/useCurrentUser"; import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; @@ -20,15 +21,12 @@ function ExportDialog({ collection, onSubmit }: Props) { const [format, setFormat] = React.useState( FileOperationFormat.MarkdownZip ); + const user = useCurrentUser(); const { showToast } = useToasts(); - const { collections, notificationSettings } = useStores(); + const { collections } = useStores(); const { t } = useTranslation(); const appName = env.APP_NAME; - React.useEffect(() => { - notificationSettings.fetchPage({}); - }, [notificationSettings]); - const handleFormatChange = React.useCallback( (ev: React.ChangeEvent) => { setFormat(ev.target.value as FileOperationFormat); @@ -86,7 +84,7 @@ function ExportDialog({ collection, onSubmit }: Props) { em: , }} />{" "} - {notificationSettings.getByEvent("emails.export_completed") && + {user.subscribedToEventType(NotificationEventType.ExportCompleted) && t("You will receive an email when it's complete.")} )} diff --git a/app/models/NotificationSetting.ts b/app/models/NotificationSetting.ts deleted file mode 100644 index d13196928..000000000 --- a/app/models/NotificationSetting.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { observable } from "mobx"; -import BaseModel from "./BaseModel"; -import Field from "./decorators/Field"; - -class NotificationSetting extends BaseModel { - @Field - @observable - id: string; - - @Field - @observable - event: string; -} - -export default NotificationSetting; diff --git a/app/models/User.ts b/app/models/User.ts index 5d70623db..be5268adb 100644 --- a/app/models/User.ts +++ b/app/models/User.ts @@ -1,7 +1,14 @@ import { subMinutes } from "date-fns"; -import { computed, observable } from "mobx"; +import { computed, action, observable } from "mobx"; import { now } from "mobx-utils"; -import type { Role, UserPreference, UserPreferences } from "@shared/types"; +import { + NotificationEventDefaults, + NotificationEventType, + UserPreference, + UserPreferences, +} from "@shared/types"; +import type { Role, NotificationSettings } from "@shared/types"; +import { client } from "~/utils/ApiClient"; import ParanoidModel from "./ParanoidModel"; import Field from "./decorators/Field"; @@ -30,6 +37,10 @@ class User extends ParanoidModel { @observable preferences: UserPreferences | null; + @Field + @observable + notificationSettings: NotificationSettings; + email: string; isAdmin: boolean; @@ -72,6 +83,49 @@ class User extends ParanoidModel { } } + /** + * Returns the current preference for the given notification event type taking + * into account the default system value. + * + * @param type The type of notification event + * @returns The current preference + */ + public subscribedToEventType = (type: NotificationEventType) => { + return ( + this.notificationSettings[type] ?? + NotificationEventDefaults[type] ?? + false + ); + }; + + /** + * Sets a preference for the users notification settings on the model and + * saves the change to the server. + * + * @param type The type of notification event + * @param value Set the preference to true/false + */ + @action + setNotificationEventType = async ( + eventType: NotificationEventType, + value: boolean + ) => { + this.notificationSettings = { + ...this.notificationSettings, + [eventType]: value, + }; + + if (value) { + await client.post(`/users.notificationsSubscribe`, { + eventType, + }); + } else { + await client.post(`/users.notificationsUnsubscribe`, { + eventType, + }); + } + }; + /** * Get the value for a specific preference key, or return the fallback if * none is set. diff --git a/app/scenes/Settings/Notifications.tsx b/app/scenes/Settings/Notifications.tsx index fbcd863d5..1ec53ee4e 100644 --- a/app/scenes/Settings/Notifications.tsx +++ b/app/scenes/Settings/Notifications.tsx @@ -3,6 +3,7 @@ import { observer } from "mobx-react"; import { EmailIcon } from "outline-icons"; import * as React from "react"; import { useTranslation, Trans } from "react-i18next"; +import { NotificationEventType } from "@shared/types"; import Heading from "~/components/Heading"; import Input from "~/components/Input"; import Notice from "~/components/Notice"; @@ -11,48 +12,60 @@ import Switch from "~/components/Switch"; import Text from "~/components/Text"; import env from "~/env"; import useCurrentUser from "~/hooks/useCurrentUser"; -import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; import isCloudHosted from "~/utils/isCloudHosted"; import SettingRow from "./components/SettingRow"; function Notifications() { - const { notificationSettings } = useStores(); const { showToast } = useToasts(); const user = useCurrentUser(); const { t } = useTranslation(); const options = [ { - event: "documents.publish", + event: NotificationEventType.PublishDocument, title: t("Document published"), description: t( "Receive a notification whenever a new document is published" ), }, { - event: "documents.update", + event: NotificationEventType.UpdateDocument, title: t("Document updated"), description: t( - "Receive a notification when a document you created is edited" + "Receive a notification when a document you are subscribed to is edited" ), }, { - event: "collections.create", + event: NotificationEventType.CreateComment, + title: t("Comment posted"), + description: t( + "Receive a notification when a document you are subscribed to or a thread you participated in receives a comment" + ), + }, + { + event: NotificationEventType.Mentioned, + title: t("Mentioned"), + description: t( + "Receive a notification when someone mentions you in a document or comment" + ), + }, + { + event: NotificationEventType.CreateCollection, title: t("Collection created"), description: t( "Receive a notification whenever a new collection is created" ), }, { - event: "emails.invite_accepted", + event: NotificationEventType.InviteAccepted, title: t("Invite accepted"), description: t( "Receive a notification when someone you invited creates an account" ), }, { - event: "emails.export_completed", + event: NotificationEventType.ExportCompleted, title: t("Export completed"), description: t( "Receive a notification when an export you requested has been completed" @@ -60,22 +73,18 @@ function Notifications() { }, { visible: isCloudHosted, - event: "emails.onboarding", + event: NotificationEventType.Onboarding, title: t("Getting started"), description: t("Tips on getting started with features and functionality"), }, { visible: isCloudHosted, - event: "emails.features", + event: NotificationEventType.Features, title: t("New features"), description: t("Receive an email when new features of note are added"), }, ]; - React.useEffect(() => { - notificationSettings.fetchPage({}); - }, [notificationSettings]); - const showSuccessMessage = debounce(() => { showToast(t("Notifications saved"), { type: "success", @@ -84,19 +93,13 @@ function Notifications() { const handleChange = React.useCallback( async (ev: React.ChangeEvent) => { - const setting = notificationSettings.getByEvent(ev.target.name); - - if (ev.target.checked) { - await notificationSettings.save({ - event: ev.target.name, - }); - } else if (setting) { - await notificationSettings.delete(setting); - } - + await user.setNotificationEventType( + ev.target.name as NotificationEventType, + ev.target.checked + ); showSuccessMessage(); }, - [notificationSettings, showSuccessMessage] + [user, showSuccessMessage] ); const showSuccessNotice = window.location.search === "?success"; @@ -130,7 +133,7 @@ function Notifications() {

{t("Notifications")}

{options.map((option) => { - const setting = notificationSettings.getByEvent(option.event); + const setting = user.subscribedToEventType(option.event); return ( ); diff --git a/app/stores/NotificationSettingsStore.ts b/app/stores/NotificationSettingsStore.ts deleted file mode 100644 index 1474f8649..000000000 --- a/app/stores/NotificationSettingsStore.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { find } from "lodash"; -import NotificationSetting from "~/models/NotificationSetting"; -import BaseStore, { RPCAction } from "./BaseStore"; -import RootStore from "./RootStore"; - -export default class NotificationSettingsStore extends BaseStore< - NotificationSetting -> { - actions = [RPCAction.List, RPCAction.Create, RPCAction.Delete]; - - constructor(rootStore: RootStore) { - super(rootStore, NotificationSetting); - } - - getByEvent = (event: string) => { - return find(this.orderedData, { - event, - }); - }; -} diff --git a/app/stores/RootStore.ts b/app/stores/RootStore.ts index 55beceac2..dcd1ee734 100644 --- a/app/stores/RootStore.ts +++ b/app/stores/RootStore.ts @@ -13,7 +13,6 @@ import GroupMembershipsStore from "./GroupMembershipsStore"; import GroupsStore from "./GroupsStore"; import IntegrationsStore from "./IntegrationsStore"; import MembershipsStore from "./MembershipsStore"; -import NotificationSettingsStore from "./NotificationSettingsStore"; import PinsStore from "./PinsStore"; import PoliciesStore from "./PoliciesStore"; import RevisionsStore from "./RevisionsStore"; @@ -41,7 +40,6 @@ export default class RootStore { groupMemberships: GroupMembershipsStore; integrations: IntegrationsStore; memberships: MembershipsStore; - notificationSettings: NotificationSettingsStore; presence: DocumentPresenceStore; pins: PinsStore; policies: PoliciesStore; @@ -74,7 +72,6 @@ export default class RootStore { this.integrations = new IntegrationsStore(this); this.memberships = new MembershipsStore(this); this.pins = new PinsStore(this); - this.notificationSettings = new NotificationSettingsStore(this); this.presence = new DocumentPresenceStore(); this.revisions = new RevisionsStore(this); this.searches = new SearchesStore(this); @@ -102,7 +99,6 @@ export default class RootStore { this.groupMemberships.clear(); this.integrations.clear(); this.memberships.clear(); - this.notificationSettings.clear(); this.presence.clear(); this.pins.clear(); this.policies.clear(); diff --git a/plugins/email/server/auth/email.test.ts b/plugins/email/server/auth/email.test.ts index 02e47bf9f..19322e8c9 100644 --- a/plugins/email/server/auth/email.test.ts +++ b/plugins/email/server/auth/email.test.ts @@ -20,7 +20,7 @@ describe("email", () => { }); it("should respond with redirect location when user is SSO enabled", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const user = await buildUser(); const res = await server.post("/auth/email", { body: { @@ -35,7 +35,7 @@ describe("email", () => { }); it("should respond with success and email to be sent when user has SSO but disabled", async () => { - const spy = jest.spyOn(SigninEmail, "schedule"); + const spy = jest.spyOn(SigninEmail.prototype, "schedule"); const team = await buildTeam({ subdomain: "example", }); @@ -76,7 +76,7 @@ describe("email", () => { env.DEPLOYMENT = "hosted"; const user = await buildUser(); - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); await buildTeam({ subdomain: "example", }); @@ -97,7 +97,7 @@ describe("email", () => { }); it("should respond with success and email to be sent when user is not SSO enabled", async () => { - const spy = jest.spyOn(SigninEmail, "schedule"); + const spy = jest.spyOn(SigninEmail.prototype, "schedule"); const team = await buildTeam({ subdomain: "example", }); @@ -120,7 +120,7 @@ describe("email", () => { }); it("should respond with success regardless of whether successful to prevent crawling email logins", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); await buildTeam({ subdomain: "example", }); @@ -140,7 +140,7 @@ describe("email", () => { }); describe("with multiple users matching email", () => { it("should default to current subdomain with SSO", async () => { - const spy = jest.spyOn(SigninEmail, "schedule"); + const spy = jest.spyOn(SigninEmail.prototype, "schedule"); env.URL = sharedEnv.URL = "http://localoutline.com"; env.SUBDOMAINS_ENABLED = sharedEnv.SUBDOMAINS_ENABLED = true; const email = "sso-user@example.org"; @@ -170,7 +170,7 @@ describe("email", () => { }); it("should default to current subdomain with guest email", async () => { - const spy = jest.spyOn(SigninEmail, "schedule"); + const spy = jest.spyOn(SigninEmail.prototype, "schedule"); env.URL = sharedEnv.URL = "http://localoutline.com"; env.SUBDOMAINS_ENABLED = sharedEnv.SUBDOMAINS_ENABLED = true; const email = "guest-user@example.org"; @@ -200,7 +200,7 @@ describe("email", () => { }); it("should default to custom domain with SSO", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const email = "sso-user-2@example.org"; const team = await buildTeam({ domain: "docs.mycompany.com", @@ -228,7 +228,7 @@ describe("email", () => { }); it("should default to custom domain with guest email", async () => { - const spy = jest.spyOn(SigninEmail, "schedule"); + const spy = jest.spyOn(SigninEmail.prototype, "schedule"); const email = "guest-user-2@example.org"; const team = await buildTeam({ domain: "docs.mycompany.com", diff --git a/plugins/email/server/auth/email.ts b/plugins/email/server/auth/email.ts index 5d5735082..3cc1f862f 100644 --- a/plugins/email/server/auth/email.ts +++ b/plugins/email/server/auth/email.ts @@ -1,5 +1,5 @@ import Router from "koa-router"; -import { Client } from "@shared/types"; +import { Client, NotificationEventType } from "@shared/types"; import { parseDomain } from "@shared/utils/domains"; import InviteAcceptedEmail from "@server/emails/templates/InviteAcceptedEmail"; import SigninEmail from "@server/emails/templates/SigninEmail"; @@ -67,12 +67,13 @@ router.post( } // send email to users email address with a short-lived token - await SigninEmail.schedule({ + await new SigninEmail({ to: user.email, token: user.getEmailSigninToken(), teamUrl: team.url, client: client === Client.Desktop ? Client.Desktop : Client.Web, - }); + }).schedule(); + user.lastSigninEmailSentAt = new Date(); await user.save(); @@ -105,19 +106,19 @@ router.get("email.callback", async (ctx) => { } if (user.isInvited) { - await WelcomeEmail.schedule({ + await new WelcomeEmail({ to: user.email, teamUrl: user.team.url, - }); + }).schedule(); const inviter = await user.$get("invitedBy"); - if (inviter) { - await InviteAcceptedEmail.schedule({ + if (inviter?.subscribedToEventType(NotificationEventType.InviteAccepted)) { + await new InviteAcceptedEmail({ to: inviter.email, inviterId: inviter.id, invitedName: user.name, teamUrl: user.team.url, - }); + }).schedule(); } } diff --git a/plugins/webhooks/server/tasks/DeliverWebhookTask.ts b/plugins/webhooks/server/tasks/DeliverWebhookTask.ts index 06d817500..ea6627eba 100644 --- a/plugins/webhooks/server/tasks/DeliverWebhookTask.ts +++ b/plugins/webhooks/server/tasks/DeliverWebhookTask.ts @@ -647,11 +647,11 @@ export default class DeliverWebhookTask extends BaseTask { ]); if (createdBy && team) { - await WebhookDisabledEmail.schedule({ + await new WebhookDisabledEmail({ to: createdBy.email, teamUrl: team.url, webhookName: subscription.name, - }); + }).schedule(); } } } diff --git a/server/commands/accountProvisioner.test.ts b/server/commands/accountProvisioner.test.ts index caef7c3ae..778ad91fd 100644 --- a/server/commands/accountProvisioner.test.ts +++ b/server/commands/accountProvisioner.test.ts @@ -18,7 +18,7 @@ describe("accountProvisioner", () => { }); it("should create a new user and team", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const { user, team, isNewTeam, isNewUser } = await accountProvisioner({ ip, user: { @@ -58,7 +58,7 @@ describe("accountProvisioner", () => { }); it("should update exising user and authentication", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const existingTeam = await buildTeam(); const providers = await existingTeam.$get("authenticationProviders"); const authenticationProvider = providers[0]; @@ -230,7 +230,7 @@ describe("accountProvisioner", () => { }); it("should create a new user in an existing team when the domain is allowed", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const { admin, team } = await seed(); const authenticationProviders = await team.$get( "authenticationProviders" @@ -280,7 +280,7 @@ describe("accountProvisioner", () => { }); it("should create a new user in an existing team", async () => { - const spy = jest.spyOn(WelcomeEmail, "schedule"); + const spy = jest.spyOn(WelcomeEmail.prototype, "schedule"); const team = await buildTeam(); const authenticationProviders = await team.$get( "authenticationProviders" diff --git a/server/commands/accountProvisioner.ts b/server/commands/accountProvisioner.ts index 6704da8c8..e2ee410a4 100644 --- a/server/commands/accountProvisioner.ts +++ b/server/commands/accountProvisioner.ts @@ -144,10 +144,10 @@ async function accountProvisioner({ const { isNewUser, user } = result; if (isNewUser) { - await WelcomeEmail.schedule({ + await new WelcomeEmail({ to: user.email, teamUrl: team.url, - }); + }).schedule(); } if (isNewUser || isNewTeam) { diff --git a/server/commands/teamPermanentDeleter.ts b/server/commands/teamPermanentDeleter.ts index 409905b66..6cffa6e8a 100644 --- a/server/commands/teamPermanentDeleter.ts +++ b/server/commands/teamPermanentDeleter.ts @@ -12,7 +12,6 @@ import { FileOperation, Group, Team, - NotificationSetting, User, UserAuthentication, Integration, @@ -154,13 +153,6 @@ async function teamPermanentDeleter(team: Team) { force: true, transaction, }); - await NotificationSetting.destroy({ - where: { - teamId, - }, - force: true, - transaction, - }); await SearchQuery.destroy({ where: { teamId, diff --git a/server/commands/userInviter.ts b/server/commands/userInviter.ts index 498b18a93..072e0f6a9 100644 --- a/server/commands/userInviter.ts +++ b/server/commands/userInviter.ts @@ -80,14 +80,14 @@ export default async function userInviter({ ip, }); - await InviteEmail.schedule({ + await new InviteEmail({ to: invite.email, name: invite.name, actorName: user.name, actorEmail: user.email, teamName: team.name, teamUrl: team.url, - }); + }).schedule(); if (env.ENVIRONMENT === "development") { Logger.info( diff --git a/server/commands/userProvisioner.ts b/server/commands/userProvisioner.ts index 79fa92d49..bc5e2c49b 100644 --- a/server/commands/userProvisioner.ts +++ b/server/commands/userProvisioner.ts @@ -181,12 +181,12 @@ export default async function userProvisioner({ if (isInvite) { const inviter = await existingUser.$get("invitedBy"); if (inviter) { - await InviteAcceptedEmail.schedule({ + await new InviteAcceptedEmail({ to: inviter.email, inviterId: inviter.id, invitedName: existingUser.name, teamUrl: existingUser.team.url, - }); + }).schedule(); } } diff --git a/server/emails/templates/BaseEmail.tsx b/server/emails/templates/BaseEmail.tsx index dd629906b..eb0516705 100644 --- a/server/emails/templates/BaseEmail.tsx +++ b/server/emails/templates/BaseEmail.tsx @@ -1,3 +1,4 @@ +import Bull from "bull"; import mailer from "@server/emails/mailer"; import Logger from "@server/logging/Logger"; import Metrics from "@server/logging/Metrics"; @@ -6,8 +7,8 @@ import { taskQueue } from "@server/queues"; import { TaskPriority } from "@server/queues/tasks/BaseTask"; import { NotificationMetadata } from "@server/types"; -interface EmailProps { - to: string; +export interface EmailProps { + to: string | null; } export default abstract class BaseEmail { @@ -17,12 +18,11 @@ export default abstract class BaseEmail { /** * Schedule this email type to be sent asyncronously by a worker. * - * @param props Properties to be used in the email template - * @param metadata Optional metadata to be stored with the notification + * @param options Options to pass to the Bull queue * @returns A promise that resolves once the email is placed on the task queue */ - public static schedule(props: T, metadata?: NotificationMetadata) { - const templateName = this.name; + public schedule(options?: Bull.JobOptions) { + const templateName = this.constructor.name; Metrics.increment("email.scheduled", { templateName, @@ -35,8 +35,8 @@ export default abstract class BaseEmail { name: "EmailTask", props: { templateName, - ...metadata, - props, + ...this.metadata, + props: this.props, }, }, { @@ -46,6 +46,7 @@ export default abstract class BaseEmail { type: "exponential", delay: 60 * 1000, }, + ...options, } ); } @@ -73,6 +74,15 @@ export default abstract class BaseEmail { return; } + if (!this.props.to) { + Logger.info( + "email", + `Email ${templateName} not sent due to missing email address`, + this.props + ); + return; + } + const data = { ...this.props, ...(bsResponse ?? ({} as S)) }; try { diff --git a/server/emails/templates/CollectionNotificationEmail.tsx b/server/emails/templates/CollectionCreatedEmail.tsx similarity index 57% rename from server/emails/templates/CollectionNotificationEmail.tsx rename to server/emails/templates/CollectionCreatedEmail.tsx index 7011b6d01..f4c60b673 100644 --- a/server/emails/templates/CollectionNotificationEmail.tsx +++ b/server/emails/templates/CollectionCreatedEmail.tsx @@ -1,6 +1,8 @@ import * as React from "react"; +import { NotificationEventType } from "@shared/types"; import env from "@server/env"; -import { Collection } from "@server/models"; +import { Collection, User } from "@server/models"; +import NotificationSettingsHelper from "@server/models/helpers/NotificationSettingsHelper"; import BaseEmail from "./BaseEmail"; import Body from "./components/Body"; import Button from "./components/Button"; @@ -12,13 +14,13 @@ import Heading from "./components/Heading"; type InputProps = { to: string; - eventName: string; + userId: string; collectionId: string; - unsubscribeUrl: string; }; type BeforeSend = { collection: Collection; + unsubscribeUrl: string; }; type Props = InputProps & BeforeSend; @@ -27,12 +29,11 @@ type Props = InputProps & BeforeSend; * Email sent to a user when they have enabled notifications of new collection * creation. */ - -export default class CollectionNotificationEmail extends BaseEmail< +export default class CollectionCreatedEmail extends BaseEmail< InputProps, BeforeSend > { - protected async beforeSend({ collectionId }: Props) { + protected async beforeSend({ userId, collectionId }: Props) { const collection = await Collection.scope("withUser").findByPk( collectionId ); @@ -40,32 +41,39 @@ export default class CollectionNotificationEmail extends BaseEmail< return false; } - return { collection }; + const user = await User.findByPk(userId); + if (!user) { + return false; + } + + return { + collection, + unsubscribeUrl: NotificationSettingsHelper.unsubscribeUrl( + user, + NotificationEventType.CreateCollection + ), + }; } - protected subject({ collection, eventName }: Props) { - return `“${collection.name}” ${eventName}`; + protected subject({ collection }: Props) { + return `“${collection.name}” created`; } - protected preview({ collection, eventName }: Props) { - return `${collection.user.name} ${eventName} a collection`; + protected preview({ collection }: Props) { + return `${collection.user.name} created a collection`; } - protected renderAsText({ collection, eventName = "created" }: Props) { + protected renderAsText({ collection }: Props) { return ` ${collection.name} -${collection.user.name} ${eventName} the collection "${collection.name}" +${collection.user.name} created the collection "${collection.name}" Open Collection: ${env.URL}${collection.url} `; } - protected render({ - collection, - eventName = "created", - unsubscribeUrl, - }: Props) { + protected render({ collection, unsubscribeUrl }: Props) { return (
@@ -73,7 +81,7 @@ Open Collection: ${env.URL}${collection.url} {collection.name}

- {collection.user.name} {eventName} the collection "{collection.name} + {collection.user.name} created the collection "{collection.name} ".

diff --git a/server/emails/templates/CommentCreatedEmail.tsx b/server/emails/templates/CommentCreatedEmail.tsx index 8f2a1aca7..a56c36878 100644 --- a/server/emails/templates/CommentCreatedEmail.tsx +++ b/server/emails/templates/CommentCreatedEmail.tsx @@ -1,8 +1,10 @@ import inlineCss from "inline-css"; import * as React from "react"; +import { NotificationEventType } from "@shared/types"; import env from "@server/env"; -import { Comment, Document } from "@server/models"; -import BaseEmail from "./BaseEmail"; +import { Comment, 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"; @@ -12,15 +14,14 @@ import Footer from "./components/Footer"; import Header from "./components/Header"; import Heading from "./components/Heading"; -type InputProps = { - to: string; +type InputProps = EmailProps & { + userId: string; documentId: string; actorName: string; isReply: boolean; commentId: string; - collectionName: string; + collectionName: string | undefined; teamUrl: string; - unsubscribeUrl: string; content: string; }; @@ -28,24 +29,35 @@ type BeforeSend = { document: Document; body: string | undefined; isFirstComment: boolean; + unsubscribeUrl: string; }; type Props = InputProps & BeforeSend; /** - * Email sent to a user when they are subscribed to a document and a new comment - * is created. + * Email sent to a user when a new comment is created in a document they are + * subscribed to. */ export default class CommentCreatedEmail extends BaseEmail< InputProps, BeforeSend > { - protected async beforeSend({ documentId, commentId, content }: InputProps) { + protected async beforeSend({ + documentId, + userId, + commentId, + content, + }: InputProps) { const document = await Document.unscoped().findByPk(documentId); if (!document) { return false; } + const user = await User.findByPk(userId); + if (!user) { + return false; + } + const firstComment = await Comment.findOne({ attributes: ["id"], where: { documentId }, @@ -64,7 +76,15 @@ export default class CommentCreatedEmail extends BaseEmail< }); } - return { document, isFirstComment, body }; + return { + document, + isFirstComment, + body, + unsubscribeUrl: NotificationSettingsHelper.unsubscribeUrl( + user, + NotificationEventType.CreateComment + ), + }; } protected subject({ isFirstComment, document }: Props) { @@ -92,7 +112,7 @@ export default class CommentCreatedEmail extends BaseEmail< return ` ${actorName} ${isReply ? "replied to a thread in" : "commented on"} "${ document.title - }", in the ${collectionName} collection. + }"${collectionName ? `in the ${collectionName} collection` : ""}. Open Thread: ${teamUrl}${document.url}?commentId=${commentId} `; @@ -118,8 +138,8 @@ Open Thread: ${teamUrl}${document.url}?commentId=${commentId} {document.title}

{actorName} {isReply ? "replied to a thread in" : "commented on"}{" "} - {document.title}, in the {collectionName}{" "} - collection. + {document.title}{" "} + {collectionName ? `in the ${collectionName} collection` : ""}.

{body && ( <> diff --git a/server/emails/templates/ConfirmUserDeleteEmail.tsx b/server/emails/templates/ConfirmUserDeleteEmail.tsx index 4e12488c6..d7123f04e 100644 --- a/server/emails/templates/ConfirmUserDeleteEmail.tsx +++ b/server/emails/templates/ConfirmUserDeleteEmail.tsx @@ -1,6 +1,6 @@ import * as React from "react"; import env from "@server/env"; -import BaseEmail from "./BaseEmail"; +import BaseEmail, { EmailProps } from "./BaseEmail"; import Body from "./components/Body"; import CopyableCode from "./components/CopyableCode"; import EmailTemplate from "./components/EmailLayout"; @@ -9,8 +9,7 @@ import Footer from "./components/Footer"; import Header from "./components/Header"; import Heading from "./components/Heading"; -type Props = { - to: string; +type Props = EmailProps & { deleteConfirmationCode: string; }; diff --git a/server/emails/templates/DocumentNotificationEmail.tsx b/server/emails/templates/DocumentNotificationEmail.tsx index f0557f3f0..1991409d4 100644 --- a/server/emails/templates/DocumentNotificationEmail.tsx +++ b/server/emails/templates/DocumentNotificationEmail.tsx @@ -1,8 +1,10 @@ import inlineCss from "inline-css"; import * as React from "react"; +import { NotificationEventType } from "@shared/types"; import env from "@server/env"; -import { Document } from "@server/models"; -import BaseEmail from "./BaseEmail"; +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"; @@ -12,19 +14,21 @@ import Footer from "./components/Footer"; import Header from "./components/Header"; import Heading from "./components/Heading"; -type InputProps = { - to: string; +type InputProps = EmailProps & { + userId: string; documentId: string; actorName: string; collectionName: string; - eventName: string; + eventType: + | NotificationEventType.PublishDocument + | NotificationEventType.UpdateDocument; teamUrl: string; - unsubscribeUrl: string; - content: string; + content?: string; }; type BeforeSend = { document: Document; + unsubscribeUrl: string; body: string | undefined; }; @@ -38,12 +42,22 @@ export default class DocumentNotificationEmail extends BaseEmail< InputProps, BeforeSend > { - protected async beforeSend({ documentId, content }: InputProps) { + protected async beforeSend({ + documentId, + eventType, + 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) { @@ -55,15 +69,33 @@ export default class DocumentNotificationEmail extends BaseEmail< }); } - return { document, body }; + return { + document, + body, + unsubscribeUrl: NotificationSettingsHelper.unsubscribeUrl( + user, + eventType + ), + }; } - protected subject({ document, eventName }: Props) { - return `“${document.title}” ${eventName}`; + eventName(eventType: NotificationEventType) { + switch (eventType) { + case NotificationEventType.PublishDocument: + return "published"; + case NotificationEventType.UpdateDocument: + return "updated"; + default: + return ""; + } } - protected preview({ actorName, eventName }: Props): string { - return `${actorName} ${eventName} a document`; + protected subject({ document, eventType }: Props) { + return `“${document.title}” ${this.eventName(eventType)}`; + } + + protected preview({ actorName, eventType }: Props): string { + return `${actorName} ${this.eventName(eventType)} a document`; } protected renderAsText({ @@ -71,8 +103,10 @@ export default class DocumentNotificationEmail extends BaseEmail< teamUrl, document, collectionName, - eventName = "published", + eventType, }: Props): string { + const eventName = this.eventName(eventType); + return ` "${document.title}" ${eventName} @@ -86,12 +120,13 @@ Open Document: ${teamUrl}${document.url} document, actorName, collectionName, - eventName = "published", + eventType, teamUrl, unsubscribeUrl, body, }: Props) { const link = `${teamUrl}${document.url}?ref=notification-email`; + const eventName = this.eventName(eventType); return ( diff --git a/server/emails/templates/ExportFailureEmail.tsx b/server/emails/templates/ExportFailureEmail.tsx index f81c358f3..d949ccc63 100644 --- a/server/emails/templates/ExportFailureEmail.tsx +++ b/server/emails/templates/ExportFailureEmail.tsx @@ -1,6 +1,8 @@ import * as React from "react"; -import { NotificationSetting } from "@server/models"; -import BaseEmail from "./BaseEmail"; +import { NotificationEventType } from "@shared/types"; +import { 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 EmailTemplate from "./components/EmailLayout"; @@ -9,27 +11,32 @@ import Footer from "./components/Footer"; import Header from "./components/Header"; import Heading from "./components/Heading"; -type Props = { - to: string; +type Props = EmailProps & { userId: string; teamUrl: string; teamId: string; }; +type BeforeSendProps = { + unsubscribeUrl: string; +}; + /** * Email sent to a user when their data export has failed for some reason. */ export default class ExportFailureEmail extends BaseEmail { - protected async beforeSend({ userId, teamId }: Props) { - const notificationSetting = await NotificationSetting.findOne({ - where: { - userId, - teamId, - event: "emails.export_completed", - }, - }); + protected async beforeSend({ userId }: Props) { + const user = await User.findByPk(userId); + if (!user) { + return false; + } - return notificationSetting !== null; + return { + unsubscribeUrl: NotificationSettingsHelper.unsubscribeUrl( + user, + NotificationEventType.ExportCompleted + ), + }; } protected subject() { @@ -49,7 +56,7 @@ section to try again – if the problem persists please contact support. `; } - protected render({ teamUrl }: Props) { + protected render({ teamUrl, unsubscribeUrl }: Props & BeforeSendProps) { return (
@@ -71,7 +78,7 @@ section to try again – if the problem persists please contact support.

-