add DELETE /admin/roles

Signed-off-by: Pierre Noël <petersg83@gmail.com>

packages/strapi-admin/ee/config/routes.json

@@ -7,6 +7,13 @@
       "config": {
         "policies": []
       }
+    }, {
+      "method": "DELETE",
+      "path": "/roles",
+      "handler": "role.delete",
+      "config": {
+        "policies": []
+      }
     }
   ]
 }

packages/strapi-admin/ee/controllers/role.js

@@ -1,6 +1,10 @@
 'use strict';
 
-const { validateRoleCreateInput, validateRoleUpdateInput } = require('../validation/role');
+const {
+  validateRoleCreateInput,
+  validateRoleUpdateInput,
+  validateRoleDeleteInput,
+} = require('../validation/role');
 
 module.exports = {
   async create(ctx) {
@@ -35,4 +39,18 @@ module.exports = {
       data: sanitizedRole,
     };
   },
+  async delete(ctx) {
+    try {
+      await validateRoleDeleteInput(ctx.request.body);
+    } catch (err) {
+      return ctx.badRequest('ValidationError', err);
+    }
+
+    let roles = await strapi.admin.services.role.delete({ id_in: ctx.request.body.ids });
+    const sanitizedRoles = roles.map(strapi.admin.services.role.sanitizeRole);
+
+    ctx.body = {
+      data: sanitizedRoles,
+    };
+  },
 };

packages/strapi-admin/ee/validation/role.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const { yup, formatYupErrors } = require('strapi-utils');
+const { intergerOrString } = require('../../validation/common-validators');
 
 const handleReject = error => Promise.reject(formatYupErrors(error));
 
@@ -27,7 +28,23 @@ const validateRoleUpdateInput = async data => {
     .catch(handleReject);
 };
 
+const validateRoleDeleteInput = async data => {
+  const roleDeleteSchema = yup
+    .object()
+    .shape({
+      ids: yup
+        .array()
+        .of(intergerOrString)
+        .min(1)
+        .required(),
+    })
+    .noUnknown();
+
+  return roleDeleteSchema.validate(data, { strict: true, abortEarly: false }).catch(handleReject);
+};
+
 module.exports = {
   validateRoleCreateInput,
   validateRoleUpdateInput,
+  validateRoleDeleteInput,
 };

packages/strapi-admin/services/__tests__/role.test.js

@@ -123,4 +123,55 @@ describe('Role', () => {
       expect(updatedRole).toStrictEqual(expectedUpdatedRole);
     });
   });
+  describe('delete', () => {
+    test('Delete a role', async () => {
+      const role = {
+        id: 1,
+        name: 'admin',
+        description: 'Description',
+        users: [],
+      };
+      const dbFind = jest.fn(() => Promise.resolve([role]));
+      const dbDelete = jest.fn(() => Promise.resolve(role));
+
+      global.strapi = {
+        query: () => ({ find: dbFind, delete: dbDelete }),
+      };
+
+      const deletedRoles = await roleService.delete({ id: role.id });
+
+      expect(dbFind).toHaveBeenCalledWith({ id: role.id });
+      expect(dbDelete).toHaveBeenCalledWith({ id_in: [role.id] });
+      expect(deletedRoles).toStrictEqual([role]);
+    });
+    test('Delete two roles', async () => {
+      const roles = [
+        {
+          id: 1,
+          name: 'admin 1',
+          description: 'Description',
+          users: [],
+        },
+        {
+          id: 2,
+          name: 'admin 2',
+          description: 'Description',
+          users: [],
+        },
+      ];
+      const rolesIds = roles.map(r => r.id);
+      const dbFind = jest.fn(() => Promise.resolve(roles));
+      const dbDelete = jest.fn(() => Promise.resolve(roles));
+
+      global.strapi = {
+        query: () => ({ find: dbFind, delete: dbDelete }),
+      };
+
+      const deletedRoles = await roleService.delete({ id_in: rolesIds });
+
+      expect(dbFind).toHaveBeenCalledWith({ id_in: rolesIds });
+      expect(dbDelete).toHaveBeenCalledWith({ id_in: rolesIds });
+      expect(deletedRoles).toStrictEqual(roles);
+    });
+  });
 });

packages/strapi-admin/services/role.js

@@ -78,6 +78,34 @@ const exists = async params => {
   return foundCount > 0;
 };
 
+/**
+ * Delete roles in database if they have no user assigned
+ * @param params query params to find the roles
+ * @returns {Promise<boolean>}
+ */
+const deleteRoles = async params => {
+  const foundRoles = await strapi.query('role', 'admin').find(params);
+
+  if (foundRoles.some(r => r.users.length !== 0)) {
+    throw strapi.errors.badRequest('ValidationError', {
+      ids: ['Some roles are still assigned to some users.'],
+    });
+  }
+
+  const rolesToDeleteIds = foundRoles.map(role => role.id);
+
+  // TODO: Waiting for permissions
+  // await strapi.admin.services.permission.delete({ roleId_in: rolesToDeleteIds });
+
+  let deletedRoles = await strapi.query('role', 'admin').delete({ id_in: rolesToDeleteIds });
+
+  if (!Array.isArray(deletedRoles)) {
+    deletedRoles = [deletedRoles];
+  }
+
+  return deletedRoles;
+};
+
 module.exports = {
   sanitizeRole,
   create,
@@ -86,4 +114,5 @@ module.exports = {
   findAll,
   update,
   exists,
+  delete: deleteRoles,
 };

packages/strapi-admin/test/admin-role-crud.test.e2e.js

@@ -1,4 +1,5 @@
-// Helpers.
+const _ = require('lodash');
+
 const { registerAndLogin } = require('../../../test/helpers/auth');
 const { createAuthRequest } = require('../../../test/helpers/request');
 
@@ -17,21 +18,22 @@ describe('Role CRUD End to End', () => {
   }, 60000);
 
   if (edition === 'EE') {
-    describe('Create a new role', () => {
+    describe('Create some roles', () => {
-      test('Can create a role successfully', async () => {
+      const rolesToCreate = [
-        const role = {
+        [{ name: 'new role 0', description: 'description' }],
-          name: 'new role',
+        [{ name: 'new role 1', description: 'description' }],
-          description: 'Description of new role',
+        [{ name: 'new role 2', description: 'description' }],
-        };
+        [{ name: 'new role 3', description: 'description' }],
-
+        [{ name: 'new role 4', description: 'description' }],
-        const res = await rq({
+        [{ name: 'new role 5', description: 'description' }],
+      ];
+      test.each(rolesToCreate)('can create %p', async role => {
+        let res = await rq({
           url: '/admin/roles',
           method: 'POST',
           body: role,
         });
 
-        data.roles.push(res.body.data);
-
         expect(res.statusCode).toBe(201);
         expect(res.body.data).toMatchObject({
           id: expect.anything(),
@@ -40,36 +42,10 @@ describe('Role CRUD End to End', () => {
           created_at: expect.anything(),
           updated_at: expect.anything(),
         });
-      });
-      test('Can create another role successfully', async () => {
-        const role = {
-          name: 'new role 2',
-          description: 'Description of new role 2',
-        };
-
-        const res = await rq({
-          url: '/admin/roles',
-          method: 'POST',
-          body: role,
-        });
-
         data.roles.push(res.body.data);
-
-        expect(res.statusCode).toBe(201);
-        expect(res.body.data).toMatchObject({
-          id: expect.anything(),
-          name: role.name,
-          description: role.description,
-          created_at: expect.anything(),
-          updated_at: expect.anything(),
-        });
       });
       test('Cannot create a role already existing', async () => {
-        const role = {
+        const role = _.pick(data.roles[0], ['name', 'description']);
-          name: 'new role',
-          description: 'Description of new role',
-        };
-
         const res = await rq({
           url: '/admin/roles',
           method: 'POST',
@@ -78,7 +54,7 @@ describe('Role CRUD End to End', () => {
 
         expect(res.statusCode).toBe(400);
         expect(res.body.data).toMatchObject({
-          name: ['The name must be unique and a role with name `new role` already exists.'],
+          name: [`The name must be unique and a role with name \`${role.name}\` already exists.`],
         });
       });
     });
@@ -183,6 +159,58 @@ describe('Role CRUD End to End', () => {
         });
       });
     });
+    describe('Delete roles', () => {
+      test('Can delete a role', async () => {
+        let res = await rq({
+          url: '/admin/roles',
+          method: 'DELETE',
+          body: { ids: [data.roles[0].id] },
+        });
+        expect(res.statusCode).toBe(200);
+        expect(res.body.data).toMatchObject([data.roles[0]]);
+
+        res = await rq({
+          url: `/admin/roles/${data.roles[0].id}`,
+          method: 'GET',
+          body: { ids: data.roles[0].id },
+        });
+        expect(res.statusCode).toBe(404);
+
+        data.roles.shift();
+      });
+      test('Can delete two roles', async () => {
+        const roles = data.roles.slice(0, 2);
+        const rolesIds = roles.map(r => r.id);
+
+        let res = await rq({
+          url: '/admin/roles',
+          method: 'DELETE',
+          body: { ids: rolesIds },
+        });
+        expect(res.statusCode).toBe(200);
+        expect(res.body.data).toMatchObject(roles);
+
+        for (let roleId of rolesIds) {
+          res = await rq({
+            url: `/admin/roles/${roleId}`,
+            method: 'GET',
+            body: { ids: data.roles[0].id },
+          });
+          expect(res.statusCode).toBe(404);
+          data.roles.shift();
+        }
+      });
+      test("No error if deleting a role that doesn't exist", async () => {
+        const res = await rq({
+          url: '/admin/roles',
+          method: 'DELETE',
+          body: { ids: ['id-that-doesnt-exist'] },
+        });
+
+        expect(res.statusCode).toBe(200);
+        expect(res.body.data).toEqual([]);
+      });
+    });
   }
 
   if (edition === 'CE') {

packages/strapi-admin/validation/common-validators.js

@@ -15,6 +15,9 @@ const validators = {
     .matches(/[a-z]/, '${path} must contain at least one lowercase character')
     .matches(/[A-Z]/, '${path} must contain at least one uppercase character')
     .matches(/\d/, '${path} must contain at least one number'),
+  intergerOrString: yup.lazy(value =>
+    typeof value === 'number' ? yup.number().integer() : yup.string()
+  ), // https://github.com/jquense/yup/issues/665
 };
 
 module.exports = validators;

packages/strapi-connector-bookshelf/lib/relations.js

@@ -24,11 +24,7 @@ const transformToArrayID = array => {
 };
 
 const getModel = (model, plugin) => {
-  return (
+  return strapi.db.getModel(model, plugin) || strapi.db.getModel(model);
-    _.get(strapi.plugins, [plugin, 'models', model]) ||
-    _.get(strapi, ['models', model]) ||
-    undefined
-  );
 };
 
 const removeUndefinedKeys = obj => _.pickBy(obj, _.negate(_.isUndefined));

packages/strapi/lib/middlewares/parser/defaults.json

@@ -1,6 +1,7 @@
 {
   "parser": {
     "enabled": true,
-    "multipart": true
+    "multipart": true,
+    "parsedMethods": ["POST", "PUT", "PATCH", "DELETE"]
   }
 }