Skip to content

Commit

Permalink
STCOR-864 correctly evaluate typeof stripes.okapi (#1497)
Browse files Browse the repository at this point in the history
Stripes should render `<ModuleContainer>` 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.

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.

Refs STCOR-864
  • Loading branch information
zburke authored Jun 26, 2024
1 parent 8185c89 commit 2bb7586
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<RootWithIntl>`. 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)
Expand Down
2 changes: 1 addition & 1 deletion src/RootWithIntl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) && (
<ModuleContainer id="content">
<OverlayContainer />
{connectedStripes.config.useSecureTokens && <SessionEventContainer history={history} />}
Expand Down
123 changes: 123 additions & 0 deletions src/RootWithIntl.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/* 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,
MainNav,
MainContainer,
ModuleContainer,
OverlayContainer,
StaleBundleWarning,
SessionEventContainer,
} from './components';

import RootWithIntl from './RootWithIntl';
import Stripes from './Stripes';

jest.mock('./components/Login', () => () => '<Login>');
jest.mock('./components/MainNav', () => () => '<MainNav>');
jest.mock('./components/OverlayContainer', () => () => '<OverlayContainer>');
jest.mock('./components/ModuleContainer', () => ({ children }) => children);
jest.mock('./components/MainContainer', () => ({ children }) => children);
jest.mock('./components/StaleBundleWarning', () => () => '<StaleBundleWarning>');
jest.mock('./components/SessionEventContainer', () => () => '<SessionEventContainer>');

const defaultHistory = createMemoryHistory();

const Harness = ({
Router = DefaultRouter,
children,
history = defaultHistory,
}) => {
return (
<Router history={history}>
{children}
</Router>
);
};

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(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated={false} /></Harness>);

expect(screen.getByText(/<Login>/)).toBeInTheDocument();
expect(screen.queryByText(/<MainNav>/)).toBeNull();
});

describe('renders MainNav', () => {
it('given isAuthenticated', async () => {
const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } });
await render(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated /></Harness>);

expect(screen.queryByText(/<Login>/)).toBeNull();
expect(screen.queryByText(/<MainNav>/)).toBeInTheDocument();
});

it('given token', async () => {
const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } });
await render(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} token /></Harness>);

expect(screen.queryByText(/<Login>/)).toBeNull();
expect(screen.queryByText(/<MainNav>/)).toBeInTheDocument();
});

it('given disableAuth', async () => {
const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, discovery: { isFinished: false } });
await render(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} disableAuth /></Harness>);

expect(screen.queryByText(/<Login>/)).toBeNull();
expect(screen.queryByText(/<MainNav>/)).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(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated /></Harness>);

expect(screen.queryByText(/<Login>/)).toBeNull();
expect(screen.queryByText(/<MainNav>/)).toBeInTheDocument();
expect(screen.getByText(/<OverlayContainer>/)).toBeInTheDocument();
});

it('if discovery is finished', async () => {
const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: {}, store, okapi: {}, discovery: { isFinished: true } });
await render(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated /></Harness>);

expect(screen.queryByText(/<Login>/)).toBeNull();
expect(screen.queryByText(/<MainNav>/)).toBeInTheDocument();
expect(screen.getByText(/<OverlayContainer>/)).toBeInTheDocument();
});
});

it('renders StaleBundleWarning', async () => {
const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: { staleBundleWarning: {} }, store, okapi: {}, discovery: { isFinished: true } });
await render(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated /></Harness>);

expect(screen.getByText(/<StaleBundleWarning>/)).toBeInTheDocument();
});

it('renders SessionEventContainer', async () => {
const stripes = new Stripes({ epics: {}, logger: {}, bindings: {}, config: { useSecureTokens: true }, store, okapi: {}, discovery: { isFinished: true } });
await render(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated /></Harness>);

expect(screen.getByText(/<SessionEventContainer>/)).toBeInTheDocument();
});

Check failure on line 121 in src/RootWithIntl.test.js

View workflow job for this annotation

GitHub Actions / ui / Install and lint / Install and lint

Block must not be padded by blank lines

});
1 change: 1 addition & 0 deletions src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
3 changes: 3 additions & 0 deletions test/bigtest/helpers/setup-application.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export default function setupApplication({
currentPerms: permissions,
isAuthenticated: true,
};
initialState.discovery = {
isFinished: true,
};
} else {
initialState.okapi = {
ssoEnabled: true,
Expand Down
1 change: 1 addition & 0 deletions test/jest/__mock__/stripesComponents.mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jest.mock('@folio/stripes-components', () => ({
<span>{children}</span>
)),
Headline: jest.fn(({ children }) => <div>{ children }</div>),
HotKeys: jest.fn(({ children }) => <>{ children }</>),
Icon: jest.fn((props) => (props && props.children ? props.children : <span />)),
IconButton: jest.fn(({
buttonProps,
Expand Down

0 comments on commit 2bb7586

Please sign in to comment.