fix(toolbar): prevent duplicate command execution on repeated interactions (#5456)

platform/core/src/hooks/__tests__/useToolbar.test.ts

@@ -0,0 +1,188 @@
+import { act } from 'react';
+import { renderHook } from '@testing-library/react';
+import { useToolbar } from '../useToolbar';
+import { useSystem } from '../../contextProviders/SystemProvider';
+
+type MockSubscription = { unsubscribe: jest.Mock<void, []> };
+
+type MockToolbarService = {
+  EVENTS: Record<string, string>;
+  getButtonSection: jest.Mock<any[], [string]>;
+  getButtonProps: jest.Mock<any, [string]>;
+  recordInteraction: jest.Mock;
+  subscribe: jest.Mock<MockSubscription, [string, (args: any) => void]>;
+  getButton: jest.Mock<any, [string]>;
+  refreshToolbarState: jest.Mock;
+};
+
+type MockViewportGridService = {
+  EVENTS: Record<string, string>;
+  subscribe: jest.Mock<MockSubscription, [string, (args: any) => void]>;
+  getActiveViewportId: jest.Mock<string, []>;
+};
+
+jest.mock('../../contextProviders/SystemProvider', () => ({
+  useSystem: jest.fn(),
+}));
+
+const mockedUseSystem = useSystem as jest.MockedFunction<typeof useSystem>;
+
+const createToolbarService = (): MockToolbarService => ({
+  EVENTS: {
+    TOOL_BAR_MODIFIED: 'TOOL_BAR_MODIFIED',
+    TOOL_BAR_STATE_MODIFIED: 'TOOL_BAR_STATE_MODIFIED',
+  },
+  getButtonSection: jest.fn().mockReturnValue([]),
+  getButtonProps: jest.fn().mockReturnValue({}),
+  recordInteraction: jest.fn(),
+  subscribe: jest.fn(() => ({ unsubscribe: jest.fn() })),
+  getButton: jest.fn(),
+  refreshToolbarState: jest.fn(),
+});
+
+const createViewportGridService = (): MockViewportGridService => ({
+  EVENTS: {
+    ACTIVE_VIEWPORT_ID_CHANGED: 'ACTIVE_VIEWPORT_ID_CHANGED',
+    VIEWPORTS_READY: 'VIEWPORTS_READY',
+    LAYOUT_CHANGED: 'LAYOUT_CHANGED',
+  },
+  subscribe: jest.fn(() => ({ unsubscribe: jest.fn() })),
+  getActiveViewportId: jest.fn().mockReturnValue('ACTIVE_VIEWPORT'),
+});
+
+describe('useToolbar', () => {
+  let toolbarService: MockToolbarService;
+  let viewportGridService: MockViewportGridService;
+  let commandsManager: { run: jest.Mock };
+  let servicesManager: {
+    services: { toolbarService: MockToolbarService; viewportGridService: MockViewportGridService };
+  };
+
+  const renderToolbarHook = () => renderHook(() => useToolbar({ buttonSection: 'primary' } as any));
+
+  beforeEach(() => {
+    toolbarService = createToolbarService();
+    viewportGridService = createViewportGridService();
+    commandsManager = { run: jest.fn() };
+    servicesManager = { services: { toolbarService, viewportGridService } };
+
+    mockedUseSystem.mockReset();
+    mockedUseSystem.mockReturnValue({
+      commandsManager,
+      servicesManager,
+    } as any);
+  });
+
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  it('aggregates commands on interaction and defers execution to the commands manager', () => {
+    const stopPropagation = jest.fn();
+
+    const option = {
+      id: 'option-1',
+      value: 'opt-1',
+      commands: 'secondaryCommand',
+      label: 'Option 1',
+    };
+    const ignoredOption = {
+      id: 'option-ignored',
+      value: 'ignored',
+      explicitRunOnly: true,
+      commands: ['ignoredCommand'],
+    };
+
+    toolbarService.getButtonProps.mockReturnValue({
+      commands: ['primaryCommand'],
+      options: [option, ignoredOption],
+    });
+
+    const { result } = renderToolbarHook();
+
+    act(() => {
+      result.current.onInteraction({
+        itemId: 'test-button',
+        viewportId: 'custom-viewport',
+        event: { stopPropagation },
+      });
+    });
+
+    expect(stopPropagation).toHaveBeenCalledTimes(1);
+    expect(toolbarService.getButtonProps).toHaveBeenCalledWith('test-button');
+
+    const [interactionArgs, interactionMeta] = toolbarService.recordInteraction.mock.calls[0];
+    const { commands } = interactionArgs;
+
+    expect(commands).toHaveLength(2);
+    commands.forEach(commandFn => expect(typeof commandFn).toBe('function'));
+
+    (commands[0] as () => void)();
+    expect(commandsManager.run).toHaveBeenLastCalledWith(
+      'primaryCommand',
+      expect.objectContaining({
+        itemId: 'test-button',
+        viewportId: 'custom-viewport',
+        event: expect.any(Object),
+      })
+    );
+
+    (commands[1] as () => void)();
+    expect(commandsManager.run).toHaveBeenLastCalledWith(
+      'secondaryCommand',
+      expect.objectContaining({
+        id: 'option-1',
+        value: 'opt-1',
+        options: expect.any(Array),
+        servicesManager,
+        commandsManager,
+      })
+    );
+
+    expect(interactionMeta).toEqual({ refreshProps: { viewportId: 'custom-viewport' } });
+  });
+
+  it('keeps button props immutable when aggregating commands', () => {
+    const stopPropagation = jest.fn();
+    const option = {
+      id: 'option-1',
+      value: 'opt-1',
+      commands: 'secondaryCommand',
+      label: 'Option 1',
+    };
+    const ignoredOption = {
+      id: 'option-ignored',
+      value: 'ignored',
+      explicitRunOnly: true,
+      commands: ['ignoredCommand'],
+    };
+    const buttonProps = {
+      commands: ['primaryCommand'],
+      options: [option, ignoredOption],
+    };
+
+    toolbarService.getButtonProps.mockReturnValue(buttonProps);
+
+    const { result } = renderToolbarHook();
+
+    act(() => {
+      result.current.onInteraction({
+        itemId: 'test-button',
+        viewportId: 'custom-viewport',
+        event: { stopPropagation },
+      });
+    });
+
+    act(() => {
+      result.current.onInteraction({
+        itemId: 'test-button',
+        viewportId: 'custom-viewport',
+        event: { stopPropagation },
+      });
+    });
+
+    expect(stopPropagation).toHaveBeenCalledTimes(2);
+    expect(buttonProps.commands).toHaveLength(1);
+    expect(toolbarService.recordInteraction).toHaveBeenCalledTimes(2);
+  });
+});

platform/core/src/hooks/useToolbar.tsx

@@ -1,6 +1,7 @@
 import { useCallback, useEffect, useState, useMemo } from 'react';
 import { useSystem } from '../contextProviders/SystemProvider';
 import { ToolbarHookReturn } from './types';
+import { buildButtonCommands } from '../utils';
 
 export function useToolbar({ buttonSection = 'primary' }: withAppTypes): ToolbarHookReturn {
   const { commandsManager, servicesManager } = useSystem();
@@ -24,48 +25,10 @@ export function useToolbar({ buttonSection = 'primary' }: withAppTypes): Toolbar
       const refreshProps = { viewportId };
 
       const buttonProps = toolbarService.getButtonProps(args.itemId);
+      const commands = buildButtonCommands(buttonProps, args, { servicesManager, commandsManager });
+      const computedProps = { ...buttonProps, commands };
 
-      if (buttonProps.commands || buttonProps.options) {
+      toolbarService.recordInteraction({ ...args, ...computedProps }, { refreshProps });
-        const allCommands = [];
-        const options = buttonProps.options || [];
-        const itemCommands = buttonProps.commands || [];
-
-        // Process item commands
-        if (itemCommands) {
-          Array.isArray(itemCommands)
-            ? allCommands.push(...itemCommands)
-            : allCommands.push(itemCommands);
-        }
-
-        // Process commands from options
-        if (options.length > 0) {
-          options.forEach(option => {
-            if (!option.commands) {
-              return;
-            }
-
-            const valueToUse = option.value;
-            const commands = Array.isArray(option.commands) ? option.commands : [option.commands];
-
-            commands.forEach(command => {
-              const commandOptions = {
-                ...option,
-                value: valueToUse,
-                options: buttonProps.options,
-                servicesManager: servicesManager,
-                commandsManager: commandsManager,
-              };
-
-              const processedCommand = () => commandsManager.run(command, commandOptions);
-              allCommands.push(processedCommand);
-            });
-          });
-        }
-
-        buttonProps.commands = allCommands;
-      }
-
-      toolbarService.recordInteraction({ ...args, ...buttonProps }, { refreshProps });
     },
     [toolbarService, viewportGridService]
   );

platform/core/src/services/ToolBarService/types.ts

@@ -40,10 +40,11 @@ export type ButtonOptions = {
   max?: number;
   step?: number;
   value?: number | number[] | string;
-  commands?: (value: unknown) => void;
+  commands?: RunCommand;
   condition?: (props: Record<string, unknown>) => boolean;
   children?: React.ReactNode | (() => React.ReactNode);
   options?: Array<{ value: string; label: string }>;
+  explicitRunOnly?: boolean;
 };
 
 export type ButtonProps = {

platform/core/src/types/index.ts

@@ -8,6 +8,7 @@ import type {
   BaseDataSourceConfigurationAPIItem,
 } from './DataSourceConfigurationAPI';
 
+export type * from '../services/ToolBarService/types';
 export type * from '../services/ViewportGridService';
 export type * from '../services/CustomizationService/types';
 // Separate out some generic types

platform/core/src/utils/buildButtonCommands.ts

@@ -0,0 +1,40 @@
+import { ButtonProps, RunCommand } from '../types';
+
+const toArray = <T>(value?: T | T[]): T[] =>
+  Array.isArray(value) ? value : value != null ? [value] : [];
+
+export const buildButtonCommands = (
+  buttonProps: ButtonProps,
+  baseArgs: Record<string, unknown>,
+  { servicesManager, commandsManager }: AppTypes.Managers
+): Array<() => unknown> => {
+  const allCommands: Array<() => unknown> = [];
+
+  // 1) normalize item-level commands
+  for (const command of toArray(buttonProps.commands as RunCommand)) {
+    allCommands.push(() => commandsManager.run(command, baseArgs));
+  }
+
+  // 2) normalize option-level commands
+  for (const option of toArray(buttonProps.options)) {
+    const shouldSkip = !option?.commands || option.explicitRunOnly;
+    if (shouldSkip) {
+      continue;
+    }
+
+    const valueToUse = option.value;
+    for (const command of toArray(option.commands)) {
+      const commandOptions = {
+        ...option,
+        value: valueToUse,
+        options: buttonProps.options,
+        servicesManager,
+        commandsManager,
+      };
+
+      allCommands.push(() => commandsManager.run(command, commandOptions));
+    }
+  }
+
+  return allCommands;
+};

platform/core/src/utils/index.ts

@@ -46,6 +46,8 @@ import getClosestOrientationFromIOP from './getClosestOrientationFromIOP';
 import calculateScanAxisNormal from './calculateScanAxisNormal';
 import areAllImageOrientationsEqual from './areAllImageOrientationsEqual';
 import { structuredCloneWithFunctions } from './structuredCloneWithFunctions';
+import { buildButtonCommands } from './buildButtonCommands';
+
 // Commented out unused functionality.
 // Need to implement new mechanism for derived displaySets using the displaySetManager.
 
@@ -134,6 +136,7 @@ export {
   createStudyBrowserTabs,
   MeasurementFilters,
   getClosestOrientationFromIOP,
+  buildButtonCommands,
 };
 
 export default utils;