fix(modeling): correctly update extension attributes

Closes #301

lib/features/modeling/cmd/UpdatePropertiesHandler.js

@@ -1,8 +1,8 @@
 'use strict';
 
-var assign = require('lodash/object/assign'),
-    pick = require('lodash/object/pick'),
-    keys = require('lodash/object/keys');
+var reduce = require('lodash/object/transform'),
+    keys = require('lodash/object/keys'),
+    forEach = require('lodash/collection/forEach');
 
 var DEFAULT_FLOW = 'default',
     NAME = 'name',
@@ -27,6 +27,21 @@ UpdatePropertiesHandler.$inject = [ 'elementRegistry' ];
 module.exports = UpdatePropertiesHandler;
 
 
+function getProperties(businessObject, propertyNames) {
+  return reduce(propertyNames, function(result, key) {
+    result[key] = businessObject.get(key);
+    return result;
+  }, {});
+}
+
+
+function setProperties(businessObject, properties) {
+  forEach(properties, function(value, key) {
+    businessObject.set(key, value);
+  });
+}
+
+
 ////// api /////////////////////////////////////////////
 
 /**
@@ -52,7 +67,7 @@ UpdatePropertiesHandler.prototype.execute = function(context) {
 
   var businessObject = element.businessObject,
       properties = context.properties,
-      oldProperties = context.oldProperties || pick(businessObject, keys(properties));
+      oldProperties = context.oldProperties || getProperties(businessObject, keys(properties));
 
   if (ID in properties) {
     elementRegistry.updateId(element, properties[ID]);
@@ -75,8 +90,7 @@ UpdatePropertiesHandler.prototype.execute = function(context) {
   }
 
   // update properties
-  assign(businessObject, properties);
-
+  setProperties(businessObject, properties);
 
   // store old values
   context.oldProperties = oldProperties;
@@ -100,7 +114,8 @@ UpdatePropertiesHandler.prototype.revert = function(context) {
       businessObject = element.businessObject,
       elementRegistry = this._elementRegistry;
 
-  assign(businessObject, context.oldProperties);
+  // update properties
+  setProperties(businessObject, oldProperties);
 
   if (ID in oldProperties) {
     elementRegistry.updateId(element, oldProperties[ID]);

test/spec/features/modeling/UpdatePropertiesSpec.js

@@ -70,7 +70,7 @@ describe('features/modeling - update properties', function() {
     }));
 
 
-    it('updating label', inject(function(elementRegistry, modeling) {
+    it('updating name', inject(function(elementRegistry, modeling) {
 
       // given
       var flowConnection = elementRegistry.get('SequenceFlow_1');
@@ -86,6 +86,19 @@ describe('features/modeling - update properties', function() {
     }));
 
 
+    it('unsetting name', inject(function(elementRegistry, modeling) {
+
+      // given
+      var flowConnection = elementRegistry.get('SequenceFlow_3');
+
+      // when
+      modeling.updateProperties(flowConnection, { name: undefined });
+
+      // then
+      expect(flowConnection.businessObject.name).not.toBeDefined();
+    }));
+
+
     it('updating id', inject(function(elementRegistry, modeling) {
 
       // given
@@ -99,6 +112,23 @@ describe('features/modeling - update properties', function() {
       expect(flowConnection.id).toBe('FOO_BAR');
     }));
 
+
+    it('updating extension elements', inject(function(elementRegistry, modeling) {
+
+      // given
+      var flowConnection = elementRegistry.get('SequenceFlow_1');
+
+      // when
+      modeling.updateProperties(flowConnection, {
+        'xmlns:foo': 'http://foo',
+        'foo:customAttr': 'FOO'
+      });
+
+      // then
+      expect(flowConnection.businessObject.get('xmlns:foo')).toBe('http://foo');
+      expect(flowConnection.businessObject.get('foo:customAttr')).toBe('FOO');
+    }));
+
   });
 
 
@@ -154,6 +184,21 @@ describe('features/modeling - update properties', function() {
     }));
 
 
+    it('unsetting name', inject(function(elementRegistry, commandStack, modeling) {
+
+      // given
+      var flowConnection = elementRegistry.get('SequenceFlow_3');
+
+      modeling.updateProperties(flowConnection, { name: undefined });
+
+      // when
+      commandStack.undo();
+
+      // then
+      expect(flowConnection.businessObject.name).toBe('conditional');
+    }));
+
+
     it('updating id', inject(function(elementRegistry, commandStack, modeling) {
 
       // given
@@ -168,6 +213,25 @@ describe('features/modeling - update properties', function() {
       expect(flowConnection.id).toBe('SequenceFlow_1');
     }));
 
+
+    it('updating extension elements', inject(function(elementRegistry, commandStack, modeling) {
+
+      // given
+      var flowConnection = elementRegistry.get('SequenceFlow_1');
+
+      modeling.updateProperties(flowConnection, {
+        'xmlns:foo': 'http://foo',
+        'foo:customAttr': 'FOO'
+      });
+
+      // when
+      commandStack.undo();
+
+      // then
+      expect(flowConnection.businessObject.get('xmlns:foo')).toBe(undefined);
+      expect(flowConnection.businessObject.get('foo:customAttr')).toBe(undefined);
+    }));
+
   });
 
 
@@ -225,6 +289,22 @@ describe('features/modeling - update properties', function() {
       expect(updatedElements).toContain(elementRegistry.get('SequenceFlow_1_label'));
     }));
 
+
+    it('unsetting name', inject(function(elementRegistry, commandStack, modeling) {
+
+      // given
+      var flowConnection = elementRegistry.get('SequenceFlow_3');
+
+      modeling.updateProperties(flowConnection, { name: undefined });
+
+      // when
+      commandStack.undo();
+      commandStack.redo();
+
+      // then
+      expect(flowConnection.businessObject.name).not.toBeDefined();
+    }));
+
   });
 
 });