From a57d90fdf11200f2318d8a1fd8aab94a1246bceb Mon Sep 17 00:00:00 2001 From: "codegen-sh[bot]" <131295404+codegen-sh[bot]@users.noreply.github.com> Date: Sat, 12 Jul 2025 09:24:53 -0400 Subject: [PATCH] Add `collaboratorIds` support to revisions (#9343) * Add collaboratorIds support to revision events - Add database migration to add collaboratorIds column to revisions table - Update server Revision model to include collaboratorIds field - Update client Revision model to include collaboratorIds field - Modify Revision.buildFromDocument to capture document collaborators - Update revisionCreator to include collaboratorIds in event data Fixes #6975 * fix to actually work * test: Add missing methods to mock * Return collaborators to client and display --------- Co-authored-by: codegen-sh[bot] <131295404+codegen-sh[bot]@users.noreply.github.com> Co-authored-by: Tom Moor --- app/components/EventListItem.tsx | 11 ++- app/models/Revision.ts | 8 ++ app/models/decorators/Relation.ts | 79 ++++++++++++++----- server/collaboration/PersistenceExtension.ts | 26 ++---- server/commands/revisionCreator.ts | 17 +++- ...35814-add-collaborator-ids-to-revisions.js | 17 ++++ server/models/Document.ts | 9 +++ server/models/Revision.ts | 33 ++++++++ server/presenters/revision.ts | 3 + server/storage/__mocks__/redis.ts | 5 ++ 10 files changed, 168 insertions(+), 40 deletions(-) create mode 100644 server/migrations/20250530235814-add-collaborator-ids-to-revisions.js diff --git a/app/components/EventListItem.tsx b/app/components/EventListItem.tsx index 01dab0496b..de1441075b 100644 --- a/app/components/EventListItem.tsx +++ b/app/components/EventListItem.tsx @@ -27,6 +27,7 @@ import useStores from "~/hooks/useStores"; import RevisionMenu from "~/menus/RevisionMenu"; import Logger from "~/utils/Logger"; import { documentHistoryPath } from "~/utils/routeHelpers"; +import Facepile from "./Facepile"; import Text from "./Text"; export type RevisionEvent = { @@ -191,6 +192,8 @@ const EventListItem = ({ event, document, ...rest }: Props) => { to = undefined; } + const revision = isRevision ? revisions.get(event.id) : undefined; + return event.name === "revisions.create" && !event.deletedAt ? ( { onClick={handleTimeClick} /> } - image={} + image={ + revision?.collaborators ? ( + + ) : ( + + ) + } subtitle={meta} actions={ isRevision && isActive && !event.latest ? ( diff --git a/app/models/Revision.ts b/app/models/Revision.ts index ce38ed0c32..01fb1489c9 100644 --- a/app/models/Revision.ts +++ b/app/models/Revision.ts @@ -36,9 +36,17 @@ class Revision extends ParanoidModel { /** HTML string representing the revision as a diff from the previous version */ html: string; + /** @deprecated Use collaborators instead*/ @Relation(() => User) createdBy: User; + /** Array of user IDs who collaborated on this revision */ + collaboratorIds: string[] = []; + + /** The user IDs who authored this revision */ + @Relation(() => User, { multiple: true, onDelete: "ignore" }) + collaborators: User[] = []; + /** * Returns the direction of the revision text, either "rtl" or "ltr" */ diff --git a/app/models/decorators/Relation.ts b/app/models/decorators/Relation.ts index 5a3f22b5cc..4264b3c4e1 100644 --- a/app/models/decorators/Relation.ts +++ b/app/models/decorators/Relation.ts @@ -1,4 +1,5 @@ import invariant from "invariant"; +import { singular } from "pluralize"; import type Model from "../base/Model"; /** The behavior of a relationship on deletion */ @@ -9,6 +10,8 @@ type ArchiveBehavior = "cascade" | "null" | "ignore"; type RelationOptions = { /** Whether this relation is required. */ required?: boolean; + /** If true, this relation is an array of IDs (one-to-many). */ + multiple?: boolean; /** Behavior of this model when relationship is deleted. */ onDelete?: DeleteBehavior | ((item: T) => DeleteBehavior); /** Behavior of this model when relationship is archived. */ @@ -72,7 +75,9 @@ export default function Relation( options?: RelationOptions ) { return function (target: any, propertyKey: string) { - const idKey = `${String(propertyKey)}Id`; + const idKey = options?.multiple + ? `${String(singular(propertyKey))}Ids` + : `${String(propertyKey)}Id`; // If the relation has options provided then register them in a map for later lookup. We can use // this to determine how to update relations when a model is deleted. @@ -89,33 +94,71 @@ export default function Relation( Object.defineProperty(target, propertyKey, { get() { - const id: string | undefined = this[idKey]; - - if (!id) { - return undefined; - } - const relationClassName = classResolver().modelName; const store = this.store.rootStore.getStoreForModelName(relationClassName); invariant(store, `Store for ${relationClassName} not found`); - return store.get(id); + if (options?.multiple) { + const ids: string[] | undefined = this[idKey]; + if (!Array.isArray(ids) || ids.length === 0) { + return []; + } + return ids.map((id) => store.get(id)).filter(Boolean); + } else { + const id: string | undefined = this[idKey]; + if (!id) { + return undefined; + } + return store.get(id); + } }, - set(newValue: Model | Partial | undefined) { - this[idKey] = newValue ? newValue.id : undefined; - - if (newValue) { + set( + newValue: + | Model + | Partial + | Array> + | undefined + ) { + if (options?.multiple) { + if (!newValue) { + this[idKey] = []; + if (options?.required) { + throw new Error( + `Cannot set required ${String( + propertyKey + )} to undefined or empty array` + ); + } + return; + } + const values = Array.isArray(newValue) ? newValue : [newValue]; + this[idKey] = values.map((v) => v.id); const relationClassName = classResolver().modelName; const store = this.store.rootStore.getStoreForModelName(relationClassName); invariant(store, `Store for ${relationClassName} not found`); - - store.add(newValue); - } else if (options?.required) { - throw new Error( - `Cannot set required ${String(propertyKey)} to undefined` - ); + values.forEach((v) => store.add(v)); + } else { + if (Array.isArray(newValue)) { + throw new Error( + `Cannot set array value to single relation property ${String( + propertyKey + )}` + ); + } + this[idKey] = newValue ? newValue.id : undefined; + if (newValue) { + const relationClassName = classResolver().modelName; + const store = + this.store.rootStore.getStoreForModelName(relationClassName); + invariant(store, `Store for ${relationClassName} not found`); + store.add(newValue); + } else if (options?.required) { + throw new Error( + `Cannot set required ${String(propertyKey)} to undefined` + ); + } } }, enumerable: true, diff --git a/server/collaboration/PersistenceExtension.ts b/server/collaboration/PersistenceExtension.ts index a4abf32423..7d0a53d515 100644 --- a/server/collaboration/PersistenceExtension.ts +++ b/server/collaboration/PersistenceExtension.ts @@ -10,17 +10,12 @@ import { trace } from "@server/logging/tracing"; import Document from "@server/models/Document"; import { ProsemirrorHelper } from "@server/models/helpers/ProsemirrorHelper"; import { sequelize } from "@server/storage/database"; +import Redis from "@server/storage/redis"; import documentCollaborativeUpdater from "../commands/documentCollaborativeUpdater"; import { withContext } from "./types"; @trace() export default class PersistenceExtension implements Extension { - /** - * Map of documentId -> userIds that have modified the document since it - * was last persisted to the database. The map is cleared on every save. - */ - documentCollaboratorIds = new Map>(); - async onLoadDocument({ documentName, ...data @@ -81,16 +76,17 @@ export default class PersistenceExtension implements Extension { } async onChange({ context, documentName }: withContext) { + const [, documentId] = documentName.split("."); + Logger.debug( "multiplayer", `${context.user?.name} changed ${documentName}` ); - const state = this.documentCollaboratorIds.get(documentName) ?? new Set(); if (context.user) { - state.add(context.user.id); + const key = Document.getCollaboratorKey(documentId); + await Redis.defaultClient.sadd(key, context.user.id); } - this.documentCollaboratorIds.set(documentName, state); } async onStoreDocument({ @@ -101,19 +97,13 @@ export default class PersistenceExtension implements Extension { }: onStoreDocumentPayload) { const [, documentId] = documentName.split("."); - // Find the collaborators that have modified the document since it was last - // persisted and clear the map, if there's no collaborators then we don't - // need to persist the document. - const documentCollaboratorIds = - this.documentCollaboratorIds.get(documentName); - if (!documentCollaboratorIds) { + const key = Document.getCollaboratorKey(documentId); + const sessionCollaboratorIds = await Redis.defaultClient.smembers(key); + if (!sessionCollaboratorIds || sessionCollaboratorIds.length === 0) { Logger.debug("multiplayer", `No changes for ${documentName}`); return; } - const sessionCollaboratorIds = Array.from(documentCollaboratorIds.values()); - this.documentCollaboratorIds.delete(documentName); - try { await documentCollaborativeUpdater({ documentId, diff --git a/server/commands/revisionCreator.ts b/server/commands/revisionCreator.ts index 6e7a279c77..486554ee20 100644 --- a/server/commands/revisionCreator.ts +++ b/server/commands/revisionCreator.ts @@ -1,5 +1,6 @@ import { Document, User, Event, Revision } from "@server/models"; import { sequelize } from "@server/storage/database"; +import Redis from "@server/storage/redis"; import { DocumentEvent, RevisionEvent } from "@server/types"; export default async function revisionCreator({ @@ -12,9 +13,19 @@ export default async function revisionCreator({ user: User; }) { return sequelize.transaction(async (transaction) => { - const revision = await Revision.createFromDocument(document, { - transaction, - }); + // Get collaborator IDs since last revision was written. + const key = Document.getCollaboratorKey(document.id); + const collaboratorIds = await Redis.defaultClient.smembers(key); + await Redis.defaultClient.del(key); + + const revision = await Revision.createFromDocument( + document, + collaboratorIds, + { + transaction, + } + ); + await Event.create( { name: "revisions.create", diff --git a/server/migrations/20250530235814-add-collaborator-ids-to-revisions.js b/server/migrations/20250530235814-add-collaborator-ids-to-revisions.js new file mode 100644 index 0000000000..1bffcffac6 --- /dev/null +++ b/server/migrations/20250530235814-add-collaborator-ids-to-revisions.js @@ -0,0 +1,17 @@ +'use strict'; + +/** @type {import('sequelize-cli').Migration} */ +module.exports = { + async up (queryInterface, Sequelize) { + await queryInterface.addColumn("revisions", "collaboratorIds", { + type: Sequelize.ARRAY(Sequelize.UUID), + allowNull: false, + defaultValue: [], + }); + }, + + async down (queryInterface, Sequelize) { + await queryInterface.removeColumn("revisions", "collaboratorIds"); + } +}; + diff --git a/server/models/Document.ts b/server/models/Document.ts index 97a0c38085..4ee9e91ef7 100644 --- a/server/models/Document.ts +++ b/server/models/Document.ts @@ -395,6 +395,15 @@ class Document extends ArchivableModel< ); } + /** + * Returns the key used to store the collaborators of a document in Redis. + * @param documentId The ID of the document. + * @returns Redis key for collaborators + */ + static getCollaboratorKey(documentId: string) { + return `collaborators:${documentId}`; + } + static getPath({ title, urlId }: { title: string; urlId: string }) { if (!title.length) { return `/doc/untitled-${urlId}`; diff --git a/server/models/Revision.ts b/server/models/Revision.ts index 7eabc9d249..15d8886164 100644 --- a/server/models/Revision.ts +++ b/server/models/Revision.ts @@ -110,6 +110,32 @@ class Revision extends ParanoidModel< @Column(DataType.UUID) userId: string; + /** Array of user IDs who collaborated on this revision */ + @Column(DataType.ARRAY(DataType.UUID)) + collaboratorIds: string[] = []; + + /** + * Get the collaborators for this revision. + */ + get collaborators() { + const otherCollaboratorIds = this.collaboratorIds.filter( + (id) => id !== this.userId + ); + + if (otherCollaboratorIds.length === 0) { + return [this.user]; + } + + return User.findAll({ + where: { + id: { + [Op.in]: otherCollaboratorIds, + }, + }, + paranoid: false, + }).then((others) => [this.user, ...others]); + } + // hooks @BeforeDestroy @@ -162,14 +188,21 @@ class Revision extends ParanoidModel< * Create a Revision model from a Document model and save it to the database * * @param document The document to create from + * @param collaboratorIds Optional array of user IDs who authored this revision * @param options Options passed to the save method * @returns A Promise that resolves when saved */ static createFromDocument( document: Document, + collaboratorIds?: string[], options?: SaveOptions> ) { const revision = this.buildFromDocument(document); + + if (collaboratorIds) { + revision.collaboratorIds = collaboratorIds; + } + return revision.save(options); } diff --git a/server/presenters/revision.ts b/server/presenters/revision.ts index 338b118762..33681798fd 100644 --- a/server/presenters/revision.ts +++ b/server/presenters/revision.ts @@ -17,6 +17,9 @@ async function presentRevision(revision: Revision, diff?: string) { icon: revision.icon ?? emoji, color: revision.color, html: diff, + collaborators: (await revision.collaborators).map((user) => + presentUser(user) + ), createdAt: revision.createdAt, createdBy: presentUser(revision.user), deletedAt: revision.deletedAt, diff --git a/server/storage/__mocks__/redis.ts b/server/storage/__mocks__/redis.ts index c0a349d492..3727575fd7 100644 --- a/server/storage/__mocks__/redis.ts +++ b/server/storage/__mocks__/redis.ts @@ -9,6 +9,11 @@ class RedisMock extends EventEmitter { get = jest.fn().mockResolvedValue(null); set = jest.fn().mockResolvedValue("OK"); del = jest.fn().mockResolvedValue(1); + hget = jest.fn().mockResolvedValue(null); + hset = jest.fn().mockResolvedValue("OK"); + hdel = jest.fn().mockResolvedValue(1); + sadd = jest.fn().mockResolvedValue(1); + smembers = jest.fn().mockResolvedValue([]); keys = jest.fn().mockResolvedValue([]); ping = jest.fn().mockResolvedValue("PONG"); disconnect = jest.fn();