From 85d09b2351d4810cca8204f8c29d7edfb8a7e373 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Tue, 25 Aug 2020 08:51:12 -0700
Subject: [PATCH] fix: Deleting a document should correctly show who deleted
 (#1488)

---
 server/api/documents.js        |  2 +-
 server/models/Document.js      | 38 +++++++++++++++++++---------------
 server/models/Document.test.js | 20 +++++++++++++++++-
 3 files changed, 41 insertions(+), 19 deletions(-)

diff --git a/server/api/documents.js b/server/api/documents.js
index 90f88e1a7..7135992d0 100644
--- a/server/api/documents.js
+++ b/server/api/documents.js
@@ -1001,7 +1001,7 @@ router.post("documents.delete", auth(), async (ctx) => {
   const document = await Document.findByPk(id, { userId: user.id });
   authorize(user, "delete", document);
 
-  await document.delete();
+  await document.delete(user.id);
 
   await Event.create({
     name: "documents.delete",
diff --git a/server/models/Document.js b/server/models/Document.js
index 6518bd1c6..4d08f077d 100644
--- a/server/models/Document.js
+++ b/server/models/Document.js
@@ -570,7 +570,7 @@ Document.prototype.archive = async function (userId) {
 };
 
 // Restore an archived document back to being visible to the team
-Document.prototype.unarchive = async function (userId) {
+Document.prototype.unarchive = async function (userId: string) {
   const collection = await this.getCollection();
 
   // check to see if the documents parent hasn't been archived also
@@ -602,23 +602,27 @@ Document.prototype.unarchive = async function (userId) {
 };
 
 // Delete a document, archived or otherwise.
-Document.prototype.delete = function (options) {
-  return sequelize.transaction(async (transaction: Transaction): Promise<*> => {
-    if (!this.archivedAt) {
-      // delete any children and remove from the document structure
-      const collection = await this.getCollection();
-      if (collection) await collection.deleteDocument(this, { transaction });
+Document.prototype.delete = function (userId: string) {
+  return sequelize.transaction(
+    async (transaction: Transaction): Promise<Document> => {
+      if (!this.archivedAt && !this.template) {
+        // delete any children and remove from the document structure
+        const collection = await this.getCollection();
+        if (collection) await collection.deleteDocument(this, { transaction });
+      }
+
+      await Revision.destroy({
+        where: { documentId: this.id },
+        transaction,
+      });
+
+      this.lastModifiedById = userId;
+      this.deletedAt = new Date();
+
+      await this.save({ transaction });
+      return this;
     }
-
-    await Revision.destroy({
-      where: { documentId: this.id },
-      transaction,
-    });
-
-    await this.destroy({ transaction, ...options });
-
-    return this;
-  });
+  );
 };
 
 Document.prototype.getTimestamp = function () {
diff --git a/server/models/Document.test.js b/server/models/Document.test.js
index 1bdae5e12..c8e25c6be 100644
--- a/server/models/Document.test.js
+++ b/server/models/Document.test.js
@@ -1,6 +1,11 @@
 /* eslint-disable flowtype/require-valid-file-annotation */
 import { Document } from "../models";
-import { buildDocument, buildCollection, buildTeam } from "../test/factories";
+import {
+  buildDocument,
+  buildCollection,
+  buildTeam,
+  buildUser,
+} from "../test/factories";
 import { flushdb } from "../test/support";
 
 beforeEach(() => flushdb());
@@ -192,3 +197,16 @@ describe("#searchForTeam", () => {
     expect(results.length).toBe(0);
   });
 });
+
+describe("#delete", () => {
+  test("should soft delete and set last modified", async () => {
+    let document = await buildDocument();
+    let user = await buildUser();
+
+    await document.delete(user.id);
+
+    document = await Document.findByPk(document.id, { paranoid: false });
+    expect(document.lastModifiedById).toBe(user.id);
+    expect(document.deletedAt).toBeTruthy();
+  });
+});