chore: Move Collection event writing to model layer (#9663)

* collections.create, collections.update, collections.delete API

* collections.archive, collections.restore

* collections.move

* file imports

* remove collectionDestroyer

* remove data field

* remove data field for collections.move

* remove data field for import flow

* use hook for permission_changed event

* simplify event type

* tiny

server/commands/collectionDestroyer.ts

@@ -1,53 +0,0 @@
-import { Transaction, Op } from "sequelize";
-import { Collection, Document, Event, User } from "@server/models";
-
-type Props = {
-  /** The collection to delete */
-  collection: Collection;
-  /** The actor who is deleting the collection */
-  user: User;
-  /** The database transaction to use */
-  transaction: Transaction;
-  /** The IP address of the current request */
-  ip: string | null;
-};
-
-export default async function collectionDestroyer({
-  collection,
-  transaction,
-  user,
-  ip,
-}: Props) {
-  await collection.destroy({ transaction });
-
-  await Document.update(
-    {
-      lastModifiedById: user.id,
-      deletedAt: new Date(),
-    },
-    {
-      transaction,
-      where: {
-        teamId: collection.teamId,
-        collectionId: collection.id,
-        archivedAt: {
-          [Op.is]: null,
-        },
-      },
-    }
-  );
-
-  await Event.create(
-    {
-      name: "collections.delete",
-      collectionId: collection.id,
-      teamId: collection.teamId,
-      actorId: user.id,
-      data: {
-        name: collection.name,
-      },
-      ip,
-    },
-    { transaction }
-  );
-}

server/models/Collection.ts

@@ -16,6 +16,7 @@ import {
   type UpdateOptions,
   type ScopeOptions,
   type SaveOptions,
+  Op,
 } from "sequelize";
 import {
   Sequelize,
@@ -50,6 +51,7 @@ import { sortNavigationNodes } from "@shared/utils/collections";
 import slugify from "@shared/utils/slugify";
 import { CollectionValidation } from "@shared/validations";
 import { ValidationError } from "@server/errors";
+import { APIContext } from "@server/types";
 import { CacheHelper } from "@server/utils/CacheHelper";
 import removeIndexCollision from "@server/utils/removeIndexCollision";
 import { generateUrlId } from "@server/utils/url";
@@ -386,6 +388,26 @@ class Collection extends ParanoidModel<
     }
   }
 
+  @BeforeDestroy
+  static async deleteDocuments(model: Collection, ctx: APIContext["context"]) {
+    await Document.update(
+      {
+        lastModifiedById: ctx.auth.user.id,
+        deletedAt: new Date(),
+      },
+      {
+        transaction: ctx.transaction,
+        where: {
+          teamId: model.teamId,
+          collectionId: model.id,
+          archivedAt: {
+            [Op.is]: null,
+          },
+        },
+      }
+    );
+  }
+
   @BeforeCreate
   static async setIndex(model: Collection, options: CreateOptions<Collection>) {
     if (model.index) {
@@ -444,6 +466,22 @@ class Collection extends ParanoidModel<
     }
   }
 
+  @BeforeUpdate
+  static async publishPermissionChangedEvent(
+    model: Collection,
+    ctx: APIContext["context"]
+  ) {
+    const privacyChanged = model.previous("permission") !== model.permission;
+    const sharingChanged = model.previous("sharing") !== model.sharing;
+
+    if (privacyChanged || sharingChanged) {
+      await this.insertEvent("permission_changed", model, {
+        ...ctx,
+        event: { publish: true },
+      });
+    }
+  }
+
   // associations
 
   @BelongsTo(() => FileOperation, "importId")

server/queues/processors/FileOperationDeletedProcessor.ts

@@ -1,5 +1,5 @@
 import { FileOperationState, FileOperationType } from "@shared/types";
-import collectionDestroyer from "@server/commands/collectionDestroyer";
+import { createContext } from "@server/context";
 import Logger from "@server/logging/Logger";
 import { Collection, FileOperation, User } from "@server/models";
 import { sequelize } from "@server/storage/database";
@@ -58,12 +58,9 @@ export default class FileOperationDeletedProcessor extends BaseProcessor {
         Logger.debug("processor", "Destroying collection created from import", {
           collectionId: collection.id,
         });
-        await collectionDestroyer({
-          collection,
-          transaction,
-          user,
-          ip: event.ip,
-        });
+        await collection.destroyWithCtx(
+          createContext({ user, ip: event.ip, transaction })
+        );
       }
     });
   }

server/queues/processors/ImportsProcessor.ts

@@ -22,7 +22,6 @@ import {
 } from "@shared/types";
 import { colorPalette } from "@shared/utils/collections";
 import { CollectionValidation } from "@shared/validations";
-import collectionDestroyer from "@server/commands/collectionDestroyer";
 import { createContext } from "@server/context";
 import { schema } from "@server/editor";
 import Logger from "@server/logging/Logger";
@@ -254,13 +253,9 @@ export default abstract class ImportsProcessor<
       Logger.debug("processor", "Destroying collection created from import", {
         collectionId: collection.id,
       });
-
-      await collectionDestroyer({
-        collection,
-        transaction,
-        user,
-        ip: event.ip,
-      });
+      await collection.destroyWithCtx(
+        createContext({ user, ip: event.ip, transaction })
+      );
     }
   }
 
@@ -386,7 +381,6 @@ export default abstract class ImportsProcessor<
                 { silent: true },
                 {
                   name: "create",
-                  data: { name: output.title, source: "import" },
                 }
               );
 

server/queues/processors/NotificationsProcessor.ts

@@ -86,9 +86,8 @@ export default class NotificationsProcessor extends BaseProcessor {
   async collectionCreated(event: CollectionEvent) {
     // never send notifications when batch importing
     if (
-      "data" in event &&
-      "source" in event.data &&
-      event.data.source === "import"
+      !!event.changes?.attributes.apiImportId ||
+      !!event.changes?.attributes.importId
     ) {
       return;
     }

server/queues/processors/WebsocketsProcessor.ts

@@ -318,11 +318,16 @@ export default class WebsocketsProcessor {
           return;
         }
 
+        const archivedAt =
+          event.name === "collections.archive"
+            ? event.changes?.attributes.archivedAt
+            : event.changes?.previous.archivedAt;
+
         return socketio
           .to(this.getCollectionEventChannels(event, collection))
           .emit(event.name, {
             id: event.collectionId,
-            archivedAt: event.data.archivedAt,
+            archivedAt,
           });
       }
 
@@ -331,7 +336,7 @@ export default class WebsocketsProcessor {
           .to(`collection-${event.collectionId}`)
           .emit("collections.update_index", {
             collectionId: event.collectionId,
-            index: event.data.index,
+            index: event.changes?.attributes.index,
           });
       }
 

server/queues/tasks/CollectionCreatedNotificationsTask.ts

@@ -13,10 +13,6 @@ export default class CollectionCreatedNotificationsTask extends BaseTask<Collect
       return;
     }
 
-    if ("source" in event.data && event.data.source === "import") {
-      return;
-    }
-
     const recipients =
       await NotificationHelper.getCollectionNotificationRecipients(
         collection,

server/queues/tasks/ImportTask.ts

@@ -304,7 +304,6 @@ export default abstract class ImportTask extends BaseTask<Props> {
     const user = await User.findByPk(fileOperation.userId, {
       rejectOnEmpty: true,
     });
-    const ip = user.lastActiveIp || undefined;
 
     try {
       await this.preprocessDocUrlIds(data);
@@ -377,14 +376,15 @@ export default abstract class ImportTask extends BaseTask<Props> {
             importId: fileOperation.id,
           };
 
+          const ctx = createContext({ user, transaction });
+
           // check if collection with name exists
-          const response = await Collection.findOrCreate({
+          const response = await Collection.findOrCreateWithCtx(ctx, {
             where: {
               teamId: fileOperation.teamId,
               name: item.name,
             },
             defaults: sharedDefaults,
-            transaction,
           });
 
           let collection = response[0];
@@ -395,32 +395,13 @@ export default abstract class ImportTask extends BaseTask<Props> {
           // with right now
           if (!isCreated) {
             const name = `${item.name} (Imported)`;
-            collection = await Collection.create(
-              {
-                ...sharedDefaults,
-                name,
-                teamId: fileOperation.teamId,
-              },
-              { transaction }
-            );
+            collection = await Collection.createWithCtx(ctx, {
+              ...sharedDefaults,
+              name,
+              teamId: fileOperation.teamId,
+            });
           }
 
-          await Event.create(
-            {
-              name: "collections.create",
-              collectionId: collection.id,
-              teamId: collection.teamId,
-              actorId: fileOperation.userId,
-              data: {
-                name: collection.name,
-              },
-              ip,
-            },
-            {
-              transaction,
-            }
-          );
-
           collections.set(item.id, collection);
 
           // Documents

server/routes/api/collections/collections.ts

@@ -6,7 +6,6 @@ import {
   FileOperationState,
   FileOperationType,
 } from "@shared/types";
-import collectionDestroyer from "@server/commands/collectionDestroyer";
 import collectionExporter from "@server/commands/collectionExporter";
 import teamUpdater from "@server/commands/teamUpdater";
 import { parser } from "@server/editor";
@@ -19,7 +18,6 @@ import {
   UserMembership,
   GroupMembership,
   Team,
-  Event,
   User,
   Group,
   Attachment,
@@ -88,15 +86,8 @@ router.post(
       collection.description = DocumentHelper.toMarkdown(collection);
     }
 
-    await collection.save({ transaction });
+    await collection.saveWithCtx(ctx);
 
-    await Event.createFromContext(ctx, {
-      name: "collections.create",
-      collectionId: collection.id,
-      data: {
-        name,
-      },
-    });
     // we must reload the collection to get memberships for policy presenter
     const reloaded = await Collection.findByPk(collection.id, {
       userId: user.id,
@@ -653,25 +644,7 @@ router.post(
       collection.commenting = commenting;
     }
 
-    await collection.save({ transaction });
-    await Event.createFromContext(ctx, {
-      name: "collections.update",
-      collectionId: collection.id,
-      data: {
-        name,
-      },
-    });
-
-    if (privacyChanged || sharingChanged) {
-      await Event.createFromContext(ctx, {
-        name: "collections.permission_changed",
-        collectionId: collection.id,
-        data: {
-          privacyChanged,
-          sharingChanged,
-        },
-      });
-    }
+    await collection.saveWithCtx(ctx);
 
     // must reload to update collection membership for correct policy calculation
     // if the privacy level has changed. Otherwise skip this query for speed.
@@ -829,12 +802,7 @@ router.post(
 
     authorize(user, "delete", collection);
 
-    await collectionDestroyer({
-      collection,
-      transaction,
-      user,
-      ip: ctx.request.ip,
-    });
+    await collection.destroyWithCtx(ctx);
 
     ctx.body = {
       success: true,
@@ -862,9 +830,12 @@ router.post(
 
     collection.archivedAt = new Date();
     collection.archivedById = user.id;
-    await collection.save({ transaction });
     collection.archivedBy = user;
 
+    await collection.saveWithCtx(ctx, undefined, {
+      name: "archive",
+    });
+
     // Archive all documents within the collection
     await Document.update(
       {
@@ -883,15 +854,6 @@ router.post(
       }
     );
 
-    await Event.createFromContext(ctx, {
-      name: "collections.archive",
-      collectionId: collection.id,
-      data: {
-        name: collection.name,
-        archivedAt: collection.archivedAt,
-      },
-    });
-
     ctx.body = {
       data: await presentCollection(ctx, collection),
       policies: presentPolicies(user, [collection]),
@@ -909,7 +871,7 @@ router.post(
     const { id } = ctx.input.body;
     const { user } = ctx.state.auth;
 
-    const collection = await Collection.findByPk(id, {
+    let collection = await Collection.findByPk(id, {
       userId: user.id,
       includeDocumentStructure: true,
       rejectOnEmpty: true,
@@ -918,8 +880,6 @@ router.post(
 
     authorize(user, "restore", collection);
 
-    const collectionArchivedAt = collection.archivedAt;
-
     await Document.update(
       {
         lastModifiedById: user.id,
@@ -937,15 +897,8 @@ router.post(
 
     collection.archivedAt = null;
     collection.archivedById = null;
-    await collection.save({ transaction });
-
-    await Event.createFromContext(ctx, {
-      name: "collections.restore",
-      collectionId: collection.id,
-      data: {
-        name: collection.name,
-        archivedAt: collectionArchivedAt,
-      },
+    collection = await collection.saveWithCtx(ctx, undefined, {
+      name: "restore",
     });
 
     ctx.body = {
@@ -971,21 +924,13 @@ router.post(
     });
     authorize(user, "move", collection);
 
-    collection = await collection.update(
+    collection = await collection.updateWithCtx(
+      ctx,
+      { index },
       {
-        index,
-      },
-      {
-        transaction,
+        name: "move",
       }
     );
-    await Event.createFromContext(ctx, {
-      name: "collections.move",
-      collectionId: collection.id,
-      data: {
-        index: collection.index,
-      },
-    });
 
     ctx.body = {
       success: true,

server/types.ts

@@ -307,44 +307,17 @@ export type DocumentGroupEvent = BaseEvent<GroupMembership> & {
   };
 };
 
-export type CollectionEvent = BaseEvent<Collection> &
-  (
-    | {
-        name: "collections.create";
-        collectionId: string;
-        data: {
-          name: string;
-          source?: "import";
-        };
-      }
-    | {
-        name:
-          | "collections.update"
-          | "collections.delete"
-          | "collections.archive"
-          | "collections.restore";
-        collectionId: string;
-        data: {
-          name: string;
-          archivedAt: string;
-        };
-      }
-    | {
-        name: "collections.move";
-        collectionId: string;
-        data: {
-          index: string;
-        };
-      }
-    | {
-        name: "collections.permission_changed";
-        collectionId: string;
-        data: {
-          privacyChanged: boolean;
-          sharingChanged: boolean;
-        };
-      }
-  );
+export type CollectionEvent = BaseEvent<Collection> & {
+  name:
+    | "collections.create"
+    | "collections.update"
+    | "collections.delete"
+    | "collections.archive"
+    | "collections.restore"
+    | "collections.move"
+    | "collections.permission_changed";
+  collectionId: string;
+};
 
 export type GroupUserEvent = BaseEvent<UserMembership> & {
   name: "groups.add_user" | "groups.remove_user";