diff --git a/server/commands/subscriptionCreator.ts b/server/commands/subscriptionCreator.ts index 7dea25ba31..c83d9665a3 100644 --- a/server/commands/subscriptionCreator.ts +++ b/server/commands/subscriptionCreator.ts @@ -1,7 +1,7 @@ -import { QueryTypes } from "sequelize"; +import { QueryTypes, type Transaction } from "sequelize"; import { SubscriptionType } from "@shared/types"; import { createContext } from "@server/context"; -import type { Document } from "@server/models"; +import type { Document, User } from "@server/models"; import { Subscription, Event } from "@server/models"; import { sequelize } from "@server/storage/database"; import type { APIContext, DocumentEvent, RevisionEvent } from "@server/types"; @@ -105,34 +105,81 @@ export default async function subscriptionCreator({ return subscription; } +/** + * Subscribe a single user to a document. The subscription is created if it + * does not exist; an existing subscription that has been deleted is left as-is + * so that the user's prior unsubscribe is respected. + * + * @param user The user to subscribe. + * @param document The document to subscribe the user to. + * @param event The event that triggered the subscription creation. + * @param options.transaction An existing transaction to run within. When + * subscribing many users in a row, callers should open a single transaction + * and pass it in to avoid the overhead of one BEGIN/COMMIT per call. + */ +export const subscribeUserToDocument = async ( + user: User, + document: Document, + event: DocumentEvent | RevisionEvent, + options: { transaction?: Transaction } = {} +): Promise => { + const run = (transaction: Transaction) => + subscriptionCreator({ + ctx: createContext({ + user, + authType: event.authType, + ip: event.ip, + transaction, + }), + documentId: document.id, + event: SubscriptionType.Document, + resubscribe: false, + }); + + if (options.transaction) { + await run(options.transaction); + return; + } + + await sequelize.transaction(run); +}; + +/** + * Subscribe a batch of users to a document inside a single transaction. + * + * @param users The users to subscribe. + * @param document The document to subscribe the users to. + * @param event The event that triggered the subscription creation. + */ +export const subscribeUsersToDocument = async ( + users: User[], + document: Document, + event: DocumentEvent | RevisionEvent +): Promise => { + if (!users.length) { + return; + } + + await sequelize.transaction(async (transaction) => { + for (const user of users) { + await subscribeUserToDocument(user, document, event, { transaction }); + } + }); +}; + /** * Create any new subscriptions that might be missing for collaborators in the * document on publish and revision creation. This does mean that there is a * short period of time where the user is not subscribed after editing until a * revision is created. * - * @param document The document to create subscriptions for - * @param event The event that triggered the subscription creation + * @param document The document to create subscriptions for. + * @param event The event that triggered the subscription creation. */ export const createSubscriptionsForDocument = async ( document: Document, event: DocumentEvent | RevisionEvent ): Promise => { const users = await document.collaborators(); - - for (const user of users) { - await sequelize.transaction(async (transaction) => { - await subscriptionCreator({ - ctx: createContext({ - user, - authType: event.authType, - ip: event.ip, - transaction, - }), - documentId: document.id, - event: SubscriptionType.Document, - resubscribe: false, - }); - }); - } + await subscribeUsersToDocument(users, document, event); }; diff --git a/server/queues/tasks/DocumentPublishedNotificationsTask.test.ts b/server/queues/tasks/DocumentPublishedNotificationsTask.test.ts index 303ee09baa..655ae88837 100644 --- a/server/queues/tasks/DocumentPublishedNotificationsTask.test.ts +++ b/server/queues/tasks/DocumentPublishedNotificationsTask.test.ts @@ -1,11 +1,16 @@ -import { v4 as uuidv4 } from "uuid"; -import { MentionType, NotificationEventType } from "@shared/types"; -import { Notification } from "@server/models"; +import { + MentionType, + NotificationEventType, + SubscriptionType, +} from "@shared/types"; +import { Notification, Subscription } from "@server/models"; import { buildDocument, buildCollection, buildGroup, buildGroupUser, + buildMention, + buildProseMirrorDoc, buildUser, } from "@server/test/factories"; import DocumentPublishedNotificationsTask from "./DocumentPublishedNotificationsTask"; @@ -141,26 +146,19 @@ describe("documents.publish", () => { const document = await buildDocument({ teamId: actor.teamId, userId: actor.id, - content: { - type: "doc", - content: [ - { - type: "paragraph", - content: [ - { - type: "mention", - attrs: { - id: uuidv4(), - type: MentionType.Group, - label: group.name, - modelId: group.id, - actorId: actor.id, - }, - }, - ], - }, - ], - }, + content: buildProseMirrorDoc([ + { + type: "paragraph", + content: [ + buildMention({ + type: MentionType.Group, + modelId: group.id, + actorId: actor.id, + label: group.name, + }), + ], + }, + ]).toJSON(), }); const processor = new DocumentPublishedNotificationsTask(); @@ -175,4 +173,139 @@ describe("documents.publish", () => { expect(spy).not.toHaveBeenCalled(); }); + + it("should subscribe a mentioned user to the document", async () => { + const actor = await buildUser(); + const mentioned = await buildUser({ teamId: actor.teamId }); + + const document = await buildDocument({ + teamId: actor.teamId, + userId: actor.id, + content: buildProseMirrorDoc([ + { + type: "paragraph", + content: [buildMention({ modelId: mentioned.id, actorId: actor.id })], + }, + ]).toJSON(), + }); + + const processor = new DocumentPublishedNotificationsTask(); + await processor.perform({ + name: "documents.publish", + documentId: document.id, + collectionId: document.collectionId!, + teamId: document.teamId, + actorId: actor.id, + ip, + }); + + const subscription = await Subscription.findOne({ + where: { + userId: mentioned.id, + documentId: document.id, + event: SubscriptionType.Document, + }, + }); + expect(subscription).not.toBeNull(); + }); + + it("should respect a prior unsubscribe when a user is mentioned", async () => { + const actor = await buildUser(); + const mentioned = await buildUser({ teamId: actor.teamId }); + + const document = await buildDocument({ + teamId: actor.teamId, + userId: actor.id, + content: buildProseMirrorDoc([ + { + type: "paragraph", + content: [buildMention({ modelId: mentioned.id, actorId: actor.id })], + }, + ]).toJSON(), + }); + + // The mentioned user previously subscribed and then unsubscribed. + const prior = await Subscription.create({ + userId: mentioned.id, + documentId: document.id, + event: SubscriptionType.Document, + }); + await prior.destroy(); + + const processor = new DocumentPublishedNotificationsTask(); + await processor.perform({ + name: "documents.publish", + documentId: document.id, + collectionId: document.collectionId!, + teamId: document.teamId, + actorId: actor.id, + ip, + }); + + // No active subscription should exist. + const active = await Subscription.findOne({ + where: { + userId: mentioned.id, + documentId: document.id, + event: SubscriptionType.Document, + }, + }); + expect(active).toBeNull(); + + // The original soft-deleted subscription should still be soft-deleted. + const withDeleted = await Subscription.findOne({ + where: { + userId: mentioned.id, + documentId: document.id, + event: SubscriptionType.Document, + }, + paranoid: false, + }); + expect(withDeleted).not.toBeNull(); + expect(withDeleted?.deletedAt).not.toBeNull(); + }); + + it("should not subscribe users mentioned via a group", async () => { + const actor = await buildUser(); + const group = await buildGroup({ teamId: actor.teamId }); + const member = await buildUser({ teamId: actor.teamId }); + await buildGroupUser({ groupId: group.id, userId: member.id }); + + const document = await buildDocument({ + teamId: actor.teamId, + userId: actor.id, + content: buildProseMirrorDoc([ + { + type: "paragraph", + content: [ + buildMention({ + type: MentionType.Group, + modelId: group.id, + actorId: actor.id, + label: group.name, + }), + ], + }, + ]).toJSON(), + }); + + const processor = new DocumentPublishedNotificationsTask(); + await processor.perform({ + name: "documents.publish", + documentId: document.id, + collectionId: document.collectionId!, + teamId: document.teamId, + actorId: actor.id, + ip, + }); + + const subscription = await Subscription.findOne({ + where: { + userId: member.id, + documentId: document.id, + event: SubscriptionType.Document, + }, + }); + expect(subscription).toBeNull(); + }); }); diff --git a/server/queues/tasks/DocumentPublishedNotificationsTask.ts b/server/queues/tasks/DocumentPublishedNotificationsTask.ts index 324e04a24f..1851c41855 100644 --- a/server/queues/tasks/DocumentPublishedNotificationsTask.ts +++ b/server/queues/tasks/DocumentPublishedNotificationsTask.ts @@ -1,5 +1,8 @@ import { MentionType, NotificationEventType } from "@shared/types"; -import { createSubscriptionsForDocument } from "@server/commands/subscriptionCreator"; +import { + createSubscriptionsForDocument, + subscribeUsersToDocument, +} from "@server/commands/subscriptionCreator"; import { Document, Group, Notification, User, GroupUser } from "@server/models"; import { DocumentHelper } from "@server/models/helpers/DocumentHelper"; import NotificationHelper from "@server/models/helpers/NotificationHelper"; @@ -22,22 +25,32 @@ export default class DocumentPublishedNotificationsTask extends BaseTask(); const userIdsMentioned: string[] = []; + const usersToSubscribe: User[] = []; for (const mention of mentions) { - if (userIdsMentioned.includes(mention.modelId)) { + if (userIdsProcessed.has(mention.modelId)) { continue; } + userIdsProcessed.add(mention.modelId); const recipient = await User.findByPk(mention.modelId); if ( - recipient && - recipient.id !== mention.actorId && + !recipient || + recipient.id === mention.actorId || + !(await canUserAccessDocument(recipient, document.id)) + ) { + continue; + } + + usersToSubscribe.push(recipient); + + if ( recipient.subscribedToEventType( NotificationEventType.MentionedInDocument - ) && - (await canUserAccessDocument(recipient, document.id)) + ) ) { await Notification.create({ event: NotificationEventType.MentionedInDocument, @@ -50,6 +63,8 @@ export default class DocumentPublishedNotificationsTask extends BaseTask { // Now add a mention – the only change is the mention node itself, which // renders as "@