From 6da1a0cb63de42ea5c8d289a399b78b12284c543 Mon Sep 17 00:00:00 2001
From: Philipp Fromme <philipp.fromme@camunda.com>
Date: Tue, 7 Jun 2016 16:45:47 +0200
Subject: [PATCH] feat(replace): keep element ID on replace

Closes #513
---
 .../behavior/ReplaceElementBehaviour.js       | 12 +++++++---
 .../behavior/ReplaceElementBehaviourSpec.js   | 22 +++++++------------
 test/spec/features/replace/BpmnReplaceSpec.js |  4 +++-
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/lib/features/modeling/behavior/ReplaceElementBehaviour.js b/lib/features/modeling/behavior/ReplaceElementBehaviour.js
index 5d54d0c8..21b7478d 100644
--- a/lib/features/modeling/behavior/ReplaceElementBehaviour.js
+++ b/lib/features/modeling/behavior/ReplaceElementBehaviour.js
@@ -57,9 +57,6 @@ function ReplaceElementBehaviour(eventBus, bpmnReplace, bpmnRules, elementRegist
         attachers = oldShape.attachers,
         canReplace;
 
-    modeling.unclaimId(oldShape.businessObject.id, oldShape.businessObject);
-    modeling.claimId(newShape.businessObject.id, newShape.businessObject);
-
     if (attachers && attachers.length) {
       canReplace = bpmnRules.canReplace(attachers, newShape);
 
@@ -67,6 +64,15 @@ function ReplaceElementBehaviour(eventBus, bpmnReplace, bpmnRules, elementRegist
     }
 
   }, this);
+
+  this.postExecuted( [ 'shape.replace' ], 1500, function(e) {
+    var context = e.context,
+        oldShape = context.oldShape,
+        newShape = context.newShape;
+
+    modeling.unclaimId(oldShape.businessObject.id, oldShape.businessObject);
+    modeling.updateProperties(newShape, { id: oldShape.id });
+  });
 }
 
 inherits(ReplaceElementBehaviour, CommandInterceptor);
diff --git a/test/spec/features/modeling/behavior/ReplaceElementBehaviourSpec.js b/test/spec/features/modeling/behavior/ReplaceElementBehaviourSpec.js
index 940a655e..329b4b68 100644
--- a/test/spec/features/modeling/behavior/ReplaceElementBehaviourSpec.js
+++ b/test/spec/features/modeling/behavior/ReplaceElementBehaviourSpec.js
@@ -31,7 +31,6 @@ describe('features/modeling - move start event behavior', function() {
       modules: testModules
     }));
 
-
     var moveShape;
 
     beforeEach(inject(function(move, dragging, elementRegistry) {
@@ -63,11 +62,7 @@ describe('features/modeling - move start event behavior', function() {
 
         dragging.end();
 
-        var replacement = elementRegistry.filter(function(element) {
-          if (is(element, 'bpmn:StartEvent') && element.parent === rootElement) {
-            return true;
-          }
-        })[0];
+        var replacement = elementRegistry.get('StartEvent_1');
 
         // then
         expect(selection.get()).to.include(replacement);
@@ -112,15 +107,13 @@ describe('features/modeling - move start event behavior', function() {
 
     var diagramXML = require('../../../../fixtures/bpmn/features/replace/cancel-events.bpmn');
 
-    beforeEach(bootstrapModeler(diagramXML, {
-      modules: testModules
-    }));
+    beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
 
 
     describe('normal', function() {
 
 
-      it('should unclaim old element ID and claim new ID',
+      it('should unclaim ID and claim same ID with new element',
         inject(function(elementRegistry, bpmnReplace) {
 
           // given
@@ -132,13 +125,14 @@ describe('features/modeling - move start event behavior', function() {
           var subProcess = bpmnReplace.replaceElement(transaction, { type: 'bpmn:SubProcess' });
 
           // then
-          expect(ids.assigned(transaction.id)).to.be.false;
+          expect(ids.assigned(transaction.id)).to.eql(subProcess.businessObject);
           expect(ids.assigned(subProcess.id)).to.eql(subProcess.businessObject);
+          expect(subProcess.id).to.eql(transaction.id);
         })
       );
 
 
-      it('should REVERT unclaim old element ID and claim new ID on UNDO',
+      it('should REVERT unclaim ID and claim same ID with new element on UNDO',
         inject(function(elementRegistry, bpmnReplace, commandStack) {
 
           // given
@@ -153,7 +147,7 @@ describe('features/modeling - move start event behavior', function() {
 
           // then
           expect(ids.assigned(transaction.id)).to.eql(transaction.businessObject);
-          expect(ids.assigned(subProcess.id)).to.be.false;
+          expect(subProcess.id).not.to.equal(transaction.id);
         })
       );
 
@@ -290,7 +284,7 @@ describe('features/modeling - move start event behavior', function() {
           // then
           expect(newBoundaryEvent.eventDefinitionTypes).to.not.exist;
           expect(newBoundaryEvent.attachedToRef).to.equal(subProcess.businessObject);
-          expect(elementRegistry.get('Transaction_1')).to.not.exist;
+          expect(elementRegistry.get('Transaction_1')).to.eql(subProcess);
         })
       );
 
diff --git a/test/spec/features/replace/BpmnReplaceSpec.js b/test/spec/features/replace/BpmnReplaceSpec.js
index 0188dfe0..12aa5443 100644
--- a/test/spec/features/replace/BpmnReplaceSpec.js
+++ b/test/spec/features/replace/BpmnReplaceSpec.js
@@ -1007,7 +1007,9 @@ describe('features/replace - bpmn replace', function() {
         modeling.moveElements([interruptingStartEvent], { x: 0, y: 200 }, root);
 
         var startEventAfter = elementRegistry.filter(function(element) {
-          return is(element, 'bpmn:StartEvent') && element.parent === root;
+          return is(element, 'bpmn:StartEvent')
+            && element.type !== 'label'
+            && element.parent === root;
         })[0];
 
         // then