feat: Allow deletion of imports (#5907)

server/commands/collectionDestroyer.ts

@@ -0,0 +1,36 @@
+import { Transaction } from "sequelize";
+import { Collection, 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;
+};
+
+export default async function collectionDestroyer({
+  collection,
+  transaction,
+  user,
+  ip,
+}: Props) {
+  await collection.destroy({ transaction });
+
+  await Event.create(
+    {
+      name: "collections.delete",
+      collectionId: collection.id,
+      teamId: collection.teamId,
+      actorId: user.id,
+      data: {
+        name: collection.name,
+      },
+      ip,
+    },
+    { transaction }
+  );
+}

server/commands/fileOperationDeleter.test.ts

@@ -1,23 +0,0 @@
-import { FileOperation } from "@server/models";
-import { buildAdmin, buildFileOperation } from "@server/test/factories";
-import fileOperationDeleter from "./fileOperationDeleter";
-
-describe("fileOperationDeleter", () => {
-  const ip = "127.0.0.1";
-
-  it("should destroy file operation", async () => {
-    const admin = await buildAdmin();
-    const fileOp = await buildFileOperation({
-      userId: admin.id,
-      teamId: admin.teamId,
-    });
-    await fileOperationDeleter(fileOp, admin, ip);
-    expect(
-      await FileOperation.count({
-        where: {
-          teamId: admin.teamId,
-        },
-      })
-    ).toEqual(0);
-  });
-});

server/commands/fileOperationDeleter.ts

@@ -1,32 +1,30 @@
+import { Transaction } from "sequelize";
 import { FileOperation, Event, User } from "@server/models";
-import { sequelize } from "@server/storage/database";
 
-export default async function fileOperationDeleter(
-  fileOperation: FileOperation,
-  user: User,
-  ip: string
-) {
-  const transaction = await sequelize.transaction();
+type Props = {
+  fileOperation: FileOperation;
+  user: User;
+  ip: string;
+  transaction: Transaction;
+};
 
-  try {
-    await fileOperation.destroy({
+export default async function fileOperationDeleter({
+  fileOperation,
+  user,
+  ip,
+  transaction,
+}: Props) {
+  await fileOperation.destroy({ transaction });
+  await Event.create(
+    {
+      name: "fileOperations.delete",
+      teamId: user.teamId,
+      actorId: user.id,
+      modelId: fileOperation.id,
+      ip,
+    },
+    {
       transaction,
-    });
-    await Event.create(
-      {
-        name: "fileOperations.delete",
-        teamId: user.teamId,
-        actorId: user.id,
-        modelId: fileOperation.id,
-        ip,
-      },
-      {
-        transaction,
-      }
-    );
-    await transaction.commit();
-  } catch (error) {
-    await transaction.rollback();
-    throw error;
-  }
+    }
+  );
 }

server/migrations/20231001032754-file-operation-paranoid.js

@@ -0,0 +1,14 @@
+'use strict';
+
+module.exports = {
+  async up (queryInterface, Sequelize) {
+    await queryInterface.addColumn("file_operations", "deletedAt", {
+      type: Sequelize.DATE,
+      allowNull: true,
+    });
+  },
+
+  async down (queryInterface) {
+    await queryInterface.removeColumn("file_operations", "deletedAt");
+  }
+};

server/models/Collection.ts

@@ -29,6 +29,7 @@ import {
   Scopes,
   DataType,
   Length as SimpleLength,
+  BeforeDestroy,
 } from "sequelize-typescript";
 import isUUID from "validator/lib/isUUID";
 import type { CollectionSort } from "@shared/types";
@@ -37,6 +38,7 @@ import { sortNavigationNodes } from "@shared/utils/collections";
 import slugify from "@shared/utils/slugify";
 import { SLUG_URL_REGEX } from "@shared/utils/urlHelpers";
 import { CollectionValidation } from "@shared/validations";
+import { ValidationError } from "@server/errors";
 import Document from "./Document";
 import FileOperation from "./FileOperation";
 import Group from "./Group";
@@ -265,6 +267,18 @@ class Collection extends ParanoidModel {
     }
   }
 
+  @BeforeDestroy
+  static async checkLastCollection(model: Collection) {
+    const total = await this.count({
+      where: {
+        teamId: model.teamId,
+      },
+    });
+    if (total === 1) {
+      throw ValidationError("Cannot delete last collection");
+    }
+  }
+
   @AfterDestroy
   static async onAfterDestroy(model: Collection) {
     await Document.destroy({

server/models/Document.ts

@@ -459,16 +459,18 @@ class Document extends ParanoidModel {
       return null;
     }
 
+    const { includeState, userId, ...rest } = options;
+
     // allow default preloading of collection membership if `userId` is passed in find options
     // almost every endpoint needs the collection membership to determine policy permissions.
     const scope = this.scope([
-      ...(options.includeState ? [] : ["withoutState"]),
+      ...(includeState ? [] : ["withoutState"]),
       "withDrafts",
       {
-        method: ["withCollectionPermissions", options.userId, options.paranoid],
+        method: ["withCollectionPermissions", userId, rest.paranoid],
       },
       {
-        method: ["withViews", options.userId],
+        method: ["withViews", userId],
       },
     ]);
 
@@ -477,7 +479,7 @@ class Document extends ParanoidModel {
         where: {
           id,
         },
-        ...options,
+        ...rest,
       });
     }
 
@@ -487,7 +489,7 @@ class Document extends ParanoidModel {
         where: {
           urlId: match[1],
         },
-        ...options,
+        ...rest,
       });
     }
 

server/models/FileOperation.ts

@@ -17,7 +17,7 @@ import FileStorage from "@server/storage/files";
 import Collection from "./Collection";
 import Team from "./Team";
 import User from "./User";
-import IdModel from "./base/IdModel";
+import ParanoidModel from "./base/ParanoidModel";
 import Fix from "./decorators/Fix";
 
 @DefaultScope(() => ({
@@ -36,7 +36,7 @@ import Fix from "./decorators/Fix";
 }))
 @Table({ tableName: "file_operations", modelName: "file_operation" })
 @Fix
-class FileOperation extends IdModel {
+class FileOperation extends ParanoidModel {
   @Column(DataType.ENUM(...Object.values(FileOperationType)))
   type: FileOperationType;
 
@@ -73,7 +73,7 @@ class FileOperation extends IdModel {
         throw err;
       }
     }
-    await this.save();
+    return this.save();
   };
 
   /**

server/policies/fileOperation.ts

@@ -1,3 +1,4 @@
+import { FileOperationState, FileOperationType } from "@shared/types";
 import { User, Team, FileOperation } from "@server/models";
 import { allow } from "./cancan";
 
@@ -13,9 +14,22 @@ allow(
   }
 );
 
-allow(User, ["read", "delete"], FileOperation, (user, fileOperation) => {
+allow(User, "read", FileOperation, (user, fileOperation) => {
   if (!fileOperation || user.isViewer || user.teamId !== fileOperation.teamId) {
     return false;
   }
   return user.isAdmin;
 });
+
+allow(User, "delete", FileOperation, (user, fileOperation) => {
+  if (!fileOperation || user.isViewer || user.teamId !== fileOperation.teamId) {
+    return false;
+  }
+  if (
+    fileOperation.type === FileOperationType.Export &&
+    fileOperation.state !== FileOperationState.Complete
+  ) {
+    return false;
+  }
+  return user.isAdmin;
+});

server/queues/processors/CollectionDeletedProcessor.ts

@@ -0,0 +1,35 @@
+import teamUpdater from "@server/commands/teamUpdater";
+import { Team, User } from "@server/models";
+import { sequelize } from "@server/storage/database";
+import { Event as TEvent, CollectionEvent } from "@server/types";
+import BaseProcessor from "./BaseProcessor";
+
+export default class CollectionDeletedProcessor extends BaseProcessor {
+  static applicableEvents: TEvent["name"][] = ["collections.delete"];
+
+  async perform(event: CollectionEvent) {
+    await sequelize.transaction(async (transaction) => {
+      const team = await Team.findByPk(event.teamId, {
+        rejectOnEmpty: true,
+        transaction,
+        lock: transaction.LOCK.UPDATE,
+      });
+
+      if (team?.defaultCollectionId === event.collectionId) {
+        const user = await User.findByPk(event.actorId, {
+          rejectOnEmpty: true,
+          paranoid: false,
+          transaction,
+        });
+
+        await teamUpdater({
+          params: { defaultCollectionId: null },
+          user,
+          team,
+          transaction,
+          ip: event.ip,
+        });
+      }
+    });
+  }
+}

server/queues/processors/EmailsProcessor.ts

@@ -8,7 +8,7 @@ import { Notification } from "@server/models";
 import { Event, NotificationEvent } from "@server/types";
 import BaseProcessor from "./BaseProcessor";
 
-export default class NotificationsProcessor extends BaseProcessor {
+export default class EmailsProcessor extends BaseProcessor {
   static applicableEvents: Event["name"][] = ["notifications.create"];
 
   async perform(event: NotificationEvent) {

server/queues/processors/FileOperationCreatedProcessor.ts

@@ -1,4 +1,3 @@
-import invariant from "invariant";
 import { FileOperationFormat, FileOperationType } from "@shared/types";
 import { FileOperation } from "@server/models";
 import { Event as TEvent, FileOperationEvent } from "@server/types";
@@ -10,16 +9,13 @@ import ImportMarkdownZipTask from "../tasks/ImportMarkdownZipTask";
 import ImportNotionTask from "../tasks/ImportNotionTask";
 import BaseProcessor from "./BaseProcessor";
 
-export default class FileOperationsProcessor extends BaseProcessor {
+export default class FileOperationCreatedProcessor extends BaseProcessor {
   static applicableEvents: TEvent["name"][] = ["fileOperations.create"];
 
   async perform(event: FileOperationEvent) {
-    if (event.name !== "fileOperations.create") {
-      return;
-    }
-
-    const fileOperation = await FileOperation.findByPk(event.modelId);
-    invariant(fileOperation, "fileOperation not found");
+    const fileOperation = await FileOperation.findByPk(event.modelId, {
+      rejectOnEmpty: true,
+    });
 
     // map file operation type and format to the appropriate task
     if (fileOperation.type === FileOperationType.Import) {

server/queues/processors/FileOperationDeletedProcessor.ts

@@ -0,0 +1,54 @@
+import { FileOperationState, FileOperationType } from "@shared/types";
+import collectionDestroyer from "@server/commands/collectionDestroyer";
+import Logger from "@server/logging/Logger";
+import { Collection, FileOperation, User } from "@server/models";
+import { sequelize } from "@server/storage/database";
+import { Event as TEvent, FileOperationEvent } from "@server/types";
+import BaseProcessor from "./BaseProcessor";
+
+export default class FileOperationDeletedProcessor extends BaseProcessor {
+  static applicableEvents: TEvent["name"][] = ["fileOperations.delete"];
+
+  async perform(event: FileOperationEvent) {
+    await sequelize.transaction(async (transaction) => {
+      const fileOperation = await FileOperation.findByPk(event.modelId, {
+        rejectOnEmpty: true,
+        paranoid: false,
+        transaction,
+      });
+      if (
+        fileOperation.type === FileOperationType.Export ||
+        fileOperation.state !== FileOperationState.Complete
+      ) {
+        return;
+      }
+
+      const user = await User.findByPk(event.actorId, {
+        rejectOnEmpty: true,
+        paranoid: false,
+        transaction,
+      });
+
+      const collections = await Collection.findAll({
+        transaction,
+        lock: transaction.LOCK.UPDATE,
+        where: {
+          teamId: fileOperation.teamId,
+          importId: fileOperation.id,
+        },
+      });
+
+      for (const collection of collections) {
+        Logger.debug("processor", "Destroying collection created from import", {
+          collectionId: collection.id,
+        });
+        await collectionDestroyer({
+          collection,
+          transaction,
+          user,
+          ip: event.ip,
+        });
+      }
+    });
+  }
+}

server/queues/tasks/DetachDraftsFromCollectionTask.test.ts

@@ -13,7 +13,7 @@ describe("DetachDraftsFromCollectionTask", () => {
       createdById: collection.createdById,
       teamId: collection.teamId,
     });
-    await collection.destroy();
+    await collection.destroy({ hooks: false });
 
     const task = new DetachDraftsFromCollectionTask();
     await task.perform({

server/routes/api/collections/collections.ts

@@ -7,9 +7,9 @@ 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 { ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
 import { rateLimiter } from "@server/middlewares/rateLimiter";
 import { transaction } from "@server/middlewares/transaction";
@@ -803,44 +803,15 @@ router.post(
     }).findByPk(id, {
       transaction,
     });
-    const team = await Team.findByPk(user.teamId);
 
     authorize(user, "delete", collection);
 
-    const total = await Collection.count({
-      where: {
-        teamId: user.teamId,
-      },
+    await collectionDestroyer({
+      collection,
+      transaction,
+      user,
+      ip: ctx.request.ip,
     });
-    if (total === 1) {
-      throw ValidationError("Cannot delete last collection");
-    }
-
-    await collection.destroy({ transaction });
-
-    if (team && team.defaultCollectionId === collection.id) {
-      await teamUpdater({
-        params: { defaultCollectionId: null },
-        ip: ctx.request.ip,
-        user,
-        team,
-        transaction,
-      });
-    }
-
-    await Event.create(
-      {
-        name: "collections.delete",
-        collectionId: collection.id,
-        teamId: collection.teamId,
-        actorId: user.id,
-        data: {
-          name: collection.name,
-        },
-        ip: ctx.request.ip,
-      },
-      { transaction }
-    );
 
     ctx.body = {
       success: true,

server/routes/api/documents/documents.test.ts

@@ -2301,7 +2301,7 @@ describe("#documents.restore", () => {
       teamId: team.id,
     });
     await document.destroy();
-    await collection.destroy();
+    await collection.destroy({ hooks: false });
     const res = await server.post("/api/documents.restore", {
       body: {
         token: user.getJwtToken(),

server/routes/api/fileOperations/fileOperations.test.ts

@@ -150,7 +150,7 @@ describe("#fileOperations.list", () => {
       userId: admin.id,
       collectionId: collection.id,
     });
-    await collection.destroy();
+    await collection.destroy({ hooks: false });
     const isCollectionPresent = await Collection.findByPk(collection.id);
     expect(isCollectionPresent).toBe(null);
     const res = await server.post("/api/fileOperations.list", {

server/routes/api/fileOperations/fileOperations.ts

@@ -3,6 +3,7 @@ import { WhereOptions } from "sequelize";
 import fileOperationDeleter from "@server/commands/fileOperationDeleter";
 import { ValidationError } from "@server/errors";
 import auth from "@server/middlewares/authentication";
+import { transaction } from "@server/middlewares/transaction";
 import validate from "@server/middlewares/validate";
 import { FileOperation, Team } from "@server/models";
 import { authorize } from "@server/policies";
@@ -105,16 +106,23 @@ router.post(
   "fileOperations.delete",
   auth({ admin: true }),
   validate(T.FileOperationsDeleteSchema),
+  transaction(),
   async (ctx: APIContext<T.FileOperationsDeleteReq>) => {
     const { id } = ctx.input.body;
     const { user } = ctx.state.auth;
+    const { transaction } = ctx.state;
 
     const fileOperation = await FileOperation.unscoped().findByPk(id, {
       rejectOnEmpty: true,
     });
     authorize(user, "delete", fileOperation);
 
-    await fileOperationDeleter(fileOperation, user, ctx.request.ip);
+    await fileOperationDeleter({
+      fileOperation,
+      user,
+      ip: ctx.request.ip,
+      transaction,
+    });
 
     ctx.body = {
       success: true,

server/routes/api/teams/teams.test.ts

@@ -216,48 +216,6 @@ describe("#team.update", () => {
     expect(body.data.defaultCollectionId).toEqual(collection.id);
   });
 
-  it("should default to home if default collection is deleted", async () => {
-    const team = await buildTeam();
-    const admin = await buildAdmin({ teamId: team.id });
-    const collection = await buildCollection({
-      teamId: team.id,
-      userId: admin.id,
-    });
-
-    await buildCollection({
-      teamId: team.id,
-      userId: admin.id,
-    });
-
-    const res = await server.post("/api/team.update", {
-      body: {
-        token: admin.getJwtToken(),
-        defaultCollectionId: collection.id,
-      },
-    });
-
-    const body = await res.json();
-    expect(res.status).toEqual(200);
-    expect(body.data.defaultCollectionId).toEqual(collection.id);
-
-    const deleteRes = await server.post("/api/collections.delete", {
-      body: {
-        token: admin.getJwtToken(),
-        id: collection.id,
-      },
-    });
-    expect(deleteRes.status).toEqual(200);
-
-    const res3 = await server.post("/api/auth.info", {
-      body: {
-        token: admin.getJwtToken(),
-      },
-    });
-    const body3 = await res3.json();
-    expect(res3.status).toEqual(200);
-    expect(body3.data.team.defaultCollectionId).toEqual(null);
-  });
-
   it("should update default collection to null when collection is made private", async () => {
     const team = await buildTeam();
     const admin = await buildAdmin({ teamId: team.id });