diff --git a/server/collaboration/ViewsExtension.ts b/server/collaboration/ViewsExtension.ts new file mode 100644 index 000000000..7b5e1ff02 --- /dev/null +++ b/server/collaboration/ViewsExtension.ts @@ -0,0 +1,73 @@ +import { + Extension, + onAwarenessUpdatePayload, + onDisconnectPayload, +} from "@hocuspocus/server"; +import { Second } from "@shared/utils/time"; +import Logger from "@server/logging/Logger"; +import { trace } from "@server/logging/tracing"; +import { View } from "@server/models"; + +@trace() +export class ViewsExtension implements Extension { + /** + * Map of socketId -> intervals + */ + intervalsBySocket: Map = new Map(); + + /** + * onAwarenessUpdate hook + * @param data The awareness payload + */ + async onAwarenessUpdate({ + documentName, + // @ts-expect-error Hocuspocus types are wrong + connection, + context, + socketId, + }: onAwarenessUpdatePayload) { + if (this.intervalsBySocket.get(socketId)) { + return; + } + + const [, documentId] = documentName.split("."); + + const updateView = async () => { + Logger.debug( + "multiplayer", + `Updating last viewed at for "${documentName}"` + ); + try { + await View.touch(documentId, context.user.id, !connection.readOnly); + } catch (err) { + Logger.error( + `Failed to update last viewed at for "${documentName}"`, + err, + { + documentId, + userId: context.user.id, + } + ); + } + }; + + // Set up an interval to update the last viewed at timestamp continuously + // while the user is connected. This should only be done once per socket. + const interval = setInterval(updateView, 30 * Second); + updateView(); + + this.intervalsBySocket.set(socketId, interval); + } + + /** + * onDisconnect hook + * @param data The disconnect payload + */ + async onDisconnect({ socketId }: onDisconnectPayload) { + const interval = this.intervalsBySocket.get(socketId); + if (interval) { + clearInterval(interval); + this.intervalsBySocket.delete(socketId); + } + } +} diff --git a/server/emails/templates/CommentCreatedEmail.tsx b/server/emails/templates/CommentCreatedEmail.tsx index bddec601b..8f2a1aca7 100644 --- a/server/emails/templates/CommentCreatedEmail.tsx +++ b/server/emails/templates/CommentCreatedEmail.tsx @@ -90,7 +90,7 @@ export default class CommentCreatedEmail extends BaseEmail< collectionName, }: Props): string { return ` -${actorName} ${isReply ? "replied in" : "commented on"} the document "${ +${actorName} ${isReply ? "replied to a thread in" : "commented on"} "${ document.title }", in the ${collectionName} collection. @@ -117,7 +117,7 @@ Open Thread: ${teamUrl}${document.url}?commentId=${commentId} {document.title}

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

diff --git a/server/models/View.ts b/server/models/View.ts index de6df60d0..1745e5559 100644 --- a/server/models/View.ts +++ b/server/models/View.ts @@ -1,5 +1,5 @@ import { subMilliseconds } from "date-fns"; -import { Op } from "sequelize"; +import { FindOrCreateOptions, Op } from "sequelize"; import { BelongsTo, Column, @@ -52,18 +52,21 @@ class View extends IdModel { @Column(DataType.UUID) documentId: string; - static async incrementOrCreate(where: { - userId?: string; - documentId?: string; - collectionId?: string; - }) { + static async incrementOrCreate( + where: { + userId: string; + documentId: string; + }, + options?: FindOrCreateOptions + ) { const [model, created] = await this.findOrCreate({ + ...options, where, }); if (!created) { model.count += 1; - model.save(); + model.save(options); } return model; @@ -104,20 +107,21 @@ class View extends IdModel { } static async touch(documentId: string, userId: string, isEditing: boolean) { - const [view] = await this.findOrCreate({ + const values: Partial = { + updatedAt: new Date(), + }; + + if (isEditing) { + values.lastEditingAt = new Date(); + } + + await this.update(values, { where: { userId, documentId, }, + returning: false, }); - - if (isEditing) { - const lastEditingAt = new Date(); - view.lastEditingAt = lastEditingAt; - await view.save(); - } - - return view; } } diff --git a/server/models/helpers/NotificationHelper.ts b/server/models/helpers/NotificationHelper.ts index 0f65cfd81..bb4c26121 100644 --- a/server/models/helpers/NotificationHelper.ts +++ b/server/models/helpers/NotificationHelper.ts @@ -1,11 +1,13 @@ import { uniqBy } from "lodash"; import { Op } from "sequelize"; +import Logger from "@server/logging/Logger"; import { Document, Collection, NotificationSetting, Subscription, Comment, + View, } from "@server/models"; export default class NotificationHelper { @@ -49,9 +51,9 @@ export default class NotificationHelper { comment: Comment, actorId: string ): Promise => { - const recipients = await this.getDocumentNotificationRecipients( + let recipients = await this.getDocumentNotificationRecipients( document, - "comments.create", + "documents.update", actorId, !comment.parentCommentId ); @@ -68,10 +70,35 @@ export default class NotificationHelper { }); const userIdsInThread = contextComments.map((c) => c.createdById); - return recipients.filter((r) => userIdsInThread.includes(r.userId)); + recipients = recipients.filter((r) => userIdsInThread.includes(r.userId)); } - return recipients; + const filtered: NotificationSetting[] = []; + + for (const recipient of recipients) { + // If this recipient has viewed the document since the comment was made + // then we can avoid sending them a useless notification, yay. + const view = await View.findOne({ + where: { + userId: recipient.userId, + documentId: document.id, + updatedAt: { + [Op.gt]: comment.createdAt, + }, + }, + }); + + if (view) { + Logger.info( + "processor", + `suppressing notification to ${recipient.userId} because doc viewed` + ); + } else { + filtered.push(recipient); + } + } + + return filtered; }; /** @@ -128,7 +155,7 @@ export default class NotificationHelper { const collectionIds = await recipient.user.collectionIds(); // Check the recipient has access to the collection this document is in. Just - // because they are subscribed doesn't meant they still have access to read + // because they are subscribed doesn't meant they "still have access to read // the document. if ( recipient.user.email && diff --git a/server/queues/processors/NotificationsProcessor.test.ts b/server/queues/processors/NotificationsProcessor.test.ts index e383e74c3..75efb4803 100644 --- a/server/queues/processors/NotificationsProcessor.test.ts +++ b/server/queues/processors/NotificationsProcessor.test.ts @@ -194,7 +194,10 @@ describe("revisions.create", () => { teamId: collaborator.teamId, event: "documents.update", }); - await View.touch(document.id, collaborator.id, true); + await View.create({ + userId: collaborator.id, + documentId: document.id, + }); const processor = new NotificationsProcessor(); await processor.perform({ @@ -581,7 +584,10 @@ describe("revisions.create", () => { teamId: collaborator.teamId, event: "documents.update", }); - await View.touch(document.id, collaborator.id, true); + await View.create({ + userId: collaborator.id, + documentId: document.id, + }); const processor = new NotificationsProcessor(); diff --git a/server/services/collaboration.ts b/server/services/collaboration.ts index ea2d68fe9..271d2ba8f 100644 --- a/server/services/collaboration.ts +++ b/server/services/collaboration.ts @@ -6,6 +6,7 @@ import Koa from "koa"; import WebSocket from "ws"; import { DocumentValidation } from "@shared/validations"; import { ConnectionLimitExtension } from "@server/collaboration/ConnectionLimitExtension"; +import { ViewsExtension } from "@server/collaboration/ViewsExtension"; import Logger from "@server/logging/Logger"; import ShutdownHelper, { ShutdownOrder } from "@server/utils/ShutdownHelper"; import AuthenticationExtension from "../collaboration/AuthenticationExtension"; @@ -32,6 +33,7 @@ export default function init( new ConnectionLimitExtension(), new AuthenticationExtension(), new PersistenceExtension(), + new ViewsExtension(), new LoggerExtension(), new MetricsExtension(), ], diff --git a/server/services/websockets.ts b/server/services/websockets.ts index b6ae895ee..fee81e6d9 100644 --- a/server/services/websockets.ts +++ b/server/services/websockets.ts @@ -276,9 +276,8 @@ async function authenticated(io: IO.Server, socket: SocketWithAuth) { const room = `document-${event.documentId}`; if (event.documentId && socket.rooms.has(room)) { - const view = await View.touch(event.documentId, user.id, event.isEditing); + await View.touch(event.documentId, user.id, event.isEditing); - view.user = user; io.to(room).emit("user.presence", { userId: user.id, documentId: event.documentId,