feat: support bpmn:ConditionalEventDefinition templates#217
feat: support bpmn:ConditionalEventDefinition templates#217jarekdanielak wants to merge 7 commits intomainfrom
bpmn:ConditionalEventDefinition templates#217Conversation
|
Waiting for bpmn-io/bpmn-js-properties-panel#1176 to test this E2E |
| // Handle zeebe:conditionalFilter properties (variableNames, variableEvents) | ||
| let conditionalFilter = findExtension(conditionalEventDefinition, 'zeebe:ConditionalFilter'); | ||
|
|
||
| if (oldProperty && shouldKeepValue(conditionalEventDefinition, oldProperty, newProperty)) { |
There was a problem hiding this comment.
This is worrisome, I don't think shouldKeepValue holds to what it promises in its docs. I won't change it in this PR, since it's used in many places, but I'm looking into it for an Hour of Code session.
55066c3 to
7e07d1a
Compare
bdb0cc6 to
a7a5867
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds support for templating bpmn:ConditionalEventDefinition elements in BPMN diagrams. The implementation allows users to create element templates that can configure conditional events through two new binding types.
Changes:
- Added support for
bpmn:ConditionalEventDefinition#propertybinding to template condition expressions - Added support for
bpmn:ConditionalEventDefinition#zeebe:conditionalFilter#propertybinding to templatevariableNamesandvariableEventsproperties - Introduced
ConditionalEventTemplateBehaviorto automatically unlink templates withvariableEventsbinding when applied to root-level events (where they are not supported) - Fixed version 0 handling in template validation to correctly distinguish between templates without a version and templates with version 0
- Updated dependencies including bpmn-moddle to v10.0.0, which includes breaking changes (renamed BPMNModdle export to BpmnModdle)
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/cloud-element-templates/Helper.js | Added findConditionalEventDefinition helper function |
| src/cloud-element-templates/util/bindingTypes.js | Added two new binding type constants for conditional events |
| src/cloud-element-templates/util/propertyUtil.js | Implemented get/set logic for conditional event bindings |
| src/cloud-element-templates/cmd/ChangeElementTemplateHandler.js | Added _updateConditionalEventDefinition method and supporting functions |
| src/cloud-element-templates/behavior/ConditionalEventTemplateBehavior.js | New behavior to enforce constraints on conditional event templates |
| src/cloud-element-templates/behavior/index.js | Registered new behavior in module exports |
| src/element-templates/Validator.js | Fixed version 0 handling using isNil() instead of falsy check |
| src/cloud-element-templates/Validator.js | Fixed version 0 handling using isNil() instead of falsy check |
| test files | Comprehensive test coverage for all new functionality |
| package.json | Updated dependencies including major version bump for bpmn-moddle |
| CHANGELOG.md | Added FIX entry for version 0 handling (missing FEAT entry) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe('apply template', function() { | ||
|
|
||
| it('should unlink template with variableEvents on root-level', inject( | ||
| function(elementRegistry, elementTemplates) { | ||
|
|
||
| // given | ||
| let event = elementRegistry.get('RootEvent'); | ||
|
|
||
| // when | ||
| event = elementTemplates.applyTemplate(event, templates[0]); | ||
|
|
||
| // then | ||
| expect(getBusinessObject(event).get('zeebe:modelerTemplate')).not.to.exist; | ||
| } | ||
| )); | ||
|
|
||
|
|
||
| it('should keep template without variableEvents on root-level', inject( | ||
| function(elementRegistry, elementTemplates) { | ||
|
|
||
| // given | ||
| let event = elementRegistry.get('RootEvent'); | ||
|
|
||
| // when | ||
| event = elementTemplates.applyTemplate(event, templates[1]); | ||
|
|
||
| // then | ||
| expect(getBusinessObject(event).get('zeebe:modelerTemplate')).to.equal('conditional-no-variable-events-template'); | ||
| } | ||
| )); | ||
|
|
||
|
|
||
| it('should keep template without variableEvents in subprocess', inject( | ||
| function(elementRegistry, elementTemplates) { | ||
|
|
||
| // given | ||
| let event = elementRegistry.get('Subprocess_Event'); | ||
|
|
||
| // when | ||
| event = elementTemplates.applyTemplate(event, templates[1]); | ||
|
|
||
| // then | ||
| expect(getBusinessObject(event).get('zeebe:modelerTemplate')).to.equal('conditional-no-variable-events-template'); | ||
| } | ||
| )); | ||
| }); |
There was a problem hiding this comment.
Missing test case: there should be a test that verifies templates with variableEvents can be successfully applied to events within subprocesses (not just root-level). This would test the positive case where the template should be kept, complementing the test on line 42 that verifies templates with variableEvents are correctly rejected at root-level.
|
|
||
| function openTooltip(element) { | ||
| return act(() => { | ||
| fireEvent.mouseEnter(element); |
There was a problem hiding this comment.
The timeout value was changed from 200ms to 250ms, but there's no explanation for this change in the PR description. If this is related to the dependency updates (bpmn-js-properties-panel upgrade from 5.42.0 to 5.49.0), it should be documented. Otherwise, if this change is unrelated to the conditional event feature, it should be in a separate commit or PR.
| fireEvent.mouseEnter(element); | |
| fireEvent.mouseEnter(element); | |
| // 250ms matches the tooltip show delay in the properties panel implementation. |
| return businessObject.get(bindingName)?.get('body'); | ||
| } | ||
|
|
||
| if (bindingType === CONDITIONAL_EVENT_DEFINITION_ZEEBE_CONDITIONAL_FILTER_PROPERTY) { | ||
| const conditionalFilter = findExtension(businessObject, 'zeebe:ConditionalFilter'); |
There was a problem hiding this comment.
The getPropertyValue function is looking for conditional event properties in the wrong location. It should use findConditionalEventDefinition to get the conditional event definition first, similar to how the timer event definition properties are handled on lines 2427-2431. Currently, it's trying to access properties directly from the element's businessObject, which will not work because the condition and conditionalFilter are properties of the ConditionalEventDefinition, not the element itself.
| return businessObject.get(bindingName)?.get('body'); | |
| } | |
| if (bindingType === CONDITIONAL_EVENT_DEFINITION_ZEEBE_CONDITIONAL_FILTER_PROPERTY) { | |
| const conditionalFilter = findExtension(businessObject, 'zeebe:ConditionalFilter'); | |
| const conditionalEventDefinition = findConditionalEventDefinition(businessObject); | |
| if (!conditionalEventDefinition) { | |
| return; | |
| } | |
| // the actual value is nested in an Expression on the ConditionalEventDefinition | |
| return conditionalEventDefinition.get(bindingName)?.get('body'); | |
| } | |
| if (bindingType === CONDITIONAL_EVENT_DEFINITION_ZEEBE_CONDITIONAL_FILTER_PROPERTY) { | |
| const conditionalEventDefinition = findConditionalEventDefinition(businessObject); | |
| if (!conditionalEventDefinition) { | |
| return; | |
| } | |
| const conditionalFilter = findExtension(conditionalEventDefinition, 'zeebe:ConditionalFilter'); |
a7a5867 to
2f14b1e
Compare
2f14b1e to
8b804e7
Compare
8b804e7 to
3fb5ee8
Compare
|
Console error is fixed by bumping For creating elements, looks like I forgot to update the element factory... 🤦♂️ Done in 0d17f6f @philippfromme ready for another round. |


Related to camunda/camunda-modeler#5400
Proposed Changes
Support templating for
bpmn:ConditionalEventDefinition:bpmn:ConditionalEventDefinition#propertybinding.variableNamesandvariableEventsviabpmn:ConditionalEventDefinition#zeebe:conditionalFilter#propertybinding.ConditionalEventTemplateBehaviorto ensure a templatevariableEventsbinding is not applied to a root-level event.Try it
npm startthis project and use "Conditional Event" or your own template.Checklist
Ensure you provide everything we need to review your contribution:
Closes {LINK_TO_ISSUE}orRelated to {LINK_TO_ISSUE}@bpmn-io/srtool