From 711fdef14dbf100c73f655330fcc729f38db6103 Mon Sep 17 00:00:00 2001 From: John Coburn Date: Tue, 25 Feb 2025 15:33:32 -0600 Subject: [PATCH 1/8] break QueryStateUpdates out to their own class-based component. --- src/RootWithIntl.js | 2 + src/components/MainNav/MainNav.js | 55 +------------ src/components/MainNav/QueryStateUpdater.js | 87 +++++++++++++++++++++ src/components/index.js | 1 + 4 files changed, 91 insertions(+), 54 deletions(-) create mode 100644 src/components/MainNav/QueryStateUpdater.js diff --git a/src/RootWithIntl.js b/src/RootWithIntl.js index 0a1391b8..337285e7 100644 --- a/src/RootWithIntl.js +++ b/src/RootWithIntl.js @@ -36,6 +36,7 @@ import { AppCtxMenuProvider, SessionEventContainer, AppOrderProvider, + QueryStateUpdater } from './components'; import StaleBundleWarning from './components/StaleBundleWarning'; import { StripesContext } from './StripesContext'; @@ -64,6 +65,7 @@ const RootWithIntl = ({ stripes, token = '', isAuthenticated = false, disableAut { isAuthenticated || token || disableAuth ? <> + diff --git a/src/components/MainNav/MainNav.js b/src/components/MainNav/MainNav.js index c8caf92e..70fcdf77 100644 --- a/src/components/MainNav/MainNav.js +++ b/src/components/MainNav/MainNav.js @@ -1,19 +1,8 @@ import React, { useEffect, useRef, useState } from 'react'; -import { isEqual } from 'lodash'; import { useIntl } from 'react-intl'; -import { useLocation, useHistory } from 'react-router-dom'; -import { useQueryClient } from 'react-query'; import { branding } from 'stripes-config'; import { Icon } from '@folio/stripes-components'; -import { - updateQueryResource, - getLocationQuery, - updateLocation, - getCurrentModule, - isQueryResourceModule, - getQueryResourceState, -} from '../../locationService'; import css from './MainNav.css'; import NavButton from './NavButton'; @@ -22,65 +11,23 @@ import { CurrentAppGroup } from './CurrentApp'; import ProfileDropdown from './ProfileDropdown'; import AppList from './AppList'; import { SkipLink } from './components'; - import { useAppOrderContext } from './AppOrderProvider'; - import { useStripes } from '../../StripesContext'; -import { useModules } from '../../ModulesContext'; const MainNav = () => { const { apps, } = useAppOrderContext(); - const queryClient = useQueryClient(); const stripes = useStripes(); - const location = useLocation(); - const modules = useModules(); - const history = useHistory(); const intl = useIntl(); - const [curModule, setCurModule] = useState(getCurrentModule(modules, location)); const [selectedApp, setSelectedApp] = useState(apps.find(entry => entry.active)); const helpUrl = useRef(stripes.config.helpUrl ?? 'https://docs.folio.org').current; - useEffect(() => { - let curQuery = getLocationQuery(location); - const prevQueryState = {}; - - const { store } = stripes; - const _unsubscribe = store.subscribe(() => { - const module = curModule; - - if (module && isQueryResourceModule(module, location)) { - const { moduleName } = module; - const queryState = getQueryResourceState(module, stripes.store); - - // only update location if query state has changed - if (!isEqual(queryState, prevQueryState[moduleName])) { - curQuery = updateLocation(module, curQuery, stripes.store, history, location); - prevQueryState[moduleName] = queryState; - } - } - }); - - // remove QueryProvider cache to be 100% sure we're starting from a clean slate. - queryClient.removeQueries(); - - return () => { - _unsubscribe(); - }; - }, [location]); // eslint-disable-line - - // if the location changes, we need to update the current module/query resource. // This logic changes the visible current app at the starting side of the Main Navigation. useEffect(() => { setSelectedApp(apps.find(entry => entry.active)); - const nextCurModule = getCurrentModule(modules, location); - if (nextCurModule) { - setCurModule(getCurrentModule(modules, location)); - updateQueryResource(location, nextCurModule, stripes.store); - } - }, [modules, location, stripes.store, apps]); + }, [apps]); return (
diff --git a/src/components/MainNav/QueryStateUpdater.js b/src/components/MainNav/QueryStateUpdater.js new file mode 100644 index 00000000..4ca7d814 --- /dev/null +++ b/src/components/MainNav/QueryStateUpdater.js @@ -0,0 +1,87 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { compose } from 'redux'; +import { injectIntl } from 'react-intl'; +import { withRouter } from 'react-router'; +import { isEqual } from 'lodash'; + +import { withModules } from '../Modules'; + +import { + updateQueryResource, + getLocationQuery, + updateLocation, + getCurrentModule, + isQueryResourceModule, + getQueryResourceState, +} from '../../locationService'; + +// onMount of stripes, sync the query state to the location. +class QueryStateUpdater extends React.Component { + static PropTypes = { + queryClient: PropTypes.shape({ + removeQueries: PropTypes.func.isRequired, + }).isRequired, + stripes: PropTypes.shape({ + store: PropTypes.shape({ + subscribe: PropTypes.func.isRequired, + }), + }), + } + + constructor(props) { + super(props); + this.store = props.stripes.store; + } + + componentDidMount() { + let curQuery = getLocationQuery(this.props.location); + const prevQueryState = {}; + + this._unsubscribe = this.store.subscribe(() => { + const { history, location } = this.props; + const module = this.curModule; + const state = this.store.getState(); + + // If user has timed out, force them to log in again. + if (state?.okapi?.token && state.okapi.authFailure + && find(state.okapi.authFailure, { type: 'error', code: 'user.timeout' })) { + this.returnToLogin(); + } + + if (module && isQueryResourceModule(module, location)) { + const { moduleName } = module; + const queryState = getQueryResourceState(module, this.store); + + // only update location if query state has changed + if (!isEqual(queryState, prevQueryState[moduleName])) { + curQuery = updateLocation(module, curQuery, this.store, history, location); + prevQueryState[moduleName] = queryState; + } + } + }); + + // remove QueryProvider cache to be 100% sure we're starting from a clean slate. + this.props.queryClient.removeQueries(); + } + + componentDidUpdate(prevProps) { + const { modules, location } = this.props; + this.curModule = getCurrentModule(modules, location); + if (this.curModule && !isEqual(location, prevProps.location)) { + updateQueryResource(location, this.curModule, this.store); + } + } + + componentWillUnmount() { + this._unsubscribe(); + } + + render = () => this.props.children; +}; + +export default compose( + injectIntl, + withRouter, + withModules, +)(QueryStateUpdater); diff --git a/src/components/index.js b/src/components/index.js index 86d950a3..6ff51dad 100644 --- a/src/components/index.js +++ b/src/components/index.js @@ -8,6 +8,7 @@ export { default as Login } from './Login'; export { default as Logout } from './Logout'; export { default as MainContainer } from './MainContainer'; export { default as MainNav } from './MainNav'; +export { default as QueryStateUpdater } from './MainNav/QueryStateUpdater'; export { AppOrderProvider } from './MainNav/AppOrderProvider'; export { default as ModuleContainer } from './ModuleContainer'; export { withModule, withModules } from './Modules'; From 7b5d11a54f818fcc90aff660654cb8df09693df9 Mon Sep 17 00:00:00 2001 From: John Coburn Date: Tue, 25 Feb 2025 16:04:28 -0600 Subject: [PATCH 2/8] mock QueryStateUpdater in RootWithIntl test --- src/RootWithIntl.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/RootWithIntl.test.js b/src/RootWithIntl.test.js index e0b20b42..2e0ecefa 100644 --- a/src/RootWithIntl.test.js +++ b/src/RootWithIntl.test.js @@ -14,6 +14,7 @@ jest.mock('./components/ModuleContainer', () => ({ children }) => children); jest.mock('./components/MainContainer', () => ({ children }) => children); jest.mock('./components/StaleBundleWarning', () => () => ''); jest.mock('./components/SessionEventContainer', () => () => ''); +jest.mock('./components/MainNav/QueryStateUpdater', () => () => null); const defaultHistory = createMemoryHistory(); From 73c559e77913f5b8939ca38102738aa8438b788b Mon Sep 17 00:00:00 2001 From: John Coburn Date: Tue, 25 Feb 2025 16:27:57 -0600 Subject: [PATCH 3/8] add proptypes to the class-based QueryStateUpdater --- src/components/MainNav/QueryStateUpdater.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/components/MainNav/QueryStateUpdater.js b/src/components/MainNav/QueryStateUpdater.js index 4ca7d814..b663e909 100644 --- a/src/components/MainNav/QueryStateUpdater.js +++ b/src/components/MainNav/QueryStateUpdater.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { compose } from 'redux'; import { injectIntl } from 'react-intl'; import { withRouter } from 'react-router'; -import { isEqual } from 'lodash'; +import { isEqual, find } from 'lodash'; import { withModules } from '../Modules'; @@ -18,7 +18,18 @@ import { // onMount of stripes, sync the query state to the location. class QueryStateUpdater extends React.Component { - static PropTypes = { + static propTypes = { + history: PropTypes.shape({ + listen: PropTypes.func.isRequired, + replace: PropTypes.func.isRequired, + push: PropTypes.func.isRequired, + }).isRequired, + location: PropTypes.shape({ + pathname: PropTypes.string, + }).isRequired, + modules: PropTypes.shape({ + app: PropTypes.arrayOf(PropTypes.object), + }), queryClient: PropTypes.shape({ removeQueries: PropTypes.func.isRequired, }).isRequired, From f0fba63a156e70e6060c487a69ee3e06c56a2d37 Mon Sep 17 00:00:00 2001 From: John Coburn Date: Wed, 26 Feb 2025 08:28:06 -0600 Subject: [PATCH 4/8] remove crufty conditional that leaned on non-http-only cookies --- src/components/MainNav/QueryStateUpdater.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/components/MainNav/QueryStateUpdater.js b/src/components/MainNav/QueryStateUpdater.js index b663e909..d8aac61c 100644 --- a/src/components/MainNav/QueryStateUpdater.js +++ b/src/components/MainNav/QueryStateUpdater.js @@ -52,13 +52,6 @@ class QueryStateUpdater extends React.Component { this._unsubscribe = this.store.subscribe(() => { const { history, location } = this.props; const module = this.curModule; - const state = this.store.getState(); - - // If user has timed out, force them to log in again. - if (state?.okapi?.token && state.okapi.authFailure - && find(state.okapi.authFailure, { type: 'error', code: 'user.timeout' })) { - this.returnToLogin(); - } if (module && isQueryResourceModule(module, location)) { const { moduleName } = module; From 0085fae910f211d0330a2367cc49d90b4e40f53c Mon Sep 17 00:00:00 2001 From: John Coburn Date: Mon, 3 Mar 2025 14:52:27 -0600 Subject: [PATCH 5/8] add QueryStateUpdater test --- src/components/MainNav/QueryStateUpdater.js | 4 +- .../MainNav/QueryStateUpdater.test.js | 49 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 src/components/MainNav/QueryStateUpdater.test.js diff --git a/src/components/MainNav/QueryStateUpdater.js b/src/components/MainNav/QueryStateUpdater.js index d8aac61c..8ede628b 100644 --- a/src/components/MainNav/QueryStateUpdater.js +++ b/src/components/MainNav/QueryStateUpdater.js @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; import { compose } from 'redux'; import { injectIntl } from 'react-intl'; import { withRouter } from 'react-router'; -import { isEqual, find } from 'lodash'; +import { isEqual } from 'lodash'; import { withModules } from '../Modules'; @@ -82,7 +82,7 @@ class QueryStateUpdater extends React.Component { } render = () => this.props.children; -}; +} export default compose( injectIntl, diff --git a/src/components/MainNav/QueryStateUpdater.test.js b/src/components/MainNav/QueryStateUpdater.test.js new file mode 100644 index 00000000..e9e08d36 --- /dev/null +++ b/src/components/MainNav/QueryStateUpdater.test.js @@ -0,0 +1,49 @@ + +import { render } from '@folio/jest-config-stripes/testing-library/react'; +import { QueryClientProvider, QueryClient } from 'react-query'; +import { MemoryRouter as Router, Route } from 'react-router-dom'; +import QueryStateUpdater from './QueryStateUpdater'; + +const mockUpdateQueryResource = jest.fn(); +jest.mock('../../locationService', () => ({ + ...jest.requireActual('../../locationService'), + updateLocation: jest.fn(), + updateQueryResource: jest.fn(() => mockUpdateQueryResource()), + getCurrentModule: jest.fn(() => true) +})); + +const mockUnsubscribe = jest.fn(); +const stripes = { + store: { + subscribe: jest.fn(() => mockUnsubscribe) + } +}; + +describe('QueryStateUpdater', () => { + let qc; + let wrapper; + beforeAll(() => { + qc = new QueryClient(); + wrapper = (testLocation = { pathname: '/initial' }) => ({ children }) => { + qc = new QueryClient(); + return ( + + + + {children} + + + + ); + }; + }); + it('renders', () => { + expect(render( + , + { wrapper: wrapper() } + )).not.toThrow(); + }); +}); From d93cb0cb87d4c020d3c41a67906ab72f7874744a Mon Sep 17 00:00:00 2001 From: John Coburn Date: Mon, 3 Mar 2025 15:11:24 -0600 Subject: [PATCH 6/8] clean test and salvage for points --- .../MainNav/QueryStateUpdater.test.js | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/components/MainNav/QueryStateUpdater.test.js b/src/components/MainNav/QueryStateUpdater.test.js index e9e08d36..a476f27a 100644 --- a/src/components/MainNav/QueryStateUpdater.test.js +++ b/src/components/MainNav/QueryStateUpdater.test.js @@ -41,9 +41,50 @@ describe('QueryStateUpdater', () => { }; }); it('renders', () => { - expect(render( + expect(() => render( , { wrapper: wrapper() } )).not.toThrow(); }); + + it('updatesQuery on mount', () => { + let testVal = { + hash: '', + key: 'f727ww', + pathname: '/initial', + search: '', + state: undefined + }; + const { rerender } = render( + , + { wrapper: wrapper(testVal) } + ); + testVal = { + hash: '', + key: 'f727w2', + pathname: '/updated', + search: '', + state: undefined + }; + rerender( + , + { wrapper: wrapper(testVal) } + ); + + expect(() => rerender( + , + { wrapper: wrapper(testVal) } + )).not.toThrow(); + }); }); From e2d7a609c6076faaa37a719c5851f239d429f2a0 Mon Sep 17 00:00:00 2001 From: John Coburn Date: Mon, 3 Mar 2025 15:32:29 -0600 Subject: [PATCH 7/8] add prop-type for children --- src/components/MainNav/QueryStateUpdater.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/MainNav/QueryStateUpdater.js b/src/components/MainNav/QueryStateUpdater.js index 8ede628b..8c03dd88 100644 --- a/src/components/MainNav/QueryStateUpdater.js +++ b/src/components/MainNav/QueryStateUpdater.js @@ -38,6 +38,10 @@ class QueryStateUpdater extends React.Component { subscribe: PropTypes.func.isRequired, }), }), + children: PropTypes.oneOfType([ + PropTypes.arrayOf(PropTypes.node), + PropTypes.node, + ]), } constructor(props) { From 7c45a74060f602786cd4646ce29dd5a903b617a8 Mon Sep 17 00:00:00 2001 From: John Coburn Date: Mon, 3 Mar 2025 16:21:25 -0600 Subject: [PATCH 8/8] header comments for general purpose of the component --- src/components/MainNav/QueryStateUpdater.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/components/MainNav/QueryStateUpdater.js b/src/components/MainNav/QueryStateUpdater.js index 8c03dd88..f02e6960 100644 --- a/src/components/MainNav/QueryStateUpdater.js +++ b/src/components/MainNav/QueryStateUpdater.js @@ -16,7 +16,11 @@ import { getQueryResourceState, } from '../../locationService'; -// onMount of stripes, sync the query state to the location. +/** QueryStateUpdater +* onMount of stripes, sync the query state to the location. +* Also if the location itself changes, sync the query resource to the current location. +*/ + class QueryStateUpdater extends React.Component { static propTypes = { history: PropTypes.shape({