From 782ad97076a09f995bf55d86aec76dbdc70805d6 Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 24 Nov 2022 14:59:09 -0500 Subject: [PATCH] Fixes several bugs in the editor related to Data Objects: 1) Correctly position the label on new data objects 2) When a Data Object is removed, remove all its references as well. 3) Avoid duplicate names showing up in the Data Objects list. 4) Allow non-words in data object names. --- .../DataObject/DataObjectHelpers.js | 7 ++- .../DataObject/DataObjectInterceptor.js | 55 +++++++++++++------ .../propertiesPanel/DataObjectArray.js | 11 +--- .../propertiesPanel/DataObjectSelect.js | 15 +++-- 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectHelpers.js b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectHelpers.js index 62265150..de081e4e 100644 --- a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectHelpers.js +++ b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectHelpers.js @@ -20,7 +20,10 @@ export function findDataObjects(parent) { return []; } for (const element of process.flowElements) { - if (element.$type === 'bpmn:DataObject') { + if ( + element.$type === 'bpmn:DataObject' && + dataObjects.indexOf(element) < 0 + ) { dataObjects.push(element); } } @@ -48,7 +51,7 @@ export function findDataReferenceShapes(processShape, id) { } export function idToHumanReadableName(id) { - const words = id.match(/[A-Za-z][a-z]*/g) || []; + const words = id.match(/[A-Za-z][a-z]*/g) || [id]; return words.map(capitalize).join(' '); function capitalize(word) { diff --git a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectInterceptor.js b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectInterceptor.js index 0af2251d..c0088302 100644 --- a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectInterceptor.js +++ b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/DataObjectInterceptor.js @@ -1,10 +1,13 @@ import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor'; -import {getDi, is} from 'bpmn-js/lib/util/ModelUtil'; -import {findDataObjects, findDataReferenceShapes, idToHumanReadableName} from './DataObjectHelpers'; -var HIGH_PRIORITY = 1500; +import { getDi, is } from 'bpmn-js/lib/util/ModelUtil'; +import { remove as collectionRemove } from 'diagram-js/lib/util/Collections'; import { - remove as collectionRemove, -} from 'diagram-js/lib/util/Collections'; + findDataObjects, + findDataReferenceShapes, + idToHumanReadableName, +} from './DataObjectHelpers'; + +const HIGH_PRIORITY = 1500; /** * This Command Interceptor functions like the BpmnUpdator in BPMN.js - It hooks into events * from Diagram.js and updates the underlying BPMN model accordingly. @@ -17,7 +20,7 @@ import { * 4) Don't allow someone to move a DataObjectReference from one process to another process. */ export default class DataObjectInterceptor extends CommandInterceptor { - constructor(eventBus, bpmnFactory, bpmnUpdater) { + constructor(eventBus, bpmnFactory, commandStack) { super(eventBus); /** @@ -26,8 +29,9 @@ export default class DataObjectInterceptor extends CommandInterceptor { * attempt to crete a dataObject immediately. We can't create the dataObject until * we know where it is placed - as we want to reuse data objects of the parent when * possible */ - this.preExecute([ 'shape.create' ], HIGH_PRIORITY, function(event) { - const context = event.context, shape = context.shape; + this.preExecute(['shape.create'], HIGH_PRIORITY, function (event) { + const { context } = event; + const { shape } = context; if (is(shape, 'bpmn:DataObjectReference') && shape.type !== 'label') { event.stopPropagation(); } @@ -36,11 +40,12 @@ export default class DataObjectInterceptor extends CommandInterceptor { /** * Don't just create a new data object, use the first existing one if it already exists */ - this.executed([ 'shape.create' ], HIGH_PRIORITY, function(event) { - const context = event.context, shape = context.shape; + this.executed(['shape.create'], HIGH_PRIORITY, function (event) { + const { context } = event; + const { shape } = context; if (is(shape, 'bpmn:DataObjectReference') && shape.type !== 'label') { - let process = shape.parent.businessObject; - let existingDataObjects = findDataObjects(process); + const process = shape.parent.businessObject; + const existingDataObjects = findDataObjects(process); let dataObject; if (existingDataObjects.length > 0) { dataObject = existingDataObjects[0]; @@ -48,20 +53,36 @@ export default class DataObjectInterceptor extends CommandInterceptor { dataObject = bpmnFactory.create('bpmn:DataObject'); } - // Update the name of the reference to match the data object's id. - shape.businessObject.name = idToHumanReadableName(dataObject.id); - // set the reference to the DataObject shape.businessObject.dataObjectRef = dataObject; } }); + /** + * In order for the label to display correctly, we need to update it in POST step. + */ + this.postExecuted(['shape.create'], HIGH_PRIORITY, function (event) { + const { context } = event; + const { shape } = context; + // set the reference to the DataObject + // Update the name of the reference to match the data object's id. + if (is(shape, 'bpmn:DataObjectReference') && shape.type !== 'label') { + commandStack.execute('element.updateProperties', { + element: shape, + moddleElement: shape.businessObject, + properties: { + name: idToHumanReadableName(shape.businessObject.dataObjectRef.id), + }, + }); + } + }); + /** * Don't remove the associated DataObject, unless all references to that data object * Difficult to do given placement of this logic in the BPMN Updater, so we have * to manually handle the removal. */ - this.executed([ 'shape.delete' ], HIGH_PRIORITY, function(event) { + this.executed(['shape.delete'], HIGH_PRIORITY, function (event) { const { context } = event; const { shape, oldParent } = context; if (is(shape, 'bpmn:DataObjectReference') && shape.type !== 'label') { @@ -97,4 +118,4 @@ export default class DataObjectInterceptor extends CommandInterceptor { } } -DataObjectInterceptor.$inject = [ 'eventBus', 'bpmnFactory', 'bpmnUpdater' ]; +DataObjectInterceptor.$inject = ['eventBus', 'bpmnFactory', 'commandStack']; diff --git a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectArray.js b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectArray.js index 4e8a9ed4..49b9487d 100644 --- a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectArray.js +++ b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectArray.js @@ -78,17 +78,10 @@ function removeFactory(props) { flowElements: without(process.get('flowElements'), dataObject), }, }); - // Also update the label of all the references + // When a data object is removed, remove all references as well. const references = findDataReferenceShapes(element, dataObject.id); for (const ref of references) { - commandStack.execute('element.updateProperties', { - element: ref, - moddleElement: ref.businessObject, - properties: { - name: '???', - }, - changed: [ref], // everything is already marked as changed, don't recalculate. - }); + commandStack.execute('shape.delete', { shape: ref }); } }; } diff --git a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectSelect.js b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectSelect.js index fdb5e966..e079e0a7 100644 --- a/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectSelect.js +++ b/bpmn-js-spiffworkflow/app/spiffworkflow/DataObject/propertiesPanel/DataObjectSelect.js @@ -1,6 +1,6 @@ import {useService } from 'bpmn-js-properties-panel'; import { SelectEntry } from '@bpmn-io/properties-panel'; -import {idToHumanReadableName} from '../DataObjectHelpers'; +import {findDataObjects, idToHumanReadableName} from '../DataObjectHelpers'; /** * Finds the value of the given type within the extensionElements @@ -54,13 +54,12 @@ export function DataObjectSelect(props) { const getOptions = value => { const businessObject = element.businessObject; const parent = businessObject.$parent; - let options = [] - for (const element of parent.flowElements) { - if (element.$type === 'bpmn:DataObject') { - options.push({label: element.id, value: element.id}) - } - } - return options + let dataObjects = findDataObjects(parent); + let options = []; + dataObjects.forEach(dataObj => { + options.push({label: dataObj.id, value: dataObj.id}) + }); + return options; } return