From 72aaa16d3d60cd2f67becfd3f502dca37a331f86 Mon Sep 17 00:00:00 2001 From: Convly Date: Tue, 11 Aug 2020 18:16:39 +0200 Subject: [PATCH] Rename fieldsGranularity to fieldsRestriction, various bug fixes Signed-off-by: Convly --- packages/strapi-admin/domain/action.js | 11 +++-- .../services/__tests__/content-type.test.js | 41 +++++++++++++++++-- .../strapi-admin/services/content-type.js | 7 ++-- .../services/permission/action-provider.js | 2 + .../validation/action-provider.js | 2 +- .../validation/common-validators.js | 24 +++++++---- .../strapi-admin/validation/permission.js | 6 +-- .../config/functions/bootstrap.js | 6 +-- 8 files changed, 73 insertions(+), 26 deletions(-) diff --git a/packages/strapi-admin/domain/action.js b/packages/strapi-admin/domain/action.js index bb59b2f55b..eefe952afe 100644 --- a/packages/strapi-admin/domain/action.js +++ b/packages/strapi-admin/domain/action.js @@ -15,7 +15,7 @@ const actionFields = [ const defaultAction = { options: { - fieldsGranularity: true, + fieldsRestriction: true, }, }; @@ -39,7 +39,7 @@ const getActionId = ({ pluginName, uid }) => { * Create a permission action * @param {Object} attributes - action attributes */ -function createAction(attributes) { +const createAction = attributes => { const action = _.cloneDeep(_.pick(attributes, actionFields)); action.actionId = getActionId(attributes); @@ -47,10 +47,13 @@ function createAction(attributes) { action.subCategory = attributes.subCategory || 'general'; } - return _.merge(action, defaultAction); -} + return _.merge({}, defaultAction, action); +}; + +const hasFieldsRestriction = _.matchesProperty('options.fieldsRestriction', true); module.exports = { getActionId, createAction, + hasFieldsRestriction, }; diff --git a/packages/strapi-admin/services/__tests__/content-type.test.js b/packages/strapi-admin/services/__tests__/content-type.test.js index 4501f6ff12..cb4bbdd906 100644 --- a/packages/strapi-admin/services/__tests__/content-type.test.js +++ b/packages/strapi-admin/services/__tests__/content-type.test.js @@ -220,7 +220,7 @@ describe('Content-Type', () => { describe('getPermissionsWithNestedFields', () => { test('1 action (no nesting)', async () => { const resultLevel1 = contentTypeService.getPermissionsWithNestedFields([ - { actionId: 'action-1', subjects: ['country'] }, + { actionId: 'action-1', subjects: ['country'], options: { fieldsRestriction: true } }, ]); expect(resultLevel1).toEqual([ { @@ -234,7 +234,13 @@ describe('Content-Type', () => { test('2 actions (with nesting level 1)', async () => { const resultLevel1 = contentTypeService.getPermissionsWithNestedFields( - [{ actionId: 'action-1', subjects: ['country', 'user'] }], + [ + { + actionId: 'action-1', + subjects: ['country', 'user'], + options: { fieldsRestriction: true }, + }, + ], { nestingLevel: 1 } ); expect(resultLevel1).toEqual([ @@ -255,7 +261,13 @@ describe('Content-Type', () => { test('2 actions (with nesting level 2)', async () => { const resultLevel1 = contentTypeService.getPermissionsWithNestedFields( - [{ actionId: 'action-1', subjects: ['country', 'user'] }], + [ + { + actionId: 'action-1', + subjects: ['country', 'user'], + options: { fieldsRestriction: true }, + }, + ], { nestingLevel: 2 } ); expect(resultLevel1).toEqual([ @@ -282,7 +294,11 @@ describe('Content-Type', () => { test('2 actions (with nesting level 100)', async () => { const resultLevel1 = contentTypeService.getPermissionsWithNestedFields([ - { actionId: 'action-1', subjects: ['country', 'user'] }, + { + actionId: 'action-1', + subjects: ['country', 'user'], + options: { fieldsRestriction: true }, + }, ]); expect(resultLevel1).toEqual([ { @@ -311,6 +327,21 @@ describe('Content-Type', () => { }); describe('cleanPermissionFields', () => { + beforeAll(() => { + global.strapi = { + ...global.strapi, + admin: { + services: { + permission: { + actionProvider: { + getByActionId: () => ({ options: { fieldsRestriction: true } }), + }, + }, + }, + }, + }; + }); + const tests = [ [undefined, ['firstname', 'car']], [null, ['firstname', 'car']], @@ -336,6 +367,7 @@ describe('Content-Type', () => { { subject: 'user', fields, + options: { fieldsRestriction: true }, }, ], { @@ -346,6 +378,7 @@ describe('Content-Type', () => { { subject: 'user', fields: expectedFields, + options: { fieldsRestriction: true }, }, ]); }); diff --git a/packages/strapi-admin/services/content-type.js b/packages/strapi-admin/services/content-type.js index de71ad3284..475e3ecf7c 100644 --- a/packages/strapi-admin/services/content-type.js +++ b/packages/strapi-admin/services/content-type.js @@ -2,6 +2,7 @@ const _ = require('lodash'); const fp = require('lodash/fp'); +const actionDomain = require('../domain/action'); const EXCLUDE_FIELDS = ['created_by', 'updated_by']; @@ -118,7 +119,7 @@ const getPermissionsWithNestedFields = (actions, { nestingLevel, restrictedSubje action.subjects .filter(subject => !restrictedSubjects.includes(subject)) .forEach(contentTypeUid => { - const fields = action.options.fieldsGranularity + const fields = actionDomain.hasFieldsRestriction(action) ? getNestedFields(strapi.contentTypes[contentTypeUid], { components: strapi.components, nestingLevel, @@ -141,13 +142,13 @@ const getPermissionsWithNestedFields = (actions, { nestingLevel, restrictedSubje * @param {number} options.nestingLevel level of nesting * @returns {array} */ -const cleanPermissionFields = (permissions, { nestingLevel }) => +const cleanPermissionFields = (permissions, { nestingLevel } = {}) => permissions.map(perm => { const { action: actionId, fields, subject } = perm; const action = strapi.admin.services.permission.actionProvider.getByActionId(actionId); let newFields = fields; - if (!action.options.fieldsGranularity) { + if (!actionDomain.hasFieldsRestriction(action)) { newFields = null; } else if (subject && strapi.contentTypes[subject]) { const possibleFields = getNestedFieldsWithIntermediate(strapi.contentTypes[subject], { diff --git a/packages/strapi-admin/services/permission/action-provider.js b/packages/strapi-admin/services/permission/action-provider.js index 4edab4a314..34c2a11945 100644 --- a/packages/strapi-admin/services/permission/action-provider.js +++ b/packages/strapi-admin/services/permission/action-provider.js @@ -62,6 +62,8 @@ const createActionProvider = () => { } actions.set(actionId, createAction(newAction)); + + return this; }); }, }; diff --git a/packages/strapi-admin/validation/action-provider.js b/packages/strapi-admin/validation/action-provider.js index cc0c016b13..6d99e95c59 100644 --- a/packages/strapi-admin/validation/action-provider.js +++ b/packages/strapi-admin/validation/action-provider.js @@ -63,7 +63,7 @@ const registerProviderActionSchema = yup ), }), options: yup.object({ - fieldsGranularity: yup.boolean(), + fieldsRestriction: yup.boolean(), }), }) .noUnknown() diff --git a/packages/strapi-admin/validation/common-validators.js b/packages/strapi-admin/validation/common-validators.js index 432414a8ea..f6a5be5632 100644 --- a/packages/strapi-admin/validation/common-validators.js +++ b/packages/strapi-admin/validation/common-validators.js @@ -2,6 +2,7 @@ const { yup } = require('strapi-utils'); const _ = require('lodash'); +const actionDomain = require('../domain/action'); const { checkFieldsAreCorrectlyNested, checkFieldsDontHaveDuplicates, @@ -45,11 +46,18 @@ const arrayOfConditionNames = yup : this.createError({ path: this.path, message: `contains conditions that don't exist` }); }); -const checkCTPermsDeleteHaveFieldsToNull = permissions => - !Array.isArray(permissions) || - permissions.every( - perm => perm.action !== 'plugins::content-manager.explorer.delete' || _.isNil(perm.fields) - ); +const checkContentTypesFieldsRestriction = permissions => { + const { actionProvider } = strapi.admin.services.permission; + + if (!_.isArray(permissions)) { + return true; + } + + return permissions.every(({ action: actionId, fields }) => { + const action = actionProvider.getByActionId(actionId); + return actionDomain.hasFieldsRestriction(action) || _.isNil(fields); + }); +}; const permissionsAreEquals = (a, b) => a.action === b.action && (a.subject === b.subject || (_.isNil(a.subject) && _.isNil(b.subject))); @@ -94,9 +102,9 @@ const updatePermissions = yup checkNoDuplicatedPermissions ) .test( - 'delete-fields-are-null', - 'The action "plugins::content-manager.explorer.delete" must have fields set to null or undefined', - checkCTPermsDeleteHaveFieldsToNull + 'fields-restriction', + 'The actions must have fields set to null or undefined', + checkContentTypesFieldsRestriction ) .noUnknown() ), diff --git a/packages/strapi-admin/validation/permission.js b/packages/strapi-admin/validation/permission.js index 96348de648..8f9a163ef4 100644 --- a/packages/strapi-admin/validation/permission.js +++ b/packages/strapi-admin/validation/permission.js @@ -102,9 +102,9 @@ const checkPermissionsExist = function(permissions) { const failIndex = permissions.findIndex( permission => !existingActions.some( - ea => - ea.actionId === permission.action && - (ea.section !== 'contentTypes' || ea.subjects.includes(permission.subject)) + action => + action.actionId === permission.action && + (action.section !== 'contentTypes' || action.subjects.includes(permission.subject)) ) ); diff --git a/packages/strapi-plugin-content-manager/config/functions/bootstrap.js b/packages/strapi-plugin-content-manager/config/functions/bootstrap.js index 0d850fe419..a6d2aa4526 100644 --- a/packages/strapi-plugin-content-manager/config/functions/bootstrap.js +++ b/packages/strapi-plugin-content-manager/config/functions/bootstrap.js @@ -79,7 +79,7 @@ const syncComponentsSchemas = async () => { const componentsToAdd = _.difference(realUIDs, DBUIDs); const componentsToDelete = _.difference(DBUIDs, realUIDs); - // delette old schemas + // delete old schemas await Promise.all(componentsToDelete.map(uid => componentService.deleteConfiguration(uid))); // create new schemas @@ -125,7 +125,7 @@ const registerPermissions = () => { pluginName: 'content-manager', subjects: contentTypesUids, options: { - fieldsGranularity: false, + fieldsRestriction: false, }, }, { @@ -135,7 +135,7 @@ const registerPermissions = () => { pluginName: 'content-manager', subjects: contentTypesUids.filter(hasDraftAndPublish), options: { - fieldsGranularity: false, + fieldsRestriction: false, }, }, {