From 6201292a2d851082a794122ddec607489e71a966 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Tue, 25 Jun 2024 16:55:39 -0400 Subject: [PATCH] STCOR-864 correctly evaluate typeof stripes.okapi (#1498) 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. 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. :| Ignore the "new" AuthnLogin test file; those tests were previously stashed in `RootWithIntl.test.js` for some reason and have just been relocated. Refs STCOR-864 --- CHANGELOG.md | 1 + src/RootWithIntl.js | 2 +- src/RootWithIntl.test.js | 126 ++++++++++--------- src/components/AuthnLogin/AuthnLogin.test.js | 76 +++++++++++ test/bigtest/helpers/setup-application.js | 3 + test/jest/__mock__/stripesComponents.mock.js | 1 + 6 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 src/components/AuthnLogin/AuthnLogin.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index fc1c2f155..9a348814c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ * Fix 404 error page when logging in after changing password in Eureka. Refs STCOR-845. * Always retrieve `clientId` and `tenant` values from `config.tenantOptions` in stripes.config.js. Retires `okapi.tenant`, `okapi.clientId`, and `config.isSingleTenant`. Refs STCOR-787. * List UI apps in "Applications/modules/interfaces" column. STCOR-773 +* Correctly evaluate `stripes.okapi` before rendering ``. Refs STCOR-864. ## [10.1.1](https://github.com/folio-org/stripes-core/tree/v10.1.1) (2024-03-25) [Full Changelog](https://github.com/folio-org/stripes-core/compare/v10.1.0...v10.1.1) diff --git a/src/RootWithIntl.js b/src/RootWithIntl.js index 5f2318d26..b51265cc9 100644 --- a/src/RootWithIntl.js +++ b/src/RootWithIntl.js @@ -72,7 +72,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 index 4da7f32b9..6aa1babad 100644 --- a/src/RootWithIntl.test.js +++ b/src/RootWithIntl.test.js @@ -2,25 +2,34 @@ /* 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 { Redirect as InternalRedirect } from 'react-router-dom'; -import Redirect from './components/Redirect'; -import { Login } from './components'; -import PreLoginLanding from './components/PreLoginLanding'; +import AuthnLogin from './components/AuthnLogin'; +import MainNav from './components/MainNav'; +import MainContainer from './components/MainContainer'; +import ModuleContainer from './components/ModuleContainer'; +import RootWithIntl from './RootWithIntl'; +import Stripes from './Stripes'; -// import { -// renderLogoutComponent -// } from './RootWithIntl'; +jest.mock('./components/AuthnLogin', () => () => ''); +jest.mock('./components/MainNav', () => () => ''); +jest.mock('./components/ModuleContainer', () => () => ''); +jest.mock('./components/MainContainer', () => ({ children }) => children); -import AuthnLogin from './components/AuthnLogin'; +const defaultHistory = createMemoryHistory(); -jest.mock('react-router-dom', () => ({ - Redirect: () => '', - withRouter: (Component) => Component, -})); -jest.mock('./components/Redirect', () => () => ''); -jest.mock('./components/Login', () => () => ''); -jest.mock('./components/PreLoginLanding', () => () => ''); +const Harness = ({ + Router = DefaultRouter, + children, + history = defaultHistory, +}) => { + return ( + + {children} + + ); +}; const store = { getState: () => ({ @@ -34,56 +43,53 @@ const store = { }; describe('RootWithIntl', () => { - describe('AuthnLogin', () => { - it('handles legacy login', () => { - const stripes = { okapi: {}, config: {}, store }; - render(); + 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.getByText(//)).toBeInTheDocument(); + expect(screen.queryByText(//)).toBeNull(); + expect(screen.queryByText(//)).toBeInTheDocument(); }); - describe('handles third-party login', () => { - it('handles single-tenant', () => { - const stripes = { - okapi: { authnUrl: 'https://barbie.com' }, - config: { - isSingleTenant: true, - tenantOptions: { - diku: { name: 'diku', clientId: 'diku-application' } - } - }, - store - }; - render(); - - expect(screen.getByText(//)).toBeInTheDocument(); - }); - - it('handles multi-tenant', () => { - const stripes = { - okapi: { authnUrl: 'https://oppie.com' }, - config: { - isSingleTenant: false, - tenantOptions: { - diku: { name: 'diku', clientId: 'diku-application' }, - diku2: { name: 'diku2', clientId: 'diku2-application' } - } - }, - store - }; - render(); - - expect(screen.getByText(//)).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('renderLogoutComponent', () => { - // it('handles legacy logout', () => { - // const stripes = { okapi: {}, config: {} }; - // render(renderLogoutComponent(stripes)); + 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(); + }); - // 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/src/components/AuthnLogin/AuthnLogin.test.js b/src/components/AuthnLogin/AuthnLogin.test.js new file mode 100644 index 000000000..e8aa7ffdb --- /dev/null +++ b/src/components/AuthnLogin/AuthnLogin.test.js @@ -0,0 +1,76 @@ +/* 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 { Redirect as InternalRedirect } from 'react-router-dom'; +import Redirect from '../Redirect'; +import Login from '../Login'; +import PreLoginLanding from '../PreLoginLanding'; + +import AuthnLogin from './AuthnLogin'; + +jest.mock('react-router-dom', () => ({ + Redirect: () => '', + withRouter: (Component) => Component, +})); +jest.mock('../Redirect', () => () => ''); +jest.mock('../Login', () => () => ''); +jest.mock('../PreLoginLanding', () => () => ''); + +const store = { + getState: () => ({ + okapi: { + token: '123', + }, + }), + dispatch: () => {}, + subscribe: () => {}, + replaceReducer: () => {}, +}; + +describe('RootWithIntl', () => { + describe('AuthnLogin', () => { + it('handles legacy login', () => { + const stripes = { okapi: {}, config: {}, store }; + render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + + describe('handles third-party login', () => { + it('handles single-tenant', () => { + const stripes = { + okapi: { authnUrl: 'https://barbie.com' }, + config: { + isSingleTenant: true, + tenantOptions: { + diku: { name: 'diku', clientId: 'diku-application' } + } + }, + store + }; + render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + + it('handles multi-tenant', () => { + const stripes = { + okapi: { authnUrl: 'https://oppie.com' }, + config: { + isSingleTenant: false, + tenantOptions: { + diku: { name: 'diku', clientId: 'diku-application' }, + diku2: { name: 'diku2', clientId: 'diku2-application' } + } + }, + store + }; + render(); + + expect(screen.getByText(//)).toBeInTheDocument(); + }); + }); + }); +}); 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, diff --git a/test/jest/__mock__/stripesComponents.mock.js b/test/jest/__mock__/stripesComponents.mock.js index 444365dbd..ba14f98ff 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,