From ae1fcdecf3d3de97c1d6fbddbeec7e89191e1aea Mon Sep 17 00:00:00 2001 From: soupette Date: Mon, 31 Aug 2020 10:17:12 +0200 Subject: [PATCH] Fix admin roles bugs. Wrong redirection after creating a role, reset and submit logic Signed-off-by: soupette --- .../src/components/Roles/Permissions/index.js | 11 +++++++++-- .../src/components/Roles/Permissions/init.js | 1 + .../src/components/Roles/Permissions/reducer.js | 11 +++++++++++ .../Roles/Permissions/tests/init.test.js | 6 ++++++ .../admin/src/containers/Roles/EditPage/index.js | 15 +++++++++++++-- .../admin/src/hooks/useFetchRole/index.js | 11 +++++++++-- .../admin/src/hooks/useFetchRole/reducer.js | 5 +++++ .../ee/admin/containers/Roles/CreatePage/index.js | 11 ++++++----- .../tests/__snapshots__/index.test.js.snap | 8 ++++++-- 9 files changed, 66 insertions(+), 13 deletions(-) diff --git a/packages/strapi-admin/admin/src/components/Roles/Permissions/index.js b/packages/strapi-admin/admin/src/components/Roles/Permissions/index.js index 193da905a3..a05c948a0b 100644 --- a/packages/strapi-admin/admin/src/components/Roles/Permissions/index.js +++ b/packages/strapi-admin/admin/src/components/Roles/Permissions/index.js @@ -1,4 +1,4 @@ -import React, { useReducer, forwardRef, useMemo, useImperativeHandle } from 'react'; +import React, { memo, useReducer, forwardRef, useMemo, useImperativeHandle } from 'react'; import PropTypes from 'prop-types'; import Tabs from '../Tabs'; @@ -24,6 +24,12 @@ const Permissions = forwardRef(({ role, permissionsLayout, rolePermissions }, re pluginsAndSettingsPermissions: state.pluginsAndSettingsPermissions, }; }, + resetForm: () => { + dispatch({ type: 'ON_RESET' }); + }, + setFormAfterSubmit: () => { + dispatch({ type: 'ON_SUBMIT_SUCCEEDED' }); + }, })); const allSingleTypesAttributes = useMemo(() => { @@ -211,4 +217,5 @@ Permissions.propTypes = { rolePermissions: PropTypes.object, role: PropTypes.object, }; -export default Permissions; + +export default memo(Permissions); diff --git a/packages/strapi-admin/admin/src/components/Roles/Permissions/init.js b/packages/strapi-admin/admin/src/components/Roles/Permissions/init.js index 0703f74a4c..825c519c01 100644 --- a/packages/strapi-admin/admin/src/components/Roles/Permissions/init.js +++ b/packages/strapi-admin/admin/src/components/Roles/Permissions/init.js @@ -3,6 +3,7 @@ const init = (state, permissionsLayout, permissions, role) => { ...state, ...permissions, permissionsLayout, + initialData: permissions, isSuperAdmin: role && role.code === 'strapi-super-admin', }; }; diff --git a/packages/strapi-admin/admin/src/components/Roles/Permissions/reducer.js b/packages/strapi-admin/admin/src/components/Roles/Permissions/reducer.js index ef3ef4831f..973bbc634a 100644 --- a/packages/strapi-admin/admin/src/components/Roles/Permissions/reducer.js +++ b/packages/strapi-admin/admin/src/components/Roles/Permissions/reducer.js @@ -11,6 +11,7 @@ export const initialState = { permissionsLayout: {}, contentTypesPermissions: {}, pluginsAndSettingsPermissions: [], + initialData: {}, isSuperAdmin: false, }; @@ -455,6 +456,16 @@ const reducer = (state, action) => break; } + case 'ON_RESET': { + draftState.contentTypesPermissions = state.initialData.contentTypesPermissions; + draftState.pluginsAndSettingsPermissions = state.initialData.pluginsAndSettingsPermissions; + break; + } + case 'ON_SUBMIT_SUCCEEDED': { + draftState.initialData.contentTypesPermissions = state.contentTypesPermissions; + draftState.initialData.pluginsAndSettingsPermissions = state.pluginsAndSettingsPermissions; + break; + } default: return draftState; } diff --git a/packages/strapi-admin/admin/src/components/Roles/Permissions/tests/init.test.js b/packages/strapi-admin/admin/src/components/Roles/Permissions/tests/init.test.js index 70b91169c5..09386023ee 100644 --- a/packages/strapi-admin/admin/src/components/Roles/Permissions/tests/init.test.js +++ b/packages/strapi-admin/admin/src/components/Roles/Permissions/tests/init.test.js @@ -7,6 +7,7 @@ describe('ADMIN | COMPONENTS | ROLE | init', () => { contentTypesPermissions: {}, pluginsAndSettingsPermissions: [], permissionsLayout: {}, + initialData: {}, isSuperAdmin: false, }; @@ -38,6 +39,11 @@ describe('ADMIN | COMPONENTS | ROLE | init', () => { }, }, isSuperAdmin: true, + initialData: { + contentTypesPermissions: { + firstPermission: true, + }, + }, }; expect(init(initialState, permissionsLayout, permissions, role)).toEqual(expected); diff --git a/packages/strapi-admin/admin/src/containers/Roles/EditPage/index.js b/packages/strapi-admin/admin/src/containers/Roles/EditPage/index.js index 03b6391e8b..9969ca7ef0 100644 --- a/packages/strapi-admin/admin/src/containers/Roles/EditPage/index.js +++ b/packages/strapi-admin/admin/src/containers/Roles/EditPage/index.js @@ -24,7 +24,12 @@ const EditPage = () => { const permissionsRef = useRef(); const { isLoading: isLayoutLoading, data: permissionsLayout } = useFetchPermissionsLayout(id); - const { role, permissions: rolePermissions, isLoading: isRoleLoading } = useFetchRole(id); + const { + role, + permissions: rolePermissions, + isLoading: isRoleLoading, + onSubmitSucceeded, + } = useFetchRole(id); /* eslint-disable indent */ const headerActions = (handleSubmit, handleReset) => @@ -37,7 +42,10 @@ const EditPage = () => { defaultMessage: 'Reset', }), disabled: role.code === 'strapi-super-admin', - onClick: handleReset, + onClick: () => { + handleReset(); + permissionsRef.current.resetForm(); + }, color: 'cancel', type: 'button', }, @@ -95,6 +103,9 @@ const EditPage = () => { } } + permissionsRef.current.setFormAfterSubmit(); + onSubmitSucceeded({ name: data.name, description: data.description }); + strapi.notification.success('notification.success.saved'); } catch (err) { console.error(err.response); diff --git a/packages/strapi-admin/admin/src/hooks/useFetchRole/index.js b/packages/strapi-admin/admin/src/hooks/useFetchRole/index.js index bfe8bd54af..532600d937 100644 --- a/packages/strapi-admin/admin/src/hooks/useFetchRole/index.js +++ b/packages/strapi-admin/admin/src/hooks/useFetchRole/index.js @@ -1,4 +1,4 @@ -import { useReducer, useEffect } from 'react'; +import { useCallback, useReducer, useEffect } from 'react'; import { request } from 'strapi-helper-plugin'; import reducer, { initialState } from './reducer'; @@ -44,7 +44,14 @@ const useFetchRole = id => { } }; - return state; + const handleSubmitSucceeded = useCallback(data => { + dispatch({ + type: 'ON_SUBMIT_SUCCEEDED', + ...data, + }); + }, []); + + return { ...state, onSubmitSucceeded: handleSubmitSucceeded }; }; export default useFetchRole; diff --git a/packages/strapi-admin/admin/src/hooks/useFetchRole/reducer.js b/packages/strapi-admin/admin/src/hooks/useFetchRole/reducer.js index d394a0cc01..84900d28ee 100644 --- a/packages/strapi-admin/admin/src/hooks/useFetchRole/reducer.js +++ b/packages/strapi-admin/admin/src/hooks/useFetchRole/reducer.js @@ -20,6 +20,11 @@ const reducer = (state, action) => draftState.isLoading = false; break; } + case 'ON_SUBMIT_SUCCEEDED': { + draftState.role.name = action.name; + draftState.role.description = action.description; + break; + } default: return draftState; } diff --git a/packages/strapi-admin/ee/admin/containers/Roles/CreatePage/index.js b/packages/strapi-admin/ee/admin/containers/Roles/CreatePage/index.js index 78544ab658..59a09b7d7a 100644 --- a/packages/strapi-admin/ee/admin/containers/Roles/CreatePage/index.js +++ b/packages/strapi-admin/ee/admin/containers/Roles/CreatePage/index.js @@ -22,7 +22,7 @@ import schema from './utils/schema'; const CreatePage = () => { const { formatMessage } = useIntl(); const [isSubmiting, setIsSubmiting] = useState(false); - const { push } = useHistory(); + const { replace } = useHistory(); const permissionsRef = useRef(); const { emitEvent, settingsBaseURL } = useGlobalContext(); const params = useRouteMatch(`${settingsBaseURL}/roles/duplicate/:id`); @@ -68,7 +68,7 @@ const CreatePage = () => { body: data, }) ) - .then(res => { + .then(async res => { const permissionsToSend = permissionsRef.current.getPermissions(); if (id) { @@ -78,7 +78,7 @@ const CreatePage = () => { } if (res.data.id && !isEmpty(permissionsToSend)) { - return request(`/admin/roles/${res.data.id}/permissions`, { + await request(`/admin/roles/${res.data.id}/permissions`, { method: 'PUT', body: { permissions: formatPermissionsToApi(permissionsToSend) }, }); @@ -87,15 +87,16 @@ const CreatePage = () => { return res; }) .then(res => { + setIsSubmiting(false); strapi.notification.success('Settings.roles.created'); - push(`${settingsBaseURL}/roles/${res.data.id}`); + replace(`${settingsBaseURL}/roles/${res.data.id}`); }) .catch(err => { console.error(err); + setIsSubmiting(false); strapi.notification.error('notification.error'); }) .finally(() => { - setIsSubmiting(false); strapi.unlockApp(); }); }; diff --git a/packages/strapi-helper-plugin/lib/src/components/PopUpWarning/tests/__snapshots__/index.test.js.snap b/packages/strapi-helper-plugin/lib/src/components/PopUpWarning/tests/__snapshots__/index.test.js.snap index 9974544ddb..02003915a1 100644 --- a/packages/strapi-helper-plugin/lib/src/components/PopUpWarning/tests/__snapshots__/index.test.js.snap +++ b/packages/strapi-helper-plugin/lib/src/components/PopUpWarning/tests/__snapshots__/index.test.js.snap @@ -124,6 +124,7 @@ exports[` should render properly should match snapshot if onlyCo .c4 { padding: 0; + text-align: center; } .c4 > div { @@ -140,8 +141,9 @@ exports[` should render properly should match snapshot if onlyCo } .c4 > div > p { + width: 100%; line-height: 1.8rem; - margin-bottom: 0; + margin: auto; min-height: 36px; } @@ -360,6 +362,7 @@ exports[` should render properly should match snapshot if onlyCo .c4 { padding: 0; + text-align: center; } .c4 > div { @@ -376,8 +379,9 @@ exports[` should render properly should match snapshot if onlyCo } .c4 > div > p { + width: 100%; line-height: 1.8rem; - margin-bottom: 0; + margin: auto; min-height: 36px; }