Skip to content

Commit

Permalink
update browser redirect logic and specs
Browse files Browse the repository at this point in the history
  • Loading branch information
Beth Shook committed Apr 16, 2024
1 parent 81e8fc2 commit f599851
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ import createTestStore from '../../../../test/createTestStore';
import { book, page } from '../../../../test/mocks/archiveLoader';
import { resetModules } from '../../../../test/utils';
import { Match } from '../../../navigation/types';
import { MiddlewareAPI, Store } from '../../../types';
import { AppServices, MiddlewareAPI, Store } from '../../../types';
import * as routes from '../../routes';
import * as content from '../../../content';
import { Params, SlugParams } from '../../types';
import { createRouterService } from '../../../navigation/routerService';

const mockFetch = (valueToReturn: any, error?: any) => () => new Promise((resolve, reject) => {
if (error) {
Expand All @@ -25,9 +27,10 @@ const testPage = 'test-page-1';

describe('locationChange', () => {
let store: Store;
let helpers: ReturnType<typeof createTestServices> & MiddlewareAPI;
let helpers: ReturnType<typeof createTestServices> & MiddlewareAPI & Pick<AppServices, 'router'>;
let match: Match<typeof routes.content>;
let hook: typeof import ('./resolveContent').default;
let router: ReturnType<typeof createRouterService>;
let resolveExternalBookReference: typeof import ('./resolveContent').resolveExternalBookReference;

const mockUUIDBook = () => {
Expand Down Expand Up @@ -75,10 +78,12 @@ describe('locationChange', () => {
beforeEach(() => {
resetModules();
store = createTestStore();
router = createRouterService(Object.values(content.routes));
helpers = {
...createTestServices(),
dispatch: store.dispatch,
getState: store.getState,
router,
};

match = {
Expand Down Expand Up @@ -165,6 +170,8 @@ describe('locationChange', () => {
helpers.bookConfigLoader.localBookConfig[testUUID] = { defaultVersion: '1.0', retired: true };
helpers.osWebLoader.getBookIdFromSlug.mockResolvedValue(testUUID);

// const match = {route: {getUrl: jest.fn(() => 'url')}} as unknown as AnyMatch;

match.params = {
book: {
slug: 'book-slug-1',
Expand All @@ -174,6 +181,8 @@ describe('locationChange', () => {
},
};

jest.spyOn(router, 'findRoute').mockReturnValue(match);

mockUUIDBook();

await expect(hook(helpers, match)).resolves.toMatchInlineSnapshot(`
Expand Down
5 changes: 4 additions & 1 deletion src/app/content/hooks/locationChange/resolveContent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ const resolvePage = async(
const pageId = match.params.page ? getPageIdFromUrlParam(book, match.params.page) : undefined;

if (!pageId) {
dispatch(receivePageNotFoundId(getIdFromPageParam(match.params.page)));
const hasRedirects = await processBrowserRedirect(services);
if (!hasRedirects) {
dispatch(receivePageNotFoundId(getIdFromPageParam(match.params.page)));
}
return;
}

Expand Down
24 changes: 0 additions & 24 deletions src/app/content/hooks/receivePageNotFoundId.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import createTestServices from '../../../test/createTestServices';
import createTestStore from '../../../test/createTestStore';
import { notFound } from '../../errors/routes';
import { AnyMatch } from '../../navigation/types';
import { MiddlewareAPI, Store } from '../../types';
import { assertWindow } from '../../utils';
import { receivePageNotFoundId } from '../actions';
Expand Down Expand Up @@ -67,26 +65,4 @@ describe('receivePageNotFoundId hook', () => {

expect(historyReplaceSpy).not.toHaveBeenCalled();
});

it('calls history.replace if redirect is found and target is within rex', async() => {
(globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: 'redirected' }]);

const match = {route: {getUrl: jest.fn(() => 'url')}, state: true} as unknown as AnyMatch;
jest.spyOn(helpers.router, 'findRoute').mockReturnValue(match);

await hook(receivePageNotFoundId('asdf'));

expect(historyReplaceSpy).toHaveBeenCalledWith('redirected');
});

it('does not call history.replace if target is not within rex', async() => {
(globalThis as any).fetch = mockFetch([{ from: helpers.history.location.pathname, to: '/redirected' }]);

const match = {route: notFound, state: false} as unknown as AnyMatch;
jest.spyOn(helpers.router, 'findRoute').mockReturnValue(match);

await hook(receivePageNotFoundId('asdf'));

expect(historyReplaceSpy).not.toHaveBeenCalled();
});
});
75 changes: 75 additions & 0 deletions src/app/content/utils/processBrowserRedirect.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import createTestServices from '../../../test/createTestServices';
import createTestStore from '../../../test/createTestStore';
import { notFound } from '../../errors/routes';
import { createRouterService } from '../../navigation/routerService';
import { AnyMatch } from '../../navigation/types';
import { AppServices, Store } from '../../types';
import { assertWindow } from '../../utils';
import { processBrowserRedirect } from './processBrowserRedirect';
import * as content from '../../content';

const mockFetch = (valueToReturn: any, error?: any) => () => new Promise((resolve, reject) => {
if (error) {
reject(error);
}
resolve({ json: () => new Promise((res) => res(valueToReturn)) });
});

describe('processBrowserRedirect', () => {
let services: AppServices;
let store: Store;
let historyReplaceSpy: jest.SpyInstance;
let fetchBackup: any;
let router: ReturnType<typeof createRouterService>;
let window: Window;

beforeEach(() => {
store = createTestStore();
window = assertWindow();
router = createRouterService(Object.values(content.routes));
services = {
...createTestServices(),
router,
};
delete (window as any).location;

window.location = {
origin: 'openstax.org',
} as any as Window['location'];

services.history.location = {
pathname: '/books/physics/pages/1-introduction301',
} as any;

historyReplaceSpy = jest.spyOn(services.history, 'replace')
.mockImplementation(jest.fn());

fetchBackup = (globalThis as any).fetch;
});

afterEach(() => {
(globalThis as any).fetch = fetchBackup;
});

it('calls history.replace if redirect is found', async() => {
(globalThis as any).fetch = mockFetch([{ from: services.history.location.pathname, to: 'redirected' }]);

const match = {route: {getUrl: jest.fn(() => 'url')}} as unknown as AnyMatch;
jest.spyOn(router, 'findRoute').mockReturnValue(match);

await processBrowserRedirect(services);

expect(historyReplaceSpy).toHaveBeenCalledWith('redirected');
});

it('updates window.location if target is not within rex', async() => {
(globalThis as any).fetch = mockFetch([{ from: services.history.location.pathname, to: '/redirected' }]);

const match = {route: notFound, state: false} as unknown as AnyMatch;
jest.spyOn(router, 'findRoute').mockReturnValue(match);

await processBrowserRedirect(services);

expect(window.location.href).toEqual('openstax.org/redirected');
});
});
4 changes: 1 addition & 3 deletions src/app/content/utils/processBrowserRedirect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ export const processBrowserRedirect = async(services: {router: RouterService, hi
for (const {from, to} of redirects) {
if (from === services.history.location.pathname) {
if (matchForRoute(notFound, services.router.findRoute(to))) {
const origin = window.location.origin;
window.location.href = origin + to;
return true;
window.location.href = window.location.origin + to;
}
services.history.replace(to);
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/app/errors/components/OuterErrorBoundary.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('OuterErrorBoundary', () => {

await runHooksAsync(renderer);

expect(loaded).toBe(true);
expect(loaded).toBe(false);
});

it('defaults to en if browser locale is not supported', async() => {
Expand Down
2 changes: 1 addition & 1 deletion src/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export interface AppServices {
analytics: typeof analytics;
archiveLoader: ReturnType<typeof createArchiveLoader>;
buyPrintConfigLoader: ReturnType<typeof createBuyPrintConfigLoader>;
router: RouterService;
router?: RouterService;
config: typeof config;
fontCollector: FontCollector;
highlightClient: ReturnType<typeof createHighlightClient>;
Expand Down
2 changes: 2 additions & 0 deletions src/test/createTestServices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ 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';
import { routes } from '../app';

jest.mock('@openstax/open-search-client');
jest.mock('@openstax/highlighter/dist/api');
Expand Down

0 comments on commit f599851

Please sign in to comment.