fifth refacto

Signed-off-by: Pierre Noël <petersg83@gmail.com>
strapi · Jul 8, 2020 · 1ee5b7f · 1ee5b7f · derrickmehaffy · Dec 16, 2020
1 parent 3084b85
commit 1ee5b7f
Show file tree

Hide file tree

Showing 27 changed files with 492 additions and 180 deletions.
diff --git a/packages/strapi-admin/config/functions/bootstrap.js b/packages/strapi-admin/config/functions/bootstrap.js
@@ -19,43 +19,6 @@ const registerAdminConditions = () => {
   });
 };
 
-const cleanPermissionInDatabase = async () => {
-  const { actionProvider } = strapi.admin.services.permission;
-  const dbPermissions = await strapi.admin.services.permission.find();
-  const allActionsMap = actionProvider.getAllByMap();
-  const permissionsToRemoveIds = [];
-
-  dbPermissions.forEach(perm => {
-    if (
-      !allActionsMap.has(perm.action) ||
-      (allActionsMap.get(perm.action).section === 'contentTypes' &&
-        !allActionsMap.get(perm.action).subjects.includes(perm.subject))
-    ) {
-      permissionsToRemoveIds.push(perm.id);
-    }
-  });
-
-  await strapi.admin.services.permission.deleteByIds(permissionsToRemoveIds);
-};
-
-const getPermissionsWithNestedFields = (actions, nestingLevel = 3) =>
-  actions.reduce((perms, action) => {
-    const newPerms = [];
-    action.subjects.forEach(contentTypeUid => {
-      const fields = strapi.admin.services['content-type'].getNestedFields(contentTypeUid, {
-        components: { ...strapi.components, ...strapi.contentTypes },
-        nestingLevel,
-      });
-      newPerms.push({
-        action: action.actionId,
-        subject: contentTypeUid,
-        fields,
-        conditions: [],
-      });
-    });
-    return perms.concat(newPerms);
-  }, []);
-
 const createRolesIfNeeded = async () => {
   const someRolesExist = await strapi.admin.services.role.exists();
   if (someRolesExist) {
@@ -104,7 +67,9 @@ const createRolesIfNeeded = async () => {
     description: 'Authors can manage and publish the content they created.',
   });
 
-  const editorPermissions = getPermissionsWithNestedFields(contentTypesActions);
+  const editorPermissions = strapi.admin.services['content-type'].getPermissionsWithNestedFields(
+    contentTypesActions
+  );
 
   const authorPermissions = editorPermissions.map(p => ({
     ...p,
@@ -145,7 +110,10 @@ const resetSuperAdminPermissions = async () => {
   const allActions = strapi.admin.services.permission.actionProvider.getAll();
   const contentTypesActions = allActions.filter(a => a.section === 'contentTypes');
 
-  const permissions = getPermissionsWithNestedFields(contentTypesActions, 1);
+  const permissions = strapi.admin.services['content-type'].getPermissionsWithNestedFields(
+    contentTypesActions,
+    1
+  );
 
   const otherActions = allActions.filter(a => a.section !== 'contentTypes');
   otherActions.forEach(action => {
@@ -165,7 +133,7 @@ const resetSuperAdminPermissions = async () => {
 module.exports = async () => {
   registerAdminConditions();
   registerPermissionActions();
-  await cleanPermissionInDatabase();
+  await strapi.admin.services.permission.cleanPermissionInDatabase();
   await createRolesIfNeeded();
   await resetSuperAdminPermissions();
   await displayWarningIfNoSuperAdmin();

diff --git a/packages/strapi-admin/controllers/__tests__/role.test.js b/packages/strapi-admin/controllers/__tests__/role.test.js
@@ -86,6 +86,7 @@ describe('Role controller', () => {
   describe('updatePermissions', () => {
     test('Fails on missing permissions input', async () => {
       const badRequest = jest.fn();
+      const findOne = jest.fn(() => Promise.resolve({ id: 1 }));
 
       const ctx = createContext(
         {
@@ -99,7 +100,7 @@ describe('Role controller', () => {
         admin: {
           services: {
             role: {
-              getSuperAdmin: jest.fn(() => undefined),
+              findOne,
             },
           },
         },
@@ -117,6 +118,7 @@ describe('Role controller', () => {
 
     test('Fails on missing action permission', async () => {
       const badRequest = jest.fn();
+      const findOne = jest.fn(() => Promise.resolve({ id: 1 }));
 
       const ctx = createContext(
         {
@@ -130,7 +132,7 @@ describe('Role controller', () => {
       global.strapi = {
         admin: {
           services: {
-            role: { getSuperAdmin: jest.fn(() => undefined) },
+            role: { findOne },
             permission: { conditionProvider: { getAll: jest.fn(() => []) } },
           },
         },

diff --git a/packages/strapi-admin/controllers/role.js b/packages/strapi-admin/controllers/role.js
@@ -1,10 +1,9 @@
 'use strict';
 
-const _ = require('lodash');
 const { yup, formatYupErrors } = require('strapi-utils');
 const { validateRoleUpdateInput } = require('../validation/role');
 const { validatedUpdatePermissionsInput } = require('../validation/permission');
-const { EDITOR_CODE, AUTHOR_CODE } = require('../services/constants');
+const { EDITOR_CODE, AUTHOR_CODE, SUPER_ADMIN_CODE } = require('../services/constants');
 
 module.exports = {
   /**
@@ -88,11 +87,15 @@ module.exports = {
    */
   async updatePermissions(ctx) {
     const { id } = ctx.params;
-    const input = _.cloneDeep(ctx.request.body);
+    const input = ctx.request.body;
+
+    const role = await strapi.admin.services.role.findOne({ id });
+    if (!role) {
+      return ctx.notFound('role.notFound');
+    }
 
     try {
-      const superAdminRole = await strapi.admin.services.role.getSuperAdmin();
-      if (superAdminRole && String(superAdminRole.id) === String(id)) {
+      if (role.code === SUPER_ADMIN_CODE) {
         const err = new yup.ValidationError("Super admin permissions can't be edited.");
         throw formatYupErrors(err);
       }
@@ -101,22 +104,24 @@ module.exports = {
       return ctx.badRequest('ValidationError', err);
     }
 
-    const role = await strapi.admin.services.role.findOne({ id });
-
-    if (!role) {
-      return ctx.notFound('role.notFound');
-    }
-
     let existingPermissions = strapi.admin.services.permission.actionProvider.getAllByMap();
+    let permissionsToAssign;
     if ([EDITOR_CODE, AUTHOR_CODE].includes(role.code)) {
-      input.permissions
+      permissionsToAssign = input.permissions.filter(
+        p => existingPermissions.get(p.action).section !== 'contentTypes'
+      );
+      const modifiedPermissions = input.permissions
         .filter(p => existingPermissions.get(p.action).section === 'contentTypes')
-        .forEach(p => {
-          p.conditions = role.code === AUTHOR_CODE ? ['admin::is-creator'] : [];
-        });
+        .map(p => ({
+          ...p,
+          conditions: role.code === AUTHOR_CODE ? ['admin::is-creator'] : [],
+        }));
+      permissionsToAssign.push(...modifiedPermissions);
+    } else {
+      permissionsToAssign = input.permissions;
     }
 
-    const permissions = await strapi.admin.services.permission.assign(role.id, input.permissions);
+    const permissions = await strapi.admin.services.permission.assign(role.id, permissionsToAssign);
 
     ctx.body = {
       data: permissions,

diff --git a/packages/strapi-admin/ee/controllers/role.js b/packages/strapi-admin/ee/controllers/role.js
@@ -8,6 +8,7 @@ const {
   validateRoleDeleteInput,
 } = require('../validation/role');
 const { validatedUpdatePermissionsInput } = require('../validation/permission');
+const { SUPER_ADMIN_CODE } = require('../../services/constants');
 
 module.exports = {
   /**
@@ -102,9 +103,13 @@ module.exports = {
     const { id } = ctx.params;
     const input = ctx.request.body;
 
+    const role = await strapi.admin.services.role.findOne({ id });
+    if (!role) {
+      return ctx.notFound('role.notFound');
+    }
+
     try {
-      const superAdminRole = await strapi.admin.services.role.getSuperAdmin();
-      if (superAdminRole && String(superAdminRole.id) === String(id)) {
+      if (role.code === SUPER_ADMIN_CODE) {
         const err = new yup.ValidationError("Super admin permissions can't be edited.");
         throw formatYupErrors(err);
       }
@@ -113,8 +118,6 @@ module.exports = {
       return ctx.badRequest('ValidationError', err);
     }
 
-    const role = await strapi.admin.services.role.findOne({ id });
-
     if (!role) {
       return ctx.notFound('role.notFound');
     }

diff --git a/packages/strapi-admin/ee/validation/permission.js b/packages/strapi-admin/ee/validation/permission.js
@@ -1,41 +1,12 @@
 'use strict';
 
-const { yup, formatYupErrors } = require('strapi-utils');
+const { formatYupErrors } = require('strapi-utils');
 const validators = require('../../validation/common-validators');
-const { checkFieldsAreCorrectlyNested } = require('../../validation/common-functions');
 
 const handleReject = error => Promise.reject(formatYupErrors(error));
 
-const updatePermissionsSchema = yup
-  .object()
-  .shape({
-    permissions: yup
-      .array()
-      .of(
-        yup
-          .object()
-          .shape({
-            action: yup.string().required(),
-            subject: yup.string().nullable(),
-            fields: yup
-              .array()
-              .of(yup.string())
-              .test(
-                'field-nested',
-                'Fields format are incorrect (duplicates or bad nesting).',
-                checkFieldsAreCorrectlyNested
-              ),
-            conditions: validators.arrayOfConditionNames,
-          })
-          .noUnknown()
-      )
-      .requiredAllowEmpty(),
-  })
-  .required()
-  .noUnknown();
-
 const validatedUpdatePermissionsInput = data => {
-  return updatePermissionsSchema
+  return validators.updatePermissions
     .validate(data, { strict: true, abortEarly: false })
     .catch(handleReject);
 };

diff --git a/packages/strapi-admin/ee/validation/role.js b/packages/strapi-admin/ee/validation/role.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const { yup, formatYupErrors } = require('strapi-utils');
+const { yup, formatYupErrors, stringIncludes, stringEquals } = require('strapi-utils');
 
 const handleReject = error => Promise.reject(formatYupErrors(error));
 
@@ -33,7 +33,7 @@ const rolesDeleteSchema = yup
       .required()
       .test('no-admin-many-delete', 'You cannot delete the super admin role', async ids => {
         const superAdminRole = await strapi.admin.services.role.getSuperAdmin();
-        return !superAdminRole || !ids.map(String).includes(String(superAdminRole.id));
+        return !superAdminRole || !stringIncludes(ids, superAdminRole.id);
       }),
   })
   .noUnknown();
@@ -43,7 +43,7 @@ const roleDeleteSchema = yup
   .required()
   .test('no-admin-single-delete', 'You cannot delete the super admin role', async function(id) {
     const superAdminRole = await strapi.admin.services.role.getSuperAdmin();
-    return !superAdminRole || String(id) !== String(superAdminRole.id)
+    return !superAdminRole || !stringEquals(id, superAdminRole.id)
       ? true
       : this.createError({ path: 'id', message: `You cannot delete the super admin role` });
   });