Skip to content

Commit 1f30ed3

Browse files
address review
1 parent 6c757cf commit 1f30ed3

4 files changed

Lines changed: 39 additions & 12 deletions

File tree

assets/src/components/kubernetes/Cluster.tsx

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
getDefaultKubernetesClusterId,
4040
isKubernetesClusterMissing,
4141
LAST_SELECTED_CLUSTER_KEY,
42+
selectMatchingCluster,
4243
} from './clusterSelection'
4344
import { DataSelectProvider } from './common/DataSelect'
4445
import { getNamespaceListLoadError } from './common/namespaceList'
@@ -138,13 +139,11 @@ export default function Cluster({
138139
() => mapExistingNodes(queryData?.clusters),
139140
[queryData?.clusters]
140141
)
141-
const currentCluster = [data?.cluster, queryData?.cluster].find(
142-
(candidate) => candidate?.id === clusterId
143-
)
144-
const cluster = currentCluster ?? clusters.find(({ id }) => id === clusterId)
145-
// Don't unmount the dashboard while the new cluster(id:) result is in flight.
146-
const clusterForContext =
147-
cluster ?? (loading ? queryData?.cluster : undefined)
142+
const cluster = selectMatchingCluster(clusterId, [
143+
data?.cluster,
144+
queryData?.cluster,
145+
...clusters,
146+
])
148147

149148
const clusterMissing = isKubernetesClusterMissing({
150149
clusterId,
@@ -206,10 +205,10 @@ export default function Cluster({
206205
({
207206
clusters,
208207
refetch,
209-
cluster: clusterForContext,
208+
cluster,
210209
namespaces,
211210
}) as ClusterContextT,
212-
[clusters, refetch, clusterForContext, namespaces]
211+
[clusters, refetch, cluster, namespaces]
213212
)
214213

215214
const defaultClusterId = useMemo(

assets/src/components/kubernetes/clusterSelection.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { replaceKubernetesClusterId } from '../../routes/kubernetesRoutesConsts'
33
import {
44
getDefaultKubernetesClusterId,
55
isKubernetesClusterMissing,
6+
selectMatchingCluster,
67
withCurrentCluster,
78
} from './clusterSelection'
89

@@ -52,6 +53,18 @@ describe('withCurrentCluster', () => {
5253
})
5354
})
5455

56+
describe('selectMatchingCluster', () => {
57+
it('never returns a cluster that does not match the route id', () => {
58+
expect(
59+
selectMatchingCluster('b', [{ id: 'a' }, undefined, { id: 'c' }])
60+
).toBeUndefined()
61+
})
62+
63+
it('returns the matching cluster even when it is not first', () => {
64+
expect(selectMatchingCluster('b', [{ id: 'a' }, { id: 'b' }])?.id).toBe('b')
65+
})
66+
})
67+
5568
describe('getDefaultKubernetesClusterId', () => {
5669
const clusters = [
5770
{ id: 'worker', self: false },

assets/src/components/kubernetes/clusterSelection.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ export function withCurrentCluster<T extends { id: string }>(
1010
return [current, ...clusters]
1111
}
1212

13+
export function selectMatchingCluster<T extends { id: string }>(
14+
clusterId: string | undefined,
15+
candidates: Array<T | null | undefined>
16+
): T | undefined {
17+
if (!clusterId) return undefined
18+
19+
return (
20+
candidates.find((candidate) => candidate?.id === clusterId) ?? undefined
21+
)
22+
}
23+
1324
export function getDefaultKubernetesClusterId<
1425
T extends { id: string; self?: boolean | null },
1526
>(clusters: T[], lastSelectedClusterId: string | null): string | undefined {

assets/src/components/kubernetes/common/ResourceList.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
useEffect,
1515
useMemo,
1616
} from 'react'
17-
import { useNavigate } from 'react-router-dom'
17+
import { useNavigate, useParams } from 'react-router-dom'
1818
import { AxiosInstance } from '../../../helpers/axios.ts'
1919

2020
import {
@@ -88,15 +88,18 @@ export function ResourceList<
8888
setRefetch,
8989
}: ResourceListProps<TResourceList>): ReactElement<any> {
9090
const navigate = useNavigate()
91+
const { clusterId: clusterIdParam } = useParams()
9192
const cluster = useCluster()
93+
// Route param is the source of truth while context cluster is still resolving.
94+
const requestClusterId = clusterIdParam || cluster?.id || ''
9295
const { filter, namespace, setNamespaced } = useDataSelect()
9396
const { sortBy, reactTableOptions } = useSortedTableOptions(initialSort, {
9497
...tableOptions,
9598
meta: { cluster, ...tableOptions?.meta },
9699
})
97100

98101
const options = queryOptions({
99-
client: AxiosInstance(cluster?.id ?? ''),
102+
client: AxiosInstance(requestClusterId),
100103
path: { ...(namespaced ? { namespace } : undefined), ...pathParams },
101104
query: {
102105
filterBy: `name,${filter}`,
@@ -109,7 +112,8 @@ export function ResourceList<
109112
const { data, isLoading, isFetching, hasNextPage, fetchNextPage, refetch } =
110113
useInfiniteQuery<TResourceList>({
111114
...options,
112-
queryKey: [...options.queryKey, 'clusterId ', cluster?.id], // Add clusterId to queryKey to refetch when cluster changes
115+
queryKey: [...options.queryKey, 'clusterId ', requestClusterId], // Add clusterId to queryKey to refetch when cluster changes
116+
enabled: !!requestClusterId,
113117
initialPageParam: DEFAULT_DATA_SELECT.page,
114118
getNextPageParam: (lastPage, allPages) => {
115119
const pages = allPages.length

0 commit comments

Comments
 (0)