From a49b48f703383e5086c48619861016f7b82c90af Mon Sep 17 00:00:00 2001 From: Beth Shook Date: Mon, 22 Apr 2024 16:05:32 -0500 Subject: [PATCH 1/3] revert some changes to fix redirect with query param regression --- .../hooks/receivePageNotFoundId.spec.ts | 26 +++++++++++++++++-- .../content/utils/processBrowserRedirect.ts | 18 ++++++------- src/config.development.js | 2 +- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/app/content/hooks/receivePageNotFoundId.spec.ts b/src/app/content/hooks/receivePageNotFoundId.spec.ts index a5224c58b0..5d86e165e5 100644 --- a/src/app/content/hooks/receivePageNotFoundId.spec.ts +++ b/src/app/content/hooks/receivePageNotFoundId.spec.ts @@ -1,9 +1,15 @@ import createTestServices from '../../../test/createTestServices'; import createTestStore from '../../../test/createTestStore'; +import { notFound } from '../../errors/routes'; import { replace } from '../../navigation/actions'; +import { createRouterService } from '../../navigation/routerService'; import { AnyMatch } from '../../navigation/types'; import { MiddlewareAPI, Store } from '../../types'; +import { assertWindow } from '../../utils'; import { receivePageNotFoundId } from '../actions'; +import { processBrowserRedirect } from '../utils/processBrowserRedirect'; +import * as errors from '../../errors'; +import * as content from '../../content'; const mockFetch = (valueToReturn: any, error?: any) => () => new Promise((resolve, reject) => { if (error) { @@ -19,12 +25,20 @@ describe('receivePageNotFoundId hook', () => { let historyReplaceSpy: jest.SpyInstance; let dispatch: jest.SpyInstance; let fetchBackup: any; + let window: Window; beforeEach(() => { store = createTestStore(); + window = assertWindow(); + delete (window as any).location; + + window.location = { + origin: 'openstax.org', + } as any as Window['location']; helpers = { ...createTestServices(), + router: createRouterService([...Object.values(content.routes), ...Object.values(errors.routes)]), dispatch: store.dispatch, getState: store.getState, }; @@ -66,8 +80,16 @@ describe('receivePageNotFoundId hook', () => { it('calls history.replace if redirect is found', async() => { (globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: '/books/redirected' }]); - await hook(receivePageNotFoundId('asdf')); + await processBrowserRedirect(helpers); + + expect(historyReplaceSpy).toHaveBeenCalledWith('/books/redirected'); + }); + + it('updates window.location if target is not within rex', async() => { + (globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: '/redirected' }]); + + await processBrowserRedirect(helpers); - expect(dispatch).toHaveBeenCalledWith(replace(helpers.router.findRoute('/books/redirected') as AnyMatch)); + expect(window.location.href).toEqual('openstax.org/redirected'); }); }); diff --git a/src/app/content/utils/processBrowserRedirect.ts b/src/app/content/utils/processBrowserRedirect.ts index 4a689c3c7d..1a2014d28e 100644 --- a/src/app/content/utils/processBrowserRedirect.ts +++ b/src/app/content/utils/processBrowserRedirect.ts @@ -1,22 +1,22 @@ import { History } from 'history'; import { Redirects } from '../../../../data/redirects/types'; import { RouterService } from '../../navigation/routerService'; -import { Dispatch } from '../../types'; -import { replace } from '../../navigation/actions'; -import { AnyMatch } from '../../navigation/types'; +import { assertWindow } from '../../utils'; +import { matchForRoute } from '../../navigation/utils'; +import { external } from '../../errors/routes'; -export const processBrowserRedirect = async(services: { - router: RouterService, - history: History, - dispatch: Dispatch -}) => { +export const processBrowserRedirect = async(services: {router: RouterService, history: History}) => { + const window = assertWindow(); const redirects: Redirects = await fetch('/rex/redirects.json') .then((res) => res.json()) .catch(() => []); for (const {from, to} of redirects) { if (from === services.history.location.pathname) { - services.dispatch(replace(services.router.findRoute(to) as AnyMatch)); + if (matchForRoute(external, services.router.findRoute(to))) { + window.location.href = window.location.origin + to; + } + services.history.replace(to); return true; } } diff --git a/src/config.development.js b/src/config.development.js index 697788146e..391261127e 100644 --- a/src/config.development.js +++ b/src/config.development.js @@ -41,5 +41,5 @@ module.exports = { PORT: process.env.PORT ? parseInt(process.env.PORT, 10) : 8000, - UNLIMITED_CONTENT: UNLIMITED_CONTENT === undefined ? true : UNLIMITED_CONTENT + UNLIMITED_CONTENT: false }; From e6d238b84362979eb99fe0f4113870cb2638a226 Mon Sep 17 00:00:00 2001 From: Beth Shook Date: Mon, 22 Apr 2024 16:06:37 -0500 Subject: [PATCH 2/3] remove dev config change --- src/config.development.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.development.js b/src/config.development.js index 391261127e..697788146e 100644 --- a/src/config.development.js +++ b/src/config.development.js @@ -41,5 +41,5 @@ module.exports = { PORT: process.env.PORT ? parseInt(process.env.PORT, 10) : 8000, - UNLIMITED_CONTENT: false + UNLIMITED_CONTENT: UNLIMITED_CONTENT === undefined ? true : UNLIMITED_CONTENT }; From 0af429ec4b8061dc4f90c652c43382db5f34a9d2 Mon Sep 17 00:00:00 2001 From: Beth Shook Date: Mon, 22 Apr 2024 16:33:55 -0500 Subject: [PATCH 3/3] lint fix --- src/app/content/hooks/receivePageNotFoundId.spec.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/app/content/hooks/receivePageNotFoundId.spec.ts b/src/app/content/hooks/receivePageNotFoundId.spec.ts index 5d86e165e5..601615946f 100644 --- a/src/app/content/hooks/receivePageNotFoundId.spec.ts +++ b/src/app/content/hooks/receivePageNotFoundId.spec.ts @@ -1,9 +1,6 @@ import createTestServices from '../../../test/createTestServices'; import createTestStore from '../../../test/createTestStore'; -import { notFound } from '../../errors/routes'; -import { replace } from '../../navigation/actions'; import { createRouterService } from '../../navigation/routerService'; -import { AnyMatch } from '../../navigation/types'; import { MiddlewareAPI, Store } from '../../types'; import { assertWindow } from '../../utils'; import { receivePageNotFoundId } from '../actions'; @@ -23,7 +20,6 @@ describe('receivePageNotFoundId hook', () => { let store: Store; let helpers: MiddlewareAPI & ReturnType; let historyReplaceSpy: jest.SpyInstance; - let dispatch: jest.SpyInstance; let fetchBackup: any; let window: Window; @@ -47,8 +43,6 @@ describe('receivePageNotFoundId hook', () => { pathname: '/books/physics/pages/1-introduction301', } as any; - dispatch = jest.spyOn(helpers, 'dispatch'); - historyReplaceSpy = jest.spyOn(helpers.history, 'replace') .mockImplementation(jest.fn());