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

Improve DMN modeling UX for Camunda Cloud #2525

Closed
Tracked by #2837
saig0 opened this issue Oct 25, 2021 · 7 comments · Fixed by #2845
Closed
Tracked by #2837

Improve DMN modeling UX for Camunda Cloud #2525

saig0 opened this issue Oct 25, 2021 · 7 comments · Fixed by #2845
Assignees
Labels
Camunda 8 Flags an issue as related to Camunda 8 DMN enhancement New feature or request

Comments

@saig0
Copy link
Member

saig0 commented Oct 25, 2021

Is your feature request related to a problem? Please describe.

In Camunda Cloud 1.3.0 1.4.0, we will support evaluating DMN decisions in Zeebe. Since Zeebe will use a different DMN engine than Camunda Platform and doesn't support all features from the Platform, we should remove some properties to improve the modeling UX.

Describe the solution you'd like

In order to improve the DMN UX for Zeebe and the new DMN engine, we should adjust the following parts:

  • decisions (in DRG view):
    • remove the property Version Tag - not supported by Zeebe
    • remove the property History Time to Live - not supported by Zeebe
    • remove the description of the Id property (similar to the Id property of processes) - in Zeebe the ID will be serialized as decisionId (in Zeebe, keys are the identifiers of the record entity)
    • tracked via Migrate properties panel to new architecture bpmn-io/dmn-js-properties-panel#29
  • decision table:
    • remove the property to select the Expression Language on inputs (i.e. input expressions), input entries, and output entries - Zeebe supports only FEEL
    • remove the property Input Variable on inputs - in FEEL, the input value can be accessed by using ? if needed
    • remove the options integer + long + double in favor of a new option number on selecting the Type of inputs and outputs - in FEEL, there is only a number type (represented as BigDecimal)
    • add the options time + dateTime + dayTimeDuration + yearMonthDuration + Any on selecting the Type of inputs and outputs - in order to cover the most common types of FEEL
  • literal expression:
    • remove the property to select the expression language - Zeebe supports only FEEL
    • remove the options integer + long + double in favor of a new option number on selecting the Variable Type - in FEEL, there is only a number type
    • add the options time + dateTime + dayTimeDuration + yearMonthDuration + Any on selecting the Variable Type of inputs and outputs - in order to cover the most common types of FEEL

Describe alternatives you've considered

No changes. It should be possible to deploy a DMN with the current modeler if no unsupported features are used. Otherwise, the deployment with the DMN is rejected.

Additional context

None.

@saig0 saig0 added DMN enhancement New feature or request Camunda 8 Flags an issue as related to Camunda 8 labels Oct 25, 2021
@MaxTru
Copy link
Contributor

MaxTru commented Oct 27, 2021

Assigning to @barmac who will work on this conceptually (how will we ship this stripped-down dmn editor)

@barmac barmac added the backlog Queued in backlog label Oct 27, 2021 — with bpmn-io-tasks
@barmac
Copy link
Contributor

barmac commented Oct 27, 2021

Thanks @saig0 for creating this issue. I am moving this currently to the backlog until we start working on it (which will happen later this quarter).

@barmac barmac added the ready Ready to be worked on label Jan 14, 2022 — with bpmn-io-tasks
@barmac barmac removed the backlog Queued in backlog label Jan 14, 2022
@barmac barmac added the in progress Currently worked on label Jan 28, 2022 — with bpmn-io-tasks
@barmac barmac removed the ready Ready to be worked on label Jan 28, 2022
@MaxTru MaxTru added the backlog Queued in backlog label Feb 11, 2022 — with bpmn-io-tasks
@MaxTru MaxTru removed the in progress Currently worked on label Feb 11, 2022
@MaxTru MaxTru added the in progress Currently worked on label Feb 11, 2022 — with bpmn-io-tasks
@MaxTru MaxTru removed the backlog Queued in backlog label Feb 11, 2022
@saig0
Copy link
Member Author

saig0 commented Feb 16, 2022

@barmac we should plan a few more adjustments to improve the UX:

Restricting the naming/usage of the decision ID

  • currently, the decisionId must be a valid QName
  • but in a DMN for Camunda Cloud, the decisionID may be referenced in a dependent decision (i.e. to access the decision result/output of a required decision)
  • the FEEL engine has the limitation that a variable should not contain any special characters or symbols (e.g. whitespace, dashes, etc.), except the variable name is escaped
  • if the decisionId has a special character (e.g. my-decision) and it is used in an input expression then the FEEL expression can't be parsed and the DMN is rejected by Zeebe
  • it works for Camunda Platform 7 because their DMN engine uses the output names instead of the decisionId (but this behavior is not correct according to the DMN specs)
  • we have the same issue for output name (i.e. the name of a decision table output) and variable name of decision literal expressions

Suggestion/idea:

  • add a new validation for the decisionId (and the other properties) and limit the value to alphanumeric names

Avoiding differences of the decision ID and the output name

  • a decision table can have one or more outputs and each output has an output name
  • currently, the output name can be named independently from the decisionId
  • but in a DMN for Camunda Cloud, the output name is not used if the decision table has only one output
  • instead, the DMN engine uses the decisionId. It uses the output names only if the decision table has more than one output.
  • if the output name is different from the decisionId and the output name is used in an input expression then the FEEL expression/the DMN decision may fail or return an unexpected result
  • it works for Camunda Platform 7 because their DMN engine uses always the output name instead of the decisionId (but this behavior is not correct according to the DMN specs)

Suggestion/idea:

  • the output name is not editable if the decision table has only one output

cc: @nikku

@nikku
Copy link
Member

nikku commented Feb 21, 2022

Makes a lot of sense @saig0.

@barmac please correct me on the following assessment:

Restricting the naming/usage of the decision ID

Straight forward to implement.

Differences of the decision ID and the output name

A little harder to implement; to be decided if we implement it (need to trade of with modeling UX) => Maybe handle via linting?


Avoiding differences of the decision ID and the output name

As I understand the engine will ignore the output name, right? @saig0.

So we could argue here that this is a case for linting (tell users their output name is going to be ignored for single output column tables).

@barmac
Copy link
Contributor

barmac commented Feb 21, 2022

I think we can change the ID validation pretty easily. However, I believe in this case linting is more suitable as the DMN is still valid but the engine cannot handle the ID (yet?). The same can be applied to the single output decision -> we should be able to model DMN as we want but get a warning if it's not supported by the engine.

@andreasgeier
Copy link

we should be able to model DMN as we want but get a warning if it's not supported by the engine.

That approach would follow the principles that we defined for validation and linting:

  1. Direct validation only for entries that would break the diagram in the modeler. Behavior: we ignore the input, refuse to change the XML, and as result keep the previous (valid) state.
  2. Everything else is part of the linting scope. No additional direct validation in the properties panel.

However, so far there is no DMN linting in place. So, doing things differently here is not preferred but is an option.

@MaxTru MaxTru added the ready Ready to be worked on label Feb 25, 2022 — with bpmn-io-tasks
@MaxTru MaxTru removed the in progress Currently worked on label Feb 25, 2022
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed ready Ready to be worked on labels Mar 24, 2022
barmac added a commit that referenced this issue Mar 24, 2022
@barmac barmac changed the title Improve DMN modeling UX for Camunda Cloud Implement DMN modeler for Camunda Cloud Mar 25, 2022
@barmac barmac changed the title Implement DMN modeler for Camunda Cloud Improve DMN modeling UX for Camunda Cloud Mar 25, 2022
@barmac
Copy link
Contributor

barmac commented Mar 25, 2022

Hey @saig0, I created follow-up issues for your suggestions in #2525 (comment):

barmac added a commit that referenced this issue Mar 25, 2022
barmac added a commit that referenced this issue Mar 25, 2022
barmac added a commit that referenced this issue Mar 25, 2022
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Mar 25, 2022
barmac added a commit that referenced this issue Mar 25, 2022
barmac added a commit that referenced this issue Mar 25, 2022
barmac added a commit that referenced this issue Mar 28, 2022
barmac added a commit that referenced this issue Mar 28, 2022
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Mar 28, 2022
barmac added a commit that referenced this issue Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Camunda 8 Flags an issue as related to Camunda 8 DMN enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants