yuuza/outline
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove NotificationSettings table (#5036

Browse Source 
* 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
This commit is contained in:
Tom Moor
2023-03-18 09:32:41 -04:00
committed by GitHub
parent 41f97b0563
commit 45831e9469
58 changed files with 972 additions and 711 deletions
Download Patch File Download Diff File Expand all files Collapse all files

176
server/queues/processors/NotificationsProcessor.test.ts
View File

@@ -1,7 +1,7 @@
import { NotificationEventType } from "@shared/types";
import DocumentNotificationEmail from "@server/emails/templates/DocumentNotificationEmail";
import {
View,
NotificationSetting,
Subscription,
Event,
Notification,
@@ -15,7 +15,6 @@ import {
import { setupTestDatabase } from "@server/test/support";
import NotificationsProcessor from "./NotificationsProcessor";
jest.mock("@server/emails/templates/DocumentNotificationEmail");
const ip = "127.0.0.1";
setupTestDatabase();
@@ -26,16 +25,18 @@ beforeEach(async () => {
describe("documents.publish", () => {
test("should not send a notification to author", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const user = await buildUser();
const document = await buildDocument({
teamId: user.teamId,
lastModifiedById: user.id,
});
await NotificationSetting.create({
userId: user.id,
teamId: user.teamId,
event: "documents.publish",
});
user.setNotificationEventType(NotificationEventType.PublishDocument);
await user.save();
const processor = new NotificationsProcessor();
await processor.perform({
@@ -49,19 +50,20 @@ describe("documents.publish", () => {
},
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
test("should send a notification to other users in team", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const user = await buildUser();
const document = await buildDocument({
teamId: user.teamId,
});
await NotificationSetting.create({
userId: user.id,
teamId: user.teamId,
event: "documents.publish",
});
user.setNotificationEventType(NotificationEventType.PublishDocument);
await user.save();
const processor = new NotificationsProcessor();
await processor.perform({
@@ -75,10 +77,14 @@ describe("documents.publish", () => {
},
ip,
});
expect(DocumentNotificationEmail.schedule).toHaveBeenCalled();
expect(schedule).toHaveBeenCalled();
});
test("should send only one notification in a 12-hour window", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const user = await buildUser();
const document = await buildDocument({
teamId: user.teamId,
@@ -90,11 +96,8 @@ describe("documents.publish", () => {
teamId: user.teamId,
});
await NotificationSetting.create({
userId: recipient.id,
teamId: recipient.teamId,
event: "documents.publish",
});
user.setNotificationEventType(NotificationEventType.PublishDocument);
await user.save();
await Notification.create({
actorId: user.id,
@@ -117,10 +120,14 @@ describe("documents.publish", () => {
},
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
test("should not send a notification to users without collection access", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const user = await buildUser();
const collection = await buildCollection({
teamId: user.teamId,
@@ -130,11 +137,9 @@ describe("documents.publish", () => {
teamId: user.teamId,
collectionId: collection.id,
});
await NotificationSetting.create({
userId: user.id,
teamId: user.teamId,
event: "documents.publish",
});
user.setNotificationEventType(NotificationEventType.PublishDocument);
await user.save();
const processor = new NotificationsProcessor();
await processor.perform({
name: "documents.publish",
@@ -147,12 +152,16 @@ describe("documents.publish", () => {
},
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
});
describe("revisions.create", () => {
test("should send a notification to other collaborators", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
await Revision.createFromDocument(document);
@@ -162,11 +171,7 @@ describe("revisions.create", () => {
const collaborator = await buildUser({ teamId: document.teamId });
document.collaboratorIds = [collaborator.id];
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
const processor = new NotificationsProcessor();
await processor.perform({
name: "revisions.create",
@@ -177,10 +182,14 @@ describe("revisions.create", () => {
modelId: revision.id,
ip,
});
expect(DocumentNotificationEmail.schedule).toHaveBeenCalled();
expect(schedule).toHaveBeenCalled();
});
test("should not send a notification if viewed since update", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
await Revision.createFromDocument(document);
document.text = "Updated body content";
@@ -189,11 +198,7 @@ describe("revisions.create", () => {
const collaborator = await buildUser({ teamId: document.teamId });
document.collaboratorIds = [collaborator.id];
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
await View.create({
userId: collaborator.id,
documentId: document.id,
@@ -209,10 +214,14 @@ describe("revisions.create", () => {
modelId: revision.id,
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
test("should not send a notification to last editor", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const user = await buildUser();
const document = await buildDocument({
teamId: user.teamId,
@@ -222,11 +231,7 @@ describe("revisions.create", () => {
document.text = "Updated body content";
document.updatedAt = new Date();
const revision = await Revision.createFromDocument(document);
await NotificationSetting.create({
userId: user.id,
teamId: user.teamId,
event: "documents.update",
});
const processor = new NotificationsProcessor();
await processor.perform({
name: "revisions.create",
@@ -237,10 +242,14 @@ describe("revisions.create", () => {
modelId: revision.id,
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
test("should send a notification for subscriptions, even to collaborator", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
await Revision.createFromDocument(document);
document.text = "Updated body content";
@@ -253,12 +262,6 @@ describe("revisions.create", () => {
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
await Subscription.create({
userId: subscriber.id,
documentId: document.id,
@@ -278,7 +281,7 @@ describe("revisions.create", () => {
ip,
});
expect(DocumentNotificationEmail.schedule).toHaveBeenCalled();
expect(schedule).toHaveBeenCalled();
});
test("should create subscriptions for collaborator", async () => {
@@ -327,6 +330,10 @@ describe("revisions.create", () => {
});
test("should not send multiple emails", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const collaborator0 = await buildUser();
const collaborator1 = await buildUser({ teamId: collaborator0.teamId });
const collaborator2 = await buildUser({ teamId: collaborator0.teamId });
@@ -370,10 +377,14 @@ describe("revisions.create", () => {
// This should send out 2 emails, one for each collaborator that did not
// participate in the edit
expect(DocumentNotificationEmail.schedule).toHaveBeenCalledTimes(2);
expect(schedule).toHaveBeenCalledTimes(2);
});
test("should not create subscriptions if previously unsubscribed", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const collaborator0 = await buildUser();
const collaborator1 = await buildUser({ teamId: collaborator0.teamId });
const collaborator2 = await buildUser({ teamId: collaborator0.teamId });
@@ -429,10 +440,14 @@ describe("revisions.create", () => {
// One notification as one collaborator performed edit and the other is
// unsubscribed
expect(DocumentNotificationEmail.schedule).toHaveBeenCalledTimes(1);
expect(schedule).toHaveBeenCalledTimes(1);
});
test("should send a notification for subscriptions to non-collaborators", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
const collaborator = await buildUser({ teamId: document.teamId });
const subscriber = await buildUser({ teamId: document.teamId });
@@ -446,12 +461,6 @@ describe("revisions.create", () => {
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
// `subscriber` subscribes to `document`'s changes.
// Specifically "documents.update" event.
await Subscription.create({
@@ -473,10 +482,14 @@ describe("revisions.create", () => {
ip,
});
expect(DocumentNotificationEmail.schedule).toHaveBeenCalled();
expect(schedule).toHaveBeenCalled();
});
test("should not send a notification for subscriptions to collaborators if unsubscribed", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
await Revision.createFromDocument(document);
document.text = "Updated body content";
@@ -490,12 +503,6 @@ describe("revisions.create", () => {
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
// `subscriber` subscribes to `document`'s changes.
// Specifically "documents.update" event.
const subscription = await Subscription.create({
@@ -520,10 +527,14 @@ describe("revisions.create", () => {
});
// Should send notification to `collaborator` and not `subscriber`.
expect(DocumentNotificationEmail.schedule).toHaveBeenCalledTimes(1);
expect(schedule).toHaveBeenCalledTimes(1);
});
test("should not send a notification for subscriptions to members outside of the team", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
await Revision.createFromDocument(document);
document.text = "Updated body content";
@@ -540,12 +551,6 @@ describe("revisions.create", () => {
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
// `subscriber` subscribes to `document`'s changes.
// Specifically "documents.update" event.
// Not sure how they got hold of this document,
@@ -570,20 +575,20 @@ describe("revisions.create", () => {
});
// Should send notification to `collaborator` and not `subscriber`.
expect(DocumentNotificationEmail.schedule).toHaveBeenCalledTimes(1);
expect(schedule).toHaveBeenCalledTimes(1);
});
test("should not send a notification if viewed since update", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const document = await buildDocument();
const revision = await Revision.createFromDocument(document);
const collaborator = await buildUser({ teamId: document.teamId });
document.collaboratorIds = [collaborator.id];
await document.save();
await NotificationSetting.create({
userId: collaborator.id,
teamId: collaborator.teamId,
event: "documents.update",
});
await View.create({
userId: collaborator.id,
documentId: document.id,
@@ -600,10 +605,14 @@ describe("revisions.create", () => {
modelId: revision.id,
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
test("should not send a notification to last editor", async () => {
const schedule = jest.spyOn(
DocumentNotificationEmail.prototype,
"schedule"
);
const user = await buildUser();
const document = await buildDocument({
teamId: user.teamId,
@@ -611,11 +620,6 @@ describe("revisions.create", () => {
});
const revision = await Revision.createFromDocument(document);
await NotificationSetting.create({
userId: user.id,
teamId: user.teamId,
event: "documents.update",
});
const processor = new NotificationsProcessor();
await processor.perform({
name: "revisions.create",
@@ -626,6 +630,6 @@ describe("revisions.create", () => {
modelId: revision.id,
ip,
});
expect(DocumentNotificationEmail.schedule).not.toHaveBeenCalled();
expect(schedule).not.toHaveBeenCalled();
});
});

78
server/queues/processors/NotificationsProcessor.ts
View File

@@ -1,10 +1,11 @@
import { subHours } from "date-fns";
import { differenceBy } from "lodash";
import { Op } from "sequelize";
import { NotificationEventType } from "@shared/types";
import { Minute } from "@shared/utils/time";
import subscriptionCreator from "@server/commands/subscriptionCreator";
import { sequelize } from "@server/database/sequelize";
import CollectionNotificationEmail from "@server/emails/templates/CollectionNotificationEmail";
import CollectionCreatedEmail from "@server/emails/templates/CollectionCreatedEmail";
import DocumentNotificationEmail from "@server/emails/templates/DocumentNotificationEmail";
import MentionNotificationEmail from "@server/emails/templates/MentionNotificationEmail";
import env from "@server/env";
@@ -89,7 +90,12 @@ export default class NotificationsProcessor extends BaseProcessor {
User.findByPk(mention.modelId),
User.findByPk(mention.actorId),
]);
if (recipient && actor && recipient.id !== actor.id) {
if (
recipient &&
actor &&
recipient.id !== actor.id &&
recipient.subscribedToEventType(NotificationEventType.Mentioned)
) {
const notification = await Notification.create({
event: event.name,
userId: recipient.id,
@@ -98,7 +104,7 @@ export default class NotificationsProcessor extends BaseProcessor {
documentId: document.id,
});
userIdsSentNotifications.push(recipient.id);
await MentionNotificationEmail.schedule(
await new MentionNotificationEmail(
{
to: recipient.email,
documentId: event.documentId,
@@ -107,44 +113,42 @@ export default class NotificationsProcessor extends BaseProcessor {
mentionId: mention.id,
},
{ notificationId: notification.id }
);
).schedule();
}
}
const recipients = (
await NotificationHelper.getDocumentNotificationRecipients(
document,
"documents.publish",
NotificationEventType.PublishDocument,
document.lastModifiedById,
false
)
).filter(
(recipient) => !userIdsSentNotifications.includes(recipient.userId)
);
).filter((recipient) => !userIdsSentNotifications.includes(recipient.id));
for (const recipient of recipients) {
const notify = await this.shouldNotify(document, recipient.user);
const notify = await this.shouldNotify(document, recipient);
if (notify) {
const notification = await Notification.create({
event: event.name,
userId: recipient.user.id,
userId: recipient.id,
actorId: document.updatedBy.id,
teamId: team.id,
documentId: document.id,
});
await DocumentNotificationEmail.schedule(
await new DocumentNotificationEmail(
{
to: recipient.user.email,
eventName: "published",
to: recipient.email,
userId: recipient.id,
eventType: NotificationEventType.PublishDocument,
documentId: document.id,
teamUrl: team.url,
actorName: document.updatedBy.name,
collectionName: collection.name,
unsubscribeUrl: recipient.unsubscribeUrl,
},
{ notificationId: notification.id }
);
).schedule();
}
}
}
@@ -175,7 +179,12 @@ export default class NotificationsProcessor extends BaseProcessor {
User.findByPk(mention.modelId),
User.findByPk(mention.actorId),
]);
if (recipient && actor && recipient.id !== actor.id) {
if (
recipient &&
actor &&
recipient.id !== actor.id &&
recipient.subscribedToEventType(NotificationEventType.Mentioned)
) {
const notification = await Notification.create({
event: event.name,
userId: recipient.id,
@@ -184,7 +193,7 @@ export default class NotificationsProcessor extends BaseProcessor {
documentId: document.id,
});
userIdsSentNotifications.push(recipient.id);
await MentionNotificationEmail.schedule(
await new MentionNotificationEmail(
{
to: recipient.email,
documentId: event.documentId,
@@ -193,20 +202,18 @@ export default class NotificationsProcessor extends BaseProcessor {
mentionId: mention.id,
},
{ notificationId: notification.id }
);
).schedule();
}
}
const recipients = (
await NotificationHelper.getDocumentNotificationRecipients(
document,
"documents.update",
NotificationEventType.UpdateDocument,
document.lastModifiedById,
true
)
).filter(
(recipient) => !userIdsSentNotifications.includes(recipient.userId)
);
).filter((recipient) => !userIdsSentNotifications.includes(recipient.id));
if (!recipients.length) {
return;
}
@@ -223,30 +230,30 @@ export default class NotificationsProcessor extends BaseProcessor {
}
for (const recipient of recipients) {
const notify = await this.shouldNotify(document, recipient.user);
const notify = await this.shouldNotify(document, recipient);
if (notify) {
const notification = await Notification.create({
event: event.name,
userId: recipient.user.id,
userId: recipient.id,
actorId: document.updatedBy.id,
teamId: team.id,
documentId: document.id,
});
await DocumentNotificationEmail.schedule(
await new DocumentNotificationEmail(
{
to: recipient.user.email,
eventName: "updated",
to: recipient.email,
userId: recipient.id,
eventType: NotificationEventType.UpdateDocument,
documentId: document.id,
teamUrl: team.url,
actorName: document.updatedBy.name,
collectionName: collection.name,
unsubscribeUrl: recipient.unsubscribeUrl,
content,
},
{ notificationId: notification.id }
);
).schedule();
}
}
}
@@ -262,21 +269,20 @@ export default class NotificationsProcessor extends BaseProcessor {
const recipients = await NotificationHelper.getCollectionNotificationRecipients(
collection,
event.name
NotificationEventType.CreateCollection
);
for (const recipient of recipients) {
// Suppress notifications for suspended users
if (recipient.user.isSuspended || !recipient.user.email) {
if (recipient.isSuspended || !recipient.email) {
continue;
}
await CollectionNotificationEmail.schedule({
to: recipient.user.email,
eventName: "created",
await new CollectionCreatedEmail({
to: recipient.email,
userId: recipient.id,
collectionId: collection.id,
unsubscribeUrl: recipient.unsubscribeUrl,
});
}).schedule();
}
}

31
server/queues/tasks/CommentCreatedNotificationTask.ts
View File

@@ -1,4 +1,5 @@
import { Node } from "prosemirror-model";
import { NotificationEventType } from "@shared/types";
import subscriptionCreator from "@server/commands/subscriptionCreator";
import { sequelize } from "@server/database/sequelize";
import { schema } from "@server/editor";
@@ -66,7 +67,12 @@ export default class CommentCreatedNotificationTask extends BaseTask<
User.findByPk(mention.modelId),
User.findByPk(mention.actorId),
]);
if (recipient && actor && recipient.id !== actor.id) {
if (
recipient &&
actor &&
recipient.id !== actor.id &&
recipient.subscribedToEventType(NotificationEventType.Mentioned)
) {
const notification = await Notification.create({
event: event.name,
userId: recipient.id,
@@ -75,9 +81,11 @@ export default class CommentCreatedNotificationTask extends BaseTask<
documentId: document.id,
});
userIdsSentNotifications.push(recipient.id);
await CommentCreatedEmail.schedule(
await new CommentCreatedEmail(
{
to: recipient.email,
userId: recipient.id,
documentId: document.id,
teamUrl: team.url,
isReply: !!comment.parentCommentId,
@@ -87,7 +95,7 @@ export default class CommentCreatedNotificationTask extends BaseTask<
collectionName: document.collection?.name,
},
{ notificationId: notification.id }
);
).schedule();
}
}
@@ -97,24 +105,20 @@ export default class CommentCreatedNotificationTask extends BaseTask<
comment,
comment.createdById
)
).filter(
(recipient) => !userIdsSentNotifications.includes(recipient.userId)
);
if (!recipients.length) {
return;
}
).filter((recipient) => !userIdsSentNotifications.includes(recipient.id));
for (const recipient of recipients) {
const notification = await Notification.create({
event: event.name,
userId: recipient.user.id,
userId: recipient.id,
actorId: comment.createdById,
teamId: team.id,
documentId: document.id,
});
await CommentCreatedEmail.schedule(
await new CommentCreatedEmail(
{
to: recipient.user.email,
to: recipient.email,
userId: recipient.id,
documentId: document.id,
teamUrl: team.url,
isReply: !!comment.parentCommentId,
@@ -122,10 +126,9 @@ export default class CommentCreatedNotificationTask extends BaseTask<
commentId: comment.id,
content,
collectionName: document.collection?.name,
unsubscribeUrl: recipient.unsubscribeUrl,
},
{ notificationId: notification.id }
);
).schedule();
}
}

32
server/queues/tasks/ExportTask.ts
View File

@@ -1,6 +1,6 @@
import fs from "fs";
import { truncate } from "lodash";
import { FileOperationState } from "@shared/types";
import { FileOperationState, NotificationEventType } from "@shared/types";
import ExportFailureEmail from "@server/emails/templates/ExportFailureEmail";
import ExportSuccessEmail from "@server/emails/templates/ExportSuccessEmail";
import Logger from "@server/logging/Logger";
@@ -70,24 +70,28 @@ export default abstract class ExportTask extends BaseTask<Props> {
url,
});
await ExportSuccessEmail.schedule({
to: user.email,
userId: user.id,
id: fileOperation.id,
teamUrl: team.url,
teamId: team.id,
});
if (user.subscribedToEventType(NotificationEventType.ExportCompleted)) {
await new ExportSuccessEmail({
to: user.email,
userId: user.id,
id: fileOperation.id,
teamUrl: team.url,
teamId: team.id,
}).schedule();
}
} catch (error) {
await this.updateFileOperation(fileOperation, {
state: FileOperationState.Error,
error,
});
await ExportFailureEmail.schedule({
to: user.email,
userId: user.id,
teamUrl: team.url,
teamId: team.id,
});
if (user.subscribedToEventType(NotificationEventType.ExportCompleted)) {
await new ExportFailureEmail({
to: user.email,
userId: user.id,
teamUrl: team.url,
teamId: team.id,
}).schedule();
}
throw error;
}
}

2
server/queues/tasks/InviteReminderTask.test.ts
View File

@@ -8,7 +8,7 @@ setupTestDatabase();
describe("InviteReminderTask", () => {
it("should not destroy documents not deleted", async () => {
const spy = jest.spyOn(InviteReminderEmail, "schedule");
const spy = jest.spyOn(InviteReminderEmail.prototype, "schedule");
// too old
await buildInvite({

4
server/queues/tasks/InviteReminderTask.ts
View File

@@ -42,14 +42,14 @@ export default class InviteReminderTask extends BaseTask<Props> {
invitedBy &&
user.getFlag(UserFlag.InviteReminderSent) === 0
) {
await InviteReminderEmail.schedule({
await new InviteReminderEmail({
to: user.email,
name: user.name,
actorName: invitedBy.name,
actorEmail: invitedBy.email,
teamName: user.team.name,
teamUrl: user.team.url,
});
}).schedule();
user.incrementFlag(UserFlag.InviteReminderSent);
await user.save({ transaction });