fix: Add ability to permanently delete documents in trash (#2192)

* Align false conditions before true

* Update documents.delete endpoint for permanent delete

* Add permanent delete to events table and integrate with socket.io

* Add permanent delete to document menu

* Update parentDocumentId of direct child to null

* Add translation

* Add test for permanent delete

* Add space

* Update app/scenes/DocumentPermanentDelete.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Update app/stores/DocumentsStore.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Update server/commands/documentPermanentDeleter.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Update app/scenes/DocumentPermanentDelete.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* Change socket room from team to collection

* Add translation

* Create log func for commands

* Move tests from utils to permanentDeleter command

* Add additional tests

* Set redirect to trash

* Return promise from beforeEach

* Add undeleted documents validation

* Include deleteAt attribute in db query

* Update server/commands/documentPermanentDeleter.js

Co-authored-by: Tom Moor <tom.moor@gmail.com>

* tweak language

Co-authored-by: Tom Moor <tom.moor@gmail.com>

server/api/documents.js

@@ -5,6 +5,7 @@ import { subtractDate } from "../../shared/utils/date";
 import documentCreator from "../commands/documentCreator";
 import documentImporter from "../commands/documentImporter";
 import documentMover from "../commands/documentMover";
+import { documentPermanentDeleter } from "../commands/documentPermanentDeleter";
 import env from "../env";
 import {
   NotFoundError,
@@ -1174,24 +1175,53 @@ router.post("documents.archive", auth(), async (ctx) => {
 });
 
 router.post("documents.delete", auth(), async (ctx) => {
-  const { id } = ctx.body;
+  const { id, permanent } = ctx.body;
   ctx.assertPresent(id, "id is required");
 
   const user = ctx.state.user;
-  const document = await Document.findByPk(id, { userId: user.id });
-  authorize(user, "delete", document);
+  if (permanent) {
+    const document = await Document.findByPk(id, {
+      userId: user.id,
+      paranoid: false,
+    });
+    authorize(user, "permanentDelete", document);
 
-  await document.delete(user.id);
+    await Document.update(
+      { parentDocumentId: null },
+      {
+        where: {
+          parentDocumentId: document.id,
+        },
+        paranoid: false,
+      }
+    );
 
-  await Event.create({
-    name: "documents.delete",
-    documentId: document.id,
-    collectionId: document.collectionId,
-    teamId: document.teamId,
-    actorId: user.id,
-    data: { title: document.title },
-    ip: ctx.request.ip,
-  });
+    await documentPermanentDeleter([document]);
+
+    await Event.create({
+      name: "documents.permanent_delete",
+      documentId: document.id,
+      collectionId: document.collectionId,
+      teamId: document.teamId,
+      actorId: user.id,
+      data: { title: document.title },
+      ip: ctx.request.ip,
+    });
+  } else {
+    const document = await Document.findByPk(id, { userId: user.id });
+    authorize(user, "delete", document);
+
+    await document.delete(user.id);
+    await Event.create({
+      name: "documents.delete",
+      documentId: document.id,
+      collectionId: document.collectionId,
+      teamId: document.teamId,
+      actorId: user.id,
+      data: { title: document.title },
+      ip: ctx.request.ip,
+    });
+  }
 
   ctx.body = {
     success: true,

server/api/documents.test.js

@@ -2201,6 +2201,26 @@ describe("#documents.delete", () => {
     expect(body.success).toEqual(true);
   });
 
+  it("should allow permanently deleting a document", async () => {
+    const user = await buildUser();
+    const document = await buildDocument({
+      userId: user.id,
+      teamId: user.teamId,
+    });
+
+    await server.post("/api/documents.delete", {
+      body: { token: user.getJwtToken(), id: document.id },
+    });
+
+    const res = await server.post("/api/documents.delete", {
+      body: { token: user.getJwtToken(), id: document.id, permanent: true },
+    });
+    const body = await res.json();
+
+    expect(res.status).toEqual(200);
+    expect(body.success).toEqual(true);
+  });
+
   it("should allow deleting document without collection", async () => {
     const { user, document, collection } = await seed();
 

server/api/utils.js

@@ -2,10 +2,10 @@
 import { subDays } from "date-fns";
 import debug from "debug";
 import Router from "koa-router";
+import { documentPermanentDeleter } from "../commands/documentPermanentDeleter";
 import { AuthenticationError } from "../errors";
-import { Document, Attachment } from "../models";
-import { Op, sequelize } from "../sequelize";
-import parseAttachmentIds from "../utils/parseAttachmentIds";
+import { Document } from "../models";
+import { Op } from "../sequelize";
 
 const router = new Router();
 const log = debug("utils");
@@ -20,7 +20,7 @@ router.post("utils.gc", async (ctx) => {
   log(`Permanently destroying upto ${limit} documents older than 30 days…`);
 
   const documents = await Document.scope("withUnpublished").findAll({
-    attributes: ["id", "teamId", "text"],
+    attributes: ["id", "teamId", "text", "deletedAt"],
     where: {
       deletedAt: {
         [Op.lt]: subDays(new Date(), 30),
@@ -30,54 +30,9 @@ router.post("utils.gc", async (ctx) => {
     limit,
   });
 
-  const query = `
-  SELECT COUNT(id)
-  FROM documents
-  WHERE "searchVector" @@ to_tsquery('english', :query) AND
-  "teamId" = :teamId AND
-  "id" != :documentId
-`;
+  const countDeletedDocument = await documentPermanentDeleter(documents);
 
-  for (const document of documents) {
-    const attachmentIds = parseAttachmentIds(document.text);
-
-    for (const attachmentId of attachmentIds) {
-      const [{ count }] = await sequelize.query(query, {
-        type: sequelize.QueryTypes.SELECT,
-        replacements: {
-          documentId: document.id,
-          teamId: document.teamId,
-          query: attachmentId,
-        },
-      });
-
-      if (parseInt(count) === 0) {
-        const attachment = await Attachment.findOne({
-          where: {
-            teamId: document.teamId,
-            id: attachmentId,
-          },
-        });
-
-        if (attachment) {
-          await attachment.destroy();
-
-          log(`Attachment ${attachmentId} deleted`);
-        } else {
-          log(`Unknown attachment ${attachmentId} ignored`);
-        }
-      }
-    }
-  }
-
-  await Document.scope("withUnpublished").destroy({
-    where: {
-      id: documents.map((document) => document.id),
-    },
-    force: true,
-  });
-
-  log(`Destroyed ${documents.length} documents`);
+  log(`Destroyed ${countDeletedDocument} documents`);
 
   ctx.body = {
     success: true,

server/api/utils.test.js

@@ -2,8 +2,8 @@
 import { subDays } from "date-fns";
 import TestServer from "fetch-test-server";
 import app from "../app";
-import { Attachment, Document } from "../models";
-import { buildAttachment, buildDocument } from "../test/factories";
+import { Document } from "../models";
+import { buildDocument } from "../test/factories";
 import { flushdb } from "../test/support";
 
 const server = new TestServer(app.callback());
@@ -67,94 +67,6 @@ describe("#utils.gc", () => {
     expect(await Document.unscoped().count({ paranoid: false })).toEqual(0);
   });
 
-  it("should destroy attachments no longer referenced", async () => {
-    const document = await buildDocument({
-      publishedAt: subDays(new Date(), 90),
-      deletedAt: subDays(new Date(), 60),
-    });
-
-    const attachment = await buildAttachment({
-      teamId: document.teamId,
-      documentId: document.id,
-    });
-
-    document.text = `![text](${attachment.redirectUrl})`;
-    await document.save();
-
-    const res = await server.post("/api/utils.gc", {
-      body: {
-        token: process.env.UTILS_SECRET,
-      },
-    });
-
-    expect(res.status).toEqual(200);
-    expect(await Attachment.count()).toEqual(0);
-    expect(await Document.unscoped().count({ paranoid: false })).toEqual(0);
-  });
-
-  it("should handle unknown attachment ids", async () => {
-    const document = await buildDocument({
-      publishedAt: subDays(new Date(), 90),
-      deletedAt: subDays(new Date(), 60),
-    });
-
-    const attachment = await buildAttachment({
-      teamId: document.teamId,
-      documentId: document.id,
-    });
-
-    document.text = `![text](${attachment.redirectUrl})`;
-    await document.save();
-
-    // remove attachment so it no longer exists in the database, this is also
-    // representative of a corrupt attachment id in the doc or the regex returning
-    // an incorrect string
-    await attachment.destroy({ force: true });
-
-    const res = await server.post("/api/utils.gc", {
-      body: {
-        token: process.env.UTILS_SECRET,
-      },
-    });
-
-    expect(res.status).toEqual(200);
-    expect(await Attachment.count()).toEqual(0);
-    expect(await Document.unscoped().count({ paranoid: false })).toEqual(0);
-  });
-
-  it("should not destroy attachments referenced in other documents", async () => {
-    const document1 = await buildDocument();
-
-    const document = await buildDocument({
-      teamId: document1.teamId,
-      publishedAt: subDays(new Date(), 90),
-      deletedAt: subDays(new Date(), 60),
-    });
-
-    const attachment = await buildAttachment({
-      teamId: document1.teamId,
-      documentId: document.id,
-    });
-
-    document1.text = `![text](${attachment.redirectUrl})`;
-    await document1.save();
-
-    document.text = `![text](${attachment.redirectUrl})`;
-    await document.save();
-
-    expect(await Attachment.count()).toEqual(1);
-
-    const res = await server.post("/api/utils.gc", {
-      body: {
-        token: process.env.UTILS_SECRET,
-      },
-    });
-
-    expect(res.status).toEqual(200);
-    expect(await Attachment.count()).toEqual(1);
-    expect(await Document.unscoped().count({ paranoid: false })).toEqual(1);
-  });
-
   it("should destroy draft documents deleted more than 30 days ago", async () => {
     await buildDocument({
       publishedAt: undefined,

server/commands/documentPermanentDeleter.js

@@ -0,0 +1,64 @@
+// @flow
+import debug from "debug";
+import { Document, Attachment } from "../models";
+import { sequelize } from "../sequelize";
+import parseAttachmentIds from "../utils/parseAttachmentIds";
+
+const log = debug("commands");
+
+export async function documentPermanentDeleter(documents: Document[]) {
+  const activeDocument = documents.find((doc) => !doc.deletedAt);
+
+  if (activeDocument) {
+    throw new Error(
+      `Cannot permanently delete ${activeDocument.id} document. Please delete it and try again.`
+    );
+  }
+
+  const query = `
+    SELECT COUNT(id)
+    FROM documents
+    WHERE "searchVector" @@ to_tsquery('english', :query) AND
+    "teamId" = :teamId AND
+    "id" != :documentId
+  `;
+
+  for (const document of documents) {
+    const attachmentIds = parseAttachmentIds(document.text);
+
+    for (const attachmentId of attachmentIds) {
+      const [{ count }] = await sequelize.query(query, {
+        type: sequelize.QueryTypes.SELECT,
+        replacements: {
+          documentId: document.id,
+          teamId: document.teamId,
+          query: attachmentId,
+        },
+      });
+
+      if (parseInt(count) === 0) {
+        const attachment = await Attachment.findOne({
+          where: {
+            teamId: document.teamId,
+            id: attachmentId,
+          },
+        });
+
+        if (attachment) {
+          await attachment.destroy();
+
+          log(`Attachment ${attachmentId} deleted`);
+        } else {
+          log(`Unknown attachment ${attachmentId} ignored`);
+        }
+      }
+    }
+  }
+
+  return Document.scope("withUnpublished").destroy({
+    where: {
+      id: documents.map((document) => document.id),
+    },
+    force: true,
+  });
+}

server/commands/documentPermanentDeleter.test.js

@@ -0,0 +1,123 @@
+// @flow
+import { subDays } from "date-fns";
+import { Attachment, Document } from "../models";
+import { buildAttachment, buildDocument } from "../test/factories";
+import { flushdb } from "../test/support";
+import { documentPermanentDeleter } from "./documentPermanentDeleter";
+
+jest.mock("aws-sdk", () => {
+  const mS3 = { deleteObject: jest.fn().mockReturnThis(), promise: jest.fn() };
+  return {
+    S3: jest.fn(() => mS3),
+    Endpoint: jest.fn(),
+  };
+});
+
+beforeEach(() => flushdb());
+
+describe("documentPermanentDeleter", () => {
+  it("should destroy documents", async () => {
+    const document = await buildDocument({
+      publishedAt: subDays(new Date(), 90),
+      deletedAt: new Date(),
+    });
+
+    const countDeletedDoc = await documentPermanentDeleter([document]);
+
+    expect(countDeletedDoc).toEqual(1);
+    expect(await Document.unscoped().count({ paranoid: false })).toEqual(0);
+  });
+
+  it("should error when trying to destroy undeleted documents", async () => {
+    const document = await buildDocument({
+      publishedAt: new Date(),
+    });
+
+    let error;
+    try {
+      await documentPermanentDeleter([document]);
+    } catch (err) {
+      error = err.message;
+    }
+
+    expect(error).toEqual(
+      `Cannot permanently delete ${document.id} document. Please delete it and try again.`
+    );
+  });
+
+  it("should destroy attachments no longer referenced", async () => {
+    const document = await buildDocument({
+      publishedAt: subDays(new Date(), 90),
+      deletedAt: new Date(),
+    });
+
+    const attachment = await buildAttachment({
+      teamId: document.teamId,
+      documentId: document.id,
+    });
+
+    document.text = `![text](${attachment.redirectUrl})`;
+    await document.save();
+
+    const countDeletedDoc = await documentPermanentDeleter([document]);
+
+    expect(countDeletedDoc).toEqual(1);
+    expect(await Attachment.count()).toEqual(0);
+    expect(await Document.unscoped().count({ paranoid: false })).toEqual(0);
+  });
+
+  it("should handle unknown attachment ids", async () => {
+    const document = await buildDocument({
+      publishedAt: subDays(new Date(), 90),
+      deletedAt: new Date(),
+    });
+
+    const attachment = await buildAttachment({
+      teamId: document.teamId,
+      documentId: document.id,
+    });
+
+    document.text = `![text](${attachment.redirectUrl})`;
+    await document.save();
+
+    // remove attachment so it no longer exists in the database, this is also
+    // representative of a corrupt attachment id in the doc or the regex returning
+    // an incorrect string
+    await attachment.destroy({ force: true });
+
+    const countDeletedDoc = await documentPermanentDeleter([document]);
+
+    expect(countDeletedDoc).toEqual(1);
+    expect(await Attachment.count()).toEqual(0);
+    expect(await Document.unscoped().count({ paranoid: false })).toEqual(0);
+  });
+
+  it("should not destroy attachments referenced in other documents", async () => {
+    const document1 = await buildDocument();
+
+    const document = await buildDocument({
+      teamId: document1.teamId,
+      publishedAt: subDays(new Date(), 90),
+      deletedAt: subDays(new Date(), 60),
+    });
+
+    const attachment = await buildAttachment({
+      teamId: document1.teamId,
+      documentId: document.id,
+    });
+
+    document1.text = `![text](${attachment.redirectUrl})`;
+    await document1.save();
+
+    document.text = `![text](${attachment.redirectUrl})`;
+    await document.save();
+
+    expect(await Attachment.count()).toEqual(1);
+
+    const countDeletedDoc = await documentPermanentDeleter([document]);
+
+    expect(countDeletedDoc).toEqual(1);
+    expect(await Attachment.count()).toEqual(1);
+    expect(await Document.unscoped().count({ paranoid: false })).toEqual(1);
+  });
+});

server/events.js

@@ -35,6 +35,7 @@ export type DocumentEvent =
   name: | "documents.create" // eslint-disable-line
         | "documents.publish"
         | "documents.delete"
+        | "documents.permanent_delete"
         | "documents.pin"
         | "documents.unpin"
         | "documents.archive"

server/models/Event.js

@@ -65,6 +65,7 @@ Event.ACTIVITY_EVENTS = [
   "documents.unpin",
   "documents.move",
   "documents.delete",
+  "documents.permanent_delete",
   "documents.restore",
   "users.create",
 ];
@@ -90,6 +91,7 @@ Event.AUDIT_EVENTS = [
   "documents.unpin",
   "documents.move",
   "documents.delete",
+  "documents.permanent_delete",
   "documents.restore",
   "groups.create",
   "groups.update",

server/policies/document.js

@@ -101,8 +101,15 @@ allow(User, ["pin", "unpin"], Document, (user, document) => {
 });
 
 allow(User, "delete", Document, (user, document) => {
-  // unpublished drafts can always be deleted
   if (user.isViewer) return false;
+  if (document.deletedAt) return false;
+
+  // allow deleting document without a collection
+  if (document.collection && cannot(user, "update", document.collection)) {
+    return false;
+  }
+
+  // unpublished drafts can always be deleted
   if (
     !document.deletedAt &&
     !document.publishedAt &&
@@ -111,13 +118,18 @@ allow(User, "delete", Document, (user, document) => {
     return true;
   }
 
+  return user.teamId === document.teamId;
+});
+
+allow(User, "permanentDelete", Document, (user, document) => {
+  if (user.isViewer) return false;
+  if (!document.deletedAt) return false;
+
   // allow deleting document without a collection
   if (document.collection && cannot(user, "update", document.collection)) {
     return false;
   }
 
-  if (document.deletedAt) return false;
-
   return user.teamId === document.teamId;
 });
 

server/services/websockets.js

@@ -79,6 +79,13 @@ export default class Websockets {
             ],
           });
       }
+      case "documents.permanent_delete": {
+        return socketio
+          .to(`collection-${event.collectionId}`)
+          .emit(event.name, {
+            documentId: event.documentId,
+          });
+      }
       case "documents.pin":
       case "documents.unpin":
       case "documents.update": {