fix: role form sluggishness (#5888)

This seems to improve the performance in the role form while still maintaining the same validation logic. A big factor was the memoization of the categories calculation and respective elements, which is especially impactful when there are many environments.
2025-06-09 01:17:06 +02:00 · 2024-01-15 08:37:53 +00:00 · 2024-01-15 08:37:53 +00:00 · 22acadf4cc
commit 22acadf4cc
parent ebd673e9fc
5 changed files with 137 additions and 106 deletions
--- a/frontend/src/component/admin/roles/RoleForm/RoleForm.tsx
+++ b/frontend/src/component/admin/roles/RoleForm/RoleForm.tsx
@ -1,29 +1,11 @@
 import { Alert, styled } from '@mui/material';
 import Input from 'component/common/Input/Input';
-import { PermissionAccordion } from './PermissionAccordion/PermissionAccordion';
-import {
-    Person as UserIcon,
-    Topic as TopicIcon,
-    CloudCircle as CloudCircleIcon,
-} from '@mui/icons-material';
-import { ICheckedPermissions, IPermission } from 'interfaces/permissions';
+import { ICheckedPermissions } from 'interfaces/permissions';
 import { IRoleFormErrors } from './useRoleForm';
-import {
-    flattenProjectPermissions,
-    getCategorizedProjectPermissions,
-    getCategorizedRootPermissions,
-    toggleAllPermissions,
-    togglePermission,
-} from 'utils/permissions';
-import usePermissions from 'hooks/api/getters/usePermissions/usePermissions';
 import { PredefinedRoleType } from 'interfaces/role';
-import {
-    ENVIRONMENT_PERMISSION_TYPE,
-    PROJECT_PERMISSION_TYPE,
-    PROJECT_ROLE_TYPES,
-    ROOT_ROLE_TYPE,
-} from '@server/util/constants';
+import { ROOT_ROLE_TYPE } from '@server/util/constants';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
+import { RolePermissionCategories } from './RolePermissionCategories/RolePermissionCategories';

 const StyledInputDescription = styled('p')(({ theme }) => ({
    display: 'flex',
@ -55,6 +37,7 @@ interface IRoleFormProps {
    setCheckedPermissions: React.Dispatch<
        React.SetStateAction<ICheckedPermissions>
    >;
+    validatePermissions: (permissions: ICheckedPermissions) => boolean;
    errors: IRoleFormErrors;
    showErrors: boolean;
 }
@ -71,41 +54,8 @@ export const RoleForm = ({
    showErrors,
    validateName,
    validateDescription,
+    validatePermissions,
 }: IRoleFormProps) => {
-    const { permissions } = usePermissions({
-        revalidateIfStale: false,
-        revalidateOnReconnect: false,
-        revalidateOnFocus: false,
-    });
-
-    const isProjectRole = PROJECT_ROLE_TYPES.includes(type);
-
-    const categories = isProjectRole
-        ? getCategorizedProjectPermissions(
-              flattenProjectPermissions(
-                  permissions.project,
-                  permissions.environments,
-              ),
-          )
-        : getCategorizedRootPermissions(permissions.root);
-
-    const onPermissionChange = (permission: IPermission) => {
-        const newCheckedPermissions = togglePermission(
-            checkedPermissions,
-            permission,
-        );
-        setCheckedPermissions(newCheckedPermissions);
-    };
-
-    const onCheckAll = (permissions: IPermission[]) => {
-        const newCheckedPermissions = toggleAllPermissions(
-            checkedPermissions,
-            permissions,
-        );
-
-        setCheckedPermissions(newCheckedPermissions);
-    };
-
    const handleOnBlur = (callback: Function) => {
        setTimeout(() => callback(), 300);
    };
@ -121,7 +71,10 @@ export const RoleForm = ({
                error={Boolean(errors.name)}
                errorText={errors.name}
                value={name}
-                onChange={(e) => setName(e.target.value)}
+                onChange={(e) => {
+                    validateName(e.target.value);
+                    setName(e.target.value);
+                }}
                onBlur={(e) => handleOnBlur(() => validateName(e.target.value))}
                autoComplete='off'
            />
@ -133,7 +86,10 @@ export const RoleForm = ({
                error={Boolean(errors.description)}
                errorText={errors.description}
                value={description}
-                onChange={(e) => setDescription(e.target.value)}
+                onChange={(e) => {
+                    validateDescription(e.target.value);
+                    setDescription(e.target.value);
+                }}
                onBlur={(e) =>
                    handleOnBlur(() => validateDescription(e.target.value))
                }
@ -145,28 +101,12 @@ export const RoleForm = ({
            <Alert severity='info'>
                You must select at least one permission.
            </Alert>
-            {categories.map(({ label, type, permissions }) => (
-                <PermissionAccordion
-                    key={label}
-                    title={`${label} permissions`}
-                    context={label.toLowerCase()}
-                    Icon={
-                        type === PROJECT_PERMISSION_TYPE ? (
-                            <TopicIcon color='disabled' sx={{ mr: 1 }} />
-                        ) : type === ENVIRONMENT_PERMISSION_TYPE ? (
-                            <CloudCircleIcon color='disabled' sx={{ mr: 1 }} />
-                        ) : (
-                            <UserIcon color='disabled' sx={{ mr: 1 }} />
-                        )
-                    }
-                    permissions={permissions}
-                    checkedPermissions={checkedPermissions}
-                    onPermissionChange={(permission: IPermission) =>
-                        onPermissionChange(permission)
-                    }
-                    onCheckAll={() => onCheckAll(permissions)}
-                />
-            ))}
+            <RolePermissionCategories
+                type={type}
+                checkedPermissions={checkedPermissions}
+                setCheckedPermissions={setCheckedPermissions}
+                validatePermissions={validatePermissions}
+            />
            <ConditionallyRender
                condition={showErrors}
                show={() => (
--- a/frontend/src/component/admin/roles/RoleForm/RolePermissionCategories/RolePermissionCategories.tsx
+++ b/frontend/src/component/admin/roles/RoleForm/RolePermissionCategories/RolePermissionCategories.tsx
@ -0,0 +1,110 @@
+import {
+    Person as UserIcon,
+    Topic as TopicIcon,
+    CloudCircle as CloudCircleIcon,
+} from '@mui/icons-material';
+import {
+    ENVIRONMENT_PERMISSION_TYPE,
+    PROJECT_PERMISSION_TYPE,
+    PROJECT_ROLE_TYPES,
+} from '@server/util/constants';
+import usePermissions from 'hooks/api/getters/usePermissions/usePermissions';
+import { ICheckedPermissions, IPermission } from 'interfaces/permissions';
+import { PredefinedRoleType } from 'interfaces/role';
+import {
+    flattenProjectPermissions,
+    getCategorizedProjectPermissions,
+    getCategorizedRootPermissions,
+    toggleAllPermissions,
+    togglePermission,
+} from 'utils/permissions';
+import { RolePermissionCategory } from './RolePermissionCategory';
+import { useMemo } from 'react';
+
+interface IPermissionCategoriesProps {
+    type: PredefinedRoleType;
+    checkedPermissions: ICheckedPermissions;
+    setCheckedPermissions: React.Dispatch<
+        React.SetStateAction<ICheckedPermissions>
+    >;
+    validatePermissions: (permissions: ICheckedPermissions) => boolean;
+}
+
+export const RolePermissionCategories = ({
+    type,
+    checkedPermissions,
+    setCheckedPermissions,
+    validatePermissions,
+}: IPermissionCategoriesProps) => {
+    const { permissions } = usePermissions({
+        revalidateIfStale: false,
+        revalidateOnReconnect: false,
+        revalidateOnFocus: false,
+    });
+
+    const isProjectRole = PROJECT_ROLE_TYPES.includes(type);
+
+    const categories = useMemo(
+        () =>
+            isProjectRole
+                ? getCategorizedProjectPermissions(
+                      flattenProjectPermissions(
+                          permissions.project,
+                          permissions.environments,
+                      ),
+                  )
+                : getCategorizedRootPermissions(permissions.root),
+        [permissions, isProjectRole],
+    );
+
+    const onPermissionChange = (permission: IPermission) => {
+        const newCheckedPermissions = togglePermission(
+            checkedPermissions,
+            permission,
+        );
+        validatePermissions(newCheckedPermissions);
+        setCheckedPermissions(newCheckedPermissions);
+    };
+
+    const onCheckAll = (permissions: IPermission[]) => {
+        const newCheckedPermissions = toggleAllPermissions(
+            checkedPermissions,
+            permissions,
+        );
+        validatePermissions(newCheckedPermissions);
+        setCheckedPermissions(newCheckedPermissions);
+    };
+
+    return useMemo(
+        () => (
+            <>
+                {categories.map(({ label, type, permissions }) => (
+                    <RolePermissionCategory
+                        key={label}
+                        title={`${label} permissions`}
+                        context={label.toLowerCase()}
+                        Icon={
+                            type === PROJECT_PERMISSION_TYPE ? (
+                                <TopicIcon color='disabled' sx={{ mr: 1 }} />
+                            ) : type === ENVIRONMENT_PERMISSION_TYPE ? (
+                                <CloudCircleIcon
+                                    color='disabled'
+                                    sx={{ mr: 1 }}
+                                />
+                            ) : (
+                                <UserIcon color='disabled' sx={{ mr: 1 }} />
+                            )
+                        }
+                        permissions={permissions}
+                        checkedPermissions={checkedPermissions}
+                        onPermissionChange={(permission: IPermission) =>
+                            onPermissionChange(permission)
+                        }
+                        onCheckAll={() => onCheckAll(permissions)}
+                    />
+                ))}
+            </>
+        ),
+        [categories, checkedPermissions],
+    );
+};
--- a/frontend/src/component/admin/roles/RoleForm/RolePermissionCategories/RolePermissionCategory.tsx
+++ b/frontend/src/component/admin/roles/RoleForm/RolePermissionCategories/RolePermissionCategory.tsx
@ -1,4 +1,4 @@
-import { ReactNode, useMemo, useState, VFC } from 'react';
+import { ReactNode, useMemo, useState } from 'react';
 import {
    Accordion,
    AccordionDetails,
@ -42,7 +42,7 @@ const StyledTitle = styled(StringTruncator)(({ theme }) => ({
    marginRight: theme.spacing(1),
 }));

-export const PermissionAccordion: VFC<IEnvironmentPermissionAccordionProps> = ({
+export const RolePermissionCategory = ({
    title,
    permissions,
    checkedPermissions,
@ -51,7 +51,7 @@ export const PermissionAccordion: VFC<IEnvironmentPermissionAccordionProps> = ({
    context,
    onPermissionChange,
    onCheckAll,
-}) => {
+}: IEnvironmentPermissionAccordionProps) => {
    const [expanded, setExpanded] = useState(isInitiallyExpanded);
    const permissionMap = useMemo(
        () =>
--- a/frontend/src/component/admin/roles/RoleForm/useRoleForm.ts
+++ b/frontend/src/component/admin/roles/RoleForm/useRoleForm.ts
@ -24,7 +24,7 @@ export const useRoleForm = (
    initialDescription = '',
    initialPermissions: IPermission[] = [],
 ) => {
-    const { roles } = useRoles();
+    const { roles, projectRoles } = useRoles();

    const [name, setName] = useState(initialName);
    const [description, setDescription] = useState(initialDescription);
@ -47,28 +47,6 @@ export const useRoleForm = (
        setCheckedPermissions(newCheckedPermissions);
    }, [initialPermissions.length]);

-    useEffect(() => {
-        if (name !== '') {
-            validateName(name);
-        } else {
-            clearError(ErrorField.NAME);
-        }
-    }, [name]);
-
-    useEffect(() => {
-        if (description !== '') {
-            validateDescription(description);
-        } else {
-            clearError(ErrorField.DESCRIPTION);
-        }
-    }, [description]);
-
-    useEffect(() => {
-        if (validated) {
-            validatePermissions(checkedPermissions);
-        }
-    }, [checkedPermissions]);
-
    const getRolePayload = (type: PredefinedRoleType = ROOT_ROLE_TYPE) => ({
        name,
        description,
@ -79,7 +57,7 @@ export const useRoleForm = (
    });

    const isNameUnique = (name: string) => {
-        return !roles.some(
+        return ![...roles, ...projectRoles].some(
            (existingRole: IRole) =>
                existingRole.name !== initialName &&
                existingRole.name.toLowerCase() === name.toLowerCase(),
@ -168,6 +146,7 @@ export const useRoleForm = (
        validateDescription,
        checkedPermissions,
        setCheckedPermissions,
+        validatePermissions,
        getRolePayload,
        errors,
        showErrors,
--- a/frontend/src/component/admin/roles/RoleModal/RoleModal.tsx
+++ b/frontend/src/component/admin/roles/RoleModal/RoleModal.tsx
@ -54,6 +54,7 @@ export const RoleModal = ({
        validateDescription,
        checkedPermissions,
        setCheckedPermissions,
+        validatePermissions,
        getRolePayload,
        errors,
        showErrors,
@ -143,6 +144,7 @@ export const RoleModal = ({
                        validateDescription={validateDescription}
                        checkedPermissions={checkedPermissions}
                        setCheckedPermissions={setCheckedPermissions}
+                        validatePermissions={validatePermissions}
                        errors={errors}
                        showErrors={showErrors}
                    />