Skip to content

Commit 06032eb

Browse files
adhityamamallanadhityamamallan
authored
Correctly render active-active domains in the domain listing page (#998)
* Create types and helpers for active-active domains (one to check if domain is active-active and one to evaluate the default active cluster for an active-active domain) * Use default active cluster in domain page redirect * Use default active cluster in domains list
1 parent 3df9287 commit 06032eb

14 files changed

+317
-11
lines changed

‎src/views/domains-page/domains-table-domain-name-cell/__tests__/domains-table-domain-name-cell.test.tsx‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import React from 'react';
22

33
import { render, screen } from '@/test-utils/rtl';
44

5+
import { mockActiveActiveDomain } from '@/views/shared/active-active/__fixtures__/active-active-domain';
6+
57
import { globalDomain } from '../../__fixtures__/domains';
68
import DomainsTableDomainNameCell from '../domains-table-domain-name-cell';
79

@@ -11,6 +13,11 @@ jest.mock('@/views/shared/domain-status-tag/domain-status-tag', () =>
1113
))
1214
);
1315

16+
jest.mock('@/views/shared/active-active/helpers/is-active-active-domain');
17+
jest.mock(
18+
'@/views/shared/active-active/helpers/get-default-cluster-for-active-active-domain'
19+
);
20+
1421
describe('DomainTableClusterCell', () => {
1522
it('should render link for domain if domain using the active cluster', async () => {
1623
render(<DomainsTableDomainNameCell {...globalDomain} />);
@@ -24,6 +31,18 @@ describe('DomainTableClusterCell', () => {
2431
});
2532
});
2633

34+
it('should render link for domain if domain is active-active using default cluster', async () => {
35+
render(<DomainsTableDomainNameCell {...mockActiveActiveDomain} />);
36+
const clusterLinks = await screen.findAllByRole('link');
37+
clusterLinks.forEach((clusterLink) => {
38+
expect(clusterLink.innerHTML).toBe(mockActiveActiveDomain.name);
39+
expect(clusterLink).toHaveAttribute(
40+
'href',
41+
`/domains/${mockActiveActiveDomain.name}/cluster0`
42+
);
43+
});
44+
});
45+
2746
it('should render tag for domain if domain has status other than registered', async () => {
2847
render(
2948
<DomainsTableDomainNameCell

‎src/views/domains-page/domains-table-domain-name-cell/domains-table-domain-name-cell.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,23 @@ import cadenceIcon from '@/assets/cadence-logo.svg';
77
import Link from '@/components/link/link';
88
import useStyletronClasses from '@/hooks/use-styletron-classes';
99
import type { DomainData } from '@/views/domains-page/domains-page.types';
10+
import getDefaultClusterForActiveActiveDomain from '@/views/shared/active-active/helpers/get-default-cluster-for-active-active-domain';
11+
import isActiveActiveDomain from '@/views/shared/active-active/helpers/is-active-active-domain';
1012
import DomainStatusTag from '@/views/shared/domain-status-tag/domain-status-tag';
1113

1214
import { cssStyles } from './domains-table-domain-name-cell.styles';
1315

1416
function DomainsTableDomainNameCell(props: DomainData) {
1517
const { cls } = useStyletronClasses(cssStyles);
1618

19+
const clusterName = isActiveActiveDomain(props)
20+
? getDefaultClusterForActiveActiveDomain(props)
21+
: props.activeClusterName;
22+
1723
return (
1824
<div className={cls.domainNameCell}>
1925
<Image width={16} height={16} alt="Cadence Icon" src={cadenceIcon} />
20-
<Link href={`/domains/${props.name}/${props.activeClusterName}`}>
21-
{props.name}
22-
</Link>
26+
<Link href={`/domains/${props.name}/${clusterName}`}>{props.name}</Link>
2327
{props.status !== 'DOMAIN_STATUS_REGISTERED' && (
2428
<DomainStatusTag status={props.status} />
2529
)}

‎src/views/domains-page/helpers/__tests__/get-unique-domains.test.ts‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,15 @@
1+
import { mockActiveActiveDomain } from '@/views/shared/active-active/__fixtures__/active-active-domain';
2+
13
import { getDomainObj } from '../../__fixtures__/domains';
24
import { type DomainData } from '../../domains-page.types';
35
import getUniqueDomains from '../get-unique-domains';
46

7+
jest.mock('@/views/shared/active-active/helpers/is-active-active-domain');
8+
9+
jest.mock(
10+
'@/views/shared/active-active/helpers/get-default-cluster-for-active-active-domain'
11+
);
12+
513
describe('getUniqueDomains', () => {
614
it('should return unique domains based on id-name-activeClusterName', () => {
715
const domains: DomainData[] = [
@@ -63,4 +71,30 @@ describe('getUniqueDomains', () => {
6371
}),
6472
]);
6573
});
74+
75+
it('should handle active-active domains using their default cluster for uniqueness', () => {
76+
const domains: DomainData[] = [
77+
mockActiveActiveDomain,
78+
getDomainObj({
79+
id: mockActiveActiveDomain.id,
80+
name: mockActiveActiveDomain.name,
81+
activeClusterName: 'cluster0', // Same as default cluster from mock
82+
clusters: [],
83+
}),
84+
getDomainObj({
85+
id: '2',
86+
name: 'Domain2',
87+
activeClusterName: 'ClusterB',
88+
clusters: [],
89+
}),
90+
];
91+
92+
const uniqueDomains = getUniqueDomains(domains);
93+
94+
// Should return only 2 domains since the first two are duplicates
95+
// (active-active domain and regular domain with same id-name-defaultCluster)
96+
expect(uniqueDomains).toHaveLength(2);
97+
expect(uniqueDomains).toContain(domains[0]); // mockActiveActiveDomain
98+
expect(uniqueDomains).toContain(domains[2]); // Domain2
99+
});
66100
});

‎src/views/domains-page/helpers/get-unique-domains.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
1+
import getDefaultClusterForActiveActiveDomain from '@/views/shared/active-active/helpers/get-default-cluster-for-active-active-domain';
2+
import isActiveActiveDomain from '@/views/shared/active-active/helpers/is-active-active-domain';
3+
14
import { type DomainData } from '../domains-page.types';
25

36
export default function getUniqueDomains(domains: DomainData[]) {
47
const allUniqueDomains: Record<string, boolean> = {};
58
return domains.filter((d: DomainData) => {
6-
if (allUniqueDomains[`${d.id}-${d.name}-${d.activeClusterName}`])
7-
return false;
9+
const defaultCluster = isActiveActiveDomain(d)
10+
? getDefaultClusterForActiveActiveDomain(d)
11+
: d.activeClusterName;
12+
13+
if (allUniqueDomains[`${d.id}-${d.name}-${defaultCluster}`]) return false;
814

9-
allUniqueDomains[`${d.id}-${d.name}-${d.activeClusterName}`] = true;
15+
allUniqueDomains[`${d.id}-${d.name}-${defaultCluster}`] = true;
1016
return true;
1117
});
1218
}

‎src/views/redirect-domain/__tests__/redirect-domain.node.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
import { getDomainObj } from '@/views/domains-page/__fixtures__/domains';
22
import { type DomainData } from '@/views/domains-page/domains-page.types';
3+
import { mockActiveActiveDomain } from '@/views/shared/active-active/__fixtures__/active-active-domain';
34

45
import RedirectDomain from '../redirect-domain';
56

7+
jest.mock('@/views/shared/active-active/helpers/is-active-active-domain');
8+
9+
jest.mock(
10+
'@/views/shared/active-active/helpers/get-default-cluster-for-active-active-domain'
11+
);
12+
613
const MOCK_ALL_DOMAINS: Array<DomainData> = [
714
getDomainObj({
815
name: 'mock-domain-unique',
@@ -31,6 +38,7 @@ const MOCK_ALL_DOMAINS: Array<DomainData> = [
3138
{ clusterName: 'mock-cluster-4' },
3239
],
3340
}),
41+
mockActiveActiveDomain,
3442
];
3543

3644
const mockRedirect = jest.fn();
@@ -69,6 +77,11 @@ describe(RedirectDomain.name, () => {
6977
urlParams: ['mock-domain-unique'],
7078
expectedRedirect: '/domains/mock-domain-unique/mock-cluster-1',
7179
},
80+
{
81+
name: 'should redirect to domain page of default UI cluster for an active-active domain',
82+
urlParams: ['mock-domain-active-active'],
83+
expectedRedirect: '/domains/mock-domain-active-active/cluster0',
84+
},
7285
{
7386
name: 'should redirect to workflow page of active cluster',
7487
urlParams: ['mock-domain-unique', 'workflows', 'mock-wfid', 'mock-runid'],
@@ -85,11 +98,11 @@ describe(RedirectDomain.name, () => {
8598
'history',
8699
],
87100
queryParams: {
88-
ht: 'ACTIVITY',
89101
hs: 'COMPLETED',
102+
ht: 'ACTIVITY',
90103
},
91104
expectedRedirect:
92-
'/domains/mock-domain-unique/mock-cluster-1/workflows/mock-wfid/mock-runid/history?ht=ACTIVITY&hs=COMPLETED',
105+
'/domains/mock-domain-unique/mock-cluster-1/workflows/mock-wfid/mock-runid/history?hs=COMPLETED&ht=ACTIVITY',
93106
},
94107
{
95108
name: 'should redirect to All Domains page with search param if multiple domains exist',
@@ -127,13 +140,14 @@ describe(RedirectDomain.name, () => {
127140
try {
128141
await RedirectDomain({
129142
params: { domainParams: test.urlParams },
143+
searchParams: test.queryParams ?? undefined,
130144
});
131-
132-
expect(mockRedirect).toHaveBeenCalledWith(test.expectedRedirect);
133145
} catch (e) {
134146
if (e instanceof Error && e.message !== 'Redirected') {
135147
expect(test.assertOnError).toBeDefined();
136148
test.assertOnError?.(e);
149+
} else if (e instanceof Error && e.message === 'Redirected') {
150+
expect(mockRedirect).toHaveBeenCalledWith(test.expectedRedirect);
137151
}
138152
}
139153
})

‎src/views/redirect-domain/redirect-domain.tsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import { notFound, redirect } from 'next/navigation';
22
import queryString from 'query-string';
33

4+
import getDefaultClusterForActiveActiveDomain from '@/views/shared/active-active/helpers/get-default-cluster-for-active-active-domain';
5+
import isActiveActiveDomain from '@/views/shared/active-active/helpers/is-active-active-domain';
6+
47
import { getCachedAllDomains } from '../domains-page/helpers/get-all-domains';
58

69
import { type Props } from './redirect-domain.types';
@@ -33,7 +36,11 @@ export default async function RedirectDomain(props: Props) {
3336
);
3437
}
3538

36-
const baseUrl = `/domains/${encodeURIComponent(domain)}/${encodeURIComponent(domainDetails.activeClusterName)}`;
39+
const clusterToRedirectTo = isActiveActiveDomain(domainDetails)
40+
? getDefaultClusterForActiveActiveDomain(domainDetails)
41+
: domainDetails.activeClusterName;
42+
43+
const baseUrl = `/domains/${encodeURIComponent(domain)}/${encodeURIComponent(clusterToRedirectTo)}`;
3744

3845
redirect(
3946
queryString.stringifyUrl({

‎src/views/shared/active-active/__fixtures__/active-active-domain.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { type ActiveActiveDomain } from '../active-active.types';
2+
3+
export const mockActiveActiveDomain: ActiveActiveDomain = {
4+
clusters: [
5+
{
6+
clusterName: 'cluster0',
7+
},
8+
{
9+
clusterName: 'cluster1',
10+
},
11+
],
12+
data: {},
13+
id: 'ac531e7e-2915-4d71-ba11-a18f11db09fd',
14+
name: 'mock-domain-active-active',
15+
status: 'DOMAIN_STATUS_REGISTERED',
16+
description: '',
17+
ownerEmail: '',
18+
workflowExecutionRetentionPeriod: {
19+
seconds: '86400',
20+
nanos: 0,
21+
},
22+
badBinaries: {
23+
binaries: {},
24+
},
25+
historyArchivalStatus: 'ARCHIVAL_STATUS_DISABLED',
26+
historyArchivalUri: '',
27+
visibilityArchivalStatus: 'ARCHIVAL_STATUS_DISABLED',
28+
visibilityArchivalUri: '',
29+
activeClusterName: '',
30+
failoverVersion: '-24',
31+
isGlobalDomain: true,
32+
failoverInfo: null,
33+
isolationGroups: {
34+
isolationGroups: [],
35+
},
36+
asyncWorkflowConfig: {
37+
enabled: false,
38+
predefinedQueueName: '',
39+
queueType: '',
40+
queueConfig: null,
41+
},
42+
activeClusters: {
43+
regionToCluster: {
44+
region0: {
45+
activeClusterName: 'cluster0',
46+
failoverVersion: '0',
47+
},
48+
region1: {
49+
activeClusterName: 'cluster1',
50+
failoverVersion: '2',
51+
},
52+
},
53+
},
54+
};

‎src/views/shared/active-active/active-active.types.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import { type ActiveClusters } from '@/__generated__/proto-ts/uber/cadence/api/v1/ActiveClusters';
2+
import { type Domain } from '@/__generated__/proto-ts/uber/cadence/api/v1/Domain';
3+
4+
export type ActiveActiveDomain = Domain & {
5+
activeClusters: ActiveClusters;
6+
};

‎src/views/shared/active-active/helpers/__mocks__/get-default-cluster-for-active-active-domain.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { type ActiveActiveDomain } from '../../active-active.types';
2+
3+
export default function getDefaultClusterForActiveActiveDomain(
4+
domain: ActiveActiveDomain
5+
) {
6+
return Object.values(domain.activeClusters.regionToCluster)[0]
7+
.activeClusterName;
8+
}

‎src/views/shared/active-active/helpers/__mocks__/is-active-active-domain.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { type DomainData } from '@/views/domains-page/domains-page.types';
2+
3+
// Manual mock for isActiveActiveDomain
4+
export default function isActiveActiveDomain(domain: DomainData) {
5+
return Boolean(
6+
domain.activeClusters?.regionToCluster &&
7+
Object.entries(domain.activeClusters.regionToCluster).length > 0
8+
);
9+
}

0 commit comments

Comments
 (0)