Skip to content
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

feat: add ad-hoc-sub-process rule #160

Merged
merged 2 commits into from
Feb 17, 2025
Merged

feat: add ad-hoc-sub-process rule #160

merged 2 commits into from
Feb 17, 2025

Conversation

jarekdanielak
Copy link
Contributor

@jarekdanielak jarekdanielak commented Feb 14, 2025

Proposed Changes

Add ad-hoc-sub-process rule which validates an ad-hoc sub-process against the BPMN specification.

Constraints:

  • Must not include start or end events.
  • Every intermediate event must have an outgoing sequence flow.

Additionally with b323933 we make sure existing sub-process rule also applies to an ad-hoc sub-process.

Related to camunda/camunda-modeler#4811

Depends on #164 for 🟢 CI.

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@jarekdanielak jarekdanielak added the enhancement New feature or request label Feb 14, 2025
@jarekdanielak jarekdanielak self-assigned this Feb 14, 2025
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Feb 14, 2025
@nikku
Copy link
Member

nikku commented Feb 16, 2025

Added dependency:

Depends on #164 for 🟢 CI.

@nikku
Copy link
Member

nikku commented Feb 16, 2025

I'm not sure on the spec intention, but does it feature a blank intermediate catch event or a link catch event as a proposed start? If not, let's not feature these elements as valid, but feature task -> task flows as valid.

Link catch: I'm not sure what semantics that should have. 🙃

@nikku nikku added the rules Concerning existing or missing rules label Feb 16, 2025
@nikku
Copy link
Member

nikku commented Feb 16, 2025

Let's consider the spec text:

However, it is possible to specify Sequence Flows between some of the contained Activities. When used, Sequence Flows will provide the same ordering constraints as in a regular Process. To have any meaning, Intermediate Events will have outgoing Sequence Flows and they can be triggered multiple times while the Ad-Hoc Sub-Process is active.

According to it, this, I think, would be a reasonable use of an ad-hoc sub process, with intermediate catch events:

image

@bastiankoerber
Copy link

Let's consider the spec text:

However, it is possible to specify Sequence Flows between some of the contained Activities. When used, Sequence Flows will provide the same ordering constraints as in a regular Process. To have any meaning, Intermediate Events will have outgoing Sequence Flows and they can be triggered multiple times while the Ad-Hoc Sub-Process is active.

According to it, this, I think, would be a reasonable use of an ad-hoc sub process, with intermediate catch events:

image

I agree with the approach.
Just to double check, i am also allowed to execute multiple times the "Do standalone task" & "Do Work" right?
I can also add a call activity to the bpmn adhoc process right?

@saig0
Copy link

saig0 commented Feb 17, 2025

@nikku, note that the intermediate events inside the ad-hoc subprocess are not active by default. Instead, they must be explicitly activated before they can be triggered.

In the example, the timer is triggered 10 minutes after its activation, not 10 minutes after the subprocess is entered. The conditional event can only be triggered if it is activated first. Like other activities inside the ad-hoc subprocess, events must be activated first.

Just to double check, i am also allowed to execute multiple times the "Do standalone task" & "Do Work" right?

Yes.

I can also add a call activity to the bpmn adhoc process right?

Yes.

@nikku
Copy link
Member

nikku commented Feb 17, 2025

Thanks both. The usual semantics of ad-hoc applies:

  • start activities must be explicitly activated
  • if activated, then "tasks execute", and "events can be triggered"
  • if not activated, then any activity or event is ignored

➡️ This will pose an interesting run-time/introspection challenge, too (ref)

@nikku
Copy link
Member

nikku commented Feb 17, 2025

@jarekdanielak as per above discussions I'll update the example in this PR to a simplified version of #160 (comment).

Verify that an ad-hoc sub-process is valid:

* Must not contain start or end events.
* Every intermediate event must have an outgoing sequence flow.

Related to camunda/camunda-modeler#4811
@nikku
Copy link
Member

nikku commented Feb 17, 2025

Updated docs via bfbe5c5, squashed.

Copy link
Member

@nikku nikku left a comment

Choose a reason for hiding this comment

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

Great stuff 👏

@@ -1,5 +1,6 @@
module.exports = {
rules: {
'ad-hoc-sub-process': 'error',
Copy link
Member

Choose a reason for hiding this comment

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

I agree!

@nikku nikku merged commit a223f0f into main Feb 17, 2025
7 of 9 checks passed
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Feb 17, 2025
@nikku nikku deleted the ad-hoc-sub branch February 17, 2025 10:41
@nikku
Copy link
Member

nikku commented Feb 17, 2025

Great stuff 👏

@nikku
Copy link
Member

nikku commented Feb 17, 2025

I'm going to release this thing, with the other nit-pics currently worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rules Concerning existing or missing rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants