zeebe-io/zeebe-modeler

New Zeebe attribute "propagateAllChildVariables" for call activities

saig0 opened this issue · 10 comments

saig0 commented

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

Zeebe supports a new (boolean) attribute "propagateAllChildVariables" for call activities. It can be set to true or false.

If it is set to true then all variables are propagated (i.e. copied) from the child instance to the parent instance. Otherwise, no variables are copied.

If an output mapping is defined on the call activity then the output mapping is applied and only the defined variables are propagated.

The Zeebe modeler should support the new attribute.

The related issue in Zeebe: camunda/zeebe#4860 (comment)

Describe the solution you'd like

In the Zeebe modeler, I can set the attribute "propagateAllChildVariables" for call activities. For example, by enabling or disabling a flag on the details section of the activity next to the process id.

By default, the attribute should be set to false. This avoids that variables are overridden accidentally in a parallel flow (e.g. multi-instance parallel call activity). If the attribute is not present then Zeebe interprets it as true to be backward-compatible. So, new BPMN workflows should serialize the attribute. But existing workflows should not serialize the attribute (or with value true) if the attribute is not changed/disabled to avoid changing in the behavior of the workflow.

The user should be notified (e.g. a warning or failure message) if the attribute is set to true and output mappings are defined. If output mappings are defined then it propagates only the variables that are defined in the mapping.

Decided design and tasks as of kickoff 20th October 2020

Also see #252 (comment)

  • Pull in latest bpmn-js-prop-panel version (so to have new I/O GUI components available) (see #261)
  • Update zeebe moddle (see camunda/zeebe-bpmn-moddle#2)
  • Migrate existing I/O tabs to use new / improved I/O GUI component and improve labels (#265)
  • Implement toggle switch for propageteAllChildVariables setting propagateAllChildVariables and disabling/deleting output parameter mappings (#267)
  • Implement migration behavior for callActivites without propagateAllChildVariables (issue tbd)

Describe alternatives you've considered

No.

Additional context

BPMN XML of a call activity:

  <bpmn:callActivity id="Activity_1h7j9o6" name="A">
      <bpmn:extensionElements>
        <zeebe:calledElement processId="demo-process" propagateAllChildVariables="false" />
      </bpmn:extensionElements>
      <bpmn:incoming>Flow_1t0ia8l</bpmn:incoming>
      <bpmn:outgoing>Flow_0ypungt</bpmn:outgoing>
    </bpmn:callActivity>
MaxTru commented

Hi there,

The user should be notified (e.g. a warning or failure message) if the attribute is set to true and output mappings are defined.

Would a validation of the respective checkbox be okay?

  • 👍1
saig0 commented

@MaxTru a validation would be an option ✔️
It is to avoid a misconfiguration. You can deploy such a workflow to Zeebe but it might not work as the user expect.

  • 👍1
MaxTru commented

Assigning it to @volkergersabeck to do PM evaluation of this issue before implementation.

saig0 commented

@volkergersabeck when do you plan to implement this feature?

This issue would solve a major issue in Zeebe. It is easy for users to run into this bug because the current (default) behavior is not intuitive.

MaxTru commented

@saig0 I scheduled a quick sync to clarify some conceptual questions (see your calendar), afterwards we can implement this rather quickly IMO. Hope this helps, please feel free to reach out in case of questions.

  • 👍1
pinussilvestrus commented

This will probably need some API updates if we need to update the bpmn-js-properties-panel dependency, cf. #261

MaxTru commented

Kickoff performed on 20.October together with @saig0, @andreasgeier, @volkergersabeck and myself.

Key decisions:

  • First we will change the I/O mapping component to the more recent version, that we also use in the Modeler (bpmn-io/bpmn-js-properties-panel#350)
  • We will adjust the label of the input / output mapping fields (currently Source and Target) to be more meaningful
  • For the CallActivity I/O tab, a toggle/slider Propagate all child variables? will be added on top of the output parameter mapping. This toggle will control whether it is possible to have output parameter mappings or not.
    • Toggle on equals to true, and off equals to false
    • If the toggle is on, it is not possible to add output parameter mappings.
    • If the toggle is set to on, existing output parameter mappings will be deleted.
    • If the toggle is off, the output parameter mapping component will work as usual.
  • [For new CallActivities created]: the toggle is off as default.
  • [Migration]: existing diagrams (without the propagateAllChildVariables will be migrated). This means:
    • An old callActivity without the property and without output parameter mappings => set the property to true (and toggle on)
    • An old callActivity without the property but with existing output parameter mappings => set the property to false (and toggle off)

I will treat this issue as the epic and create child issues for the respective tasks.

  • 👍1
pinussilvestrus commented

Closed via #271