From dc7860161a444dd1708a91170b57b91a1baea8a7 Mon Sep 17 00:00:00 2001 From: Maciej Barelkowski Date: Tue, 10 Dec 2024 17:58:40 +0100 Subject: [PATCH] chore: refactor with code review results in mind --- .../CreateZeebeUserTaskBehavior.js | 74 +++++---- lib/camunda-cloud/util/ZeebeUserTaskUtil.js | 42 ------ .../CreateZeebeUserTaskBehaviorSpec.js | 140 +++++++++++++----- test/camunda-cloud/FormsBehaviorSpec.js | 36 +++-- 4 files changed, 169 insertions(+), 123 deletions(-) delete mode 100644 lib/camunda-cloud/util/ZeebeUserTaskUtil.js diff --git a/lib/camunda-cloud/CreateZeebeUserTaskBehavior.js b/lib/camunda-cloud/CreateZeebeUserTaskBehavior.js index f0d6896..197abea 100644 --- a/lib/camunda-cloud/CreateZeebeUserTaskBehavior.js +++ b/lib/camunda-cloud/CreateZeebeUserTaskBehavior.js @@ -1,8 +1,9 @@ -import { createElement } from '../util/ElementUtil'; -import { getZeebeUserTaskElement } from './util/ZeebeUserTaskUtil'; import { getBusinessObject, is } from 'bpmn-js/lib/util/ModelUtil'; import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor'; +import { createElement } from '../util/ElementUtil'; +import { getExtensionElementsList } from '../util/ExtensionElementsUtil'; + const HIGH_PRIORITY = 5000; /** @@ -20,44 +21,43 @@ export default class CreateZeebeUserTaskBehavior extends CommandInterceptor { HIGH_PRIORITY, function(context) { const shape = context.shape || context.newShape; - const isCopy = context.hints && context.hints.createElementsBehavior === false; + const explicitlyDisabled = context.hints && context.hints.createElementsBehavior === false; - if (!is(shape, 'bpmn:UserTask') || isCopy) { + if (!is(shape, 'bpmn:UserTask') || explicitlyDisabled) { return; } - const businessObject = getBusinessObject(shape); - - // Use getZeebeUserTaskElement to check if zeebe:userTask already exists - let userTaskElement = getZeebeUserTaskElement(businessObject); - - if (!userTaskElement) { - let extensionElements = businessObject.get('extensionElements'); - - if (!extensionElements) { - extensionElements = createElement( - 'bpmn:ExtensionElements', - { - values: [], - }, - businessObject, - bpmnFactory - ); + let userTaskElement = getZeebeUserTask(shape); + if (userTaskElement) { + return; + } - modeling.updateProperties(shape, { extensionElements }); - } + const businessObject = getBusinessObject(shape); + let extensionElements = businessObject.get('extensionElements'); - userTaskElement = createElement( - 'zeebe:UserTask', - {}, - extensionElements, + if (!extensionElements) { + extensionElements = createElement( + 'bpmn:ExtensionElements', + { + values: [], + }, + businessObject, bpmnFactory ); - modeling.updateModdleProperties(shape, extensionElements, { - values: [ ...(extensionElements.values || []), userTaskElement ], - }); + modeling.updateProperties(shape, { extensionElements }); } + + userTaskElement = createElement( + 'zeebe:UserTask', + {}, + extensionElements, + bpmnFactory + ); + + modeling.updateModdleProperties(shape, extensionElements, { + values: [ ...(extensionElements.values || []), userTaskElement ], + }); }, true ); @@ -65,3 +65,17 @@ export default class CreateZeebeUserTaskBehavior extends CommandInterceptor { } CreateZeebeUserTaskBehavior.$inject = [ 'bpmnFactory', 'eventBus', 'modeling' ]; + +/** + * Get zeebe:userTask extension. + * + * @param {djs.model.Base|ModdleElement} element + * + * @returns {ModdleElement|null} + */ +function getZeebeUserTask(element) { + const businessObject = getBusinessObject(element); + const userTaskElements = getExtensionElementsList(businessObject, 'zeebe:UserTask'); + + return userTaskElements[0] || null; +} diff --git a/lib/camunda-cloud/util/ZeebeUserTaskUtil.js b/lib/camunda-cloud/util/ZeebeUserTaskUtil.js deleted file mode 100644 index 1d49899..0000000 --- a/lib/camunda-cloud/util/ZeebeUserTaskUtil.js +++ /dev/null @@ -1,42 +0,0 @@ -import { getExtensionElementsList } from '../../util/ExtensionElementsUtil'; - -import { getBusinessObject, is } from 'bpmn-js/lib/util/ModelUtil'; - -/** - * Get all zeebe:userTask elements of an element. - * - * @param {djs.model.Base|ModdleElement} element - * - * @returns {Array} - */ -export function getZeebeUserTaskElements(element) { - const businessObject = getBusinessObject(element); - return getExtensionElementsList(businessObject, 'zeebe:UserTask'); -} - -/** - * Get the first zeebe:userTask element of an element. - * - * @param {djs.model.Base|ModdleElement} element - * - * @returns {ModdleElement|null} - */ -export function getZeebeUserTaskElement(element) { - const userTaskElements = getZeebeUserTaskElements(element); - return userTaskElements[0] || null; -} - -/** - * Check whether a zeebe:userTask extension element is set on an element. - * - * @param {djs.model.Base|ModdleElement} element - * - * @returns {boolean} - */ -export function hasZeebeUserTaskExtension(element) { - if (!is(element, 'bpmn:UserTask')) { - return false; - } - - return !!getZeebeUserTaskElement(element); -} diff --git a/test/camunda-cloud/CreateZeebeUserTaskBehaviorSpec.js b/test/camunda-cloud/CreateZeebeUserTaskBehaviorSpec.js index c59f530..bd42f6f 100644 --- a/test/camunda-cloud/CreateZeebeUserTaskBehaviorSpec.js +++ b/test/camunda-cloud/CreateZeebeUserTaskBehaviorSpec.js @@ -1,21 +1,21 @@ import { bootstrapCamundaCloudModeler, inject } from 'test/TestHelper'; +import { getBusinessObject, is } from 'bpmn-js/lib/util/ModelUtil'; import { find } from 'min-dash'; -import { - getZeebeUserTaskElement, - getZeebeUserTaskElements, -} from '../../lib/camunda-cloud/util/ZeebeUserTaskUtil'; +import { getExtensionElementsList } from 'lib/util/ExtensionElementsUtil'; -import { getBusinessObject, is } from 'bpmn-js/lib/util/ModelUtil'; import emptyProcessDiagramXML from './process-empty.bpmn'; import userTasksXML from './process-user-tasks.bpmn'; describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', function() { - describe('when creating new shapes', function() { + + describe('when a shape is created', function() { + beforeEach(bootstrapCamundaCloudModeler(emptyProcessDiagramXML)); + it('should execute when creating bpmn:UserTask', inject(function( canvas, modeling @@ -33,15 +33,44 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi // then const businessObject = getBusinessObject(newShape), - extensionElements = businessObject.get('extensionElements'), - zeebeUserTaskExtension = getZeebeUserTaskElement(newShape); + zeebeUserTaskExtensions = getExtensionElementsList(businessObject, 'zeebe:UserTask'); - expect(zeebeUserTaskExtension).to.exist; - expect(extensionElements).to.exist; - expect(zeebeUserTaskExtension.$parent).to.equal(extensionElements); + expect(zeebeUserTaskExtensions).to.exist; + expect(zeebeUserTaskExtensions).to.have.lengthOf(1); })); - it('should not execute when creating bpmn:Task', inject(function( + + it('should NOT execute when zeebe:UserTask already present', inject(function( + canvas, + bpmnFactory, + modeling + ) { + + // given + const rootElement = canvas.getRootElement(), + bo = bpmnFactory.create('bpmn:UserTask', { + extensionElements: bpmnFactory.create('bpmn:ExtensionElements', { + values: [ bpmnFactory.create('zeebe:UserTask') ], + }), + }); + + // when + const newShape = modeling.createShape( + { type: 'bpmn:UserTask', businessObject: bo }, + { x: 100, y: 100 }, + rootElement + ); + + // then + const businessObject = getBusinessObject(newShape), + zeebeUserTaskExtensions = getExtensionElementsList(businessObject, 'zeebe:UserTask'); + + expect(zeebeUserTaskExtensions).to.exist; + expect(zeebeUserTaskExtensions).to.have.lengthOf(1); + })); + + + it('should NOT execute when creating bpmn:Task', inject(function( canvas, modeling ) { @@ -57,16 +86,19 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi ); // then - const zeebeUserTaskExtension = getZeebeUserTaskElement(newShape); + const zeebeUserTaskExtension = getZeebeUserTask(newShape); expect(zeebeUserTaskExtension).not.to.exist; })); }); - describe('when copying bpmn:UserTask', function() { + + describe('when a shape is pasted', function() { + beforeEach(bootstrapCamundaCloudModeler(userTasksXML)); - it('should re-use existing extensionElement', inject(function( + + it('should NOT add zeebe:UserTask', inject(function( canvas, copyPaste, elementRegistry @@ -74,7 +106,7 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi // given const rootElement = canvas.getRootElement(); - const userTask = elementRegistry.get('withZeebeUserTask'); + const userTask = elementRegistry.get('UserTask_1'); // when copyPaste.copy(userTask); @@ -92,17 +124,13 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi is(element, 'bpmn:UserTask') ); - const businessObject = getBusinessObject(pastedUserTask), - extensionElements = businessObject.get('extensionElements'), - zeebeUserTaskExtensions = getZeebeUserTaskElements(pastedUserTask); + const zeebeUserTask = getZeebeUserTask(pastedUserTask); - expect(zeebeUserTaskExtensions).to.exist; - expect(extensionElements).to.exist; - expect(zeebeUserTaskExtensions.length).to.equal(1); - expect(zeebeUserTaskExtensions[0].$parent).to.equal(extensionElements); + expect(zeebeUserTask).not.to.exist; })); - it('should not add zeebe:UserTask if it was not already present', inject(function( + + it('should keep existing zeebe:UserTask', inject(function( canvas, copyPaste, elementRegistry @@ -110,7 +138,7 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi // given const rootElement = canvas.getRootElement(); - const userTask = elementRegistry.get('withEmptyExternalReference'); + const userTask = elementRegistry.get('withZeebeUserTask'); // when copyPaste.copy(userTask); @@ -127,21 +155,20 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi const pastedUserTask = find(elements, (element) => is(element, 'bpmn:UserTask') ); + const zeebeUserTasks = getExtensionElementsList(pastedUserTask, 'zeebe:UserTask'); - const businessObject = getBusinessObject(pastedUserTask), - extensionElements = businessObject.get('extensionElements'), - zeebeUserTaskExtensions = getZeebeUserTaskElements(pastedUserTask); - - expect(zeebeUserTaskExtensions).to.exist; - expect(extensionElements).to.exist; - expect(zeebeUserTaskExtensions.length).to.equal(0); + expect(zeebeUserTasks).to.exist; + expect(zeebeUserTasks).to.have.lengthOf(1); })); }); - describe('when replacing bpmn:Task', function() { + + describe('when a shape is replaced', function() { + beforeEach(bootstrapCamundaCloudModeler(userTasksXML)); - it('should execute when replacing to bpmn:UserTask', inject(function( + + it('should add zeebe:UserTask when target is bpmn:UserTask', inject(function( elementRegistry, bpmnReplace, canvas, @@ -161,9 +188,52 @@ describe('camunda-cloud/features/modeling - CreateZeebeUserTaskBehavior', functi // then const updatedTask = elementRegistry.get(task.id), - zeebeUserTaskExtension = getZeebeUserTaskElement(updatedTask); + zeebeUserTaskExtension = getZeebeUserTask(updatedTask); expect(zeebeUserTaskExtension).to.exist; })); + + + it('should NOT add zeebe:UserTask when target is bpmn:ServiceTask', inject(function( + elementRegistry, + bpmnReplace, + canvas, + modeling + ) { + + // given + const rootElement = canvas.getRootElement(); + + // when + const task = modeling.createShape( + { type: 'bpmn:Task', id: 'simpleTask' }, + { x: 100, y: 100 }, + rootElement + ); + bpmnReplace.replaceElement(task, { type: 'bpmn:ServiceTask' }); + + // then + const updatedTask = elementRegistry.get(task.id), + zeebeUserTask = getZeebeUserTask(updatedTask); + + expect(zeebeUserTask).not.to.exist; + })); }); }); + + +// helpers ////////// + +/** + * Get the first zeebe:userTask element of an element. + * + * @param {djs.model.Base|ModdleElement} element + * + * @returns {ModdleElement|null} + */ +function getZeebeUserTask(element) { + const businessObject = getBusinessObject(element); + const userTaskElements = getExtensionElementsList(businessObject, 'zeebe:UserTask'); + + return userTaskElements[0] || null; +} diff --git a/test/camunda-cloud/FormsBehaviorSpec.js b/test/camunda-cloud/FormsBehaviorSpec.js index e453ea8..833cad2 100644 --- a/test/camunda-cloud/FormsBehaviorSpec.js +++ b/test/camunda-cloud/FormsBehaviorSpec.js @@ -181,17 +181,15 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { describe('no existing form definition or user task form', function() { - it('should not create and reference new user task form', inject(function(canvas, elementFactory, modeling) { + it('should not create and reference new user task form', inject(function(canvas, elementFactory) { // given - const rootElement = canvas.getRootElement(); - const element = elementFactory.createShape({ type: 'bpmn:UserTask' }); // when - modeling.createShape(element, { x: 100, y: 100 }, rootElement, { createElementsBehavior: false }); + createShape(element); // then expect(getUserTaskForm(element)).not.to.exist; @@ -203,11 +201,9 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { describe('existing form definition, no user task form', function() { - it('should not create and reference new form definition', inject(function(bpmnFactory, canvas, elementFactory, modeling) { + it('should not create and reference new form definition', inject(function(bpmnFactory, canvas, elementFactory) { // given - const rootElement = canvas.getRootElement(); - const formDefinition = bpmnFactory.create('zeebe:FormDefinition', { formId: 'foo' }); @@ -230,7 +226,7 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { }); // when - modeling.createShape(element, { x: 100, y: 100 }, rootElement, { createElementsBehavior: false }); + createShape(element); // then expect(getUserTaskForm(element)).not.to.exist; @@ -242,11 +238,9 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { describe('existing form definition, user task form not referenced', function() { - it('should not create and reference new form definition', inject(function(bpmnFactory, canvas, elementFactory, modeling) { + it('should not create and reference new form definition', inject(function(bpmnFactory, canvas, elementFactory) { // given - const rootElement = canvas.getRootElement(); - const formDefinition = bpmnFactory.create('zeebe:FormDefinition', { formKey: userTaskFormIdToFormKey('UserTaskForm_3') }); @@ -269,7 +263,7 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { }); // when - modeling.createShape(element, { x: 100, y: 100 }, rootElement, { createElementsBehavior: false }); + createShape(element); // then expect(getUserTaskForm(element)).to.exist; @@ -283,11 +277,9 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { let element; - beforeEach(inject(function(canvas, bpmnFactory, elementFactory, modeling) { + beforeEach(inject(function(bpmnFactory, elementFactory) { // given - const rootElement = canvas.getRootElement(); - const formDefinition = bpmnFactory.create('zeebe:FormDefinition', { formKey: userTaskFormIdToFormKey('UserTaskForm_1') }); @@ -310,7 +302,7 @@ describe('camunda-cloud/features/modeling - FormsBehavior', function() { }); // when - modeling.createShape(element, { x: 100, y: 100 }, rootElement, { createElementsBehavior: false }); + createShape(element); })); @@ -852,3 +844,15 @@ function hasUsertaskForm(id, userTaskForms) { return userTaskForm.get('zeebe:id') === id; }); } + +/** + * Create shape without invoking create behavior. + * This allows to create a job-worker user task for the tests purpose. + * @param {ModdleElement} element + */ +function createShape(element) { + return getBpmnJS().invoke(function(canvas, modeling) { + const rootElement = canvas.getRootElement(); + return modeling.createShape(element, { x: 100, y: 100 }, rootElement, { createElementsBehavior: false }); + }); +}