-
Notifications
You must be signed in to change notification settings - Fork 153
feat(drd): Add Decision Services for DMN 1.5 Support #983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces DMN 1.5 Decision Service support in the DRD modeler/viewer, adding the new element to creation tools, rendering, modeling behaviors, and XML/DI handling.
Changes:
- Add Decision Service creation entry points (palette/context pad) plus label editing + variable name syncing support.
- Introduce modeling behavior to manage DecisionService compartments (output/encapsulated) and auto-maintain input references.
- Add rendering + DI factory support and extend test coverage with new fixtures/specs.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/dmn-js-shared/src/features/modeling/behavior/NameChangeBehavior.js | Sync DecisionService variable name on label/property name changes. |
| packages/dmn-js-drd/test/spec/features/palette/PaletteProviderSpec.js | Update palette entry count for new Decision Service tool. |
| packages/dmn-js-drd/test/spec/features/modeling/drd-updater-decision-service.dmn | New DMN test diagram covering DecisionService + move-in/out behavior. |
| packages/dmn-js-drd/test/spec/features/modeling/behavior/decision-service-behavior.dmn | New DMN test diagram for DecisionServiceBehavior scenarios. |
| packages/dmn-js-drd/test/spec/features/modeling/behavior/DecisionServiceBehaviorSpec.js | New unit tests for DecisionServiceBehavior (sectioning, refs, inputs). |
| packages/dmn-js-drd/test/spec/features/modeling/ElementFactorySpec.js | Add element factory tests for DecisionService defaults + variable creation. |
| packages/dmn-js-drd/test/spec/features/modeling/DrdUpdaterSpec.js | Extend DrdUpdater tests for move behavior involving DecisionServices. |
| packages/dmn-js-drd/test/spec/features/modeling/DrdFactorySpec.js | Add DI factory test for DMNDecisionServiceDividerLine creation. |
| packages/dmn-js-drd/test/spec/draw/DrdRendererSpec.js | Add smoke test for rendering a DecisionService fixture. |
| packages/dmn-js-drd/test/fixtures/dmn/decision-service.dmn | New rendering fixture containing a DecisionService + divider line. |
| packages/dmn-js-drd/src/features/rules/DrdRules.js | Allow DecisionService create/move/resize and define connection rules. |
| packages/dmn-js-drd/src/features/palette/PaletteProvider.js | Add palette entry to create Decision Service. |
| packages/dmn-js-drd/src/features/modeling/behavior/index.js | Register DecisionServiceBehavior in modeling behaviors module. |
| packages/dmn-js-drd/src/features/modeling/behavior/DecisionServiceBehavior.js | Implement DecisionService compartment logic + input ref auto-update. |
| packages/dmn-js-drd/src/features/modeling/behavior/CreateConnectionBehavior.js | Extend connection ref creation logic for DecisionService-related connections. |
| packages/dmn-js-drd/src/features/modeling/ElementFactory.js | Add DecisionService default sizing, DI divider line, and variable initialization. |
| packages/dmn-js-drd/src/features/modeling/DrdUpdater.js | Update decision services on moves/deletes and on requirement changes. |
| packages/dmn-js-drd/src/features/modeling/DrdFactory.js | Add helper to create DMNDecisionServiceDividerLine DI elements. |
| packages/dmn-js-drd/src/features/label-editing/LabelUtil.js | Enable label editing for DecisionService (name attribute). |
| packages/dmn-js-drd/src/features/label-editing/LabelEditingProvider.js | Adjust label editing positioning behavior for DecisionService. |
| packages/dmn-js-drd/src/features/context-pad/ContextPadProvider.js | Enable context pad append actions for DecisionService. |
| packages/dmn-js-drd/src/draw/DrdRenderer.js | Add visual rendering of DecisionService (box, divider, section labels). |
| packages/dmn-js-drd/assets/css/dmn-js-drd.css | Add palette icon styling for Decision Service entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/dmn-js-drd/test/spec/features/modeling/drd-updater-decision-service.dmn
Outdated
Show resolved
Hide resolved
| 'dmn:DecisionService': function(p, element) { | ||
| var rect = drawRect(p, element.width, element.height, 12, { | ||
| stroke: getStrokeColor(element, defaultStrokeColor), | ||
| strokeWidth: 4, | ||
| fill: getFillColor(element, defaultFillColor) | ||
| }); | ||
|
|
||
| var dividerY = element.height / 2; | ||
| var line = svgCreate('line'); | ||
| svgAttr(line, { | ||
| x1: 0, | ||
| y1: dividerY, | ||
| x2: element.width, | ||
| y2: dividerY, | ||
| stroke: getStrokeColor(element, defaultStrokeColor), | ||
| strokeWidth: 2 |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The divider line is rendered at element.height / 2, ignoring the imported/stored DI divider line (DMNDecisionServiceDividerLine). This will render DecisionServices incorrectly when the divider is not centered (e.g. after divider resize or when importing DI). Compute the divider position from element.businessObject.di.decisionServiceDividerLine.waypoint (convert absolute -> local coordinates), with a center fallback when DI is missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct, we should take the XML values into account.
| DecisionServiceBehavior.prototype._getDividerPosition = function(decisionServiceElement) { | ||
| return decisionServiceElement.y + (decisionServiceElement.height / 2); | ||
| }; |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_getDividerPosition always assumes the divider is centered (height / 2). If the divider line can be moved/resized or if DI imports a non-centered divider, section classification (output vs encapsulated) will be wrong. Derive the divider Y from the DecisionService DI (decisionServiceBo.di.decisionServiceDividerLine.waypoint) and fall back to center only when it is not available.
| if (is(businessObject, 'dmn:DecisionService')) { | ||
| size = this._getDefaultSize(businessObject); | ||
| var dividerY = (attrs.y || 0) + (size.height / 2); | ||
| var dividerWaypoints = [ | ||
| { x: attrs.x || 0, y: dividerY }, | ||
| { x: (attrs.x || 0) + size.width, y: dividerY } | ||
| ]; | ||
| businessObject.di.decisionServiceDividerLine = drdFactory.createDiDividerLine(dividerWaypoints); | ||
|
|
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ElementFactory creates and stores decisionServiceDividerLine waypoints once at creation time, but there is no code updating these waypoints when the DecisionService is moved/resized (or when the divider is moved). This will cause exported DMN DI to become stale. Consider updating the divider line waypoints in DrdUpdater.updateBounds / relevant resize handlers whenever the DecisionService bounds (or divider position) changes.
| <di:waypoint x="0" y="75" /> | ||
| <di:waypoint x="200" y="75" /> |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DMNDecisionServiceDividerLine waypoints here (x=0..200, y=75) don't line up with the shape bounds (x=330, y=225, width=400, height=335). If divider line waypoints are in diagram coordinates (like other DMNDI waypoints), this fixture will import with an incorrect divider position. Align the divider waypoints with the shape bounds (or clarify/update if divider waypoints are meant to be local coordinates).
| <di:waypoint x="0" y="75" /> | |
| <di:waypoint x="200" y="75" /> | |
| <di:waypoint x="330" y="300" /> | |
| <di:waypoint x="530" y="300" /> |
| if (is(target, 'dmn:DecisionService')) { | ||
| return { type: 'dmn:KnowledgeRequirement' }; | ||
| } | ||
|
|
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
canConnect allows Decision -> DecisionService and returns a dmn:KnowledgeRequirement (lines 100-112). However CreateConnectionBehavior determines the required* ref based on the source element type, so this path will create a KnowledgeRequirement with requiredDecision instead of requiredKnowledge. DrdImporter._getSource only looks at requiredKnowledge for KnowledgeRequirements, so the connection will not import/roundtrip correctly. Either disallow this connection type or ensure KnowledgeRequirement refs are always written to requiredKnowledge regardless of source element type.
| if (is(target, 'dmn:DecisionService')) { | |
| return { type: 'dmn:KnowledgeRequirement' }; | |
| } |
packages/dmn-js-drd/src/features/modeling/behavior/CreateConnectionBehavior.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
Thank you for your pull request! Before we merge it, let's make sure that it meets the definition of done. It looks that the interaction is not quite there yet. We should render the decision service below other elements so that they don't get hidden. I'd also envision that the encapsulated & output decisions should move together with the decision service element. Let me know if anything is unclear. Screen.Recording.2026-02-10.at.15.28.51.movScreen.Recording.2026-02-10.at.15.29.18.mov |
|
Thank you for the feedback @barmac For the first point, I have addressed the rendering issue by setting the DecisionService Element fill to transparent, which prevents contained decisions from being hidden. This works as expected and maintains visibility of all elements. If the accurate interaction is for contained Decisions to move together with the DecisionService when the DecisionService is placed on it, could you recommend the best approach to implement this? |

Proposed Changes
These changes implement the following requirements for the Decision Service element:
Checklist
Ensure you provide everything we need to review your contribution:
@bpmn-io/srtoolCloses #957