From f1a1e82b2ca5a2e618acb7eeaf1d87c9c72aad1e Mon Sep 17 00:00:00 2001 From: Alexandre Bodin Date: Mon, 24 Jun 2019 15:31:22 +0200 Subject: [PATCH] CRUD group schemas --- examples/getstarted/groups/cta_facebook.json | 6 ++ .../config/routes.json | 24 +++++ .../controllers/Groups.js | 90 ++++++++++++++++-- .../utils/__tests__/yup-formatter.test.js | 74 +++++++++++++++ .../controllers/utils/yup-formatter.js | 19 ++++ .../package.json | 3 +- .../services/Groups.js | 94 ++++++++++++++++++- .../test/groups.test.e2e.js | 37 +++++++- packages/strapi/lib/core/fs.js | 31 ++++-- packages/strapi/lib/middlewares/boom/index.js | 45 +++++++-- packages/strapi/lib/services/groups/index.js | 4 +- yarn.lock | 13 +++ 12 files changed, 408 insertions(+), 32 deletions(-) create mode 100644 examples/getstarted/groups/cta_facebook.json create mode 100644 packages/strapi-plugin-content-type-builder/controllers/utils/__tests__/yup-formatter.test.js create mode 100644 packages/strapi-plugin-content-type-builder/controllers/utils/yup-formatter.js diff --git a/examples/getstarted/groups/cta_facebook.json b/examples/getstarted/groups/cta_facebook.json new file mode 100644 index 0000000000..8e86ea7ad0 --- /dev/null +++ b/examples/getstarted/groups/cta_facebook.json @@ -0,0 +1,6 @@ +{ + "name": "CTA Facebook", + "connection": "default", + "collectionName": "cta_facebook_aa", + "attributes": {} +} \ No newline at end of file diff --git a/packages/strapi-plugin-content-type-builder/config/routes.json b/packages/strapi-plugin-content-type-builder/config/routes.json index 1e302280ce..7e0691f563 100644 --- a/packages/strapi-plugin-content-type-builder/config/routes.json +++ b/packages/strapi-plugin-content-type-builder/config/routes.json @@ -63,6 +63,30 @@ "config": { "policies": [] } + }, + { + "method": "POST", + "path": "/groups", + "handler": "Groups.createGroup", + "config": { + "policies": [] + } + }, + { + "method": "PUT", + "path": "/groups/:uid", + "handler": "Groups.updateGroup", + "config": { + "policies": [] + } + }, + { + "method": "DELETE", + "path": "/groups/:uid", + "handler": "Groups.deleteGroup", + "config": { + "policies": [] + } } ] } diff --git a/packages/strapi-plugin-content-type-builder/controllers/Groups.js b/packages/strapi-plugin-content-type-builder/controllers/Groups.js index 8c80f95c8d..e74df67ba8 100644 --- a/packages/strapi-plugin-content-type-builder/controllers/Groups.js +++ b/packages/strapi-plugin-content-type-builder/controllers/Groups.js @@ -1,5 +1,21 @@ 'use strict'; +const yup = require('yup'); +const formatYupErrors = require('./utils/yup-formatter'); + +const groupSchema = yup.object().shape({ + name: yup.string().required(), + connection: yup.string(), + collectionName: yup.string(), + attributes: yup.object().required(), +}); + +const internals = { + get service() { + return strapi.plugins['content-type-builder'].services.groups; + }, +}; + /** * Groups controller */ @@ -11,7 +27,7 @@ module.exports = { */ async getGroups(ctx) { const data = await strapi.groupManager.all(); - ctx.body = { data }; + ctx.send({ data }); }, /** @@ -25,18 +41,74 @@ module.exports = { const group = await strapi.groupManager.get(uid); if (!group) { - ctx.status = 404; - ctx.body = { - error: 'group.notFound', - }; + return ctx.send({ error: 'group.notFound' }, 404); } - ctx.body = { data: group }; + ctx.send({ data: group }); }, - async createGroup() {}, + async createGroup(ctx) { + const { body } = ctx.request; - async updateGroup() {}, + try { + await groupSchema.validate(body, { + strict: true, + abortEarly: false, + }); + } catch (error) { + return ctx.send({ error: formatYupErrors(error) }, 400); + } - async deleteGroup() {}, + const uid = internals.service.createGroupUID(body.name); + + if (strapi.groupManager.get(uid) !== undefined) { + return ctx.send({ error: 'group.alreadyExists' }, 400); + } + + const newGroup = await internals.service.createGroup(uid, body); + + ctx.send({ data: newGroup }, 201); + }, + + /** + * Updates a group and return it + * @param {Object} ctx - enhanced koa context + */ + async updateGroup(ctx) { + const { uid } = ctx.params; + const { body } = ctx.request; + + const group = await strapi.groupManager.get(uid); + + if (!group) { + return ctx.send({ error: 'group.notFound' }, 404); + } + + try { + await groupSchema.validate(body, { + strict: true, + abortEarly: false, + }); + } catch (error) { + return ctx.send({ error: formatYupErrors(error) }, 400); + } + + const updatedGroup = await internals.service.updateGroup(group, body); + + ctx.send({ data: updatedGroup }, 200); + }, + + async deleteGroup(ctx) { + const { uid } = ctx.params; + + const group = await strapi.groupManager.get(uid); + + if (!group) { + return ctx.send({ error: 'group.notFound' }, 404); + } + + await internals.service.deleteGroup(group); + + ctx.send({ data: group }, 200); + }, }; diff --git a/packages/strapi-plugin-content-type-builder/controllers/utils/__tests__/yup-formatter.test.js b/packages/strapi-plugin-content-type-builder/controllers/utils/__tests__/yup-formatter.test.js new file mode 100644 index 0000000000..cf0fa42f26 --- /dev/null +++ b/packages/strapi-plugin-content-type-builder/controllers/utils/__tests__/yup-formatter.test.js @@ -0,0 +1,74 @@ +const yup = require('yup'); +const formatYupErrors = require('../yup-formatter'); + +describe('Format yup errors', () => { + test('Format single errors', async () => { + expect.hasAssertions(); + return yup + .object({ + name: yup.string().required('name is required'), + }) + .validate({}) + .catch(err => { + expect(formatYupErrors(err)).toMatchObject({ + name: ['name is required'], + }); + }); + }); + + test('Format multiple errors', async () => { + expect.hasAssertions(); + return yup + .object({ + name: yup + .string() + .min(2, 'min length is 2') + .required(), + }) + .validate( + { + name: '1', + }, + { + strict: true, + abortEarly: false, + } + ) + .catch(err => { + expect(formatYupErrors(err)).toMatchObject({ + name: ['min length is 2'], + }); + }); + }); + + test('Format multiple errors on multiple keys', async () => { + expect.hasAssertions(); + return yup + .object({ + name: yup + .string() + .min(2, 'min length is 2') + .typeError('name must be a string') + .required(), + price: yup + .number() + .integer() + .required('price is required'), + }) + .validate( + { + name: 12, + }, + { + strict: true, + abortEarly: false, + } + ) + .catch(err => { + expect(formatYupErrors(err)).toMatchObject({ + price: ['price is required'], + name: ['name must be a string'], + }); + }); + }); +}); diff --git a/packages/strapi-plugin-content-type-builder/controllers/utils/yup-formatter.js b/packages/strapi-plugin-content-type-builder/controllers/utils/yup-formatter.js new file mode 100644 index 0000000000..a125aaef46 --- /dev/null +++ b/packages/strapi-plugin-content-type-builder/controllers/utils/yup-formatter.js @@ -0,0 +1,19 @@ +'use strict'; + +/** + * Returns a formatted error for http responses + * @param {Object} validationError - a Yup ValidationError + */ +const formatYupErrors = validationError => { + if (validationError.inner.length === 0) { + if (validationError.path === undefined) return validationError.errors; + return { [validationError.path]: validationError.errors }; + } + + return validationError.inner.reduce((acc, err) => { + acc[err.path] = err.errors; + return acc; + }, {}); +}; + +module.exports = formatYupErrors; diff --git a/packages/strapi-plugin-content-type-builder/package.json b/packages/strapi-plugin-content-type-builder/package.json index 7d46d922c5..8e258c94ee 100644 --- a/packages/strapi-plugin-content-type-builder/package.json +++ b/packages/strapi-plugin-content-type-builder/package.json @@ -8,9 +8,10 @@ "description": "content-type-builder.plugin.description" }, "scripts": { - "test": "echo \"no tests yet\"" + "test": "jest --verbose controllers" }, "dependencies": { + "@sindresorhus/slugify": "^0.9.1", "classnames": "^2.2.6", "fs-extra": "^7.0.0", "immutable": "^3.8.2", diff --git a/packages/strapi-plugin-content-type-builder/services/Groups.js b/packages/strapi-plugin-content-type-builder/services/Groups.js index 8b46fbbaad..6e022225bd 100644 --- a/packages/strapi-plugin-content-type-builder/services/Groups.js +++ b/packages/strapi-plugin-content-type-builder/services/Groups.js @@ -1,3 +1,95 @@ 'use strict'; -module.exports = {}; +const { pick } = require('lodash'); +const slugify = require('@sindresorhus/slugify'); + +const VALID_FIELDS = ['name', 'connection', 'collectionName', 'attributes']; + +const createSchema = infos => { + const { name, connection = 'default', collectionName } = infos; + const uid = createGroupUID(name); + + return { + name, + connection, + collectionName: collectionName || uid, + attributes: {}, + }; +}; + +const updateSchema = (oldSchema, newSchema) => + pick({ ...oldSchema, ...newSchema }, VALID_FIELDS); + +const createGroupUID = str => slugify(str, { separator: '_' }); + +/** + * Creates a group schema file + * @param {*} uid + * @param {*} infos + */ +async function createGroup(uid, infos) { + const schema = createSchema(infos); + + return writeSchema(uid, schema); +} + +/** + * Updates a group schema file + * @param {*} group + * @param {*} infos + */ +async function updateGroup(group, infos) { + const { uid } = group; + const updatedSchema = updateSchema(group.schema, infos); + + if (infos.name !== group.schema.name) { + await deleteSchema(uid); + + const newUid = createGroupUID(infos.name); + return writeSchema(newUid, updatedSchema); + } + + return writeSchema(uid, updatedSchema); +} + +async function deleteGroup(group) { + await deleteSchema(group.uid); + process.nextTick(() => strapi.reload()); +} + +/** + * Writes a group schema file + */ +async function writeSchema(uid, schema) { + strapi.reload.isWatching = false; + + await strapi.fs.writeAppFile( + `groups/${uid}.json`, + JSON.stringify(schema, null, 2) + ); + + strapi.reload.isWatching = true; + process.nextTick(() => strapi.reload()); + + return { + uid, + schema, + }; +} + +/** + * Deletes a group schema file + * @param {string} ui + */ +async function deleteSchema(uid) { + strapi.reload.isWatching = false; + await strapi.fs.removeAppFile(`groups/${uid}.json`); + strapi.reload.isWatching = true; +} + +module.exports = { + createGroup, + createGroupUID, + updateGroup, + deleteGroup, +}; diff --git a/packages/strapi-plugin-content-type-builder/test/groups.test.e2e.js b/packages/strapi-plugin-content-type-builder/test/groups.test.e2e.js index 95b8314943..e103815ff7 100644 --- a/packages/strapi-plugin-content-type-builder/test/groups.test.e2e.js +++ b/packages/strapi-plugin-content-type-builder/test/groups.test.e2e.js @@ -23,13 +23,44 @@ describe.only('Content Type Builder - Groups', () => { res.body.data.forEach(el => { expect(el).toMatchObject({ - id: expect.any(String), + uid: expect.any(String), name: expect.any(String), - icon: expect.any(String), - // later schema: expect.objectContaining({}), }); }); }); }); + + describe('GET /group/:uid', () => { + test('Returns 404 on not found', async () => { + const res = await rq({ + method: 'GET', + url: '/content-type-build/groups/nonexistent-group', + }); + + expect(res.statusCode).toBe(404); + }); + + test('Returns correct format', async () => { + const res = await rq({ + method: 'GET', + url: '/content-type-build/groups/existing-group', + }); + + expect(res.statusCode).toBe(200); + expect(res.body).toMatchObject({ + data: { + uid: 'existing-group', + name: 'EXISTING-GROUP', + schema: { + connection: 'default', + collectionName: 'existing_groups', + attributes: { + //... + }, + }, + }, + }); + }); + }); }); diff --git a/packages/strapi/lib/core/fs.js b/packages/strapi/lib/core/fs.js index 331594a110..7b2e3b33cd 100644 --- a/packages/strapi/lib/core/fs.js +++ b/packages/strapi/lib/core/fs.js @@ -1,12 +1,20 @@ 'use strict'; const path = require('path'); -const fs = require('fs-extra'); +const fse = require('fs-extra'); /** * create strapi fs layer */ module.exports = strapi => { + function normalizePath(optPath) { + const filePath = Array.isArray(optPath) ? optPath.join('/') : optPath; + + const normalizedPath = path.normalize(filePath).replace(/^(\/?\.\.?)+/, ''); + + return path.join(strapi.dir, normalizedPath); + } + const strapiFS = { /** * Writes a file in a strapi app @@ -14,15 +22,10 @@ module.exports = strapi => { * @param {string} data - content */ writeAppFile(optPath, data) { - const filePath = Array.isArray(optPath) ? optPath.join('/') : optPath; - - const normalizedPath = path - .normalize(filePath) - .replace(/^(\/?\.\.?)+/, ''); - - const writePath = path.join(strapi.dir, normalizedPath); - - return fs.ensureFile(writePath).then(() => fs.writeFile(writePath, data)); + const writePath = normalizePath(optPath); + return fse + .ensureFile(writePath) + .then(() => fse.writeFile(writePath, data)); }, /** @@ -35,6 +38,14 @@ module.exports = strapi => { const newPath = ['extensions', plugin].concat(optPath).join('/'); return strapiFS.writeAppFile(newPath, data); }, + + /** + * Removes a file in strapi app + */ + removeAppFile(optPath) { + const removePath = normalizePath(optPath); + return fse.remove(removePath); + }, }; return strapiFS; diff --git a/packages/strapi/lib/middlewares/boom/index.js b/packages/strapi/lib/middlewares/boom/index.js index c3602b25b3..ba8b77fc3d 100644 --- a/packages/strapi/lib/middlewares/boom/index.js +++ b/packages/strapi/lib/middlewares/boom/index.js @@ -9,6 +9,39 @@ const _ = require('lodash'); const Boom = require('boom'); const delegate = require('delegates'); +const boomMethods = [ + 'badRequest', + 'unauthorized', + 'paymentRequired', + 'forbidden', + 'notFound', + 'methodNotAllowed', + 'notAcceptable', + 'proxyAuthRequired', + 'clientTimeout', + 'conflict', + 'resourceGone', + 'lengthRequired', + 'preconditionFailed', + 'entityTooLarge', + 'uriTooLong', + 'unsupportedMediaType', + 'rangeNotSatisfiable', + 'expectationFailed', + 'teapot', + 'badData', + 'locked', + 'failedDependency', + 'preconditionRequired', + 'tooManyRequests', + 'illegal', + 'badImplementation', + 'notImplemented', + 'badGateway', + 'serverUnavailable', + 'gatewayTimeout', +]; + module.exports = strapi => { return { /** @@ -69,19 +102,19 @@ module.exports = strapi => { // Custom function to avoid ctx.body repeat createResponses() { - Object.keys(Boom).forEach(key => { - strapi.app.response[key] = function(...rest) { - const error = Boom[key](...rest) || {}; + boomMethods.forEach(method => { + strapi.app.response[method] = function(...rest) { + const error = Boom[method](...rest) || {}; this.status = error.isBoom ? error.output.statusCode : this.status; this.body = error; }; - this.delegator.method(key); + this.delegator.method(method); }); - strapi.app.response.send = function(data) { - this.status = 200; + strapi.app.response.send = function(data, status = 200) { + this.status = status; this.body = data; }; diff --git a/packages/strapi/lib/services/groups/index.js b/packages/strapi/lib/services/groups/index.js index 80b62b3dac..f44622c211 100644 --- a/packages/strapi/lib/services/groups/index.js +++ b/packages/strapi/lib/services/groups/index.js @@ -3,6 +3,7 @@ const initMap = groups => { Object.keys(groups).forEach(key => { const { + name, connection, collectionName, description, @@ -11,8 +12,7 @@ const initMap = groups => { map.set(key, { uid: key, - name: key.toUpperCase(), // get the display name som - schema: { connection, collectionName, description, attributes }, + schema: { name, connection, collectionName, description, attributes }, }); }); diff --git a/yarn.lock b/yarn.lock index 42fb5d3464..658ea0b6f5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2008,6 +2008,14 @@ resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-0.7.0.tgz#9a06f4f137ee84d7df0460c1fdb1135ffa6c50fd" integrity sha512-ONhaKPIufzzrlNbqtWFFd+jlnemX6lJAgq9ZeiZtS7I1PIf/la7CW4m83rTXRnVnsMbW2k56pGYu7AUFJD9Pow== +"@sindresorhus/slugify@^0.9.1": + version "0.9.1" + resolved "https://registry.yarnpkg.com/@sindresorhus/slugify/-/slugify-0.9.1.tgz#892ad24d70b442c0a14fe519cb4019d59bc5069f" + integrity sha512-b6heYM9dzZD13t2GOiEQTDE0qX+I1GyOotMwKh9VQqzuNiVdPVT8dM43fe9HNb/3ul+Qwd5oKSEDrDIfhq3bnQ== + dependencies: + escape-string-regexp "^1.0.5" + lodash.deburr "^4.1.0" + "@snyk/composer-lockfile-parser@1.0.2": version "1.0.2" resolved "https://registry.yarnpkg.com/@snyk/composer-lockfile-parser/-/composer-lockfile-parser-1.0.2.tgz#d748e56076bc1c25b130c1f13ed705fa285a1994" @@ -10873,6 +10881,11 @@ lodash.clonedeep@^4.3.0, lodash.clonedeep@^4.3.2, lodash.clonedeep@^4.5.0: resolved "https://registry.yarnpkg.com/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz#e23f3f9c4f8fbdde872529c1071857a086e5ccef" integrity sha1-4j8/nE+Pvd6HJSnBBxhXoIblzO8= +lodash.deburr@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/lodash.deburr/-/lodash.deburr-4.1.0.tgz#ddb1bbb3ef07458c0177ba07de14422cb033ff9b" + integrity sha1-3bG7s+8HRYwBd7oH3hRCLLAz/5s= + lodash.defaults@^4.2.0: version "4.2.0" resolved "https://registry.yarnpkg.com/lodash.defaults/-/lodash.defaults-4.2.0.tgz#d09178716ffea4dde9e5fb7b37f6f0802274580c"