From c846ca6215aec5f40d64ee6704fbde1d442bf19f Mon Sep 17 00:00:00 2001 From: Jonas Dyrlie Date: Mon, 9 Sep 2024 09:47:02 +0200 Subject: [PATCH] fix: update config panel when deleting a connection (#13437) --- .../src/hooks/useBpmnEditor.test.tsx | 45 ++++++++++++------- .../process-editor/src/hooks/useBpmnEditor.ts | 29 +++++++----- .../src/types/SelectionChangeEvent.ts | 16 +++++++ .../process-editor/add-new-bpmn-task.spec.ts | 1 + 4 files changed, 62 insertions(+), 29 deletions(-) create mode 100644 frontend/packages/process-editor/src/types/SelectionChangeEvent.ts diff --git a/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx b/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx index 68b5100e889..6d99b251773 100644 --- a/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx +++ b/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx @@ -5,7 +5,6 @@ import { BpmnContextProvider, useBpmnContext } from '../contexts/BpmnContext'; import { BpmnApiContextProvider } from '../contexts/BpmnApiContext'; import { useBpmnModeler } from './useBpmnModeler'; import type { BpmnDetails } from '../types/BpmnDetails'; -import type { BpmnTaskType } from '../types/BpmnTaskType'; import type { LayoutSets } from 'app-shared/types/api/LayoutSetsResponse'; import { getMockBpmnElementForTask, mockBpmnDetails } from '../../test/mocks/bpmnDetailsMock'; import { mockModelerRef } from '../../test/mocks/bpmnModelerMock'; @@ -24,22 +23,21 @@ const layoutSetsMock: LayoutSets = { class BpmnModelerMockImpl { public readonly _currentEventName: string; - public readonly _currentTaskType: BpmnTaskType; + public readonly _currentEvent: any; private readonly eventBus: any; - constructor(currentEventName: string, currentTaskType: BpmnTaskType) { + constructor(currentEventName: string, currentEvent) { this._currentEventName = currentEventName; - this._currentTaskType = currentTaskType; + this._currentEvent = currentEvent; this.eventBus = { _currentEventName: this._currentEventName, - _currentTaskType: this._currentTaskType, on: this.on, }; } on(eventName: string, listener: (event: any) => void) { if (eventName === this._currentEventName) { - listener({ element: getMockBpmnElementForTask(this._currentTaskType) }); + listener(this._currentEvent); } } @@ -88,9 +86,9 @@ const overrideUseBpmnContext = () => { }); }; -const overrideUseBpmnModeler = (currentEventName: string, currentTaskType: BpmnTaskType) => { +const overrideUseBpmnModeler = (currentEventName: string, currentEvent: any) => { (useBpmnModeler as jest.Mock).mockReturnValue({ - getModeler: () => new BpmnModelerMockImpl(currentEventName, currentTaskType), + getModeler: () => new BpmnModelerMockImpl(currentEventName, currentEvent), destroyModeler: jest.fn(), }); }; @@ -125,40 +123,53 @@ describe('useBpmnEditor', () => { it('should call saveBpmn when "commandStack.changed" event is triggered on modelerInstance', async () => { const currentEventName = 'commandStack.changed'; - renderUseBpmnEditor(false, currentEventName); + const currentEvent = { element: getMockBpmnElementForTask('data') }; + renderUseBpmnEditor(false, currentEventName, currentEvent); await waitFor(() => expect(saveBpmnMock).toHaveBeenCalledTimes(1)); }); it('should handle "shape.added" event', async () => { - renderUseBpmnEditor(false, 'shape.added'); + const currentEvent = { element: getMockBpmnElementForTask('data') }; + renderUseBpmnEditor(false, 'shape.added', currentEvent); await waitFor(() => expect(onProcessTaskAddMock).toHaveBeenCalledTimes(1)); }); it('should handle "shape.remove" event', async () => { - renderUseBpmnEditor(false, 'shape.remove'); + const currentEvent = { element: getMockBpmnElementForTask('data') }; + renderUseBpmnEditor(false, 'shape.remove', currentEvent); await waitFor(() => expect(onProcessTaskRemoveMock).toHaveBeenCalledTimes(1)); }); - it('should call setBpmnDetails when "element.click" event is triggered on eventBus', () => { - const currentEventName = 'element.click'; - renderUseBpmnEditor(true, currentEventName); + it('should call setBpmnDetails with selected object when "selection.changed" event is triggered with new selection', async () => { + const currentEventName = 'selection.changed'; + const currentEvent = { newSelection: [getMockBpmnElementForTask('data')], oldSelection: [] }; + renderUseBpmnEditor(true, currentEventName, currentEvent); - expect(setBpmnDetailsMock).toHaveBeenCalledTimes(1); + await waitFor(() => expect(setBpmnDetailsMock).toHaveBeenCalledTimes(1)); expect(setBpmnDetailsMock).toHaveBeenCalledWith(expect.objectContaining(mockBpmnDetails)); }); + + it('should call setBpmnDetails with null when "selection.changed" event is triggered with no new selected object', async () => { + const currentEventName = 'selection.changed'; + const currentEvent = { oldSelection: [getMockBpmnElementForTask('data')], newSelection: [] }; + renderUseBpmnEditor(true, currentEventName, currentEvent); + + await waitFor(() => expect(setBpmnDetailsMock).toHaveBeenCalledTimes(1)); + expect(setBpmnDetailsMock).toHaveBeenCalledWith(null); + }); }); const renderUseBpmnEditor = ( overrideBpmnContext: boolean, currentEventName: string, - currentTaskType: BpmnTaskType = 'data', + currentEvent: any, bpmnDetails = mockBpmnDetails, ) => { overrideBpmnContext && overrideUseBpmnContext(); overrideGetBpmnEditorDetailsFromBusinessObject(bpmnDetails); - overrideUseBpmnModeler(currentEventName, currentTaskType); + overrideUseBpmnModeler(currentEventName, currentEvent); return renderHook(() => useBpmnEditor(), { wrapper }); }; diff --git a/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts b/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts index 40a9919a8a2..89e0b6b0e7f 100644 --- a/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts +++ b/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts @@ -5,6 +5,7 @@ import { useBpmnModeler } from './useBpmnModeler'; import { useBpmnConfigPanelFormContext } from '../contexts/BpmnConfigPanelContext'; import { useBpmnApiContext } from '../contexts/BpmnApiContext'; import type { TaskEvent } from '../types/TaskEvent'; +import type { SelectionChangedEvent } from '../types/SelectionChangeEvent'; import { getBpmnEditorDetailsFromBusinessObject } from '../utils/bpmnObjectBuilders'; import { useStudioRecommendedNextActionContext } from '@studio/components'; @@ -36,7 +37,6 @@ export const useBpmnEditor = (): UseBpmnViewerResult => { taskType: bpmnDetails.taskType, }); addAction(bpmnDetails.id); - updateBpmnDetailsByTaskEvent(taskEvent); }; const handleShapeRemove = (taskEvent: TaskEvent): void => { @@ -45,14 +45,22 @@ export const useBpmnEditor = (): UseBpmnViewerResult => { taskEvent, taskType: bpmnDetails.taskType, }); - setBpmnDetails(null); }; - const updateBpmnDetailsByTaskEvent = useCallback( - (taskEvent: TaskEvent) => { + const handleSelectionChange = (selectionEvent: SelectionChangedEvent): void => { + if (selectionEvent.newSelection.length !== 1) { + setBpmnDetails(null); + return; + } + const selectedElement = selectionEvent.newSelection[0]; + updateBpmnDetails(selectedElement); + }; + + const updateBpmnDetails = useCallback( + (element: any) => { const bpmnDetails = { - ...getBpmnEditorDetailsFromBusinessObject(taskEvent.element?.businessObject), - element: taskEvent.element, + ...getBpmnEditorDetailsFromBusinessObject(element?.businessObject), + element: element, }; setBpmnDetails(bpmnDetails); }, @@ -80,6 +88,9 @@ export const useBpmnEditor = (): UseBpmnViewerResult => { modelerRef.current.on('shape.remove', (taskEvent: TaskEvent): void => { handleShapeRemove(taskEvent); }); + modelerRef.current.on('selection.changed', (selectionEvent: SelectionChangedEvent): void => { + handleSelectionChange(selectionEvent); + }); }; useEffect(() => { @@ -99,12 +110,6 @@ export const useBpmnEditor = (): UseBpmnViewerResult => { // eslint-disable-next-line react-hooks/exhaustive-deps }, []); // Missing dependencies are not added to avoid getModeler to be called multiple times - useEffect(() => { - if (!modelerRef.current) return; - const eventBus: BpmnModeler = modelerRef.current.get('eventBus'); - eventBus.on('element.click', updateBpmnDetailsByTaskEvent); - }, [modelerRef, updateBpmnDetailsByTaskEvent]); - useEffect(() => { // Destroy the modeler instance when the component is unmounted return () => { diff --git a/frontend/packages/process-editor/src/types/SelectionChangeEvent.ts b/frontend/packages/process-editor/src/types/SelectionChangeEvent.ts new file mode 100644 index 00000000000..c630ab3f99c --- /dev/null +++ b/frontend/packages/process-editor/src/types/SelectionChangeEvent.ts @@ -0,0 +1,16 @@ +import { type BpmnBusinessObjectEditor } from './BpmnBusinessObjectEditor'; + +export type SelectionChangedEvent = { + newSelection: Array<{ + element: { + id: string; + businessObject: BpmnBusinessObjectEditor; + }; + }>; + oldSelection: Array<{ + element: { + id: string; + businessObject: BpmnBusinessObjectEditor; + }; + }>; +}; diff --git a/frontend/testing/playwright/tests/process-editor/add-new-bpmn-task.spec.ts b/frontend/testing/playwright/tests/process-editor/add-new-bpmn-task.spec.ts index 9fc2aa7f9f9..898a1b29760 100644 --- a/frontend/testing/playwright/tests/process-editor/add-new-bpmn-task.spec.ts +++ b/frontend/testing/playwright/tests/process-editor/add-new-bpmn-task.spec.ts @@ -99,6 +99,7 @@ test('that the user can add a sequence arrow between two tasks', async ({ page, const initialTaskSelector: string = await bpmnJSQuery.getTaskByIdAndType(initialId, 'g'); await processEditorPage.clickOnTaskInBpmnEditor(initialTaskSelector); + await page.waitForTimeout(500); // avoid double click, causing name editor to open await processEditorPage.clickOnTaskInBpmnEditor(initialTaskSelector); await processEditorPage.waitForInitialTaskHeaderToBeVisible();