STCOR-864 correctly evaluate typeof stripes.okapi

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. 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
folio-org · Jun 25, 2024 · 9f18531 · 9f18531
1 parent eed1ba5
commit 9f18531
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 61 deletions.
diff --git 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 `<RootWithIntl>`. 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
@@ -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) && (
                             <ModuleContainer id="content">
                               <OverlayContainer />
                               {connectedStripes.config.useSecureTokens && <SessionEventContainer history={history} />}

diff --git 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', () => () => '<AuthnLogin>');
+jest.mock('./components/MainNav', () => () => '<MainNav>');
+jest.mock('./components/ModuleContainer', () => () => '<ModuleContainer>');
+jest.mock('./components/MainContainer', () => ({ children }) => children);
 
-import AuthnLogin from './components/AuthnLogin';
+const defaultHistory = createMemoryHistory();
 
-jest.mock('react-router-dom', () => ({
-  Redirect: () => '<internalredirect>',
-  withRouter: (Component) => Component,
-}));
-jest.mock('./components/Redirect', () => () => '<redirect>');
-jest.mock('./components/Login', () => () => '<login>');
-jest.mock('./components/PreLoginLanding', () => () => '<preloginlanding>');
+const Harness = ({
+  Router = DefaultRouter,
+  children,
+  history = defaultHistory,
+}) => {
+  return (
+    <Router history={history}>
+      {children}
+    </Router>
+  );
+};
 
 const store = {
   getState: () => ({
@@ -34,56 +43,53 @@ const store = {
 };
 
 describe('RootWithIntl', () => {
-  describe('AuthnLogin', () => {
-    it('handles legacy login', () => {
-      const stripes = { okapi: {}, config: {}, store };
-      render(<AuthnLogin stripes={stripes} />);
+  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(/<AuthnLogin>/)).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.getByText(/<login>/)).toBeInTheDocument();
+      expect(screen.queryByText(/<AuthnLogin>/)).toBeNull();
+      expect(screen.queryByText(/<MainNav>/)).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(<AuthnLogin stripes={stripes} />);
-
-        expect(screen.getByText(/<redirect>/)).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(<AuthnLogin stripes={stripes} />);
-
-        expect(screen.getByText(/<preloginlanding>/)).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(/<AuthnLogin>/)).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(/<AuthnLogin>/)).toBeNull();
+      expect(screen.queryByText(/<MainNav>/)).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(<Harness><RootWithIntl stripes={stripes} history={defaultHistory} isAuthenticated /></Harness>);
+
+      expect(screen.getByText(/<ModuleContainer>/)).toBeInTheDocument();
+    });
 
-  //     expect(screen.getByText(/<internalredirect>/)).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.getByText(/<ModuleContainer>/)).toBeInTheDocument();
+    });
+  });
 });
diff --git a/src/components/AuthnLogin/AuthnLogin.test.js 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: () => '<internalredirect>',
+  withRouter: (Component) => Component,
+}));
+jest.mock('../Redirect', () => () => '<redirect>');
+jest.mock('../Login', () => () => '<login>');
+jest.mock('../PreLoginLanding', () => () => '<preloginlanding>');
+
+const store = {
+  getState: () => ({
+    okapi: {
+      token: '123',
+    },
+  }),
+  dispatch: () => {},
+  subscribe: () => {},
+  replaceReducer: () => {},
+};
+
+describe('RootWithIntl', () => {
+  describe('AuthnLogin', () => {
+    it('handles legacy login', () => {
+      const stripes = { okapi: {}, config: {}, store };
+      render(<AuthnLogin stripes={stripes} />);
+
+      expect(screen.getByText(/<login>/)).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(<AuthnLogin stripes={stripes} />);
+
+        expect(screen.getByText(/<redirect>/)).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(<AuthnLogin stripes={stripes} />);
+
+        expect(screen.getByText(/<preloginlanding>/)).toBeInTheDocument();
+      });
+    });
+  });
+});
diff --git a/test/jest/__mock__/stripesComponents.mock.js b/test/jest/__mock__/stripesComponents.mock.js
@@ -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,