browser redirects to non-rex targets (#2222)

* fix browser redirect for non rex targets

* remove log

* remove whitespace

* update specs

* lint

* try a thing

* lint and update spec

* fix specs

* tidy

* lint

* add service to prerender script

* remove repetitive bits

* exclude router from appoptions type

* lint

* lint fix

* remove router from test services

* lint fix

* update browser redirect logic and specs

* restore router to test services

* test cleanup

* cleanup
;

* lint fix

* pass empty array for test routes

* remove check for redirects before dispatching pageNotFound

* add external route

* fix tests

* lint fix

* update specs and lint fix

---------

Co-authored-by: Beth Shook <[email protected]>
Co-authored-by: tom <[email protected]>

src/app/content/hooks/locationChange/resolveContent-in-production.spec.ts

@@ -174,6 +174,8 @@ describe('locationChange', () => {
         },
       };
 
+      jest.spyOn(helpers.router, 'findRoute').mockReturnValue(match);
+
       mockUUIDBook();
 
       await expect(hook(helpers, match)).resolves.toMatchInlineSnapshot(`

src/app/content/hooks/receivePageNotFoundId.spec.ts

@@ -1,5 +1,7 @@
 import createTestServices from '../../../test/createTestServices';
 import createTestStore from '../../../test/createTestStore';
+import { replace } from '../../navigation/actions';
+import { AnyMatch } from '../../navigation/types';
 import { MiddlewareAPI, Store } from '../../types';
 import { receivePageNotFoundId } from '../actions';
 
@@ -15,6 +17,7 @@ describe('receivePageNotFoundId hook', () => {
   let store: Store;
   let helpers: MiddlewareAPI & ReturnType<typeof createTestServices>;
   let historyReplaceSpy: jest.SpyInstance;
+  let dispatch: jest.SpyInstance;
   let fetchBackup: any;
 
   beforeEach(() => {
@@ -30,6 +33,8 @@ 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());
 
@@ -59,10 +64,10 @@ describe('receivePageNotFoundId hook', () => {
   });
 
   it('calls history.replace if redirect is found', async() => {
-    (globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: 'redirected' }]);
+    (globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: '/books/redirected' }]);
 
     await hook(receivePageNotFoundId('asdf'));
 
-    expect(historyReplaceSpy).toHaveBeenCalledWith('redirected');
+    expect(dispatch).toHaveBeenCalledWith(replace(helpers.router.findRoute('/books/redirected') as AnyMatch));
   });
 });

src/app/content/utils/processBrowserRedirect.ts

@@ -1,14 +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';
 
-export const processBrowserRedirect = async(services: {history: History}) => {
+export const processBrowserRedirect = async(services: {
+  router: RouterService,
+  history: History,
+  dispatch: Dispatch
+}) => {
   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.history.replace(to);
+      services.dispatch(replace(services.router.findRoute(to) as AnyMatch));
       return true;
     }
   }

src/app/developer/components/__snapshots__/Home.spec.tsx.snap

@@ -573,7 +573,17 @@ Array [
                   NotFound
                 </h3>
                 path: 
-                /(.*)
+                /books/(.*)
+                <br />
+              </div>
+              <div>
+                <h3
+                  className="c9"
+                >
+                  External
+                </h3>
+                path: 
+                :url(/.*)
                 <br />
               </div>
             </div>

src/app/errors/reducer.ts

@@ -3,7 +3,7 @@ import { getType } from 'typesafe-actions';
 import * as navigation from '../navigation';
 import { AnyAction } from '../types';
 import * as actions from './actions';
-import { notFound } from './routes';
+import { external, notFound } from './routes';
 import { State } from './types';
 
 export const initialState: State = {
@@ -23,6 +23,7 @@ const reducer: Reducer<State, AnyAction> = (state = initialState, action) => {
       return { ...state, sentryMessageIdStack: [ action.payload, ...state.sentryMessageIdStack] };
     case getType(navigation.actions.locationChange):
       return navigation.utils.matchForRoute(notFound, action.payload.match)
+        || navigation.utils.matchForRoute(external, action.payload.match)
         || action.payload.match === undefined
         ? {...state, code: 404}
         : {...state, code: 200};

src/app/errors/routes.spec.ts

@@ -1,12 +1,10 @@
 import pathToRegexp from 'path-to-regexp';
-import { notFound } from './routes';
+import { external, notFound } from './routes';
 
 describe('notFound', () => {
-  it('matches any route', () => {
+  it('matches any rex route', () => {
     const path = notFound.paths[0];
     const re = pathToRegexp(path, [], {end: true});
-    expect(re.exec('/woooo')).not.toEqual(null);
-    expect(re.exec('/foo/bar')).not.toEqual(null);
     expect(re.exec('/books/book/pages/page')).not.toEqual(null);
   });
 
@@ -21,3 +19,23 @@ describe('notFound', () => {
     expect(notFound.getSearch({url: 'url'})).toEqual('path=url');
   });
 });
+
+describe('external', () => {
+  it('matches any route', () => {
+    const path = external.paths[0];
+    const re = pathToRegexp(path, [], {end: true});
+    expect(re.exec('/woooo')).not.toEqual(null);
+    expect(re.exec('/foo/bar')).not.toEqual(null);
+  });
+
+  it('produces a relative url', () => {
+    expect(external.getUrl({url: 'url'})).toEqual('url');
+  });
+
+  it('produces a query string', () => {
+    if (!external.getSearch) {
+      return expect(notFound.getSearch).toBeTruthy();
+    }
+    expect(external.getSearch({url: 'url'})).toEqual('path=url');
+  });
+});

src/app/errors/routes.ts

@@ -1,7 +1,7 @@
 import Loadable from 'react-loadable';
 import { Route } from '../navigation/types';
 
-const CATCH_ALL = '/(.*)';
+const CATCH_ALL = '/books/(.*)';
 
 type Params = {
   url: string;
@@ -19,3 +19,15 @@ export const notFound: Route<Params> = {
   name: 'NotFound',
   paths: [CATCH_ALL],
 };
+
+export const external: Route<Params> = {
+  component: Loadable({
+    loader: () => import(/* webpackChunkName: "LoaderCentered" */ './components/LoaderCentered'),
+    loading: () => null,
+    modules: ['LoaderCentered'],
+    webpack: /* istanbul ignore next */ () => [(require as any).resolveWeak('./components/LoaderCentered')],
+  }),
+  getUrl: (params: Params) => params.url,
+  name: 'External',
+  paths: [':url(/.*)'],
+};

src/app/index.tsx

@@ -24,6 +24,7 @@ import { matchPathname } from './navigation/utils';
 import * as notifications from './notifications';
 import createReducer from './reducer';
 import { AppServices, AppState, Middleware } from './types';
+import { createRouterService } from './navigation/routerService';
 
 export const actions = {
   app: appAactions,
@@ -67,7 +68,8 @@ const defaultServices = () => ({
 export interface AppOptions {
   initialState?: Partial<AppState>;
   initialEntries?: AnyMatch[];
-  services: Pick<AppServices, Exclude<keyof AppServices, 'history' | keyof ReturnType<typeof defaultServices>>>;
+  services:
+    Pick<AppServices, Exclude<keyof AppServices, 'history' | 'router' | keyof ReturnType<typeof defaultServices>>>;
 }
 
 export default (options: AppOptions) => {
@@ -94,6 +96,7 @@ export default (options: AppOptions) => {
   const services: AppServices = {
     ...defaultServices(),
     ...options.services,
+    router: createRouterService(routes),
     history,
   };
 

src/app/navigation/middleware.ts

@@ -1,7 +1,7 @@
 import { History } from 'history';
 import queryString from 'query-string';
 import { getType } from 'typesafe-actions';
-import { notFound } from '../errors/routes';
+import { external, notFound } from '../errors/routes';
 import { AnyAction, Dispatch, Middleware } from '../types';
 import { assertWindow } from '../utils/browser-assertions';
 import * as actions from './actions';
@@ -17,10 +17,7 @@ export default (routes: AnyRoute[], history: History): Middleware => ({getState,
       return next(action);
     }
 
-    // special case for notFound because we want to hit the osweb page
-    // this could be made more generic with an `external` flag on the
-    // route or something
-    if (matchForRoute(notFound, action.payload)) {
+    if (matchForRoute(notFound, action.payload) || matchForRoute(external, action.payload)) {
       const { location } = assertWindow();
       const method = action.payload.method === 'push'
         ? location.assign.bind(location)

src/app/navigation/routerService.ts

@@ -0,0 +1,11 @@
+import { findRouteMatch } from './utils';
+import { Location } from 'history';
+import { AnyMatch, AnyRoute } from './types';
+
+export interface RouterService {
+  findRoute: (input: Location | string) => AnyMatch | undefined;
+}
+
+export const createRouterService = (routes: AnyRoute[]): RouterService => ({
+ findRoute: (input) => findRouteMatch(routes, input),
+});

src/app/navigation/utils/index.ts

@@ -55,14 +55,14 @@ const formatRouteMatch = <R extends AnyRoute>(route: R, state: RouteState<R>, ke
   state,
 } as AnyMatch);
 
-export const findRouteMatch = (routes: AnyRoute[], location: Location): AnyMatch | undefined => {
+export const findRouteMatch = (routes: AnyRoute[], location: Location | string): AnyMatch | undefined => {
   for (const route of routes) {
     for (const path of route.paths) {
       const keys: Key[] = [];
       const re = pathToRegexp(path, keys, {end: true});
-      const match = re.exec(location.pathname);
+      const match = re.exec(typeof location === 'string' ? location : location.pathname);
       if (match) {
-        return formatRouteMatch(route, location.state || {}, keys, match);
+        return formatRouteMatch(route, (typeof location !== 'string' && location.state) ?? {}, keys, match);
       }
     }
   }

src/app/types.ts

@@ -29,6 +29,7 @@ import { State as featureFlagsState } from './featureFlags/types';
 import { State as headState } from './head/types';
 import { State as navigationState } from './navigation/types';
 import { State as notificationState } from './notifications/types';
+import { RouterService } from './navigation/routerService';
 
 export interface AppState {
   content: contentState;
@@ -44,6 +45,7 @@ export interface AppServices {
   analytics: typeof analytics;
   archiveLoader: ReturnType<typeof createArchiveLoader>;
   buyPrintConfigLoader: ReturnType<typeof createBuyPrintConfigLoader>;
+  router: RouterService;
   config: typeof config;
   fontCollector: FontCollector;
   highlightClient: ReturnType<typeof createHighlightClient>;

src/test/createTestServices.ts

@@ -13,6 +13,7 @@ import mockbookConfigLoader from './mocks/bookConfigLoader';
 import mockOsWebLoader from './mocks/osWebLoader';
 import mockUserLoader from './mocks/userLoader';
 import createImageCDNUtils from '../gateways/createImageCDNUtils';
+import { createRouterService } from '../app/navigation/routerService';
 
 jest.mock('@openstax/open-search-client');
 jest.mock('@openstax/highlighter/dist/api');
@@ -35,6 +36,7 @@ export const createTestServices = (args?: {prefetchResolutions: boolean}) => ({
   searchClient: new SearchApi(),
   userLoader: mockUserLoader(),
   imageCDNUtils: createImageCDNUtils(args),
+  router: createRouterService([]),
 });
 
 export default createTestServices;