Skip to content

Commit 8afcf00

Browse files
Add special redirect route to load domains when the cluster is not known (#839)
* Add redirect logic * Resolve comments * Add not found page * Fix tests * Add comment and query params --------- Co-authored-by: Assem Hafez <[email protected]>
1 parent 2a852c6 commit 8afcf00

File tree

12 files changed

+276
-1
lines changed

12 files changed

+276
-1
lines changed

next.config.mjs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import { fileURLToPath } from 'url';
77
const __filename = fileURLToPath(import.meta.url);
88
const __dirname = path.dirname(__filename);
99

10-
const BUILD_OUTPUT = process.env.NEXT_CONFIG_BUILD_OUTPUT === 'standalone' ? 'standalone' : undefined;
10+
const BUILD_OUTPUT =
11+
process.env.NEXT_CONFIG_BUILD_OUTPUT === 'standalone'
12+
? 'standalone'
13+
: undefined;
1114

1215
const nextConfig = {
1316
webpack: (config) => {
@@ -20,6 +23,13 @@ const nextConfig = {
2023
redirects: async () => {
2124
// TODO - load tabs configs here to dynamically define redirects
2225
return [
26+
{
27+
// This regex matches paths that try to load a domain or workflow without specifying the active cluster
28+
source:
29+
'/domains/:path((?:[^/]+)(?:/(?:workflows|metadata|settings|archival|task-lists)(?:/.*)?)?)',
30+
destination: '/redirects/domain/:path',
31+
permanent: true,
32+
},
2333
{
2434
source: '/domains/:domain/:cluster',
2535
destination: '/domains/:domain/:cluster/workflows',
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
'use client';
2+
import RedirectDomainError from '@/views/redirect-domain/redirect-domain-error/redirect-domain-error';
3+
4+
export default RedirectDomainError;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import SectionLoadingIndicator from '@/components/section-loading-indicator/section-loading-indicator';
2+
3+
export default SectionLoadingIndicator;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import RedirectDomainNotFound from '@/views/redirect-domain/redirect-domain-not-found/redirect-domain-not-found';
2+
3+
export default RedirectDomainNotFound;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import RedirectDomain from '@/views/redirect-domain/redirect-domain';
2+
3+
export default RedirectDomain;
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
import { getDomainObj } from '@/views/domains-page/__fixtures__/domains';
2+
import { type DomainData } from '@/views/domains-page/domains-page.types';
3+
4+
import RedirectDomain from '../redirect-domain';
5+
6+
const MOCK_ALL_DOMAINS: Array<DomainData> = [
7+
getDomainObj({
8+
name: 'mock-domain-unique',
9+
id: 'mock-domain-id-unique',
10+
activeClusterName: 'mock-cluster-1',
11+
clusters: [
12+
{ clusterName: 'mock-cluster-1' },
13+
{ clusterName: 'mock-cluster-2' },
14+
],
15+
}),
16+
getDomainObj({
17+
name: 'mock-domain-shared-name',
18+
id: 'mock-domain-id-shared-name-1',
19+
activeClusterName: 'mock-cluster-1',
20+
clusters: [
21+
{ clusterName: 'mock-cluster-1' },
22+
{ clusterName: 'mock-cluster-2' },
23+
],
24+
}),
25+
getDomainObj({
26+
name: 'mock-domain-shared-name',
27+
id: 'mock-domain-id-shared-name-2',
28+
activeClusterName: 'mock-cluster-3',
29+
clusters: [
30+
{ clusterName: 'mock-cluster-3' },
31+
{ clusterName: 'mock-cluster-4' },
32+
],
33+
}),
34+
];
35+
36+
const mockRedirect = jest.fn();
37+
jest.mock('next/navigation', () => ({
38+
redirect: (url: string) => {
39+
mockRedirect(url);
40+
// Server component stops rendering after a redirect is called
41+
throw new Error('Redirected');
42+
},
43+
notFound: () => {
44+
// Server component stops rendering after notFound is called
45+
throw new Error('Not found');
46+
},
47+
}));
48+
49+
jest.mock(
50+
'@/views/domains-page/helpers/get-all-domains',
51+
jest.fn(() => ({
52+
getCachedAllDomains: jest.fn(() => ({
53+
domains: MOCK_ALL_DOMAINS,
54+
failedClusters: [],
55+
})),
56+
}))
57+
);
58+
59+
describe(RedirectDomain.name, () => {
60+
const tests: Array<{
61+
name: string;
62+
urlParams: Array<string>;
63+
queryParams?: { [key: string]: string | string[] | undefined };
64+
assertOnError?: (e: Error) => void;
65+
expectedRedirect?: string;
66+
}> = [
67+
{
68+
name: 'should redirect to domain page of active cluster',
69+
urlParams: ['mock-domain-unique'],
70+
expectedRedirect: '/domains/mock-domain-unique/mock-cluster-1',
71+
},
72+
{
73+
name: 'should redirect to workflow page of active cluster',
74+
urlParams: ['mock-domain-unique', 'workflows', 'mock-wfid', 'mock-runid'],
75+
expectedRedirect:
76+
'/domains/mock-domain-unique/mock-cluster-1/workflows/mock-wfid/mock-runid',
77+
},
78+
{
79+
name: 'should redirect with query params',
80+
urlParams: [
81+
'mock-domain-unique',
82+
'workflows',
83+
'mock-wfid',
84+
'mock-runid',
85+
'history',
86+
],
87+
queryParams: {
88+
ht: 'ACTIVITY',
89+
hs: 'COMPLETED',
90+
},
91+
expectedRedirect:
92+
'/domains/mock-domain-unique/mock-cluster-1/workflows/mock-wfid/mock-runid/history?ht=ACTIVITY&hs=COMPLETED',
93+
},
94+
{
95+
name: 'should redirect to All Domains page with search param if multiple domains exist',
96+
urlParams: ['mock-domain-shared-name'],
97+
expectedRedirect: '/domains?s=mock-domain-shared-name',
98+
},
99+
{
100+
name: 'should redirect to All Domains page with search param if multiple domains exist, for workflow link',
101+
urlParams: [
102+
'mock-domain-shared-name',
103+
'workflows',
104+
'mock-wfid',
105+
'mock-runid',
106+
],
107+
expectedRedirect: '/domains?s=mock-domain-shared-name',
108+
},
109+
{
110+
name: 'should call notFound if no domain exists',
111+
urlParams: ['mock-domain-nonexistent'],
112+
assertOnError: (e) => {
113+
expect(e.message).toEqual('Not found');
114+
},
115+
},
116+
{
117+
// This never happens in practice because the router simply would not route to this component
118+
name: 'should throw if domain is invalid',
119+
urlParams: [],
120+
assertOnError: (e) =>
121+
expect(e.message).toEqual('Invalid domain URL param'),
122+
},
123+
];
124+
125+
tests.forEach((test) =>
126+
it(test.name, async () => {
127+
try {
128+
await RedirectDomain({
129+
params: { domainParams: test.urlParams },
130+
});
131+
132+
expect(mockRedirect).toHaveBeenCalledWith(test.expectedRedirect);
133+
} catch (e) {
134+
if (e instanceof Error && e.message !== 'Redirected') {
135+
expect(test.assertOnError).toBeDefined();
136+
test.assertOnError?.(e);
137+
}
138+
}
139+
})
140+
);
141+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
'use client';
2+
import { useParams } from 'next/navigation';
3+
4+
import ErrorPanel from '@/components/error-panel/error-panel';
5+
6+
import { type RouteParams } from '../redirect-domain.types';
7+
8+
import { type Props } from './redirect-domain-error.types';
9+
10+
export default function RedirectDomainError({ error }: Props) {
11+
const { domainParams } = useParams<RouteParams>();
12+
13+
const domain = decodeURIComponent(domainParams[0]);
14+
15+
return (
16+
<ErrorPanel
17+
error={error}
18+
message={`Could not redirect to domain "${domain}"`}
19+
actions={[
20+
{
21+
kind: 'link-internal',
22+
label: 'Go to domain overview',
23+
link: '/domains',
24+
},
25+
]}
26+
omitLogging={true}
27+
/>
28+
);
29+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export type Props = {
2+
error: Error;
3+
};
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
'use client';
2+
import { useParams } from 'next/navigation';
3+
4+
import ErrorPanel from '@/components/error-panel/error-panel';
5+
6+
import { type RouteParams } from '../redirect-domain.types';
7+
8+
export default function RedirectDomainNotFound() {
9+
const { domainParams } = useParams<RouteParams>();
10+
11+
const domain = decodeURIComponent(domainParams[0]);
12+
13+
return (
14+
<ErrorPanel
15+
message={`The domain "${domain}" was not found`}
16+
actions={[
17+
{
18+
kind: 'link-internal',
19+
label: 'Go to domain overview',
20+
link: '/domains',
21+
},
22+
]}
23+
omitLogging={true}
24+
/>
25+
);
26+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { notFound, redirect } from 'next/navigation';
2+
import queryString from 'query-string';
3+
4+
import { getCachedAllDomains } from '../domains-page/helpers/get-all-domains';
5+
6+
import { type Props } from './redirect-domain.types';
7+
8+
export default async function RedirectDomain(props: Props) {
9+
const [encodedDomain, ...restParams] = props.params.domainParams;
10+
if (!encodedDomain) {
11+
throw new Error('Invalid domain URL param');
12+
}
13+
14+
const domain = decodeURIComponent(encodedDomain);
15+
16+
const { domains } = await getCachedAllDomains();
17+
18+
const [domainDetails, ...restDomains] = domains.filter(
19+
(d) => d.name === domain
20+
);
21+
22+
if (!domainDetails) {
23+
notFound();
24+
} else if (restDomains.length > 0) {
25+
redirect(
26+
queryString.stringifyUrl({
27+
url: '/domains',
28+
query: {
29+
// TODO @assem.hafez: see if this type can be asserted
30+
s: domain,
31+
},
32+
})
33+
);
34+
}
35+
36+
const baseUrl = `/domains/${encodeURIComponent(domain)}/${encodeURIComponent(domainDetails.activeClusterName)}`;
37+
38+
redirect(
39+
queryString.stringifyUrl({
40+
url: baseUrl + (restParams.length > 0 ? `/${restParams.join('/')}` : ''),
41+
query: props.searchParams,
42+
})
43+
);
44+
}

0 commit comments

Comments
 (0)