From 303047656e7a040c7f7d89bb11c2b69b9e0e1d99 Mon Sep 17 00:00:00 2001 From: Ali <83188384+testA113@users.noreply.github.com> Date: Fri, 27 Jun 2025 22:48:40 +1200 Subject: [PATCH] fix(k8s-services): avoid rerendering services table [r8s-387] (#832) --- .../queries/useClusterRoles.ts | 2 +- .../namespaces/queries/useNamespacesQuery.ts | 9 +- .../ServicesDatatable.test.tsx | 148 ++++++++++++++++++ .../ServicesDatatable/ServicesDatatable.tsx | 57 +++---- app/react/kubernetes/services/service.ts | 13 +- .../volumes/ListView/VolumesDatatable.tsx | 28 ++-- .../volumes/queries/useDeleteVolumes.ts | 2 +- .../volumes/queries/useNamespaceVolumes.ts | 51 ------ .../volumes/queries/useVolumesQuery.ts | 5 +- 9 files changed, 216 insertions(+), 99 deletions(-) create mode 100644 app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.test.tsx delete mode 100644 app/react/kubernetes/volumes/queries/useNamespaceVolumes.ts diff --git a/app/react/kubernetes/more-resources/ClusterRolesView/ClusterRolesDatatable/queries/useClusterRoles.ts b/app/react/kubernetes/more-resources/ClusterRolesView/ClusterRolesDatatable/queries/useClusterRoles.ts index dbacc4f4c..f27ffe642 100644 --- a/app/react/kubernetes/more-resources/ClusterRolesView/ClusterRolesDatatable/queries/useClusterRoles.ts +++ b/app/react/kubernetes/more-resources/ClusterRolesView/ClusterRolesDatatable/queries/useClusterRoles.ts @@ -21,7 +21,7 @@ export function useClusterRoles( }, { ...withGlobalError('Unable to get cluster roles'), - ...options, + refetchInterval: options?.autoRefreshRate, } ); } diff --git a/app/react/kubernetes/namespaces/queries/useNamespacesQuery.ts b/app/react/kubernetes/namespaces/queries/useNamespacesQuery.ts index 6e521e0aa..b4cd2d712 100644 --- a/app/react/kubernetes/namespaces/queries/useNamespacesQuery.ts +++ b/app/react/kubernetes/namespaces/queries/useNamespacesQuery.ts @@ -8,9 +8,13 @@ import { PortainerNamespace } from '../types'; import { queryKeys } from './queryKeys'; -export function useNamespacesQuery( +export function useNamespacesQuery( environmentId: EnvironmentId, - options?: { autoRefreshRate?: number; withResourceQuota?: boolean } + options?: { + autoRefreshRate?: number; + withResourceQuota?: boolean; + select?: (namespaces: PortainerNamespace[]) => T; + } ) { return useQuery( queryKeys.list(environmentId, { @@ -22,6 +26,7 @@ export function useNamespacesQuery( refetchInterval() { return options?.autoRefreshRate ?? false; }, + select: options?.select, } ); } diff --git a/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.test.tsx b/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.test.tsx new file mode 100644 index 000000000..0d4dfe7fd --- /dev/null +++ b/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.test.tsx @@ -0,0 +1,148 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { HttpResponse } from 'msw'; + +import { withTestRouter } from '@/react/test-utils/withRouter'; +import { withUserProvider } from '@/react/test-utils/withUserProvider'; +import { withTestQueryProvider } from '@/react/test-utils/withTestQuery'; +import { createMockUsers } from '@/react-tools/test-mocks'; +import { server, http } from '@/setup-tests/server'; + +import { ServicesDatatable } from './ServicesDatatable'; + +vi.mock('@/react/hooks/useEnvironmentId', () => ({ + useEnvironmentId: () => 1, +})); + +vi.mock('@/portainer/services/notifications', () => ({ + notifyError: vi.fn(), + notifySuccess: vi.fn(), +})); +function createMockServices(count: number) { + return Array.from({ length: count }, (_, i) => { + let namespace = 'default'; + if (i % 3 === 0) { + namespace = 'kube-system'; + } else if (i % 2 !== 0) { + namespace = 'my-namespace'; + } + + let type = 'ClusterIP'; + if (i % 4 === 1) { + type = 'NodePort'; + } else if (i % 4 === 2) { + type = 'LoadBalancer'; + } else if (i % 4 === 3) { + type = 'ExternalName'; + } + + return { + UID: `service-${i}`, + Name: `service-${i}`, + Namespace: namespace, + Type: type, + Ports: [{ Port: 80 + i, TargetPort: 8080 + i, Protocol: 'TCP' }], + Selector: { app: `app-${i}` }, + CreationTimestamp: new Date(Date.now() - i * 1000 * 60).toISOString(), + ApplicationOwner: '', + Applications: [{ Name: `app-${i}` }], + }; + }); +} + +const mockServices = createMockServices(4); + +const mockNamespaces = [ + { + Name: 'default', + IsSystem: false, + Status: 'Active', + CreationTimestamp: '2024-01-01T00:00:00Z', + }, + { + Name: 'kube-system', + IsSystem: true, + Status: 'Active', + CreationTimestamp: '2024-01-01T00:00:00Z', + }, + { + Name: 'my-namespace', + IsSystem: false, + Status: 'Active', + CreationTimestamp: '2024-01-01T00:00:00Z', + }, +]; + +beforeEach(() => { + server.use( + http.get('/api/kubernetes/1/services', () => + HttpResponse.json(mockServices) + ), + http.get('/api/kubernetes/1/namespaces', () => + HttpResponse.json(mockNamespaces) + ) + ); +}); +const mockUser = { + ...createMockUsers(1)[0], + PortainerAuthorizations: { + K8sAccessSystemNamespaces: true, + K8sServiceW: true, + }, +}; + +function createTestComponent() { + return withTestRouter( + withUserProvider(withTestQueryProvider(ServicesDatatable), mockUser), + { + route: '/kubernetes/services', + stateConfig: [ + { + name: 'kubernetes.services', + url: '/kubernetes/services', + params: { endpointId: '1' }, + }, + ], + } + ); +} + +describe('ServicesDatatable', () => { + it('renders services data correctly', async () => { + const TestComponent = createTestComponent(); + render(); + + expect(await screen.findByText('service-1')).toBeInTheDocument(); + expect(screen.getByText('service-2')).toBeInTheDocument(); + }); + + it('should filter system resources correctly when toggled', async () => { + const TestComponent = createTestComponent(); + render(); + + const settingsButton = screen.getByRole('button', { name: /settings/i }); + await userEvent.click(settingsButton); + + await waitFor(() => { + expect(screen.queryByText('service-0')).not.toBeInTheDocument(); + }); + + const systemToggle = await screen.findByTestId('show-system-resources'); + await userEvent.click(systemToggle); + + await waitFor(() => { + expect(screen.queryByText('service-0')).toBeInTheDocument(); + }); + + expect(screen.getByText('service-3')).toBeInTheDocument(); + expect(screen.getByText('service-1')).toBeInTheDocument(); + expect(screen.getByText('service-2')).toBeInTheDocument(); + }); + + it('should show loading state when data is loading', async () => { + const TestComponent = createTestComponent(); + render(); + + expect(screen.getByText('Loading...')).toBeInTheDocument(); + }); +}); diff --git a/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.tsx b/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.tsx index 4786eaf2f..265808564 100644 --- a/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.tsx +++ b/app/react/kubernetes/services/ServicesView/ServicesDatatable/ServicesDatatable.tsx @@ -16,6 +16,7 @@ import { DefaultDatatableSettings } from '@/react/kubernetes/datatables/DefaultD import { SystemResourceDescription } from '@/react/kubernetes/datatables/SystemResourceDescription'; import { useNamespacesQuery } from '@/react/kubernetes/namespaces/queries/useNamespacesQuery'; import { CreateFromManifestButton } from '@/react/kubernetes/components/CreateFromManifestButton'; +import { createStore } from '@/react/kubernetes/datatables/default-kube-datatable-store'; import { Datatable, Table, TableSettingsMenu } from '@@/datatables'; import { useTableState } from '@@/datatables/useTableState'; @@ -25,7 +26,6 @@ import { useMutationDeleteServices, useClusterServices } from '../../service'; import { Service } from '../../types'; import { columns } from './columns'; -import { createStore } from './datatable-store'; import { ServiceRowData } from './types'; const storageKey = 'k8sServicesDatatable'; @@ -34,52 +34,53 @@ const settingsStore = createStore(storageKey); export function ServicesDatatable() { const tableState = useTableState(settingsStore, storageKey); const environmentId = useEnvironmentId(); - const { data: namespacesArray, ...namespacesQuery } = - useNamespacesQuery(environmentId); + const { data: namespaces, ...namespacesQuery } = useNamespacesQuery( + environmentId, + { + select: (namespacesArray) => + namespacesArray?.reduce>( + (acc, namespace) => { + acc[namespace.Name] = namespace; + return acc; + }, + {} + ), + } + ); + const { authorized: canWrite } = useAuthorizations(['K8sServiceW']); + const { authorized: canAccessSystemResources } = useAuthorizations( + 'K8sAccessSystemNamespaces' + ); const { data: services, ...servicesQuery } = useClusterServices( environmentId, { autoRefreshRate: tableState.autoRefreshRate * 1000, withApplications: true, + select: (services) => + services?.filter( + (service) => + (canAccessSystemResources && tableState.showSystemResources) || + !namespaces?.[service.Namespace]?.IsSystem + ), } ); - const namespaces: Record = {}; - if (Array.isArray(namespacesArray)) { - for (let i = 0; i < namespacesArray.length; i++) { - const namespace = namespacesArray[i]; - namespaces[namespace.Name] = namespace; - } - } - - const { authorized: canWrite } = useAuthorizations(['K8sServiceW']); - const readOnly = !canWrite; - const { authorized: canAccessSystemResources } = useAuthorizations( - 'K8sAccessSystemNamespaces' - ); - const filteredServices = services?.filter( - (service) => - (canAccessSystemResources && tableState.showSystemResources) || - !namespaces?.[service.Namespace]?.IsSystem - ); - - const servicesWithIsSystem = useServicesRowData( - filteredServices || [], - namespaces - ); + const servicesWithIsSystem = useServicesRowData(services || [], namespaces); return ( row.UID} isRowSelectable={(row) => !namespaces?.[row.original.Namespace]?.IsSystem} - disableSelect={readOnly} + disableSelect={!canWrite} renderTableActions={(selectedRows) => ( )} diff --git a/app/react/kubernetes/services/service.ts b/app/react/kubernetes/services/service.ts index a36f25db9..a7cdbc7f2 100644 --- a/app/react/kubernetes/services/service.ts +++ b/app/react/kubernetes/services/service.ts @@ -23,18 +23,21 @@ export const queryKeys = { * * @returns The result of the query. */ -export function useClusterServices( +export function useClusterServices( environmentId: EnvironmentId, - options?: { autoRefreshRate?: number; withApplications?: boolean } + options?: { + autoRefreshRate?: number; + withApplications?: boolean; + select?: (services: Service[]) => T; + } ) { return useQuery( queryKeys.clusterServices(environmentId), async () => getClusterServices(environmentId, options?.withApplications), { ...withGlobalError('Unable to get services.'), - refetchInterval() { - return options?.autoRefreshRate ?? false; - }, + refetchInterval: options?.autoRefreshRate, + select: options?.select, } ); } diff --git a/app/react/kubernetes/volumes/ListView/VolumesDatatable.tsx b/app/react/kubernetes/volumes/ListView/VolumesDatatable.tsx index b81f60e9a..032fe63aa 100644 --- a/app/react/kubernetes/volumes/ListView/VolumesDatatable.tsx +++ b/app/react/kubernetes/volumes/ListView/VolumesDatatable.tsx @@ -16,12 +16,17 @@ import { } from '../../datatables/DefaultDatatableSettings'; import { SystemResourceDescription } from '../../datatables/SystemResourceDescription'; import { useNamespacesQuery } from '../../namespaces/queries/useNamespacesQuery'; -import { useAllVolumesQuery } from '../queries/useVolumesQuery'; +import { + convertToVolumeViewModels, + useAllVolumesQuery, +} from '../queries/useVolumesQuery'; import { isSystemNamespace } from '../../namespaces/queries/useIsSystemNamespace'; import { useDeleteVolumes } from '../queries/useDeleteVolumes'; import { isVolumeUsed } from '../utils'; +import { K8sVolumeInfo } from '../types'; import { columns } from './columns'; +import { VolumeViewModel } from './types'; export function VolumesDatatable() { const tableState = useTableStateWithStorage( @@ -45,21 +50,15 @@ export function VolumesDatatable() { const namespaces = namespaceListQuery.data ?? []; const volumesQuery = useAllVolumesQuery(envId, { refetchInterval: tableState.autoRefreshRate * 1000, + select: transformAndFilterVolumes, }); const volumes = volumesQuery.data ?? []; - const filteredVolumes = tableState.showSystemResources - ? volumes - : volumes.filter( - (volume) => - !isSystemNamespace(volume.ResourcePool.Namespace.Name, namespaces) - ); - return ( ); + + function transformAndFilterVolumes( + volumes: K8sVolumeInfo[] + ): VolumeViewModel[] { + const transformedVolumes = convertToVolumeViewModels(volumes); + return transformedVolumes.filter( + (volume) => + tableState.showSystemResources || + !isSystemNamespace(volume.ResourcePool.Namespace.Name, namespaces) + ); + } } diff --git a/app/react/kubernetes/volumes/queries/useDeleteVolumes.ts b/app/react/kubernetes/volumes/queries/useDeleteVolumes.ts index 78d5a295d..7732c2b58 100644 --- a/app/react/kubernetes/volumes/queries/useDeleteVolumes.ts +++ b/app/react/kubernetes/volumes/queries/useDeleteVolumes.ts @@ -36,7 +36,7 @@ export function useDeleteVolumes(environmentId: EnvironmentId) { ); } queryClient.invalidateQueries(queryKeys.storages(environmentId)); - queryClient.invalidateQueries(queryKeys.volumes(environmentId)); + return queryClient.invalidateQueries(queryKeys.volumes(environmentId)); }, ...withGlobalError('Unable to remove volumes'), }); diff --git a/app/react/kubernetes/volumes/queries/useNamespaceVolumes.ts b/app/react/kubernetes/volumes/queries/useNamespaceVolumes.ts deleted file mode 100644 index cdf6b6ee0..000000000 --- a/app/react/kubernetes/volumes/queries/useNamespaceVolumes.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { useQuery } from '@tanstack/react-query'; - -import { EnvironmentId } from '@/react/portainer/environments/types'; -import { withGlobalError } from '@/react-tools/react-query'; -import axios, { parseAxiosError } from '@/portainer/services/axios'; - -import { K8sVolumeInfo } from '../types'; - -import { queryKeys } from './query-keys'; -import { convertToVolumeViewModels } from './useVolumesQuery'; - -// useQuery to get a list of all volumes in a cluster -export function useNamespaceVolumes( - environmentId: EnvironmentId, - namespace: string, - queryOptions?: { - refetchInterval?: number; - withApplications?: boolean; - } -) { - return useQuery( - queryKeys.volumes(environmentId), - () => - getNamespaceVolumes(environmentId, namespace, { - withApplications: queryOptions?.withApplications ?? false, - }), - { - enabled: !!namespace, - refetchInterval: queryOptions?.refetchInterval, - select: convertToVolumeViewModels, - ...withGlobalError('Unable to retrieve volumes'), - } - ); -} - -// get all volumes in a cluster -async function getNamespaceVolumes( - environmentId: EnvironmentId, - namespace: string, - params?: { withApplications: boolean } -) { - try { - const { data } = await axios.get( - `/kubernetes/${environmentId}/namespaces/${namespace}/volumes`, - { params } - ); - return data; - } catch (e) { - throw parseAxiosError(e, 'Unable to retrieve volumes'); - } -} diff --git a/app/react/kubernetes/volumes/queries/useVolumesQuery.ts b/app/react/kubernetes/volumes/queries/useVolumesQuery.ts index 94b30345b..646065758 100644 --- a/app/react/kubernetes/volumes/queries/useVolumesQuery.ts +++ b/app/react/kubernetes/volumes/queries/useVolumesQuery.ts @@ -14,10 +14,11 @@ import { appOwnerLabel } from '../../applications/constants'; import { queryKeys } from './query-keys'; // useQuery to get a list of all volumes in a cluster -export function useAllVolumesQuery( +export function useAllVolumesQuery( environmentId: EnvironmentId, queryOptions?: { refetchInterval?: number; + select?: (volumes: K8sVolumeInfo[]) => T[]; } ) { return useQuery( @@ -25,7 +26,7 @@ export function useAllVolumesQuery( () => getAllVolumes(environmentId, { withApplications: true }), { refetchInterval: queryOptions?.refetchInterval, - select: convertToVolumeViewModels, + select: queryOptions?.select, ...withGlobalError('Unable to retrieve volumes'), } );