refacto

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

packages/strapi-admin/admin/src/components/Roles/RoleRow.js

@@ -16,7 +16,9 @@ const RoleRow = ({ role, onClick, links, prefix }) => {
         <RoleDescription>{role.description}</RoleDescription>
       </td>
       <td>
-        <Text>123</Text>
+        <Text>
+          {role.usersCount} user{role.usersCount === 1 ? '' : 's'}
+        </Text>
       </td>
       <td>
         <IconLinks links={links} />

packages/strapi-admin/admin/src/hooks/useRolesList/reducer.js

@@ -8,19 +8,19 @@ export const initialState = {
       id: 1,
       name: 'Super admin',
       description: 'This is the fake description of the super admin role.',
-      users: [1],
+      usersCounts: 3,
     },
     {
       id: 2,
       name: 'Editor',
       description:
         'This is the fake description of the editor role. This is the fake description of the editor role.',
-      users: [7, 2, 3, 4],
+      usersCounts: 1,
     },
     {
       id: 3,
       name: 'Author',
-      users: [5, 34],
+      usersCounts: 0,
     },
   ],
   isLoading: true,

packages/strapi-admin/controllers/role.js

@@ -5,18 +5,25 @@ const { validateRoleUpdateInput } = require('../validation/role');
 module.exports = {
   async findOne(ctx) {
     const { id } = ctx.params;
-    const role = await strapi.admin.services.role.findOne({ id }, []);
+    const role = await strapi.admin.services.role.findOne({ id });
 
     if (!role) {
       return ctx.notFound('role.notFound');
     }
 
+    const usersCounts = await strapi.admin.services.user.countUsersForRoles([id]);
+    role.usersCount = usersCounts[0];
+
     ctx.body = {
       data: role,
     };
   },
   async findAll(ctx) {
-    const roles = await strapi.admin.services.role.findAll([]);
+    const roles = await strapi.admin.services.role.findAll();
+    const rolesIds = roles.map(r => r.id);
+    const usersCounts = await strapi.admin.services.user.countUsersForRoles(rolesIds);
+    roles.forEach((role, index) => (role.usersCount = usersCounts[index]));
+
     ctx.body = {
       data: roles,
     };

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

@@ -36,7 +36,7 @@ describe('Role', () => {
 
       const foundRole = await roleService.findOne({ id: role.id });
 
-      expect(dbFindOne).toHaveBeenCalledWith({ id: role.id }, undefined);
+      expect(dbFindOne).toHaveBeenCalledWith({ id: role.id }, []);
       expect(foundRole).toStrictEqual(role);
     });
   });
@@ -57,7 +57,7 @@ describe('Role', () => {
 
       const foundRoles = await roleService.find();
 
-      expect(dbFind).toHaveBeenCalledWith({}, undefined);
+      expect(dbFind).toHaveBeenCalledWith({}, []);
       expect(foundRoles).toStrictEqual(roles);
     });
   });
@@ -78,7 +78,7 @@ describe('Role', () => {
 
       const foundRoles = await roleService.findAll();
 
-      expect(dbFind).toHaveBeenCalledWith({ _limit: -1 }, undefined);
+      expect(dbFind).toHaveBeenCalledWith({ _limit: -1 }, []);
       expect(foundRoles).toStrictEqual(roles);
     });
   });
@@ -132,17 +132,22 @@ describe('Role', () => {
         users: [],
       };
       const dbDelete = jest.fn(() => Promise.resolve(role));
-      const dbCount = jest.fn(() => Promise.resolve(0));
+      const dbCountUsersForRoles = jest.fn(() => Promise.resolve([0]));
       const dbDeleteByRolesIds = jest.fn(() => Promise.resolve());
 
       global.strapi = {
-        query: () => ({ delete: dbDelete, count: dbCount }),
+        query: () => ({ delete: dbDelete }),
-        admin: { services: { permission: { deleteByRolesIds: dbDeleteByRolesIds } } },
+        admin: {
+          services: {
+            permission: { deleteByRolesIds: dbDeleteByRolesIds },
+            user: { countUsersForRoles: dbCountUsersForRoles },
+          },
+        },
       };
 
       const deletedRoles = await roleService.deleteByIds([role.id]);
 
-      expect(dbCount).toHaveBeenCalledWith({ 'roles.id': role.id });
+      expect(dbCountUsersForRoles).toHaveBeenCalledWith([role.id]);
       expect(dbDelete).toHaveBeenCalledWith({ id_in: [role.id] });
       expect(deletedRoles).toStrictEqual([role]);
     });
@@ -163,18 +168,22 @@ describe('Role', () => {
       ];
       const rolesIds = roles.map(r => r.id);
       const dbDelete = jest.fn(() => Promise.resolve(roles));
-      const dbCount = jest.fn(() => Promise.resolve(0));
+      const dbCountUsersForRoles = jest.fn(() => Promise.resolve([0, 0]));
       const dbDeleteByRolesIds = jest.fn(() => Promise.resolve());
 
       global.strapi = {
-        query: () => ({ delete: dbDelete, count: dbCount }),
+        query: () => ({ delete: dbDelete }),
-        admin: { services: { permission: { deleteByRolesIds: dbDeleteByRolesIds } } },
+        admin: {
+          services: {
+            permission: { deleteByRolesIds: dbDeleteByRolesIds },
+            user: { countUsersForRoles: dbCountUsersForRoles },
+          },
+        },
       };
 
       const deletedRoles = await roleService.deleteByIds(rolesIds);
 
-      expect(dbCount).toHaveBeenNthCalledWith(1, { 'roles.id': rolesIds[0] });
+      expect(dbCountUsersForRoles).toHaveBeenCalledWith(rolesIds);
-      expect(dbCount).toHaveBeenNthCalledWith(2, { 'roles.id': rolesIds[1] });
       expect(dbDelete).toHaveBeenCalledWith({ id_in: rolesIds });
       expect(deletedRoles).toStrictEqual(roles);
     });

packages/strapi-admin/services/permission.js

@@ -1,3 +1,10 @@
+'use strict';
+
+/**
+ * Delete permissions of roles in database
+ * @param params ids of roles
+ * @returns {Promise<array>}
+ */
 const deleteByRolesIds = rolesIds => {
   return strapi.query('permission', 'admin').delete({ role_in: rolesIds });
 };

packages/strapi-admin/services/role.js

@@ -1,3 +1,5 @@
+'use strict';
+
 const _ = require('lodash');
 
 const sanitizeRole = role => {
@@ -25,7 +27,7 @@ const create = async attributes => {
  * @param params query params to find the role
  * @returns {Promise<role>}
  */
-const findOne = (params = {}, populate) => {
+const findOne = (params = {}, populate = []) => {
   return strapi.query('role', 'admin').findOne(params, populate);
 };
 
@@ -34,7 +36,7 @@ const findOne = (params = {}, populate) => {
  * @param params query params to find the roles
  * @returns {Promise<array>}
  */
-const find = (params = {}, populate) => {
+const find = (params = {}, populate = []) => {
   return strapi.query('role', 'admin').find(params, populate);
 };
 
@@ -42,7 +44,7 @@ const find = (params = {}, populate) => {
  * Find all roles in database
  * @returns {Promise<array>}
  */
-const findAll = populate => {
+const findAll = (populate = []) => {
   return strapi.query('role', 'admin').find({ _limit: -1 }, populate);
 };
 
@@ -80,18 +82,16 @@ const exists = async params => {
 
 /**
  * Delete roles in database if they have no user assigned
- * @param params query params to find the roles
+ * @param ids query params to find the roles
- * @returns {Promise<boolean>}
+ * @returns {Promise<array>}
  */
 const deleteByIds = async (ids = []) => {
-  for (let id of ids) {
+  const usersCounts = await strapi.admin.services.user.countUsersForRoles(ids);
-    const count = await strapi.query('user', 'admin').count({ 'roles.id': id });
+  if (usersCounts.some(count => count !== 0)) {
-    if (count !== 0) {
     throw strapi.errors.badRequest('ValidationError', {
       ids: ['Some roles are still assigned to some users.'],
     });
   }
-  }
 
   await strapi.admin.services.permission.deleteByRolesIds(ids);
 

packages/strapi-admin/services/user.js

@@ -120,6 +120,20 @@ const searchPage = async query => {
   return strapi.query('user', 'admin').searchPage(query);
 };
 
+/** Count the number of users for some roles
+ * @param rolesIds
+ * @returns {Promise<array>}
+ */
+const countUsersForRoles = async (rolesIds = []) => {
+  const counts = [];
+  for (let roleId of rolesIds) {
+    const count = await strapi.query('user', 'admin').count({ 'roles.id': roleId });
+    counts.push(count);
+  }
+
+  return counts;
+};
+
 module.exports = {
   create,
   update,
@@ -129,4 +143,5 @@ module.exports = {
   sanitizeUser,
   findPage,
   searchPage,
+  countUsersForRoles,
 };

packages/strapi-utils/lib/buildQuery.js

@@ -3,17 +3,6 @@
 const _ = require('lodash');
 const parseType = require('./parse-type');
 
-const findModelByAssoc = assoc => {
-  let models;
-  if (assoc.plugin === 'admin') {
-    models = strapi.admin.models;
-  } else {
-    models = assoc.plugin ? strapi.plugins[assoc.plugin].models : strapi.models;
-  }
-
-  return models[assoc.collection || assoc.model];
-};
-
 const isAttribute = (model, field) =>
   _.has(model.allAttributes, field) || model.primaryKey === field || field === 'id';
 
@@ -38,7 +27,7 @@ const getAssociationFromFieldKey = ({ model, field }) => {
 
     if (assoc) {
       association = assoc;
-      tmpModel = findModelByAssoc(assoc);
+      tmpModel = strapi.db.getModelByAssoc(assoc);
       continue;
     }