From 351c53eebff5b6da5147d3fe621d5be021d958da Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 22 May 2023 12:20:52 +0200 Subject: [PATCH] Chore: Refactor useModels to use react-query rather than useReducer --- packages/core/admin/admin/src/hooks/index.js | 2 +- .../admin/src/hooks/useContentTypes/index.js | 1 + .../tests/useContentTypes.test.js | 142 ++++++++++++++++++ .../hooks/useContentTypes/useContentTypes.js | 45 ++++++ .../admin/admin/src/hooks/useModels/index.js | 58 ------- .../admin/src/hooks/useModels/reducer.js | 45 ------ .../src/hooks/useModels/tests/reducer.test.js | 132 ---------------- .../admin/admin/src/pages/HomePage/index.js | 4 +- .../src/pages/HomePage/tests/index.test.js | 14 +- .../pages/Webhooks/EditView/index.js | 4 +- 10 files changed, 199 insertions(+), 248 deletions(-) create mode 100644 packages/core/admin/admin/src/hooks/useContentTypes/index.js create mode 100644 packages/core/admin/admin/src/hooks/useContentTypes/tests/useContentTypes.test.js create mode 100644 packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js delete mode 100644 packages/core/admin/admin/src/hooks/useModels/index.js delete mode 100644 packages/core/admin/admin/src/hooks/useModels/reducer.js delete mode 100644 packages/core/admin/admin/src/hooks/useModels/tests/reducer.test.js diff --git a/packages/core/admin/admin/src/hooks/index.js b/packages/core/admin/admin/src/hooks/index.js index b71c0840ad..c740cd9d75 100644 --- a/packages/core/admin/admin/src/hooks/index.js +++ b/packages/core/admin/admin/src/hooks/index.js @@ -1,5 +1,5 @@ export { default as useConfigurations } from './useConfigurations'; -export { default as useModels } from './useModels'; +export { useContentTypes } from './useContentTypes'; export { default as useFetchPermissionsLayout } from './useFetchPermissionsLayout'; export { default as useFetchRole } from './useFetchRole'; export { default as useMenu } from './useMenu'; diff --git a/packages/core/admin/admin/src/hooks/useContentTypes/index.js b/packages/core/admin/admin/src/hooks/useContentTypes/index.js new file mode 100644 index 0000000000..c550533769 --- /dev/null +++ b/packages/core/admin/admin/src/hooks/useContentTypes/index.js @@ -0,0 +1 @@ +export * from './useContentTypes'; diff --git a/packages/core/admin/admin/src/hooks/useContentTypes/tests/useContentTypes.test.js b/packages/core/admin/admin/src/hooks/useContentTypes/tests/useContentTypes.test.js new file mode 100644 index 0000000000..5ad751afa3 --- /dev/null +++ b/packages/core/admin/admin/src/hooks/useContentTypes/tests/useContentTypes.test.js @@ -0,0 +1,142 @@ +import * as React from 'react'; +import { setupServer } from 'msw/node'; +import { rest } from 'msw'; +import { renderHook } from '@testing-library/react-hooks'; +import { IntlProvider } from 'react-intl'; +import { QueryClient, QueryClientProvider } from 'react-query'; + +import { useContentTypes } from '../useContentTypes'; + +jest.mock('@strapi/helper-plugin', () => ({ + ...jest.requireActual('@strapi/helper-plugin'), + useNotification: jest.fn().mockReturnValue(jest.fn), +})); + +const server = setupServer( + rest.get('*/content-manager/content-types', (req, res, ctx) => + res( + ctx.json({ + data: [ + { + uid: 'admin::collectionType', + isDisplayed: true, + apiID: 'permission', + kind: 'collectionType', + }, + + { + uid: 'admin::collectionTypeNotDispalyed', + isDisplayed: false, + apiID: 'permission', + kind: 'collectionType', + }, + + { + uid: 'admin::singleType', + isDisplayed: true, + kind: 'singleType', + }, + + { + uid: 'admin::singleTypeNotDispalyed', + isDisplayed: false, + kind: 'singleType', + }, + ], + }) + ) + ), + rest.get('*/content-manager/components', (req, res, ctx) => + res( + ctx.json({ + data: [ + { + uid: 'basic.relation', + isDisplayed: true, + apiID: 'relation', + category: 'basic', + info: { + displayName: 'Relation', + }, + options: {}, + attributes: { + id: { + type: 'integer', + }, + categories: { + type: 'relation', + relation: 'oneToMany', + target: 'api::category.category', + targetModel: 'api::category.category', + relationType: 'oneToMany', + }, + }, + }, + ], + }) + ) + ) +); + +const setup = () => + renderHook(() => useContentTypes(), { + wrapper({ children }) { + const client = new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }); + + return ( + + + {children} + + + ); + }, + }); + +describe('useContentTypes', () => { + beforeAll(() => { + server.listen(); + }); + + afterAll(() => { + server.close(); + }); + + test('fetches models and content-types', async () => { + const { result, waitFor } = setup(); + + expect(result.current.isLoading).toBe(true); + + expect(result.current.components).toStrictEqual([]); + expect(result.current.singleTypes).toStrictEqual([]); + expect(result.current.collectionTypes).toStrictEqual([]); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + + expect(result.current.components).toStrictEqual( + expect.arrayContaining([ + expect.objectContaining({ + uid: 'basic.relation', + }), + ]) + ); + + expect(result.current.collectionTypes).toStrictEqual([ + expect.objectContaining({ + uid: 'admin::collectionType', + }), + ]); + + expect(result.current.singleTypes).toStrictEqual([ + expect.objectContaining({ + uid: 'admin::singleType', + }), + ]); + }); +}); diff --git a/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js b/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js new file mode 100644 index 0000000000..52ba5fe4fb --- /dev/null +++ b/packages/core/admin/admin/src/hooks/useContentTypes/useContentTypes.js @@ -0,0 +1,45 @@ +import { useAPIErrorHandler, useFetchClient, useNotification } from '@strapi/helper-plugin'; +import { useQueries } from 'react-query'; + +export function useContentTypes() { + const { get } = useFetchClient(); + const { formatAPIError } = useAPIErrorHandler(); + const toggleNotification = useNotification(); + const queries = useQueries( + ['components', 'content-types'].map((type) => { + return { + queryKey: ['content-manager', type], + async queryFn() { + const { + data: { data }, + } = await get(`/content-manager/${type}`); + + return data; + }, + onError(error) { + toggleNotification({ + type: 'warning', + message: formatAPIError(error), + }); + }, + }; + }) + ); + + 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 + ); + + return { + isLoading, + components: components?.data ?? [], + collectionTypes, + singleTypes, + }; +} diff --git a/packages/core/admin/admin/src/hooks/useModels/index.js b/packages/core/admin/admin/src/hooks/useModels/index.js deleted file mode 100644 index 7d631ac661..0000000000 --- a/packages/core/admin/admin/src/hooks/useModels/index.js +++ /dev/null @@ -1,58 +0,0 @@ -import { useReducer, useEffect, useCallback } from 'react'; -import { useFetchClient, useNotification } from '@strapi/helper-plugin'; -import reducer, { initialState } from './reducer'; - -/** - * TODO: refactor this to not use the `useReducer` hook, - * it's not really necessary. Also use `useQuery`? - */ -const useModels = () => { - const toggleNotification = useNotification(); - const [state, dispatch] = useReducer(reducer, initialState); - - const { get } = useFetchClient(); - - const fetchModels = useCallback(async () => { - dispatch({ - type: 'GET_MODELS', - }); - - try { - const [ - { - data: { data: components }, - }, - { - data: { data: contentTypes }, - }, - ] = await Promise.all( - ['components', 'content-types'].map((endPoint) => get(`/content-manager/${endPoint}`)) - ); - - dispatch({ - type: 'GET_MODELS_SUCCEDED', - contentTypes, - components, - }); - } catch (err) { - dispatch({ - type: 'GET_MODELS_ERROR', - }); - toggleNotification({ - type: 'warning', - message: { id: 'notification.error' }, - }); - } - }, [toggleNotification, get]); - - useEffect(() => { - fetchModels(); - }, [fetchModels]); - - return { - ...state, - getData: fetchModels, - }; -}; - -export default useModels; diff --git a/packages/core/admin/admin/src/hooks/useModels/reducer.js b/packages/core/admin/admin/src/hooks/useModels/reducer.js deleted file mode 100644 index 4c1aac530a..0000000000 --- a/packages/core/admin/admin/src/hooks/useModels/reducer.js +++ /dev/null @@ -1,45 +0,0 @@ -/* eslint-disable consistent-return */ -import produce from 'immer'; - -export const initialState = { - collectionTypes: [], - components: [], - isLoading: true, - singleTypes: [], -}; - -const reducer = (state, action) => - produce(state, (draftState) => { - switch (action.type) { - case 'GET_MODELS': { - draftState.collectionTypes = initialState.collectionTypes; - draftState.singleTypes = initialState.singleTypes; - draftState.components = initialState.components; - draftState.isLoading = true; - break; - } - case 'GET_MODELS_ERROR': { - draftState.collectionTypes = initialState.collectionTypes; - draftState.singleTypes = initialState.singleTypes; - draftState.components = initialState.components; - draftState.isLoading = false; - break; - } - case 'GET_MODELS_SUCCEDED': { - const getContentTypeByKind = (kind) => - action.contentTypes.filter( - (contentType) => contentType.isDisplayed && contentType.kind === kind - ); - - draftState.isLoading = false; - draftState.collectionTypes = getContentTypeByKind('collectionType'); - draftState.singleTypes = getContentTypeByKind('singleType'); - draftState.components = action.components; - break; - } - default: - return draftState; - } - }); - -export default reducer; diff --git a/packages/core/admin/admin/src/hooks/useModels/tests/reducer.test.js b/packages/core/admin/admin/src/hooks/useModels/tests/reducer.test.js deleted file mode 100644 index 22a4f5005a..0000000000 --- a/packages/core/admin/admin/src/hooks/useModels/tests/reducer.test.js +++ /dev/null @@ -1,132 +0,0 @@ -import reducer from '../reducer'; - -describe('ADMIN | HOOKS | useModels | reducer', () => { - describe('DEFAULT_ACTION', () => { - it('should return the initialState', () => { - const state = { - test: true, - }; - - expect(reducer(state, {})).toEqual(state); - }); - }); - - describe('GET_MODELS_ERROR', () => { - it('should set isLoading to false is an error occured', () => { - const action = { - type: 'GET_MODELS_ERROR', - }; - const initialState = { - collectionTypes: [], - components: [], - singleTypes: [ - { - uid: 'app.homepage', - isDisplayed: true, - kind: 'singleType', - }, - ], - isLoading: true, - }; - const expected = { - collectionTypes: [], - components: [], - singleTypes: [], - isLoading: false, - }; - - expect(reducer(initialState, action)).toEqual(expected); - }); - }); - - describe('GET_MODELS', () => { - it('should set isLoading to true to start getting the data', () => { - const action = { - type: 'GET_MODELS', - }; - const initialState = { - collectionTypes: [ - { - uid: 'app.category', - isDisplayed: true, - kind: 'collectionType', - }, - { - uid: 'app.category', - isDisplayed: true, - kind: 'collectionType', - }, - ], - singleTypes: [ - { - uid: 'app.homepage', - isDisplayed: true, - kind: 'singleType', - }, - ], - components: [{}], - isLoading: false, - }; - const expected = { - collectionTypes: [], - components: [], - singleTypes: [], - isLoading: true, - }; - - expect(reducer(initialState, action)).toEqual(expected); - }); - }); - - describe('GET_MODELS_SUCCEDED', () => { - it('should return the state with the collectionTypes and singleTypes', () => { - const action = { - type: 'GET_MODELS_SUCCEDED', - contentTypes: [ - { - uid: 'app.homepage', - isDisplayed: true, - kind: 'singleType', - }, - { - uid: 'permissions.role', - isDisplayed: false, - kind: 'collectionType', - }, - { - uid: 'app.category', - isDisplayed: true, - kind: 'collectionType', - }, - ], - components: [], - }; - const initialState = { - collectionTypes: [], - components: [], - singleTypes: [], - isLoading: true, - }; - const expected = { - collectionTypes: [ - { - uid: 'app.category', - isDisplayed: true, - kind: 'collectionType', - }, - ], - singleTypes: [ - { - uid: 'app.homepage', - isDisplayed: true, - kind: 'singleType', - }, - ], - components: [], - isLoading: false, - }; - - expect(reducer(initialState, action)).toEqual(expected); - }); - }); -}); diff --git a/packages/core/admin/admin/src/pages/HomePage/index.js b/packages/core/admin/admin/src/pages/HomePage/index.js index 72836a42a6..125efe811f 100644 --- a/packages/core/admin/admin/src/pages/HomePage/index.js +++ b/packages/core/admin/admin/src/pages/HomePage/index.js @@ -12,7 +12,7 @@ import { LoadingIndicatorPage, useGuidedTour } from '@strapi/helper-plugin'; import { Layout, Main, Box, Grid, GridItem } from '@strapi/design-system'; import useLicenseLimitNotification from 'ee_else_ce/hooks/useLicenseLimitNotification'; import cornerOrnamentPath from './assets/corner-ornament.svg'; -import { useModels } from '../../hooks'; +import { useContentTypes } from '../../hooks/useContentTypes'; import isGuidedTourCompleted from '../../components/GuidedTour/utils/isGuidedTourCompleted'; import GuidedTourHomepage from '../../components/GuidedTour/Homepage'; import SocialLinks from './SocialLinks'; @@ -31,7 +31,7 @@ const LogoContainer = styled(Box)` const HomePage = () => { // Temporary until we develop the menu API - const { collectionTypes, singleTypes, isLoading: isLoadingForModels } = useModels(); + const { collectionTypes, singleTypes, isLoading: isLoadingForModels } = useContentTypes(); const { guidedTourState, isGuidedTourVisible, isSkipped } = useGuidedTour(); useLicenseLimitNotification(); diff --git a/packages/core/admin/admin/src/pages/HomePage/tests/index.test.js b/packages/core/admin/admin/src/pages/HomePage/tests/index.test.js index 40ff2e2a4d..54d8e495ac 100644 --- a/packages/core/admin/admin/src/pages/HomePage/tests/index.test.js +++ b/packages/core/admin/admin/src/pages/HomePage/tests/index.test.js @@ -6,7 +6,7 @@ import { IntlProvider } from 'react-intl'; import { useAppInfo } from '@strapi/helper-plugin'; import { ThemeProvider, lightTheme } from '@strapi/design-system'; import HomePage from '../index'; -import { useModels } from '../../../hooks'; +import { useContentTypes } from '../../../hooks/useContentTypes'; jest.mock('@strapi/helper-plugin', () => ({ ...jest.requireActual('@strapi/helper-plugin'), @@ -30,9 +30,7 @@ jest.mock('@strapi/helper-plugin', () => ({ })), })); -jest.mock('../../../hooks', () => ({ - useModels: jest.fn(), -})); +jest.mock('../../../hooks/useContentTypes'); jest.mock('ee_else_ce/hooks/useLicenseLimitNotification', () => ({ __esModule: true, @@ -52,11 +50,11 @@ const App = ( ); describe('Homepage', () => { - useModels.mockImplementation(() => ({ + useContentTypes.mockReturnValue({ isLoading: false, collectionTypes: [], singleTypes: [], - })); + }); test('should render all homepage links', () => { const { getByRole } = render(App); @@ -113,11 +111,11 @@ describe('Homepage', () => { }); it('should display particular text and action when there are collectionTypes and singletypes', () => { - useModels.mockImplementation(() => ({ + useContentTypes.mockReturnValue({ isLoading: false, collectionTypes: [{ uuid: 102 }], singleTypes: [{ isDisplayed: true }], - })); + }); const { getByText, getByRole } = render(App); diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/Webhooks/EditView/index.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/Webhooks/EditView/index.js index 0153c6a883..1b03a04e15 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/Webhooks/EditView/index.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/pages/Webhooks/EditView/index.js @@ -14,7 +14,7 @@ import { import { Main } from '@strapi/design-system'; import { useMutation, useQuery, useQueryClient } from 'react-query'; import { useHistory, useRouteMatch } from 'react-router-dom'; -import { useModels } from '../../../../../hooks'; +import { useContentTypes } from '../../../../../hooks/useContentTypes'; import WebhookForm from './components/WebhookForm'; import cleanData from './utils/formatData'; @@ -27,7 +27,7 @@ const EditView = () => { const { lockApp, unlockApp } = useOverlayBlocker(); const toggleNotification = useNotification(); const queryClient = useQueryClient(); - const { isLoading: isLoadingForModels, collectionTypes } = useModels(); + const { isLoading: isLoadingForModels, collectionTypes } = useContentTypes(); const { put, get, post } = useFetchClient(); const isCreating = id === 'create';