From 3b3637e584c37f22d870ded760f997cdb89550f4 Mon Sep 17 00:00:00 2001 From: soupette Date: Fri, 2 Apr 2021 15:47:53 +0200 Subject: [PATCH 1/3] Improve CM error management Signed-off-by: soupette --- .../src/containers/ErrorBoundary/index.js | 49 ----------------- .../ErrorBoundary/tests/index.test.js | 22 -------- .../src/containers/PluginDispatcher/index.js | 8 +-- packages/strapi-admin/package.json | 1 + packages/strapi-admin/webpack.alias.js | 1 + .../lib/src/components/ErrorFallback/index.js | 29 ++++++++++ .../strapi-helper-plugin/lib/src/index.js | 1 + .../CollectionTypeRecursivePath/index.js | 55 ++++++++++--------- .../admin/src/containers/ListView/index.js | 6 +- yarn.lock | 7 +++ 10 files changed, 74 insertions(+), 105 deletions(-) delete mode 100644 packages/strapi-admin/admin/src/containers/ErrorBoundary/index.js delete mode 100644 packages/strapi-admin/admin/src/containers/ErrorBoundary/tests/index.test.js create mode 100644 packages/strapi-helper-plugin/lib/src/components/ErrorFallback/index.js diff --git a/packages/strapi-admin/admin/src/containers/ErrorBoundary/index.js b/packages/strapi-admin/admin/src/containers/ErrorBoundary/index.js deleted file mode 100644 index c759509e40..0000000000 --- a/packages/strapi-admin/admin/src/containers/ErrorBoundary/index.js +++ /dev/null @@ -1,49 +0,0 @@ -/** - * - * ErrorBoundary - * - */ - -import React from 'react'; -import PropTypes from 'prop-types'; - -class ErrorBoundary extends React.Component { - // eslint-disable-line react/prefer-stateless-function - state = { error: null, errorInfo: null }; - - componentDidCatch(error, errorInfo) { - this.setState({ - error, - errorInfo, - }); - } - - render() { - const { error, errorInfo } = this.state; - - if (errorInfo) { - return ( -
-

Something went wrong.

-
- {error && error.toString()} -
- {errorInfo.componentStack} -
-
- ); - } - - return this.props.children; - } -} - -ErrorBoundary.defaultProps = { - children: null, -}; - -ErrorBoundary.propTypes = { - children: PropTypes.node, -}; - -export default ErrorBoundary; diff --git a/packages/strapi-admin/admin/src/containers/ErrorBoundary/tests/index.test.js b/packages/strapi-admin/admin/src/containers/ErrorBoundary/tests/index.test.js deleted file mode 100644 index 07d754e90a..0000000000 --- a/packages/strapi-admin/admin/src/containers/ErrorBoundary/tests/index.test.js +++ /dev/null @@ -1,22 +0,0 @@ -import React from 'react'; -import { shallow } from 'enzyme'; - -import ErrorBoundary from '../index'; - -describe('', () => { - it('should not crash', () => { - shallow(); - }); - - it('should render its child', () => { - const Child = () =>
test
; - - const wrapper = shallow( - - - , - ); - - expect(wrapper.find(Child)).toHaveLength(1); - }); -}); diff --git a/packages/strapi-admin/admin/src/containers/PluginDispatcher/index.js b/packages/strapi-admin/admin/src/containers/PluginDispatcher/index.js index 49c382fd31..260d312bf2 100644 --- a/packages/strapi-admin/admin/src/containers/PluginDispatcher/index.js +++ b/packages/strapi-admin/admin/src/containers/PluginDispatcher/index.js @@ -8,12 +8,10 @@ import React, { memo } from 'react'; import PropTypes from 'prop-types'; import { Redirect } from 'react-router-dom'; import { get } from 'lodash'; - -import { BlockerComponent } from 'strapi-helper-plugin'; +import { ErrorBoundary } from 'react-error-boundary'; +import { BlockerComponent, ErrorFallback } from 'strapi-helper-plugin'; import PageTitle from '../../components/PageTitle'; - import { LOGIN_LOGO } from '../../config'; -import ErrorBoundary from '../ErrorBoundary'; export function PluginDispatcher(props) { const { @@ -46,7 +44,7 @@ export function PluginDispatcher(props) { return (
- + +

Something went wrong.

+
+ {error.message} +
+ {error.stack} +
+
+ ); +} + +ErrorFallback.defaultProps = { + error: { message: null }, + // resetErrorBoundary: PropTypes.func, +}; + +ErrorFallback.propTypes = { + error: PropTypes.shape({ message: PropTypes.string }), + // resetErrorBoundary: PropTypes.func, +}; + +export default ErrorFallback; diff --git a/packages/strapi-helper-plugin/lib/src/index.js b/packages/strapi-helper-plugin/lib/src/index.js index 7bdad8d27f..665f06528a 100644 --- a/packages/strapi-helper-plugin/lib/src/index.js +++ b/packages/strapi-helper-plugin/lib/src/index.js @@ -17,6 +17,7 @@ export { default as CircleButton } from './components/CircleButton'; export { default as ContainerFluid } from './components/ContainerFluid'; export { default as ErrorBoundary } from './components/ErrorBoundary'; export { default as ExtendComponent } from './components/ExtendComponent'; +export { default as ErrorFallback } from './components/ErrorFallback'; export { default as FilterButton } from './components/FilterButton'; export { default as GlobalPagination } from './components/GlobalPagination'; export { default as HeaderNav } from './components/HeaderNav'; diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeRecursivePath/index.js b/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeRecursivePath/index.js index e21871a1d4..df3dd64c43 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeRecursivePath/index.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeRecursivePath/index.js @@ -1,8 +1,9 @@ import React, { memo, useMemo } from 'react'; import { Switch, Route } from 'react-router-dom'; +import { ErrorBoundary } from 'react-error-boundary'; import { get } from 'lodash'; import PropTypes from 'prop-types'; -import { LoadingIndicatorPage, CheckPagePermissions } from 'strapi-helper-plugin'; +import { ErrorFallback, LoadingIndicatorPage, CheckPagePermissions } from 'strapi-helper-plugin'; import pluginPermissions from '../../permissions'; import { ContentTypeLayoutContext } from '../../contexts'; import { useFetchContentTypeLayout } from '../../hooks'; @@ -82,31 +83,33 @@ const CollectionTypeRecursivePath = ({ )); return ( - - - - - - - - - - - - - {routes} - - + + + + + + + + + + + + + + {routes} + + + ); }; diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/ListView/index.js b/packages/strapi-plugin-content-manager/admin/src/containers/ListView/index.js index e704dbc37b..c7e5776700 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/ListView/index.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/ListView/index.js @@ -10,14 +10,14 @@ import { Flex, Padded } from '@buffetjs/core'; import isEqual from 'react-fast-compare'; import { stringify } from 'qs'; import { - PopUpWarning, - request, CheckPermissions, - useGlobalContext, InjectionZone, InjectionZoneList, + PopUpWarning, + useGlobalContext, useQueryParams, useUser, + request, } from 'strapi-helper-plugin'; import pluginId from '../../pluginId'; import pluginPermissions from '../../permissions'; diff --git a/yarn.lock b/yarn.lock index 0a482e4b65..92bd94322f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16327,6 +16327,13 @@ react-dom@^16.9.0: prop-types "^15.6.2" scheduler "^0.19.1" +react-error-boundary@3.1.1: + version "3.1.1" + resolved "https://registry.yarnpkg.com/react-error-boundary/-/react-error-boundary-3.1.1.tgz#932c5ca5cbab8ec4fe37fd7b415aa5c3a47597e7" + integrity sha512-W3xCd9zXnanqrTUeViceufD3mIW8Ut29BUD+S2f0eO2XCOU8b6UrJfY46RDGe5lxCJzfe4j0yvIfh0RbTZhKJw== + dependencies: + "@babel/runtime" "^7.12.5" + react-fast-compare@^2.0.1: version "2.0.4" resolved "https://registry.yarnpkg.com/react-fast-compare/-/react-fast-compare-2.0.4.tgz#e84b4d455b0fec113e0402c329352715196f81f9" From 119fdd946d6b3fe2017a871040216ff5f6538143 Mon Sep 17 00:00:00 2001 From: soupette Date: Fri, 2 Apr 2021 16:06:55 +0200 Subject: [PATCH 2/3] Ensure the LV always does not have an empty query Signed-off-by: soupette --- .../containers/CollectionTypeFormWrapper/index.js | 8 ++++---- .../CollectionTypeFormWrapper/utils/index.js | 2 +- .../src/containers/ListViewLayoutManager/index.js | 12 +++++++++++- .../admin/src/hooks/index.js | 1 + .../src/hooks/useFindRedirectionLink/index.js | 14 ++++++++++++++ .../useFindRedirectionLink}/selectors.js | 0 .../utils/getRedirectionLink.js | 0 .../utils/tests/getRedirectionLink.test.js | 0 .../admin/src/utils/formatFiltersFromQuery.js | 2 +- 9 files changed, 32 insertions(+), 7 deletions(-) create mode 100644 packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/index.js rename packages/strapi-plugin-content-manager/admin/src/{containers/CollectionTypeFormWrapper => hooks/useFindRedirectionLink}/selectors.js (100%) rename packages/strapi-plugin-content-manager/admin/src/{containers/CollectionTypeFormWrapper => hooks/useFindRedirectionLink}/utils/getRedirectionLink.js (100%) rename packages/strapi-plugin-content-manager/admin/src/{containers/CollectionTypeFormWrapper => hooks/useFindRedirectionLink}/utils/tests/getRedirectionLink.test.js (100%) diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/index.js b/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/index.js index 1544777809..376bb9b869 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/index.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/index.js @@ -13,6 +13,7 @@ import PropTypes from 'prop-types'; import isEqual from 'react-fast-compare'; import { createDefaultForm, getTrad, removePasswordFieldsFromData } from '../../utils'; import pluginId from '../../pluginId'; +import { useFindRedirectionLink } from '../../hooks'; import { getData, getDataSucceeded, @@ -23,8 +24,8 @@ import { submitSucceeded, } from '../../sharedReducers/crudReducer/actions'; import selectCrudReducer from '../../sharedReducers/crudReducer/selectors'; -import { getRedirectionLink, getRequestUrl } from './utils'; -import selectMenuLinks from './selectors'; +import { getRequestUrl } from './utils'; + // This container is used to handle the CRUD const CollectionTypeFormWrapper = ({ allLayoutData, children, slug, id, origin }) => { const { emitEvent } = useGlobalContext(); @@ -38,8 +39,7 @@ const CollectionTypeFormWrapper = ({ allLayoutData, children, slug, id, origin } isLoading, status, } = useSelector(selectCrudReducer); - const collectionTypesMenuLinks = useSelector(selectMenuLinks); - const redirectionLink = getRedirectionLink(collectionTypesMenuLinks, slug, rawQuery); + const redirectionLink = useFindRedirectionLink(slug); const isMounted = useRef(true); const emitEventRef = useRef(emitEvent); diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/index.js b/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/index.js index f9bcbe1a74..966ba7fe36 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/index.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/index.js @@ -1,2 +1,2 @@ -export { default as getRedirectionLink } from './getRedirectionLink'; +// eslint-disable-next-line import/prefer-default-export export { default as getRequestUrl } from './getRequestUrl'; diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/ListViewLayoutManager/index.js b/packages/strapi-plugin-content-manager/admin/src/containers/ListViewLayoutManager/index.js index 156d6fd423..d3d85d23cf 100644 --- a/packages/strapi-plugin-content-manager/admin/src/containers/ListViewLayoutManager/index.js +++ b/packages/strapi-plugin-content-manager/admin/src/containers/ListViewLayoutManager/index.js @@ -1,15 +1,25 @@ import React, { useEffect } from 'react'; import { useDispatch } from 'react-redux'; import PropTypes from 'prop-types'; +import { useHistory } from 'react-router-dom'; import { useQueryParams } from 'strapi-helper-plugin'; +import { useFindRedirectionLink } from '../../hooks'; import { resetProps, setLayout } from '../ListView/actions'; import useSyncRbac from '../RBACManager/useSyncRbac'; import Permissions from './Permissions'; const ListViewLayout = ({ layout, ...props }) => { const dispatch = useDispatch(); - const [{ query }] = useQueryParams(); + const { replace } = useHistory(); + const [{ query, rawQuery }] = useQueryParams(); const permissions = useSyncRbac(query, props.slug, 'listView'); + const redirectionLink = useFindRedirectionLink(props.slug); + + useEffect(() => { + if (!rawQuery) { + replace(redirectionLink); + } + }, [rawQuery, replace, redirectionLink]); useEffect(() => { dispatch(setLayout(layout.contentType)); diff --git a/packages/strapi-plugin-content-manager/admin/src/hooks/index.js b/packages/strapi-plugin-content-manager/admin/src/hooks/index.js index 4e11f3bd05..8d93474141 100644 --- a/packages/strapi-plugin-content-manager/admin/src/hooks/index.js +++ b/packages/strapi-plugin-content-manager/admin/src/hooks/index.js @@ -1,5 +1,6 @@ export { default as useContentTypeLayout } from './useContentTypeLayout'; export { default as useFetchContentTypeLayout } from './useFetchContentTypeLayout'; +export { default as useFindRedirectionLink } from './useFindRedirectionLink'; export { default as useLayoutDnd } from './useLayoutDnd'; export { default as useListView } from './useListView'; export { default as useWysiwyg } from './useWysiwyg'; diff --git a/packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/index.js b/packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/index.js new file mode 100644 index 0000000000..4d51334bb3 --- /dev/null +++ b/packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/index.js @@ -0,0 +1,14 @@ +import { useSelector } from 'react-redux'; +import { useQueryParams } from 'strapi-helper-plugin'; +import selectMenuLinks from './selectors'; +import getRedirectionLink from './utils/getRedirectionLink'; + +const useFindRedirectionLink = slug => { + const [{ rawQuery }] = useQueryParams(); + const collectionTypesMenuLinks = useSelector(selectMenuLinks); + const redirectionLink = getRedirectionLink(collectionTypesMenuLinks, slug, rawQuery); + + return redirectionLink; +}; + +export default useFindRedirectionLink; diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/selectors.js b/packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/selectors.js similarity index 100% rename from packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/selectors.js rename to packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/selectors.js diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/getRedirectionLink.js b/packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/utils/getRedirectionLink.js similarity index 100% rename from packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/getRedirectionLink.js rename to packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/utils/getRedirectionLink.js diff --git a/packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/tests/getRedirectionLink.test.js b/packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/utils/tests/getRedirectionLink.test.js similarity index 100% rename from packages/strapi-plugin-content-manager/admin/src/containers/CollectionTypeFormWrapper/utils/tests/getRedirectionLink.test.js rename to packages/strapi-plugin-content-manager/admin/src/hooks/useFindRedirectionLink/utils/tests/getRedirectionLink.test.js diff --git a/packages/strapi-plugin-content-manager/admin/src/utils/formatFiltersFromQuery.js b/packages/strapi-plugin-content-manager/admin/src/utils/formatFiltersFromQuery.js index 46c856f3b2..39c260e28e 100644 --- a/packages/strapi-plugin-content-manager/admin/src/utils/formatFiltersFromQuery.js +++ b/packages/strapi-plugin-content-manager/admin/src/utils/formatFiltersFromQuery.js @@ -15,7 +15,7 @@ const VALID_REST_OPERATORS = [ 'null', ]; -// from strapi-utims/convert-rest-query-params +// from strapi-utils/convert-rest-query-params const findAppliedFilter = whereClause => { // Useful to remove the mainField of relation fields. const formattedWhereClause = whereClause.split('.')[0]; From e45ac1e39637c89fed6fcdede82f479e3a2a37a3 Mon Sep 17 00:00:00 2001 From: soupette Date: Tue, 6 Apr 2021 09:50:05 +0200 Subject: [PATCH 3/3] Fix PR feedback Signed-off-by: soupette --- .../lib/src/components/ErrorFallback/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/strapi-helper-plugin/lib/src/components/ErrorFallback/index.js b/packages/strapi-helper-plugin/lib/src/components/ErrorFallback/index.js index 944ffbcee1..ce52281095 100644 --- a/packages/strapi-helper-plugin/lib/src/components/ErrorFallback/index.js +++ b/packages/strapi-helper-plugin/lib/src/components/ErrorFallback/index.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; // https://github.com/bvaughn/react-error-boundary#usage -function ErrorFallback({ error /* resetErrorBoundary */ }) { +function ErrorFallback({ error }) { return (

Something went wrong.

@@ -18,12 +18,10 @@ function ErrorFallback({ error /* resetErrorBoundary */ }) { ErrorFallback.defaultProps = { error: { message: null }, - // resetErrorBoundary: PropTypes.func, }; ErrorFallback.propTypes = { error: PropTypes.shape({ message: PropTypes.string }), - // resetErrorBoundary: PropTypes.func, }; export default ErrorFallback;