From 81718c8ee1c8b90ef8f1d687b50750f42bf91f19 Mon Sep 17 00:00:00 2001
From: Saumya Pandey <sp160899@gmail.com>
Date: Thu, 7 Oct 2021 09:38:45 +0530
Subject: [PATCH] fix: Delete collection exports (#2595)

---
 app/menus/FileOperationMenu.js                | 43 +++++++++++++++++++
 app/scenes/Settings/ImportExport.js           | 14 ++++++
 .../components/FileOperationListItem.js       | 27 +++++++-----
 app/stores/FileOperationsStore.js             |  2 +-
 server/commands/fileOperationDeleter.js       | 31 +++++++++++++
 server/commands/fileOperationDeleter.test.js  | 31 +++++++++++++
 server/models/FileOperation.js                |  6 ++-
 server/routes/api/fileOperations.js           | 23 +++++++++-
 server/routes/api/fileOperations.test.js      | 36 +++++++++++++++-
 server/test/factories.js                      |  9 ++--
 server/types.js                               |  2 +-
 shared/i18n/locales/en_US/translation.json    |  3 ++
 12 files changed, 204 insertions(+), 23 deletions(-)
 create mode 100644 app/menus/FileOperationMenu.js
 create mode 100644 server/commands/fileOperationDeleter.js
 create mode 100644 server/commands/fileOperationDeleter.test.js

diff --git a/app/menus/FileOperationMenu.js b/app/menus/FileOperationMenu.js
new file mode 100644
index 00000000..913d59e0
--- /dev/null
+++ b/app/menus/FileOperationMenu.js
@@ -0,0 +1,43 @@
+// @flow
+import * as React from "react";
+import { useTranslation } from "react-i18next";
+import { useMenuState } from "reakit/Menu";
+import ContextMenu from "components/ContextMenu";
+import OverflowMenuButton from "components/ContextMenu/OverflowMenuButton";
+import Template from "components/ContextMenu/Template";
+
+type Props = {|
+  id: string,
+  onDelete: (ev: SyntheticEvent<>) => Promise<void>,
+|};
+
+function FileOperationMenu({ id, onDelete }: Props) {
+  const { t } = useTranslation();
+  const menu = useMenuState({ modal: true });
+
+  return (
+    <>
+      <OverflowMenuButton aria-label={t("Show Menu")} {...menu} />
+      <ContextMenu {...menu} aria-label={t("File Operation options")}>
+        <Template
+          {...menu}
+          items={[
+            {
+              title: t("Download"),
+              href: "/api/fileOperations.redirect?id=" + id,
+            },
+            {
+              type: "separator",
+            },
+            {
+              title: t("Delete"),
+              onClick: onDelete,
+            },
+          ]}
+        />
+      </ContextMenu>
+    </>
+  );
+}
+
+export default FileOperationMenu;
diff --git a/app/scenes/Settings/ImportExport.js b/app/scenes/Settings/ImportExport.js
index 1f29e6ab..e4122dd9 100644
--- a/app/scenes/Settings/ImportExport.js
+++ b/app/scenes/Settings/ImportExport.js
@@ -7,6 +7,7 @@ import { useTranslation, Trans } from "react-i18next";
 import { VisuallyHidden } from "reakit/VisuallyHidden";
 import styled from "styled-components";
 import { parseOutlineExport } from "shared/utils/zip";
+import FileOperation from "models/FileOperation";
 import Button from "components/Button";
 import Heading from "components/Heading";
 import HelpText from "components/HelpText";
@@ -102,6 +103,18 @@ function ImportExport() {
     [t, collections, showToast]
   );
 
+  const handleDelete = React.useCallback(
+    async (fileOperation: FileOperation) => {
+      try {
+        await fileOperations.delete(fileOperation);
+        showToast(t("Export deleted"));
+      } catch (err) {
+        showToast(err.message, { type: "error" });
+      }
+    },
+    [fileOperations, showToast, t]
+  );
+
   const hasCollections = importDetails
     ? !!importDetails.filter((detail) => detail.type === "collection").length
     : false;
@@ -216,6 +229,7 @@ function ImportExport() {
           <FileOperationListItem
             key={item.id + item.state}
             fileOperation={item}
+            handleDelete={handleDelete}
           />
         )}
       />
diff --git a/app/scenes/Settings/components/FileOperationListItem.js b/app/scenes/Settings/components/FileOperationListItem.js
index 2b426b5b..0898d7d5 100644
--- a/app/scenes/Settings/components/FileOperationListItem.js
+++ b/app/scenes/Settings/components/FileOperationListItem.js
@@ -2,16 +2,19 @@
 import * as React from "react";
 import { useTranslation } from "react-i18next";
 import FileOperation from "models/FileOperation";
-import Button from "components/Button";
+import { Action } from "components/Actions";
 import ListItem from "components/List/Item";
 import Time from "components/Time";
-
+import useCurrentUser from "hooks/useCurrentUser";
+import FileOperationMenu from "menus/FileOperationMenu";
 type Props = {|
   fileOperation: FileOperation,
+  handleDelete: (FileOperation) => Promise<void>,
 |};
 
-const FileOperationListItem = ({ fileOperation }: Props) => {
+const FileOperationListItem = ({ fileOperation, handleDelete }: Props) => {
   const { t } = useTranslation();
+  const user = useCurrentUser();
 
   const stateMapping = {
     creating: t("Processing"),
@@ -34,7 +37,7 @@ const FileOperationListItem = ({ fileOperation }: Props) => {
           )}
           {t(`{{userName}} requested`, {
             userName:
-              fileOperation.id === fileOperation.user.id
+              user.id === fileOperation.user.id
                 ? t("You")
                 : fileOperation.user.name,
           })}
@@ -45,13 +48,15 @@ const FileOperationListItem = ({ fileOperation }: Props) => {
       }
       actions={
         fileOperation.state === "complete" ? (
-          <Button
-            as="a"
-            href={`/api/fileOperations.redirect?id=${fileOperation.id}`}
-            neutral
-          >
-            {t("Download")}
-          </Button>
+          <Action>
+            <FileOperationMenu
+              id={fileOperation.id}
+              onDelete={async (ev) => {
+                ev.preventDefault();
+                await handleDelete(fileOperation);
+              }}
+            />
+          </Action>
         ) : undefined
       }
     />
diff --git a/app/stores/FileOperationsStore.js b/app/stores/FileOperationsStore.js
index 0a3ad9d2..cea615d0 100644
--- a/app/stores/FileOperationsStore.js
+++ b/app/stores/FileOperationsStore.js
@@ -6,7 +6,7 @@ import BaseStore from "./BaseStore";
 import RootStore from "./RootStore";
 
 export default class FileOperationsStore extends BaseStore<FileOperation> {
-  actions = ["list", "info"];
+  actions = ["list", "info", "delete"];
 
   constructor(rootStore: RootStore) {
     super(rootStore, FileOperation);
diff --git a/server/commands/fileOperationDeleter.js b/server/commands/fileOperationDeleter.js
new file mode 100644
index 00000000..16800319
--- /dev/null
+++ b/server/commands/fileOperationDeleter.js
@@ -0,0 +1,31 @@
+// @flow
+import { FileOperation, Event, User } from "../models";
+import { sequelize } from "../sequelize";
+
+export default async function fileOperationDeleter(
+  fileOp: FileOperation,
+  user: User,
+  ip: string
+) {
+  let transaction = await sequelize.transaction();
+
+  try {
+    await fileOp.destroy({ transaction });
+
+    await Event.create(
+      {
+        name: "fileOperations.delete",
+        teamId: user.teamId,
+        actorId: user.id,
+        data: fileOp.dataValues,
+        ip,
+      },
+      { transaction }
+    );
+
+    await transaction.commit();
+  } catch (error) {
+    await transaction.rollback();
+    throw error;
+  }
+}
diff --git a/server/commands/fileOperationDeleter.test.js b/server/commands/fileOperationDeleter.test.js
new file mode 100644
index 00000000..dd70ee45
--- /dev/null
+++ b/server/commands/fileOperationDeleter.test.js
@@ -0,0 +1,31 @@
+// @flow
+import { FileOperation } from "../models";
+import { buildAdmin, buildFileOperation } from "../test/factories";
+import { flushdb } from "../test/support";
+import fileOperationDeleter from "./fileOperationDeleter";
+
+jest.mock("aws-sdk", () => {
+  const mS3 = { deleteObject: jest.fn().mockReturnThis(), promise: jest.fn() };
+  return {
+    S3: jest.fn(() => mS3),
+    Endpoint: jest.fn(),
+  };
+});
+
+beforeEach(() => flushdb());
+
+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()).toEqual(0);
+  });
+});
diff --git a/server/models/FileOperation.js b/server/models/FileOperation.js
index f78ed8c4..0f9aefa8 100644
--- a/server/models/FileOperation.js
+++ b/server/models/FileOperation.js
@@ -34,10 +34,14 @@ const FileOperation = sequelize.define("file_operations", {
   },
 });
 
+FileOperation.beforeDestroy(async (model) => {
+  await deleteFromS3(model.key);
+});
+
 FileOperation.prototype.expire = async function () {
   this.state = "expired";
   await deleteFromS3(this.key);
-  this.save();
+  await this.save();
 };
 
 FileOperation.associate = (models) => {
diff --git a/server/routes/api/fileOperations.js b/server/routes/api/fileOperations.js
index acc12f2d..02956ec1 100644
--- a/server/routes/api/fileOperations.js
+++ b/server/routes/api/fileOperations.js
@@ -1,5 +1,6 @@
 // @flow
 import Router from "koa-router";
+import fileOperationDeleter from "../../commands/fileOperationDeleter";
 import { NotFoundError, ValidationError } from "../../errors";
 import auth from "../../middlewares/authentication";
 import { FileOperation, Team } from "../../models";
@@ -88,7 +89,7 @@ router.post("fileOperations.redirect", auth(), async (ctx) => {
   authorize(user, fileOp.type, team);
 
   if (fileOp.state !== "complete") {
-    throw new ValidationError("file operation is not complete yet");
+    throw new ValidationError(`${fileOp.type} is not complete yet`);
   }
 
   const accessUrl = await getSignedUrl(fileOp.key);
@@ -96,4 +97,24 @@ router.post("fileOperations.redirect", auth(), async (ctx) => {
   ctx.redirect(accessUrl);
 });
 
+router.post("fileOperations.delete", auth(), async (ctx) => {
+  const { id } = ctx.body;
+  ctx.assertUuid(id, "id is required");
+
+  const user = ctx.state.user;
+  const team = await Team.findByPk(user.teamId);
+  const fileOp = await FileOperation.findByPk(id);
+
+  if (!fileOp) {
+    throw new NotFoundError();
+  }
+
+  authorize(user, fileOp.type, team);
+
+  await fileOperationDeleter(fileOp, user, ctx.request.ip);
+
+  ctx.body = {
+    success: true,
+  };
+});
 export default router;
diff --git a/server/routes/api/fileOperations.test.js b/server/routes/api/fileOperations.test.js
index 2bc1a840..8afad576 100644
--- a/server/routes/api/fileOperations.test.js
+++ b/server/routes/api/fileOperations.test.js
@@ -1,7 +1,7 @@
 /* eslint-disable flowtype/require-valid-file-annotation */
 import TestServer from "fetch-test-server";
 
-import { Collection, User } from "../../models";
+import { Collection, User, Event, FileOperation } from "../../models";
 import webService from "../../services/web";
 import {
   buildAdmin,
@@ -15,6 +15,14 @@ import { flushdb } from "../../test/support";
 const app = webService();
 const server = new TestServer(app.callback());
 
+jest.mock("aws-sdk", () => {
+  const mS3 = { deleteObject: jest.fn().mockReturnThis(), promise: jest.fn() };
+  return {
+    S3: jest.fn(() => mS3),
+    Endpoint: jest.fn(),
+  };
+});
+
 beforeEach(() => flushdb());
 afterAll(() => server.close());
 
@@ -234,7 +242,7 @@ describe("#fileOperations.redirect", () => {
 
     const body = await res.json();
     expect(res.status).toEqual(400);
-    expect(body.message).toEqual("file operation is not complete yet");
+    expect(body.message).toEqual("export is not complete yet");
   });
 });
 
@@ -281,3 +289,27 @@ describe("#fileOperations.info", () => {
     expect(res.status).toBe(403);
   });
 });
+
+describe("#fileOperations.delete", () => {
+  it("should delete file operation", async () => {
+    const team = await buildTeam();
+    const admin = await buildAdmin({ teamId: team.id });
+    const exportData = await buildFileOperation({
+      type: "export",
+      teamId: team.id,
+      userId: admin.id,
+      state: "complete",
+    });
+
+    const deleteResponse = await server.post("/api/fileOperations.delete", {
+      body: {
+        token: admin.getJwtToken(),
+        id: exportData.id,
+      },
+    });
+
+    expect(deleteResponse.status).toBe(200);
+    expect(await Event.count()).toBe(1);
+    expect(await FileOperation.count()).toBe(0);
+  });
+});
diff --git a/server/test/factories.js b/server/test/factories.js
index 7fdf2ce4..b7dd824e 100644
--- a/server/test/factories.js
+++ b/server/test/factories.js
@@ -268,15 +268,12 @@ export async function buildFileOperation(overrides: Object = {}) {
     overrides.userId = user.id;
   }
 
-  if (!overrides.collectionId) {
-    const collection = await buildCollection(overrides);
-    overrides.collectionId = collection.id;
-  }
-
   return FileOperation.create({
     state: "creating",
     size: 0,
-    key: "key/to/aws/file.zip",
+    key: "uploads/key/to/file.zip",
+    collectionId: null,
+    type: "export",
     url: "https://www.urltos3file.com/file.zip",
     ...overrides,
   });
diff --git a/server/types.js b/server/types.js
index b3515751..8f1cb640 100644
--- a/server/types.js
+++ b/server/types.js
@@ -132,7 +132,7 @@ export type CollectionExportAllEvent = {
 };
 
 export type FileOperationEvent = {
-  name: "fileOperations.update",
+  name: "fileOperations.update" | "fileOperation.delete",
   teamId: string,
   actorId: string,
   data: {
diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json
index 93061ff7..a7738ef4 100644
--- a/shared/i18n/locales/en_US/translation.json
+++ b/shared/i18n/locales/en_US/translation.json
@@ -227,6 +227,8 @@
   "Print": "Print",
   "Move {{ documentName }}": "Move {{ documentName }}",
   "Permanently delete {{ documentName }}": "Permanently delete {{ documentName }}",
+  "Show Menu": "Show Menu",
+  "File Operation options": "File Operation options",
   "Edit group": "Edit group",
   "Delete group": "Delete group",
   "Group options": "Group options",
@@ -518,6 +520,7 @@
   "No groups have been created yet": "No groups have been created yet",
   "Import started": "Import started",
   "Export in progress…": "Export in progress…",
+  "Export deleted": "Export deleted",
   "It is possible to import a zip file of folders and Markdown files previously exported from an Outline instance. Support will soon be added for importing from other services.": "It is possible to import a zip file of folders and Markdown files previously exported from an Outline instance. Support will soon be added for importing from other services.",
   "Your file has been uploaded and the import is currently being processed, you can safely leave this page while it completes.": "Your file has been uploaded and the import is currently being processed, you can safely leave this page while it completes.",
   "Sorry, the file <em>{{ fileName }}</em> is missing valid collections or documents.": "Sorry, the file <em>{{ fileName }}</em> is missing valid collections or documents.",