From 6f37fbb69e3e93ad58ac1c1dcbba47b0ce7d5f6b Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Fri, 11 Aug 2023 15:37:53 +0200 Subject: [PATCH] Chore: Dissolve useFetchRole --- .../admin/src/hooks/useFetchRole/index.js | 64 ------------- .../admin/src/hooks/useFetchRole/reducer.js | 31 ------- .../hooks/useFetchRole/tests/reducer.test.js | 90 ------------------- .../admin/src/hooks/usePlugins.js | 16 +++- .../admin/src/pages/Roles/pages/EditPage.js | 24 +++-- 5 files changed, 30 insertions(+), 195 deletions(-) delete mode 100644 packages/plugins/users-permissions/admin/src/hooks/useFetchRole/index.js delete mode 100644 packages/plugins/users-permissions/admin/src/hooks/useFetchRole/reducer.js delete mode 100644 packages/plugins/users-permissions/admin/src/hooks/useFetchRole/tests/reducer.test.js diff --git a/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/index.js b/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/index.js deleted file mode 100644 index b4661f9ef7..0000000000 --- a/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/index.js +++ /dev/null @@ -1,64 +0,0 @@ -import { useCallback, useEffect, useReducer, useRef } from 'react'; - -import { useFetchClient, useNotification } from '@strapi/helper-plugin'; - -import reducer, { initialState } from './reducer'; - -// TODO: Refactor to use react-query -export const useFetchRole = (id) => { - const [state, dispatch] = useReducer(reducer, initialState); - const toggleNotification = useNotification(); - const isMounted = useRef(null); - const { get } = useFetchClient(); - - useEffect(() => { - isMounted.current = true; - - if (id) { - fetchRole(id); - } else { - dispatch({ - type: 'GET_DATA_SUCCEEDED', - role: {}, - }); - } - - return () => (isMounted.current = false); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [id]); - - const fetchRole = async (roleId) => { - try { - const { - data: { role }, - } = await get(`/users-permissions/roles/${roleId}`); - - // Prevent updating state on an unmounted component - if (isMounted.current) { - dispatch({ - type: 'GET_DATA_SUCCEEDED', - role, - }); - } - } catch (err) { - console.error(err); - - dispatch({ - type: 'GET_DATA_ERROR', - }); - toggleNotification({ - type: 'warning', - message: { id: 'notification.error' }, - }); - } - }; - - const handleSubmitSucceeded = useCallback((data) => { - dispatch({ - type: 'ON_SUBMIT_SUCCEEDED', - ...data, - }); - }, []); - - return { ...state, onSubmitSucceeded: handleSubmitSucceeded }; -}; diff --git a/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/reducer.js b/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/reducer.js deleted file mode 100644 index 99dcf0d190..0000000000 --- a/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/reducer.js +++ /dev/null @@ -1,31 +0,0 @@ -/* eslint-disable consistent-return */ -import produce from 'immer'; - -export const initialState = { - role: {}, - isLoading: true, -}; - -const reducer = (state, action) => - produce(state, (draftState) => { - switch (action.type) { - case 'GET_DATA_SUCCEEDED': { - draftState.role = action.role; - draftState.isLoading = false; - break; - } - case 'GET_DATA_ERROR': { - draftState.isLoading = false; - break; - } - case 'ON_SUBMIT_SUCCEEDED': { - draftState.role.name = action.name; - draftState.role.description = action.description; - break; - } - default: - return draftState; - } - }); - -export default reducer; diff --git a/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/tests/reducer.test.js b/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/tests/reducer.test.js deleted file mode 100644 index 2d42ffd996..0000000000 --- a/packages/plugins/users-permissions/admin/src/hooks/useFetchRole/tests/reducer.test.js +++ /dev/null @@ -1,90 +0,0 @@ -import reducer from '../reducer'; - -describe('USERS PERMISSIONS | HOOKS | useFetchRole | reducer', () => { - describe('DEFAULT_ACTION', () => { - it('should return the initialState', () => { - const state = { - test: true, - }; - - expect(reducer(state, {})).toEqual(state); - }); - }); - - describe('GET_DATA_ERROR', () => { - it('should set isLoading to false is an error occured', () => { - const action = { - type: 'GET_DATA_ERROR', - }; - const initialState = { - role: {}, - isLoading: true, - }; - const expected = { - role: {}, - isLoading: false, - }; - - expect(reducer(initialState, action)).toEqual(expected); - }); - }); - - describe('GET_DATA_SUCCEEDED', () => { - it('should return the state with the data', () => { - const action = { - type: 'GET_DATA_SUCCEEDED', - role: { - id: 1, - name: 'Authenticated', - description: 'This is the Authenticated role', - permissions: {}, - }, - }; - const initialState = { - role: {}, - isLoading: true, - }; - const expected = { - role: { - id: 1, - name: 'Authenticated', - description: 'This is the Authenticated role', - permissions: {}, - }, - isLoading: false, - }; - - expect(reducer(initialState, action)).toEqual(expected); - }); - }); - - describe('ON_SUBMIT_SUCCEEDED', () => { - it("should set the role's name and description correctly", () => { - const state = { - role: { - id: 1, - name: 'Authenticated', - description: 'This is the Authenticated role', - permissions: {}, - }, - }; - - const action = { - type: 'ON_SUBMIT_SUCCEEDED', - name: 'Public', - description: 'test', - }; - - const expected = { - role: { - id: 1, - name: 'Public', - description: 'test', - permissions: {}, - }, - }; - - expect(reducer(state, action)).toEqual(expected); - }); - }); -}); diff --git a/packages/plugins/users-permissions/admin/src/hooks/usePlugins.js b/packages/plugins/users-permissions/admin/src/hooks/usePlugins.js index 0fbfa02be9..4e910b4b9e 100644 --- a/packages/plugins/users-permissions/admin/src/hooks/usePlugins.js +++ b/packages/plugins/users-permissions/admin/src/hooks/usePlugins.js @@ -22,17 +22,21 @@ export const usePlugins = () => { { queryKey: ['users-permissions', 'permissions'], async queryFn() { - const res = await get(`/users-permissions/permissions`); + const { + data: { permissions }, + } = await get(`/users-permissions/permissions`); - return res.data.permissions; + return permissions; }, }, { queryKey: ['users-permissions', 'routes'], async queryFn() { - const res = await get(`/users-permissions/routes`); + const { + data: { routes }, + } = await get(`/users-permissions/routes`); - return res.data.routes; + return routes; }, }, ]); @@ -62,8 +66,12 @@ export const usePlugins = () => { const isLoading = isLoadingPermissions || isLoadingRoutes; return { + // TODO: these return values need to be memoized, otherwise + // they will create infinite rendering loops when used as + // effect dependencies permissions: permissions ? cleanPermissions(permissions) : {}, routes: routes ?? {}, + getData: refetchQueries, isLoading, }; diff --git a/packages/plugins/users-permissions/admin/src/pages/Roles/pages/EditPage.js b/packages/plugins/users-permissions/admin/src/pages/Roles/pages/EditPage.js index cc5ef15294..ca13cef6b4 100644 --- a/packages/plugins/users-permissions/admin/src/pages/Roles/pages/EditPage.js +++ b/packages/plugins/users-permissions/admin/src/pages/Roles/pages/EditPage.js @@ -13,23 +13,22 @@ import { Grid, } from '@strapi/design-system'; import { - useFetchClient, useOverlayBlocker, SettingsPageTitle, LoadingIndicatorPage, Form, useAPIErrorHandler, + useFetchClient, useNotification, Link, } from '@strapi/helper-plugin'; import { ArrowLeft, Check } from '@strapi/icons'; import { Formik } from 'formik'; import { useIntl } from 'react-intl'; -import { useMutation } from 'react-query'; +import { useQuery, useMutation } from 'react-query'; import { useRouteMatch } from 'react-router-dom'; import UsersPermissions from '../../../components/UsersPermissions'; -import { useFetchRole } from '../../../hooks/useFetchRole'; import { usePlugins } from '../../../hooks/usePlugins'; import getTrad from '../../../utils/getTrad'; import { createRoleSchema } from '../constants'; @@ -41,8 +40,21 @@ export const EditPage = () => { const { params: { id }, } = useRouteMatch(`/settings/users-permissions/roles/:id`); + const { get } = useFetchClient(); const { isLoading: isLoadingPlugins, routes } = usePlugins(); - const { role, onSubmitSucceeded, isLoading: isLoadingRole } = useFetchRole(id); + const { + data: role, + isLoading: isLoadingRole, + refetch: refetchRole, + } = useQuery(['users-permissions', 'role', id], async () => { + // TODO: why doesn't this endpoint follow the admin API conventions? + const { + data: { role }, + } = await get(`/users-permissions/roles/${id}`); + + return role; + }); + const permissionsRef = React.useRef(); const { put } = useFetchClient(); const { formatAPIError } = useAPIErrorHandler(); @@ -54,7 +66,7 @@ export const EditPage = () => { }); }, - onSuccess(data) { + async onSuccess() { toggleNotification({ type: 'success', message: { @@ -63,7 +75,7 @@ export const EditPage = () => { }, }); - onSubmitSucceeded({ name: data.name, description: data.description }); + await refetchRole(); }, });