Move session expiry page to own route #649
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #649 +/- ##
==========================================
- Coverage 75.20% 75.20% -0.01%
==========================================
Files 225 226 +1
Lines 4517 4521 +4
Branches 1208 1206 -2
==========================================
+ Hits 3397 3400 +3
- Misses 1081 1083 +2
+ Partials 39 38 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
There should be tests for these changes too - we use testing-library (with user-event) that should be relatively straight forward to test the behaviour of this new component.
Checks for when this component is rendered:
- local storage is cleared
- ...?
Please also add some storybook stories for this new component :)
src/components/appointments/CreateAppointment/CreateAppointment.js
Outdated
Show resolved
Hide resolved
|
|
||
| // register localized error messages in the default zod error map | ||
| useZodErrorMap(); | ||
|
|
||
| const isAppointment = form.appointmentOptions?.isAppointment ?? false; | ||
| if (isAppointment && !appointmentMatch && !appointmentCancelMatch) { | ||
| if (isAppointment && !appointmentMatch && !appointmentCancelMatch && !isSessionExpiryMatch) { |
There was a problem hiding this comment.
@nikkiysendoorn1 this was the root cause of jest running forever and eventually going out-of-memory on CI - the session expiry redirect ended up in this checks, which sends it back to the appointments index, which sends it back to session expirty etc., basically an infinite redirect cycle.
ideally, we'd solve this in the loader of the router instead of doing these redirects here, but currently we don't have the available form details in that context to be able to do that, so patching it like this for now.
| flagNoActiveSubmission(); | ||
| window.localStorage.clear(); | ||
| window.sessionStorage.clear(); | ||
| if (expiryDate !== null) reset(); |
There was a problem hiding this comment.
@nikkiysendoorn1 this one is needed for when the session expired, you go to the dedicated page, but then you click the "here" link to restart and just end up on this same page again because the session is still expired
if there's an expiry date set still, we reset the session expiry so that users can restart the flows.
| await waitFor(async () => { | ||
| await screen.findByText('Your session has expired'); | ||
| }); | ||
| await screen.findByText('Your session has expired', undefined, {timeout: 2000}); |
There was a problem hiding this comment.
failed because of the internal timeout being too low, so the session-will-expire-over-1000ms-and-then-render-a-different-page cycle exceeded that. Giving 1s for the rendering etc. is sufficient here.
| supportsMultipleProducts | ||
| ? withValidate(() => arrayHelpers.push({productId: '', amount: 1})) | ||
| : undefined |
There was a problem hiding this comment.
noticed a prop type warning in the dev tools, it's not related to this PR but might as well fix it along the way
| const timeoutId = window.setTimeout( | ||
| () => { | ||
| if (!mounted) return; | ||
| markExpired(); | ||
| }, | ||
| expiryInMs - 500 // be a bit pro-active | ||
| ); |
There was a problem hiding this comment.
this was causing problems because the pro-active/eager redirect to the session expiry page then caused an issue/crash saying the session was not expired yet - it was 500ms too early.
So instead of having this mechanism, I've opted to just let the component re-render through useUpdate from react-use, after which the derived expired state is used to invoke the redirect. Works better, and not so much magic-number usage.
sergei-maertens
changed the title
The component is responsible for cleanup of local/session storage and resetting the session expiry if not done so yet. Because the component is mounted on its own route, it will cause all other components that trigger the redirect to be unmounted and their state will automatically be reset too, leaving only the persistent storages like session/local storage to be cleaned up.
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.
Should be undefined rather than false when multiple products is not supported.
sergei-maertens
force-pushed
the
feature/645-session-expiry
branch
from
6d535d6 to
6ee53f1
Compare
Closes #645