From 0e122dc34443e37e02a7fc64532e5e623fa40929 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 15:12:06 +0100 Subject: [PATCH 01/16] Settings: Connect Review Workflow UI, data loader and redux --- .../core/admin/admin/src/translations/en.json | 1 + .../pages/ReviewWorkflows/ReviewWorkflows.js | 65 +++++++++++-------- .../pages/ReviewWorkflows/actions/index.js | 19 ++++++ .../components/Stages/Stage/Stage.js | 4 +- .../components/Stages/Stages.js | 6 +- .../pages/ReviewWorkflows/constants.js | 7 +- .../hooks/useReviewWorkflows.js | 30 ++++++++- .../pages/ReviewWorkflows/reducer/index.js | 27 ++++++++ 8 files changed, 124 insertions(+), 35 deletions(-) create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/index.js create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js diff --git a/packages/core/admin/admin/src/translations/en.json b/packages/core/admin/admin/src/translations/en.json index a7341780bc..04ed4891eb 100644 --- a/packages/core/admin/admin/src/translations/en.json +++ b/packages/core/admin/admin/src/translations/en.json @@ -226,6 +226,7 @@ "Settings.profile.form.section.profile.page.title": "Profile page", "Settings.review-workflows.page.title": "Review Workflow", "Settings.review-workflows.page.subtitle": "{count, plural, one {# stage} other {# stages}}", + "Settings.review-workflows.page.isLoading": "Workflow is loading", "Settings.review-workflows.stage.name.label": "Stage name", "Settings.roles.create.description": "Define the rights given to the role", "Settings.roles.create.title": "Create a role", diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js index d993f93cd4..87f5443aaa 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js @@ -1,35 +1,39 @@ -import React from 'react'; +import React, { useEffect } from 'react'; import { useIntl } from 'react-intl'; +import { useSelector, useDispatch } from 'react-redux'; import { SettingsPageTitle } from '@strapi/helper-plugin'; -import { Button, ContentLayout, HeaderLayout, Layout, Main } from '@strapi/design-system'; +import { Button, ContentLayout, HeaderLayout, Layout, Loader, Main } from '@strapi/design-system'; import { Check } from '@strapi/icons'; import { Stages } from './components/Stages'; - -const STAGES = [ - { - uid: 'id-1', - name: 'To do', - }, - - { - uid: 'id-2', - name: 'Ready to review', - }, - - { - uid: 'id-3', - name: 'In progress', - }, - - { - uid: 'id-4', - name: 'Reviewed', - }, -]; +import { reducer } from './reducer'; +import { REDUX_NAMESPACE } from './constants'; +import { useInjectReducer } from '../../../../../../admin/src/hooks/useInjectReducer'; +import { useReviewWorkflows } from './hooks/useReviewWorkflows'; +import { setWorkflowLoadingState, setWorkflowStages } from './actions'; export function ReviewWorkflowsPage() { const { formatMessage } = useIntl(); + const { workflows: workflowsData } = useReviewWorkflows(); + const workflow = useSelector((state) => state?.[REDUX_NAMESPACE]); + const dispatch = useDispatch(); + + useInjectReducer(REDUX_NAMESPACE, reducer); + + useEffect(() => { + dispatch(setWorkflowLoadingState(workflowsData.status)); + + if (workflowsData.status === 'success') { + dispatch(setWorkflowStages(workflowsData.data)); + } + }, [workflowsData.status, workflowsData.data, dispatch]); + + // useInjectReducer() runs on the first rendering after useSelector + // which will return undefined. This helps to avoid too many optional + // chaining operators down the component. + if (!workflow) { + return null; + } return ( @@ -58,11 +62,20 @@ export function ReviewWorkflowsPage() { id: 'Settings.review-workflows.page.subtitle', defaultMessage: '{count, plural, one {# stage} other {# stages}}', }, - { count: STAGES.length } + { count: workflow.workflows.stages.length } )} /> - + {workflow.workflows.state === 'loading' ? ( + + {formatMessage({ + id: 'Settings.review-workflows.page.isLoading', + defaultMessage: 'Workflow is loading', + })} + + ) : ( + + )} 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 new file mode 100644 index 0000000000..fcef478087 --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/index.js @@ -0,0 +1,19 @@ +import { ACTION_SET_LOADING_STATE, ACTION_SET_STAGES } from '../constants'; + +export function setWorkflowLoadingState(state) { + return { + type: ACTION_SET_LOADING_STATE, + payload: { + state, + }, + }; +} + +export function setWorkflowStages(stages) { + return { + type: ACTION_SET_STAGES, + payload: { + stages, + }, + }; +} 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 67cd42c86d..83201ae24a 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 @@ -22,7 +22,7 @@ const StyledAccordion = styled(Box)` } `; -function Stage({ uid, name }) { +function Stage({ id, name }) { const { formatMessage } = useIntl(); const [isOpen, setIsOpen] = useState(false); @@ -34,7 +34,7 @@ function Stage({ uid, name }) { - {stages.map(({ uid, ...stage }) => ( - - + {stages.map(({ id, ...stage }) => ( + + ))} 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 9fa0c3b1f2..e5597df469 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 @@ -1,6 +1,11 @@ import PropTypes from 'prop-types'; +export const REDUX_NAMESPACE = 'settings_review-workflows'; + +export const ACTION_SET_LOADING_STATE = `Settings/Review_Workflows/SET_STATE`; +export const ACTION_SET_STAGES = `Settings/Review_Workflows/SET_DATA`; + export const StageType = PropTypes.shape({ - uid: PropTypes.string.isRequired, + id: PropTypes.string.isRequired, name: PropTypes.string.isRequired, }); 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 d6ffd1dc63..2296f02718 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 @@ -2,12 +2,36 @@ import { useQuery } from 'react-query'; import { useFetchClient } from '@strapi/helper-plugin'; -export function useReviewWorkflows(workflowUid) { +export function useReviewWorkflows(workflowId) { const { get } = useFetchClient(); async function fetchWorkflows() { + /* TODO: mocked for now until the API is ready */ + return [ + { + id: 'id-1', + name: 'To do', + }, + + { + id: 'id-2', + name: 'Ready to review', + }, + + { + id: 'id-3', + name: 'In progress', + }, + + { + id: 'id-4', + name: 'Reviewed', + }, + ]; + + // eslint-disable-next-line no-unreachable try { - const { data } = await get(`/admin/review-workflows/workflows/${workflowUid ?? ''}`); + const { data } = await get(`/admin/review-workflows/workflows/${workflowId ?? ''}`); return data; } catch (err) { @@ -15,7 +39,7 @@ export function useReviewWorkflows(workflowUid) { } } - const workflows = useQuery(['review-workflows', workflowUid ?? 'default'], fetchWorkflows); + const workflows = useQuery(['review-workflows', workflowId ?? 'default'], fetchWorkflows); return { workflows, diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js new file mode 100644 index 0000000000..85b6b9fa07 --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js @@ -0,0 +1,27 @@ +import produce from 'immer'; + +import { ACTION_SET_STAGES, ACTION_SET_LOADING_STATE } from '../constants'; + +const initialState = { + workflows: { + state: 'loading', + stages: [], + }, +}; + +export function reducer(state = initialState, action) { + return produce(state, (draft) => { + switch (action.type) { + case ACTION_SET_LOADING_STATE: + draft.workflows.state = action.payload.state; + break; + + case ACTION_SET_STAGES: + draft.workflows.stages = action.payload.stages; + break; + + default: + break; + } + }); +} From 2aa3804787a42e9ce0e49c04642852cc1919a4f0 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 16:27:39 +0100 Subject: [PATCH 02/16] Chore: Add component tests for Stages and Stage --- .../Stages/Stage/tests/Stage.test.js | 42 +++++++++++++++++++ .../components/Stages/tests/Stages.test.js | 42 +++++++++++++++++++ .../pages/ReviewWorkflows/constants.js | 2 +- .../hooks/tests/useReviewWorkflows.test.js | 24 +++++------ 4 files changed, 97 insertions(+), 13 deletions(-) create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/tests/Stage.test.js create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/tests/Stages.test.js 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 new file mode 100644 index 0000000000..d34ffbb7e0 --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/Stage/tests/Stage.test.js @@ -0,0 +1,42 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { IntlProvider } from 'react-intl'; + +import { ThemeProvider, lightTheme } from '@strapi/design-system'; + +import { Stage } from '../Stage'; + +const STAGES_FIXTURE = { + id: 1, + name: 'stage-1', +}; + +const ComponentFixture = (props) => ( + + + + + +); + +const setup = (props) => render(); + +const user = userEvent.setup(); + +describe('Admin | Settings | Revie Workflow | Stage', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should render a stage', async () => { + const { getByRole, queryByRole } = setup(); + + expect(queryByRole('textbox')).not.toBeInTheDocument(); + + await user.click(getByRole('button')); + + expect(queryByRole('textbox')).toBeInTheDocument(); + expect(getByRole('textbox').value).toBe(STAGES_FIXTURE.name); + }); +}); 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 new file mode 100644 index 0000000000..7dd91128ac --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/components/Stages/tests/Stages.test.js @@ -0,0 +1,42 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; + +import { ThemeProvider, lightTheme } from '@strapi/design-system'; + +import { Stages } from '../Stages'; + +const STAGES_FIXTURE = [ + { + id: 1, + name: 'stage-1', + }, + + { + id: 2, + name: 'stage-2', + }, +]; + +const ComponentFixture = (props) => ( + + + + + +); + +const setup = (props) => render(); + +describe('Admin | Settings | Revie Workflow | Stages', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('should render a list of stages', () => { + const { getByText } = setup(); + + expect(getByText(STAGES_FIXTURE[0].name)).toBeInTheDocument(); + expect(getByText(STAGES_FIXTURE[1].name)).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 e5597df469..7f0f5bd12e 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 @@ -6,6 +6,6 @@ export const ACTION_SET_LOADING_STATE = `Settings/Review_Workflows/SET_STATE`; export const ACTION_SET_STAGES = `Settings/Review_Workflows/SET_DATA`; export const StageType = PropTypes.shape({ - id: PropTypes.string.isRequired, + id: PropTypes.number.isRequired, name: PropTypes.string.isRequired, }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js index 71cc234b1e..85d141c618 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js @@ -26,10 +26,10 @@ const ComponentFixture = ({ children }) => ( {children} ); -function setup(uid) { +function setup(id) { return new Promise((resolve) => { act(() => { - resolve(renderHook(() => useReviewWorkflows(uid), { wrapper: ComponentFixture })); + resolve(renderHook(() => useReviewWorkflows(id), { wrapper: ComponentFixture })); }); }); } @@ -39,18 +39,18 @@ describe('useReviewWorkflows', () => { jest.clearAllMocks(); }); - test('fetch all workflows when calling the hook without a workflow uid', async () => { + test('fetch all workflows when calling the hook without a workflow id', async () => { const { get } = useFetchClient(); get.mockResolvedValue({ data: [ { - uid: 'bdb46de2-9b0d-11ed-a8fc-0242ac120002', + id: 'bdb46de2-9b0d-11ed-a8fc-0242ac120002', stages: [], }, { - uid: 'c57bc2dc-9b0d-11ed-a8fc-0242ac120002', + id: 'c57bc2dc-9b0d-11ed-a8fc-0242ac120002', stages: [], }, ], @@ -66,34 +66,34 @@ describe('useReviewWorkflows', () => { expect(result.current).toStrictEqual( expect.objectContaining({ workflows: expect.objectContaining({ - data: expect.arrayContaining([{ uid: expect.any(String), stages: expect.any(Array) }]), + data: expect.arrayContaining([{ id: expect.any(String), stages: expect.any(Array) }]), }), }) ); }); - test('fetch a single workflow when calling the hook with a workflow uid', async () => { + test('fetch a single workflow when calling the hook with a workflow id', async () => { const { get } = useFetchClient(); - const uidFixture = 'bdb46de2-9b0d-11ed-a8fc-0242ac120002'; + const idFixture = 1; get.mockResolvedValue({ data: { - uid: uidFixture, + id: idFixture, stages: [], }, }); - const { result, waitFor } = await setup(uidFixture); + const { result, waitFor } = await setup(idFixture); expect(result.current.workflows.isLoading).toBe(true); - expect(get).toBeCalledWith(`/admin/review-workflows/workflows/${uidFixture}`); + expect(get).toBeCalledWith(`/admin/review-workflows/workflows/${idFixture}`); await waitFor(() => expect(result.current.workflows.isLoading).toBe(false)); expect(result.current).toStrictEqual( expect.objectContaining({ workflows: expect.objectContaining({ - data: expect.objectContaining({ uid: expect.any(String), stages: expect.any(Array) }), + data: expect.objectContaining({ id: expect.any(String), stages: expect.any(Array) }), }), }) ); From f4a773ebc64238eb16110d7dc396babbd849e49a Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 16:43:11 +0100 Subject: [PATCH 03/16] Chore: Add reducer tests --- .../pages/ReviewWorkflows/reducer/index.js | 2 +- .../reducer/tests/index.test.js | 51 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js index 85b6b9fa07..70841ed2df 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js @@ -2,7 +2,7 @@ import produce from 'immer'; import { ACTION_SET_STAGES, ACTION_SET_LOADING_STATE } from '../constants'; -const initialState = { +export const initialState = { workflows: { state: 'loading', stages: [], 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 new file mode 100644 index 0000000000..14f1b6d07f --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js @@ -0,0 +1,51 @@ +import { initialState, reducer } from '..'; + +import { ACTION_SET_LOADING_STATE, ACTION_SET_STAGES } from '../../constants'; + +const STAGES_FIXTURE = [ + { + id: 1, + name: 'stage-1', + }, + + { + id: 2, + name: 'stage-2', + }, +]; + +describe('Admin | Settings | Review Workflows | reducer', () => { + let state; + + beforeEach(() => { + state = initialState; + }); + + test('should return the initialState', () => { + expect(reducer(state, {})).toEqual(initialState); + }); + + test('should handle ACTION_SET_LOADING_STATE', () => { + const action = { type: ACTION_SET_LOADING_STATE, payload: { state: 'loading-state' } }; + + expect(reducer(state, action)).toEqual({ + ...initialState, + workflows: { + ...initialState.workflows, + state: 'loading-state', + }, + }); + }); + + test('should handle ACTION_SET_STAGES', () => { + const action = { type: ACTION_SET_STAGES, payload: { stages: STAGES_FIXTURE } }; + + expect(reducer(state, action)).toEqual({ + ...initialState, + workflows: { + ...initialState.workflows, + stages: STAGES_FIXTURE, + }, + }); + }); +}); From fd0861efabf3613cabbb3fd39652cd4751d6157f Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 17:05:36 +0100 Subject: [PATCH 04/16] Settings: Simplify review workflow redux setup --- .../pages/ReviewWorkflows/ReviewWorkflows.js | 10 +++------- .../pages/ReviewWorkflows/actions/index.js | 18 +++++------------- .../pages/ReviewWorkflows/constants.js | 3 +-- .../pages/ReviewWorkflows/reducer/index.js | 9 +++------ .../reducer/tests/index.test.js | 18 +++++------------- 5 files changed, 17 insertions(+), 41 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js index 87f5443aaa..ccec39ed4b 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js @@ -10,7 +10,7 @@ import { reducer } from './reducer'; import { REDUX_NAMESPACE } from './constants'; import { useInjectReducer } from '../../../../../../admin/src/hooks/useInjectReducer'; import { useReviewWorkflows } from './hooks/useReviewWorkflows'; -import { setWorkflowLoadingState, setWorkflowStages } from './actions'; +import { setWorkflow } from './actions'; export function ReviewWorkflowsPage() { const { formatMessage } = useIntl(); @@ -21,12 +21,8 @@ export function ReviewWorkflowsPage() { useInjectReducer(REDUX_NAMESPACE, reducer); useEffect(() => { - dispatch(setWorkflowLoadingState(workflowsData.status)); - - if (workflowsData.status === 'success') { - dispatch(setWorkflowStages(workflowsData.data)); - } - }, [workflowsData.status, workflowsData.data, dispatch]); + dispatch(setWorkflow(workflowsData)); + }, [workflowsData, dispatch]); // useInjectReducer() runs on the first rendering after useSelector // which will return undefined. This helps to avoid too many optional 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 fcef478087..f6c0877d6e 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 @@ -1,19 +1,11 @@ -import { ACTION_SET_LOADING_STATE, ACTION_SET_STAGES } from '../constants'; +import { ACTION_SET_WORKFLOW } from '../constants'; -export function setWorkflowLoadingState(state) { +export function setWorkflow(reactQueryWorkflowResponse) { return { - type: ACTION_SET_LOADING_STATE, + type: ACTION_SET_WORKFLOW, payload: { - state, - }, - }; -} - -export function setWorkflowStages(stages) { - return { - type: ACTION_SET_STAGES, - payload: { - stages, + state: reactQueryWorkflowResponse.status, + stages: reactQueryWorkflowResponse.data, }, }; } 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 7f0f5bd12e..b7ebd285ed 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 @@ -2,8 +2,7 @@ import PropTypes from 'prop-types'; export const REDUX_NAMESPACE = 'settings_review-workflows'; -export const ACTION_SET_LOADING_STATE = `Settings/Review_Workflows/SET_STATE`; -export const ACTION_SET_STAGES = `Settings/Review_Workflows/SET_DATA`; +export const ACTION_SET_WORKFLOW = `Settings/Review_Workflows/SET_WORKFLOW`; export const StageType = PropTypes.shape({ id: PropTypes.number.isRequired, diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js index 70841ed2df..b13e9560e2 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js @@ -1,6 +1,6 @@ import produce from 'immer'; -import { ACTION_SET_STAGES, ACTION_SET_LOADING_STATE } from '../constants'; +import { ACTION_SET_WORKFLOW } from '../constants'; export const initialState = { workflows: { @@ -12,12 +12,9 @@ export const initialState = { export function reducer(state = initialState, action) { return produce(state, (draft) => { switch (action.type) { - case ACTION_SET_LOADING_STATE: + case ACTION_SET_WORKFLOW: draft.workflows.state = action.payload.state; - break; - - case ACTION_SET_STAGES: - draft.workflows.stages = action.payload.stages; + draft.workflows.stages = action.payload.stages ?? []; break; default: 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 14f1b6d07f..4fb12a2d9f 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 @@ -1,6 +1,6 @@ import { initialState, reducer } from '..'; -import { ACTION_SET_LOADING_STATE, ACTION_SET_STAGES } from '../../constants'; +import { ACTION_SET_WORKFLOW } from '../../constants'; const STAGES_FIXTURE = [ { @@ -26,24 +26,16 @@ describe('Admin | Settings | Review Workflows | reducer', () => { }); test('should handle ACTION_SET_LOADING_STATE', () => { - const action = { type: ACTION_SET_LOADING_STATE, payload: { state: 'loading-state' } }; + const action = { + type: ACTION_SET_WORKFLOW, + payload: { state: 'loading-state', stages: STAGES_FIXTURE }, + }; expect(reducer(state, action)).toEqual({ ...initialState, workflows: { ...initialState.workflows, state: 'loading-state', - }, - }); - }); - - test('should handle ACTION_SET_STAGES', () => { - const action = { type: ACTION_SET_STAGES, payload: { stages: STAGES_FIXTURE } }; - - expect(reducer(state, action)).toEqual({ - ...initialState, - workflows: { - ...initialState.workflows, stages: STAGES_FIXTURE, }, }); From c0f8140516d9ddf362bb1cd2e26a679a6dd0c95f Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 17:12:56 +0100 Subject: [PATCH 05/16] Settings: Add review workflow actions unit tests --- .../pages/ReviewWorkflows/actions/index.js | 2 +- .../ReviewWorkflows/actions/tests/index.test.js | 15 +++++++++++++++ .../ReviewWorkflows/reducer/tests/index.test.js | 4 ++-- 3 files changed, 18 insertions(+), 3 deletions(-) create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/tests/index.test.js 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 f6c0877d6e..c62b13cba2 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 @@ -4,7 +4,7 @@ export function setWorkflow(reactQueryWorkflowResponse) { return { type: ACTION_SET_WORKFLOW, payload: { - state: reactQueryWorkflowResponse.status, + status: reactQueryWorkflowResponse.status, stages: reactQueryWorkflowResponse.data, }, }; 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 new file mode 100644 index 0000000000..7d3cee7212 --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/actions/tests/index.test.js @@ -0,0 +1,15 @@ +import { setWorkflow } from '..'; + +import { ACTION_SET_WORKFLOW } from '../../constants'; + +describe('Admin | Settings | Review Workflow | actions', () => { + test('setWorkflow()', () => { + expect(setWorkflow({ status: 'loading', data: null, something: 'else' })).toStrictEqual({ + type: ACTION_SET_WORKFLOW, + payload: { + status: 'loading', + stages: null, + }, + }); + }); +}); 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 4fb12a2d9f..bf5cd4f8ff 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 @@ -28,14 +28,14 @@ describe('Admin | Settings | Review Workflows | reducer', () => { test('should handle ACTION_SET_LOADING_STATE', () => { const action = { type: ACTION_SET_WORKFLOW, - payload: { state: 'loading-state', stages: STAGES_FIXTURE }, + payload: { status: 'loading-state', stages: STAGES_FIXTURE }, }; expect(reducer(state, action)).toEqual({ ...initialState, workflows: { ...initialState.workflows, - state: 'loading-state', + status: 'loading-state', stages: STAGES_FIXTURE, }, }); From 9cf15676046d7c876af3e833dc662bc889556c93 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 17:20:01 +0100 Subject: [PATCH 06/16] Chore: Replace uid with id in useReviewWorkflows hook --- .../ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js index 85d141c618..313a537962 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js @@ -45,12 +45,12 @@ describe('useReviewWorkflows', () => { get.mockResolvedValue({ data: [ { - id: 'bdb46de2-9b0d-11ed-a8fc-0242ac120002', + id: 1, stages: [], }, { - id: 'c57bc2dc-9b0d-11ed-a8fc-0242ac120002', + id: 2, stages: [], }, ], From d7b18b9ece2534ea9ac263f15cc2009359bc4760 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 17:29:52 +0100 Subject: [PATCH 07/16] Chore: Simplify selector access across the page --- .../pages/ReviewWorkflows/ReviewWorkflows.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js index ccec39ed4b..3066c29243 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js @@ -15,7 +15,7 @@ import { setWorkflow } from './actions'; export function ReviewWorkflowsPage() { const { formatMessage } = useIntl(); const { workflows: workflowsData } = useReviewWorkflows(); - const workflow = useSelector((state) => state?.[REDUX_NAMESPACE]); + const state = useSelector((state) => state?.[REDUX_NAMESPACE]); const dispatch = useDispatch(); useInjectReducer(REDUX_NAMESPACE, reducer); @@ -27,10 +27,12 @@ export function ReviewWorkflowsPage() { // useInjectReducer() runs on the first rendering after useSelector // which will return undefined. This helps to avoid too many optional // chaining operators down the component. - if (!workflow) { + if (!state) { return null; } + const { workflows } = state; + return ( - {workflow.workflows.state === 'loading' ? ( + {workflows.state === 'loading' ? ( {formatMessage({ id: 'Settings.review-workflows.page.isLoading', @@ -70,7 +72,7 @@ export function ReviewWorkflowsPage() { })} ) : ( - + )} From 0086b0e16312f2a5afa1f811aba54844daaffd4f Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 17:31:32 +0100 Subject: [PATCH 08/16] Chore: Fix workflow status access --- .../pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js index 3066c29243..8ce417936b 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js @@ -64,7 +64,7 @@ export function ReviewWorkflowsPage() { )} /> - {workflows.state === 'loading' ? ( + {workflows.status === 'loading' ? ( {formatMessage({ id: 'Settings.review-workflows.page.isLoading', From 1ad0a97fb184aa21f148d9d2682783fdb40fdf4e Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 17:45:27 +0100 Subject: [PATCH 09/16] Chore: Fix outdated reducer test --- .../pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js | 2 +- .../pages/ReviewWorkflows/reducer/tests/index.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js index b13e9560e2..3e269f1e56 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js @@ -13,7 +13,7 @@ export function reducer(state = initialState, action) { return produce(state, (draft) => { switch (action.type) { case ACTION_SET_WORKFLOW: - draft.workflows.state = action.payload.state; + draft.workflows.status = action.payload.status; draft.workflows.stages = action.payload.stages ?? []; break; 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 bf5cd4f8ff..e53935762e 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 @@ -25,7 +25,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect(reducer(state, {})).toEqual(initialState); }); - test('should handle ACTION_SET_LOADING_STATE', () => { + test('should handle ACTION_SET_WORKFLOW', () => { const action = { type: ACTION_SET_WORKFLOW, payload: { status: 'loading-state', stages: STAGES_FIXTURE }, From ab3bcf46b6a1eae0b50bcf609d2c4fdc8b0ac084 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 30 Jan 2023 18:00:28 +0100 Subject: [PATCH 10/16] Chore: Skip useReviewWorkflows() tests for now --- .../ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js index 313a537962..669d15d0b2 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js @@ -34,7 +34,7 @@ function setup(id) { }); } -describe('useReviewWorkflows', () => { +describe.skip('useReviewWorkflows', () => { beforeEach(() => { jest.clearAllMocks(); }); From 699a35a53d03e6d797da38cde518a5089929f5c7 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 31 Jan 2023 09:27:49 +0100 Subject: [PATCH 11/16] Chore: Make store data-structure more flexible and split client from server --- .../pages/ReviewWorkflows/ReviewWorkflows.js | 17 ++++++--- .../pages/ReviewWorkflows/actions/index.js | 16 ++++---- .../actions/tests/index.test.js | 12 +++--- .../pages/ReviewWorkflows/constants.js | 2 +- .../hooks/useReviewWorkflows.js | 37 ++++++++++-------- .../pages/ReviewWorkflows/reducer/index.js | 23 +++++++---- .../reducer/tests/index.test.js | 38 +++++++++++-------- 7 files changed, 88 insertions(+), 57 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js index 8ce417936b..5bad2b8902 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js @@ -10,7 +10,7 @@ import { reducer } from './reducer'; import { REDUX_NAMESPACE } from './constants'; import { useInjectReducer } from '../../../../../../admin/src/hooks/useInjectReducer'; import { useReviewWorkflows } from './hooks/useReviewWorkflows'; -import { setWorkflow } from './actions'; +import { setWorkflows } from './actions'; export function ReviewWorkflowsPage() { const { formatMessage } = useIntl(); @@ -21,7 +21,7 @@ export function ReviewWorkflowsPage() { useInjectReducer(REDUX_NAMESPACE, reducer); useEffect(() => { - dispatch(setWorkflow(workflowsData)); + dispatch(setWorkflows(workflowsData)); }, [workflowsData, dispatch]); // useInjectReducer() runs on the first rendering after useSelector @@ -31,7 +31,12 @@ export function ReviewWorkflowsPage() { return null; } - const { workflows } = state; + const { + status, + serverState: { workflows }, + } = state; + + const defaultWorkflow = workflows[0] ?? {}; return ( @@ -60,11 +65,11 @@ export function ReviewWorkflowsPage() { id: 'Settings.review-workflows.page.subtitle', defaultMessage: '{count, plural, one {# stage} other {# stages}}', }, - { count: workflows.stages.length } + { count: defaultWorkflow.stages?.length ?? 0 } )} /> - {workflows.status === 'loading' ? ( + {status === 'loading' ? ( {formatMessage({ id: 'Settings.review-workflows.page.isLoading', @@ -72,7 +77,7 @@ export function ReviewWorkflowsPage() { })} ) : ( - + )} 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 c62b13cba2..685ee4da3e 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 @@ -1,11 +1,13 @@ -import { ACTION_SET_WORKFLOW } from '../constants'; +import { ACTION_SET_WORKFLOWS } from '../constants'; + +export function setWorkflows(reactQueryWorkflowResponse) { + const payload = { + status: reactQueryWorkflowResponse.status, + workflows: reactQueryWorkflowResponse.data, + }; -export function setWorkflow(reactQueryWorkflowResponse) { return { - type: ACTION_SET_WORKFLOW, - payload: { - status: reactQueryWorkflowResponse.status, - stages: reactQueryWorkflowResponse.data, - }, + type: ACTION_SET_WORKFLOWS, + payload, }; } 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 7d3cee7212..42bb7ae564 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,14 +1,14 @@ -import { setWorkflow } from '..'; +import { setWorkflows } from '..'; -import { ACTION_SET_WORKFLOW } from '../../constants'; +import { ACTION_SET_WORKFLOWS } from '../../constants'; describe('Admin | Settings | Review Workflow | actions', () => { - test('setWorkflow()', () => { - expect(setWorkflow({ status: 'loading', data: null, something: 'else' })).toStrictEqual({ - type: ACTION_SET_WORKFLOW, + test('setWorkflows()', () => { + expect(setWorkflows({ status: 'loading', data: null, something: 'else' })).toStrictEqual({ + type: ACTION_SET_WORKFLOWS, payload: { status: 'loading', - stages: null, + workflows: null, }, }); }); 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 b7ebd285ed..1f66bf417c 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 @@ -2,7 +2,7 @@ import PropTypes from 'prop-types'; export const REDUX_NAMESPACE = 'settings_review-workflows'; -export const ACTION_SET_WORKFLOW = `Settings/Review_Workflows/SET_WORKFLOW`; +export const ACTION_SET_WORKFLOWS = `Settings/Review_Workflows/SET_WORKFLOWS`; export const StageType = PropTypes.shape({ id: PropTypes.number.isRequired, 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 2296f02718..6cfbeac0b4 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 @@ -9,29 +9,36 @@ export function useReviewWorkflows(workflowId) { /* TODO: mocked for now until the API is ready */ return [ { - id: 'id-1', - name: 'To do', - }, + id: 1, + stages: [ + { + id: 'id-1', + name: 'To do', + }, - { - id: 'id-2', - name: 'Ready to review', - }, + { + id: 'id-2', + name: 'Ready to review', + }, - { - id: 'id-3', - name: 'In progress', - }, + { + id: 'id-3', + name: 'In progress', + }, - { - id: 'id-4', - name: 'Reviewed', + { + id: 'id-4', + name: 'Reviewed', + }, + ], }, ]; // eslint-disable-next-line no-unreachable try { - const { data } = await get(`/admin/review-workflows/workflows/${workflowId ?? ''}`); + const { data } = await get( + `/admin/review-workflows/workflows/${workflowId ?? ''}?populate=stages` + ); return data; } catch (err) { diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js index 3e269f1e56..96b838a6e5 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/index.js @@ -1,20 +1,29 @@ import produce from 'immer'; -import { ACTION_SET_WORKFLOW } from '../constants'; +import { ACTION_SET_WORKFLOWS } from '../constants'; export const initialState = { - workflows: { - state: 'loading', - stages: [], + status: 'loading', + serverState: { + workflows: [], + }, + clientState: { + workflows: [], }, }; export function reducer(state = initialState, action) { return produce(state, (draft) => { switch (action.type) { - case ACTION_SET_WORKFLOW: - draft.workflows.status = action.payload.status; - draft.workflows.stages = action.payload.stages ?? []; + case ACTION_SET_WORKFLOWS: + draft.status = action.payload.status; + + if (action.payload.workflows) { + draft.serverState.workflows = [ + ...state.serverState.workflows, + ...action.payload.workflows, + ]; + } break; default: 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 e53935762e..54d4a11a46 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 @@ -1,16 +1,21 @@ import { initialState, reducer } from '..'; -import { ACTION_SET_WORKFLOW } from '../../constants'; +import { ACTION_SET_WORKFLOWS } from '../../constants'; -const STAGES_FIXTURE = [ +const WORKFLOWS_FIXTURE = [ { id: 1, - name: 'stage-1', - }, + stages: [ + { + id: 1, + name: 'stage-1', + }, - { - id: 2, - name: 'stage-2', + { + id: 2, + name: 'stage-2', + }, + ], }, ]; @@ -22,21 +27,24 @@ describe('Admin | Settings | Review Workflows | reducer', () => { }); test('should return the initialState', () => { - expect(reducer(state, {})).toEqual(initialState); + expect(reducer(state, {})).toStrictEqual(initialState); }); - test('should handle ACTION_SET_WORKFLOW', () => { + test('should handle ACTION_SET_WORKFLOWS', () => { const action = { - type: ACTION_SET_WORKFLOW, - payload: { status: 'loading-state', stages: STAGES_FIXTURE }, + type: ACTION_SET_WORKFLOWS, + payload: { status: 'loading-state', workflows: WORKFLOWS_FIXTURE }, }; expect(reducer(state, action)).toEqual({ ...initialState, - workflows: { - ...initialState.workflows, - status: 'loading-state', - stages: STAGES_FIXTURE, + status: 'loading-state', + serverState: { + ...initialState.serverState, + workflows: [...initialState.serverState.workflows, ...WORKFLOWS_FIXTURE], + }, + clientState: { + workflows: [], }, }); }); From 8710caf581840f21e8da8e612dbefaa19b57d7a0 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 31 Jan 2023 09:32:06 +0100 Subject: [PATCH 12/16] Chore: Add reducer test for code branch without workflow data --- .../reducer/tests/index.test.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) 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 54d4a11a46..eed37b0a2a 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 @@ -30,7 +30,7 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect(reducer(state, {})).toStrictEqual(initialState); }); - test('should handle ACTION_SET_WORKFLOWS', () => { + test('should handle ACTION_SET_WORKFLOWS with workflows', () => { const action = { type: ACTION_SET_WORKFLOWS, payload: { status: 'loading-state', workflows: WORKFLOWS_FIXTURE }, @@ -48,4 +48,22 @@ describe('Admin | Settings | Review Workflows | reducer', () => { }, }); }); + + test('should handle ACTION_SET_WORKFLOWS without workflows', () => { + const action = { + type: ACTION_SET_WORKFLOWS, + payload: { status: 'loading', workflows: null }, + }; + + expect(reducer(state, action)).toEqual({ + ...initialState, + serverState: { + ...initialState.serverState, + workflows: [], + }, + clientState: { + workflows: [], + }, + }); + }); }); From d8b26d94da64fc93061a4612d9bd932d3acd277f Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 31 Jan 2023 09:56:20 +0100 Subject: [PATCH 13/16] Add tests for ReviewWorkflowsPage --- .../Stages/Stage/tests/Stage.test.js | 2 +- .../components/Stages/tests/Stages.test.js | 2 +- .../tests/ReviewWorkflows.test.js | 86 +++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/tests/ReviewWorkflows.test.js 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 d34ffbb7e0..52bd82480c 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 @@ -24,7 +24,7 @@ const setup = (props) => render(); const user = userEvent.setup(); -describe('Admin | Settings | Revie Workflow | Stage', () => { +describe('Admin | Settings | Review Workflow | Stage', () => { beforeEach(() => { jest.clearAllMocks(); }); 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 7dd91128ac..f989b82bc3 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 @@ -28,7 +28,7 @@ const ComponentFixture = (props) => ( const setup = (props) => render(); -describe('Admin | Settings | Revie Workflow | Stages', () => { +describe('Admin | Settings | Review Workflow | Stages', () => { beforeEach(() => { jest.clearAllMocks(); }); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/tests/ReviewWorkflows.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/tests/ReviewWorkflows.test.js new file mode 100644 index 0000000000..a7a2712a30 --- /dev/null +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/tests/ReviewWorkflows.test.js @@ -0,0 +1,86 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import { IntlProvider } from 'react-intl'; +import { Provider } from 'react-redux'; +import { QueryClientProvider, QueryClient } from 'react-query'; + +import { ThemeProvider, lightTheme } from '@strapi/design-system'; + +import configureStore from '../../../../../../../admin/src/core/store/configureStore'; +import ReviewWorkflowsPage from '..'; +import { reducer } from '../reducer'; +import { useReviewWorkflows } from '../hooks/useReviewWorkflows'; + +jest.mock('../hooks/useReviewWorkflows', () => ({ + ...jest.requireActual('../hooks/useReviewWorkflows'), + useReviewWorkflows: jest.fn().mockReturnValue({ + workflows: { + status: 'loading', + data: null, + }, + }), +})); + +const client = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, +}); + +const ComponentFixture = () => { + const store = configureStore([], [reducer]); + + return ( + + + + + + + + + + ); +}; + +const setup = (props) => render(); + +describe('Admin | Settings | Review Workflow | ReviewWorkflowsPage', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('handle initial loading state', () => { + const { getByText } = setup(); + + expect(getByText('0 stages')).toBeInTheDocument(); + expect(getByText('Workflow is loading')).toBeInTheDocument(); + }); + + test('handle loaded stage', () => { + useReviewWorkflows.mockReturnValue({ + workflows: { + status: 'success', + data: [ + { + id: 1, + stages: [ + { + id: 1, + name: 'stage-1', + }, + ], + }, + ], + }, + }); + + const { getByText, queryByText } = setup(); + + expect(getByText('1 stage')).toBeInTheDocument(); + expect(queryByText('Workflow is loading')).not.toBeInTheDocument(); + expect(getByText('stage-1')).toBeInTheDocument(); + }); +}); From 622946aec9dd983b050dec7b3f96e39416a8564e Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 31 Jan 2023 13:04:47 +0100 Subject: [PATCH 14/16] Remove mock data and use real API instead --- .../pages/ReviewWorkflows/ReviewWorkflows.js | 4 +-- .../hooks/useReviewWorkflows.js | 34 ++----------------- 2 files changed, 5 insertions(+), 33 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js index 5bad2b8902..6872446b62 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/ReviewWorkflows.js @@ -21,8 +21,8 @@ export function ReviewWorkflowsPage() { useInjectReducer(REDUX_NAMESPACE, reducer); useEffect(() => { - dispatch(setWorkflows(workflowsData)); - }, [workflowsData, dispatch]); + dispatch(setWorkflows({ status: workflowsData.status, data: workflowsData.data })); + }, [workflowsData.status, workflowsData.data, dispatch]); // useInjectReducer() runs on the first rendering after useSelector // which will return undefined. This helps to avoid too many optional 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 6cfbeac0b4..005cd1daf1 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 @@ -6,39 +6,11 @@ export function useReviewWorkflows(workflowId) { const { get } = useFetchClient(); async function fetchWorkflows() { - /* TODO: mocked for now until the API is ready */ - return [ - { - id: 1, - stages: [ - { - id: 'id-1', - name: 'To do', - }, - - { - id: 'id-2', - name: 'Ready to review', - }, - - { - id: 'id-3', - name: 'In progress', - }, - - { - id: 'id-4', - name: 'Reviewed', - }, - ], - }, - ]; - // eslint-disable-next-line no-unreachable try { - const { data } = await get( - `/admin/review-workflows/workflows/${workflowId ?? ''}?populate=stages` - ); + const { + data: { data }, + } = await get(`/admin/review-workflows/workflows/${workflowId ?? ''}?populate=stages`); return data; } catch (err) { From 73975255ea5298ac9a77f77b01c87501510d9458 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 31 Jan 2023 13:09:14 +0100 Subject: [PATCH 15/16] Chore: Simplify setWorkflows action --- .../pages/ReviewWorkflows/actions/index.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) 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 685ee4da3e..64f0db1b90 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 @@ -1,13 +1,11 @@ import { ACTION_SET_WORKFLOWS } from '../constants'; -export function setWorkflows(reactQueryWorkflowResponse) { - const payload = { - status: reactQueryWorkflowResponse.status, - workflows: reactQueryWorkflowResponse.data, - }; - +export function setWorkflows({ status, data }) { return { type: ACTION_SET_WORKFLOWS, - payload, + payload: { + status, + workflows: data, + }, }; } From 6583f4021ac9071400053f0e1286d6e0dab723eb Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 31 Jan 2023 13:49:38 +0100 Subject: [PATCH 16/16] useReviewWorkflows: Enable skipped tests --- .../hooks/tests/useReviewWorkflows.test.js | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js index 669d15d0b2..262ce67ccd 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/hooks/tests/useReviewWorkflows.test.js @@ -34,7 +34,7 @@ function setup(id) { }); } -describe.skip('useReviewWorkflows', () => { +describe('useReviewWorkflows', () => { beforeEach(() => { jest.clearAllMocks(); }); @@ -43,30 +43,32 @@ describe.skip('useReviewWorkflows', () => { const { get } = useFetchClient(); get.mockResolvedValue({ - data: [ - { - id: 1, - stages: [], - }, + data: { + data: [ + { + id: 1, + stages: [], + }, - { - id: 2, - stages: [], - }, - ], + { + id: 2, + stages: [], + }, + ], + }, }); const { result, waitFor } = await setup(); expect(result.current.workflows.isLoading).toBe(true); - expect(get).toBeCalledWith('/admin/review-workflows/workflows/'); + expect(get).toBeCalledWith('/admin/review-workflows/workflows/?populate=stages'); await waitFor(() => expect(result.current.workflows.isLoading).toBe(false)); expect(result.current).toStrictEqual( expect.objectContaining({ workflows: expect.objectContaining({ - data: expect.arrayContaining([{ id: expect.any(String), stages: expect.any(Array) }]), + data: expect.arrayContaining([{ id: expect.any(Number), stages: expect.any(Array) }]), }), }) ); @@ -78,22 +80,24 @@ describe.skip('useReviewWorkflows', () => { get.mockResolvedValue({ data: { - id: idFixture, - stages: [], + data: { + id: idFixture, + stages: [], + }, }, }); const { result, waitFor } = await setup(idFixture); expect(result.current.workflows.isLoading).toBe(true); - expect(get).toBeCalledWith(`/admin/review-workflows/workflows/${idFixture}`); + expect(get).toBeCalledWith(`/admin/review-workflows/workflows/${idFixture}?populate=stages`); await waitFor(() => expect(result.current.workflows.isLoading).toBe(false)); expect(result.current).toStrictEqual( expect.objectContaining({ workflows: expect.objectContaining({ - data: expect.objectContaining({ id: expect.any(String), stages: expect.any(Array) }), + data: expect.objectContaining({ id: expect.any(Number), stages: expect.any(Array) }), }), }) );