STCOR-875 restore the simplicity of logout in STCOR-865

The bugfix for STCOR-865, #1500, resulted in vastly simpler logic
in the SessionEventContainer event handlers as well as simple and
predictable and behavior in the Logout and LogoutTimeout components.
Restore that logic; it's better.

Refs STCOR-865, STCOR-875

src/components/Logout/Logout.js

@@ -1,5 +1,4 @@
 import { useEffect, useState } from 'react';
-import PropTypes from 'prop-types';
 import { Redirect } from 'react-router';
 import { FormattedMessage } from 'react-intl';
 
@@ -13,16 +12,15 @@ import { getLocale, logout } from '../../loginServices';
  * This corresponds to the '/logout' route, allowing that route to be directly
  * accessible rather than only accessible through the menu action.
  *
- * @param {object} history
  */
-const Logout = ({ history }) => {
+const Logout = () => {
   const stripes = useStripes();
   const [didLogout, setDidLogout] = useState(false);
 
   useEffect(
     () => {
       getLocale(stripes.okapi.url, stripes.store, stripes.okapi.tenant)
-        .then(logout(stripes.okapi.url, stripes.store, history))
+        .then(logout(stripes.okapi.url, stripes.store))
         .then(setDidLogout(true));
     },
     // no dependencies because we only want to start the logout process once.
@@ -35,8 +33,4 @@ const Logout = ({ history }) => {
   return didLogout ? <Redirect to="/" /> : <FormattedMessage id="stripes-core.logoutPending" />;
 };
 
-Logout.propTypes = {
-  history: PropTypes.object,
-};
-
 export default Logout;

src/components/LogoutTimeout/LogoutTimeout.js

@@ -1,48 +1,66 @@
+import { useEffect, useState } from 'react';
 import { FormattedMessage } from 'react-intl';
 import { branding } from 'stripes-config';
-import { Redirect } from 'react-router';
 
 import {
   Button,
   Col,
   Headline,
+  LoadingView,
   Row,
 } from '@folio/stripes-components';
 
 import OrganizationLogo from '../OrganizationLogo';
 import { useStripes } from '../../StripesContext';
-
-import styles from './LogoutTimeout.css';
 import {
   getUnauthorizedPathFromSession,
+  logout,
   removeUnauthorizedPathFromSession,
 } from '../../loginServices';
 
+import styles from './LogoutTimeout.css';
+
 /**
  * LogoutTimeout
- * For unauthenticated users, show a "sorry, your session timed out" message
- * with a link to login page. For authenticated users, redirect to / since
- * showing such a message would be a misleading lie.
+ * Show a "sorry, your session timed out message"; if the session is still
+ * active, call logout() to end it.
  *
  * Having a static route to this page allows the logout handler to choose
  * between redirecting straight to the login page (if the user chose to
- * logout) or to this page (if the session timeout out).
+ * logout) or to this page (if the session timed out).
  *
  * This corresponds to the '/logout-timeout' route.
  */
 const LogoutTimeout = () => {
   const stripes = useStripes();
+  const [didLogout, setDidLogout] = useState(false);
 
-  if (stripes.okapi.isAuthenticated) {
-    return <Redirect to="/" />;
-  }
+  useEffect(
+    () => {
+      if (stripes.okapi.isAuthenticated) {
+        // returns a promise, which we ignore
+        logout(stripes.okapi.url, stripes.store)
+          .then(setDidLogout(true));
+      } else {
+        setDidLogout(true);
+      }
+    },
+    // no dependencies because we only want to start the logout process once.
+    // we don't care about changes to stripes; certainly it'll be updated as
+    // part of the logout process
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    []
+  );
 
   const handleClick = (_e) => {
     removeUnauthorizedPathFromSession();
   };
 
-  const previousPath = getUnauthorizedPathFromSession();
-  const redirectTo = previousPath ?? '/';
+  const redirectTo = getUnauthorizedPathFromSession() || '/';
+
+  if (!didLogout) {
+    return <LoadingView />;
+  }
 
   return (
     <main>

src/components/LogoutTimeout/LogoutTimeout.test.js

@@ -3,14 +3,21 @@ import { userEvent } from '@folio/jest-config-stripes/testing-library/user-event
 
 import LogoutTimeout from './LogoutTimeout';
 import { useStripes } from '../../StripesContext';
-import { getUnauthorizedPathFromSession, setUnauthorizedPathToSession } from '../../loginServices';
+import { getUnauthorizedPathFromSession, logout, setUnauthorizedPathToSession } from '../../loginServices';
 
 jest.mock('../OrganizationLogo');
 jest.mock('../../StripesContext');
 jest.mock('react-router', () => ({
   Redirect: () => <div>Redirect</div>,
 }));
 
+jest.mock('../../loginServices', () => ({
+  logout: jest.fn(() => Promise.resolve()),
+  getUnauthorizedPathFromSession: jest.fn(),
+  removeUnauthorizedPathFromSession: jest.fn(),
+  setUnauthorizedPathToSession: jest.fn(),
+}));
+
 describe('LogoutTimeout', () => {
   describe('if not authenticated', () => {
     it('renders a timeout message', async () => {
@@ -21,7 +28,7 @@ describe('LogoutTimeout', () => {
       screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad');
     });
 
-    it('clears previous path from storage after clicking', async () => {
+    it('clears previous path from storage after clicking "log in again"', async () => {
       const previousPath = '/monkey?bagel';
       setUnauthorizedPathToSession(previousPath);
       const user = userEvent.setup();
@@ -31,16 +38,34 @@ describe('LogoutTimeout', () => {
       render(<LogoutTimeout />);
 
       await user.click(screen.getByRole('button'));
-
-      expect(getUnauthorizedPathFromSession()).toBe(null);
+      expect(getUnauthorizedPathFromSession()).toBeFalsy();
     });
   });
 
-  it('if authenticated, renders a redirect', async () => {
-    const mockUseStripes = useStripes;
-    mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } });
+  describe('if not authenticated', () => {
+    it('calls logout then renders a timeout message', async () => {
+      const mockUseStripes = useStripes;
+      mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } });
+
+      render(<LogoutTimeout />);
+      expect(logout).toHaveBeenCalled();
+      screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad');
+    });
+
+    it('clears previous path from storage after clicking "log in again"', async () => {
+      const previousPath = '/monkey?bagel';
+      setUnauthorizedPathToSession(previousPath);
+      const user = userEvent.setup();
+      const mockUseStripes = useStripes;
+      mockUseStripes.mockReturnValue({ okapi: { isAuthenticated: true } });
+
+      render(<LogoutTimeout />);
+
+      expect(logout).toHaveBeenCalled();
+      screen.getByText('stripes-core.rtr.idleSession.sessionExpiredSoSad');
 
-    render(<LogoutTimeout />);
-    screen.getByText('Redirect');
+      await user.click(screen.getByRole('button'));
+      expect(getUnauthorizedPathFromSession()).toBeFalsy();
+    });
   });
 });

src/components/SessionEventContainer/SessionEventContainer.js

@@ -3,7 +3,7 @@ import PropTypes from 'prop-types';
 import createInactivityTimer from 'inactivity-timer';
 import ms from 'ms';
 
-import { logout, SESSION_NAME, setUnauthorizedPathToSession } from '../../loginServices';
+import { SESSION_NAME, setUnauthorizedPathToSession } from '../../loginServices';
 import KeepWorkingModal from './KeepWorkingModal';
 import { useStripes } from '../../StripesContext';
 import {
@@ -22,26 +22,20 @@ import FixedLengthSessionWarning from './FixedLengthSessionWarning';
 //
 
 // RTR error in this window: logout
-export const thisWindowRtrError = (_e, stripes, history, queryClient) => {
+export const thisWindowRtrError = (_e, stripes, history) => {
   console.warn('rtr error; logging out'); // eslint-disable-line no-console
   setUnauthorizedPathToSession();
-  return logout(stripes.okapi.url, stripes.store, queryClient)
-    .then(() => {
-      history.push('/logout-timeout');
-    });
+  history.push('/logout-timeout');
 };
 
 // idle session timeout in this window: logout
-export const thisWindowRtrIstTimeout = (_e, stripes, history, queryClient) => {
+export const thisWindowRtrIstTimeout = (_e, stripes, history) => {
   stripes.logger.log('rtr', 'idle session timeout; logging out');
   setUnauthorizedPathToSession();
-  return logout(stripes.okapi.url, stripes.store, queryClient)
-    .then(() => {
-      history.push('/logout-timeout');
-    });
+  history.push('/logout-timeout');
 };
 
-// fixed-length session warning in this window
+// fixed-length session warning in this window: logout
 export const thisWindowRtrFlsWarning = (_e, stripes, setIsFlsVisible) => {
   stripes.logger.log('rtr', 'fixed-length session warning');
   setIsFlsVisible(true);
@@ -58,14 +52,11 @@ export const thisWindowRtrFlsTimeout = (_e, stripes, history) => {
 // logout if it was a timeout event or if SESSION_NAME is being
 // removed from localStorage, an indicator that logout is in-progress
 // in another window and so must occur here as well
-export const otherWindowStorage = (e, stripes, history, queryClient) => {
+export const otherWindowStorage = (e, stripes, history) => {
   if (e.key === RTR_TIMEOUT_EVENT) {
     stripes.logger.log('rtr', 'idle session timeout; logging out');
     setUnauthorizedPathToSession();
-    return logout(stripes.okapi.url, stripes.store, queryClient)
-      .then(() => {
-        history.push('/logout-timeout');
-      });
+    history.push('/logout-timeout');
   } else if (!localStorage.getItem(SESSION_NAME)) {
     stripes.logger.log('rtr', 'external localstorage change; logging out');
     setUnauthorizedPathToSession();
@@ -138,7 +129,7 @@ export const thisWindowActivity = (_e, stripes, timers, broadcastChannel) => {
  * @param {object} history
  * @returns KeepWorkingModal or null
  */
-const SessionEventContainer = ({ history, queryClient }) => {
+const SessionEventContainer = ({ history }) => {
   // is the "keep working?" modal visible?
   const [isVisible, setIsVisible] = useState(false);
 
@@ -220,13 +211,13 @@ const SessionEventContainer = ({ history, queryClient }) => {
       timers.current = { showModalIT, logoutIT };
 
       // RTR error in this window: logout
-      channels.window[RTR_ERROR_EVENT] = (e) => thisWindowRtrError(e, stripes, history, queryClient);
+      channels.window[RTR_ERROR_EVENT] = (e) => thisWindowRtrError(e, stripes, history);
 
       // idle session timeout in this window: logout
-      channels.window[RTR_TIMEOUT_EVENT] = (e) => thisWindowRtrIstTimeout(e, stripes, history, queryClient);
+      channels.window[RTR_TIMEOUT_EVENT] = (e) => thisWindowRtrIstTimeout(e, stripes, history);
 
       // localstorage change in another window: logout?
-      channels.window.storage = (e) => otherWindowStorage(e, stripes, history, queryClient);
+      channels.window.storage = (e) => otherWindowStorage(e, stripes, history);
 
       // activity in another window: send keep-alive to idle-timers.
       channels.bc.message = (message) => otherWindowActivity(message, stripes, timers, setIsVisible);
@@ -292,7 +283,6 @@ const SessionEventContainer = ({ history, queryClient }) => {
 
 SessionEventContainer.propTypes = {
   history: PropTypes.object,
-  queryClient: PropTypes.object,
 };
 
 export default SessionEventContainer;

src/components/SessionEventContainer/SessionEventContainer.test.js

@@ -10,7 +10,6 @@ import SessionEventContainer, {
   thisWindowRtrIstTimeout,
 } from './SessionEventContainer';
 import {
-  logout,
   setUnauthorizedPathToSession,
   SESSION_NAME,
 } from '../../loginServices';
@@ -73,14 +72,11 @@ describe('SessionEventContainer', () => {
 describe('SessionEventContainer event listeners', () => {
   it('thisWindowRtrError', async () => {
     const history = { push: jest.fn() };
-    const logoutMock = logout;
-    logoutMock.mockReturnValue(Promise.resolve());
 
     const setUnauthorizedPathToSessionMock = setUnauthorizedPathToSession;
     setUnauthorizedPathToSessionMock.mockReturnValue(null);
 
-    await thisWindowRtrError(null, { okapi: { url: 'http' } }, history);
-    expect(logout).toHaveBeenCalled();
+    thisWindowRtrError(null, { okapi: { url: 'http' } }, history);
     expect(setUnauthorizedPathToSession).toHaveBeenCalled();
     expect(history.push).toHaveBeenCalledWith('/logout-timeout');
   });
@@ -98,12 +94,7 @@ describe('SessionEventContainer event listeners', () => {
 
     const history = { push: jest.fn() };
 
-    const setUnauthorizedPathToSessionMock = setUnauthorizedPathToSession;
-    setUnauthorizedPathToSessionMock.mockReturnValue(null);
-
-    await thisWindowRtrIstTimeout(null, s, history);
-    expect(logout).toHaveBeenCalled();
-    expect(setUnauthorizedPathToSession).toHaveBeenCalled();
+    thisWindowRtrIstTimeout(null, s, history);
     expect(history.push).toHaveBeenCalledWith('/logout-timeout');
   });
 
@@ -124,13 +115,8 @@ describe('SessionEventContainer event listeners', () => {
         }
       };
       const history = { push: jest.fn() };
-      const qc = {};
-      const setUnauthorizedPathToSessionMock = setUnauthorizedPathToSession;
-      setUnauthorizedPathToSessionMock.mockReturnValue(null);
 
-      await otherWindowStorage(e, s, history, qc);
-      expect(logout).toHaveBeenCalledWith(s.okapi.url, s.store, qc);
-      expect(setUnauthorizedPathToSession).toHaveBeenCalled();
+      otherWindowStorage(e, s, history);
       expect(history.push).toHaveBeenCalledWith('/logout-timeout');
     });
 
@@ -146,10 +132,8 @@ describe('SessionEventContainer event listeners', () => {
         }
       };
       const history = { push: jest.fn() };
-      const qc = {};
 
-      await otherWindowStorage(e, s, history, qc);
-      expect(logout).toHaveBeenCalledWith(s.okapi.url, s.store, qc);
+      otherWindowStorage(e, s, history);
       expect(history.push).toHaveBeenCalledWith('/logout');
     });
   });