fix: Deleting a document should correctly show who deleted (#1488)

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",

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) {
+Document.prototype.delete = function (userId: string) {
-  return sequelize.transaction(async (transaction: Transaction): Promise<*> => {
+  return sequelize.transaction(
-    if (!this.archivedAt) {
+    async (transaction: Transaction): Promise<Document> => {
-      // delete any children and remove from the document structure
+      if (!this.archivedAt && !this.template) {
-      const collection = await this.getCollection();
+        // delete any children and remove from the document structure
-      if (collection) await collection.deleteDocument(this, { transaction });
+        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 () {

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();
+  });
+});