apply feedback

This commit is contained in:
Pierre Noël 2022-04-12 16:32:05 +02:00
parent cfa968c537
commit 041f2f6eb3
17 changed files with 74 additions and 89 deletions

View File

@ -92,17 +92,20 @@ describe('Test type date', () => {
});
test('Updating entry sets the right value and format JSON', async () => {
const now = new Date(2018, 7, 5);
const res = await rq.post('/content-manager/collection-types/api::withdate.withdate', {
body: {
field: '2018-06-05',
field: now,
},
});
const newDate = new Date(2017, 10, 23);
const updateRes = await rq.put(
`/content-manager/collection-types/api::withdate.withdate/${res.body.id}`,
{
body: {
field: '2017-11-23',
field: newDate,
},
}
);

View File

@ -95,7 +95,7 @@ module.exports = {
target: 'plugin::upload.folder',
inversedBy: 'files',
},
location: {
folderPath: {
type: 'string',
min: 1,
required: true,

View File

@ -45,7 +45,7 @@ module.exports = {
target: 'plugin::upload.file',
mappedBy: 'folder',
},
location: {
path: {
type: 'string',
min: 1,
required: true,

View File

@ -58,8 +58,11 @@ module.exports = {
id
);
await getService('upload').remove(file);
const [body] = await Promise.all([
pm.sanitizeOutput(file, { action: ACTIONS.read }),
getService('upload').remove(file),
]);
ctx.body = await pm.sanitizeOutput(file, { action: ACTIONS.read });
ctx.body = body;
},
};

View File

@ -39,10 +39,10 @@ module.exports = {
await validateCreateFolder(body);
const { setLocationAndUID } = getService('folder');
const { setPathAndUID } = getService('folder');
// TODO: wrap with a transaction
const enrichFolder = pipeAsync(setLocationAndUID, setCreatorFields({ user }));
const enrichFolder = pipeAsync(setPathAndUID, setCreatorFields({ user }));
const enrichedFolder = await enrichFolder(body);
const folder = await strapi.entityService.create(folderModel, {

View File

@ -75,11 +75,11 @@ module.exports = {
request: { files: { files } = {} },
} = ctx;
if (id && (_.isEmpty(files) || files.size === 0)) {
return this.updateFileInfo(ctx);
}
if (_.isEmpty(files) || files.size === 0) {
if (id) {
return this.updateFileInfo(ctx);
}
throw new ApplicationError('Files are empty');
}

View File

@ -9,9 +9,9 @@ const validateUploadBody = require('./validation/content-api/upload');
const { sanitize } = utils;
const { ValidationError } = utils.errors;
const removeLocation = data => {
if (isArray(data)) return data.map(omit('location'));
if (isPlainObject(data)) return omit('location', data);
const removeFolderPath = data => {
if (isArray(data)) return data.map(omit('folderPath'));
if (isPlainObject(data)) return omit('folderPath', data);
return data;
};
@ -19,7 +19,7 @@ const sanitizeOutput = (data, ctx) => {
const schema = strapi.getModel('plugin::upload.file');
const { auth } = ctx.state;
return sanitize.contentAPI.output(removeLocation(data), schema, { auth });
return sanitize.contentAPI.output(removeFolderPath(data), schema, { auth });
};
module.exports = {

View File

@ -1,12 +1,10 @@
'use strict';
const _ = require('lodash');
const { contentTypes: contentTypesUtils } = require('@strapi/utils');
const { CREATED_BY_ATTRIBUTE } = require('@strapi/utils').contentTypes.constants;
const { NotFoundError, ForbiddenError } = require('@strapi/utils').errors;
const { getService } = require('../../utils');
const { CREATED_BY_ATTRIBUTE } = contentTypesUtils.constants;
const findEntityAndCheckPermissions = async (ability, action, model, id) => {
const file = await getService('upload').findOne(id, [CREATED_BY_ATTRIBUTE, 'folder']);

View File

@ -11,7 +11,7 @@ const fileInfoSchema = yup.object({
folder: yup
.strapiID()
.nullable()
.test('folder-exists', "the folder doesn't exist", async folderId => {
.test('folder-exists', 'the folder does not exist', async folderId => {
if (isNil(folderId)) {
return true;
}

View File

@ -1,25 +1,25 @@
'use strict';
const { getLocation } = require('../file');
const { getFolderPath } = require('../file');
const folderLocation = '/9bc2352b-e29b-4ba3-810f-7b91033222de';
const folderPath = '/9bc2352b-e29b-4ba3-810f-7b91033222de';
describe('file', () => {
describe('getLocation', () => {
describe('getFolderPath', () => {
beforeAll(() => {
global.strapi = {
entityService: {
findOne: jest.fn(() => ({ location: folderLocation })),
findOne: jest.fn(() => ({ path: folderPath })),
},
};
});
test.each([
[[1, 'myFile.txt'], folderLocation],
[[1, 'myFile.txt'], folderPath],
[[undefined, 'myFile.txt'], '/'],
[[null, 'myFile.txt'], '/'],
])('inputs %s should give %s', async (args, expectedResult) => {
const result = await getLocation(...args);
const result = await getFolderPath(...args);
expect(result).toBe(expectedResult);
});

View File

@ -1,38 +1,38 @@
'use strict';
const { setLocationAndUID } = require('../folder');
const { setPathAndUID } = require('../folder');
const folderUID = '9bc2352b-e29b-4ba3-810f-7b91033222de';
const uuidRegex = /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
const rootLocationRegex = /^\/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
const folderLocationRegex = new RegExp(
const rootPathRegex = /^\/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
const folderPathRegex = new RegExp(
'^/' + folderUID + '/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$',
'i'
);
describe('folder', () => {
describe('setLocationAndUID', () => {
describe('setPathAndUID', () => {
beforeAll(() => {
global.strapi = {
entityService: {
findOne: jest.fn(() => ({ location: `/${folderUID}` })),
findOne: jest.fn(() => ({ path: `/${folderUID}` })),
},
};
});
test.each([
[{ parent: 1 }, folderLocationRegex],
[{}, rootLocationRegex],
[{ parent: null }, rootLocationRegex],
])('inputs %s', async (folder, expectedLocation) => {
[{ parent: 1 }, folderPathRegex],
[{}, rootPathRegex],
[{ parent: null }, rootPathRegex],
])('inputs %s', async (folder, expectedPath) => {
const clonedFolder = { ...folder };
const result = await setLocationAndUID(clonedFolder);
const result = await setPathAndUID(clonedFolder);
expect(result).toBe(clonedFolder);
expect(result).toMatchObject({
...folder,
uid: expect.stringMatching(uuidRegex),
location: expect.stringMatching(expectedLocation),
path: expect.stringMatching(expectedPath),
});
});
});

View File

@ -9,7 +9,7 @@ describe('Upload service', () => {
upload: {
services: {
file: {
getLocation: () => '/a-location',
getFolderPath: () => '/a-folder-path',
},
},
},

View File

@ -2,14 +2,14 @@
const folderModel = 'plugin::upload.folder';
const getLocation = async folderId => {
const getFolderPath = async folderId => {
if (!folderId) return '/';
const parentFolder = await strapi.entityService.findOne(folderModel, folderId);
return parentFolder.location;
return parentFolder.path;
};
module.exports = {
getLocation,
getFolderPath,
};

View File

@ -1,39 +1,23 @@
'use strict';
const uuid = require('uuid/v4');
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 { 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 generateUID = () => uuid();
const setLocationAndUID = async folder => {
const setPathAndUID = async folder => {
const uid = generateUID();
let parentLocation = '/';
let parentPath = '/';
if (folder.parent) {
const parentFolder = await strapi.entityService.findOne(folderModel, folder.parent);
parentLocation = parentFolder.location;
parentPath = parentFolder.path;
}
return Object.assign(folder, {
uid,
location: joinBy('/', parentLocation, uid),
path: joinBy('/', parentPath, uid),
});
};
@ -61,5 +45,5 @@ const exists = async (params = {}) => {
module.exports = {
exists,
deleteByIds,
setLocationAndUID,
setPathAndUID,
};

View File

@ -75,7 +75,7 @@ module.exports = ({ strapi }) => ({
alternativeText: fileInfo.alternativeText,
caption: fileInfo.caption,
folder: fileInfo.folder,
location: await fileService.getLocation(fileInfo.folder),
folderPath: await fileService.getFolderPath(fileInfo.folder),
hash: generateFileName(basename),
ext,
mime: type,
@ -216,7 +216,7 @@ module.exports = ({ strapi }) => ({
alternativeText: _.isNil(alternativeText) ? dbFile.alternativeText : alternativeText,
caption: _.isNil(caption) ? dbFile.caption : caption,
folder: _.isUndefined(folder) ? dbFile.folder : folder,
location: _.isUndefined(folder) ? dbFile.path : await fileService.getLocation(folder),
folderPath: _.isUndefined(folder) ? dbFile.path : await fileService.getFolderPath(folder),
};
return this.update(id, newInfos, { user });

View File

@ -76,7 +76,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: null,
location: '/',
folderPath: '/',
});
data.files.push(file);
@ -115,7 +115,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: { id: data.folders[0].id },
location: data.folders[0].location,
folderPath: data.folders[0].path,
});
data.files.push(file);
@ -134,7 +134,7 @@ describe('File', () => {
});
expect(res.status).toBe(400);
expect(res.body.error.message).toBe("the folder doesn't exist");
expect(res.body.error.message).toBe('the folder does not exist');
});
});
@ -170,7 +170,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: { id: data.folders[1].id },
location: data.folders[1].location,
folderPath: data.folders[1].path,
});
data.files[1] = file;
});
@ -204,7 +204,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: { id: data.folders[0].id },
location: data.folders[0].location,
folderPath: data.folders[0].path,
});
data.files[1] = file;
});
@ -240,7 +240,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: { id: data.folders[0].id },
location: data.folders[0].location,
folderPath: data.folders[0].path,
});
data.files[0] = file;
});
@ -274,7 +274,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: { id: data.folders[1].id },
location: data.folders[1].location,
folderPath: data.folders[1].path,
});
data.files[1] = file;
});
@ -311,7 +311,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: null,
location: '/',
folderPath: '/',
});
data.files[0] = file;
});
@ -345,7 +345,7 @@ describe('File', () => {
url: expect.any(String),
provider: 'local',
folder: null,
location: '/',
folderPath: '/',
});
data.files[1] = file;
});
@ -365,7 +365,7 @@ describe('File', () => {
});
expect(res.status).toBe(400);
expect(res.body.error.message).toBe("the folder doesn't exist");
expect(res.body.error.message).toBe('the folder does not exist');
});
test('whithout replacing the file', async () => {
@ -374,13 +374,13 @@ describe('File', () => {
url: `/upload?id=${data.files[1].id}`,
formData: {
fileInfo: JSON.stringify({
folder: '1234', // id that doesn't exist
folder: '1234', // id that does not exist
}),
},
});
expect(res.status).toBe(400);
expect(res.body.error.message).toBe("the folder doesn't exist");
expect(res.body.error.message).toBe('the folder does not exist');
});
});
});

View File

@ -15,8 +15,8 @@ let data = {
};
const uuidRegex = /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
const rootLocationRegex = /^\/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
const getFolderLocationRegex = uid =>
const rootPathRegex = /^\/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i;
const getFolderPathRegex = uid =>
new RegExp(
'^/' + uid + '/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$',
'i'
@ -51,12 +51,12 @@ describe('Folder', () => {
id: expect.anything(),
name: 'folder 1',
uid: expect.stringMatching(uuidRegex),
location: expect.stringMatching(rootLocationRegex),
path: expect.stringMatching(rootPathRegex),
createdAt: expect.anything(),
updatedAt: expect.anything(),
parent: null,
});
expect(res.body.data.uid).toBe(res.body.data.location.split('/').pop());
expect(res.body.data.uid).toBe(res.body.data.path.split('/').pop());
data.folders.push(omit('parent', res.body.data));
});
@ -75,12 +75,12 @@ describe('Folder', () => {
id: expect.anything(),
name: 'folder-2',
uid: expect.stringMatching(uuidRegex),
location: expect.stringMatching(getFolderLocationRegex(data.folders[0].uid)),
path: expect.stringMatching(getFolderPathRegex(data.folders[0].uid)),
createdAt: expect.anything(),
updatedAt: expect.anything(),
parent: data.folders[0],
});
expect(res.body.data.uid).toBe(res.body.data.location.split('/').pop());
expect(res.body.data.uid).toBe(res.body.data.path.split('/').pop());
data.folders.push(omit('parent', res.body.data));
});
@ -188,10 +188,7 @@ describe('Folder', () => {
username: null,
},
files: { count: 0 },
parent: pick(
['createdAt', 'id', 'name', 'location', 'uid', 'updatedAt'],
data.folders[0]
),
parent: pick(['createdAt', 'id', 'name', 'path', 'uid', 'updatedAt'], data.folders[0]),
updatedBy: {
firstname: expect.anything(),
id: expect.anything(),
@ -216,8 +213,8 @@ describe('Folder', () => {
expect(res.body.data).toEqual(
expect.arrayContaining([
pick(['id', 'name', 'location', 'uid', 'updatedAt', 'createdAt'])(data.folders[0]),
pick(['id', 'name', 'location', 'uid', 'updatedAt', 'createdAt'])(data.folders[1]),
pick(['id', 'name', 'path', 'uid', 'updatedAt', 'createdAt'])(data.folders[0]),
pick(['id', 'name', 'path', 'uid', 'updatedAt', 'createdAt'])(data.folders[1]),
])
);
});