From 023cb18a30a1dc0b7f182c77fa43012e239eb82b Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Tue, 25 Jun 2024 14:52:16 -0400 Subject: [PATCH 1/3] STCOR-864 correctly evaluate typeof stripes.okapi Stripes should render `` either when discovery is complete or when okapi isn't present at all, i.e. when `stripes.config.js` doesn't even contain an `okapi` entry. What's most amazing about this bug is not the bug, which is a relatively simple typo, but that it didn't bite us for more than six years. Refs STCOR-864 --- CHANGELOG.md | 1 + src/RootWithIntl.js | 2 +- src/RootWithIntl.test.js | 95 ++++++++++++++++++++ test/jest/__mock__/stripesComponents.mock.js | 1 + 4 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 src/RootWithIntl.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ee664887..7175b90dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ * Avoid deprecated `getChildContext`. Refs STCOR-842. * Read locale from stripes-config before defaulting to `en-US`. Refs STCOR-851. * Correctly populate `stripes.user.user` on reload. Refs STCOR-860. +* Correctly evaluate `stripes.okapi` before rendering ``. Refs STCOR-864. ## [10.1.0](https://github.com/folio-org/stripes-core/tree/v10.1.0) (2024-03-12) [Full Changelog](https://github.com/folio-org/stripes-core/compare/v10.0.0...v10.1.0) diff --git a/src/RootWithIntl.js b/src/RootWithIntl.js index 8658fd941..62e825355 100644 --- a/src/RootWithIntl.js +++ b/src/RootWithIntl.js @@ -70,7 +70,7 @@ const RootWithIntl = ({ stripes, token = '', isAuthenticated = false, disableAut event={events.LOGIN} stripes={connectedStripes} /> - { (connectedStripes.okapi !== 'object' || connectedStripes.discovery.isFinished) && ( + { (typeof connectedStripes.okapi !== 'object' || connectedStripes.discovery.isFinished) && ( {connectedStripes.config.useSecureTokens && } diff --git a/src/RootWithIntl.test.js b/src/RootWithIntl.test.js new file mode 100644 index 000000000..2beb96b77 --- /dev/null +++ b/src/RootWithIntl.test.js @@ -0,0 +1,95 @@ +/* shhhh, eslint, it's ok. we need "unused" imports for mocks */ +/* eslint-disable no-unused-vars */ + +import { render, screen } from '@folio/jest-config-stripes/testing-library/react'; +import { Router as DefaultRouter } from 'react-router-dom'; +import { createMemoryHistory } from 'history'; + +import Login from './components/Login'; +import MainNav from './components/MainNav'; +import MainContainer from './components/MainContainer'; +import ModuleContainer from './components/ModuleContainer'; +import RootWithIntl from './RootWithIntl'; +import Stripes from './Stripes'; + +jest.mock('./components/Login', () => () => ''); +jest.mock('./components/MainNav', () => () => ''); +jest.mock('./components/ModuleContainer', () => () => ''); +jest.mock('./components/MainContainer', () => ({ children }) => children); + +const defaultHistory = createMemoryHistory(); + +const Harness = ({ + Router = DefaultRouter, + children, + history = defaultHistory, +}) => { + return ( + + {children} + + ); +}; + +const store = { + getState: () => ({ + okapi: { + token: '123', + }, + }), + dispatch: () => {}, + subscribe: () => {}, + replaceReducer: () => {}, +}; + +describe('RootWithIntl', () => { + it('renders login without one of (isAuthenticated, token, disableAuth)', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } }); + await render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + expect(screen.queryByText(//)).toBeNull(); + }); + + describe('renders MainNav', () => { + it('given isAuthenticated', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } }); + await render(); + + expect(screen.queryByText(//)).toBeNull(); + expect(screen.queryByText(//)).toBeInTheDocument(); + }); + + it('given token', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } }); + await render(); + + expect(screen.queryByText(//)).toBeNull(); + expect(screen.queryByText(//)).toBeInTheDocument(); + }); + + it('given disableAuth', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } }); + await render(); + + expect(screen.queryByText(//)).toBeNull(); + expect(screen.queryByText(//)).toBeInTheDocument(); + }); + }); + + describe('renders ModuleContainer', () => { + it('if config.okapi is not an object', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: true } }); + await render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + + it('if discovery is finished', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, okapi: {}, discovery: { isFinished: true } }); + await render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + }); +}); diff --git a/test/jest/__mock__/stripesComponents.mock.js b/test/jest/__mock__/stripesComponents.mock.js index 81647d4c6..1f5a9e44f 100644 --- a/test/jest/__mock__/stripesComponents.mock.js +++ b/test/jest/__mock__/stripesComponents.mock.js @@ -49,6 +49,7 @@ jest.mock('@folio/stripes-components', () => ({ {children} )), Headline: jest.fn(({ children }) =>
{ children }
), + HotKeys: jest.fn(({ children }) => <>{ children }), Icon: jest.fn((props) => (props && props.children ? props.children : )), IconButton: jest.fn(({ buttonProps, From 45e5f8c0031ee4376e05440f310222ae342bcb12 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Tue, 25 Jun 2024 16:40:56 -0400 Subject: [PATCH 2/3] BTOG tests relied on this bug, which was fun BTOG init never conducted discovery, but _did_ pass an okapi object during application setup, which is another way of saying that our application didn't have anything that relied on the presence of this bug, but our test suite did. --- test/bigtest/helpers/setup-application.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/bigtest/helpers/setup-application.js b/test/bigtest/helpers/setup-application.js index c54aa590c..7346300d8 100644 --- a/test/bigtest/helpers/setup-application.js +++ b/test/bigtest/helpers/setup-application.js @@ -54,6 +54,9 @@ export default function setupApplication({ currentPerms: permissions, isAuthenticated: true, }; + initialState.discovery = { + isFinished: true, + }; } else { initialState.okapi = { ssoEnabled: true, From 253227cc6fa2cb9c48889a5378e66950244e4db8 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Tue, 25 Jun 2024 22:22:42 -0400 Subject: [PATCH 3/3] additional test coverage --- src/RootWithIntl.test.js | 42 +++++++++++++++++++++++++++++++++------- src/components/index.js | 1 + 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/RootWithIntl.test.js b/src/RootWithIntl.test.js index 2beb96b77..24507b40d 100644 --- a/src/RootWithIntl.test.js +++ b/src/RootWithIntl.test.js @@ -5,17 +5,26 @@ import { render, screen } from '@folio/jest-config-stripes/testing-library/react import { Router as DefaultRouter } from 'react-router-dom'; import { createMemoryHistory } from 'history'; -import Login from './components/Login'; -import MainNav from './components/MainNav'; -import MainContainer from './components/MainContainer'; -import ModuleContainer from './components/ModuleContainer'; +import { + Login, + MainNav, + MainContainer, + ModuleContainer, + OverlayContainer, + StaleBundleWarning, + SessionEventContainer, +} from './components'; + import RootWithIntl from './RootWithIntl'; import Stripes from './Stripes'; jest.mock('./components/Login', () => () => ''); jest.mock('./components/MainNav', () => () => ''); -jest.mock('./components/ModuleContainer', () => () => ''); +jest.mock('./components/OverlayContainer', () => () => ''); +jest.mock('./components/ModuleContainer', () => ({ children }) => children); jest.mock('./components/MainContainer', () => ({ children }) => children); +jest.mock('./components/StaleBundleWarning', () => () => ''); +jest.mock('./components/SessionEventContainer', () => () => ''); const defaultHistory = createMemoryHistory(); @@ -82,14 +91,33 @@ describe('RootWithIntl', () => { const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: true } }); await render(); - expect(screen.getByText(//)).toBeInTheDocument(); + expect(screen.queryByText(//)).toBeNull(); + expect(screen.queryByText(//)).toBeInTheDocument(); + expect(screen.getByText(//)).toBeInTheDocument(); }); it('if discovery is finished', async () => { const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, okapi: {}, discovery: { isFinished: true } }); await render(); - expect(screen.getByText(//)).toBeInTheDocument(); + expect(screen.queryByText(//)).toBeNull(); + expect(screen.queryByText(//)).toBeInTheDocument(); + expect(screen.getByText(//)).toBeInTheDocument(); }); }); + + it('renders StaleBundleWarning', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: { staleBundleWarning: {} }, store, okapi: {}, discovery: { isFinished: true } }); + await render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + + it('renders SessionEventContainer', async () => { + const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: { useSecureTokens: true }, store, okapi: {}, discovery: { isFinished: true } }); + await render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + }); diff --git a/src/components/index.js b/src/components/index.js index 99e2dcd54..f6e471bba 100644 --- a/src/components/index.js +++ b/src/components/index.js @@ -31,6 +31,7 @@ export { default as BadRequestScreen } from './BadRequestScreen'; export { default as NoPermissionScreen } from './NoPermissionScreen'; export { default as ResetPasswordNotAvailableScreen } from './ResetPasswordNotAvailableScreen'; export { default as SessionEventContainer } from './SessionEventContainer'; +export { default as StaleBundleWarning } from './StaleBundleWarning'; export * from './ModuleHierarchy'; export * from './Namespace';