Skip to content

Commit 43106fc

Browse files
authored
Change domain listing failure message (#983)
* Change domain listing failure message * fix typecheck * remove unnessary comments
1 parent 59d9204 commit 43106fc

File tree

6 files changed

+148
-10
lines changed

6 files changed

+148
-10
lines changed
Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { MdWarning } from 'react-icons/md';
22

3-
import { type Props } from '../domains-page-error-banner/domains-page-error-banner.types';
3+
import { getDomainsErrorMessage } from '../domains-page-error-banner/helpers/get-domains-error-message';
44

55
const domainsPageErrorBannerConfig = {
66
icon: MdWarning,
7-
getErrorMessage: ({ failedClusters }: Props) =>
8-
`Failed to fetch domains for following clusters: ${failedClusters.join(', ')}`,
7+
getErrorMessage: getDomainsErrorMessage,
98
};
109

1110
export default domainsPageErrorBannerConfig;

src/views/domains-page/domains-page-error-banner/__tests__/domains-page-error-banner.test.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,26 @@ import { type Props } from '../domains-page-error-banner.types';
88
jest.mock('../../config/domains-page-error-banner.config', () => ({
99
icon: () => <div data-testid="mock-error-icon" />,
1010
getErrorMessage: ({ failedClusters }: Props) =>
11-
`Mock message for clusters failure: ${failedClusters.join(', ')}`,
11+
`Mock message for clusters failure: ${failedClusters
12+
.map(({ clusterName, httpStatus }) => `${clusterName} (${httpStatus})`)
13+
.join(', ')}`,
1214
}));
1315

1416
describe(DomainsPageErrorBanner.name, () => {
1517
it('should render if there are failed clusters', async () => {
1618
render(
17-
<DomainsPageErrorBanner failedClusters={['cluster_1', 'cluster_2']} />
19+
<DomainsPageErrorBanner
20+
failedClusters={[
21+
{ clusterName: 'cluster_1', httpStatus: 404 },
22+
{ clusterName: 'cluster_2', httpStatus: 503 },
23+
]}
24+
/>
1825
);
1926

2027
expect(await screen.findByTestId('mock-error-icon')).toBeInTheDocument();
2128
expect(
2229
await screen.findByText(
23-
'Mock message for clusters failure: cluster_1, cluster_2'
30+
'Mock message for clusters failure: cluster_1 (404), cluster_2 (503)'
2431
)
2532
).toBeInTheDocument();
2633
});
Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
11
export type Props = {
2-
failedClusters: Array<string>;
2+
failedClusters: DomainsListingFailedCluster[];
3+
};
4+
5+
export type DomainsListingFailedCluster = {
6+
clusterName: string;
7+
httpStatus: number;
38
};
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import { type DomainsListingFailedCluster } from '../../domains-page-error-banner.types';
2+
import { getDomainsErrorMessage } from '../get-domains-error-message';
3+
4+
describe('getDomainsErrorMessage', () => {
5+
it('should return empty string when failedClusters is empty', () => {
6+
const result = getDomainsErrorMessage({ failedClusters: [] });
7+
expect(result).toBe('');
8+
});
9+
10+
it('should return empty string when failedClusters is null', () => {
11+
const result = getDomainsErrorMessage({ failedClusters: null as any });
12+
expect(result).toBe('');
13+
});
14+
15+
it('should return empty string when failedClusters is undefined', () => {
16+
const result = getDomainsErrorMessage({ failedClusters: undefined as any });
17+
expect(result).toBe('');
18+
});
19+
20+
it('should return service unavailable message when all clusters have 503 status', () => {
21+
const failedClusters: DomainsListingFailedCluster[] = [
22+
{ clusterName: 'cluster-1', httpStatus: 503 },
23+
{ clusterName: 'cluster-2', httpStatus: 503 },
24+
{ clusterName: 'cluster-3', httpStatus: 503 },
25+
];
26+
27+
const result = getDomainsErrorMessage({ failedClusters });
28+
29+
expect(result).toBe(
30+
'Failed to connect to the following clusters: cluster-1, cluster-2, cluster-3'
31+
);
32+
});
33+
34+
it('should return API failure message when all clusters have non-503 status', () => {
35+
const failedClusters: DomainsListingFailedCluster[] = [
36+
{ clusterName: 'cluster-1', httpStatus: 500 },
37+
{ clusterName: 'cluster-2', httpStatus: 404 },
38+
{ clusterName: 'cluster-3', httpStatus: 403 },
39+
];
40+
41+
const result = getDomainsErrorMessage({ failedClusters });
42+
43+
expect(result).toBe(
44+
'Failed to fetch domains for following clusters: cluster-1, cluster-2, cluster-3'
45+
);
46+
});
47+
48+
it('should return detailed message with status codes when clusters have mixed status codes', () => {
49+
const failedClusters: DomainsListingFailedCluster[] = [
50+
{ clusterName: 'cluster-1', httpStatus: 503 },
51+
{ clusterName: 'cluster-2', httpStatus: 500 },
52+
{ clusterName: 'cluster-3', httpStatus: 404 },
53+
];
54+
55+
const result = getDomainsErrorMessage({ failedClusters });
56+
57+
expect(result).toBe(
58+
'Failed to fetch domains for following clusters: cluster-1 (503), cluster-2 (500), cluster-3 (404)'
59+
);
60+
});
61+
62+
it('should handle single cluster with 503 status', () => {
63+
const failedClusters: DomainsListingFailedCluster[] = [
64+
{ clusterName: 'single-cluster', httpStatus: 503 },
65+
];
66+
67+
const result = getDomainsErrorMessage({ failedClusters });
68+
69+
expect(result).toBe(
70+
'Failed to connect to the following clusters: single-cluster'
71+
);
72+
});
73+
74+
it('should handle single cluster with non-503 status', () => {
75+
const failedClusters: DomainsListingFailedCluster[] = [
76+
{ clusterName: 'single-cluster', httpStatus: 500 },
77+
];
78+
79+
const result = getDomainsErrorMessage({ failedClusters });
80+
81+
expect(result).toBe(
82+
'Failed to fetch domains for following clusters: single-cluster'
83+
);
84+
});
85+
});
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { type DomainsListingFailedCluster } from '../domains-page-error-banner.types';
2+
3+
export function getDomainsErrorMessage({
4+
failedClusters,
5+
}: {
6+
failedClusters: DomainsListingFailedCluster[];
7+
}): string {
8+
if (!failedClusters || failedClusters.length === 0) {
9+
return '';
10+
}
11+
12+
const clusterNames = failedClusters
13+
.map((cluster) => cluster.clusterName)
14+
.join(', ');
15+
16+
const allServiceUnavailable = failedClusters.every(
17+
(cluster) => cluster.httpStatus === 503
18+
);
19+
20+
if (allServiceUnavailable) {
21+
return `Failed to connect to the following clusters: ${clusterNames}`;
22+
}
23+
24+
const allApiFailures = failedClusters.every(
25+
(cluster) => cluster.httpStatus !== 503
26+
);
27+
28+
const clusterDetails = failedClusters
29+
.map((cluster) => `${cluster.clusterName} (${cluster.httpStatus})`)
30+
.join(', ');
31+
32+
return `Failed to fetch domains for following clusters: ${allApiFailures ? clusterNames : clusterDetails}`;
33+
}

src/views/domains-page/helpers/get-all-domains.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,18 @@ export const getAllDomains = async () => {
4747
domains: getUniqueDomains(
4848
results.flatMap((res) => (res.status === 'fulfilled' ? res.value : []))
4949
),
50-
failedClusters: CLUSTERS_CONFIGS.map((config) => config.clusterName).filter(
51-
(_, index) => results[index].status === 'rejected'
52-
),
50+
failedClusters: CLUSTERS_CONFIGS.map((config) => ({
51+
clusterName: config.clusterName,
52+
rejection: results.find((res) => res.status === 'rejected'),
53+
}))
54+
.filter((res) => res.rejection)
55+
.map((res) => ({
56+
clusterName: res.clusterName,
57+
httpStatus:
58+
res.rejection && 'reason' in res.rejection
59+
? res.rejection.reason.httpStatusCode
60+
: undefined,
61+
})),
5362
};
5463
};
5564

0 commit comments

Comments
 (0)