Squashed 'bpmn-js-spiffworkflow/' changes from e92f48d..aca23dc

aca23dc 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.
05660e9 Per bug list we went over today, fixing a deep irritation with data object renaming, hopefully this feels better.

git-subtree-dir: bpmn-js-spiffworkflow
git-subtree-split: aca23dc56e5d37aa1ed0a3cf11acb55f76a36da7

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);
     }
   }
@@ -46,3 +49,12 @@ export function findDataReferenceShapes(processShape, id) {
   }
   return refs;
 }
+
+export function idToHumanReadableName(id) {
+  const words = id.match(/[A-Za-z][a-z]*/g) || [id];
+  return words.map(capitalize).join(' ');
+
+  function capitalize(word) {
+    return word.charAt(0).toUpperCase() + word.substring(1);
+  }
+}

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 { getDi, is } from 'bpmn-js/lib/util/ModelUtil';
-import {findDataObjects, findDataReferenceShapes} from './DataObjectHelpers';
+import { remove as collectionRemove } from 'diagram-js/lib/util/Collections';
-var HIGH_PRIORITY = 1500;
 import {
-  remove as collectionRemove,
+  findDataObjects,
-} from 'diagram-js/lib/util/Collections';
+  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) {
+    this.preExecute(['shape.create'], HIGH_PRIORITY, function (event) {
-      const context = event.context, shape = context.shape;
+      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) {
+    this.executed(['shape.create'], HIGH_PRIORITY, function (event) {
-      const context = event.context, shape = context.shape;
+      const { context } = event;
+      const { shape } = context;
       if (is(shape, 'bpmn:DataObjectReference') && shape.type !== 'label') {
-        let process = shape.parent.businessObject;
+        const process = shape.parent.businessObject;
-        let existingDataObjects = findDataObjects(process);
+        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 = 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'];

app/spiffworkflow/DataObject/propertiesPanel/DataObjectArray.js

@@ -5,7 +5,7 @@ import {
 } from '@bpmn-io/properties-panel';
 import { without } from 'min-dash';
 import { is } from 'bpmn-js/lib/util/ModelUtil';
-import { findDataObjects, findDataReferenceShapes } from '../DataObjectHelpers';
+import {findDataObjects, findDataReferenceShapes, idToHumanReadableName} from '../DataObjectHelpers';
 
 /**
  * Provides a list of data objects, and allows you to add / remove data objects, and change their ids.
@@ -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', {
+      commandStack.execute('shape.delete', { shape: ref });
-        element: ref,
-        moddleElement: ref.businessObject,
-        properties: {
-          name: '???',
-        },
-        changed: [ref], // everything is already marked as changed, don't recalculate.
-      });
     }
   };
 }
@@ -129,7 +122,7 @@ function DataObjectTextField(props) {
         element: ref,
         moddleElement: ref.businessObject,
         properties: {
-          name: value,
+          name: idToHumanReadableName(value),
         },
         changed: [ref], // everything is already marked as changed, don't recalculate.
       });

app/spiffworkflow/DataObject/propertiesPanel/DataObjectSelect.js

@@ -1,5 +1,6 @@
 import {useService } from 'bpmn-js-properties-panel';
 import { SelectEntry } from '@bpmn-io/properties-panel';
+import {findDataObjects, idToHumanReadableName} from '../DataObjectHelpers';
 
 /**
  * Finds the value of the given type within the extensionElements
@@ -43,7 +44,7 @@ export function DataObjectSelect(props) {
           element,
           moddleElement: businessObject,
           properties: {
-            'name': flowElem.id
+            'name': idToHumanReadableName(flowElem.id)
           }
         });
       }
@@ -53,13 +54,12 @@ export function DataObjectSelect(props) {
   const getOptions = value => {
     const businessObject = element.businessObject;
     const parent = businessObject.$parent;
-    let options = []
+    let dataObjects = findDataObjects(parent);
-    for (const element of parent.flowElements) {
+    let options = [];
-      if (element.$type === 'bpmn:DataObject') {
+    dataObjects.forEach(dataObj => {
-        options.push({label: element.id, value: element.id})
+      options.push({label: dataObj.id, value: dataObj.id})
-      }
+    });
-    }
+    return options;
-    return options
   }
 
   return <SelectEntry

test/spec/DataObjectInterceptorSpec.js

@@ -4,7 +4,7 @@ import { BpmnPropertiesPanelModule, BpmnPropertiesProviderModule } from 'bpmn-js
 import {
   inject,
 } from 'bpmn-js/test/helper';
-import { findDataObjects } from '../../app/spiffworkflow/DataObject/DataObjectHelpers';
+import {findDataObjects, idToHumanReadableName} from '../../app/spiffworkflow/DataObject/DataObjectHelpers';
 
 describe('DataObject Interceptor', function() {
 
@@ -93,7 +93,8 @@ describe('DataObject Interceptor', function() {
       { x: 220, y: 220 }, rootShape);
 
     const dataObjects = findDataObjects(rootShape.businessObject);
-    expect(dataObjectRefShape1.businessObject.name).to.equal(dataObjects[0].id);
+    const human_readable_name = idToHumanReadableName(dataObjects[0].id)
+    expect(dataObjectRefShape1.businessObject.name).to.equal(human_readable_name);
   }));
 
   it('should allow you to add a data object to a subprocess', inject(function(canvas, modeling, elementRegistry) {

test/spec/DataObjectPropsSpec.js

@@ -75,7 +75,7 @@ describe('Properties Panel for Data Objects', function() {
 
     // THEN - both the data object itself, and the label of any references are updated.
     expect(my_data_ref_1.businessObject.dataObjectRef.id).to.equal('my_nifty_new_name');
-    expect(my_data_ref_1.businessObject.name).to.equal('my_nifty_new_name');
+    expect(my_data_ref_1.businessObject.name).to.equal('My Nifty New Name');
   });
 
 });