Merge pull request #429 from bartoval/fix_process_group_selection

fix(Topology): 🐞 Wrong group selection with show only neighbours
skupperproject · Jun 14, 2024 · 92eafa5 · 92eafa5
2 parents 1415316 + 8763835
commit 92eafa5
Show file tree

Hide file tree

Showing 18 changed files with 152 additions and 111 deletions.
diff --git a/mocks/data/PROCESSES.json b/mocks/data/PROCESSES.json
@@ -61,7 +61,7 @@
       "parentName": "site 1",
       "imageName": "process op:v0.3.0",
       "groupName": "op",
-      "groupIdentity": "group-1",
+      "groupIdentity": "group-op",
       "hostName": "10.242.0.8",
       "sourceHost": "172.17.63.130",
       "processBinding": "bound",

diff --git a/src/pages/Topology/__tests__/TopologyComponents.spec.tsx b/src/pages/Topology/__tests__/TopologyComponents.spec.tsx
@@ -53,7 +53,7 @@ describe('Topology Components', () => {
       <Wrapper>
         <Suspense fallback={<LoadingPage />}>
           <TopologyComponent
-            id={TopologyController.transformStringIdsToIds(componentResult[2].name)}
+            ids={TopologyController.transformStringIdsToIds(componentResult[2].name)}
             GraphComponent={MockGraphComponent}
           />
         </Suspense>
@@ -86,7 +86,7 @@ describe('Topology Components', () => {
     const handleSelected = jest.fn();
 
     jest.spyOn(useTopologySiteState, 'default').mockImplementation(() => ({
-      idSelected: TopologyController.transformStringIdsToIds(componentResult[2].identity),
+      idsSelected: TopologyController.transformStringIdsToIds(componentResult[2].identity),
       showOnlyNeighbours: false,
       moveToNodeSelected: false,
       displayOptionsSelected: [],
@@ -121,7 +121,7 @@ describe('Topology Components', () => {
     const handleShowOnlyNeighbours = jest.fn();
 
     jest.spyOn(useTopologySiteState, 'default').mockImplementation(() => ({
-      idSelected: TopologyController.transformStringIdsToIds(componentResult[2].identity),
+      idsSelected: TopologyController.transformStringIdsToIds(componentResult[2].identity),
       showOnlyNeighbours: false,
       moveToNodeSelected: false,
       displayOptionsSelected: [],

diff --git a/src/pages/Topology/__tests__/TopologyProcesses.spec.tsx b/src/pages/Topology/__tests__/TopologyProcesses.spec.tsx
@@ -37,7 +37,7 @@ const mockHandleMoveToNodeSelectedChecked = jest.fn();
 const MockGetDisplaySelectedFromLocalStorage = jest.fn();
 
 const mockUseTopologyStateResults = {
-  idSelected: TopologyController.transformStringIdsToIds(processesResults[0].identity),
+  idsSelected: TopologyController.transformStringIdsToIds(processesResults[0].identity),
   showOnlyNeighbours: false,
   moveToNodeSelected: false,
   displayOptionsSelected: [],
@@ -111,7 +111,7 @@ describe('Topology Process', () => {
       <Wrapper>
         <Suspense fallback={<LoadingPage />}>
           <TopologyProcesses
-            id={TopologyController.transformStringIdsToIds(processesResults[2].name)}
+            ids={TopologyController.transformStringIdsToIds(processesResults[2].name)}
             GraphComponent={MockGraphComponent}
             ModalComponent={MockTopologyModalComponent}
             serviceIds={serviceResults.map(({ identity }) => identity)}

diff --git a/src/pages/Topology/__tests__/TopologySite.spec.tsx b/src/pages/Topology/__tests__/TopologySite.spec.tsx
@@ -52,7 +52,7 @@ describe('TopologySite', () => {
       <Wrapper>
         <Suspense fallback={<LoadingPage />}>
           <TopologySite
-            id={TopologyController.transformStringIdsToIds(sitesResults[2].identity)}
+            ids={TopologyController.transformStringIdsToIds(sitesResults[2].identity)}
             GraphComponent={MockGraphComponent}
           />
         </Suspense>
@@ -103,7 +103,7 @@ describe('TopologySite', () => {
     const handleSelected = jest.fn();
 
     jest.spyOn(useTopologySiteState, 'default').mockImplementation(() => ({
-      idSelected: TopologyController.transformStringIdsToIds(sitesResults[2].identity),
+      idsSelected: TopologyController.transformStringIdsToIds(sitesResults[2].identity),
       showOnlyNeighbours: false,
       moveToNodeSelected: false,
       displayOptionsSelected: [],
@@ -128,7 +128,7 @@ describe('TopologySite', () => {
     const handleShowOnlyNeighbours = jest.fn();
 
     jest.spyOn(useTopologySiteState, 'default').mockImplementation(() => ({
-      idSelected: TopologyController.transformStringIdsToIds(sitesResults[2].identity),
+      idsSelected: TopologyController.transformStringIdsToIds(sitesResults[2].identity),
       showOnlyNeighbours: false,
       moveToNodeSelected: false,
       displayOptionsSelected: [],
@@ -155,7 +155,7 @@ describe('TopologySite', () => {
     const handleMoveToNodeSelectedChecked = jest.fn();
 
     jest.spyOn(useTopologySiteState, 'default').mockImplementation(() => ({
-      idSelected: TopologyController.transformStringIdsToIds(sitesResults[2].identity),
+      idsSelected: TopologyController.transformStringIdsToIds(sitesResults[2].identity),
       showOnlyNeighbours: false,
       moveToNodeSelected: false,
       displayOptionsSelected: [],

diff --git a/src/pages/Topology/__tests__/useTopologyProcessData.spec.ts b/src/pages/Topology/__tests__/useTopologyProcessData.spec.ts
@@ -28,7 +28,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologyProcessData({
-        idSelected: ['process1'],
+        idsSelected: ['process1'],
         showBytes: true,
         showByteRate: true,
         showLatency: true
@@ -55,7 +55,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologyProcessData({
-        idSelected: [],
+        idsSelected: [],
         showBytes: true,
         showByteRate: true,
         showLatency: true
@@ -82,7 +82,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologyProcessData({
-        idSelected: ['process3'],
+        idsSelected: ['process3'],
         showBytes: true,
         showByteRate: true,
         showLatency: true

diff --git a/src/pages/Topology/__tests__/useTopologySiteData.spec.ts b/src/pages/Topology/__tests__/useTopologySiteData.spec.ts
@@ -26,7 +26,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologySiteData({
-        idSelected: ['site1'],
+        idsSelected: ['site1'],
         showDataLink: true,
         showBytes: true,
         showByteRate: true,
@@ -59,7 +59,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologySiteData({
-        idSelected: ['site1'],
+        idsSelected: ['site1'],
         showDataLink: true,
         showBytes: true,
         showByteRate: true,
@@ -152,7 +152,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologySiteData({
-        idSelected: [],
+        idsSelected: [],
         showDataLink: true,
         showBytes: true,
         showByteRate: true,
@@ -182,7 +182,7 @@ describe('useTopologySiteData', () => {
 
     const { result } = renderHook(() =>
       useTopologySiteData({
-        idSelected: ['site3'],
+        idsSelected: ['site3'],
         showDataLink: true,
         showBytes: true,
         showByteRate: true,

diff --git a/src/pages/Topology/__tests__/useTopologySiteState.spec.tsx b/src/pages/Topology/__tests__/useTopologySiteState.spec.tsx
@@ -10,21 +10,21 @@ describe('useTopologySiteState', () => {
   });
 
   it('should initialize with empty display options', () => {
-    const { result } = renderHook(() => useTopologyState({ id: undefined }));
+    const { result } = renderHook(() => useTopologyState({ ids: undefined }));
 
     expect(result.current.displayOptionsSelected).toEqual([]);
   });
 
   it('should initialize with default display options', () => {
     const { result } = renderHook(() =>
-      useTopologyState({ id: undefined, initDisplayOptionsEnabled: [SHOW_ROUTER_LINKS, SHOW_SITE_KEY] })
+      useTopologyState({ ids: undefined, initDisplayOptionsEnabled: [SHOW_ROUTER_LINKS, SHOW_SITE_KEY] })
     );
 
     expect(result.current.displayOptionsSelected).toEqual([SHOW_ROUTER_LINKS, SHOW_SITE_KEY]);
   });
 
   it('should handle display options selection', () => {
-    const { result } = renderHook(() => useTopologyState({ id: undefined, displayOptionsEnabledKey: 'test-key' }));
+    const { result } = renderHook(() => useTopologyState({ ids: undefined, displayOptionsEnabledKey: 'test-key' }));
     const newDisplayOptions = ['new_option'];
 
     act(() => {
@@ -37,23 +37,23 @@ describe('useTopologySiteState', () => {
 
   it('sshould initialize the component with the correct selected id', () => {
     const { result } = renderHook(() =>
-      useTopologyState({ id: TopologyController.transformStringIdsToIds('site_id') })
+      useTopologyState({ ids: TopologyController.transformStringIdsToIds('site_id') })
     );
-    expect(result.current.idSelected).toEqual(['site_id']);
+    expect(result.current.idsSelected).toEqual(['site_id']);
   });
 
   it('should update the selected ID when a site is selected', () => {
-    const { result } = renderHook(() => useTopologyState({ id: undefined }));
+    const { result } = renderHook(() => useTopologyState({ ids: undefined }));
 
     act(() => {
       result.current.handleSelected(TopologyController.transformStringIdsToIds('site_id'));
     });
 
-    expect(result.current.idSelected).toEqual(['site_id']);
+    expect(result.current.idsSelected).toEqual(['site_id']);
   });
 
   it('should handle show only neighbours', () => {
-    const { result } = renderHook(() => useTopologyState({ id: undefined }));
+    const { result } = renderHook(() => useTopologyState({ ids: undefined }));
 
     act(() => {
       result.current.handleShowOnlyNeighbours(true);
@@ -63,7 +63,7 @@ describe('useTopologySiteState', () => {
   });
 
   it('should handle move to node selection', () => {
-    const { result } = renderHook(() => useTopologyState({ id: undefined }));
+    const { result } = renderHook(() => useTopologyState({ ids: undefined }));
 
     act(() => {
       result.current.handleMoveToNodeSelectedChecked(true);

diff --git a/src/pages/Topology/components/DisplayResources.tsx b/src/pages/Topology/components/DisplayResources.tsx
@@ -1,4 +1,4 @@
-import { FC, FormEvent, Ref, useCallback, useRef, useState } from 'react';
+import { FC, FormEvent, Ref, useCallback, useEffect, useRef, useState } from 'react';
 
 import {
   Button,
@@ -35,39 +35,29 @@ const DisplayResources: FC<DisplayResourcesProps> = function ({
   onSelect,
   options
 }) {
-  const [inputValue, setInputValue] = useState<string>(findOptionSelected(options, id)?.name || '');
+  const [inputValue, setInputValue] = useState<string>('');
   const [isOpen, setIsOpen] = useState(false);
 
   const filterValueRef = useRef<string>('');
   const textInputRef = useRef<HTMLInputElement>();
 
   function handleToggleMenu() {
-    setIsOpen(!isOpen);
+    setIsOpen((prevIsOpen) => !prevIsOpen);
   }
 
-  function handleClear() {
+  const handleClear = useCallback(() => {
     setInputValue('');
     filterValueRef.current = '';
     textInputRef.current?.focus();
-
-    if (onSelect) {
-      onSelect(undefined);
-    }
-  }
+    onSelect?.(undefined);
+  }, [onSelect]);
 
   const handleSelect = useCallback(
     (idSelected: string) => {
-      const nameSelected = findOptionSelected(options, idSelected)?.name || '';
-
       setIsOpen(false);
-      setInputValue(nameSelected);
-      filterValueRef.current = '';
-
-      if (onSelect) {
-        onSelect(idSelected);
-      }
+      onSelect?.(idSelected);
     },
-    [onSelect, options]
+    [onSelect]
   );
 
   const handleTextInputChange = (_: FormEvent<HTMLInputElement>, selection: string) => {
@@ -80,6 +70,17 @@ const DisplayResources: FC<DisplayResourcesProps> = function ({
       !filterValueRef.current || name.toString().toLowerCase().includes(filterValueRef.current.toLowerCase())
   );
 
+  useEffect(() => {
+    if (id) {
+      setInputValue(findOptionNameSelected(options, id));
+      filterValueRef.current = '';
+    }
+  }, [id, options]);
+
+  useEffect(() => {
+    setIsOpen(!!inputValue && !!filterValueRef.current);
+  }, [inputValue]);
+
   return (
     <Select
       role="resource-select"
@@ -130,6 +131,6 @@ const DisplayResources: FC<DisplayResourcesProps> = function ({
 
 export default DisplayResources;
 
-function findOptionSelected(options: ResourcesOptionsProps[], idSelected?: string) {
-  return options.find(({ identity }) => identity === idSelected);
+function findOptionNameSelected(options: ResourcesOptionsProps[], idSelected?: string) {
+  return options.find(({ identity }) => identity === idSelected)?.name || '';
 }
diff --git a/src/pages/Topology/components/TopologyComponent.tsx b/src/pages/Topology/components/TopologyComponent.tsx
@@ -16,25 +16,25 @@ import { TopologyController } from '../services';
 import { TopologyComponentController } from '../services/topologyComponentController';
 import { TopologyLabels } from '../Topology.enum';
 
-const TopologyComponent: FC<{ id?: string[]; GraphComponent?: ComponentType<GraphReactAdaptorProps> }> = function ({
-  id,
+const TopologyComponent: FC<{ ids?: string[]; GraphComponent?: ComponentType<GraphReactAdaptorProps> }> = function ({
+  ids,
   GraphComponent = GraphReactAdaptor
 }) {
   const navigate = useNavigate();
   const graphRef = useRef<GraphReactAdaptorExposedMethods>();
   const toastRef = useRef<ToastExposeMethods>(null);
 
   const {
-    idSelected,
+    idsSelected,
     showOnlyNeighbours,
     moveToNodeSelected,
     handleSelected,
     handleShowOnlyNeighbours,
     handleMoveToNodeSelectedChecked
-  } = useTopologyState({ id });
+  } = useTopologyState({ ids });
 
   const { components, componentsPairs } = useTopologyComponentData({
-    idSelected: showOnlyNeighbours ? idSelected : undefined
+    idSelected: showOnlyNeighbours ? idsSelected : undefined
   });
 
   const handleShowDetails = useCallback(
@@ -69,7 +69,7 @@ const TopologyComponent: FC<{ id?: string[]; GraphComponent?: ComponentType<Grap
   }, [graphRef, toastRef]);
 
   const { nodeIdSelected, nodes, edges } = TopologyComponentController.dataTransformer({
-    idSelected,
+    idsSelected,
     components,
     componentsPairs
   });
@@ -99,8 +99,8 @@ const TopologyComponent: FC<{ id?: string[]; GraphComponent?: ComponentType<Grap
             edges={edges}
             itemSelected={nodeIdSelected}
             onClickNode={handleShowDetails}
-            layout={showOnlyNeighbours && idSelected ? LAYOUT_TOPOLOGY_SINGLE_NODE : LAYOUT_TOPOLOGY_DEFAULT}
-            moveToSelectedNode={moveToNodeSelected && !!idSelected && !showOnlyNeighbours}
+            layout={showOnlyNeighbours && idsSelected ? LAYOUT_TOPOLOGY_SINGLE_NODE : LAYOUT_TOPOLOGY_DEFAULT}
+            moveToSelectedNode={moveToNodeSelected && !!idsSelected && !showOnlyNeighbours}
           />
         </StackItem>
       </Stack>