added role cards to permission screen (#4401)

* simplify creating new roles by auto-replacing input

src/Controller/PermissionController.php

@@ -16,6 +16,7 @@ use App\Event\PermissionSectionsEvent;
 use App\Event\PermissionsEvent;
 use App\Form\RoleType;
 use App\Model\PermissionSection;
+use App\Repository\Query\UserQuery;
 use App\Repository\RoleRepository;
 use App\Repository\UserRepository;
 use App\Security\RolePermissionManager;
@@ -46,7 +47,7 @@ final class PermissionController extends AbstractController
 
     #[Route(path: '', name: 'admin_user_permissions', methods: ['GET', 'POST'])]
     #[IsGranted('role_permissions')]
-    public function permissions(EventDispatcherInterface $dispatcher, CsrfTokenManagerInterface $csrfTokenManager, RoleService $roleService): Response
+    public function permissions(EventDispatcherInterface $dispatcher, CsrfTokenManagerInterface $csrfTokenManager, RoleService $roleService, UserRepository $userRepository): Response
     {
         $all = $this->roleRepository->findAll();
         $existing = [];
@@ -151,7 +152,10 @@ final class PermissionController extends AbstractController
         $page->setHelp('permissions.html');
         $page->setActionName('user_permissions');
 
+        $users = $userRepository->getUsersForQuery(new UserQuery());
+
         return $this->render('permission/permissions.html.twig', [
+            'users' => $users,
             'page_setup' => $page,
             'token' => $csrfTokenManager->refreshToken(self::TOKEN_NAME)->getValue(),
             'roles' => array_values($roles),

src/Entity/Role.php

@@ -46,4 +46,9 @@ class Role
 
         return $this;
     }
+
+    public function isUser(): bool
+    {
+        return $this->name === User::ROLE_USER;
+    }
 }

src/Form/RoleType.php

@@ -34,22 +34,43 @@ final class RoleType extends AbstractType
             ]
         ]);
 
-        // help the user to figure out the allowed name
+        $replacer = function ($roleName): ?string {
-        $builder->get('name')->addViewTransformer(
+            if ($roleName === null) {
-            new CallbackTransformer(
+                return null;
-                function ($roleName) {
+            }
+
             if (\is_string($roleName)) {
-                        $roleName = str_replace(' ', '_', $roleName);
+                $roleName = preg_replace('/[^a-zA-Z_]/', '_', $roleName);
-                        $roleName = str_replace('-', '_', $roleName);
+                $roleName = preg_replace('/_+/', '_', $roleName ?? '');
+                $roleName = ltrim($roleName ?? '', '_');
+                $roleName = rtrim($roleName, '_');
+                $roleName = strtoupper($roleName);
             }
 
             return $roleName;
+        };
+
+        // help the user to figure out the allowed name
+        $builder->get('name')->addViewTransformer(
+            new CallbackTransformer(
+                function ($roleName) use ($replacer) {
+                    return $replacer($roleName);
                 },
                 function ($roleName) {
                     return $roleName;
                 }
             )
         );
+        $builder->get('name')->addModelTransformer(
+            new CallbackTransformer(
+                function ($roleName) {
+                    return $roleName;
+                },
+                function ($roleName) use ($replacer) {
+                    return $replacer($roleName);
+                }
+            )
+        );
     }
 
     public function configureOptions(OptionsResolver $resolver): void

templates/permission/permissions.html.twig

@@ -9,6 +9,7 @@
         'name': 'alwaysVisible',
     } %}
     {% set canEditPermissions = is_granted('role_permissions') %}
+    <div class="row g-2 mb-3">
     {% for role in roles %}
         {% set class = 'alwaysVisible text-center' %}
         {% if role.name == 'ROLE_SUPER_ADMIN' %}
@@ -16,18 +17,49 @@
         {% elseif role.name == 'ROLE_USER' %}
             {% set class = class ~ ' bg-green-lt' %}
         {% endif %}
-        {% set options = {'class': class} %}
+        {% set columns = columns|merge({(role.name): {'class': class}}) %}
-        {% if canEditPermissions and (role.name not in system_roles) %}
+        {% set roleUsers = [] %}
-            {% set widget %}
+        {% for user in users %}
-                &nbsp;<a href="{{ path('admin_user_role_delete', {'id': role.id, 'csrfToken': token}) }}" class="confirmation-link" data-question="confirm.delete">{{ icon('trash') }}</a>
+            {% if user.hasRole(role.getName()) %}
-            {% endset %}
+                {% set roleUsers = roleUsers|merge([user]) %}
-            {% set options = options|merge({'html_after': widget}) %}
             {% endif %}
-        {% set columns = columns|merge({
-            (role.name): options,
-        }) %}
         {% endfor %}
 
+        {% if not role.isUser() %}
+        <div class="col-xl-4 col-lg-6 col-md-6">
+            <div class="card h-100">
+                <div class="card-body">
+                    <div class="d-flex justify-content-between mb-2">
+                        <h4 class="card-title">
+                            {{ role.getName()|trans }}
+                            {% if canEditPermissions and (role.name not in system_roles) %}
+                                &nbsp;
+                                <a href="{{ path('admin_user_role_delete', {'id': role.id, 'csrfToken': token}) }}" class="confirmation-link" data-question="confirm.delete">{{ icon('trash') }}</a>
+                            {% endif %}
+                        </h4>
+                        <div class="list-unstyled d-flex mb-0">
+                            {% if is_granted('view_user') %}
+                                <a href="{{ path('admin_user', {'role': role.getName()}) }}" class="fs-6 mb-1">{{ 'amount'|trans }}: {{ roleUsers|length }}</a>
+                            {% else %}
+                                <span class="fs-6 mb-1">Total {{ roleUsers|length }} users</span>
+                            {% endif %}
+                        </div>
+                    </div>
+                    <div class="d-flex justify-content-between align-items-end">
+                        <div class="list-unstyled d-flex align-items-center avatars avatar-list avatar-list-stacked mb-0">
+                            {% for user in roleUsers %}
+                                {{ widgets.user_avatar(user, user.displayName, 'avatar-rounded avatar-xs')  }}
+                            {% endfor %}
+                        </div>
+                    </div>
+                </div>
+            </div>
+        </div>
+        {% endif %}
+    {% endfor %}
+    </div>
+
+
     {% set tableName = 'user_admin_permissions' %}
 
     {{ tables.datatable_header(tableName, columns, null, {'reload': 'kimai.userRoleUpdate'}) }}

tests/Controller/PermissionControllerTest.php

@@ -19,17 +19,17 @@ use App\Entity\User;
  */
 class PermissionControllerTest extends ControllerBaseTest
 {
-    public function testIsSecure()
+    public function testIsSecure(): void
     {
         $this->assertUrlIsSecured('/admin/permissions');
     }
 
-    public function testIsSecureForRole()
+    public function testIsSecureForRole(): void
     {
         $this->assertUrlIsSecuredForRole(User::ROLE_ADMIN, '/admin/permissions');
     }
 
-    public function testPermissions()
+    public function testPermissions(): void
     {
         $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN);
         $this->assertAccessIsGranted($client, '/admin/permissions');
@@ -52,17 +52,17 @@ class PermissionControllerTest extends ControllerBaseTest
         self::assertStringContainsString('<th data-field="ROLE_SUPER_ADMIN" class="alwaysVisible text-center bg-orange-lt col_ROLE_SUPER_ADMIN">', $content);
     }
 
-    public function testCreateRoleIsSecured()
+    public function testCreateRoleIsSecured(): void
     {
         $this->assertUrlIsSecured('/admin/permissions/roles/create');
     }
 
-    public function testCreateRoleIsSecuredForRole()
+    public function testCreateRoleIsSecuredForRole(): void
     {
         $this->assertUrlIsSecuredForRole(User::ROLE_ADMIN, '/admin/permissions');
     }
 
-    public function testCreateRole()
+    public function testCreateRole(): void
     {
         $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN);
         $this->assertAccessIsGranted($client, '/admin/permissions/roles/create');
@@ -79,17 +79,17 @@ class PermissionControllerTest extends ControllerBaseTest
         $this->assertTableHeader($content);
     }
 
-    public function testDeleteRoleIsSecured()
+    public function testDeleteRoleIsSecured(): void
     {
         $this->assertUrlIsSecured('/admin/permissions/roles/1/delete/sdfsdfsdfsd');
     }
 
-    public function testDeleteRoleIsSecuredForRole()
+    public function testDeleteRoleIsSecuredForRole(): void
     {
         $this->assertUrlIsSecuredForRole(User::ROLE_ADMIN, '/admin/permissions');
     }
 
-    public function testDeleteRole()
+    public function testDeleteRole(): void
     {
         $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN);
         $this->assertAccessIsGranted($client, '/admin/permissions/roles/create');
@@ -132,7 +132,7 @@ class PermissionControllerTest extends ControllerBaseTest
         $this->assertEquals(['ROLE_TEAMLEAD', 'ROLE_SUPER_ADMIN', 'TEST_ROLE', 'ROLE_USER'], $user->getRoles());
 
         $this->request($client, '/admin/permissions');
-        $node = $client->getCrawler()->filter('table.dataTable thead th a.confirmation-link');
+        $node = $client->getCrawler()->filter('div.card .card-title a.confirmation-link');
         self::assertEquals(1, $node->count());
 
         $this->request($client, $node->attr('href'));
@@ -148,17 +148,17 @@ class PermissionControllerTest extends ControllerBaseTest
         $this->assertEquals(['ROLE_TEAMLEAD', 'ROLE_SUPER_ADMIN', 'ROLE_USER'], $user->getRoles());
     }
 
-    public function testSavePermissionIsSecured()
+    public function testSavePermissionIsSecured(): void
     {
         $this->assertUrlIsSecured('/admin/permissions/roles/1/view_user/1/asdfasdf', 'POST');
     }
 
-    public function testSavePermissionIsSecuredForRole()
+    public function testSavePermissionIsSecuredForRole(): void
     {
         $this->assertUrlIsSecuredForRole(User::ROLE_ADMIN, '/admin/permissions');
     }
 
-    public function testSavePermission()
+    public function testSavePermission(): void
     {
         $client = $this->getClientForAuthenticatedUser(User::ROLE_SUPER_ADMIN);
         $this->assertAccessIsGranted($client, '/admin/permissions/roles/create');

tests/Entity/RoleTest.php

@@ -22,6 +22,7 @@ class RoleTest extends TestCase
         $sut = new Role();
         self::assertNull($sut->getId());
         self::assertNull($sut->getName());
+        self::assertFalse($sut->isUser());
     }
 
     public function testSetterAndGetter(): void
@@ -30,8 +31,13 @@ class RoleTest extends TestCase
 
         $sut->setName('foo');
         self::assertEquals('FOO', $sut->getName());
+        self::assertFalse($sut->isUser());
 
         $sut->setName('BAR');
         self::assertEquals('BAR', $sut->getName());
+        self::assertFalse($sut->isUser());
+
+        $sut->setName('ROLE_USER');
+        self::assertTrue($sut->isUser());
     }
 }

tests/phpstan.neon

@@ -2572,66 +2572,6 @@ parameters:
             count: 1
             path: Controller/PermissionControllerTest.php
 
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testCreateRole\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testCreateRoleIsSecured\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testCreateRoleIsSecuredForRole\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testDeleteRole\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testDeleteRoleIsSecured\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testDeleteRoleIsSecuredForRole\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testIsSecure\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testIsSecureForRole\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testPermissions\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testSavePermission\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testSavePermissionIsSecured\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
-        -
-            message: "#^Method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:testSavePermissionIsSecuredForRole\\(\\) has no return type specified\\.$#"
-            count: 1
-            path: Controller/PermissionControllerTest.php
-
         -
             message: "#^Parameter \\#1 \\$content of method App\\\\Tests\\\\Controller\\\\PermissionControllerTest\\:\\:assertTableHeader\\(\\) expects string, string\\|false given\\.$#"
             count: 2