From 7885de36a96d8c76b11f691d820e289fd62bb168 Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 11 Oct 2024 15:18:24 +0100 Subject: [PATCH 1/3] fix: add isCreator role by default when creating new role --- .../PermissionSettings.ee.jsx | 3 -- .../PermissionCatalogField/index.ee.jsx | 8 +---- .../web/src/helpers/computePermissions.ee.js | 33 +++++++++++++++++++ .../web/src/pages/CreateRole/index.ee.jsx | 28 ++++++++++++---- packages/web/src/pages/EditRole/index.ee.jsx | 28 +++++++++++++--- 5 files changed, 79 insertions(+), 21 deletions(-) diff --git a/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx b/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx index 14237a20..625c6daa 100644 --- a/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx +++ b/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx @@ -25,7 +25,6 @@ function PermissionSettings(props) { subject, actions, conditions, - defaultChecked, } = props; const formatMessage = useFormatMessage(); const { getValues, resetField } = useFormContext(); @@ -106,7 +105,6 @@ function PermissionSettings(props) { dataTest={`${ condition.key }-${action.key.toLowerCase()}-checkbox`} - defaultValue={defaultChecked} disabled={ getValues( `${fieldPrefix}.${action.key}.value`, @@ -144,7 +142,6 @@ PermissionSettings.propTypes = { fieldPrefix: PropTypes.string.isRequired, subject: PropTypes.string.isRequired, open: PropTypes.bool, - defaultChecked: PropTypes.bool, actions: PropTypes.arrayOf( PropTypes.shape({ label: PropTypes.string, diff --git a/packages/web/src/components/PermissionCatalogField/index.ee.jsx b/packages/web/src/components/PermissionCatalogField/index.ee.jsx index 92bcb1a5..fe8f42fa 100644 --- a/packages/web/src/components/PermissionCatalogField/index.ee.jsx +++ b/packages/web/src/components/PermissionCatalogField/index.ee.jsx @@ -17,11 +17,7 @@ import usePermissionCatalog from 'hooks/usePermissionCatalog.ee'; import PermissionSettings from './PermissionSettings.ee'; import PermissionCatalogFieldLoader from './PermissionCatalogFieldLoader'; -const PermissionCatalogField = ({ - name = 'permissions', - disabled = false, - defaultChecked = false, -}) => { +const PermissionCatalogField = ({ name = 'permissions', disabled = false }) => { const { data, isLoading: isPermissionCatalogLoading } = usePermissionCatalog(); const permissionCatalog = data?.data; @@ -100,7 +96,6 @@ const PermissionCatalogField = ({ subject={subject.key} actions={permissionCatalog?.actions} conditions={permissionCatalog?.conditions} - defaultChecked={defaultChecked} /> @@ -114,7 +109,6 @@ const PermissionCatalogField = ({ PermissionCatalogField.propTypes = { name: PropTypes.string, disabled: PropTypes.bool, - defaultChecked: PropTypes.bool, }; export default PermissionCatalogField; diff --git a/packages/web/src/helpers/computePermissions.ee.js b/packages/web/src/helpers/computePermissions.ee.js index 7c6ae20e..3bcfa521 100644 --- a/packages/web/src/helpers/computePermissions.ee.js +++ b/packages/web/src/helpers/computePermissions.ee.js @@ -45,3 +45,36 @@ export function getPermissions(computedPermissions) { [], ); } + +export const getComputedPermissionsDefaultValues = ( + data, + conditionsInitialValues, +) => { + if (!data) return {}; + + const conditions = {}; + data.conditions.forEach((condition) => { + conditions[condition.key] = + conditionsInitialValues?.[condition.key] || false; + }); + + const result = {}; + + data.subjects.forEach((subject) => { + const subjectKey = subject.key; + result[subjectKey] = {}; + + data.actions.forEach((action) => { + const actionKey = action.key; + + if (action.subjects.includes(subjectKey)) { + result[subjectKey][actionKey] = { + value: false, + conditions: { ...conditions }, + }; + } + }); + }); + + return result; +}; diff --git a/packages/web/src/pages/CreateRole/index.ee.jsx b/packages/web/src/pages/CreateRole/index.ee.jsx index a7767e76..b5ff22c9 100644 --- a/packages/web/src/pages/CreateRole/index.ee.jsx +++ b/packages/web/src/pages/CreateRole/index.ee.jsx @@ -11,9 +11,13 @@ import Form from 'components/Form'; import PageTitle from 'components/PageTitle'; import TextField from 'components/TextField'; import * as URLS from 'config/urls'; -import { getPermissions } from 'helpers/computePermissions.ee'; +import { + getComputedPermissionsDefaultValues, + getPermissions, +} from 'helpers/computePermissions.ee'; import useFormatMessage from 'hooks/useFormatMessage'; import useAdminCreateRole from 'hooks/useAdminCreateRole'; +import usePermissionCatalog from 'hooks/usePermissionCatalog.ee'; export default function CreateRole() { const navigate = useNavigate(); @@ -21,6 +25,21 @@ export default function CreateRole() { const enqueueSnackbar = useEnqueueSnackbar(); const { mutateAsync: createRole, isPending: isCreateRolePending } = useAdminCreateRole(); + const { data: permissionCatalogData } = usePermissionCatalog(); + + const defaultValues = React.useMemo( + () => ({ + name: '', + description: '', + computedPermissions: getComputedPermissionsDefaultValues( + permissionCatalogData?.data, + { + isCreator: true, + }, + ), + }), + [permissionCatalogData], + ); const handleRoleCreation = async (roleData) => { try { @@ -64,7 +83,7 @@ export default function CreateRole() { -
+ - + getComputedPermissionsDefaultValues(permissionCatalogData?.data), + [permissionCatalogData], + ); + + const defaultValues = React.useMemo( + () => ({ + ...roleWithComputedPermissions, + computedPermissions: merge( + {}, + computedPermissionsDefaultValues, + roleWithComputedPermissions.computedPermissions, + ), + }), + [roleWithComputedPermissions, computedPermissionsDefaultValues], + ); + return ( @@ -64,10 +85,7 @@ export default function EditRole() { - + {isRoleLoading && ( <> From 4696a03db1658c4b2b64a0208b35f20c45ec4ffe Mon Sep 17 00:00:00 2001 From: "kasia.oczkowska" Date: Fri, 18 Oct 2024 11:22:29 +0100 Subject: [PATCH 2/3] feat: sync isCreator value when editing role settings --- .../PermissionCatalogField/ActionField.jsx | 60 +++++++++++++++++++ .../PermissionSettings.ee.jsx | 11 ++-- .../PermissionCatalogField/index.ee.jsx | 32 +++++----- packages/web/src/pages/EditRole/index.ee.jsx | 1 + 4 files changed, 85 insertions(+), 19 deletions(-) create mode 100644 packages/web/src/components/PermissionCatalogField/ActionField.jsx diff --git a/packages/web/src/components/PermissionCatalogField/ActionField.jsx b/packages/web/src/components/PermissionCatalogField/ActionField.jsx new file mode 100644 index 00000000..43d86de8 --- /dev/null +++ b/packages/web/src/components/PermissionCatalogField/ActionField.jsx @@ -0,0 +1,60 @@ +import React, { useEffect, useRef } from 'react'; +import { useFormContext } from 'react-hook-form'; +import { Typography } from '@mui/material'; +import PropTypes from 'prop-types'; +import ControlledCheckbox from 'components/ControlledCheckbox'; + +const ActionField = ({ action, subject, disabled, name, syncIsCreator }) => { + const { watch, formState, resetField } = useFormContext(); + + const actionFieldName = `${name}.${subject.key}.${action.key}.value`; + const actionFieldValue = watch(actionFieldName); + + const conditionFieldName = `${name}.${subject.key}.${action.key}.conditions.isCreator`; + const conditionFieldTouched = + formState.touchedFields?.[name]?.[subject.key]?.[action.key]?.conditions + ?.isCreator === true; + + const defaultActionFieldValueRef = useRef(actionFieldValue); + + useEffect(() => { + if (defaultActionFieldValueRef?.current === undefined) { + defaultActionFieldValueRef.current = actionFieldValue; + } else if ( + syncIsCreator && + defaultActionFieldValueRef?.current === false && + !conditionFieldTouched + ) { + resetField(conditionFieldName, { defaultValue: actionFieldValue }); + } + }, [actionFieldValue, syncIsCreator]); + + return ( + + {action.subjects.includes(subject.key) && ( + + )} + + {!action.subjects.includes(subject.key) && '-'} + + ); +}; + +ActionField.propTypes = { + action: PropTypes.shape({ + key: PropTypes.string.isRequired, + subjects: PropTypes.arrayOf(PropTypes.string).isRequired, + }), + subject: PropTypes.shape({ + key: PropTypes.string.isRequired, + }).isRequired, + disabled: PropTypes.bool, + name: PropTypes.string.isRequired, + syncIsCreator: PropTypes.bool, +}; + +export default ActionField; diff --git a/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx b/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx index 625c6daa..71cc4691 100644 --- a/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx +++ b/packages/web/src/components/PermissionCatalogField/PermissionSettings.ee.jsx @@ -33,7 +33,7 @@ function PermissionSettings(props) { for (const action of actions) { for (const condition of conditions) { const fieldName = `${fieldPrefix}.${action.key}.conditions.${condition.key}`; - resetField(fieldName); + resetField(fieldName, { keepTouched: true }); } } onClose(); @@ -44,7 +44,7 @@ function PermissionSettings(props) { for (const condition of conditions) { const fieldName = `${fieldPrefix}.${action.key}.conditions.${condition.key}`; const value = getValues(fieldName); - resetField(fieldName, { defaultValue: value }); + resetField(fieldName, { defaultValue: value, keepTouched: true }); } } onClose(); @@ -55,6 +55,7 @@ function PermissionSettings(props) { open={open} onClose={cancel} data-test={`${subject}-role-conditions-modal`} + keepMounted > {formatMessage('permissionSettings.title')} @@ -64,10 +65,10 @@ function PermissionSettings(props) { - {actions.map((action) => ( - + {condition.label} @@ -98,7 +99,7 @@ function PermissionSettings(props) { key={`${action.key}.${condition.key}`} align="center" > - + {action.subjects.includes(subject) && ( { +const PermissionCatalogField = ({ + name = 'permissions', + disabled = false, + syncIsCreator = false, +}) => { const { data, isLoading: isPermissionCatalogLoading } = usePermissionCatalog(); const permissionCatalog = data?.data; @@ -35,6 +39,7 @@ const PermissionCatalogField = ({ name = 'permissions', disabled = false }) => { {permissionCatalog?.actions.map((action) => ( { data-test={`${subject.key}-permission-row`} > - {subject.label} + + {subject.label} + {permissionCatalog?.actions.map((action) => ( - - {action.subjects.includes(subject.key) && ( - - )} - - {!action.subjects.includes(subject.key) && '-'} - + ))} @@ -109,6 +112,7 @@ const PermissionCatalogField = ({ name = 'permissions', disabled = false }) => { PermissionCatalogField.propTypes = { name: PropTypes.string, disabled: PropTypes.bool, + syncIsCreator: PropTypes.bool, }; export default PermissionCatalogField; diff --git a/packages/web/src/pages/EditRole/index.ee.jsx b/packages/web/src/pages/EditRole/index.ee.jsx index 42832080..29e0e584 100644 --- a/packages/web/src/pages/EditRole/index.ee.jsx +++ b/packages/web/src/pages/EditRole/index.ee.jsx @@ -117,6 +117,7 @@ export default function EditRole() { Date: Thu, 24 Oct 2024 15:18:57 +0100 Subject: [PATCH 3/3] feat: improve syncing isCreator value --- .../PermissionCatalogField/ActionField.jsx | 43 ++++++++----------- .../PermissionCatalogField/index.ee.jsx | 19 +++++--- packages/web/src/pages/EditRole/index.ee.jsx | 25 +++++------ 3 files changed, 39 insertions(+), 48 deletions(-) diff --git a/packages/web/src/components/PermissionCatalogField/ActionField.jsx b/packages/web/src/components/PermissionCatalogField/ActionField.jsx index 43d86de8..3bf2dde6 100644 --- a/packages/web/src/components/PermissionCatalogField/ActionField.jsx +++ b/packages/web/src/components/PermissionCatalogField/ActionField.jsx @@ -1,46 +1,37 @@ -import React, { useEffect, useRef } from 'react'; +import React from 'react'; import { useFormContext } from 'react-hook-form'; -import { Typography } from '@mui/material'; import PropTypes from 'prop-types'; import ControlledCheckbox from 'components/ControlledCheckbox'; const ActionField = ({ action, subject, disabled, name, syncIsCreator }) => { - const { watch, formState, resetField } = useFormContext(); - - const actionFieldName = `${name}.${subject.key}.${action.key}.value`; - const actionFieldValue = watch(actionFieldName); + const { formState, resetField } = useFormContext(); + const actionDefaultValue = + formState.defaultValues?.[name]?.[subject.key]?.[action.key].value; const conditionFieldName = `${name}.${subject.key}.${action.key}.conditions.isCreator`; const conditionFieldTouched = formState.touchedFields?.[name]?.[subject.key]?.[action.key]?.conditions ?.isCreator === true; - const defaultActionFieldValueRef = useRef(actionFieldValue); - - useEffect(() => { - if (defaultActionFieldValueRef?.current === undefined) { - defaultActionFieldValueRef.current = actionFieldValue; - } else if ( + const handleSyncIsCreator = (newValue) => { + if ( syncIsCreator && - defaultActionFieldValueRef?.current === false && + actionDefaultValue === false && !conditionFieldTouched ) { - resetField(conditionFieldName, { defaultValue: actionFieldValue }); + resetField(conditionFieldName, { defaultValue: newValue }); } - }, [actionFieldValue, syncIsCreator]); + }; return ( - - {action.subjects.includes(subject.key) && ( - - )} - - {!action.subjects.includes(subject.key) && '-'} - + { + handleSyncIsCreator(value); + }} + /> ); }; diff --git a/packages/web/src/components/PermissionCatalogField/index.ee.jsx b/packages/web/src/components/PermissionCatalogField/index.ee.jsx index 3846ada5..5f2bf909 100644 --- a/packages/web/src/components/PermissionCatalogField/index.ee.jsx +++ b/packages/web/src/components/PermissionCatalogField/index.ee.jsx @@ -70,13 +70,18 @@ const PermissionCatalogField = ({ {permissionCatalog?.actions.map((action) => ( - + + {action.subjects.includes(subject.key) && ( + + )} + {!action.subjects.includes(subject.key) && '-'} + ))} diff --git a/packages/web/src/pages/EditRole/index.ee.jsx b/packages/web/src/pages/EditRole/index.ee.jsx index 29e0e584..1ed4881c 100644 --- a/packages/web/src/pages/EditRole/index.ee.jsx +++ b/packages/web/src/pages/EditRole/index.ee.jsx @@ -27,11 +27,12 @@ export default function EditRole() { const formatMessage = useFormatMessage(); const navigate = useNavigate(); const { roleId } = useParams(); - const { data, isLoading: isRoleLoading } = useRole({ roleId }); + const { data: roleData, isLoading: isRoleLoading } = useRole({ roleId }); const { mutateAsync: updateRole, isPending: isUpdateRolePending } = useAdminUpdateRole(roleId); const { data: permissionCatalogData } = usePermissionCatalog(); - const role = data?.data; + const role = roleData?.data; + const permissionCatalog = permissionCatalogData?.data; const enqueueSnackbar = useEnqueueSnackbar(); const handleRoleUpdate = async (roleData) => { @@ -56,24 +57,20 @@ export default function EditRole() { } }; - const roleWithComputedPermissions = getRoleWithComputedPermissions(role); + const defaultValues = React.useMemo(() => { + const roleWithComputedPermissions = getRoleWithComputedPermissions(role); + const computedPermissionsDefaultValues = + getComputedPermissionsDefaultValues(permissionCatalog); - const computedPermissionsDefaultValues = React.useMemo( - () => getComputedPermissionsDefaultValues(permissionCatalogData?.data), - [permissionCatalogData], - ); - - const defaultValues = React.useMemo( - () => ({ + return { ...roleWithComputedPermissions, computedPermissions: merge( {}, computedPermissionsDefaultValues, roleWithComputedPermissions.computedPermissions, ), - }), - [roleWithComputedPermissions, computedPermissionsDefaultValues], - ); + }; + }, [role, permissionCatalog]); return ( @@ -113,13 +110,11 @@ export default function EditRole() { /> )} - -