remove path for files + add base for delete folders

packages/core/upload/server/content-types/file/schema.js

@@ -95,10 +95,5 @@ module.exports = {
       target: 'plugin::upload.folder',
       inversedBy: 'files',
     },
-    path: {
-      type: 'string',
-      min: 1,
-      required: true,
-    },
   },
 };

packages/core/upload/server/controllers/admin-folder.js

@@ -1,9 +1,8 @@
 'use strict';
 
 const { setCreatorFields, pipeAsync } = require('@strapi/utils');
-const { ApplicationError } = require('@strapi/utils').errors;
 const { getService } = require('../utils');
-const { validateCreateFolder } = require('./validation/folder');
+const { validateCreateFolder, validateDeleteManyFolders } = require('./validation/folder');
 
 const folderModel = 'plugin::upload.folder';
 
@@ -40,16 +39,6 @@ module.exports = {
 
     await validateCreateFolder(body);
 
-    const existingFolders = await strapi.entityService.findMany(folderModel, {
-      filters: {
-        parent: body.parent || null,
-        name: body.name,
-      },
-    });
-    if (existingFolders.length > 0) {
-      throw new ApplicationError('name already taken');
-    }
-
     const { setPathAndUID } = getService('folder');
 
     // TODO: wrap with a transaction
@@ -70,4 +59,18 @@ module.exports = {
       data: await permissionsManager.sanitizeOutput(folder),
     };
   },
+  // deleteMany WIP
+  async deleteMany(ctx) {
+    const { body } = ctx.request;
+
+    await validateDeleteManyFolders(body);
+
+    const { deleteByIds } = getService('folder');
+
+    const deletedFolders = await deleteByIds(body.ids);
+
+    ctx.body = {
+      data: deletedFolders,
+    };
+  },
 };

packages/core/upload/server/controllers/validation/folder.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const { yup, validateYupSchema } = require('@strapi/utils');
+const { getService } = require('../../utils');
 
 const NO_SLASH_REGEX = /^[^/]+$/;
 const NO_SPACES_AROUND = /^(?! ).+(?<! )$/;
@@ -16,9 +17,27 @@ const validateCreateFolderSchema = yup
       .required(),
     parent: yup.strapiID().nullable(),
   })
+  .test('is-folder-unique', 'name already taken', async folder => {
+    const { exists } = getService('folder');
+    const doesExist = await exists({ parent: folder.parent || null, name: folder.name });
+    return !doesExist;
+  })
+  .noUnknown()
+  .required();
+
+const validateDeleteManyFoldersSchema = yup
+  .object()
+  .shape({
+    ids: yup
+      .array()
+      .min(1)
+      .of(yup.strapiID().required())
+      .required(),
+  })
   .noUnknown()
   .required();
 
 module.exports = {
   validateCreateFolder: validateYupSchema(validateCreateFolderSchema),
+  validateDeleteManyFolders: validateYupSchema(validateDeleteManyFoldersSchema),
 };

packages/core/upload/server/controllers/validation/upload.js

@@ -1,12 +1,23 @@
 'use strict';
 
 const { yup, validateYupSchema } = require('@strapi/utils');
+const { isNil } = require('lodash/fp');
+const { getService } = require('../../utils');
 
 const fileInfoSchema = yup.object({
   name: yup.string().nullable(),
   alternativeText: yup.string().nullable(),
   caption: yup.string().nullable(),
-  folder: yup.strapiID().nullable(),
+  folder: yup
+    .strapiID()
+    .nullable()
+    .test('folder-exists', "the folder doesn't exist", async folderId => {
+      if (isNil(folderId)) {
+        return true;
+      }
+
+      return getService('folder').exists({ id: folderId });
+    }),
 });
 
 const uploadSchema = yup.object({

packages/core/upload/server/routes/admin.js

@@ -123,5 +123,21 @@ module.exports = {
         ],
       },
     },
+    {
+      method: 'POST',
+      path: '/folders/batch-delete',
+      handler: 'admin-folder.deleteMany',
+      config: {
+        policies: [
+          'admin::isAuthenticatedAdmin',
+          {
+            name: 'admin::hasPermissions',
+            config: {
+              actions: ['plugin::upload.read'],
+            },
+          },
+        ],
+      },
+    },
   ],
 };

packages/core/upload/server/services/__tests__/file.test.js

@@ -1,25 +0,0 @@
-'use strict';
-
-const { getPath } = require('../file');
-
-describe('file', () => {
-  describe('getPath', () => {
-    beforeAll(() => {
-      global.strapi = {
-        entityService: {
-          findOne: jest.fn(() => ({ path: '/parent-path' })),
-        },
-      };
-    });
-
-    test.each([
-      [[1, 'myFile.txt'], '/parent-path/myFile.txt'],
-      [[undefined, 'myFile.txt'], '/myFile.txt'],
-      [[null, 'myFile.txt'], '/myFile.txt'],
-    ])('inputs %s should give %s', async (args, expectedResult) => {
-      const result = await getPath(...args);
-
-      expect(result).toBe(expectedResult);
-    });
-  });
-});

packages/core/upload/server/services/file.js

@@ -1,32 +0,0 @@
-'use strict';
-
-const { trimChars, trimCharsEnd, trimCharsStart } = require('lodash/fp');
-
-// TODO: to use once https://github.com/strapi/strapi/pull/12534 is merged
-// const { joinBy } = require('@strapi/utils');
-
-const folderModel = 'plugin::upload.folder';
-
-const joinBy = (joint, ...args) => {
-  const trim = trimChars(joint);
-  const trimEnd = trimCharsEnd(joint);
-  const trimStart = trimCharsStart(joint);
-
-  return args.reduce((url, path, index) => {
-    if (args.length === 1) return path;
-    if (index === 0) return trimEnd(path);
-    if (index === args.length - 1) return url + joint + trimStart(path);
-    return url + joint + trim(path);
-  }, '');
-};
-
-const getPath = async (folderId, fileName) => {
-  if (!folderId) return joinBy('/', '/', fileName);
-
-  const parentFolder = await strapi.entityService.findOne(folderModel, folderId);
-  return joinBy('/', parentFolder.path, fileName);
-};
-
-module.exports = {
-  getPath,
-};

packages/core/upload/server/services/folder.js

@@ -36,6 +36,29 @@ const setPathAndUID = async folder => {
   });
 };
 
+const deleteByIds = async ids => {
+  const deletedFolders = [];
+  for (const id of ids) {
+    const deletedFolder = await strapi.entityService.delete(folderModel, id);
+
+    deletedFolders.push(deletedFolder);
+  }
+
+  return deletedFolders;
+};
+
+/**
+ * Check if a folder exists in database
+ * @param params query params to find the folder
+ * @returns {Promise<boolean>}
+ */
+const exists = async (params = {}) => {
+  const count = await strapi.query(folderModel).count({ where: params });
+  return count > 0;
+};
+
 module.exports = {
+  exists,
+  deleteByIds,
   setPathAndUID,
 };

packages/core/upload/server/services/index.js

@@ -4,12 +4,10 @@ const provider = require('./provider');
 const upload = require('./upload');
 const imageManipulation = require('./image-manipulation');
 const folder = require('./folder');
-const file = require('./file');
 
 module.exports = {
   provider,
   upload,
   folder,
-  file,
   'image-manipulation': imageManipulation,
 };

packages/core/upload/server/services/upload.js

@@ -64,8 +64,6 @@ module.exports = ({ strapi }) => ({
   },
 
   async formatFileInfo({ filename, type, size }, fileInfo = {}, metas = {}) {
-    const fileService = getService('file');
-
     const ext = path.extname(filename);
     const basename = path.basename(fileInfo.name || filename, ext);
     const usedName = fileInfo.name || filename;
@@ -75,7 +73,6 @@ module.exports = ({ strapi }) => ({
       alternativeText: fileInfo.alternativeText,
       caption: fileInfo.caption,
       folder: fileInfo.folder,
-      path: await fileService.getPath(fileInfo.folder, usedName),
       hash: generateFileName(basename),
       ext,
       mime: type,
@@ -208,15 +205,12 @@ module.exports = ({ strapi }) => ({
       throw new NotFoundError();
     }
 
-    const fileService = getService('file');
-
     const newName = _.isNil(name) ? dbFile.name : name;
     const newInfos = {
       name: newName,
       alternativeText: _.isNil(alternativeText) ? dbFile.alternativeText : alternativeText,
       caption: _.isNil(caption) ? dbFile.caption : caption,
       folder: _.isUndefined(folder) ? dbFile.folder : folder,
-      path: _.isUndefined(folder) ? dbFile.path : await fileService.getPath(folder, newName),
     };
 
     return this.update(id, newInfos, { user });

packages/core/upload/tests/admin/file.test.e2e.js

@@ -26,13 +26,21 @@ describe('File', () => {
       const folderRes = await rq({
         method: 'POST',
         url: '/upload/folders',
-        body: { name: `folder ${i}` },
+        body: { name: `my folder ${i}` },
       });
       data.folders.push(folderRes.body.data);
     }
   });
 
   afterAll(async () => {
+    await rq({
+      method: 'POST',
+      url: '/upload/folders/batch-delete',
+      body: {
+        ids: data.folders.map(f => f.id),
+      },
+    });
+
     await strapi.destroy();
     await builder.cleanup();
   });
@@ -67,7 +75,6 @@ describe('File', () => {
         height: expect.any(Number),
         url: expect.any(String),
         provider: 'local',
-        path: '/rec.jpg',
         folder: null,
       });
 
@@ -106,12 +113,27 @@ describe('File', () => {
         height: expect.any(Number),
         url: expect.any(String),
         provider: 'local',
-        path: '/folder 1/rec.jpg',
+        folder: { id: data.folders[0].id },
-        folder: { id: 1 },
       });
 
       data.files.push(file);
     });
+
+    test("Cannot create a file inside a folder that doesn't exist", async () => {
+      const res = await rq({
+        method: 'POST',
+        url: '/upload',
+        formData: {
+          files: fs.createReadStream(path.join(__dirname, '../utils/rec.jpg')),
+          fileInfo: JSON.stringify({
+            folder: '1234', // id that doesn't exist
+          }),
+        },
+      });
+
+      expect(res.status).toBe(400);
+      expect(res.body.error.message).toBe("the folder doesn't exist");
+    });
   });
 
   describe('Update info', () => {
@@ -145,7 +167,6 @@ describe('File', () => {
           height: expect.any(Number),
           url: expect.any(String),
           provider: 'local',
-          path: '/folder 2/rec.pdf',
           folder: { id: data.folders[1].id },
         });
         data.files[1] = file;
@@ -179,7 +200,6 @@ describe('File', () => {
           height: expect.any(Number),
           url: expect.any(String),
           provider: 'local',
-          path: '/folder 1/rec.pdf',
           folder: { id: data.folders[0].id },
         });
         data.files[1] = file;
@@ -215,7 +235,6 @@ describe('File', () => {
           height: expect.any(Number),
           url: expect.any(String),
           provider: 'local',
-          path: '/folder 1/rec.pdf',
           folder: { id: data.folders[0].id },
         });
         data.files[0] = file;
@@ -249,7 +268,6 @@ describe('File', () => {
           height: expect.any(Number),
           url: expect.any(String),
           provider: 'local',
-          path: '/folder 2/rec.pdf',
           folder: { id: data.folders[1].id },
         });
         data.files[1] = file;
@@ -286,7 +304,6 @@ describe('File', () => {
           height: expect.any(Number),
           url: expect.any(String),
           provider: 'local',
-          path: '/rec.jpg',
           folder: null,
         });
         data.files[0] = file;
@@ -320,11 +337,44 @@ describe('File', () => {
           height: expect.any(Number),
           url: expect.any(String),
           provider: 'local',
-          path: '/rec.pdf',
           folder: null,
         });
         data.files[1] = file;
       });
     });
+
+    describe("Cannot create a file inside a folder that doesn't exist", () => {
+      test('when replacing the file', async () => {
+        const res = await rq({
+          method: 'POST',
+          url: `/upload?id=${data.files[1].id}`,
+          formData: {
+            files: fs.createReadStream(path.join(__dirname, '../utils/rec.jpg')),
+            fileInfo: JSON.stringify({
+              folder: '1234', // id that doesn't exist
+            }),
+          },
+        });
+
+        console.log('res.body', res.body);
+        expect(res.status).toBe(400);
+        expect(res.body.error.message).toBe("the folder doesn't exist");
+      });
+
+      test('whithout replacing the file', async () => {
+        const res = await rq({
+          method: 'POST',
+          url: `/upload?id=${data.files[1].id}`,
+          formData: {
+            fileInfo: JSON.stringify({
+              folder: '1234', // id that doesn't exist
+            }),
+          },
+        });
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.message).toBe("the folder doesn't exist");
+      });
+    });
   });
 });

packages/core/upload/tests/admin/folder.test.e2e.js

@@ -38,6 +38,7 @@ describe('Folder', () => {
         },
       });
 
+      expect(res.status).toBe(200);
       expect(res.body.data).toMatchObject({
         id: expect.anything(),
         name: 'folder 1',
@@ -51,51 +52,6 @@ describe('Folder', () => {
       data.folders.push(omit('parent', res.body.data));
     });
 
-    test('Cannot create a folder with duplicated name at root level', async () => {
-      const res = await rq({
-        method: 'POST',
-        url: '/upload/folders?populate=parent',
-        body: {
-          name: 'folder 1',
-          parent: null,
-        },
-      });
-
-      expect(res.status).toBe(400);
-      expect(res.body.error.message).toBe('name already taken');
-    });
-
-    test('Cannot create a folder with name containing a slash', async () => {
-      const res = await rq({
-        method: 'POST',
-        url: '/upload/folders?populate=parent',
-        body: {
-          name: 'folder 1/2',
-          parent: null,
-        },
-      });
-
-      expect(res.status).toBe(400);
-      expect(res.body.error.message).toBe('name cannot contain slashes');
-    });
-
-    test.each([[' abc'], [' abc '], ['abc '], ['   abc    '], ['   abc    ']])(
-      'Cannot create a folder with name starting or ending with a whitespace',
-      async name => {
-        const res = await rq({
-          method: 'POST',
-          url: '/upload/folders?populate=parent',
-          body: {
-            name,
-            parent: null,
-          },
-        });
-
-        expect(res.status).toBe(400);
-        expect(res.body.error.message).toBe('name cannot start or end with a whitespace');
-      }
-    );
-
     test('Can create a folder inside another folder', async () => {
       const res = await rq({
         method: 'POST',
@@ -118,6 +74,65 @@ describe('Folder', () => {
 
       data.folders.push(omit('parent', res.body.data));
     });
+
+    test('Cannot create a folder with duplicated name at root level', async () => {
+      const res = await rq({
+        method: 'POST',
+        url: '/upload/folders?populate=parent',
+        body: {
+          name: 'folder 1',
+          parent: null,
+        },
+      });
+
+      expect(res.status).toBe(400);
+      expect(res.body.error.message).toBe('name already taken');
+    });
+
+    test('Cannot create a folder with duplicated name inside a folder', async () => {
+      const res = await rq({
+        method: 'POST',
+        url: '/upload/folders?populate=parent',
+        body: {
+          name: 'folder-2',
+          parent: data.folders[0],
+        },
+      });
+
+      expect(res.status).toBe(400);
+      expect(res.body.error.message).toBe('name already taken');
+    });
+
+    test('Cannot create a folder with name containing a slash', async () => {
+      const res = await rq({
+        method: 'POST',
+        url: '/upload/folders?populate=parent',
+        body: {
+          name: 'folder 1/2',
+          parent: null,
+        },
+      });
+
+      expect(res.status).toBe(400);
+      expect(res.body.error.message).toBe('name cannot contain slashes');
+    });
+
+    test.each([[' abc'], [' abc '], ['abc '], ['   abc    '], ['   abc    ']])(
+      'Cannot create a folder with name starting or ending with a whitespace (%p)',
+      async name => {
+        const res = await rq({
+          method: 'POST',
+          url: '/upload/folders?populate=parent',
+          body: {
+            name,
+            parent: null,
+          },
+        });
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.message).toBe('name cannot start or end with a whitespace');
+      }
+    );
   });
 
   describe('read', () => {
@@ -133,7 +148,6 @@ describe('Folder', () => {
         pageSize: 10,
         total: 2,
       });
-      expect(res.body.results).toHaveLength(2);
       expect(res.body.results).toEqual(
         expect.arrayContaining([
           {
@@ -193,4 +207,37 @@ describe('Folder', () => {
       );
     });
   });
+
+  describe('delete', () => {
+    test('Can delete folders', async () => {
+      const res = await rq({
+        method: 'POST',
+        url: '/upload/folders/batch-delete',
+        body: {
+          ids: data.folders.map(f => f.id),
+        },
+      });
+
+      expect(res.body.data).toEqual(
+        expect.arrayContaining([
+          {
+            createdAt: expect.anything(),
+            id: expect.anything(),
+            name: 'folder 1',
+            path: '/folder 1',
+            uid: expect.anything(),
+            updatedAt: expect.anything(),
+          },
+          {
+            createdAt: expect.anything(),
+            id: expect.anything(),
+            name: 'folder-2',
+            path: '/folder 1/folder-2',
+            uid: expect.anything(),
+            updatedAt: expect.anything(),
+          },
+        ])
+      );
+    });
+  });
 });