From caee7afde229cfcd20d2c565038a330ec37a3e74 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 28 Dec 2020 18:51:12 -0800 Subject: [PATCH] refactor: documents.batchImport -> collections.import --- app/scenes/Settings/ImportExport.js | 6 +-- app/stores/CollectionsStore.js | 18 +++++++- app/stores/DocumentsStore.js | 15 ------- server/api/collections.js | 41 ++++++++++++++++++- server/api/documents.js | 39 ------------------ ...BatchImporter.js => collectionImporter.js} | 2 +- ...ter.test.js => collectionImporter.test.js} | 10 ++--- server/policies/collection.js | 5 +++ server/policies/document.js | 6 --- shared/i18n/locales/en_US/translation.json | 2 +- 10 files changed, 72 insertions(+), 72 deletions(-) rename server/commands/{documentBatchImporter.js => collectionImporter.js} (98%) rename server/commands/{documentBatchImporter.test.js => collectionImporter.test.js} (89%) diff --git a/app/scenes/Settings/ImportExport.js b/app/scenes/Settings/ImportExport.js index a25bc652..c90c3286 100644 --- a/app/scenes/Settings/ImportExport.js +++ b/app/scenes/Settings/ImportExport.js @@ -19,7 +19,7 @@ function ImportExport() { const { t } = useTranslation(); const user = useCurrentUser(); const fileRef = React.useRef(); - const { ui, collections, documents } = useStores(); + const { ui, collections } = useStores(); const { showToast } = ui; const [isLoading, setLoading] = React.useState(false); const [isImporting, setImporting] = React.useState(false); @@ -34,7 +34,7 @@ function ImportExport() { setImporting(true); try { - const { documentCount, collectionCount } = await documents.batchImport( + const { documentCount, collectionCount } = await collections.import( file ); showToast(t("Import completed")); @@ -50,7 +50,7 @@ function ImportExport() { setImportDetails(undefined); } }, - [t, file, documents, showToast] + [t, file, collections, showToast] ); const handleFilePicked = React.useCallback(async (ev) => { diff --git a/app/stores/CollectionsStore.js b/app/stores/CollectionsStore.js index 1f77a2b2..c034992c 100644 --- a/app/stores/CollectionsStore.js +++ b/app/stores/CollectionsStore.js @@ -1,6 +1,7 @@ // @flow +import invariant from "invariant"; import { concat, filter, last } from "lodash"; -import { computed } from "mobx"; +import { action, computed } from "mobx"; import naturalSort from "shared/utils/naturalSort"; import Collection from "models/Collection"; @@ -88,6 +89,21 @@ export default class CollectionsStore extends BaseStore { }); } + @action + import = async (file: File) => { + const formData = new FormData(); + formData.append("type", "outline"); + formData.append("file", file); + + const res = await client.post("/collections.import", formData); + invariant(res && res.data, "Data should be available"); + + this.addPolicies(res.policies); + res.data.collections.forEach(this.add); + + return res.data; + }; + getPathForDocument(documentId: string): ?DocumentPath { return this.pathsToDocuments.find((path) => path.id === documentId); } diff --git a/app/stores/DocumentsStore.js b/app/stores/DocumentsStore.js index 3c76189e..4d4e77a0 100644 --- a/app/stores/DocumentsStore.js +++ b/app/stores/DocumentsStore.js @@ -497,21 +497,6 @@ export default class DocumentsStore extends BaseStore { return this.add(res.data); }; - @action - batchImport = async (file: File) => { - const formData = new FormData(); - formData.append("type", "outline"); - formData.append("file", file); - - const res = await client.post("/documents.batchImport", formData); - invariant(res && res.data, "Data should be available"); - - this.addPolicies(res.policies); - res.data.collections.forEach(this.rootStore.collections.add); - - return res.data; - }; - @action import = async ( file: File, diff --git a/server/api/collections.js b/server/api/collections.js index 740d2349..ec097bd2 100644 --- a/server/api/collections.js +++ b/server/api/collections.js @@ -1,7 +1,8 @@ // @flow import fs from "fs"; import Router from "koa-router"; -import { ValidationError } from "../errors"; +import collectionImporter from "../commands/collectionImporter"; +import { ValidationError, InvalidRequestError } from "../errors"; import { exportCollections } from "../logistics"; import auth from "../middlewares/authentication"; import { @@ -89,6 +90,44 @@ router.post("collections.info", auth(), async (ctx) => { }; }); +router.post("collections.import", auth(), async (ctx) => { + const { type } = ctx.body; + ctx.assertIn(type, ["outline"], "type must be one of 'outline'"); + + if (!ctx.is("multipart/form-data")) { + throw new InvalidRequestError("Request type must be multipart/form-data"); + } + + const file: any = Object.values(ctx.request.files)[0]; + ctx.assertPresent(file, "file is required"); + + if (file.type !== "application/zip") { + throw new InvalidRequestError("File type must be a zip"); + } + + const user = ctx.state.user; + authorize(user, "import", Collection); + + const { documents, attachments, collections } = await collectionImporter({ + file, + user, + type, + ip: ctx.request.ip, + }); + + ctx.body = { + data: { + attachmentCount: attachments.length, + documentCount: documents.length, + collectionCount: collections.length, + collections: collections.map((collection) => + presentCollection(collection) + ), + }, + policies: presentPolicies(user, collections), + }; +}); + router.post("collections.add_group", auth(), async (ctx) => { const { id, groupId, permission = "read_write" } = ctx.body; ctx.assertUuid(id, "id is required"); diff --git a/server/api/documents.js b/server/api/documents.js index d9e642de..038496d0 100644 --- a/server/api/documents.js +++ b/server/api/documents.js @@ -2,7 +2,6 @@ import Router from "koa-router"; import Sequelize from "sequelize"; import { subtractDate } from "../../shared/utils/date"; -import documentBatchImporter from "../commands/documentBatchImporter"; import documentCreator from "../commands/documentCreator"; import documentImporter from "../commands/documentImporter"; import documentMover from "../commands/documentMover"; @@ -1106,44 +1105,6 @@ router.post("documents.unpublish", auth(), async (ctx) => { }; }); -router.post("documents.batchImport", auth(), async (ctx) => { - const { type } = ctx.body; - ctx.assertIn(type, ["outline"], "type must be one of 'outline'"); - - if (!ctx.is("multipart/form-data")) { - throw new InvalidRequestError("Request type must be multipart/form-data"); - } - - const file: any = Object.values(ctx.request.files)[0]; - ctx.assertPresent(file, "file is required"); - - if (file.type !== "application/zip") { - throw new InvalidRequestError("File type must be a zip"); - } - - const user = ctx.state.user; - authorize(user, "batchImport", Document); - - const { documents, attachments, collections } = await documentBatchImporter({ - file, - user, - type, - ip: ctx.request.ip, - }); - - ctx.body = { - data: { - attachmentCount: attachments.length, - documentCount: documents.length, - collectionCount: collections.length, - collections: collections.map((collection) => - presentCollection(collection) - ), - }, - policies: presentPolicies(user, collections), - }; -}); - router.post("documents.import", auth(), async (ctx) => { const { publish, collectionId, parentDocumentId, index } = ctx.body; diff --git a/server/commands/documentBatchImporter.js b/server/commands/collectionImporter.js similarity index 98% rename from server/commands/documentBatchImporter.js rename to server/commands/collectionImporter.js index a34b1502..a5be2fec 100644 --- a/server/commands/documentBatchImporter.js +++ b/server/commands/collectionImporter.js @@ -16,7 +16,7 @@ import documentImporter from "./documentImporter"; const log = debug("commands"); -export default async function documentBatchImporter({ +export default async function collectionImporter({ file, type, user, diff --git a/server/commands/documentBatchImporter.test.js b/server/commands/collectionImporter.test.js similarity index 89% rename from server/commands/documentBatchImporter.test.js rename to server/commands/collectionImporter.test.js index 2f296076..950d18f3 100644 --- a/server/commands/documentBatchImporter.test.js +++ b/server/commands/collectionImporter.test.js @@ -4,13 +4,13 @@ import File from "formidable/lib/file"; import { Attachment, Document, Collection } from "../models"; import { buildUser } from "../test/factories"; import { flushdb } from "../test/support"; -import documentBatchImporter from "./documentBatchImporter"; +import collectionImporter from "./collectionImporter"; jest.mock("../utils/s3"); beforeEach(() => flushdb()); -describe("documentBatchImporter", () => { +describe("collectionImporter", () => { const ip = "127.0.0.1"; it("should import documents in outline format", async () => { @@ -22,7 +22,7 @@ describe("documentBatchImporter", () => { path: path.resolve(__dirname, "..", "test", "fixtures", name), }); - const response = await documentBatchImporter({ + const response = await collectionImporter({ type: "outline", user, file, @@ -49,7 +49,7 @@ describe("documentBatchImporter", () => { let error; try { - await documentBatchImporter({ + await collectionImporter({ type: "outline", user, file, @@ -73,7 +73,7 @@ describe("documentBatchImporter", () => { let error; try { - await documentBatchImporter({ + await collectionImporter({ type: "outline", user, file, diff --git a/server/policies/collection.js b/server/policies/collection.js index e0fc7c1d..7aa207a5 100644 --- a/server/policies/collection.js +++ b/server/policies/collection.js @@ -9,6 +9,11 @@ const { allow } = policy; allow(User, "create", Collection); +allow(User, "import", Collection, (actor) => { + if (actor.isAdmin) return true; + throw new AdminRequiredError(); +}); + allow(User, ["read", "export"], Collection, (user, collection) => { if (!collection || user.teamId !== collection.teamId) return false; diff --git a/server/policies/document.js b/server/policies/document.js index 203d8055..310734a8 100644 --- a/server/policies/document.js +++ b/server/policies/document.js @@ -1,6 +1,5 @@ // @flow import invariant from "invariant"; -import { AdminRequiredError } from "../errors"; import { Document, Revision, User } from "../models"; import policy from "./policy"; @@ -8,11 +7,6 @@ const { allow, cannot } = policy; allow(User, "create", Document); -allow(User, "batchImport", Document, (actor) => { - if (actor.isAdmin) return true; - throw new AdminRequiredError(); -}); - allow(User, ["read", "download"], Document, (user, document) => { // existence of collection option is not required here to account for share tokens if (document.collection && cannot(user, "read", document.collection)) { diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json index 80cfee22..4a5b034e 100644 --- a/shared/i18n/locales/en_US/translation.json +++ b/shared/i18n/locales/en_US/translation.json @@ -311,4 +311,4 @@ "Suspended": "Suspended", "Edit Profile": "Edit Profile", "{{ userName }} hasn’t updated any documents yet.": "{{ userName }} hasn’t updated any documents yet." -} \ No newline at end of file +}