From 795bea005b9c22fc782ad68c7d275b5ac2903fda Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 1 Aug 2023 14:28:44 +0200 Subject: [PATCH 1/2] Enhancement: Render permissions for each workflow stage --- .../admin/src/hooks/useAdminRoles/index.js | 2 +- .../useLicenseLimits/useLicenseLimits.js | 12 +- .../pages/ReviewWorkflows/actions/index.js | 39 ++- .../actions/tests/index.test.js | 79 ++++++- .../components/Stages/Stage/Stage.js | 112 ++++++++- .../Stages/Stage/tests/Stage.test.js | 214 ++++++++++++++--- .../components/Stages/tests/Stages.test.js | 88 +++---- .../WorkflowAttributes/WorkflowAttributes.js | 31 +-- .../tests/WorkflowAttributes.test.js | 198 ++++++++-------- .../pages/ReviewWorkflows/constants.js | 4 + .../pages/CreateView/CreateView.js | 89 +++++-- .../pages/EditView/EditView.js | 137 ++++++++--- .../pages/ReviewWorkflows/reducer/index.js | 62 ++--- .../reducer/tests/index.test.js | 222 +++++------------- .../pages/ReviewWorkflows/selectors.js | 45 ++++ .../utils/tests/validateWorkflow.test.js | 75 ++++++ .../ReviewWorkflows/utils/validateWorkflow.js | 27 +++ 17 files changed, 963 insertions(+), 473 deletions(-) create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/selectors.js diff --git a/packages/core/admin/admin/src/hooks/useAdminRoles/index.js b/packages/core/admin/admin/src/hooks/useAdminRoles/index.js index 86ffcda4dc..a0ea4520aa 100644 --- a/packages/core/admin/admin/src/hooks/useAdminRoles/index.js +++ b/packages/core/admin/admin/src/hooks/useAdminRoles/index.js @@ -31,7 +31,7 @@ export const useAdminRoles = (params = {}, queryOptions = {}) => { } return { - roles: roles.sort((a, b) => formatter.compare(a.name, b.name)), + roles: [...roles].sort((a, b) => formatter.compare(a.name, b.name)), error, isError, isLoading, diff --git a/packages/core/admin/ee/admin/hooks/useLicenseLimits/useLicenseLimits.js b/packages/core/admin/ee/admin/hooks/useLicenseLimits/useLicenseLimits.js index 774d49c9dc..60e690c5e3 100644 --- a/packages/core/admin/ee/admin/hooks/useLicenseLimits/useLicenseLimits.js +++ b/packages/core/admin/ee/admin/hooks/useLicenseLimits/useLicenseLimits.js @@ -3,7 +3,7 @@ import * as React from 'react'; import { useFetchClient } from '@strapi/helper-plugin'; import { useQuery } from 'react-query'; -export function useLicenseLimits({ enabled } = { enabled: true }) { +export function useLicenseLimits(queryOptions = {}) { const { get } = useFetchClient(); const { data, isError, isLoading } = useQuery( ['ee', 'license-limit-info'], @@ -15,11 +15,17 @@ export function useLicenseLimits({ enabled } = { enabled: true }) { return data; }, { - enabled, + ...queryOptions, + + // the request is expected to fail sometimes if a user does not + // have permissions + retry: false, } ); - const license = data ?? {}; + const license = React.useMemo(() => { + return data ?? {}; + }, [data]); const getFeature = React.useCallback( (name) => { diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/index.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/index.js index 9ebb777b13..5711ee9924 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/index.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/index.js @@ -2,19 +2,27 @@ import { ACTION_ADD_STAGE, ACTION_DELETE_STAGE, ACTION_RESET_WORKFLOW, + ACTION_SET_CONTENT_TYPES, + ACTION_SET_IS_LOADING, + ACTION_SET_ROLES, ACTION_SET_WORKFLOW, + ACTION_SET_WORKFLOWS, ACTION_UPDATE_STAGE, ACTION_UPDATE_STAGE_POSITION, ACTION_UPDATE_WORKFLOW, } from '../constants'; -export function setWorkflow({ status, data }) { +export function setWorkflow({ workflow }) { return { type: ACTION_SET_WORKFLOW, - payload: { - status, - workflow: data, - }, + payload: workflow, + }; +} + +export function setWorkflows({ workflows }) { + return { + type: ACTION_SET_WORKFLOWS, + payload: workflows, }; } @@ -66,3 +74,24 @@ export function resetWorkflow() { type: ACTION_RESET_WORKFLOW, }; } + +export function setContentTypes(payload) { + return { + type: ACTION_SET_CONTENT_TYPES, + payload, + }; +} + +export function setRoles(payload) { + return { + type: ACTION_SET_ROLES, + payload, + }; +} + +export function setIsLoading(isLoading) { + return { + type: ACTION_SET_IS_LOADING, + payload: isLoading, + }; +} diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/tests/index.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/tests/index.test.js index 0cb1ac3177..45e0e2699e 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/tests/index.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/tests/index.test.js @@ -1,19 +1,42 @@ -import { addStage, deleteStage, setWorkflow, updateStage } from '..'; +import { + addStage, + deleteStage, + setWorkflow, + setWorkflows, + updateStage, + updateStagePosition, + updateWorkflow, + resetWorkflow, + setContentTypes, + setIsLoading, + setRoles, +} from '..'; import { ACTION_SET_WORKFLOW, ACTION_DELETE_STAGE, ACTION_ADD_STAGE, ACTION_UPDATE_STAGE, + ACTION_SET_CONTENT_TYPES, + ACTION_SET_IS_LOADING, + ACTION_SET_ROLES, + ACTION_SET_WORKFLOWS, + ACTION_UPDATE_STAGE_POSITION, + ACTION_RESET_WORKFLOW, + ACTION_UPDATE_WORKFLOW, } from '../../constants'; describe('Admin | Settings | Review Workflow | actions', () => { test('setWorkflow()', () => { - expect(setWorkflow({ status: 'loading', data: null, something: 'else' })).toStrictEqual({ + expect(setWorkflow({ workflow: null, something: 'else' })).toStrictEqual({ type: ACTION_SET_WORKFLOW, - payload: { - status: 'loading', - workflow: null, - }, + payload: null, + }); + }); + + test('setWorkflows()', () => { + expect(setWorkflows({ workflows: [] })).toStrictEqual({ + type: ACTION_SET_WORKFLOWS, + payload: [], }); }); @@ -49,4 +72,48 @@ describe('Admin | Settings | Review Workflow | actions', () => { }, }); }); + + test('updateStagePosition()', () => { + expect(updateStagePosition(1, 2)).toStrictEqual({ + type: ACTION_UPDATE_STAGE_POSITION, + payload: { + newIndex: 2, + oldIndex: 1, + }, + }); + }); + + test('updateWorkflow()', () => { + expect(updateWorkflow({})).toStrictEqual({ + type: ACTION_UPDATE_WORKFLOW, + payload: {}, + }); + }); + + test('resetWorkflow()', () => { + expect(resetWorkflow()).toStrictEqual({ + type: ACTION_RESET_WORKFLOW, + }); + }); + + test('setContentTypes()', () => { + expect(setContentTypes({})).toStrictEqual({ + type: ACTION_SET_CONTENT_TYPES, + payload: {}, + }); + }); + + test('setRoles()', () => { + expect(setRoles({})).toStrictEqual({ + type: ACTION_SET_ROLES, + payload: {}, + }); + }); + + test('setIsLoading()', () => { + expect(setIsLoading(true)).toStrictEqual({ + type: ACTION_SET_IS_LOADING, + payload: true, + }); + }); }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/Stage.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/Stage.js index efd000bc14..5d420eb95e 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/Stage.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/Stage.js @@ -9,25 +9,34 @@ import { Grid, GridItem, IconButton, + MultiSelect, + MultiSelectGroup, + MultiSelectOption, SingleSelect, SingleSelectOption, TextInput, VisuallyHidden, } from '@strapi/design-system'; -import { useTracking } from '@strapi/helper-plugin'; +import { NotAllowedInput, useTracking } from '@strapi/helper-plugin'; import { Drag, Trash } from '@strapi/icons'; import { useField } from 'formik'; import PropTypes from 'prop-types'; import { getEmptyImage } from 'react-dnd-html5-backend'; import { useIntl } from 'react-intl'; -import { useDispatch } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; +import styled from 'styled-components'; import { useDragAndDrop } from '../../../../../../../../../admin/src/content-manager/hooks'; import { composeRefs } from '../../../../../../../../../admin/src/content-manager/utils'; import { deleteStage, updateStage, updateStagePosition } from '../../../actions'; import { DRAG_DROP_TYPES } from '../../../constants'; +import { selectRoles } from '../../../selectors'; import { getAvailableStageColors, getStageColorByHex } from '../../../utils/colors'; +const NestedOption = styled(MultiSelectOption)` + padding-left: ${({ theme }) => theme.spaces[7]}; +`; + const AVAILABLE_COLORS = getAvailableStageColors(); function StageDropPreview() { @@ -144,6 +153,10 @@ export function Stage({ const [isOpen, setIsOpen] = React.useState(isOpenDefault); const [nameField, nameMeta, nameHelper] = useField(`stages.${index}.name`); const [colorField, colorMeta, colorHelper] = useField(`stages.${index}.color`); + const [permissionsField, permissionsMeta, permissionsHelper] = useField( + `stages.${index}.permissions` + ); + const roles = useSelector(selectRoles); const [{ handlerId, isDragging, handleKeyDown }, stageRef, dropRef, dragRef, dragPreviewRef] = useDragAndDrop(canReorder, { index, @@ -171,12 +184,17 @@ export function Stage({ color: hex, })); + const { themeColorName } = getStageColorByHex(colorField.value) ?? {}; + + const filteredRoles = roles + // Super admins always have permissions to do everything and therefore + // there is no point for this role to show up in the role combobox + .filter((role) => role.code !== 'strapi-super-admin'); + React.useEffect(() => { dragPreviewRef(getEmptyImage(), { captureDraggingState: false }); }, [dragPreviewRef, index]); - const { themeColorName } = getStageColorByHex(colorField.value) ?? {}; - return ( {liveText && {liveText}} @@ -315,6 +333,92 @@ export function Stage({ })} + + + {filteredRoles.length === 0 ? ( + + ) : ( + { + // Because the select components expects strings for values, but + // the yup schema validates numbers are sent to the API, we have + // to coerce the string value back to a number + const nextValues = values.map((value) => ({ + role: parseInt(value, 10), + action: 'admin::review-workflows.stage.transition', + })); + + permissionsHelper.setValue(nextValues); + + dispatch(updateStage(id, { permissions: nextValues })); + }} + placeholder={formatMessage({ + id: 'Settings.review-workflows.stage.permissions.placeholder', + defaultMessage: 'Select a role', + })} + required + // The Select component expects strings for values + value={(permissionsField.value ?? []).map((permission) => `${permission.role}`)} + withTags + > + {[ + { + value: null, + label: formatMessage({ + id: 'Settings.review-workflows.stage.permissions.allRoles.label', + defaultMessage: 'All roles', + }), + children: filteredRoles.map((role) => ({ + value: `${role.id}`, + label: role.name, + })), + }, + ].map((role) => { + if ('children' in role) { + return ( + child.value)} + > + {role.children.map((role) => { + return ( + + {role.label} + + ); + })} + + ); + } + + return ( + + {role.label} + + ); + })} + + )} + diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/tests/Stage.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/tests/Stage.test.js index f31a7a8290..1ee2a8f7e6 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/tests/Stage.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/tests/Stage.test.js @@ -1,16 +1,16 @@ import React from 'react'; import { lightTheme, ThemeProvider } from '@strapi/design-system'; -import { render } from '@testing-library/react'; +import { render, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { FormikProvider, useFormik } from 'formik'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; import { IntlProvider } from 'react-intl'; import { Provider } from 'react-redux'; +import { createStore } from 'redux'; -import configureStore from '../../../../../../../../../../admin/src/core/store/configureStore'; -import { STAGE_COLOR_DEFAULT } from '../../../../constants'; +import { REDUX_NAMESPACE, STAGE_COLOR_DEFAULT } from '../../../../constants'; import { reducer } from '../../../../reducer'; import { Stage } from '../Stage'; @@ -19,18 +19,86 @@ const STAGES_FIXTURE = { index: 0, }; +const WORKFLOWS_FIXTURE = [ + { + id: 1, + name: 'Default', + contentTypes: ['uid1'], + stages: [], + }, + + { + id: 2, + name: 'Default 2', + contentTypes: ['uid2'], + stages: [], + }, +]; + +const CONTENT_TYPES_FIXTURE = { + collectionTypes: [ + { + uid: 'uid1', + info: { + displayName: 'Collection CT 1', + }, + }, + + { + uid: 'uid2', + info: { + displayName: 'Collection CT 2', + }, + }, + ], + singleTypes: [ + { + uid: 'single-uid1', + info: { + displayName: 'Single CT 1', + }, + }, + + { + uid: 'single-uid2', + info: { + displayName: 'Single CT 2', + }, + }, + ], +}; + +const ROLES_FIXTURE = [ + { + id: 1, + code: 'strapi-editor', + name: 'Editor', + }, + + { + id: 2, + code: 'strapi-author', + name: 'Author', + }, + + { + id: 3, + code: 'strapi-super-admin', + name: 'Super Admin', + }, +]; + const ComponentFixture = ({ // eslint-disable-next-line react/prop-types stages = [ { color: STAGE_COLOR_DEFAULT, name: 'something', + permissions: [{ role: 1, action: 'admin::review-workflows.stage.transition' }], }, ], ...props }) => { - const store = configureStore([], [reducer]); - const formik = useFormik({ enableReinitialize: true, initialValues: { @@ -40,21 +108,43 @@ const ComponentFixture = ({ }); return ( - - - - - - - - - - - + + + ); }; -const setup = (props) => render(); +const setup = ({ roles, ...props } = {}) => + render(, { + wrapper({ children }) { + const store = createStore(reducer, { + [REDUX_NAMESPACE]: { + serverState: { + contentTypes: CONTENT_TYPES_FIXTURE, + roles: roles || ROLES_FIXTURE, + workflow: WORKFLOWS_FIXTURE[0], + workflows: WORKFLOWS_FIXTURE, + }, + + clientState: { + currentWorkflow: { + data: WORKFLOWS_FIXTURE[0], + }, + }, + }, + }); + + return ( + + + + {children} + + + + ); + }, + }); const user = userEvent.setup(); @@ -72,11 +162,25 @@ describe('Admin | Settings | Review Workflow | Stage', () => { // does not have better identifiers await user.click(container.querySelector('button[aria-expanded]')); - expect(queryByRole('textbox')).toBeInTheDocument(); - expect(getByRole('textbox').value).toBe('something'); + // Expect the accordion header to have the same value as the textbox + expect(getByRole('button', { name: /something/i })); expect(getByRole('textbox').getAttribute('name')).toBe('stages.0.name'); - expect(getByRole('combobox')).toHaveTextContent('Blue'); + // Name + expect(queryByRole('textbox')).toBeInTheDocument(); + expect(getByRole('textbox').value).toBe('something'); + + // Color combobox + await waitFor(() => + expect(getByRole('combobox', { name: /color/i })).toHaveTextContent('Blue') + ); + + // Permissions combobox + await waitFor(() => + expect( + getByRole('combobox', { name: /roles that can change this stage/i }) + ).toHaveTextContent('Editor') + ); expect( queryByRole('button', { @@ -88,27 +192,31 @@ describe('Admin | Settings | Review Workflow | Stage', () => { it('should open the accordion panel if isOpen = true', async () => { const { queryByRole } = setup({ isOpen: true }); - expect(queryByRole('textbox')).toBeInTheDocument(); + await waitFor(() => expect(queryByRole('textbox')).toBeInTheDocument()); }); it('should not render the delete button if canDelete=false', async () => { const { queryByRole } = setup({ isOpen: true, canDelete: false }); - expect( - queryByRole('button', { - name: /delete stage/i, - }) - ).not.toBeInTheDocument(); + await waitFor(() => + expect( + queryByRole('button', { + name: /delete stage/i, + }) + ).not.toBeInTheDocument() + ); }); it('should not render delete drag button if canUpdate=false', async () => { const { queryByRole } = setup({ isOpen: true, canUpdate: false }); - expect( - queryByRole('button', { - name: /drag/i, - }) - ).not.toBeInTheDocument(); + await waitFor(() => + expect( + queryByRole('button', { + name: /drag/i, + }) + ).not.toBeInTheDocument() + ); }); it('should not crash on a custom color code', async () => { @@ -131,7 +239,49 @@ describe('Admin | Settings | Review Workflow | Stage', () => { await user.click(container.querySelector('button[aria-expanded]')); + // Name expect(getByRole('textbox')).toHaveAttribute('disabled'); - expect(getByRole('combobox')).toHaveAttribute('data-disabled'); + + // Color + expect(getByRole('combobox', { name: /color/i })).toHaveAttribute('data-disabled'); + + // Permissions + expect(getByRole('combobox', { name: /roles that can change this stage/i })).toHaveAttribute( + 'data-disabled' + ); + }); + + it('should render a list of all available roles (except super admins)', async () => { + const { container, getByRole, queryByRole } = setup({ canUpdate: true }); + + await user.click(container.querySelector('button[aria-expanded]')); + + await waitFor(() => + expect( + getByRole('combobox', { name: /roles that can change this stage/i }) + ).toBeInTheDocument() + ); + + await user.click(getByRole('combobox', { name: /roles that can change this stage/i })); + + await waitFor(() => expect(getByRole('option', { name: /All roles/i })).toBeInTheDocument()); + await waitFor(() => expect(getByRole('option', { name: /Editor/i })).toBeInTheDocument()); + await waitFor(() => expect(getByRole('option', { name: /Author/i })).toBeInTheDocument()); + await waitFor(() => + expect(queryByRole('option', { name: /Super Admin/i })).not.toBeInTheDocument() + ); + }); + + it('should render a no permissions fallback, if no roles are available', async () => { + const { container, getByText } = setup({ + canUpdate: true, + roles: [...ROLES_FIXTURE].filter((role) => role.code === 'strapi-super-admin'), + }); + + await user.click(container.querySelector('button[aria-expanded]')); + + await waitFor(() => + expect(getByText(/you don’t have the permission to see roles/i)).toBeInTheDocument() + ); }); }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/tests/Stages.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/tests/Stages.test.js index 07f6286491..9877245eed 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/tests/Stages.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/tests/Stages.test.js @@ -1,9 +1,8 @@ import React from 'react'; import { lightTheme, ThemeProvider } from '@strapi/design-system'; -import { fireEvent, render } from '@testing-library/react'; +import { render } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { FormikProvider, useFormik } from 'formik'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; import { IntlProvider } from 'react-intl'; @@ -11,7 +10,7 @@ import { Provider } from 'react-redux'; import configureStore from '../../../../../../../../../admin/src/core/store/configureStore'; import * as actions from '../../../actions'; -import { ACTION_SET_WORKFLOW, STAGE_COLOR_DEFAULT } from '../../../constants'; +import { STAGE_COLOR_DEFAULT } from '../../../constants'; import { reducer } from '../../../reducer'; import { Stages } from '../Stages'; @@ -21,6 +20,15 @@ jest.mock('../../../actions', () => ({ ...jest.requireActual('../../../actions'), })); +// A single stage needs a formik provider, which is a bit complicated to setup. +// Since we don't want to test the single stages, but the overall composition +// it is the easiest for the test setup to just render an id instead of the +// whole component. +jest.mock('../Stage', () => ({ + __esModule: true, + Stage: ({ id }) => id, +})); + const STAGES_FIXTURE = [ { id: 1, @@ -35,44 +43,25 @@ const STAGES_FIXTURE = [ }, ]; -const WORKFLOWS_FIXTURE = [ - { - id: 1, - stages: STAGES_FIXTURE, - }, -]; +const setup = (props) => ({ + ...render(, { + wrapper({ children }) { + const store = configureStore([], [reducer]); -const ComponentFixture = (props) => { - const store = configureStore([], [reducer]); - - store.dispatch({ type: ACTION_SET_WORKFLOW, payload: { workflows: WORKFLOWS_FIXTURE } }); - - const formik = useFormik({ - enableReinitialize: true, - initialValues: { - stages: STAGES_FIXTURE, + return ( + + + + {children} + + + + ); }, - validateOnChange: false, - }); + }), - return ( - - - - - - - - - - - - ); -}; - -const setup = (props) => render(); - -const user = userEvent.setup(); + user: userEvent.setup(), +}); describe('Admin | Settings | Review Workflow | Stages', () => { beforeEach(() => { @@ -82,8 +71,8 @@ describe('Admin | Settings | Review Workflow | Stages', () => { it('should render a list of stages', () => { const { getByText } = setup(); - expect(getByText(STAGES_FIXTURE[0].name)).toBeInTheDocument(); - expect(getByText(STAGES_FIXTURE[1].name)).toBeInTheDocument(); + expect(getByText(STAGES_FIXTURE[0].id)).toBeInTheDocument(); + expect(getByText(STAGES_FIXTURE[1].id)).toBeInTheDocument(); }); it('should render a "add new stage" button', () => { @@ -93,7 +82,7 @@ describe('Admin | Settings | Review Workflow | Stages', () => { }); it('should append a new stage when clicking "add new stage"', async () => { - const { getByRole } = setup(); + const { getByRole, user } = setup(); const spy = jest.spyOn(actions, 'addStage'); await user.click( @@ -106,23 +95,6 @@ describe('Admin | Settings | Review Workflow | Stages', () => { expect(spy).toBeCalledWith({ name: '' }); }); - it('should update the name of a stage by changing the input value', async () => { - const { queryByRole, getByRole } = setup(); - const spy = jest.spyOn(actions, 'updateStage'); - - await user.click(getByRole('button', { name: /stage-2/i })); - - const input = queryByRole('textbox', { - name: /stage name/i, - }); - - fireEvent.change(input, { target: { value: 'New name' } }); - - expect(spy).toBeCalledWith(2, { - name: 'New name', - }); - }); - it('should not render the "add stage" button if canUpdate = false', () => { const { queryByText } = setup({ canUpdate: false }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/WorkflowAttributes.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/WorkflowAttributes.js index 46f6c45f4e..af8f0e72c3 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/WorkflowAttributes.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/WorkflowAttributes.js @@ -13,10 +13,11 @@ import { useCollator } from '@strapi/helper-plugin'; import { useField } from 'formik'; import PropTypes from 'prop-types'; import { useIntl } from 'react-intl'; -import { useDispatch } from 'react-redux'; +import { useDispatch, useSelector } from 'react-redux'; import styled from 'styled-components'; import { updateWorkflow } from '../../actions'; +import { selectContentTypes, selectCurrentWorkflow, selectWorkflows } from '../../selectors'; const NestedOption = styled(MultiSelectOption)` padding-left: ${({ theme }) => theme.spaces[7]}; @@ -26,14 +27,12 @@ const ContentTypeTakeNotice = styled(Typography)` font-style: italic; `; -export function WorkflowAttributes({ - canUpdate, - contentTypes: { collectionTypes, singleTypes }, - currentWorkflow, - workflows, -}) { +export function WorkflowAttributes({ canUpdate }) { const { formatMessage, locale } = useIntl(); const dispatch = useDispatch(); + const { collectionTypes, singleTypes } = useSelector(selectContentTypes); + const currentWorkflow = useSelector(selectCurrentWorkflow); + const workflows = useSelector(selectWorkflows); const [nameField, nameMeta, nameHelper] = useField('name'); const [contentTypesField, contentTypesMeta, contentTypesHelper] = useField('contentTypes'); const formatter = useCollator(locale, { @@ -97,7 +96,7 @@ export function WorkflowAttributes({ id: 'Settings.review-workflows.workflow.contentTypes.collectionTypes.label', defaultMessage: 'Collection Types', }), - children: collectionTypes + children: [...collectionTypes] .sort((a, b) => formatter.compare(a.info.displayName, b.info.displayName)) .map((contentType) => ({ label: contentType.info.displayName, @@ -114,7 +113,7 @@ export function WorkflowAttributes({ id: 'Settings.review-workflows.workflow.contentTypes.singleTypes.label', defaultMessage: 'Single Types', }), - children: singleTypes.map((contentType) => ({ + children: [...singleTypes].map((contentType) => ({ label: contentType.info.displayName, value: contentType.uid, })), @@ -178,24 +177,10 @@ export function WorkflowAttributes({ ); } -const ContentTypeType = PropTypes.shape({ - uid: PropTypes.string.isRequired, - info: PropTypes.shape({ - displayName: PropTypes.string.isRequired, - }).isRequired, -}); - WorkflowAttributes.defaultProps = { canUpdate: true, - currentWorkflow: undefined, }; WorkflowAttributes.propTypes = { canUpdate: PropTypes.bool, - contentTypes: PropTypes.shape({ - collectionTypes: PropTypes.arrayOf(ContentTypeType).isRequired, - singleTypes: PropTypes.arrayOf(ContentTypeType).isRequired, - }).isRequired, - currentWorkflow: PropTypes.object, - workflows: PropTypes.array.isRequired, }; diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/tests/WorkflowAttributes.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/tests/WorkflowAttributes.test.js index 732e18a430..04edf9ac0b 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/tests/WorkflowAttributes.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/WorkflowAttributes/tests/WorkflowAttributes.test.js @@ -8,31 +8,12 @@ import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; import { IntlProvider } from 'react-intl'; import { Provider } from 'react-redux'; +import { createStore } from 'redux'; -import configureStore from '../../../../../../../../../admin/src/core/store/configureStore'; +import { REDUX_NAMESPACE } from '../../../constants'; import { reducer } from '../../../reducer'; import { WorkflowAttributes } from '../WorkflowAttributes'; -const CONTENT_TYPES_FIXTURE = { - collectionTypes: [ - { - uid: 'uid1', - info: { - displayName: 'Content Type 1', - }, - }, - ], - - singleTypes: [ - { - uid: 'uid2', - info: { - displayName: 'Content Type 2', - }, - }, - ], -}; - const WORKFLOWS_FIXTURE = [ { id: 1, @@ -43,48 +24,63 @@ const WORKFLOWS_FIXTURE = [ { id: 2, - name: 'Workflow 1', - contentTypes: [], + name: 'Default 2', + contentTypes: ['uid2'], stages: [], }, ]; -const CURRENT_WORKFLOW_FIXTURE = { - ...WORKFLOWS_FIXTURE[0], +const CONTENT_TYPES_FIXTURE = { + collectionTypes: [ + { + uid: 'uid1', + info: { + displayName: 'Collection CT 1', + }, + }, + + { + uid: 'uid2', + info: { + displayName: 'Collection CT 2', + }, + }, + ], + singleTypes: [ + { + uid: 'single-uid1', + info: { + displayName: 'Single CT 1', + }, + }, + + { + uid: 'single-uid2', + info: { + displayName: 'Single CT 2', + }, + }, + ], }; -const ComponentFixture = (props) => { - const store = configureStore([], [reducer]); +const ROLES_FIXTURE = []; +// eslint-disable-next-line react/prop-types +const ComponentFixture = ({ currentWorkflow, ...props } = {}) => { const formik = useFormik({ enableReinitialize: true, - initialValues: { - name: 'workflow name', - contentTypes: ['uid1', 'uid1'], - }, + initialValues: currentWorkflow || WORKFLOWS_FIXTURE[0], validateOnChange: false, }); return ( - - - - - - - - - - - + + + ); }; +// eslint-disable-next-line no-unused-vars const withMarkup = (query) => (text) => query((content, node) => { const hasText = (node) => node.textContent === text; @@ -93,8 +89,40 @@ const withMarkup = (query) => (text) => return hasText(node) && childrenDontHaveText; }); -const setup = (props) => ({ - ...render(), +const setup = ({ collectionTypes, singleTypes, currentWorkflow, ...props } = {}) => ({ + ...render(, { + wrapper({ children }) { + const store = createStore(reducer, { + [REDUX_NAMESPACE]: { + serverState: { + contentTypes: { + collectionTypes: collectionTypes || CONTENT_TYPES_FIXTURE.collectionTypes, + singleTypes: singleTypes || CONTENT_TYPES_FIXTURE.singleTypes, + }, + roles: ROLES_FIXTURE, + workflow: WORKFLOWS_FIXTURE[0], + workflows: WORKFLOWS_FIXTURE, + }, + + clientState: { + currentWorkflow: { + data: currentWorkflow || WORKFLOWS_FIXTURE[0], + }, + }, + }, + }); + + return ( + + + + {children} + + + + ); + }, + }), user: userEvent.setup(), }); @@ -102,20 +130,18 @@ describe('Admin | Settings | Review Workflow | WorkflowAttributes', () => { it('should render values', async () => { const { getByRole, getByText, user } = setup(); - const contentTypesSelect = getByRole('combobox', { name: /associated to/i }); - - expect(getByRole('textbox')).toHaveValue('workflow name'); - expect(getByText(/2 content types selected/i)).toBeInTheDocument(); + await waitFor(() => expect(getByText(/workflow name/i)).toBeInTheDocument()); + expect(getByRole('textbox', { name: /workflow name \*/i })).toHaveValue('Default'); + expect(getByText(/1 content type selected/i)).toBeInTheDocument(); expect(getByRole('textbox')).not.toHaveAttribute('disabled'); expect(getByRole('combobox', { name: /associated to/i })).not.toHaveAttribute('data-disabled'); - await user.click(contentTypesSelect); + await user.click(getByRole('combobox', { name: /associated to/i })); - await waitFor(() => { - expect(getByRole('option', { name: /content type 1/i })).toBeInTheDocument(); - expect(getByRole('option', { name: /content type 2/i })).toBeInTheDocument(); - }); + await waitFor(() => + expect(getByRole('option', { name: /Collection CT 1/i })).toBeInTheDocument() + ); }); it('should disabled fields if canUpdate = false', async () => { @@ -127,20 +153,9 @@ describe('Admin | Settings | Review Workflow | WorkflowAttributes', () => { }); }); - it('should not render a collection-type group if there are not collection-types', async () => { + it('should not render a collection-type group if there are no collection-types', async () => { const { getByRole, queryByRole, user } = setup({ - contentTypes: { - collectionTypes: [], - - singleTypes: [ - { - uid: 'uid2', - info: { - displayName: 'Content Type 2', - }, - }, - ], - }, + collectionTypes: [], }); const contentTypesSelect = getByRole('combobox', { name: /associated to/i }); @@ -153,20 +168,9 @@ describe('Admin | Settings | Review Workflow | WorkflowAttributes', () => { }); }); - it('should not render a collection-type group if there are not single-types', async () => { + it('should not render a collection-type group if there are no single-types', async () => { const { getByRole, queryByRole, user } = setup({ - contentTypes: { - collectionTypes: [ - { - uid: 'uid2', - info: { - displayName: 'Content Type 2', - }, - }, - ], - - singleTypes: [], - }, + singleTypes: [], }); const contentTypesSelect = getByRole('combobox', { name: /associated to/i }); @@ -188,23 +192,29 @@ describe('Admin | Settings | Review Workflow | WorkflowAttributes', () => { }); }); - it('should not render assigned content-types to the current workflow', async () => { + it('should not render the assigned content-types notice to the current workflow', async () => { const { getByRole, queryByText, user } = setup(); + await waitFor(() => + expect(getByRole('combobox', { name: /associated to/i })).toBeInTheDocument() + ); + const contentTypesSelect = getByRole('combobox', { name: /associated to/i }); const queryByTextWithMarkup = withMarkup(queryByText); await user.click(contentTypesSelect); - await waitFor(() => { - expect(queryByTextWithMarkup('(assigned to Default workflow)')).not.toBeInTheDocument(); - }); + await waitFor(() => + expect(queryByTextWithMarkup('(assigned to Default workflow)')).not.toBeInTheDocument() + ); }); it('should render assigned content-types to the other workflows', async () => { - const { getByRole, getByText, user } = setup({ - currentWorkflow: { ...WORKFLOWS_FIXTURE[1] }, - }); + const { getByRole, getByText, user } = setup(); + + await waitFor(() => + expect(getByRole('combobox', { name: /associated to/i })).toBeInTheDocument() + ); const contentTypesSelect = getByRole('combobox', { name: /associated to/i }); const getByTextWithMarkup = withMarkup(getByText); @@ -212,11 +222,11 @@ describe('Admin | Settings | Review Workflow | WorkflowAttributes', () => { await user.click(contentTypesSelect); await waitFor(() => { - expect(getByTextWithMarkup('(assigned to Default workflow)')).toBeInTheDocument(); + expect(getByTextWithMarkup('(assigned to Default 2 workflow)')).toBeInTheDocument(); }); }); - it('should render assigned content-types to the other workflows, when currentWorkflow is not passed', async () => { + it('should render assigned content-types of other workflows, when currentWorkflow is not passed', async () => { const { getByRole, getByText, user } = setup({ currentWorkflow: undefined, }); @@ -227,7 +237,7 @@ describe('Admin | Settings | Review Workflow | WorkflowAttributes', () => { await user.click(contentTypesSelect); await waitFor(() => { - expect(getByTextWithMarkup('(assigned to Default workflow)')).toBeInTheDocument(); + expect(getByTextWithMarkup('(assigned to Default 2 workflow)')).toBeInTheDocument(); }); }); }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/constants.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/constants.js index aa63aa210b..742c843afa 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/constants.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/constants.js @@ -3,7 +3,11 @@ import { lightTheme } from '@strapi/design-system'; export const REDUX_NAMESPACE = 'settings_review-workflows'; export const ACTION_RESET_WORKFLOW = `Settings/Review_Workflows/RESET_WORKFLOW`; +export const ACTION_SET_CONTENT_TYPES = `Settings/Review_Workflows/SET_CONTENT_TYPES`; +export const ACTION_SET_IS_LOADING = `Settings/Review_Workflows/SET_IS_LOADING`; +export const ACTION_SET_ROLES = `Settings/Review_Workflows/SET_ROLES`; export const ACTION_SET_WORKFLOW = `Settings/Review_Workflows/SET_WORKFLOW`; +export const ACTION_SET_WORKFLOWS = `Settings/Review_Workflows/SET_WORKFLOWS`; export const ACTION_DELETE_STAGE = `Settings/Review_Workflows/WORKFLOW_DELETE_STAGE`; export const ACTION_ADD_STAGE = `Settings/Review_Workflows/WORKFLOW_ADD_STAGE`; export const ACTION_UPDATE_STAGE = `Settings/Review_Workflows/WORKFLOW_UPDATE_STAGE`; diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/CreateView/CreateView.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/CreateView/CreateView.js index 466fe26091..c439146a5d 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/CreateView/CreateView.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/CreateView/CreateView.js @@ -15,10 +15,18 @@ import { useMutation } from 'react-query'; import { useDispatch, useSelector } from 'react-redux'; import { useHistory } from 'react-router-dom'; +import { useAdminRoles } from '../../../../../../../../admin/src/hooks/useAdminRoles'; import { useContentTypes } from '../../../../../../../../admin/src/hooks/useContentTypes'; import { useInjectReducer } from '../../../../../../../../admin/src/hooks/useInjectReducer'; import { useLicenseLimits } from '../../../../../../hooks'; -import { addStage, resetWorkflow } from '../../actions'; +import { + addStage, + resetWorkflow, + setContentTypes, + setIsLoading, + setRoles, + setWorkflows, +} from '../../actions'; import * as Layout from '../../components/Layout'; import * as LimitsModal from '../../components/LimitsModal'; import { Stages } from '../../components/Stages'; @@ -29,7 +37,13 @@ import { REDUX_NAMESPACE, } from '../../constants'; import { useReviewWorkflows } from '../../hooks/useReviewWorkflows'; -import { reducer, initialState } from '../../reducer'; +import { reducer } from '../../reducer'; +import { + selectIsLoading, + selectIsWorkflowDirty, + selectCurrentWorkflow, + selectRoles, +} from '../../selectors'; import { validateWorkflow } from '../../utils/validateWorkflow'; export function ReviewWorkflowsCreateView() { @@ -39,13 +53,15 @@ export function ReviewWorkflowsCreateView() { const { formatAPIError } = useAPIErrorHandler(); const dispatch = useDispatch(); const toggleNotification = useNotification(); - const { collectionTypes, singleTypes, isLoading: isLoadingModels } = useContentTypes(); - const { isLoading: isWorkflowLoading, meta, workflows } = useReviewWorkflows(); - const { - clientState: { - currentWorkflow: { data: currentWorkflow, isDirty: currentWorkflowIsDirty }, - }, - } = useSelector((state) => state?.[REDUX_NAMESPACE] ?? initialState); + const { collectionTypes, singleTypes, isLoading: isLoadingContentTypes } = useContentTypes(); + const { isLoading: isLoadingWorkflow, meta, workflows } = useReviewWorkflows(); + const { isLoading: isLoadingRoles, roles: serverRoles } = useAdminRoles(undefined, { + retry: false, + }); + const isLoading = useSelector(selectIsLoading); + const currentWorkflowIsDirty = useSelector(selectIsWorkflowDirty); + const currentWorkflow = useSelector(selectCurrentWorkflow); + const roles = useSelector(selectRoles); const [showLimitModal, setShowLimitModal] = React.useState(false); const { isLoading: isLicenseLoading, getFeature } = useLicenseLimits(); const [initialErrors, setInitialErrors] = React.useState(null); @@ -54,7 +70,7 @@ export function ReviewWorkflowsCreateView() { const limits = getFeature('review-workflows'); const contentTypesFromOtherWorkflows = workflows.flatMap((workflow) => workflow.contentTypes); - const { mutateAsync, isLoading } = useMutation( + const { mutateAsync, isLoading: isLoadingMutation } = useMutation( async ({ workflow }) => { const { data: { data }, @@ -167,13 +183,36 @@ export function ReviewWorkflowsCreateView() { React.useEffect(() => { dispatch(resetWorkflow()); + if (!isLoadingWorkflow) { + dispatch(setWorkflows({ workflows })); + } + + if (!isLoadingContentTypes) { + dispatch(setContentTypes({ collectionTypes, singleTypes })); + } + + if (!isLoadingRoles) { + dispatch(setRoles(serverRoles)); + } + + dispatch(setIsLoading(isLoadingContentTypes || isLoadingRoles)); + // Create an empty default stage dispatch( addStage({ name: '', }) ); - }, [dispatch]); + }, [ + collectionTypes, + dispatch, + isLoadingContentTypes, + isLoadingRoles, + isLoadingWorkflow, + serverRoles, + singleTypes, + workflows, + ]); /** * If the current license has a limit: @@ -189,7 +228,7 @@ export function ReviewWorkflowsCreateView() { */ React.useEffect(() => { - if (!isWorkflowLoading && !isLicenseLoading) { + if (!isLoadingWorkflow && !isLicenseLoading) { if ( limits?.[CHARGEBEE_WORKFLOW_ENTITLEMENT_NAME] && meta?.workflowsTotal >= parseInt(limits[CHARGEBEE_WORKFLOW_ENTITLEMENT_NAME], 10) @@ -205,12 +244,27 @@ export function ReviewWorkflowsCreateView() { } }, [ isLicenseLoading, - isWorkflowLoading, + isLoadingWorkflow, limits, meta?.workflowsTotal, currentWorkflow.stages.length, ]); + React.useEffect(() => { + const filteredRoles = roles.filter((role) => role.code !== 'strapi-super-admin'); + + if (!isLoading && filteredRoles.length === 0) { + toggleNotification({ + blockTransition: true, + type: 'warning', + message: formatMessage({ + id: 'Settings.review-workflows.stage.permissions.noPermissions.description', + defaultMessage: 'You don’t have the permission to see roles', + }), + }); + } + }, [formatMessage, isLoading, roles, toggleNotification]); + return ( <> @@ -225,7 +279,7 @@ export function ReviewWorkflowsCreateView() { type="submit" size="M" disabled={!currentWorkflowIsDirty} - isLoading={isLoading} + isLoading={isLoadingMutation} > {formatMessage({ id: 'global.save', @@ -247,7 +301,7 @@ export function ReviewWorkflowsCreateView() { /> - {isLoadingModels ? ( + {isLoading ? ( {formatMessage({ id: 'Settings.review-workflows.page.isLoading', @@ -256,10 +310,7 @@ export function ReviewWorkflowsCreateView() { ) : ( - + )} diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/EditView/EditView.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/EditView/EditView.js index e86286af07..351d07bc00 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/EditView/EditView.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/pages/EditView/EditView.js @@ -16,11 +16,19 @@ import { useMutation } from 'react-query'; import { useSelector, useDispatch } from 'react-redux'; import { useParams } from 'react-router-dom'; +import { useAdminRoles } from '../../../../../../../../admin/src/hooks/useAdminRoles'; import { useContentTypes } from '../../../../../../../../admin/src/hooks/useContentTypes'; import { useInjectReducer } from '../../../../../../../../admin/src/hooks/useInjectReducer'; import { selectAdminPermissions } from '../../../../../../../../admin/src/pages/App/selectors'; import { useLicenseLimits } from '../../../../../../hooks'; -import { resetWorkflow, setWorkflow } from '../../actions'; +import { + resetWorkflow, + setIsLoading, + setWorkflow, + setContentTypes, + setRoles, + setWorkflows, +} from '../../actions'; import * as Layout from '../../components/Layout'; import * as LimitsModal from '../../components/LimitsModal'; import { Stages } from '../../components/Stages'; @@ -31,7 +39,15 @@ import { REDUX_NAMESPACE, } from '../../constants'; import { useReviewWorkflows } from '../../hooks/useReviewWorkflows'; -import { reducer, initialState } from '../../reducer'; +import { reducer } from '../../reducer'; +import { + selectIsWorkflowDirty, + selectCurrentWorkflow, + selectHasDeletedServerStages, + selectIsLoading, + selectRoles, + selectServerState, +} from '../../selectors'; import { validateWorkflow } from '../../utils/validateWorkflow'; export function ReviewWorkflowsEditView() { @@ -42,29 +58,22 @@ export function ReviewWorkflowsEditView() { const { put } = useFetchClient(); const { formatAPIError } = useAPIErrorHandler(); const toggleNotification = useNotification(); - const { - isLoading: isWorkflowLoading, - meta, - workflows, - status: workflowStatus, - refetch, - } = useReviewWorkflows(); - const { collectionTypes, singleTypes, isLoading: isLoadingModels } = useContentTypes(); - const { - status, - clientState: { - currentWorkflow: { - data: currentWorkflow, - isDirty: currentWorkflowIsDirty, - hasDeletedServerStages, - }, - }, - } = useSelector((state) => state?.[REDUX_NAMESPACE] ?? initialState); + const { isLoading: isLoadingWorkflow, meta, workflows, refetch } = useReviewWorkflows(); + const { collectionTypes, singleTypes, isLoading: isLoadingContentTypes } = useContentTypes(); + const serverState = useSelector(selectServerState); + const currentWorkflowIsDirty = useSelector(selectIsWorkflowDirty); + const currentWorkflow = useSelector(selectCurrentWorkflow); + const hasDeletedServerStages = useSelector(selectHasDeletedServerStages); + const roles = useSelector(selectRoles); + const isLoading = useSelector(selectIsLoading); const { allowedActions: { canDelete, canUpdate }, } = useRBAC(permissions.settings['review-workflows']); const [savePrompts, setSavePrompts] = React.useState({}); const { getFeature, isLoading: isLicenseLoading } = useLicenseLimits(); + const { isLoading: isLoadingRoles, roles: serverRoles } = useAdminRoles(undefined, { + retry: false, + }); const [showLimitModal, setShowLimitModal] = React.useState(false); const [initialErrors, setInitialErrors] = React.useState(null); @@ -73,7 +82,7 @@ export function ReviewWorkflowsEditView() { .filter((workflow) => workflow.id !== parseInt(workflowId, 10)) .flatMap((workflow) => workflow.contentTypes); - const { mutateAsync, isLoading } = useMutation( + const { mutateAsync, isLoading: isLoadingMutation } = useMutation( async ({ workflow }) => { const { data: { data }, @@ -98,7 +107,32 @@ export function ReviewWorkflowsEditView() { setInitialErrors(null); try { - const res = await mutateAsync({ workflow }); + const res = await mutateAsync({ + workflow: { + ...workflow, + + // compare permissions of stages and only submit the ones which have + // changed; this enables partial updates e.g. for users who don't have + // permissions to see roles + stages: workflow.stages.map((stage) => { + const hasUpdatedPermissions = + stage?.permissions?.length > 0 + ? stage.permissions.some( + ({ role }) => + !serverState.workflow.stages.find( + (stage) => + !!(stage.permissions ?? []).find((permission) => permission.role === role) + ) + ) + : false; + + return { + ...stage, + permissions: hasUpdatedPermissions ? stage.permissions : undefined, + }; + }), + }, + }); return res; } catch (error) { @@ -194,14 +228,37 @@ export function ReviewWorkflowsEditView() { const limits = getFeature('review-workflows'); React.useEffect(() => { - dispatch(setWorkflow({ status: workflowStatus, data: workflow })); + if (!isLoadingWorkflow) { + dispatch(setWorkflow({ workflow })); + dispatch(setWorkflows({ workflows })); + } + + if (!isLoadingContentTypes) { + dispatch(setContentTypes({ collectionTypes, singleTypes })); + } + + if (!isLoadingRoles) { + dispatch(setRoles(serverRoles)); + } + + dispatch(setIsLoading(isLoadingWorkflow || isLoadingContentTypes || isLoadingRoles)); // reset the state to the initial state to avoid flashes if a user // navigates from an edit-view to a create-view return () => { dispatch(resetWorkflow()); }; - }, [workflowStatus, workflow, dispatch]); + }, [ + collectionTypes, + dispatch, + isLoadingContentTypes, + isLoadingWorkflow, + isLoadingRoles, + serverRoles, + singleTypes, + workflow, + workflows, + ]); /** * If the current license has a limit: @@ -217,7 +274,7 @@ export function ReviewWorkflowsEditView() { */ React.useEffect(() => { - if (!isWorkflowLoading && !isLicenseLoading) { + if (!isLoadingWorkflow && !isLicenseLoading) { if ( limits?.[CHARGEBEE_WORKFLOW_ENTITLEMENT_NAME] && meta?.workflowCount > parseInt(limits[CHARGEBEE_WORKFLOW_ENTITLEMENT_NAME], 10) @@ -234,12 +291,27 @@ export function ReviewWorkflowsEditView() { }, [ currentWorkflow.stages.length, isLicenseLoading, - isWorkflowLoading, + isLoadingWorkflow, limits, meta?.workflowCount, meta.workflowsTotal, ]); + React.useEffect(() => { + const filteredRoles = roles.filter((role) => role.code !== 'strapi-super-admin'); + + if (!isLoading && filteredRoles.length === 0) { + toggleNotification({ + blockTransition: true, + type: 'warning', + message: formatMessage({ + id: 'Settings.review-workflows.stage.permissions.noPermissions.description', + defaultMessage: 'You don’t have the permission to see roles', + }), + }); + } + }, [formatMessage, isLoading, roles, toggleNotification]); + // TODO: redirect back to list-view if workflow is not found? return ( @@ -259,7 +331,7 @@ export function ReviewWorkflowsEditView() { disabled={!currentWorkflowIsDirty} // if the confirm dialog is open the loading state is on // the confirm button already - loading={!Object.keys(savePrompts).length > 0 && isLoading} + loading={!Object.keys(savePrompts).length > 0 && isLoadingMutation} > {formatMessage({ id: 'global.save', @@ -269,7 +341,7 @@ export function ReviewWorkflowsEditView() { ) } subtitle={ - currentWorkflow.stages.length > 0 && + !isLoadingWorkflow && formatMessage( { id: 'Settings.review-workflows.page.subtitle', @@ -282,7 +354,7 @@ export function ReviewWorkflowsEditView() { /> - {isLoadingModels || status === 'loading' ? ( + {isLoading ? ( {formatMessage({ @@ -293,12 +365,7 @@ export function ReviewWorkflowsEditView() { ) : ( - + ({ ...stage, // A safety net in case a stage does not have a color assigned; - // this normallly should not happen + // this should not happen color: stage?.color ?? STAGE_COLOR_DEFAULT, })), }; } + break; + } - draft.clientState.currentWorkflow.hasDeletedServerStages = false; + case ACTION_SET_WORKFLOWS: { + draft.serverState.workflows = payload; break; } case ACTION_RESET_WORKFLOW: { - draft.clientState.currentWorkflow.data = initialState.clientState.currentWorkflow.data; + draft.clientState = initialState.clientState; draft.serverState = initialState.serverState; break; } @@ -71,12 +95,6 @@ export function reducer(state = initialState, action) { (stage) => (stage?.id ?? stage.__temp_key__) !== stageId ); - if (!currentWorkflow.hasDeletedServerStages) { - draft.clientState.currentWorkflow.hasDeletedServerStages = !!( - state.serverState.workflow?.stages ?? [] - ).find((stage) => stage.id === stageId); - } - break; } @@ -149,16 +167,6 @@ export function reducer(state = initialState, action) { default: break; } - - if (state.clientState.currentWorkflow.data && draft.serverState.workflow) { - draft.clientState.currentWorkflow.isDirty = !isEqual( - current(draft.clientState.currentWorkflow).data, - draft.serverState.workflow - ); - } else { - // if there is no workflow on the server, the workflow is awalys considered dirty - draft.clientState.currentWorkflow.isDirty = true; - } }); } diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js index 1c4d4d84f5..09f059d08f 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js @@ -3,6 +3,9 @@ import { ACTION_ADD_STAGE, ACTION_DELETE_STAGE, ACTION_RESET_WORKFLOW, + ACTION_SET_CONTENT_TYPES, + ACTION_SET_IS_LOADING, + ACTION_SET_ROLES, ACTION_SET_WORKFLOW, ACTION_UPDATE_STAGE, ACTION_UPDATE_STAGE_POSITION, @@ -33,10 +36,57 @@ describe('Admin | Settings | Review Workflows | reducer', () => { state = initialState; }); + test('ACTION_SET_IS_LOADING', () => { + const action = { + type: ACTION_SET_IS_LOADING, + payload: true, + }; + + expect(reducer(state, action)).toStrictEqual( + expect.objectContaining({ + clientState: expect.objectContaining({ + isLoading: true, + }), + }) + ); + }); + + test('ACTION_SET_CONTENT_TYPES', () => { + const action = { + type: ACTION_SET_CONTENT_TYPES, + payload: { collectionTypes: [{ id: 1 }] }, + }; + + expect(reducer(state, action)).toStrictEqual( + expect.objectContaining({ + serverState: expect.objectContaining({ + contentTypes: { + collectionTypes: [{ id: 1 }], + }, + }), + }) + ); + }); + + test('ACTION_SET_ROLES', () => { + const action = { + type: ACTION_SET_ROLES, + payload: [{ id: 1 }], + }; + + expect(reducer(state, action)).toStrictEqual( + expect.objectContaining({ + serverState: expect.objectContaining({ + roles: [{ id: 1 }], + }), + }) + ); + }); + test('ACTION_SET_WORKFLOW with workflows', () => { const action = { type: ACTION_SET_WORKFLOW, - payload: { status: 'loading-state', workflow: WORKFLOW_FIXTURE }, + payload: WORKFLOW_FIXTURE, }; const DEFAULT_WORKFLOW_FIXTURE = { @@ -51,15 +101,12 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect(reducer(state, action)).toStrictEqual( expect.objectContaining({ - status: 'loading-state', serverState: expect.objectContaining({ workflow: WORKFLOW_FIXTURE, }), clientState: expect.objectContaining({ currentWorkflow: expect.objectContaining({ data: DEFAULT_WORKFLOW_FIXTURE, - isDirty: false, - hasDeletedServerStages: false, }), }), }) @@ -78,7 +125,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { workflow: WORKFLOW_FIXTURE, }, clientState: { - currentWorkflow: { data: WORKFLOW_FIXTURE, isDirty: false }, + currentWorkflow: { data: WORKFLOW_FIXTURE }, }, }; @@ -95,107 +142,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { ); }); - test('ACTION_DELETE_STAGE - set hasDeletedServerStages to true if stageId exists on the server', () => { - const action = { - type: ACTION_DELETE_STAGE, - payload: { stageId: 1 }, - }; - - state = { - status: expect.any(String), - serverState: { - workflow: WORKFLOW_FIXTURE, - }, - clientState: { - currentWorkflow: { - data: WORKFLOW_FIXTURE, - isDirty: false, - }, - }, - }; - - expect(reducer(state, action)).toStrictEqual( - expect.objectContaining({ - clientState: expect.objectContaining({ - currentWorkflow: expect.objectContaining({ - hasDeletedServerStages: true, - }), - }), - }) - ); - }); - - test('ACTION_DELETE_STAGE - set hasDeletedServerStages to false if stageId does not exist on the server', () => { - const action = { - type: ACTION_DELETE_STAGE, - payload: { stageId: 3 }, - }; - - state = { - status: expect.any(String), - serverState: { - workflow: WORKFLOW_FIXTURE, - }, - clientState: { - currentWorkflow: { - data: { - ...WORKFLOW_FIXTURE, - stages: [...WORKFLOW_FIXTURE.stages, { __temp_key__: 3, name: 'something' }], - }, - isDirty: false, - }, - }, - }; - - expect(reducer(state, action)).toStrictEqual( - expect.objectContaining({ - clientState: expect.objectContaining({ - currentWorkflow: expect.objectContaining({ - hasDeletedServerStages: false, - }), - }), - }) - ); - }); - - test('ACTION_DELETE_STAGE - keep hasDeletedServerStages true as soon as one server stage has been deleted', () => { - const actionDeleteServerStage = { - type: ACTION_DELETE_STAGE, - payload: { stageId: 1 }, - }; - - const actionDeleteClientStage = { - type: ACTION_DELETE_STAGE, - payload: { stageId: 3 }, - }; - - state = { - status: expect.any(String), - serverState: { - workflow: WORKFLOW_FIXTURE, - }, - clientState: { - currentWorkflow: { - data: WORKFLOW_FIXTURE, - isDirty: false, - }, - }, - }; - - state = reducer(state, actionDeleteServerStage); - state = reducer(state, actionDeleteClientStage); - - expect(state).toStrictEqual( - expect.objectContaining({ - clientState: expect.objectContaining({ - currentWorkflow: expect.objectContaining({ - hasDeletedServerStages: true, - }), - }), - }) - ); - }); - test('ACTION_ADD_STAGE', () => { const action = { type: ACTION_ADD_STAGE, @@ -206,7 +152,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { status: expect.any(String), serverState: expect.any(Object), clientState: { - currentWorkflow: { data: WORKFLOW_FIXTURE, isDirty: false }, + currentWorkflow: { data: WORKFLOW_FIXTURE }, }, }; @@ -239,7 +185,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { status: expect.any(String), serverState: expect.any(Object), clientState: { - currentWorkflow: { data: null, isDirty: false }, + currentWorkflow: { data: null }, }, }; @@ -287,7 +233,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { status: expect.any(String), serverState: expect.any(Object), clientState: { - currentWorkflow: { data: WORKFLOW_FIXTURE, isDirty: false }, + currentWorkflow: { data: WORKFLOW_FIXTURE }, }, }; @@ -320,7 +266,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { status: expect.any(String), serverState: expect.any(Object), clientState: { - currentWorkflow: { data: WORKFLOW_FIXTURE, isDirty: false }, + currentWorkflow: { data: WORKFLOW_FIXTURE }, }, }; @@ -343,52 +289,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { ); }); - test('properly compare serverState and clientState and set isDirty accordingly', () => { - const actionAddStage = { - type: ACTION_ADD_STAGE, - payload: { name: 'something' }, - }; - - state = { - status: expect.any(String), - serverState: { - workflow: WORKFLOW_FIXTURE, - }, - clientState: { - currentWorkflow: { data: WORKFLOW_FIXTURE, isDirty: false }, - }, - }; - - state = reducer(state, actionAddStage); - - expect(state).toStrictEqual( - expect.objectContaining({ - clientState: expect.objectContaining({ - currentWorkflow: expect.objectContaining({ - isDirty: true, - }), - }), - }) - ); - - const actionDeleteStage = { - type: ACTION_DELETE_STAGE, - payload: { stageId: 3 }, - }; - - state = reducer(state, actionDeleteStage); - - expect(state).toStrictEqual( - expect.objectContaining({ - clientState: expect.objectContaining({ - currentWorkflow: expect.objectContaining({ - isDirty: false, - }), - }), - }) - ); - }); - test('ACTION_UPDATE_STAGE_POSITION', () => { const action = { type: ACTION_UPDATE_STAGE_POSITION, @@ -403,7 +303,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { clientState: { currentWorkflow: { data: WORKFLOW_FIXTURE, - isDirty: false, }, }, }; @@ -418,7 +317,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect.objectContaining({ name: 'stage-1' }), ], }), - isDirty: true, }), }), }) @@ -439,7 +337,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { clientState: { currentWorkflow: { data: WORKFLOW_FIXTURE, - isDirty: false, }, }, }; @@ -454,7 +351,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect.objectContaining({ name: 'stage-2' }), ], }), - isDirty: false, }), }), }) @@ -475,7 +371,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { clientState: { currentWorkflow: { data: WORKFLOW_FIXTURE, - isDirty: false, }, }, }; @@ -490,7 +385,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect.objectContaining({ name: 'stage-2' }), ], }), - isDirty: false, }), }), }) @@ -511,7 +405,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { clientState: { currentWorkflow: { data: WORKFLOW_FIXTURE, - isDirty: false, }, }, }; @@ -523,7 +416,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { data: expect.objectContaining({ name: 'test', }), - isDirty: true, }), }), }) @@ -543,7 +435,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { clientState: { currentWorkflow: { data: WORKFLOW_FIXTURE, - isDirty: false, }, }, }; @@ -556,7 +447,6 @@ describe('Admin | Settings | Review Workflows | reducer', () => { name: '', stages: [], }), - isDirty: true, }), }), }) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/selectors.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/selectors.js new file mode 100644 index 0000000000..a65581e83a --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/selectors.js @@ -0,0 +1,45 @@ +import isEqual from 'lodash/isEqual'; +import { createSelector } from 'reselect'; + +import { REDUX_NAMESPACE } from './constants'; +import { initialState } from './reducer'; + +export const selectNamespace = (state) => state[REDUX_NAMESPACE] ?? initialState; + +export const selectContentTypes = createSelector( + selectNamespace, + ({ serverState: { contentTypes } }) => contentTypes +); + +export const selectRoles = createSelector(selectNamespace, ({ serverState: { roles } }) => roles); + +export const selectCurrentWorkflow = createSelector( + selectNamespace, + ({ clientState: { currentWorkflow } }) => currentWorkflow.data +); + +export const selectWorkflows = createSelector( + selectNamespace, + ({ serverState: { workflows } }) => workflows +); + +export const selectIsWorkflowDirty = createSelector( + selectNamespace, + ({ serverState, clientState: { currentWorkflow } }) => + !isEqual(serverState.workflow, currentWorkflow.data) +); + +export const selectHasDeletedServerStages = createSelector( + selectNamespace, + ({ serverState, clientState: { currentWorkflow } }) => + !(serverState.workflow?.stages ?? []).every( + (stage) => !!currentWorkflow.data.stages.find(({ id }) => id === stage.id) + ) +); + +export const selectIsLoading = createSelector( + selectNamespace, + ({ clientState: { isLoading } }) => isLoading +); + +export const selectServerState = createSelector(selectNamespace, ({ serverState }) => serverState); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/tests/validateWorkflow.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/tests/validateWorkflow.test.js index 9fe921b9da..f7c65a84d1 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/tests/validateWorkflow.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/tests/validateWorkflow.test.js @@ -145,4 +145,79 @@ describe('Settings | Review Workflows | validateWorkflow()', () => { } `); }); + + test('stages.permissions: array', async () => { + expect( + await setup({ + name: 'name', + stages: [ + { + name: 'stage-1', + color: '#ffffff', + permissions: [{ role: 1, action: 'admin::review-workflow.stage.transition' }], + }, + ], + }) + ).toEqual(true); + + expect( + await setup({ + name: 'name', + stages: [ + { + name: 'stage-1', + color: '#ffffff', + permissions: [], + }, + ], + }) + ).toMatchInlineSnapshot(` + { + "stages": [ + { + "permissions": "Must be either an array or undefined", + }, + ], + } + `); + + expect( + await setup({ + name: 'name', + stages: [ + { + name: 'stage-1', + color: '#ffffff', + permissions: { role: '1', action: 'admin::review-workflow.stage.transition' }, + }, + ], + }) + ).toMatchInlineSnapshot(` + { + "stages": [ + { + "permissions": "stages[0].permissions must be a \`array\` type, but the final value was: \`{ + "role": "\\"1\\"", + "action": "\\"admin::review-workflow.stage.transition\\"" + }\`.", + }, + ], + } + `); + }); + + test('stages.permissions: undefined', async () => { + expect( + await setup({ + name: 'name', + stages: [ + { + name: 'stage-1', + color: '#ffffff', + permissions: undefined, + }, + ], + }) + ).toEqual(true); + }); }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/validateWorkflow.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/validateWorkflow.js index cdac8102ce..1c1fc22225 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/validateWorkflow.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/utils/validateWorkflow.js @@ -57,6 +57,33 @@ export async function validateWorkflow({ values, formatMessage }) { }) ) .matches(/^#(?:[0-9a-fA-F]{3}){1,2}$/i), + + permissions: yup + .array( + yup.object({ + role: yup + .number() + .strict() + .typeError( + formatMessage({ + id: 'Settings.review-workflows.validation.stage.permissions.role.number', + defaultMessage: 'Role must be of type number', + }) + ).required, + action: yup.string().required({ + id: 'Settings.review-workflows.validation.stage.permissions.action.required', + defaultMessage: 'Action is a required argument', + }), + }) + ) + .strict() + .min( + 1, + formatMessage({ + id: 'Settings.review-workflows.validation.stage.permissions', + defaultMessage: 'Must be either an array or undefined', + }) + ), }) ) .min(1), From 6bbc43e3062887edb931cac155fd7720db5e4a19 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 8 Aug 2023 12:43:54 +0200 Subject: [PATCH 2/2] Fix: Make return values of data-fetching hooks stable --- .../admin/src/hooks/useAdminRoles/index.js | 24 ++++++++++++------ .../src/hooks/useAdminUsers/useAdminUsers.js | 22 ++++++++++------ .../hooks/useContentTypes/useContentTypes.js | 25 +++++++++++++------ .../hooks/useReviewWorkflows.js | 23 +++++++++++------ 4 files changed, 66 insertions(+), 28 deletions(-) diff --git a/packages/core/admin/admin/src/hooks/useAdminRoles/index.js b/packages/core/admin/admin/src/hooks/useAdminRoles/index.js index a0ea4520aa..c934afbbc9 100644 --- a/packages/core/admin/admin/src/hooks/useAdminRoles/index.js +++ b/packages/core/admin/admin/src/hooks/useAdminRoles/index.js @@ -1,3 +1,5 @@ +import * as React from 'react'; + import { useCollator, useFetchClient } from '@strapi/helper-plugin'; import { useIntl } from 'react-intl'; import { useQuery } from 'react-query'; @@ -22,16 +24,24 @@ export const useAdminRoles = (params = {}, queryOptions = {}) => { queryOptions ); - let roles = []; + // the return value needs to be memoized, because intantiating + // an empty array as default value would lead to an unstable return + // value, which later on triggers infinite loops if used in the + // dependency arrays of other hooks + const roles = React.useMemo(() => { + let roles = []; - if (id && data) { - roles = [data.data]; - } else if (Array.isArray(data?.data)) { - roles = data.data; - } + if (id && data) { + roles = [data.data]; + } else if (Array.isArray(data?.data)) { + roles = data.data; + } + + return [...roles].sort((a, b) => formatter.compare(a.name, b.name)); + }, [data, id, formatter]); return { - roles: [...roles].sort((a, b) => formatter.compare(a.name, b.name)), + roles, error, isError, isLoading, diff --git a/packages/core/admin/admin/src/hooks/useAdminUsers/useAdminUsers.js b/packages/core/admin/admin/src/hooks/useAdminUsers/useAdminUsers.js index a4ea385bb1..f477e1aa5e 100644 --- a/packages/core/admin/admin/src/hooks/useAdminUsers/useAdminUsers.js +++ b/packages/core/admin/admin/src/hooks/useAdminUsers/useAdminUsers.js @@ -1,3 +1,5 @@ +import * as React from 'react'; + import { useFetchClient } from '@strapi/helper-plugin'; import { useQuery } from 'react-query'; @@ -20,17 +22,23 @@ export function useAdminUsers(params = {}, queryOptions = {}) { queryOptions ); - let users = []; + // the return value needs to be memoized, because intantiating + // an empty array as default value would lead to an unstable return + // value, which later on triggers infinite loops if used in the + // dependency arrays of other hooks + const users = React.useMemo(() => { + if (id && data) { + return [data]; + } else if (Array.isArray(data?.results)) { + return data.results; + } - if (id && data) { - users = [data]; - } else if (Array.isArray(data?.results)) { - users = data.results; - } + return []; + }, [data, id]); return { users, - pagination: data?.pagination ?? null, + pagination: React.useMemo(() => data?.pagination ?? null, [data?.pagination]), isLoading, isError, refetch, diff --git a/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js b/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js index 52ba5fe4fb..1917f9bc69 100644 --- a/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js +++ b/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js @@ -1,3 +1,5 @@ +import * as React from 'react'; + import { useAPIErrorHandler, useFetchClient, useNotification } from '@strapi/helper-plugin'; import { useQueries } from 'react-query'; @@ -29,16 +31,25 @@ export function useContentTypes() { const [components, contentTypes] = queries; const isLoading = components.isLoading || contentTypes.isLoading; - const collectionTypes = (contentTypes?.data ?? []).filter( - (contentType) => contentType.kind === 'collectionType' && contentType.isDisplayed - ); - const singleTypes = (contentTypes?.data ?? []).filter( - (contentType) => contentType.kind !== 'collectionType' && contentType.isDisplayed - ); + // the return value needs to be memoized, because intantiating + // an empty array as default value would lead to an unstable return + // value, which later on triggers infinite loops if used in the + // dependency arrays of other hooks + const collectionTypes = React.useMemo(() => { + return (contentTypes?.data ?? []).filter( + (contentType) => contentType.kind === 'collectionType' && contentType.isDisplayed + ); + }, [contentTypes?.data]); + + const singleTypes = React.useMemo(() => { + return (contentTypes?.data ?? []).filter( + (contentType) => contentType.kind !== 'collectionType' && contentType.isDisplayed + ); + }, [contentTypes?.data]); return { isLoading, - components: components?.data ?? [], + components: React.useMemo(() => components?.data ?? [], [components?.data]), collectionTypes, singleTypes, }; diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/useReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/useReviewWorkflows.js index 2cc79fedb5..696dff44bf 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/useReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/useReviewWorkflows.js @@ -1,3 +1,5 @@ +import * as React from 'react'; + import { useFetchClient } from '@strapi/helper-plugin'; import { useQuery } from 'react-query'; @@ -20,18 +22,25 @@ export function useReviewWorkflows(params = {}) { } ); - let workflows = []; + // the return value needs to be memoized, because intantiating + // an empty array as default value would lead to an unstable return + // value, which later on triggers infinite loops if used in the + // dependency arrays of other hooks - if (id && data?.data) { - workflows = [data.data]; - } else if (Array.isArray(data?.data)) { - workflows = data.data; - } + const workflows = React.useMemo(() => { + if (id && data?.data) { + return [data.data]; + } else if (Array.isArray(data?.data)) { + return data.data; + } + + return []; + }, [data?.data, id]); return { // meta contains e.g. the total of all workflows. we can not use // the pagination object here, because the list is not paginated. - meta: data?.meta ?? {}, + meta: React.useMemo(() => data?.meta ?? {}, [data?.meta]), workflows, isLoading, status,