Maintain search params on errors

src/app/services/useSessionExpired.tsx

@@ -4,7 +4,7 @@ import {KEY, persistence} from "./localStorage";
 import {useCheckCurrentUserOnActivity} from "./useCheckCurrentUserOnActivity";
 
 export const setAfterRenewPath = () => {
-    persistence.session.save(KEY.AFTER_SESSION_RENEW_PATH, window.location.pathname);
+    persistence.session.save(KEY.AFTER_SESSION_RENEW_PATH, window.location.href);
 };
 
 export const useSessionExpired = (): [string, () => void] => {
@@ -33,4 +33,4 @@ export const useSessionExpired = (): [string, () => void] => {
     }, [loggedBackIn]);
 
     return [target, clearRenewPath];
-};
+};

src/app/state/actions/index.tsx

@@ -597,7 +597,7 @@ export const continueToAfterAuthPath = (user?: {readonly role?: UserRole, readon
 
 // Hard redirect (refreshes page)
 export const redirectTo = (path: string) => {
-    window.location.href = window.location.origin + path;
+    window.location.href = path;
 };
 
 export const registerPageChange = (path: string) => {

src/app/state/middleware/userConsistencyChecker.tsx

@@ -101,7 +101,7 @@ export const userConsistencyCheckerMiddleware: Middleware = (api: MiddlewareAPI)
         // Redirect after action has been processed so that the notificationManager sees a logged-out user when deciding
         // whether to show the "required fields" modal and recording that in local storage.
         // TODO this might not be the case anymore since this is now a hard redirect now.
-        redirectTo(redirect);
+        redirectTo(window.location.origin + redirect);
     }
 
     return result;

src/test/pages/SessionCookieExpired.test.tsx

@@ -1,7 +1,8 @@
-import {navigateToMyAccount, renderTestEnvironment} from "../testUtils";
+import {navigateToMyAccount, renderTestEnvironment, setUrl} from "../testUtils";
 import {NOW, SOME_FIXED_FUTURE_DATE} from "../dateUtils";
 import {redirectTo} from "../../app/state";
 import * as Actions from "../../app/state/actions";
+import { screen } from "@testing-library/dom";
 
 
 describe("SessionExpired", () => {
@@ -10,7 +11,9 @@ describe("SessionExpired", () => {
         //  Annoyingly, the hard-redirect we use is not implemented in JSDOM/RTL, so we need to mock it out.
         jest.spyOn(Actions, "redirectTo");
         // @ts-ignore
-        redirectTo.mockImplementation(() => true);
+        redirectTo.mockImplementation(async (path) => {
+            await setUrl({pathname: path});
+        });
 
         //  Set the session expiry to be now
         await renderTestEnvironment({role: "STUDENT", sessionExpires: new Date(NOW).toUTCString()});
@@ -25,7 +28,7 @@ describe("SessionExpired", () => {
         // Assert
         //  Check we were redirected to the login page. Ideally we'd check the page itself, but this is the best we can
         //  do for the reasons described above.
-        expect(redirectTo).toHaveBeenLastCalledWith("/error_expired");
+        expect(redirectTo).toHaveBeenLastCalledWith(window.location.origin + "/error_expired");
 
         // Teardown
         // @ts-ignore
@@ -36,7 +39,9 @@ describe("SessionExpired", () => {
         // Arrange
         jest.spyOn(Actions, "redirectTo");
         // @ts-ignore
-        redirectTo.mockImplementation(() => true);
+        redirectTo.mockImplementation(async (path) => {
+            await setUrl({pathname: path});
+        });
 
         //  Set the session expiry to in the future
         await renderTestEnvironment({role: "STUDENT", sessionExpires: new Date(SOME_FIXED_FUTURE_DATE).toUTCString()});
@@ -51,6 +56,30 @@ describe("SessionExpired", () => {
         // Assert
         //  We should still be where we were.
         expect(window.location.pathname).toEqual("/account");
-        expect(redirectTo).not.toHaveBeenLastCalledWith("/error_expired");
+        expect(redirectTo).not.toHaveBeenLastCalledWith(window.location.origin + "/error_expired");
+
+        // @ts-ignore
+        redirectTo.mockRestore();
+    });
+
+    it("should restore correct url with 'continue' button", async () => {
+        jest.spyOn(Actions, "redirectTo");
+        // @ts-ignore
+        redirectTo.mockImplementation(async (path) => {
+            await setUrl({pathname: path});
+        });
+
+        await renderTestEnvironment({role: "STUDENT", sessionExpires: new Date(NOW).toUTCString()});
+        await navigateToMyAccount();
+        await setUrl({pathname: "/account", search: "?some=param"});
+
+        await new Promise((r) => setTimeout(r, 1500));
+
+        const continueLink = screen.getByRole("link", {name: "Continue"});
+
+        expect(continueLink.getAttribute("href")).toEqual(window.location.origin + "/account?some=param");
+
+        // @ts-ignore
+        redirectTo.mockRestore();
     });
 });

src/test/testUtils.tsx

@@ -242,6 +242,8 @@ export const withSizedWindow = async (width: number, height: number, cb: () => v
 export type PathString = `/${string}`;
 export type SearchString = `?${string}`;
 export const setUrl = async (location: Partial<URL>) => {
+    // this act seems to be generating a lot of warnings for both being there and for not being there if you remove it. 
+    // seems like an RTL issue: https://github.com/testing-library/react-testing-library/issues/1413
     await act(async () => {
         // push a new state, then go to it
         history.pushState({}, "", `${location?.pathname}${location?.search ?? ''}${location?.hash ?? ''}`);