From cd24b27768bfd97f3cc1411ee19dd4718146ffb2 Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Thu, 7 Dec 2017 23:12:00 +0100 Subject: [PATCH] fix(copy-paste): ignore data associations during cloning * use bpmnFactory for cloning to ensure all relevant elements have actual IDs * don't copy dataAssociations, as they are visual elements that will be created during element re-connection NOTE: This fixes data input association not properly being wired during target replace, too. Closes #694, #693 --- lib/features/copy-paste/BpmnCopyPaste.js | 2 +- lib/features/replace/BpmnReplace.js | 2 +- lib/util/model/ModelCloneHelper.js | 13 ++-- lib/util/model/ModelCloneUtils.js | 4 +- .../features/copy-paste/BpmnCopyPasteSpec.js | 64 +++++++++++-------- .../replace/BpmnReplace.dataObjects.bpmn | 46 +++++++++++++ test/spec/features/replace/BpmnReplaceSpec.js | 42 ++++++++++++ test/spec/util/ModelCloneHelperSpec.js | 12 +++- 8 files changed, 147 insertions(+), 38 deletions(-) create mode 100644 test/spec/features/replace/BpmnReplace.dataObjects.bpmn diff --git a/lib/features/copy-paste/BpmnCopyPaste.js b/lib/features/copy-paste/BpmnCopyPaste.js index d12ff3b7..fc2f8522 100644 --- a/lib/features/copy-paste/BpmnCopyPaste.js +++ b/lib/features/copy-paste/BpmnCopyPaste.js @@ -34,7 +34,7 @@ function BpmnCopyPaste( bpmnFactory, eventBus, copyPaste, clipboard, canvas, bpmnRules) { - var helper = new ModelCloneHelper(eventBus); + var helper = new ModelCloneHelper(eventBus, bpmnFactory); copyPaste.registerDescriptor(function(element, descriptor) { var businessObject = descriptor.oldBusinessObject = getBusinessObject(element); diff --git a/lib/features/replace/BpmnReplace.js b/lib/features/replace/BpmnReplace.js index 5b252fa7..7b01f04a 100644 --- a/lib/features/replace/BpmnReplace.js +++ b/lib/features/replace/BpmnReplace.js @@ -56,7 +56,7 @@ function toggeling(element, target) { */ function BpmnReplace(bpmnFactory, replace, selection, modeling, eventBus) { - var helper = new ModelCloneHelper(eventBus); + var helper = new ModelCloneHelper(eventBus, bpmnFactory); /** * Prepares a new business object for the replacement element diff --git a/lib/util/model/ModelCloneHelper.js b/lib/util/model/ModelCloneHelper.js index 706ecf2e..81a3e975 100644 --- a/lib/util/model/ModelCloneHelper.js +++ b/lib/util/model/ModelCloneHelper.js @@ -29,8 +29,9 @@ function isType(element, types) { * A bpmn properties cloning interface * */ -function ModelCloneHelper(eventBus) { +function ModelCloneHelper(eventBus, bpmnFactory) { this._eventBus = eventBus; + this._bpmnFactory = bpmnFactory; } module.exports = ModelCloneHelper; @@ -60,8 +61,11 @@ ModelCloneHelper.prototype.clone = function(refElement, newElement, properties) // - same values from simple types // - cloning id's // - cloning reference elements - if (newElementProp === refElementProp || - (propDescriptor && (propDescriptor.isId || propDescriptor.isReference))) { + if (newElementProp === refElementProp) { + return; + } + + if (propDescriptor && (propDescriptor.isId || propDescriptor.isReference)) { return; } @@ -110,8 +114,9 @@ ModelCloneHelper.prototype.clone = function(refElement, newElement, properties) ModelCloneHelper.prototype._deepClone = function _deepClone(propertyElement, context) { var eventBus = this._eventBus; + var bpmnFactory = this._bpmnFactory; - var newProp = propertyElement.$model.create(propertyElement.$type); + var newProp = bpmnFactory.create(propertyElement.$type); var properties = filter(Object.keys(propertyElement), function(prop) { var descriptor = newProp.$model.getPropertyDescriptor(newProp, prop); diff --git a/lib/util/model/ModelCloneUtils.js b/lib/util/model/ModelCloneUtils.js index b658c4c7..9098817f 100644 --- a/lib/util/model/ModelCloneUtils.js +++ b/lib/util/model/ModelCloneUtils.js @@ -13,7 +13,9 @@ module.exports.IGNORED_PROPERTIES = [ 'outgoing', 'artifacts', 'default', - 'flowElements' + 'flowElements', + 'dataInputAssociations', + 'dataOutputAssociations' ]; diff --git a/test/spec/features/copy-paste/BpmnCopyPasteSpec.js b/test/spec/features/copy-paste/BpmnCopyPasteSpec.js index 7eedd566..6ed23d78 100644 --- a/test/spec/features/copy-paste/BpmnCopyPasteSpec.js +++ b/test/spec/features/copy-paste/BpmnCopyPasteSpec.js @@ -523,9 +523,15 @@ describe('features/copy-paste', function() { describe('integration', function() { - it('multiple participants', inject(integrationTest([ 'Participant_0pgdgt4', 'Participant_1id96b4' ]))); + it('multiple participants', inject(integrationTest([ + 'Participant_0pgdgt4', + 'Participant_1id96b4' + ]))); - it('multiple participants', inject(integrationTest([ 'Participant_0pgdgt4', 'Participant_1id96b4' ]))); + it('multiple participants', inject(integrationTest([ + 'Participant_0pgdgt4', + 'Participant_1id96b4' + ]))); }); @@ -537,42 +543,44 @@ describe('features/copy-paste', function() { beforeEach(bootstrapModeler(collaborationAssociations, { modules: testModules })); - it('copying participant should copy process as well', inject(function(elementRegistry, copyPaste, canvas) { + it('copying participant should copy process as well', inject( + function(elementRegistry, copyPaste, canvas) { - // given - var participants = map([ 'Participant_Input', 'Participant_Output' ], function(e) { - return elementRegistry.get(e); - }); - var rootElement = canvas.getRootElement(); + // given + var participants = map([ 'Participant_Input', 'Participant_Output' ], function(e) { + return elementRegistry.get(e); + }); + var rootElement = canvas.getRootElement(); - // when - copyPaste.copy(participants); + // when + copyPaste.copy(participants); - copyPaste.paste({ - element: rootElement, - point: { - x: 4000, - y: 4500 - } - }); + copyPaste.paste({ + element: rootElement, + point: { + x: 4000, + y: 4500 + } + }); - // then - var elements = elementRegistry.filter(function(element) { - return element.type === 'bpmn:Participant'; - }); + // then + var elements = elementRegistry.filter(function(element) { + return element.type === 'bpmn:Participant'; + }); - var processIds = map(elements, function(e) { - return e.businessObject.processRef.id; - }); + var processIds = map(elements, function(e) { + return e.businessObject.processRef.id; + }); - expect(uniq(processIds)).to.have.length(4); - })); + expect(uniq(processIds)).to.have.length(4); + } + )); - it('participant with OutputDataAssociation', inject(integrationTest([ 'Participant_Output' ]))); + it('participant with DataOutputAssociation', inject(integrationTest([ 'Participant_Output' ]))); - it('participant with InputDataAssociation', inject(integrationTest([ 'Participant_Input' ]))); + it('participant with DataInputAssociation', inject(integrationTest([ 'Participant_Input' ]))); }); diff --git a/test/spec/features/replace/BpmnReplace.dataObjects.bpmn b/test/spec/features/replace/BpmnReplace.dataObjects.bpmn new file mode 100644 index 00000000..5d3e77f0 --- /dev/null +++ b/test/spec/features/replace/BpmnReplace.dataObjects.bpmn @@ -0,0 +1,46 @@ + + + + + + + DataObjectReference_IN + TaskPlaceholderProperty + + + DataObjectReference_OUT + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/spec/features/replace/BpmnReplaceSpec.js b/test/spec/features/replace/BpmnReplaceSpec.js index 6b192d07..7aca37f9 100644 --- a/test/spec/features/replace/BpmnReplaceSpec.js +++ b/test/spec/features/replace/BpmnReplaceSpec.js @@ -236,6 +236,48 @@ describe('features/replace - bpmn replace', function() { }); + describe('should replace with data objects', function() { + + var diagramXML = require('./BpmnReplace.dataObjects.bpmn'); + + beforeEach(bootstrapModeler(diagramXML, { + modules: testModules + })); + + + it('restoring dataAssociations', inject(function(elementRegistry, bpmnReplace) { + + // given + var task = elementRegistry.get('Task'); + + // when + var serviceTask = bpmnReplace.replaceElement(task, { type: 'bpmn:ServiceTask' }); + var bo = serviceTask.businessObject; + + // then + // expect one incoming connection + expect(serviceTask.incoming).to.have.length(1); + + var inputAssociations = bo.dataInputAssociations; + expect(inputAssociations).to.have.length(1); + + var inputAssociation = inputAssociations[0]; + + // expect input association references __target_ref_placeholder property + expect(inputAssociation.targetRef).to.equal(bo.properties[0]); + + // ...and + // expect one outgoing connection + expect(serviceTask.outgoing).to.have.length(1); + + var outputAssociations = bo.dataOutputAssociations; + expect(outputAssociations).to.have.length(1); + + })); + + }); + + describe('position and size', function() { var diagramXML = require('../../../fixtures/bpmn/features/replace/01_replace.bpmn'); diff --git a/test/spec/util/ModelCloneHelperSpec.js b/test/spec/util/ModelCloneHelperSpec.js index 5f7c613d..9a4b6adf 100644 --- a/test/spec/util/ModelCloneHelperSpec.js +++ b/test/spec/util/ModelCloneHelperSpec.js @@ -5,6 +5,7 @@ require('../../TestHelper'); /* global bootstrapModeler, inject */ var coreModule = require('../../../lib/core'); +var modelingModule = require('../../../lib/features/modeling'); var ModelCloneHelper = require('../../../lib/util/model/ModelCloneHelper'); @@ -18,7 +19,11 @@ function getProp(element, property) { describe('util/ModelCloneHelper', function() { - var testModules = [ camundaModdleModule, coreModule ]; + var testModules = [ + camundaModdleModule, + coreModule, + modelingModule + ]; var basicXML = require('../../fixtures/bpmn/basic.bpmn'); @@ -31,8 +36,8 @@ describe('util/ModelCloneHelper', function() { var helper; - beforeEach(inject(function(eventBus) { - helper = new ModelCloneHelper(eventBus); + beforeEach(inject(function(eventBus, bpmnFactory) { + helper = new ModelCloneHelper(eventBus, bpmnFactory); })); describe('simple', function() { @@ -64,6 +69,7 @@ describe('util/ModelCloneHelper', function() { }); + describe('nested', function() { it('should pass nested property - documentation', inject(function(moddle) {