chore: Use string key instead of number for modal action key (KaotoIO…

…#1537)
igarashitm · Oct 11, 2024 · 21803d0 · 21803d0
1 parent c9f7a97
commit 21803d0
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 48 deletions.
diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteGroup.tsx
@@ -3,7 +3,10 @@ import { ContextMenuItem } from '@patternfly/react-topology';
 import { FunctionComponent, PropsWithChildren, useCallback, useContext } from 'react';
 import { IDataTestID } from '../../../../models';
 import { IVisualizationNode } from '../../../../models/visualization/base-visual-entity';
-import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider';
+import {
+  ACTION_ID_CONFIRM,
+  ActionConfirmationModalContext,
+} from '../../../../providers/action-confirmation-modal.provider';
 import { EntitiesContext } from '../../../../providers/entities.provider';
 
 interface ItemDeleteGroupProps extends PropsWithChildren<IDataTestID> {
@@ -22,7 +25,7 @@ export const ItemDeleteGroup: FunctionComponent<ItemDeleteGroupProps> = (props)
       text: 'All steps will be lost.',
     });
 
-    if (!isDeleteConfirmed) return;
+    if (isDeleteConfirmed !== ACTION_ID_CONFIRM) return;
 
     entitiesContext?.camelResource.removeEntity(flowId);
     entitiesContext?.updateEntitiesFromCamelResource();

diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.test.tsx
@@ -1,7 +1,10 @@
 import { fireEvent, render, waitFor } from '@testing-library/react';
 import { createVisualizationNode, IVisualizationNode } from '../../../../models';
 import { ItemDeleteStep } from './ItemDeleteStep';
-import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider';
+import {
+  ACTION_ID_CONFIRM,
+  ActionConfirmationModalContext,
+} from '../../../../providers/action-confirmation-modal.provider';
 
 describe('ItemDeleteStep', () => {
   const vizNode = createVisualizationNode('test', {});
@@ -39,7 +42,7 @@ describe('ItemDeleteStep', () => {
   });
 
   it('should call removechild if deletion is confirmed', async () => {
-    mockDeleteModalContext.actionConfirmation.mockResolvedValueOnce(true);
+    mockDeleteModalContext.actionConfirmation.mockResolvedValueOnce(ACTION_ID_CONFIRM);
     const wrapper = render(
       <ActionConfirmationModalContext.Provider value={mockDeleteModalContext}>
         <ItemDeleteStep vizNode={mockVizNode} loadActionConfirmationModal={true} />

diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemDeleteStep.tsx
@@ -4,7 +4,10 @@ import { FunctionComponent, PropsWithChildren, useCallback, useContext } from 'r
 import { IDataTestID } from '../../../../models';
 import { IVisualizationNode } from '../../../../models/visualization/base-visual-entity';
 import { EntitiesContext } from '../../../../providers/entities.provider';
-import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider';
+import {
+  ACTION_ID_CONFIRM,
+  ActionConfirmationModalContext,
+} from '../../../../providers/action-confirmation-modal.provider';
 
 interface ItemDeleteStepProps extends PropsWithChildren<IDataTestID> {
   vizNode: IVisualizationNode;
@@ -23,7 +26,7 @@ export const ItemDeleteStep: FunctionComponent<ItemDeleteStepProps> = (props) =>
         text: 'Step and its children will be lost.',
       });
 
-      if (!isDeleteConfirmed) return;
+      if (isDeleteConfirmed !== ACTION_ID_CONFIRM) return;
     }
 
     props.vizNode?.removeChild();

diff --git a/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.tsx b/packages/ui/src/components/Visualization/Custom/ContextMenu/ItemReplaceStep.tsx
@@ -5,7 +5,10 @@ import { IDataTestID } from '../../../../models';
 import { AddStepMode, IVisualizationNode } from '../../../../models/visualization/base-visual-entity';
 import { CatalogModalContext } from '../../../../providers/catalog-modal.provider';
 import { EntitiesContext } from '../../../../providers/entities.provider';
-import { ActionConfirmationModalContext } from '../../../../providers/action-confirmation-modal.provider';
+import {
+  ACTION_ID_CONFIRM,
+  ActionConfirmationModalContext,
+} from '../../../../providers/action-confirmation-modal.provider';
 
 interface ItemReplaceStepProps extends PropsWithChildren<IDataTestID> {
   vizNode: IVisualizationNode;
@@ -27,7 +30,7 @@ export const ItemReplaceStep: FunctionComponent<ItemReplaceStepProps> = (props)
         text: 'Step and its children will be lost.',
       });
 
-      if (!isReplaceConfirmed) return;
+      if (isReplaceConfirmed !== ACTION_ID_CONFIRM) return;
     }
 
     /** Find compatible components */
@@ -45,7 +48,7 @@ export const ItemReplaceStep: FunctionComponent<ItemReplaceStepProps> = (props)
 
     /** Update entity */
     entitiesContext.updateEntitiesFromCamelResource();
-  }, [replaceModalContext, catalogModalContext, entitiesContext, props.vizNode]);
+  }, [props.vizNode, props.loadActionConfirmationModal, entitiesContext, catalogModalContext, replaceModalContext]);
 
   return (
     <ContextMenuItem onClick={onReplaceNode} data-testid={props['data-testid']}>

diff --git a/packages/ui/src/providers/__snapshots__/action-confirmaton-modal.provider.test.tsx.snap b/packages/ui/src/providers/__snapshots__/action-confirmaton-modal.provider.test.tsx.snap
@@ -90,7 +90,7 @@ exports[`ActionConfirmationModalProvider should allow consumers to update the mo
       data-ouia-component-id="OUIA-Generated-Button-danger-3"
       data-ouia-component-type="PF5/Button"
       data-ouia-safe="true"
-      data-testid="action-confirmation-modal-btn-1"
+      data-testid="action-confirmation-modal-btn-confirm"
       type="button"
     >
       Confirm
@@ -101,7 +101,7 @@ exports[`ActionConfirmationModalProvider should allow consumers to update the mo
       data-ouia-component-id="OUIA-Generated-Button-link-3"
       data-ouia-component-type="PF5/Button"
       data-ouia-safe="true"
-      data-testid="action-confirmation-modal-btn-0"
+      data-testid="action-confirmation-modal-btn-cancel"
       type="button"
     >
       Cancel

diff --git a/packages/ui/src/providers/action-confirmation-modal.provider.tsx b/packages/ui/src/providers/action-confirmation-modal.provider.tsx
@@ -1,10 +1,9 @@
 import { Button, ButtonVariant, Modal, ModalVariant } from '@patternfly/react-core';
 import { FunctionComponent, PropsWithChildren, createContext, useCallback, useMemo, useRef, useState } from 'react';
 
-export const ACTION_INDEX_CANCEL = 0;
-export const ACTION_INDEX_CONFIRM = 1;
+export const ACTION_ID_CANCEL = 'cancel';
+export const ACTION_ID_CONFIRM = 'confirm';
 export interface ActionConfirmationButtonOption {
-  index: number;
   buttonText: string;
   variant: ButtonVariant;
   isDanger?: boolean;
@@ -14,9 +13,9 @@ interface ActionConfirmationModalContextValue {
   actionConfirmation: (options: {
     title?: string;
     text?: string;
-    buttonOptions?: ActionConfirmationButtonOption[];
+    buttonOptions?: Record<string, ActionConfirmationButtonOption>;
     additionalModalText?: string;
-  }) => Promise<number>;
+  }) => Promise<string>;
 }
 
 export const ActionConfirmationModalContext = createContext<ActionConfirmationModalContextValue | undefined>(undefined);
@@ -29,20 +28,20 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
   const [isModalOpen, setIsModalOpen] = useState(false);
   const [title, setTitle] = useState('');
   const [textParagraphs, setTextParagraphs] = useState<string[]>([]);
-  const [buttonOptions, setButtonOptions] = useState<ActionConfirmationButtonOption[]>([]);
+  const [buttonOptions, setButtonOptions] = useState<Record<string, ActionConfirmationButtonOption>>({});
   const actionConfirmationRef = useRef<{
-    resolve: (index: number) => void;
+    resolve: (actionId: string) => void;
     reject: (error: unknown) => unknown;
   }>();
 
   const handleCloseModal = useCallback(() => {
     setIsModalOpen(false);
-    actionConfirmationRef.current?.resolve(ACTION_INDEX_CANCEL);
+    actionConfirmationRef.current?.resolve(ACTION_ID_CANCEL);
   }, []);
 
-  const handleAction = useCallback((index: number) => {
+  const handleAction = useCallback((actionId: string) => {
     setIsModalOpen(false);
-    actionConfirmationRef.current?.resolve(index);
+    actionConfirmationRef.current?.resolve(actionId);
   }, []);
 
   const actionConfirmation = useCallback(
@@ -51,10 +50,10 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
         title?: string;
         text?: string;
         additionalModalText?: string;
-        buttonOptions?: ActionConfirmationButtonOption[];
+        buttonOptions?: Record<string, ActionConfirmationButtonOption>;
       } = {},
     ) => {
-      const actionConfirmationPromise = new Promise<number>((resolve, reject) => {
+      const actionConfirmationPromise = new Promise<string>((resolve, reject) => {
         /** Set both resolve and reject functions to be used once the user choose an action */
         actionConfirmationRef.current = { resolve, reject };
       });
@@ -67,7 +66,7 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
       setTextParagraphs(textParagraphs);
       options.buttonOptions
         ? setButtonOptions(options.buttonOptions)
-        : setButtonOptions([{ index: ACTION_INDEX_CONFIRM, buttonText: 'Confirm', variant: ButtonVariant.danger }]);
+        : setButtonOptions({ [ACTION_ID_CONFIRM]: { buttonText: 'Confirm', variant: ButtonVariant.danger } });
       setIsModalOpen(true);
 
       return actionConfirmationPromise;
@@ -95,22 +94,22 @@ export const ActionConfirmationModalContextProvider: FunctionComponent<PropsWith
           onClose={handleCloseModal}
           ouiaId="ActionConfirmationModal"
           actions={[
-            ...buttonOptions.map((op) => (
+            ...Object.entries(buttonOptions).map(([actionId, option]) => (
               <Button
-                key={op.index}
-                variant={op.variant}
-                onClick={() => handleAction(op.index)}
-                data-testid={`action-confirmation-modal-btn-${op.index}`}
-                isDanger={op.isDanger}
+                key={actionId}
+                variant={option.variant}
+                onClick={() => handleAction(actionId)}
+                data-testid={`action-confirmation-modal-btn-${actionId}`}
+                isDanger={option.isDanger}
               >
-                {op.buttonText}
+                {option.buttonText}
               </Button>
             )),
             <Button
               key="cancel"
               variant="link"
               onClick={handleCloseModal}
-              data-testid={`action-confirmation-modal-btn-${ACTION_INDEX_CANCEL}`}
+              data-testid={`action-confirmation-modal-btn-${ACTION_ID_CANCEL}`}
             >
               Cancel
             </Button>,

diff --git a/packages/ui/src/providers/action-confirmaton-modal.provider.test.tsx b/packages/ui/src/providers/action-confirmaton-modal.provider.test.tsx
@@ -1,13 +1,15 @@
 import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
 import { FunctionComponent, useContext } from 'react';
 import {
+  ACTION_ID_CANCEL,
+  ACTION_ID_CONFIRM,
   ActionConfirmationButtonOption,
   ActionConfirmationModalContext,
   ActionConfirmationModalContextProvider,
 } from './action-confirmation-modal.provider';
 import { ButtonVariant } from '@patternfly/react-core';
 
-let actionConfirmationResult: number | undefined;
+let actionConfirmationResult: string | undefined;
 
 describe('ActionConfirmationModalProvider', () => {
   beforeEach(() => {
@@ -28,7 +30,7 @@ describe('ActionConfirmationModalProvider', () => {
     fireEvent.click(confirmButton);
 
     // Wait for actionConfirmation promise to resolve
-    await waitFor(() => expect(actionConfirmationResult).toEqual(1));
+    await waitFor(() => expect(actionConfirmationResult).toEqual(ACTION_ID_CONFIRM));
   });
 
   it('calls actionConfirmation with false when Cancel button is clicked', async () => {
@@ -45,7 +47,7 @@ describe('ActionConfirmationModalProvider', () => {
     fireEvent.click(cancelButton);
 
     // Wait for actionConfirmation promise to resolve
-    await waitFor(() => expect(actionConfirmationResult).toEqual(0));
+    await waitFor(() => expect(actionConfirmationResult).toEqual(ACTION_ID_CANCEL));
   });
 
   it('should allow consumers to update the modal title and text', () => {
@@ -74,19 +76,17 @@ describe('ActionConfirmationModalProvider', () => {
           title="Custom title"
           text="Custom text"
           additionalModalText="Additional text is added in the modal description"
-          buttonOptions={[
-            {
-              index: 1,
+          buttonOptions={{
+            'del-step-and-file': {
               buttonText: 'Delete the step, and delete the file(s)',
               variant: ButtonVariant.danger,
             },
-            {
-              index: 2,
+            'del-step-only': {
               buttonText: 'Delete the step, but keep the file(s)',
               variant: ButtonVariant.secondary,
               isDanger: true,
             },
-          ]}
+          }}
         />
       </ActionConfirmationModalContextProvider>,
     );
@@ -98,38 +98,38 @@ describe('ActionConfirmationModalProvider', () => {
     const modalDialog = wrapper.getByRole('dialog');
     expect(modalDialog.textContent).toContain('Additional text is added in the modal description');
     act(() => {
-      const cancelButton = wrapper.getByTestId('action-confirmation-modal-btn-0');
+      const cancelButton = wrapper.getByTestId('action-confirmation-modal-btn-cancel');
       expect(cancelButton.textContent).toEqual('Cancel');
       fireEvent.click(cancelButton);
     });
     await waitFor(() => {
-      expect(actionConfirmationResult).toEqual(0);
+      expect(actionConfirmationResult).toEqual(ACTION_ID_CANCEL);
     });
 
     act(() => {
       const deleteButton = wrapper.getByText('Delete');
       fireEvent.click(deleteButton);
     });
     act(() => {
-      const deleteStepAndFileButton = wrapper.getByTestId('action-confirmation-modal-btn-1');
+      const deleteStepAndFileButton = wrapper.getByTestId('action-confirmation-modal-btn-del-step-and-file');
       expect(deleteStepAndFileButton.textContent).toEqual('Delete the step, and delete the file(s)');
       fireEvent.click(deleteStepAndFileButton);
     });
     await waitFor(() => {
-      expect(actionConfirmationResult).toEqual(1);
+      expect(actionConfirmationResult).toEqual('del-step-and-file');
     });
 
     act(() => {
       const deleteButton = wrapper.getByText('Delete');
       fireEvent.click(deleteButton);
     });
     act(() => {
-      const deleteStepOnlyButton = wrapper.getByTestId('action-confirmation-modal-btn-2');
+      const deleteStepOnlyButton = wrapper.getByTestId('action-confirmation-modal-btn-del-step-only');
       expect(deleteStepOnlyButton.textContent).toEqual('Delete the step, but keep the file(s)');
       fireEvent.click(deleteStepOnlyButton);
     });
     await waitFor(() => {
-      expect(actionConfirmationResult).toEqual(2);
+      expect(actionConfirmationResult).toEqual('del-step-only');
     });
   });
 });
@@ -138,7 +138,7 @@ interface TestComponentProps {
   title: string;
   text: string;
   additionalModalText?: string;
-  buttonOptions?: ActionConfirmationButtonOption[];
+  buttonOptions?: Record<string, ActionConfirmationButtonOption>;
 }
 
 const TestComponent: FunctionComponent<TestComponentProps> = (props) => {