From da16cb301f48c290892920f9c7c0ce1d6e3f8f7e Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Mon, 14 Aug 2023 13:03:29 +0200 Subject: [PATCH] Chore: Refactor data-fetching and simplify ApplicationInfosPage --- .../core/admin/admin/src/pages/Admin/index.js | 4 +- .../components/SettingsNav/index.js | 6 +- .../admin/src/pages/SettingsPage/index.js | 42 +++---- .../pages/ApplicationInfosPage/index.js | 104 ++++++++++++------ .../ApplicationInfosPage/tests/index.test.js | 14 --- .../pages/ApplicationInfosPage/utils/api.js | 23 ---- .../utils/prefixAllUrls.js | 17 --- .../utils/createSectionsRoutes.js | 11 -- .../utils/getSectionsToDisplay.js | 5 - .../src/pages/SettingsPage/utils/index.js | 2 - .../utils/tests/createSectionsRoutes.js | 40 ------- .../utils/tests/getSectionsToDisplay.test.js | 39 ------- 12 files changed, 91 insertions(+), 216 deletions(-) delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/api.js delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/prefixAllUrls.js delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/utils/createSectionsRoutes.js delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/utils/getSectionsToDisplay.js delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/utils/index.js delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/utils/tests/createSectionsRoutes.js delete mode 100644 packages/core/admin/admin/src/pages/SettingsPage/utils/tests/getSectionsToDisplay.test.js diff --git a/packages/core/admin/admin/src/pages/Admin/index.js b/packages/core/admin/admin/src/pages/Admin/index.js index 9b22045af3..de3f0d8139 100644 --- a/packages/core/admin/admin/src/pages/Admin/index.js +++ b/packages/core/admin/admin/src/pages/Admin/index.js @@ -42,7 +42,9 @@ const ProfilePage = lazy(() => import(/* webpackChunkName: "Admin_profilePage" */ '../ProfilePage') ); const SettingsPage = lazy(() => - import(/* webpackChunkName: "Admin_settingsPage" */ '../SettingsPage') + import(/* webpackChunkName: "Admin_settingsPage" */ '../SettingsPage').then((module) => ({ + default: module.SettingsPage, + })) ); // Simple hook easier for testing diff --git a/packages/core/admin/admin/src/pages/SettingsPage/components/SettingsNav/index.js b/packages/core/admin/admin/src/pages/SettingsPage/components/SettingsNav/index.js index 4c86686c7a..07265d8230 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/components/SettingsNav/index.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/components/SettingsNav/index.js @@ -12,14 +12,14 @@ import PropTypes from 'prop-types'; import { useIntl } from 'react-intl'; import { NavLink, useLocation } from 'react-router-dom'; -import { getSectionsToDisplay } from '../../utils'; - const SettingsNav = ({ menu }) => { const { formatMessage } = useIntl(); const { trackUsage } = useTracking(); const { pathname } = useLocation(); - const filteredMenu = getSectionsToDisplay(menu); + const filteredMenu = menu.filter( + (section) => !section.links.every((link) => link.isDisplayed === false) + ); const sections = filteredMenu.map((section) => { return { diff --git a/packages/core/admin/admin/src/pages/SettingsPage/index.js b/packages/core/admin/admin/src/pages/SettingsPage/index.js index 8cb573afe8..a647420d44 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/index.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/index.js @@ -1,15 +1,4 @@ -/** - * - * SettingsPage - * - */ - -// NOTE TO PLUGINS DEVELOPERS: -// If you modify this file you also need to update the documentation accordingly -// Here's the file: strapi/docs/3.0.0-beta.x/plugin-development/frontend-settings-api.md -// IF THE DOC IS NOT UPDATED THE PULL REQUEST WILL NOT BE MERGED - -import React, { memo, useMemo } from 'react'; +import * as React from 'react'; import { Layout } from '@strapi/design-system'; import { LoadingIndicatorPage, useStrapiApp } from '@strapi/helper-plugin'; @@ -19,14 +8,14 @@ import { Redirect, Route, Switch, useParams } from 'react-router-dom'; import { useSettingsMenu } from '../../hooks'; import { useEnterprise } from '../../hooks/useEnterprise'; -import { createRoute, makeUniqueRoutes } from '../../utils'; +import createRoute from '../../utils/createRoute'; +import makeUniqueRoutes from '../../utils/makeUniqueRoutes'; import SettingsNav from './components/SettingsNav'; import { ROUTES_CE } from './constants'; import ApplicationInfosPage from './pages/ApplicationInfosPage'; -import { createSectionsRoutes } from './utils'; -function SettingsPage() { +export function SettingsPage() { const { settingId } = useParams(); const { settings } = useStrapiApp(); const { formatMessage } = useIntl(); @@ -43,13 +32,17 @@ function SettingsPage() { ); // Creates the admin routes - const adminRoutes = useMemo(() => { + const adminRoutes = React.useMemo(() => { return makeUniqueRoutes( routes.map(({ to, Component, exact }) => createRoute(Component, to, exact)) ); }, [routes]); - const pluginsRoutes = createSectionsRoutes(settings); + const pluginsRoutes = Object.values(settings).flatMap((section) => { + const { links } = section; + + return links.map((link) => createRoute(link.Component, link.to, link.exact || false)); + }); // Since the useSettingsMenu hook can make API calls in order to check the links permissions // We need to add a loading state to prevent redirecting the user while permissions are being checked @@ -61,14 +54,14 @@ function SettingsPage() { return ; } - const settingTitle = formatMessage({ - id: 'global.settings', - defaultMessage: 'Settings', - }); - return ( }> - + @@ -78,6 +71,3 @@ function SettingsPage() { ); } - -export default memo(SettingsPage); -export { SettingsPage }; diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/index.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/index.js index f1261d3dde..d863c007df 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/index.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/index.js @@ -1,4 +1,4 @@ -import React, { useRef } from 'react'; +import * as React from 'react'; import { Button, @@ -14,8 +14,11 @@ import { Typography, } from '@strapi/design-system'; import { + prefixFileUrlWithBackendUrl, SettingsPageTitle, + useAPIErrorHandler, useAppInfo, + useFetchClient, useFocusWhenNavigate, useNotification, useRBAC, @@ -23,7 +26,7 @@ import { } from '@strapi/helper-plugin'; import { Check, ExternalLink } from '@strapi/icons'; import { useIntl } from 'react-intl'; -import { useMutation, useQuery, useQueryClient } from 'react-query'; +import { useMutation, useQuery } from 'react-query'; import { useSelector } from 'react-redux'; import { useConfigurations } from '../../../../hooks'; @@ -31,18 +34,20 @@ import { useEnterprise } from '../../../../hooks/useEnterprise'; import { selectAdminPermissions } from '../../../App/selectors'; import CustomizationInfos from './components/CustomizationInfos'; -import { fetchProjectSettings, postProjectSettings } from './utils/api'; import getFormData from './utils/getFormData'; const AdminSeatInfoCE = () => null; const ApplicationInfosPage = () => { - const inputsRef = useRef(); + const inputsRef = React.useRef(); const toggleNotification = useNotification(); const { trackUsage } = useTracking(); const { formatMessage } = useIntl(); - const queryClient = useQueryClient(); - useFocusWhenNavigate(); + const { get, post } = useFetchClient(); + const { updateProjectSettings } = useConfigurations(); + const permissions = useSelector(selectAdminPermissions); + const { formatAPIError } = useAPIErrorHandler(); + const { communityEdition, latestStrapiReleaseTag, @@ -50,8 +55,7 @@ const ApplicationInfosPage = () => { shouldUpdateStrapi, strapiVersion, } = useAppInfo(); - const { updateProjectSettings } = useConfigurations(); - const permissions = useSelector(selectAdminPermissions); + const AdminSeatInfo = useEnterprise( AdminSeatInfoCE, async () => @@ -65,38 +69,68 @@ const ApplicationInfosPage = () => { const { allowedActions: { canRead, canUpdate }, } = useRBAC(permissions.settings['project-settings']); - const canSubmit = canRead && canUpdate; - const { data, isLoading } = useQuery('project-settings', fetchProjectSettings, { - enabled: canRead, - }); + useFocusWhenNavigate(); - const submitMutation = useMutation((body) => postProjectSettings(body), { - async onSuccess({ menuLogo, authLogo }) { - await queryClient.invalidateQueries('project-settings', { refetchActive: true }); - updateProjectSettings({ menuLogo: menuLogo?.url, authLogo: authLogo?.url }); + const { data, isLoading } = useQuery( + ['project-settings'], + async () => { + const { data } = await get('/admin/project-settings'); + + return data; }, - }); + { + cacheTime: 0, + enabled: canRead, + select(data) { + return { + ...data, - const handleSubmit = (e) => { - e.preventDefault(); + authLogo: data.authLogo + ? { + ...data.authLogo, + url: prefixFileUrlWithBackendUrl(data.authLogo.url), + } + : data.authLogo, - if (!canUpdate) return; + menuLogo: data.menuLogo + ? { + ...data.menuLogo, + url: prefixFileUrlWithBackendUrl(data.menuLogo.url), + } + : data.menuLogo, + }; + }, + } + ); - const inputValues = inputsRef.current.getValues(); - const formData = getFormData(inputValues); + const submitMutation = useMutation( + (body) => + post('/admin/project-settings', body, { + headers: { + 'Content-Type': 'multipart/form-data', + }, + }), + { + onError(error) { + toggleNotification({ + type: 'warning', + message: formatAPIError(error), + }); + }, - submitMutation.mutate(formData, { - onSuccess() { - const { menuLogo, authLogo } = inputValues; + async onSuccess(data) { + const { menuLogo, authLogo } = data; - if (menuLogo.rawFile) { + updateProjectSettings({ menuLogo: menuLogo?.url, authLogo: authLogo?.url }); + + if (menuLogo?.rawFile) { trackUsage('didChangeLogo', { logo: 'menu', }); } - if (authLogo.rawFile) { + if (authLogo?.rawFile) { trackUsage('didChangeLogo', { logo: 'auth', }); @@ -107,13 +141,13 @@ const ApplicationInfosPage = () => { message: formatMessage({ id: 'app', defaultMessage: 'Saved' }), }); }, - onError() { - toggleNotification({ - type: 'warning', - message: { id: 'notification.error', defaultMessage: 'An error occurred' }, - }); - }, - }); + } + ); + + const handleSubmit = (e) => { + e.preventDefault(); + + submitMutation.mutate(getFormData(inputsRef.current.getValues())); }; // block rendering until the EE component is fully loaded @@ -145,7 +179,7 @@ const ApplicationInfosPage = () => { defaultMessage: 'Administration panel’s global information', })} primaryAction={ - canSubmit && ( + canUpdate && ( diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/tests/index.test.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/tests/index.test.js index dcd3d367d0..c468e94169 100644 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/tests/index.test.js +++ b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/tests/index.test.js @@ -56,20 +56,6 @@ jest.mock('@strapi/helper-plugin', () => ({ useAppInfo: jest.fn(() => ({ shouldUpdateStrapi: false, latestStrapiReleaseTag: 'v3.6.8' })), useNotification: jest.fn(() => jest.fn()), useRBAC: jest.fn(() => ({ allowedActions: { canRead: true, canUpdate: true } })), - useFetchClient: jest.fn().mockReturnValue({ - get: jest.fn().mockResolvedValue({ - data: { - menuLogo: { - ext: 'png', - height: 256, - name: 'image.png', - size: 27.4, - url: 'uploads/image_fe95c5abb9.png', - width: 246, - }, - }, - }), - }), })); jest.mock('../../../../../hooks', () => ({ diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/api.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/api.js deleted file mode 100644 index e82ae1ae3a..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/api.js +++ /dev/null @@ -1,23 +0,0 @@ -import { getFetchClient } from '@strapi/helper-plugin'; - -import prefixAllUrls from './prefixAllUrls'; - -const fetchProjectSettings = async () => { - const { get } = getFetchClient(); - const { data } = await get('/admin/project-settings'); - - return prefixAllUrls(data); -}; - -const postProjectSettings = async (body) => { - const { post } = getFetchClient(); - const { data } = await post('/admin/project-settings', body, { - headers: { - 'Content-Type': 'multipart/form-data', - }, - }); - - return prefixAllUrls(data); -}; - -export { fetchProjectSettings, postProjectSettings }; diff --git a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/prefixAllUrls.js b/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/prefixAllUrls.js deleted file mode 100644 index b4836a3312..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/prefixAllUrls.js +++ /dev/null @@ -1,17 +0,0 @@ -import { prefixFileUrlWithBackendUrl } from '@strapi/helper-plugin'; -import transform from 'lodash/transform'; - -const prefixAllUrls = (data) => - transform( - data, - (result, value, key) => { - if (value && value.url) { - result[key] = { ...value, url: prefixFileUrlWithBackendUrl(value.url) }; - } else { - result[key] = value; - } - }, - {} - ); - -export default prefixAllUrls; diff --git a/packages/core/admin/admin/src/pages/SettingsPage/utils/createSectionsRoutes.js b/packages/core/admin/admin/src/pages/SettingsPage/utils/createSectionsRoutes.js deleted file mode 100644 index 38dd49bb5c..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/utils/createSectionsRoutes.js +++ /dev/null @@ -1,11 +0,0 @@ -import flatMap from 'lodash/flatMap'; - -import { createRoute } from '../../../utils'; - -const createSectionsRoutes = (settings) => { - const allLinks = flatMap(settings, (section) => section.links); - - return allLinks.map((link) => createRoute(link.Component, link.to, link.exact || false)); -}; - -export default createSectionsRoutes; diff --git a/packages/core/admin/admin/src/pages/SettingsPage/utils/getSectionsToDisplay.js b/packages/core/admin/admin/src/pages/SettingsPage/utils/getSectionsToDisplay.js deleted file mode 100644 index fda9b5856f..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/utils/getSectionsToDisplay.js +++ /dev/null @@ -1,5 +0,0 @@ -const getSectionsToDisplay = (menu) => { - return menu.filter((section) => !section.links.every((link) => link.isDisplayed === false)); -}; - -export default getSectionsToDisplay; diff --git a/packages/core/admin/admin/src/pages/SettingsPage/utils/index.js b/packages/core/admin/admin/src/pages/SettingsPage/utils/index.js deleted file mode 100644 index 3b1fbde662..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/utils/index.js +++ /dev/null @@ -1,2 +0,0 @@ -export { default as createSectionsRoutes } from './createSectionsRoutes'; -export { default as getSectionsToDisplay } from './getSectionsToDisplay'; diff --git a/packages/core/admin/admin/src/pages/SettingsPage/utils/tests/createSectionsRoutes.js b/packages/core/admin/admin/src/pages/SettingsPage/utils/tests/createSectionsRoutes.js deleted file mode 100644 index c7fc814c0f..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/utils/tests/createSectionsRoutes.js +++ /dev/null @@ -1,40 +0,0 @@ -import createSectionsRoutes from '../createSectionsRoutes'; - -describe('ADMIN | CONTAINERS | SettingsPage | utils', () => { - describe('createSectionsRoutes', () => { - it('should return an empty array', () => { - expect(createSectionsRoutes([])).toEqual([]); - }); - - it('should return the correct data', () => { - const data = [ - { - id: 'global', - links: [], - }, - { - id: 'permissions', - links: [ - { - Component: () => 'test', - to: '/test', - exact: true, - }, - { - Component: null, - to: '/test1', - exact: true, - }, - ], - }, - ]; - - const results = createSectionsRoutes(data); - - expect(results).toHaveLength(1); - expect(results[0].key).toEqual('/test'); - expect(results[0].props.path).toEqual('/test'); - expect(results[0].props.component()).toEqual('test'); - }); - }); -}); diff --git a/packages/core/admin/admin/src/pages/SettingsPage/utils/tests/getSectionsToDisplay.test.js b/packages/core/admin/admin/src/pages/SettingsPage/utils/tests/getSectionsToDisplay.test.js deleted file mode 100644 index 59cc7cb33d..0000000000 --- a/packages/core/admin/admin/src/pages/SettingsPage/utils/tests/getSectionsToDisplay.test.js +++ /dev/null @@ -1,39 +0,0 @@ -import getSectionsToDisplay from '../getSectionsToDisplay'; - -describe('ADMIN | Container | SettingsPage | utils | getSectionToDisplay', () => { - it('should filter the sections that have all links with the isDisplayed property to false', () => { - const data = [ - { - id: 'global', - links: [ - { - isDisplayed: false, - }, - { - isDisplayed: false, - }, - ], - }, - { - id: 'permissions', - links: [ - { - isDisplayed: true, - }, - { - isDisplayed: false, - }, - ], - }, - { - id: 'test', - links: [], - }, - ]; - - const results = getSectionsToDisplay(data); - - expect(results).toHaveLength(1); - expect(results[0].id).toEqual('permissions'); - }); -});