refactor(Topology): ♻️ Make testable displayOption

skupperproject · Sep 18, 2024 · 47f57b4 · 47f57b4
1 parent c10f1c5
commit 47f57b4
Show file tree

Hide file tree

Showing 5 changed files with 173 additions and 119 deletions.
diff --git a/__tests__/pages/Topology/DisplayOptions.spec.tsx b/__tests__/pages/Topology/DisplayOptions.spec.tsx
@@ -1,7 +1,9 @@
 import { renderHook, render, act } from '@testing-library/react';
 import eventUser from '@testing-library/user-event';
 
-import DisplayOptions, { useDisplayOptions } from '../../../src/pages/Topology/components/DisplayOptions';
+import { useDisplayOptionsState } from '@pages/Topology/hooks/useDisplayOptionsState';
+
+import DisplayOptions from '../../../src/pages/Topology/components/DisplayOptions';
 import { SHOW_DATA_LINKS, SHOW_ROUTER_LINKS } from '../../../src/pages/Topology/Topology.constants';
 import { TopologyLabels } from '../../../src/pages/Topology/Topology.enum';
 
@@ -21,13 +23,13 @@ const options = {
 describe('useDisplayOptions', () => {
   it('initializes with defaultSelected values', () => {
     const { result } = renderHook(() =>
-      useDisplayOptions({ defaultSelected: ['option1', 'option2'], onSelected: mockOnSelected })
+      useDisplayOptionsState({ defaultSelected: ['option1', 'option2'], onSelected: mockOnSelected })
     );
     expect(result.current.displayOptionsSelected).toEqual(['option1', 'option2']);
   });
 
   it('adds and removes options correctly', () => {
-    const { result } = renderHook(() => useDisplayOptions({ defaultSelected: [], onSelected: mockOnSelected }));
+    const { result } = renderHook(() => useDisplayOptionsState({ defaultSelected: [], onSelected: mockOnSelected }));
     const { selectDisplayOptions } = result.current;
 
     // Add an option
@@ -41,7 +43,7 @@ describe('useDisplayOptions', () => {
 
   it('toggles between SHOW_DATA_LINKS and SHOW_ROUTER_LINKS correctly', () => {
     const { result } = renderHook(() =>
-      useDisplayOptions({ defaultSelected: [SHOW_DATA_LINKS], onSelected: mockOnSelected })
+      useDisplayOptionsState({ defaultSelected: [SHOW_DATA_LINKS], onSelected: mockOnSelected })
     );
 
     // Initially, SHOW_DATA_LINKS is selected
@@ -56,7 +58,7 @@ describe('useDisplayOptions', () => {
 
   it('clicks on SHOW_DATA_LINKS already checked and SHOW_ROUTER_LINKS correctly', () => {
     const { result } = renderHook(() =>
-      useDisplayOptions({ defaultSelected: [SHOW_DATA_LINKS], onSelected: mockOnSelected })
+      useDisplayOptionsState({ defaultSelected: [SHOW_DATA_LINKS], onSelected: mockOnSelected })
     );
 
     // Initially, SHOW_DATA_LINKS is selected
@@ -69,7 +71,7 @@ describe('useDisplayOptions', () => {
   });
 
   it('calls onSelect callback with updated options and selected option', () => {
-    const { result } = renderHook(() => useDisplayOptions({ defaultSelected: [], onSelected: mockOnSelected }));
+    const { result } = renderHook(() => useDisplayOptionsState({ defaultSelected: [], onSelected: mockOnSelected }));
     const { selectDisplayOptions } = result.current;
 
     act(() => selectDisplayOptions('option1'));

diff --git a/mocks/server.ts b/mocks/server.ts
@@ -121,6 +121,8 @@ mockSitePairsForPerf.forEach((_, index) => {
       routerId: '',
       sourceSiteId: site1.identity,
       destinationSiteId: site2.identity,
+      sourceSiteName: site1.sourceName,
+      destinationSiteName: site2.destinationName,
       status: 'up'
     },
     {
@@ -136,6 +138,8 @@ mockSitePairsForPerf.forEach((_, index) => {
       routerId: '',
       sourceSiteId: site2.identity,
       destinationSiteId: site1.identity,
+      sourceSiteName: site2.sourceName,
+      destinationSiteName: site1.destinationName,
       status: 'up'
     }
   );

diff --git a/src/pages/Topology/Topology.constants.ts b/src/pages/Topology/Topology.constants.ts
@@ -18,26 +18,6 @@ export const SHOW_LINK_METRIC_DISTRIBUTION = 'show-metric-distribution';
 export const SHOW_LINK_METRIC_VALUE = 'show-metric-value';
 
 export const displayOptionsForProcesses: TopologyDisplayOptionsMenu[] = [
-  {
-    title: TopologyLabels.TitleGroupDisplayOptionsMenuMetrics,
-    items: [
-      {
-        key: SHOW_LINK_BYTES,
-        value: SHOW_LINK_BYTES,
-        label: TopologyLabels.CheckboxShowTotalBytes
-      },
-      {
-        key: SHOW_LINK_BYTERATE,
-        value: SHOW_LINK_BYTERATE,
-        label: TopologyLabels.CheckboxShowCurrentByteRate
-      },
-      {
-        key: SHOW_LINK_LATENCY,
-        value: SHOW_LINK_LATENCY,
-        label: TopologyLabels.CheckboxShowLatency
-      }
-    ]
-  },
   {
     title: TopologyLabels.TitleGroupDisplayOptionsMenuMetricVisualization,
     items: [
@@ -58,6 +38,27 @@ export const displayOptionsForProcesses: TopologyDisplayOptionsMenu[] = [
       }
     ]
   },
+  {
+    title: TopologyLabels.TitleGroupDisplayOptionsMenuMetrics,
+    items: [
+      {
+        key: SHOW_LINK_BYTES,
+        value: SHOW_LINK_BYTES,
+        label: TopologyLabels.CheckboxShowTotalBytes
+      },
+      {
+        key: SHOW_LINK_BYTERATE,
+        value: SHOW_LINK_BYTERATE,
+        label: TopologyLabels.CheckboxShowCurrentByteRate
+      },
+      {
+        key: SHOW_LINK_LATENCY,
+        value: SHOW_LINK_LATENCY,
+        label: TopologyLabels.CheckboxShowLatency
+      }
+    ]
+  },
+
   {
     title: TopologyLabels.TitleGroupDisplayOptionsMenuOther,
     items: [
@@ -95,26 +96,6 @@ export const displayOptionsForSites: TopologyDisplayOptionsMenu[] = [
       }
     ]
   },
-  {
-    title: TopologyLabels.TitleGroupDisplayOptionsMenuMetrics,
-    items: [
-      {
-        key: SHOW_LINK_BYTES,
-        value: SHOW_LINK_BYTES,
-        label: TopologyLabels.CheckboxShowTotalBytes
-      },
-      {
-        key: SHOW_LINK_BYTERATE,
-        value: SHOW_LINK_BYTERATE,
-        label: TopologyLabels.CheckboxShowCurrentByteRate
-      },
-      {
-        key: SHOW_LINK_LATENCY,
-        value: SHOW_LINK_LATENCY,
-        label: TopologyLabels.CheckboxShowLatency
-      }
-    ]
-  },
   {
     title: TopologyLabels.TitleGroupDisplayOptionsMenuMetricVisualization,
     items: [
@@ -134,6 +115,26 @@ export const displayOptionsForSites: TopologyDisplayOptionsMenu[] = [
         label: TopologyLabels.CheckboxShowLabelReverse
       }
     ]
+  },
+  {
+    title: TopologyLabels.TitleGroupDisplayOptionsMenuMetrics,
+    items: [
+      {
+        key: SHOW_LINK_BYTES,
+        value: SHOW_LINK_BYTES,
+        label: TopologyLabels.CheckboxShowTotalBytes
+      },
+      {
+        key: SHOW_LINK_BYTERATE,
+        value: SHOW_LINK_BYTERATE,
+        label: TopologyLabels.CheckboxShowCurrentByteRate
+      },
+      {
+        key: SHOW_LINK_LATENCY,
+        value: SHOW_LINK_LATENCY,
+        label: TopologyLabels.CheckboxShowLatency
+      }
+    ]
   }
 ];
 

diff --git a/src/pages/Topology/components/DisplayOptions.tsx b/src/pages/Topology/components/DisplayOptions.tsx
@@ -1,113 +1,63 @@
-import { FC, Ref, useCallback, useState } from 'react';
+import { FC, MouseEvent as ReactMouseEvent, Ref, useCallback, useState } from 'react';
 
 import { MenuToggle, MenuToggleElement, Select, SelectGroup, SelectOption } from '@patternfly/react-core';
 
 import { TopologyDisplayOptionsMenu } from '@sk-types/Topology.interfaces';
 
-import {
-  SHOW_DATA_LINKS,
-  SHOW_LINK_BYTERATE,
-  SHOW_LINK_BYTES,
-  SHOW_LINK_LATENCY,
-  SHOW_ROUTER_LINKS
-} from '../Topology.constants';
+import { DisplayUseOptionsStateProps, useDisplayOptionsState } from '../hooks/useDisplayOptionsState';
 import { TopologyLabels } from '../Topology.enum';
 
-interface DisplayUseOptionsProps {
-  defaultSelected?: string[];
-  onSelected: (items: string[], item: string) => void;
-}
-
-interface DisplayOptionsProps extends DisplayUseOptionsProps {
+interface DisplayOptionsProps extends DisplayUseOptionsStateProps {
   options: TopologyDisplayOptionsMenu[];
   optionsDisabled?: Record<string, boolean>;
 }
 
-export const useDisplayOptions = ({ defaultSelected = [], onSelected }: DisplayUseOptionsProps) => {
-  const [displayOptionsSelected, setDisplayOptions] = useState(defaultSelected);
-
-  const selectDisplayOptions = useCallback(
-    (selectedOption: string) => {
-      const isOptionSelected = displayOptionsSelected.includes(selectedOption);
-
-      let updatedDisplayOptions = isOptionSelected
-        ? displayOptionsSelected.filter((option) => option !== selectedOption)
-        : [...displayOptionsSelected, selectedOption];
-
-      if (
-        selectedOption === SHOW_LINK_BYTES ||
-        selectedOption === SHOW_LINK_BYTERATE ||
-        selectedOption === SHOW_LINK_LATENCY
-      ) {
-        updatedDisplayOptions = isOptionSelected
-          ? displayOptionsSelected.filter((option) => option !== selectedOption)
-          : [
-              ...displayOptionsSelected.filter(
-                (option) => option !== SHOW_LINK_BYTES && option !== SHOW_LINK_BYTERATE && option !== SHOW_LINK_LATENCY
-              ),
-              selectedOption
-            ];
-      }
-
-      if (selectedOption === SHOW_DATA_LINKS || selectedOption === SHOW_ROUTER_LINKS) {
-        const otherOption = selectedOption === SHOW_DATA_LINKS ? SHOW_ROUTER_LINKS : SHOW_DATA_LINKS;
-
-        updatedDisplayOptions = isOptionSelected
-          ? [...updatedDisplayOptions, otherOption]
-          : updatedDisplayOptions.filter((option) => option !== otherOption);
-      }
-
-      setDisplayOptions(updatedDisplayOptions);
-      onSelected(updatedDisplayOptions, selectedOption);
-    },
-    [displayOptionsSelected, onSelected]
-  );
-
-  return { displayOptionsSelected, selectDisplayOptions };
-};
-
 const DisplayOptions: FC<DisplayOptionsProps> = function ({
   defaultSelected,
   options,
   onSelected,
   optionsDisabled = {}
 }) {
   const [isDisplayMenuOpen, setIsDisplayMenuOpen] = useState(false);
-  const { displayOptionsSelected, selectDisplayOptions } = useDisplayOptions({
+  const { displayOptionsSelected, selectDisplayOptions } = useDisplayOptionsState({
     defaultSelected,
     onSelected
   });
 
-  function toggleDisplayMenu() {
-    setIsDisplayMenuOpen(!isDisplayMenuOpen);
-  }
+  const toggleDisplayMenu = useCallback(() => {
+    setIsDisplayMenuOpen((prev) => !prev);
+  }, []);
 
-  const toggle = (toggleRef: Ref<MenuToggleElement>) => (
-    <MenuToggle ref={toggleRef} onClick={toggleDisplayMenu} isExpanded={isDisplayMenuOpen}>
-      {TopologyLabels.DisplayPlaceholderText}
-    </MenuToggle>
+  const handleSelect = useCallback(
+    (_?: ReactMouseEvent<Element, MouseEvent>, selection?: string | number) =>
+      selectDisplayOptions(selection!.toString()),
+    [selectDisplayOptions]
   );
 
+  const handleOpenChange = useCallback((open: boolean) => {
+    if (!open) {
+      setIsDisplayMenuOpen(false);
+    }
+  }, []);
+
   return (
     <Select
       isOpen={isDisplayMenuOpen}
-      onSelect={(_, selection) => selectDisplayOptions(selection!.toString())}
-      onOpenChange={(open) => !open && setIsDisplayMenuOpen(false)}
+      onSelect={handleSelect}
+      onOpenChange={handleOpenChange}
       selected={displayOptionsSelected}
-      toggle={toggle}
+      toggle={(toggleRef) => <Toggle toggleRef={toggleRef} isOpen={isDisplayMenuOpen} onClick={toggleDisplayMenu} />}
     >
       {options.map((group, index) => (
         <SelectGroup key={index} label={group.title}>
           {group.items.map(({ key, value, label }) => (
-            <SelectOption
+            <Option
               key={key}
               value={value}
+              label={label}
               isDisabled={optionsDisabled[value]}
-              hasCheckbox
               isSelected={displayOptionsSelected.includes(value)}
-            >
-              {label}
-            </SelectOption>
+            />
           ))}
         </SelectGroup>
       ))}
@@ -116,3 +66,29 @@ const DisplayOptions: FC<DisplayOptionsProps> = function ({
 };
 
 export default DisplayOptions;
+
+const Toggle: FC<{ toggleRef: Ref<MenuToggleElement>; isOpen: boolean; onClick: () => void }> = function ({
+  toggleRef,
+  isOpen,
+  onClick
+}) {
+  return (
+    <MenuToggle ref={toggleRef} onClick={onClick} isExpanded={isOpen}>
+      {TopologyLabels.DisplayPlaceholderText}
+    </MenuToggle>
+  );
+};
+
+const Option: FC<{
+  key: string;
+  value: string;
+  label: string;
+  isDisabled: boolean;
+  isSelected: boolean;
+}> = function ({ key, value, label, isDisabled, isSelected }) {
+  return (
+    <SelectOption key={key} value={value} isDisabled={isDisabled} hasCheckbox isSelected={isSelected}>
+      {label}
+    </SelectOption>
+  );
+};