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

Disallow Condition If Sequence Flow Is Not Exclusive or Inclusive Gateway #82

Closed
jonathanlukas opened this issue Feb 28, 2023 · 8 comments · Fixed by #97
Closed

Disallow Condition If Sequence Flow Is Not Exclusive or Inclusive Gateway #82

jonathanlukas opened this issue Feb 28, 2023 · 8 comments · Fixed by #97
Assignees
Labels
rules spring cleaning Could be cleaned up one day

Comments

@jonathanlukas
Copy link

The rule should detect the following modeling patterns

Incorrect: Camunda 8.1 will reject this:
converted-c8-conditional-flow.bpmn.txt

'converted-c8-conditional-flow.bpmn': - Element: DoSomethingTask
    - ERROR: Conditional sequence flows are only supported at exclusive or inclusive gateway

Correct: Camunda 8.1 will accept this:
converted-c8-inclusive-gateway.bpmn.txt

In addition, we consider modeling implicit gateways a bad practice, so even if zeebe will support conditional flows, this rule would still apply as warning or info.

How does the rule improve the BPMN diagram?

This rule will prevent users from creating undeployable diagrams.

Rule Details

  • Name: no-conditional-flows
  • Default notification level: error

What alternatives did you consider?

@marstamm
Copy link
Member

marstamm commented Mar 1, 2023

The rule makes sense, thanks for suggesting it

@marstamm marstamm added spring cleaning Could be cleaned up one day backlog Queued in backlog labels Mar 1, 2023
@philippfromme philippfromme self-assigned this May 15, 2023
@philippfromme philippfromme added in progress Currently worked on and removed backlog Queued in backlog labels May 15, 2023
@philippfromme philippfromme changed the title Lint conditional flows Disallow Condition If Sequence Flow Is Not Exclusive or Inclusive Gateway May 15, 2023
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels May 16, 2023
@nikku
Copy link
Member

nikku commented May 16, 2023

In addition, we consider modeling implicit gateways a bad practice, so even if zeebe will support conditional flows, this rule would still apply as warning or info.

This would be "modeling guidance" then. With this plug-in we strictly validated compatibility for execution.

@nikku
Copy link
Member

nikku commented May 16, 2023

Interestingly enough Zeebe does not fail on default flows, only conditional 🙈. This diagram deploys just fine, but makes no sense without condition support:

image

@nikku
Copy link
Member

nikku commented May 16, 2023

Related issue that we do not capture yet (in terms of run-time compatibility):

Conditions must not be empty:

Command 'CREATE' rejected with code 'INVALID_ARGUMENT': Expected to deploy new resources, but encountered the following errors:
'Playground Process.bpmn': - Element: Activity_119f0sp
    - ERROR: Conditional sequence flows are only supported at exclusive or inclusive gateway
- Element: Flow_1f2h1nt > conditionExpression
    - ERROR: Expected expression but found static value ''. An expression must start with '=' (e.g. '=').
[ deploy-error ] 16/5/2023 18:0:38

@nikku
Copy link
Member

nikku commented May 17, 2023

Closed via #97.

@nikku nikku closed this as completed May 17, 2023
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label May 17, 2023
@philippfromme
Copy link
Collaborator

  • Element: Flow_1f2h1nt > conditionExpression
    • ERROR: Expected expression but found static value ''. An expression must start with '=' (e.g. '=').
      [ deploy-error ] 16/5/2023 18:0:38

How did you end up with an empty expression? The conditionExpression shouldn't exist unless there is a value.

@nikku
Copy link
Member

nikku commented May 17, 2023

Change a flow into a conditional flow.

@philippfromme
Copy link
Collaborator

philippfromme commented May 17, 2023

Change a flow into a conditional flow.

Ah! Yeah, that part is a bit confusing. So any sequence flow can have a condition, but only when the source is an activity it must also be rendered as a conditional sequence flow. I guess we need the empty expression to be able to determine that the sequence flow is conditional and to render it as such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rules spring cleaning Could be cleaned up one day
Projects
None yet
4 participants