STCOR-864 correctly evaluate typeof stripes.okapi (#1498)

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. :| 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 · 6201292 · 6201292
1 parent eed1ba5
commit 6201292
Show file tree

Hide file tree

Showing 6 changed files with 148 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/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
@@ -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,