Skip to content

feat(drd): create default variable for BKMs and decision services - #1054

Open
hippi345 wants to merge 2 commits into
bpmn-io:developfrom
hippi345:bkm-ds-default-variable
Open

hippi345 wants to merge 2 commits into
bpmn-io:developfrom
hippi345:bkm-ds-default-variable

Conversation

@hippi345

@hippi345 hippi345 commented Oct 30, 2026 •

Copy link
Copy Markdown

Proposed Changes

New business knowledge models now get a <variable> (named like the BKM, typeRef="Any"), the same way decisions and input data already do. Per review, default variable creation now lives only in CreateShapeBehavior (preExecute on shape.create) for decisions, input data, BKMs and decision services; ElementFactory no longer creates variables (it keeps the divider DI and the default New Decision Service name). Decision service variables also get typeRef="Any". Existing variables are never touched, and the name sync in NameChangeBehavior already covers BKMs.

Tests: BKM and decision service cases live in CreateShapeBehaviorSpec (created via modeling.createShape); the decision service variable assertions moved out of ElementFactorySpec. dmn-js-drd karma suite passes locally (466 passed, ChromeHeadless); npm run ci passes. Merged upstream develop to resolve the CHANGELOG.md conflict; the entry stays under Unreleased.

No UI change, so no screenshots.

To try it out: npx @bpmn-io/sr bpmn-io/dmn-js#bkm-ds-default-variable (or check out the branch and run the DRD starter), create a BKM or decision service, and inspect the exported XML for a <variable name="..." typeRef="Any" />.

Closes #1051

Checklist

Ensure you provide everything we need to review your contribution:

  • Contribution meets our definition of done
  • Pull request establishes context
    • Link to related issue(s), i.e. Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}
    • Brief textual description of the changes
    • Screenshots or short videos showing UI/UX changes
    • Steps to try out, i.e. using the @bpmn-io/sr tool

@hippi345
hippi345 requested review from a team and a balanced review from Copilot October 30, 2026 01:28
@fake-join fake-join Bot added needs:pr-quality Requires changes to pull request needs:cla Requires CLA to be signed needs:ci-approval Requires approval of workflow runs labels Oct 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the default-variable changes.

No open issues 🎉

What changed in this PR

Addresses #1051 by extending default variable creation in dmn-js-drd to business knowledge models and adding the missing type reference for decision services, while preserving existing variables.

Changes:

  • Create named variables with typeRef="Any" for new business knowledge models.
  • Set typeRef="Any" on newly created decision service variables.
  • Add regression assertions and document the feature in the changelog.
File Description
packages/​dmn-js/​CHANGELOG.md Documents the default-variable changes and links #1051.
packages/​dmn-js-drd/​test/​spec/​features/​modeling/​ElementFactorySpec.js Checks the decision service variable’s default type.
packages/​dmn-js-drd/​test/​spec/​features/​modeling/​behavior/​CreateShapeBehaviorSpec.js Tests business knowledge model variable creation, name, type, and parent.
packages/​dmn-js-drd/​src/​features/​modeling/​ElementFactory.js Adds typeRef="Any" to new decision service variables.
packages/​dmn-js-drd/​src/​features/​modeling/​behavior/​CreateShapeBehavior.js Extends default variable creation to business knowledge models and updates its documentation.

Review effort: Balanced

@CLAassistant

CLAassistant commented Oct 30, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@fake-join fake-join Bot added stale This pull request will be automatically closed unless there is activity. and removed needs:cla Requires CLA to be signed labels Oct 31, 2026
@fake-join

fake-join Bot commented Nov 6, 2026

Copy link
Copy Markdown

This pull request went stale - we have not heard from you in more than 7 days.

To move it forward, please:

This unblocks a review - it is not a guarantee of being merged.

Without activity this pull request will be closed in 14 days. Any activity - a push, a comment, a description update, a re-requested review - keeps it open.

@barmac
barmac self-requested a review November 6, 2026 08:18
@fake-join fake-join Bot removed the needs:ci-approval Requires approval of workflow runs label Nov 6, 2026

@barmac barmac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have a look into the comments. We need to fix the broken implementation, and not build on top of it.

@@ -81,7 +81,8 @@ ElementFactory.prototype.createDrdElement = function(elementType, attrs) {
// Create variable for Decision Service

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding variable should not be done via ElementFactory, I missed that in the review. The right place for such defaults is via behaviors.

Comment on lines +69 to +70
var bkm = elementFactory.createShape({ type: 'dmn:BusinessKnowledgeModel' });
modeling.createShape(bkm, { x: 100, y: 100 }, rootElement);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail when we adopt correct architecture (adding variable via behavior).

@fake-join fake-join Bot added needs:changes Requires follow-ups on review hints needs:ci-approval Requires approval of workflow runs and removed stale This pull request will be automatically closed unless there is activity. labels Nov 10, 2026
@hippi345

Copy link
Copy Markdown
Author

Thanks for the review — agreed that default variables belong in modeling behaviors, not ElementFactory.

This update removes decision-service variable creation from ElementFactory entirely (the old hook never set typeRef, which caused #1051). BKMs and decision services now get their default InformationItem in CreateShapeBehavior on shape.create, same as decisions and input data. ElementFactory only keeps decision-service-specific DI (divider) and the default name.

Tests for BKM and decision service variables are in CreateShapeBehaviorSpec and assert after modeling.createShape; the ElementFactorySpec case for decision service variables is gone.

hippi345 and others added 2 commits November 14, 2026 18:46
…a behavior

Assign default InformationItem variables (typeRef="Any") for business
knowledge models and decision services in CreateShapeBehavior on
shape.create. Remove variable creation from ElementFactory so defaults
follow the same pattern as decisions and input data.

Closes bpmn-io#1051

Co-authored-by: Joel Shearon <hippi345@users.noreply.github.com>
Co-authored-by: Joel Shearon  <hippi345@users.noreply.github.com>
Co-authored-by: Joel Shearon  <hippi345@users.noreply.github.com>
@cursor
cursor Bot force-pushed the bkm-ds-default-variable branch from 7f01a52 to ac4d5c9 Compare November 14, 2026 18:57
@fake-join fake-join Bot removed needs:pr-quality Requires changes to pull request needs:ci-approval Requires approval of workflow runs labels Nov 14, 2026
@hippi345

Copy link
Copy Markdown
Author

@barmac Rebuilt the branch on current develop without merge commits, so the history check should pass. The behavior fix is unchanged from my earlier note; I only added a test that default variables aren't created until shape.create runs.

@fake-join fake-join Bot added the needs:ci-approval Requires approval of workflow runs label Nov 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs:changes Requires follow-ups on review hints needs:ci-approval Requires approval of workflow runs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing variable typerefs in BKMs and Decision Services in dmn-js 17.13.0

4 participants