fix(modeling): correctly handle missing bpmndi:Label bounds

Closes #794

CHANGELOG.md

@@ -6,9 +6,10 @@ All notable changes to [bpmn-js](https://github.com/bpmn-io/bpmn-js) are documen
 
 ___Note:__ Yet to be released changes appear here._
 
-### BREAKING CHANGES
+### Breaking Changes
 
 * `CHORE`: update to [`diagram-js@2.0.0`](https://github.com/bpmn-io/diagram-js/blob/master/CHANGELOG.md#200)
+* `FIX`: correctly handle missing `bpmndi:Label` bounds during model updating ([#794](https://github.com/bpmn-io/bpmn-js/issues/794))
 
 ## 1.3.3
 

lib/features/modeling/BpmnUpdater.js

@@ -361,7 +361,14 @@ BpmnUpdater.prototype.updateBounds = function(shape) {
 
   var di = shape.businessObject.di;
 
-  var bounds = (shape instanceof Label) ? this._getLabel(di).bounds : di.bounds;
+  var target = (shape instanceof Label) ? this._getLabel(di) : di;
+
+  var bounds = target.bounds;
+
+  if (!bounds) {
+    bounds = this._bpmnFactory.createDiBounds();
+    target.set('bounds', bounds);
+  }
 
   assign(bounds, {
     x: shape.x,

test/spec/features/modeling/BpmnUpdater.missingBounds.bpmn

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<semantic:definitions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:semantic="http://www.omg.org/spec/BPMN/20100524/MODEL" id="Definitions" targetNamespace="http://bpmn.io/schema/bpmn">
+  <semantic:process id="Process">
+    <semantic:startEvent id="StartEvent" />
+  </semantic:process>
+  <bpmndi:BPMNDiagram id="Trisotech.Visio-_6" name="Untitled Diagram" documentation="" resolution="96.00000267028808">
+    <bpmndi:BPMNPlane bpmnElement="Process">
+      <bpmndi:BPMNShape id="StartEvent_di" bpmnElement="StartEvent">
+        <dc:Bounds x="192" y="123" width="36" height="36" />
+        <bpmndi:BPMNLabel />
+      </bpmndi:BPMNShape>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</semantic:definitions>

test/spec/features/modeling/BpmnUpdaterSpec.js

@@ -6,71 +6,120 @@ import {
 import modelingModule from 'lib/features/modeling';
 import coreModule from 'lib/core';
 
+var testModules = [ modelingModule, coreModule ];
+
 
 describe('features - bpmn-updater', function() {
 
-  var diagramXML = require('./BpmnUpdater.bpmn');
-
-  var testModules = [ modelingModule, coreModule ];
-
-  beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
-
-
   describe('connection di', function() {
 
-    it('should update after deleting intermediate element', inject(function(modeling, elementRegistry) {
+    var diagramXML = require('./BpmnUpdater.bpmn');
 
-      // given
+    beforeEach(bootstrapModeler(diagramXML, {
-      // sequence flow with existing sourceElement and targetElement di information
+      modules: testModules
-      var task = elementRegistry.get('Task_1'),
-          sequenceFlowDi = elementRegistry.get('SequenceFlow_1').businessObject.di,
-          startEventDi = elementRegistry.get('StartEvent_1').businessObject.di,
-          endEventDi = elementRegistry.get('EndEvent_1').businessObject.di;
-
-      // when
-      modeling.removeElements([ task ]);
-
-      // then
-      expect(sequenceFlowDi.sourceElement).to.equal(startEventDi);
-      expect(sequenceFlowDi.targetElement).to.equal(endEventDi);
     }));
 
 
-    it('should update on drop on flow', inject(function(modeling, elementRegistry, elementFactory) {
+    it('should update after deleting intermediate element', inject(
+      function(modeling, elementRegistry) {
 
-      // given
+        // given
-      // sequence flow with existing sourceElement and targetElement di information
+        // sequence flow with existing sourceElement and targetElement di information
-      var sequenceFlow = elementRegistry.get('SequenceFlow_3'),
+        var task = elementRegistry.get('Task_1'),
-          startEventDi = elementRegistry.get('StartEvent_2').businessObject.di;
+            sequenceFlowDi = elementRegistry.get('SequenceFlow_1').businessObject.di,
+            startEventDi = elementRegistry.get('StartEvent_1').businessObject.di,
+            endEventDi = elementRegistry.get('EndEvent_1').businessObject.di;
 
-      var intermediateThrowEvent = elementFactory.createShape({ type: 'bpmn:IntermediateThrowEvent' }),
+        // when
-          dropPosition = { x: 320, y: 260 };
+        modeling.removeElements([ task ]);
 
-      // when
+        // then
-      var event = modeling.createShape(intermediateThrowEvent, dropPosition, sequenceFlow);
+        expect(sequenceFlowDi.sourceElement).to.equal(startEventDi);
+        expect(sequenceFlowDi.targetElement).to.equal(endEventDi);
+      }
+    ));
 
-      // then
+
-      expect(sequenceFlow.businessObject.di.sourceElement).to.equal(startEventDi);
+    it('should update on drop on flow', inject(
-      expect(sequenceFlow.businessObject.di.targetElement).to.equal(event.businessObject.di);
+      function(modeling, elementRegistry, elementFactory) {
+
+        // given
+        // sequence flow with existing sourceElement and targetElement di information
+        var sequenceFlow = elementRegistry.get('SequenceFlow_3'),
+            startEventDi = elementRegistry.get('StartEvent_2').businessObject.di;
+
+        var intermediateThrowEvent = elementFactory.createShape({
+          type: 'bpmn:IntermediateThrowEvent'
+        });
+
+        var dropPosition = { x: 320, y: 260 };
+
+        // when
+        var event = modeling.createShape(intermediateThrowEvent, dropPosition, sequenceFlow);
+
+        // then
+        expect(sequenceFlow.businessObject.di.sourceElement).to.equal(startEventDi);
+        expect(sequenceFlow.businessObject.di.targetElement).to.equal(event.businessObject.di);
+      }
+    ));
+
+
+    it('should not create new di refs', inject(
+      function(modeling, elementRegistry, elementFactory) {
+
+        // given
+        // sequence flow without any sourceElement and targetElement di information
+        var sequenceFlow = elementRegistry.get('SequenceFlow_4');
+
+        var intermediateThrowEvent = elementFactory.createShape({
+          type: 'bpmn:IntermediateThrowEvent'
+        });
+
+        var dropPosition = { x: 320, y: 260 };
+
+        // when
+        modeling.createShape(intermediateThrowEvent, dropPosition, sequenceFlow);
+
+        // then
+        expect(sequenceFlow.businessObject.di.sourceElement).not.to.exist;
+        expect(sequenceFlow.businessObject.di.targetElement).not.to.exist;
+      }
+    ));
+
+  });
+
+
+  describe('missing bpmndi:Bounds', function() {
+
+    var diagramXML = require('./BpmnUpdater.missingBounds.bpmn');
+
+    beforeEach(bootstrapModeler(diagramXML, {
+      modules: testModules
     }));
 
 
-    it('should not create new di refs', inject(function(modeling, elementRegistry, elementFactory) {
+    it('should add bpmndi:Bounds', inject(
+      function(modeling, elementRegistry) {
 
-      // given
+        // given
-      // sequence flow without any sourceElement and targetElement di information
+        var event = elementRegistry.get('StartEvent'),
-      var sequenceFlow = elementRegistry.get('SequenceFlow_4');
+            label = event.label,
+            di = event.businessObject.di;
 
-      var intermediateThrowEvent = elementFactory.createShape({ type: 'bpmn:IntermediateThrowEvent' }),
+        // when
-          dropPosition = { x: 320, y: 260 };
+        modeling.moveElements([ label ], { x: 20, y: 20 });
 
-      // when
+        var labelBounds = di.label.bounds;
-      modeling.createShape(intermediateThrowEvent, dropPosition, sequenceFlow);
 
-      // then
+        // then
-      expect(sequenceFlow.businessObject.di.sourceElement).not.to.exist;
+        expect(labelBounds).to.exist;
-      expect(sequenceFlow.businessObject.di.targetElement).not.to.exist;
+
-    }));
+        expect(labelBounds).to.include.keys(
+          'x', 'y',
+          'width', 'height'
+        );
+      }
+    ));
 
   });