diff --git a/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx b/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx index 57c7cf9d435..e028c15c7f9 100644 --- a/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx +++ b/frontend/packages/process-editor/src/hooks/useBpmnEditor.test.tsx @@ -8,14 +8,26 @@ import { getBpmnEditorDetailsFromBusinessObject } from '../utils/hookUtils'; import type { BpmnDetails } from '../types/BpmnDetails'; import { BpmnTypeEnum } from '../enum/BpmnTypeEnum'; import type { BpmnTaskType } from '../types/BpmnTaskType'; +import type { LayoutSets } from 'app-shared/types/api/LayoutSetsResponse'; +const taskId = 'testId'; +const layoutSetId = 'someLayoutSetId'; const bpmnDetailsMock: BpmnDetails = { - id: 'testId', + id: taskId, name: 'mockName', type: BpmnTypeEnum.Task, taskType: 'data', }; +const layoutSetsMock: LayoutSets = { + sets: [ + { + id: layoutSetId, + tasks: [taskId], + }, + ], +}; + class BpmnModelerMockImpl { public readonly _currentEventName: string; private readonly eventBus: any; @@ -40,12 +52,7 @@ class BpmnModelerMockImpl { } jest.mock('../utils/hookUtils', () => ({ - getBpmnEditorDetailsFromBusinessObject: jest.fn().mockReturnValue({ - id: 'testId', - name: 'mockName', - type: 'bpmn:Task', - taskType: 'data', - }), + getBpmnEditorDetailsFromBusinessObject: jest.fn().mockReturnValue({}), })); jest.mock('../contexts/BpmnConfigPanelContext', () => ({ @@ -74,9 +81,7 @@ const overrideUseBpmnContext = () => { }; jest.mock('./useBpmnModeler', () => ({ - useBpmnModeler: jest.fn().mockReturnValue({ - getModeler: jest.fn(), - }), + useBpmnModeler: jest.fn().mockReturnValue({}), })); const overrideUseBpmnModeler = (currentEventName: string) => { @@ -85,12 +90,17 @@ const overrideUseBpmnModeler = (currentEventName: string) => { }); }; +const overrideGetBpmnEditorDetailsFromBusinessObject = (bpmnDetails: BpmnDetails) => { + (getBpmnEditorDetailsFromBusinessObject as jest.Mock).mockReturnValue(bpmnDetails); +}; + const wrapper = ({ children }) => ( {children} @@ -129,35 +139,56 @@ describe('useBpmnEditor', () => { it.each(['confirmation', 'signing', 'feedback', 'endEvent'])( 'should not call saveBpmn when "shape.add" event is triggered on modelerInstance when taskType is not data', (taskType: BpmnTaskType) => { - const mockBpmnDetailsConfirm: BpmnDetails = { + const mockBpmnDetailsNotData: BpmnDetails = { id: 'otherTestId', name: 'mockName', type: BpmnTypeEnum.Task, taskType: taskType, }; - (getBpmnEditorDetailsFromBusinessObject as jest.Mock).mockReturnValue(mockBpmnDetailsConfirm); const currentEventName = 'shape.add'; - renderUseBpmnEditor(true, currentEventName); + renderUseBpmnEditor(true, currentEventName, mockBpmnDetailsNotData); expect(addLayoutSetMock).not.toHaveBeenCalled(); expect(setBpmnDetailsMock).toHaveBeenCalledTimes(1); - expect(setBpmnDetailsMock).toHaveBeenCalledWith(mockBpmnDetailsConfirm); + expect(setBpmnDetailsMock).toHaveBeenCalledWith(mockBpmnDetailsNotData); + }, + ); + + it.each(['data', 'confirmation', 'signing', 'feedback', 'endEvent'])( + 'should call deleteLayoutSet when "shape.remove" event is triggered on modelerInstance and the task has a connected layout set', + (taskType: BpmnTaskType) => { + const mockBpmnDetailsWithConnectedLayoutSet: BpmnDetails = { + id: taskId, + name: 'mockName', + type: BpmnTypeEnum.Task, + taskType: taskType, + }; + const currentEventName = 'shape.remove'; + renderUseBpmnEditor(true, currentEventName, mockBpmnDetailsWithConnectedLayoutSet); + + expect(deleteLayoutSetMock).toHaveBeenCalledTimes(1); + expect(deleteLayoutSetMock).toHaveBeenCalledWith({ layoutSetIdToUpdate: layoutSetId }); + expect(setBpmnDetailsMock).toHaveBeenCalled(); + expect(setBpmnDetailsMock).toHaveBeenCalledWith(null); }, ); - it('should call deleteLayoutSet when "shape.remove" event is triggered on modelerInstance', () => { - (getBpmnEditorDetailsFromBusinessObject as jest.Mock).mockReturnValue(bpmnDetailsMock); + it('should not call deleteLayoutSet when "shape.remove" event is triggered on modelerInstance and deleted task has no layoutSet', () => { + const mockBpmnDetailsWithoutConnectedLayoutSet: BpmnDetails = { + id: 'TaskIdNotPresetInLayoutSets', + name: 'mockName', + type: BpmnTypeEnum.Task, + taskType: 'data', + }; const currentEventName = 'shape.remove'; - renderUseBpmnEditor(true, currentEventName); + renderUseBpmnEditor(true, currentEventName, mockBpmnDetailsWithoutConnectedLayoutSet); - expect(deleteLayoutSetMock).toHaveBeenCalledTimes(1); - expect(deleteLayoutSetMock).toHaveBeenCalledWith({ layoutSetIdToUpdate: bpmnDetailsMock.id }); + expect(deleteLayoutSetMock).not.toHaveBeenCalled(); expect(setBpmnDetailsMock).toHaveBeenCalled(); expect(setBpmnDetailsMock).toHaveBeenCalledWith(null); }); it('should call setBpmnDetails when "element.click" event is triggered on eventBus', () => { - (getBpmnEditorDetailsFromBusinessObject as jest.Mock).mockReturnValue(bpmnDetailsMock); const currentEventName = 'element.click'; renderUseBpmnEditor(true, currentEventName); @@ -169,8 +200,13 @@ describe('useBpmnEditor', () => { }); }); -const renderUseBpmnEditor = (overrideBpmnContext: boolean, currentEventName: string) => { +const renderUseBpmnEditor = ( + overrideBpmnContext: boolean, + currentEventName: string, + bpmnDetails = bpmnDetailsMock, +) => { overrideBpmnContext && overrideUseBpmnContext(); + overrideGetBpmnEditorDetailsFromBusinessObject(bpmnDetails); overrideUseBpmnModeler(currentEventName); renderHook(() => useBpmnEditor(), { wrapper }); }; diff --git a/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts b/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts index f798e845bab..08a1e14ae84 100644 --- a/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts +++ b/frontend/packages/process-editor/src/hooks/useBpmnEditor.ts @@ -1,5 +1,5 @@ import type { MutableRefObject } from 'react'; -import { useEffect, useRef } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; import type BpmnModeler from 'bpmn-js/lib/Modeler'; import { useBpmnContext } from '../contexts/BpmnContext'; import { useBpmnModeler } from './useBpmnModeler'; @@ -7,6 +7,7 @@ import { getBpmnEditorDetailsFromBusinessObject } from '../utils/hookUtils'; import { useBpmnConfigPanelFormContext } from '../contexts/BpmnConfigPanelContext'; import { useBpmnApiContext } from '../contexts/BpmnApiContext'; import { BpmnTypeEnum } from '../enum/BpmnTypeEnum'; +import { getLayoutSetIdFromTaskId } from '../utils/hookUtils/hookUtils'; // Wrapper around bpmn-js to Reactify it @@ -20,23 +21,14 @@ export const useBpmnEditor = (): UseBpmnViewerResult => { const canvasRef = useRef(null); const { metaDataFormRef, resetForm } = useBpmnConfigPanelFormContext(); const { getModeler } = useBpmnModeler(); - const { addLayoutSet, deleteLayoutSet, saveBpmn } = useBpmnApiContext(); + const { addLayoutSet, deleteLayoutSet, saveBpmn, layoutSets } = useBpmnApiContext(); + //let modelerRef: BpmnModeler | null = null; const handleCommandStackChanged = async () => { saveBpmn(await getUpdatedXml(), metaDataFormRef.current || null); resetForm(); }; - const handleShapeRemove = (e) => { - const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(e?.element?.businessObject); - if (bpmnDetails.type === BpmnTypeEnum.Task) { - deleteLayoutSet({ - layoutSetIdToUpdate: bpmnDetails.id, - }); - } - setBpmnDetails(null); - }; - const handleShapeAdd = (e) => { const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(e?.element?.businessObject); setBpmnDetails(bpmnDetails); @@ -48,62 +40,68 @@ export const useBpmnEditor = (): UseBpmnViewerResult => { } }; - const handleSetBpmnDetails = (e) => { - const bpmnDetails = { - ...getBpmnEditorDetailsFromBusinessObject(e.element?.businessObject), - element: e.element, - }; - setBpmnDetails(bpmnDetails); + const handleShapeRemove = (e) => { + const bpmnDetails = getBpmnEditorDetailsFromBusinessObject(e?.element?.businessObject); + if (bpmnDetails.type === BpmnTypeEnum.Task) { + const layoutSetId = getLayoutSetIdFromTaskId(bpmnDetails, layoutSets); + if (layoutSetId) { + deleteLayoutSet({ + layoutSetIdToUpdate: layoutSetId, + }); + } + } + setBpmnDetails(null); + }; + + const handleSetBpmnDetails = useCallback( + (e) => { + const bpmnDetails = { + ...getBpmnEditorDetailsFromBusinessObject(e.element?.businessObject), + element: e.element, + }; + setBpmnDetails(bpmnDetails); + }, + [setBpmnDetails], + ); + + const initializeEditor = async () => { + try { + await modelerRef.current.importXML(bpmnXml); + const canvas: any = modelerRef.current.get('canvas'); + canvas.zoom('fit-viewport'); + } catch (exception) { + console.log('An error occurred while rendering the viewer:', exception); + } + }; + + const initializeBpmnChanges = () => { + modelerRef.current.on('commandStack.changed', async () => { + await handleCommandStackChanged(); + }); + modelerRef.current.on('shape.add', (e) => { + handleShapeAdd(e); + }); + modelerRef.current.on('shape.remove', (e) => { + handleShapeRemove(e); + }); }; useEffect(() => { if (!canvasRef.current) { console.log('Canvas reference is not yet available in the DOM.'); - return; } - const modelerInstance: BpmnModeler = getModeler(canvasRef.current); - - // set modelerRef.current to the Context so that it can be used in other components - modelerRef.current = modelerInstance; - - const initializeEditor = async () => { - try { - await modelerInstance.importXML(bpmnXml); - const canvas: any = modelerInstance.get('canvas'); - canvas.zoom('fit-viewport'); - } catch (exception) { - console.log('An error occurred while rendering the viewer:', exception); - } - }; - + // GetModeler can only be fetched from this hook once since the modeler creates a + // new instance and will attach the same canvasRef container to all instances it fetches + modelerRef.current = getModeler(canvasRef.current); initializeEditor(); - }, [modelerRef]); // Missing dependencies are not added due to resulting wierd behaviour in the process editor - - useEffect(() => { - const initializeBpmnChanges = () => { - const modelerInstance = getModeler(canvasRef.current); - - modelerInstance.on('commandStack.changed', async () => { - await handleCommandStackChanged(); - }); - - modelerInstance.on('shape.add', (e) => { - handleShapeAdd(e); - }); - - modelerInstance.on('shape.remove', (e) => { - handleShapeRemove(e); - }); - }; - initializeBpmnChanges(); - }, []); // Missing dependencies are not added due to resulting wierd behaviour in the process editor + // set modelerRef.current to the Context so that it can be used in other components + }, []); // Missing dependencies are not added to avoid getModeler to be called multiple times useEffect(() => { - const modelerInstance: BpmnModeler = getModeler(canvasRef.current); - const eventBus: BpmnModeler = modelerInstance.get('eventBus'); + const eventBus: BpmnModeler = modelerRef.current.get('eventBus'); eventBus.on('element.click', handleSetBpmnDetails); - }, [getModeler, handleSetBpmnDetails]); + }, [modelerRef, handleSetBpmnDetails]); return { canvasRef, modelerRef }; }; diff --git a/frontend/packages/process-editor/src/hooks/useBpmnModeler/useBpmnModeler.test.ts b/frontend/packages/process-editor/src/hooks/useBpmnModeler/useBpmnModeler.test.ts index 19977e74814..fac817172ca 100644 --- a/frontend/packages/process-editor/src/hooks/useBpmnModeler/useBpmnModeler.test.ts +++ b/frontend/packages/process-editor/src/hooks/useBpmnModeler/useBpmnModeler.test.ts @@ -17,7 +17,6 @@ jest.mock( ); const mockedCanvasHTMLDivElement = `
MockedHtml
` as unknown as HTMLDivElement; -const mockedCanvasHTMLDivElement2 = `
MockedHtml2
` as unknown as HTMLDivElement; describe('useBpmnModeler', () => { it('should create instance of the BpmnModeler when calling getModeler', () => { @@ -27,16 +26,4 @@ describe('useBpmnModeler', () => { const modelerInstance = getModeler(mockedCanvasHTMLDivElement) as ModelerMock; expect(modelerInstance.container.container).toBe(mockedCanvasHTMLDivElement); }); - - it('should avoid creating a new instance of the class if it already has an instance', () => { - const { result } = renderHook(() => useBpmnModeler()); - const { getModeler } = result.current; - - const modelerInstance = getModeler(mockedCanvasHTMLDivElement) as ModelerMock; - expect(modelerInstance.container.container).toBe(mockedCanvasHTMLDivElement); - - const modelerInstance2 = getModeler(mockedCanvasHTMLDivElement2) as ModelerMock; - expect(modelerInstance2.container.container).not.toBe(mockedCanvasHTMLDivElement2); - expect(modelerInstance2.container.container).toBe(mockedCanvasHTMLDivElement); - }); }); diff --git a/frontend/packages/process-editor/src/utils/StudioBpmnModeler.ts b/frontend/packages/process-editor/src/utils/StudioBpmnModeler.ts index 786f4edeac2..bb52c8844d2 100644 --- a/frontend/packages/process-editor/src/utils/StudioBpmnModeler.ts +++ b/frontend/packages/process-editor/src/utils/StudioBpmnModeler.ts @@ -6,20 +6,19 @@ import { altinnCustomTasks } from '../extensions/altinnCustomTasks'; export class StudioBpmnModeler { private static instance: BpmnModeler | null = null; - // Singleton pattern to ensure only one instance of the StudioBpmnModeler is created + // Need to create a new modelerInstance in order to get the updated + // canvasContainer when switching between tabs in Studio public static getInstance(canvasContainer: HTMLDivElement): BpmnModeler { - if (!StudioBpmnModeler.instance) { - StudioBpmnModeler.instance = new BpmnModeler({ - container: canvasContainer, - keyboard: { - bindTo: document, - }, - additionalModules: [SupportedPaletteProvider, SupportedContextPadProvider], - moddleExtensions: { - altinn: altinnCustomTasks, - }, - }); - } + StudioBpmnModeler.instance = new BpmnModeler({ + container: canvasContainer, + keyboard: { + bindTo: document, + }, + additionalModules: [SupportedPaletteProvider, SupportedContextPadProvider], + moddleExtensions: { + altinn: altinnCustomTasks, + }, + }); return StudioBpmnModeler.instance; } } diff --git a/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.test.ts b/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.test.ts index 14757da4438..cf494df7784 100644 --- a/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.test.ts +++ b/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.test.ts @@ -9,6 +9,7 @@ import type { BpmnTaskType } from '../../types/BpmnTaskType'; import { getBpmnEditorDetailsFromBusinessObject, getBpmnViewerDetailsFromBusinessObject, + getLayoutSetIdFromTaskId, } from './hookUtils'; describe('hookUtils', () => { @@ -109,4 +110,37 @@ describe('hookUtils', () => { expect(bpmnDetails.taskType).toBeNull(); }); }); + + const mockTaskId = 'mockId'; + const bpmnDetailsMock: BpmnDetails = { + id: mockTaskId, + name: 'mockName', + type: BpmnTypeEnum.Task, + taskType: 'data', + }; + + const layoutSets = { + sets: [ + { id: 'layoutSet1', tasks: ['task1'] }, + { id: 'layoutSet2', tasks: ['task2'] }, + ], + }; + + describe('getLayoutSetIdFromTaskId', () => { + it('should return the layout set id corresponding to the task id', () => { + const result = getLayoutSetIdFromTaskId({ ...bpmnDetailsMock, id: 'task1' }, layoutSets); + expect(result).toBe('layoutSet1'); + }); + + it('should return undefined if task id does not exist in any layout set', () => { + const result = getLayoutSetIdFromTaskId(bpmnDetailsMock, layoutSets); + expect(result).toBeUndefined(); + }); + + it('should return undefined if layout sets are empty', () => { + const layoutSets = { sets: [] }; + const result = getLayoutSetIdFromTaskId(bpmnDetailsMock, layoutSets); + expect(result).toBeUndefined(); + }); + }); }); diff --git a/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.ts b/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.ts index e3703a14250..89342493069 100644 --- a/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.ts +++ b/frontend/packages/process-editor/src/utils/hookUtils/hookUtils.ts @@ -1,6 +1,7 @@ import type { BpmnDetails } from '../../types/BpmnDetails'; import type { BpmnBusinessObjectViewer } from '../../types/BpmnBusinessObjectViewer'; import type { BpmnBusinessObjectEditor } from '../../types/BpmnBusinessObjectEditor'; +import type { LayoutSets } from 'app-shared/types/api/LayoutSetsResponse'; /** * Gets the bpmn details from the business object in viewer mode @@ -44,3 +45,8 @@ export const getBpmnEditorDetailsFromBusinessObject = ( }; return bpmnDetails; }; + +export const getLayoutSetIdFromTaskId = (bpmnDetails: BpmnDetails, layoutSets: LayoutSets) => { + const layoutSet = layoutSets.sets.find((set) => set.tasks[0] === bpmnDetails.id); + return layoutSet?.id; +};