Merge pull request #17669 from strapi/chore/application-infos

Chore: Refactor data-fetching and simplify ApplicationInfosPage

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

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 {

packages/core/admin/admin/src/pages/SettingsPage/index.js

@@ -1,15 +1,4 @@
-/**
+import * as React from 'react';
- *
- * 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 { 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 <Redirect to="/settings/application-infos" />;
   }
 
-  const settingTitle = formatMessage({
-    id: 'global.settings',
-    defaultMessage: 'Settings',
-  });
-
   return (
     <Layout sideNav={<SettingsNav menu={menu} />}>
-      <Helmet title={settingTitle} />
+      <Helmet
+        title={formatMessage({
+          id: 'global.settings',
+          defaultMessage: 'Settings',
+        })}
+      />
 
       <Switch>
         <Route path="/settings/application-infos" component={ApplicationInfosPage} exact />
@@ -78,6 +71,3 @@ function SettingsPage() {
     </Layout>
   );
 }
-
-export default memo(SettingsPage);
-export { SettingsPage };

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();
+  const { get, post } = useFetchClient();
-  useFocusWhenNavigate();
+  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, {
+  useFocusWhenNavigate();
-    enabled: canRead,
-  });
 
-  const submitMutation = useMutation((body) => postProjectSettings(body), {
+  const { data, isLoading } = useQuery(
-    async onSuccess({ menuLogo, authLogo }) {
+    ['project-settings'],
-      await queryClient.invalidateQueries('project-settings', { refetchActive: true });
+    async () => {
-      updateProjectSettings({ menuLogo: menuLogo?.url, authLogo: authLogo?.url });
+      const { data } = await get('/admin/project-settings');
+
+      return data;
     },
-  });
+    {
+      cacheTime: 0,
+      enabled: canRead,
+      select(data) {
+        return {
+          ...data,
 
-  const handleSubmit = (e) => {
+          authLogo: data.authLogo
-    e.preventDefault();
+            ? {
+                ...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 submitMutation = useMutation(
-    const formData = getFormData(inputValues);
+    (body) =>
+      post('/admin/project-settings', body, {
+        headers: {
+          'Content-Type': 'multipart/form-data',
+        },
+      }),
+    {
+      onError(error) {
+        toggleNotification({
+          type: 'warning',
+          message: formatAPIError(error),
+        });
+      },
 
-    submitMutation.mutate(formData, {
+      async onSuccess(data) {
-      onSuccess() {
+        const { menuLogo, authLogo } = data;
-        const { menuLogo, authLogo } = inputValues;
 
-        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 && (
                   <Button type="submit" startIcon={<Check />}>
                     {formatMessage({ id: 'global.save', defaultMessage: 'Save' })}
                   </Button>

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', () => ({

packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/api.js

@@ -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 };

packages/core/admin/admin/src/pages/SettingsPage/pages/ApplicationInfosPage/utils/prefixAllUrls.js

@@ -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;

packages/core/admin/admin/src/pages/SettingsPage/utils/createSectionsRoutes.js

@@ -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;

packages/core/admin/admin/src/pages/SettingsPage/utils/getSectionsToDisplay.js

@@ -1,5 +0,0 @@
-const getSectionsToDisplay = (menu) => {
-  return menu.filter((section) => !section.links.every((link) => link.isDisplayed === false));
-};
-
-export default getSectionsToDisplay;

packages/core/admin/admin/src/pages/SettingsPage/utils/index.js

@@ -1,2 +0,0 @@
-export { default as createSectionsRoutes } from './createSectionsRoutes';
-export { default as getSectionsToDisplay } from './getSectionsToDisplay';

packages/core/admin/admin/src/pages/SettingsPage/utils/tests/createSectionsRoutes.js

@@ -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');
-    });
-  });
-});

packages/core/admin/admin/src/pages/SettingsPage/utils/tests/getSectionsToDisplay.test.js

@@ -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');
-  });
-});