fix: Move notifications to be revision driven (#2709)

This commit is contained in:
Tom Moor
2021-10-31 18:36:16 -07:00
committed by GitHub
parent b6a058147e
commit 5f00e1394d
4 changed files with 58 additions and 44 deletions

View File

@ -10,13 +10,18 @@ import {
NotificationSetting, NotificationSetting,
} from "../../models"; } from "../../models";
import { Op } from "../../sequelize"; import { Op } from "../../sequelize";
import type { DocumentEvent, CollectionEvent, Event } from "../../types"; import type {
DocumentEvent,
CollectionEvent,
RevisionEvent,
Event,
} from "../../types";
export default class NotificationsProcessor { export default class NotificationsProcessor {
async on(event: Event) { async on(event: Event) {
switch (event.name) { switch (event.name) {
case "documents.publish": case "documents.publish":
case "documents.update.debounced": case "revisions.create":
return this.documentUpdated(event); return this.documentUpdated(event);
case "collections.create": case "collections.create":
return this.collectionCreated(event); return this.collectionCreated(event);
@ -24,18 +29,17 @@ export default class NotificationsProcessor {
} }
} }
async documentUpdated(event: DocumentEvent) { async documentUpdated(event: DocumentEvent | RevisionEvent) {
// never send notifications when batch importing documents // never send notifications when batch importing documents
if (event.data && event.data.source === "import") return; if (event.data && event.data.source === "import") return;
const document = await Document.findByPk(event.documentId); const [document, team] = await Promise.all([
if (!document) return; Document.findByPk(event.documentId),
Team.findByPk(event.teamId),
]);
if (!document || !team || !document.collection) return;
const { collection } = document; const { collection } = document;
if (!collection) return;
const team = await Team.findByPk(document.teamId);
if (!team) return;
const notificationSettings = await NotificationSetting.findAll({ const notificationSettings = await NotificationSetting.findAll({
where: { where: {

View File

@ -93,8 +93,8 @@ describe("documents.publish", () => {
}); });
}); });
describe("documents.update.debounced", () => { describe("revisions.create", () => {
test("should send a notification to other collaborator", async () => { test("should send a notification to other collaborators", async () => {
const document = await buildDocument(); const document = await buildDocument();
const collaborator = await buildUser({ teamId: document.teamId }); const collaborator = await buildUser({ teamId: document.teamId });
document.collaboratorIds = [collaborator.id]; document.collaboratorIds = [collaborator.id];
@ -107,7 +107,7 @@ describe("documents.update.debounced", () => {
}); });
await Notifications.on({ await Notifications.on({
name: "documents.update.debounced", name: "revisions.create",
documentId: document.id, documentId: document.id,
collectionId: document.collectionId, collectionId: document.collectionId,
teamId: document.teamId, teamId: document.teamId,
@ -132,7 +132,7 @@ describe("documents.update.debounced", () => {
await View.touch(document.id, collaborator.id, true); await View.touch(document.id, collaborator.id, true);
await Notifications.on({ await Notifications.on({
name: "documents.update.debounced", name: "revisions.create",
documentId: document.id, documentId: document.id,
collectionId: document.collectionId, collectionId: document.collectionId,
teamId: document.teamId, teamId: document.teamId,
@ -156,7 +156,7 @@ describe("documents.update.debounced", () => {
}); });
await Notifications.on({ await Notifications.on({
name: "documents.update.debounced", name: "revisions.create",
documentId: document.id, documentId: document.id,
collectionId: document.collectionId, collectionId: document.collectionId,
teamId: document.teamId, teamId: document.teamId,

View File

@ -2,13 +2,18 @@
import fetch from "fetch-with-proxy"; import fetch from "fetch-with-proxy";
import { Document, Integration, Collection, Team } from "../../models"; import { Document, Integration, Collection, Team } from "../../models";
import { presentSlackAttachment } from "../../presenters"; import { presentSlackAttachment } from "../../presenters";
import type { DocumentEvent, IntegrationEvent, Event } from "../../types"; import type {
DocumentEvent,
IntegrationEvent,
RevisionEvent,
Event,
} from "../../types";
export default class SlackProcessor { export default class SlackProcessor {
async on(event: Event) { async on(event: Event) {
switch (event.name) { switch (event.name) {
case "documents.publish": case "documents.publish":
case "documents.update.debounced": case "revisions.create":
return this.documentUpdated(event); return this.documentUpdated(event);
case "integrations.create": case "integrations.create":
return this.integrationCreated(event); return this.integrationCreated(event);
@ -55,11 +60,15 @@ export default class SlackProcessor {
}); });
} }
async documentUpdated(event: DocumentEvent) { async documentUpdated(event: DocumentEvent | RevisionEvent) {
// never send notifications when batch importing documents // never send notifications when batch importing documents
if (event.data && event.data.source === "import") return; if (event.data && event.data.source === "import") return;
const document = await Document.findByPk(event.documentId); const [document, team] = await Promise.all([
Document.findByPk(event.documentId),
Team.findByPk(event.teamId),
]);
if (!document) return; if (!document) return;
// never send notifications for draft documents // never send notifications for draft documents
@ -75,8 +84,6 @@ export default class SlackProcessor {
}); });
if (!integration) return; if (!integration) return;
const team = await Team.findByPk(document.teamId);
let text = `${document.updatedBy.name} updated a document`; let text = `${document.updatedBy.name} updated a document`;
if (event.name === "documents.publish") { if (event.name === "documents.publish") {

View File

@ -1048,6 +1048,7 @@ router.post("documents.update", auth(), async (ctx) => {
document.lastModifiedById = user.id; document.lastModifiedById = user.id;
const { collection } = document; const { collection } = document;
const changed = document.changed();
let transaction; let transaction;
try { try {
@ -1066,30 +1067,32 @@ router.post("documents.update", auth(), async (ctx) => {
throw err; throw err;
} }
if (publish) { if (changed) {
await Event.create({ if (publish) {
name: "documents.publish", await Event.create({
documentId: document.id, name: "documents.publish",
collectionId: document.collectionId, documentId: document.id,
teamId: document.teamId, collectionId: document.collectionId,
actorId: user.id, teamId: document.teamId,
data: { title: document.title }, actorId: user.id,
ip: ctx.request.ip, data: { title: document.title },
}); ip: ctx.request.ip,
} else { });
await Event.create({ } else {
name: "documents.update", await Event.create({
documentId: document.id, name: "documents.update",
collectionId: document.collectionId, documentId: document.id,
teamId: document.teamId, collectionId: document.collectionId,
actorId: user.id, teamId: document.teamId,
data: { actorId: user.id,
autosave, data: {
done, autosave,
title: document.title, done,
}, title: document.title,
ip: ctx.request.ip, },
}); ip: ctx.request.ip,
});
}
} }
if (document.title !== previousTitle) { if (document.title !== previousTitle) {