♻️ [#645] Refactor useSessionTimeout hook and expiry modal

The expiry hook now tracks when the session will expire and performs a redirect when the session is effectively expired, which in turn triggers the necessary cleanup effects. The SessionTrackerModal is a rename of the RequireSession component, which now only has to keep track of imminent expiry of the session so that the end-user receives a warning and can opt to extend the session. Usage of these components is cleaned up, which in general simplifies state management a bit.
open-formulieren · Mar 12, 2024 · 9020f06 · 9020f06
1 parent 9a51320
commit 9020f06
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 103 deletions.
diff --git a/src/components/Form.js b/src/components/Form.js
@@ -14,7 +14,7 @@ import Loader from 'components/Loader';
 import {ConfirmationView, StartPaymentView} from 'components/PostCompletionViews';
 import ProgressIndicator from 'components/ProgressIndicator';
 import RequireSubmission from 'components/RequireSubmission';
-import {RequireSession} from 'components/Sessions';
+import {SessionTrackerModal} from 'components/Sessions';
 import SubmissionSummary from 'components/Summary';
 import {START_FORM_QUERY_PARAM} from 'components/constants';
 import {findNextApplicableStep} from 'components/utils';
@@ -122,7 +122,6 @@ const Form = () => {
   const [state, dispatch] = useImmerReducer(reducer, initialStateFromProps);
 
   const onSubmissionLoaded = (submission, next = '') => {
-    if (sessionExpired) return;
     dispatch({
       type: 'SUBMISSION_LOADED',
       payload: submission,
@@ -140,11 +139,7 @@ const Form = () => {
     onSubmissionLoaded
   );
 
-  const [sessionExpired, expiryDate, resetSession] = useSessionTimeout(() => {
-    removeSubmissionId();
-    dispatch({type: 'DESTROY_SUBMISSION'});
-    flagNoActiveSubmission();
-  });
+  const [, expiryDate, resetSession] = useSessionTimeout();
 
   const {value: analyticsToolsConfigInfo, loading: loadingAnalyticsConfig} = useAsync(async () => {
     return await get(`${config.baseUrl}analytics/analytics_tools_config_info`);
@@ -362,7 +357,7 @@ const Form = () => {
         path="overzicht"
         element={
           <ErrorBoundary useCard>
-            <RequireSession expired={sessionExpired} expiryDate={expiryDate}>
+            <SessionTrackerModal expiryDate={expiryDate}>
               <RequireSubmission
                 submission={state.submission}
                 form={form}
@@ -372,7 +367,7 @@ const Form = () => {
                 onClearProcessingErrors={() => dispatch({type: 'CLEAR_PROCESSING_ERROR'})}
                 onDestroySession={onDestroySession}
               />
-            </RequireSession>
+            </SessionTrackerModal>
           </ErrorBoundary>
         }
       />
@@ -411,7 +406,7 @@ const Form = () => {
         path="stap/:step"
         element={
           <ErrorBoundary useCard>
-            <RequireSession expired={sessionExpired} expiryDate={expiryDate}>
+            <SessionTrackerModal expiryDate={expiryDate}>
               <RequireSubmission
                 form={form}
                 submission={state.submission}
@@ -422,7 +417,7 @@ const Form = () => {
                 component={FormStep}
                 onDestroySession={onDestroySession}
               />
-            </RequireSession>
+            </SessionTrackerModal>
           </ErrorBoundary>
         }
       />

diff --git a/src/components/Sessions/RequireSession.js → ...omponents/Sessions/SessionTrackerModal.js b/src/components/Sessions/RequireSession.js → ...omponents/Sessions/SessionTrackerModal.js
@@ -6,9 +6,7 @@ import {useTimeout, useTimeoutFn} from 'react-use';
 import {ConfigContext} from 'Context';
 import {apiCall} from 'api';
 import {OFButton} from 'components/Button';
-import Card from 'components/Card';
 import ErrorMessage from 'components/Errors/ErrorMessage';
-import Link from 'components/Link';
 import {Toolbar, ToolbarList} from 'components/Toolbar';
 import Modal from 'components/modals/Modal';
 
@@ -46,7 +44,7 @@ const useTriggerWarning = numSeconds => {
   ];
 };
 
-const RequireSession = ({expired = false, expiryDate = null, onNavigate, children}) => {
+const SessionTrackerModal = ({expiryDate = null, children}) => {
   const [warningDismissed, setWarningDismissed] = useState(false);
 
   // re-render when the session is expired to show the error message
@@ -70,33 +68,6 @@ const RequireSession = ({expired = false, expiryDate = null, onNavigate, childre
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [expiryDate]);
 
-  if (expired) {
-    return (
-      <Card
-        title={
-          <FormattedMessage
-            description="Session expired card title"
-            defaultMessage="Your session has expired"
-          />
-        }
-      >
-        <ErrorMessage>
-          <FormattedMessage
-            description="Session expired error message"
-            defaultMessage="Your session has expired. Click <link>here</link> to restart."
-            values={{
-              link: chunks => (
-                <Link onClick={onNavigate} to="/">
-                  {chunks}
-                </Link>
-              ),
-            }}
-          />
-        </ErrorMessage>
-      </Card>
-    );
-  }
-
   const showWarning = !warningDismissed && warningTriggered;
   const secondsToExpiry = parseInt((expiryDate - now) / 1000);
   // ensure that the components don't get unmounted when there's no expiryDate -> do not
@@ -115,10 +86,8 @@ const RequireSession = ({expired = false, expiryDate = null, onNavigate, childre
   );
 };
 
-RequireSession.propTypes = {
-  expired: PropTypes.bool,
+SessionTrackerModal.propTypes = {
   expiryDate: PropTypes.instanceOf(Date),
-  onNavigate: PropTypes.func,
   children: PropTypes.node,
 };
 
@@ -173,4 +142,4 @@ ExpiryModal.propTypes = {
   setWarningDismissed: PropTypes.func.isRequired,
 };
 
-export default RequireSession;
+export default SessionTrackerModal;
diff --git a/src/components/Sessions/index.js b/src/components/Sessions/index.js
@@ -1 +1,4 @@
-export {default as RequireSession} from './RequireSession';
+import SessionExpired from './SessionExpired';
+import SessionTrackerModal from './SessionTrackerModal';
+
+export {SessionExpired, SessionTrackerModal};
diff --git a/src/components/Summary/CosignSummary.js b/src/components/Summary/CosignSummary.js
@@ -5,7 +5,7 @@ import {useAsync} from 'react-use';
 
 import {post} from 'api';
 import {LiteralsProvider} from 'components/Literal';
-import {RequireSession} from 'components/Sessions';
+import {SessionTrackerModal} from 'components/Sessions';
 import {SUBMISSION_ALLOWED} from 'components/constants';
 import useRecycleSubmission from 'hooks/useRecycleSubmission';
 import useSessionTimeout from 'hooks/useSessionTimeout';
@@ -50,16 +50,21 @@ const CosignSummary = ({
     onCosignComplete(response.data.reportDownloadUrl);
   };
 
-  const [sessionExpired, expiryDate] = useSessionTimeout(async () => {
-    await onDestroySession();
+  const destroySession = async () => {
+    removeSubmissionId();
+    onDestroySession();
+  };
+
+  const [, expiryDate] = useSessionTimeout(async () => {
+    await destroySession();
   });
 
   if (!(loading || loadingData) && !summaryData) {
     throw new Error('Could not load the data for this submission.');
   }
 
   return (
-    <RequireSession expired={sessionExpired} expiryDate={expiryDate}>
+    <SessionTrackerModal expiryDate={expiryDate}>
       <LiteralsProvider literals={form.literals}>
         <GenericSummary
           title={
@@ -78,7 +83,7 @@ const CosignSummary = ({
           onDestroySession={onDestroySession}
         />
       </LiteralsProvider>
-    </RequireSession>
+    </SessionTrackerModal>
   );
 };
 

diff --git a/src/components/appointments/CreateAppointment/CreateAppointment.js b/src/components/appointments/CreateAppointment/CreateAppointment.js
@@ -6,7 +6,7 @@ import ErrorBoundary from 'components/Errors/ErrorBoundary';
 import FormDisplay from 'components/FormDisplay';
 import {LiteralsProvider} from 'components/Literal';
 import Loader from 'components/Loader';
-import {RequireSession} from 'components/Sessions';
+import {SessionTrackerModal} from 'components/Sessions';
 import {checkMatchesPath} from 'components/utils/routers';
 import useFormContext from 'hooks/useFormContext';
 import useGetOrCreateSubmission from 'hooks/useGetOrCreateSubmission';
@@ -36,7 +36,7 @@ const CreateAppointment = () => {
   } = useGetOrCreateSubmission(form, skipSubmissionCreation);
   if (error) throw error;
 
-  const [sessionExpired, expiryDate, resetSession] = useSessionTimeout(clearSubmission);
+  const [, expiryDate, resetSession] = useSessionTimeout();
 
   const supportsMultipleProducts = form?.appointmentOptions.supportsMultipleProducts ?? false;
 
@@ -55,36 +55,36 @@ const CreateAppointment = () => {
 
   return (
     <AppointmentConfigContext.Provider value={{supportsMultipleProducts}}>
-      <CreateAppointmentState
-        currentStep={currentStep}
-        submission={submission}
-        resetSession={reset}
-      >
-        <FormDisplay progressIndicator={progressIndicator}>
-          <Wrapper sessionExpired={sessionExpired} title={form.name}>
-            <ErrorBoundary>
-              {isLoading ? (
-                <Loader modifiers={['centered']} />
-              ) : (
-                <RequireSession expired={sessionExpired} expiryDate={expiryDate} onNavigate={reset}>
+      <SessionTrackerModal expiryDate={expiryDate}>
+        <CreateAppointmentState
+          currentStep={currentStep}
+          submission={submission}
+          resetSession={reset}
+        >
+          <FormDisplay progressIndicator={progressIndicator}>
+            <Wrapper title={form.name}>
+              <ErrorBoundary>
+                {isLoading ? (
+                  <Loader modifiers={['centered']} />
+                ) : (
                   <LiteralsProvider literals={form.literals}>
                     <Outlet />
                   </LiteralsProvider>
-                </RequireSession>
-              )}
-            </ErrorBoundary>
-          </Wrapper>
-        </FormDisplay>
-      </CreateAppointmentState>
+                )}
+              </ErrorBoundary>
+            </Wrapper>
+          </FormDisplay>
+        </CreateAppointmentState>
+      </SessionTrackerModal>
     </AppointmentConfigContext.Provider>
   );
 };
 
 CreateAppointment.propTypes = {};
 
-const Wrapper = ({sessionExpired = false, children, ...props}) => {
+const Wrapper = ({children, ...props}) => {
   const isConfirmation = useIsConfirmation();
-  if (sessionExpired || isConfirmation) return <>{children}</>;
+  if (isConfirmation) return <>{children}</>;
 
   return (
     <Card titleComponent="h1" modifiers={['mobile-header-hidden']} {...props}>

diff --git a/src/components/appointments/CreateAppointment/CreateAppointment.spec.js b/src/components/appointments/CreateAppointment/CreateAppointment.spec.js
@@ -2,6 +2,7 @@ import {act, render, screen, waitFor} from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import messagesEN from 'i18n/compiled/en.json';
 import {IntlProvider} from 'react-intl';
+import ReactModal from 'react-modal';
 import {RouterProvider, createMemoryRouter} from 'react-router-dom';
 
 import {ConfigContext, FormContext} from 'Context';
@@ -67,6 +68,8 @@ const renderApp = (initialRoute = '/') => {
 };
 
 beforeEach(() => {
+  // silence some warnings in tests when the modal opens
+  ReactModal.setAppElement(document.documentElement);
   sessionStorage.clear();
 });
 
@@ -110,9 +113,7 @@ describe('Create appointment session expiration', () => {
 
     // now finally let the session timeout in 1s
     act(() => updateSessionExpiry(1));
-    await waitFor(async () => {
-      await screen.findByText('Your session has expired');
-    });
+    await screen.findByText('Your session has expired', undefined, {timeout: 2000});
 
     // and click the link to restart...
     const restartLink = await screen.findByRole('link', {name: 'here'});

diff --git a/src/components/appointments/CreateAppointment/CreateAppointmentState.js b/src/components/appointments/CreateAppointment/CreateAppointmentState.js
@@ -79,11 +79,8 @@ export const CreateAppointmentState = ({currentStep, submission, resetSession, c
   });
   const [processingError, setProcessingError] = useState('');
 
-  // reset the local state if the session times out
-  useSessionTimeout(() => {
-    setAppointmentData({});
-    window.localStorage.clear();
-  });
+  // check if the session is expired
+  useSessionTimeout();
 
   const contextValue = buildContextValue({
     submission,

diff --git a/src/hooks/useSessionTimeout.js b/src/hooks/useSessionTimeout.js
@@ -1,56 +1,62 @@
-import {useCallback, useEffect, useState} from 'react';
+import {useCallback, useEffect} from 'react';
+import {useMatch, useNavigate} from 'react-router-dom';
+import {useUpdate} from 'react-use';
 import {useGlobalState} from 'state-pool';
 
 import {sessionExpiresAt} from 'api';
 
 const useSessionTimeout = onTimeout => {
-  const [expired, setExpired] = useState(false);
   const [expiresAt, setExpiryDate] = useGlobalState(sessionExpiresAt);
+  const navigate = useNavigate();
+  const update = useUpdate();
   const expiryDate = expiresAt?.expiry;
 
-  const markExpired = useCallback(() => {
+  const expiryInMs = expiryDate - new Date();
+  const expired = expiryInMs <= 0;
+
+  const sessionMatch = useMatch('/sessie-verlopen');
+
+  const handleExpired = useCallback(() => {
     onTimeout && onTimeout();
-    setExpired(true);
-  }, [onTimeout]);
+    if (!sessionMatch) navigate('/sessie-verlopen');
+  }, [onTimeout, navigate, sessionMatch]);
 
   useEffect(() => {
     let mounted = true;
     if (expiryDate == null) return;
 
-    const expiryInMs = expiryDate - new Date();
-
     // fun one! admin sessions can span multiple days, so if the expiry is in the far future
     // (> 1 day), don't even bother with checking/marking things as expired. It's not relevant.
     // If we do allow large values, there's a risk we exceed the max value for a 32 bit number,
     // leading to overflows in setTimeout and immediately marking the session as expired
     // as a consequence.
     if (expiryInMs > 1000 * 60 * 60 * 24) return;
 
-    // don't schedule the expiry-setter (which leads to state updates and re-renders)
-    // if the session is already expired.
-    if (expired) return;
-
-    if (expiryInMs <= 0) {
-      markExpired();
+    if (expired) {
+      handleExpired();
       return;
     }
 
-    const timeoutId = window.setTimeout(
-      () => {
-        if (!mounted) return;
-        markExpired();
-      },
-      expiryInMs - 500 // be a bit pro-active
-    );
+    // at this point, we have not expired and there is an expiry soon-ish in the future.
+    // schedule a re-render at the expected expiry time, which will flip the 'expired'
+    // state from false to true and result in the expiry handler being called.
+    // Note that the timeouts used are not exact, so they could arrive slightly earlier
+    // or later - this is okay:
+    // * if they arrive later, the expiry will definitely be a given
+    // * if they arrive sooner, there is no expiry yet, but a new re-render will be
+    //   scheduled very soon, which brings us into the previous case again.
+    const timeoutId = window.setTimeout(() => {
+      if (!mounted) return;
+      update();
+    }, expiryInMs + 5);
 
     return () => {
       mounted = false;
       window.clearTimeout(timeoutId);
     };
-  }, [expired, expiryDate, markExpired]);
+  }, [expired, expiryInMs, expiryDate, handleExpired, update]);
 
   const reset = () => {
-    setExpired(false);
     setExpiryDate({expiry: null});
   };