From 82eccbbfb05918aa48a342d214027d0ab2e2a525 Mon Sep 17 00:00:00 2001 From: Abdul Ahad Date: Mon, 26 Aug 2024 12:04:58 +0200 Subject: [PATCH 1/4] fix: variable name changes when element name/label changes Closes #863 --- .../src/features/modeling/index.js | 5 +- .../behavior/NameChangeBehaviorSpec.js | 40 +++++++++ .../behavior/name-change-behavior.dmn | 31 +++++++ .../src/features/modeling/behavior/index.js | 5 +- .../modeling/cmd/UpdatePropertiesHandler.js | 34 ++++---- .../src/features/replace/DrdReplace.js | 5 +- .../behavior/NameChangeBehaviorSpec.js | 37 ++++++++ .../behavior/name-change-behavior.dmn | 28 ++++++ .../src/features/modeling/Modeling.js | 9 ++ .../src/features/modeling/index.js | 6 +- .../behavior/NameChangeBehaviorSpec.js | 37 ++++++++ .../modeling/behavior/NameChangeBehavior.js | 87 +++++++++++++++++++ 12 files changed, 301 insertions(+), 23 deletions(-) create mode 100644 packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js create mode 100644 packages/dmn-js-decision-table/test/spec/features/modeling/behavior/name-change-behavior.dmn create mode 100644 packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js create mode 100644 packages/dmn-js-drd/test/spec/features/modeling/behavior/name-change-behavior.dmn create mode 100644 packages/dmn-js-literal-expression/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js create mode 100644 packages/dmn-js-shared/src/features/modeling/behavior/NameChangeBehavior.js diff --git a/packages/dmn-js-decision-table/src/features/modeling/index.js b/packages/dmn-js-decision-table/src/features/modeling/index.js index 7ad1bc6b3..a52913f00 100644 --- a/packages/dmn-js-decision-table/src/features/modeling/index.js +++ b/packages/dmn-js-decision-table/src/features/modeling/index.js @@ -4,15 +4,18 @@ import DmnFactory from './DmnFactory'; import ElementFactory from './ElementFactory'; import IdChangeBehavior from 'dmn-js-shared/lib/features/modeling/behavior/IdChangeBehavior'; +import NameChangeBehavior from + 'dmn-js-shared/lib/features/modeling/behavior/NameChangeBehavior'; import Modeling from './Modeling'; import Behavior from './behavior'; export default { - __init__: [ 'dmnUpdater', 'idChangeBehavior', 'modeling' ], + __init__: [ 'dmnUpdater', 'idChangeBehavior', 'nameChangeBehavior', 'modeling' ], __depends__: [ Behavior, CommandStack ], dmnUpdater: [ 'type', DmnUpdater ], dmnFactory: [ 'type', DmnFactory ], elementFactory: [ 'type', ElementFactory ], idChangeBehavior: [ 'type', IdChangeBehavior ], + nameChangeBehavior: [ 'type', NameChangeBehavior ], modeling: [ 'type', Modeling ] }; \ No newline at end of file diff --git a/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js b/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js new file mode 100644 index 000000000..8bd5be6a4 --- /dev/null +++ b/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js @@ -0,0 +1,40 @@ +import { bootstrapModeler, inject } from 'test/helper'; + +import decisionTableXML from './name-change-behavior.dmn'; + +import CoreModule from 'src/core'; +import Modeling from 'src/features/modeling'; + + +describe('NameChangeBehavior', function() { + + describe('with existing variable', function() { + + beforeEach(bootstrapModeler(decisionTableXML, { + modules: [ + CoreModule, + Modeling + ], + })); + + + it('should update variable name when element name is changed', inject( + function(modeling, sheet) { + + // given + const root = sheet.getRoot(), + decisionTable = root.businessObject; + + const decision = decisionTable.$parent; + + // when + modeling.editDecisionTableName('foo'); + + // then + const variable = decision.get('variable'); + + expect(variable.get('name')).to.equal('foo'); + } + )); + }); +}); diff --git a/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/name-change-behavior.dmn b/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/name-change-behavior.dmn new file mode 100644 index 000000000..a8a4d73da --- /dev/null +++ b/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/name-change-behavior.dmn @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/dmn-js-drd/src/features/modeling/behavior/index.js b/packages/dmn-js-drd/src/features/modeling/behavior/index.js index 5857aba71..40de7c033 100644 --- a/packages/dmn-js-drd/src/features/modeling/behavior/index.js +++ b/packages/dmn-js-drd/src/features/modeling/behavior/index.js @@ -4,17 +4,20 @@ import ReplaceConnectionBehavior from './ReplaceConnectionBehavior'; import ReplaceElementBehavior from './ReplaceElementBehavior'; import IdChangeBehavior from 'dmn-js-shared/lib/features/modeling/behavior/IdChangeBehavior'; - +import NameChangeBehavior from + 'dmn-js-shared/lib/features/modeling/behavior/NameChangeBehavior'; export default { __init__: [ 'createConnectionBehavior', 'idChangeBehavior', + 'nameChangeBehavior', 'layoutConnectionBehavior', 'replaceConnectionBehavior', 'replaceElementBehavior' ], createConnectionBehavior: [ 'type', CreateConnectionBehavior ], idChangeBehavior: [ 'type', IdChangeBehavior ], + nameChangeBehavior: [ 'type', NameChangeBehavior ], layoutConnectionBehavior: [ 'type', LayoutConnectionBehavior ], replaceConnectionBehavior: [ 'type', ReplaceConnectionBehavior ], replaceElementBehavior: [ 'type', ReplaceElementBehavior ] diff --git a/packages/dmn-js-drd/src/features/modeling/cmd/UpdatePropertiesHandler.js b/packages/dmn-js-drd/src/features/modeling/cmd/UpdatePropertiesHandler.js index 70ea76711..fd85e2a27 100644 --- a/packages/dmn-js-drd/src/features/modeling/cmd/UpdatePropertiesHandler.js +++ b/packages/dmn-js-drd/src/features/modeling/cmd/UpdatePropertiesHandler.js @@ -4,8 +4,10 @@ import { forEach } from 'min-dash'; -var NAME = 'name', - ID = 'id'; +import { getBusinessObject } from 'dmn-js-shared/lib/util/ModelUtil'; + +const NAME = 'name', + ID = 'id'; /** @@ -37,22 +39,21 @@ UpdatePropertiesHandler.$inject = [ 'elementRegistry', 'moddle' ]; */ UpdatePropertiesHandler.prototype.execute = function(context) { - var element = context.element, - changed = [ element ]; + const { element, properties } = context, + changed = [ element ]; if (!element) { throw new Error('element required'); } - var elementRegistry = this._elementRegistry, - ids = this._moddle.ids; + const elementRegistry = this._elementRegistry, + ids = this._moddle.ids; - var businessObject = element.businessObject, - properties = context.properties, - oldProperties = ( - context.oldProperties || + const businessObject = getBusinessObject(element), + oldProperties = ( + context.oldProperties || getProperties(businessObject, keys(properties)) - ); + ); if (isIdChange(properties, businessObject)) { ids.unclaim(businessObject[ID]); @@ -86,13 +87,10 @@ UpdatePropertiesHandler.prototype.execute = function(context) { * @return {djs.model.Base} the updated element */ UpdatePropertiesHandler.prototype.revert = function(context) { - - var element = context.element, - properties = context.properties, - oldProperties = context.oldProperties, - businessObject = element.businessObject, - elementRegistry = this._elementRegistry, - ids = this._moddle.ids; + const { element, properties, oldProperties } = context; + const businessObject = getBusinessObject(element); + const elementRegistry = this._elementRegistry, + ids = this._moddle.ids; // update properties setProperties(businessObject, oldProperties); diff --git a/packages/dmn-js-drd/src/features/replace/DrdReplace.js b/packages/dmn-js-drd/src/features/replace/DrdReplace.js index 0540db471..e1e8bc67e 100644 --- a/packages/dmn-js-drd/src/features/replace/DrdReplace.js +++ b/packages/dmn-js-drd/src/features/replace/DrdReplace.js @@ -59,8 +59,11 @@ export default function DrdReplace(drdFactory, replace, selection, modeling) { } if (target.expression) { + + // variable set to element name var literalExpression = drdFactory.create('dmn:LiteralExpression'), - variable = drdFactory.create('dmn:InformationItem'); + variable = drdFactory.create('dmn:InformationItem', + { name: oldBusinessObject.name }); setBoxedExpression(newBusinessObject, literalExpression, drdFactory, variable); } diff --git a/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js b/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js new file mode 100644 index 000000000..1da32df3e --- /dev/null +++ b/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js @@ -0,0 +1,37 @@ +import { bootstrapModeler, inject } from 'test/helper'; + +import simpleStringEditXML from './name-change-behavior.dmn'; + +import CoreModule from 'src/core'; +import Modeling from 'src/features/modeling'; + + +describe('NameChangeBehavior', function() { + + describe('with label change', function() { + + beforeEach(bootstrapModeler(simpleStringEditXML, { + modules: [ + CoreModule, + Modeling + ], + })); + + + it('should update variable name when label is changed', inject( + function(modeling, elementRegistry) { + + // given + const decision = elementRegistry.get('season'), + bo = decision.businessObject, + variable = bo.variable; + + // when + modeling.updateLabel(decision,'foo'); + + // then + expect(variable.get('name')).to.equal('foo'); + } + )); + }); +}); diff --git a/packages/dmn-js-drd/test/spec/features/modeling/behavior/name-change-behavior.dmn b/packages/dmn-js-drd/test/spec/features/modeling/behavior/name-change-behavior.dmn new file mode 100644 index 000000000..bdf8de402 --- /dev/null +++ b/packages/dmn-js-drd/test/spec/features/modeling/behavior/name-change-behavior.dmn @@ -0,0 +1,28 @@ + + + + + + + + + calendar.getSeason(date) + + + + + + + + + + + + + + + + + + + diff --git a/packages/dmn-js-literal-expression/src/features/modeling/Modeling.js b/packages/dmn-js-literal-expression/src/features/modeling/Modeling.js index 58d922f86..519ca5b33 100644 --- a/packages/dmn-js-literal-expression/src/features/modeling/Modeling.js +++ b/packages/dmn-js-literal-expression/src/features/modeling/Modeling.js @@ -115,6 +115,15 @@ export default class Modeling { this._commandStack.execute('element.updateProperties', context); } + + updateProperties(el, props) { + const context = { + element: el, + properties: props + }; + + this._commandStack.execute('element.updateProperties', context); + } } Modeling.$inject = [ 'commandStack', 'viewer', 'eventBus' ]; diff --git a/packages/dmn-js-literal-expression/src/features/modeling/index.js b/packages/dmn-js-literal-expression/src/features/modeling/index.js index dec3ff4ce..959fe535c 100644 --- a/packages/dmn-js-literal-expression/src/features/modeling/index.js +++ b/packages/dmn-js-literal-expression/src/features/modeling/index.js @@ -1,12 +1,14 @@ import CommandStack from 'diagram-js/lib/command/CommandStack'; import IdChangeBehavior from 'dmn-js-shared/lib/features/modeling/behavior/IdChangeBehavior'; - +import NameChangeBehavior from + 'dmn-js-shared/lib/features/modeling/behavior/NameChangeBehavior'; import Modeling from './Modeling'; export default { - __init__: [ 'idChangeBehavior', 'modeling' ], + __init__: [ 'idChangeBehavior', 'nameChangeBehavior', 'modeling' ], commandStack: [ 'type', CommandStack ], idChangeBehavior: [ 'type', IdChangeBehavior ], + nameChangeBehavior: [ 'type', NameChangeBehavior ], modeling: [ 'type', Modeling ] }; \ No newline at end of file diff --git a/packages/dmn-js-literal-expression/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js b/packages/dmn-js-literal-expression/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js new file mode 100644 index 000000000..0d0126a25 --- /dev/null +++ b/packages/dmn-js-literal-expression/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js @@ -0,0 +1,37 @@ +import { bootstrapModeler, inject } from 'test/helper'; + +import simpleStringEditXML from '../../../literal-expression.dmn'; + +import CoreModule from 'src/core'; +import Modeling from 'src/features/modeling'; + + +describe('NameChangeBehavior', function() { + + describe('with existing variable', function() { + + beforeEach(bootstrapModeler(simpleStringEditXML, { + modules: [ + CoreModule, + Modeling + ], + })); + + + it('should update variable name when element name is changed', inject( + function(modeling, viewer) { + + // given + const decision = viewer.getDecision(); + + // when + modeling.editDecisionName('foo'); + + // then + const variable = decision.get('variable'); + + expect(variable.get('name')).to.equal('foo'); + } + )); + }); +}); diff --git a/packages/dmn-js-shared/src/features/modeling/behavior/NameChangeBehavior.js b/packages/dmn-js-shared/src/features/modeling/behavior/NameChangeBehavior.js new file mode 100644 index 000000000..b1a277b1c --- /dev/null +++ b/packages/dmn-js-shared/src/features/modeling/behavior/NameChangeBehavior.js @@ -0,0 +1,87 @@ +import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor'; + +import { + getBusinessObject, + is +} from 'dmn-js-shared/lib/util/ModelUtil'; + + +export default class NameChangeBehavior extends CommandInterceptor { + + static $inject = [ 'eventBus', 'modeling' ]; + + constructor(eventBus, modeling) { + super(eventBus); + + this._modeling = modeling; + + this.postExecuted('element.updateProperties', this.updateVariableFromElement); + this.postExecuted('element.updateLabel', this.updateVariableFromLabel); + } + + updateVariableFromLabel = ({ context }) => { + const { element, newLabel } = context; + const bo = getBusinessObject(element), + variable = bo.variable; + + if (!variable) { + return; + } + + this._modeling.updateProperties(variable, { name: newLabel }); + }; + + updateVariableFromElement = ({ context }) => { + const { element, properties } = context; + const bo = getBusinessObject(element); + + if (!bo.variable) { + return; + } + + if (!(is(element, 'dmn:Decision') || is(element, 'dmn:BusinessKnowledgeModel'))) { + return; + } + + if (!this.isNameChanged(properties)) { + return; + } + + if (this.isVariable(element)) { + return; + } + + else if (!this.isElementVariable(element)) { + this.syncElementVariableChange(bo); + } + }; + + isNameChanged(properties) { + return 'name' in properties; + } + + isVariable(element) { + const parent = getParent(element); + return ( + is(element, 'dmn:InformationItem') && + parent && parent.get('variable') === element + ); + } + + isElementVariable(element) { + const variable = element.get('variable'); + return variable && (element.name === variable.name); + } + + syncElementVariableChange(businessObject) { + const name = businessObject.get('name'); + const variable = businessObject.variable; + this._modeling.updateProperties(variable, { name }); + } +} + +// helpers ////////////////////// + +function getParent(element) { + return element.$parent; +} \ No newline at end of file From dffb29efa8bc167b7afe960e1ab3b027d2494a89 Mon Sep 17 00:00:00 2001 From: Abdul Ahad Date: Fri, 30 Aug 2024 12:37:58 +0200 Subject: [PATCH 2/4] test: add cases for undo/redo Co-authored-by: Nico Rehwaldt --- .../behavior/NameChangeBehaviorSpec.js | 74 +++++++++++++++---- .../behavior/NameChangeBehaviorSpec.js | 44 ++++++++++- 2 files changed, 100 insertions(+), 18 deletions(-) diff --git a/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js b/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js index 8bd5be6a4..c232a63c2 100644 --- a/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js +++ b/packages/dmn-js-decision-table/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js @@ -6,7 +6,7 @@ import CoreModule from 'src/core'; import Modeling from 'src/features/modeling'; -describe('NameChangeBehavior', function() { +describe('features/modeling/behavior - NameChangeBehavior', function() { describe('with existing variable', function() { @@ -17,24 +17,70 @@ describe('NameChangeBehavior', function() { ], })); + describe('should update variable name when element name is changed', function() { - it('should update variable name when element name is changed', inject( - function(modeling, sheet) { - // given - const root = sheet.getRoot(), - decisionTable = root.businessObject; + it('', inject( + function(modeling, sheet) { - const decision = decisionTable.$parent; + // given + const root = sheet.getRoot(), + decisionTable = root.businessObject; - // when - modeling.editDecisionTableName('foo'); + const decision = decisionTable.$parent; - // then - const variable = decision.get('variable'); + // when + modeling.editDecisionTableName('foo'); - expect(variable.get('name')).to.equal('foo'); - } - )); + // then + const variable = decision.get('variable'); + + expect(variable.get('name')).to.equal('foo'); + } + )); + + + it('', inject( + function(modeling, sheet, commandStack) { + + // given + const root = sheet.getRoot(), + decisionTable = root.businessObject; + + const decision = decisionTable.$parent; + modeling.editDecisionTableName('foo'); + + // when + commandStack.undo(); + + // then + const variable = decision.get('variable'); + + expect(variable.get('name')).to.equal('Season'); + } + )); + + + it('', inject( + function(modeling, sheet, commandStack) { + + // given + const root = sheet.getRoot(), + decisionTable = root.businessObject; + + const decision = decisionTable.$parent; + modeling.editDecisionTableName('foo'); + + // when + commandStack.undo(); + commandStack.redo(); + + // then + const variable = decision.get('variable'); + + expect(variable.get('name')).to.equal('foo'); + } + )); + }); }); }); diff --git a/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js b/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js index 1da32df3e..423692715 100644 --- a/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js +++ b/packages/dmn-js-drd/test/spec/features/modeling/behavior/NameChangeBehaviorSpec.js @@ -17,21 +17,57 @@ describe('NameChangeBehavior', function() { ], })); + describe('should update variable name when label is changed', function() { - it('should update variable name when label is changed', inject( - function(modeling, elementRegistry) { + + it('', inject( + function(modeling, elementRegistry) { + + // given + const decision = elementRegistry.get('season'), + bo = decision.businessObject, + variable = bo.variable; + + // when + modeling.updateLabel(decision,'foo'); + + // then + expect(variable.get('name')).to.equal('foo'); + } + )); + + + it('', inject(function(modeling, elementRegistry, commandStack) { // given const decision = elementRegistry.get('season'), bo = decision.businessObject, variable = bo.variable; + modeling.updateLabel(decision,'foo'); // when + commandStack.undo(); + + // then + expect(variable.get('name')).to.equal('season'); + })); + + + it('', inject(function(modeling, elementRegistry, commandStack) { + + // given + const decision = elementRegistry.get('season'), + bo = decision.businessObject, + variable = bo.variable; modeling.updateLabel(decision,'foo'); + // when + commandStack.undo(); + commandStack.redo(); + // then expect(variable.get('name')).to.equal('foo'); - } - )); + })); + }); }); }); From a6fffe1a459b14940816af052b0841cb6d626958 Mon Sep 17 00:00:00 2001 From: Abdul Ahad Date: Tue, 17 Sep 2024 15:06:17 +0200 Subject: [PATCH 3/4] fix: variable name state updating --- ...eralExpressionPropertiesEditorComponent.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/dmn-js-literal-expression/src/features/literal-expression-properties/components/LiteralExpressionPropertiesEditorComponent.js b/packages/dmn-js-literal-expression/src/features/literal-expression-properties/components/LiteralExpressionPropertiesEditorComponent.js index fcbde9cfb..edb58f05b 100644 --- a/packages/dmn-js-literal-expression/src/features/literal-expression-properties/components/LiteralExpressionPropertiesEditorComponent.js +++ b/packages/dmn-js-literal-expression/src/features/literal-expression-properties/components/LiteralExpressionPropertiesEditorComponent.js @@ -12,7 +12,7 @@ export default class LiteralExpressionPropertiesComponent extends Component { this._viewer = context.injector.get('viewer'); this._modeling = context.injector.get('modeling'); this._dataTypes = context.injector.get('dataTypes'); - + this._eventBus = context.injector.get('eventBus'); const decision = this._viewer.getDecision(); this.state = { @@ -32,6 +32,23 @@ export default class LiteralExpressionPropertiesComponent extends Component { }); } + componentWillMount() { + this._eventBus.on('elements.changed', this.onChange); + } + + componentWillUnmount() { + this._eventBus.off('elements.changed', this.onChange); + } + + onChange = () => { + const decision = this._viewer.getDecision(); + if (decision.variable) { + this.setState({ + name: decision.variable.name + }); + } + }; + setVariableType(typeRef) { if (typeRef === '') { this._modeling.editVariableType(undefined); From b38147127f9816adb99d81f7edc6f4cda8a2a024 Mon Sep 17 00:00:00 2001 From: Abdul Ahad Date: Thu, 3 Oct 2024 16:29:48 +0200 Subject: [PATCH 4/4] chore(CHANGELOG): update --- packages/dmn-js/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/dmn-js/CHANGELOG.md b/packages/dmn-js/CHANGELOG.md index b7fec4351..1c9b2c1bb 100644 --- a/packages/dmn-js/CHANGELOG.md +++ b/packages/dmn-js/CHANGELOG.md @@ -9,6 +9,7 @@ ___Note:__ Yet to be released changes appear here._ * `FEAT`: support Camunda 8 FEEL built-ins * `FIX`: improve validation of `first-item` FEEL rule * `DEPS`: update to `@bpmn-io/feel-editor@1.9.0` +* `FIX`: variable name changes when element name\label changes ([#893](https://github.com/bpmn-io/dmn-js/pull/893)) ## 16.7.1