Skip to content

Commit

Permalink
Merge pull request #5265 from dfe-analytical-services/EES-5464
Browse files Browse the repository at this point in the history 
EES-5464 replace CSV download rewrite with middleware redirect
  • Loading branch information
@bennettstuart
bennettstuart committed Sep 20, 2024
2 parents 4b0654e + 6f105ed commit ed346de
Show file tree
Hide file tree
Showing 12 changed files with 262 additions and 101 deletions.
7 changes: 7 additions & 0 deletions pnpm-lock.yaml
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions src/explore-education-statistics-frontend/next.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,6 @@ const nextConfig = {
},
];
},
async rewrites() {
return [
{
source: '/data-catalogue/data-set/:dataSetFileId/csv',
destination: `${process.env.CONTENT_API_BASE_URL}/data-set-files/:dataSetFileId/download`,
},
];
},
webpack(config, options) {
const { dev, isServer } = options;

Expand Down
1 change: 1 addition & 0 deletions src/explore-education-statistics-frontend/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
"stylelint": "^15.7.0",
"stylelint-webpack-plugin": "^4.1.1",
"typescript": "^5.5.4",
"urlpattern-polyfill": "^10.0.0",
"webpack-dev-server": "^4.3.1"
},
"scripts": {
Expand Down
8 changes: 4 additions & 4 deletions src/explore-education-statistics-frontend/src/middleware.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import redirectPages from '@frontend/middleware/pages/redirectPages';
import type { NextRequest } from 'next/server';
import chain from './middleware/pages/chain';
import rewritePaths from './middleware/pages/rewritePaths';

export default async function middleware(request: NextRequest) {
return redirectPages(request);
}
export default chain([rewritePaths, redirectPages]);

// Only run the middleware on the specified paths below.
// Ideally we'd just exclude build files and run it on all routes,
Expand All @@ -16,6 +15,7 @@ export const config = {
'/find-statistics/:path*/:path*',
'/data-tables',
'/data-catalogue',
'/data-catalogue/data-set/:dataSetFileId/csv',
'/methodology/:path*',
'/subscriptions',
],
Expand Down
164 changes: 79 additions & 85 deletions ...xplore-education-statistics-frontend/src/middleware/pages/__tests__/redirectPages.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import redirectPages from '@frontend/middleware/pages/redirectPages';
import _redirectService, {
Redirects,
} from '@frontend/services/redirectService';
import redirectPages from '@frontend/middleware/pages/redirectPages';
import { NextResponse, NextRequest } from 'next/server';
import { NextResponse } from 'next/server';
import runMiddleware from './util/runMiddleware';

jest.mock('@frontend/services/redirectService');
const redirectService = _redirectService as jest.Mocked<
Expand All @@ -26,50 +27,45 @@ describe('redirectPages', () => {

test('does not re-request the list of redirects once it has been fetched', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/methodology/original-slug'),

await runMiddleware(
redirectPages,
'https://my-env/methodology/original-slug',
);
await redirectPages(req);

expect(redirectService.list).toHaveBeenCalledTimes(1);

const req2 = new NextRequest(
new Request('https://my-env/methodology/another-slug'),
await runMiddleware(
redirectPages,
'https://my-env/methodology/another-slug',
);
await redirectPages(req2);

expect(redirectService.list).toHaveBeenCalledTimes(1);
});

test('does not check for redirects for non release or methodology pages', async () => {
await redirectPages(
new NextRequest(new Request('https://my-env/find-statistics')),
);
await runMiddleware(redirectPages, 'https://my-env/find-statistics');

expect(redirectService.list).not.toHaveBeenCalled();
expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);

await redirectPages(
new NextRequest(new Request('https://my-env/methodology')),
);
await runMiddleware(redirectPages, 'https://my-env/methodology');

expect(redirectService.list).not.toHaveBeenCalled();
expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(2);

await redirectPages(
new NextRequest(new Request('https://my-env/data-tables/something')),
);
await runMiddleware(redirectPages, 'https://my-env/data-tables/something');

expect(redirectService.list).not.toHaveBeenCalled();
expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(3);
});

test('redirects urls with uppercase characters to lowercase', async () => {
redirectService.list.mockResolvedValue(testRedirects);
await redirectPages(
new NextRequest(new Request('https://my-env/Find-Statistics')),
);
await runMiddleware(redirectPages, 'https://my-env/Find-Statistics');

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -80,12 +76,9 @@ describe('redirectPages', () => {
);
expect(nextSpy).not.toHaveBeenCalled();

await redirectPages(
new NextRequest(
new Request(
'https://my-env/find-statistics/RELEASE-NAME?testParam=Something',
),
),
await runMiddleware(
redirectPages,
'https://my-env/find-statistics/RELEASE-NAME?testParam=Something',
);

expect(redirectSpy).toHaveBeenCalledTimes(2);
Expand All @@ -101,10 +94,11 @@ describe('redirectPages', () => {
describe('redirect methodology pages', () => {
test('redirects the request when the slug for the requested page has changed', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/methodology/original-slug-1'),

await runMiddleware(
redirectPages,
'https://my-env/methodology/original-slug-1',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -118,40 +112,40 @@ describe('redirectPages', () => {

test('does not redirect when the slug for the requested page has not changed', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/methodology/my-methodology'),

await runMiddleware(
redirectPages,
'https://my-env/methodology/my-methodology',
);
await redirectPages(req);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);
});

test('does not redirect if the `fromSlug` only partially matches', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/methodology/original'),
);
await redirectPages(req);

await runMiddleware(redirectPages, 'https://my-env/methodology/original');

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);

const req2 = new NextRequest(
new Request('https://my-env/methodology/original-slug-and-something'),
await runMiddleware(
redirectPages,
'https://my-env/methodology/original-slug-and-something',
);
await redirectPages(req2);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(2);
});

test('redirects child pages', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/methodology/original-slug-1/child-page'),

await runMiddleware(
redirectPages,
'https://my-env/methodology/original-slug-1/child-page',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -165,12 +159,11 @@ describe('redirectPages', () => {

test('redirects with query params', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request(
'https://my-env/methodology/original-slug-1?search=something',
),

await runMiddleware(
redirectPages,
'https://my-env/methodology/original-slug-1?search=something',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -184,22 +177,23 @@ describe('redirectPages', () => {

test('does not redirect when the slug matches a `fromSlug` in a different page type', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/methodology/original-slug-4'),
);

await redirectPages(req);
await runMiddleware(
redirectPages,
'https://my-env/methodology/original-slug-4',
);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);
});

test('redirects with uppercase characters', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/Methodology/original-SLUG-1'),

await runMiddleware(
redirectPages,
'https://my-env/Methodology/original-SLUG-1',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -215,10 +209,11 @@ describe('redirectPages', () => {
describe('redirect publication pages', () => {
test('redirects the request when the slug for the requested page has changed', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/find-statistics/original-slug-3'),

await runMiddleware(
redirectPages,
'https://my-env/find-statistics/original-slug-3',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -232,44 +227,43 @@ describe('redirectPages', () => {

test('does not redirect when the slug for the requested page has not changed', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/find-statistics/my-publication'),

await runMiddleware(
redirectPages,
'https://my-env/find-statistics/my-publication',
);
await redirectPages(req);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);
});

test('does not redirect if the `fromSlug` only partially matches', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/find-statistics/original'),

await runMiddleware(
redirectPages,
'https://my-env/find-statistics/original',
);
await redirectPages(req);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);

const req2 = new NextRequest(
new Request(
'https://my-env/find-statistics/original-slug-and-something',
),
await runMiddleware(
redirectPages,
'https://my-env/find-statistics/original-slug-and-something',
);
await redirectPages(req2);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(2);
});

test('redirects child pages', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request(
'https://my-env/find-statistics/original-slug-3/child-page',
),

await runMiddleware(
redirectPages,
'https://my-env/find-statistics/original-slug-3/child-page',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand All @@ -283,12 +277,11 @@ describe('redirectPages', () => {

test('redirects with query params', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request(
'https://my-env/find-statistics/original-slug-3?search=something',
),

await runMiddleware(
redirectPages,
'https://my-env/find-statistics/original-slug-3?search=something',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);

Expand All @@ -303,22 +296,23 @@ describe('redirectPages', () => {

test('does not redirect when the slug matches a `fromSlug` in a different page type', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/find-statistics/original-slug-1'),
);

await redirectPages(req);
await runMiddleware(
redirectPages,
'https://my-env/find-statistics/original-slug-1',
);

expect(redirectSpy).not.toHaveBeenCalled();
expect(nextSpy).toHaveBeenCalledTimes(1);
});

test('redirects with uppercase characters', async () => {
redirectService.list.mockResolvedValue(testRedirects);
const req = new NextRequest(
new Request('https://my-env/find-Statistics/original-SLUG-3'),

await runMiddleware(
redirectPages,
'https://my-env/find-Statistics/original-SLUG-3',
);
await redirectPages(req);

expect(redirectSpy).toHaveBeenCalledTimes(1);
expect(redirectSpy).toHaveBeenCalledWith(
Expand Down
Loading

0 comments on commit ed346de

Please sign in to comment.