camunda / camunda-modeler

An integrated modeling solution for BPMN, DMN and Forms based on bpmn.io.
https://camunda.com/products/modeler
MIT License
1.51k stars 483 forks source link

Support Zeebe user task #4087

Closed barmac closed 8 months ago

barmac commented 9 months ago

What should we do?

<bpmn:userTask id="Activity_1kl1v6a">
  <bpmn:extensionElements>
    <zeebe:formDefinition externalReference="customFormKey" />
    <zeebe:userTask />
  </bpmn:extensionElements>
</bpmn:userTask>

zeebe-bpmn-moddle

properties panel

linting

Why should we do it?

Part of efforts: User Task lifecycle & listeners in Zeebe

Epic: https://github.com/camunda/product-hub/issues/1823

barmac commented 9 months ago

I clarified the task listeners, and we are not implementing this feature yet (cf. https://github.com/camunda/product-hub/issues/1823#issuecomment-1918589148).

tmetzke commented 9 months ago

@barmac, we'll call the corresponding property of user task records in Zeebe externalFormReference to have enough context in a user task, looking at the attribute. Should we align this here and also go for that name in modeling or do you think it's fine to have a slight difference here since the externalReference is part of the form definition in the model already?

nikku commented 9 months ago

@tmetzke As far as I know we treated a form key not starting with camunda-form as an external reference in the past. How/why did we decide to change course?

Does this mean we'll now have 4 options for our users to choose?

@barmac Let's take a focused amount of time to think about the UX rather soon.

tmetzke commented 9 months ago

Hey @nikku, thanks for asking, we missed to publicly clarify this anywhere, I guess. As described in the (internal) technical proposal, there will be 2 options in the Zeebe user tasks: linked Camunda forms (via formId) or custom forms (via externalReference). As job-based user tasks are meant to be deprecated in the future, those will be the two options moving forward. For job-based user tasks (the current default), nothing changes in that regard.

Why the change? There has been quite some confusion around the formKey in Camunda 8 in the past:

  1. We more or less took this over from how it's done in Camunda 7. However, since Zeebe uses a key-based terminology instead of Camunda 7's id-based one for unique identification of entities (e.g. processInstanceKey instead of processInstanceId), this led to name clashes and misunderstandings. For example, all key-based attributes in a job record can be used as Long value-based references to other entities, while the formKey can't. This is merely a String value, describing how to find the related form.
  2. The value of the formKey attribute is quite cumbersome for users to decode. The specific way the form is linked can only be obtained through dedicated String parsing and pattern matching, analyzing potential prefixes in the value to exist. @christian-konrad also expressed his preference of a clear separation of linked forms and custom forms in modeling (internally) over here.

As a result, we (internally) decided to go for formId and externalReference for user task forms, moving forward. We intend to highlight this in the public documentation as well.

Since we're still in development here and in alpha-status, I'm absolutely open to discussing this if you see any hard requirements to do this differently. Sorry for not involving the modeling team more in this move. With @christian-konrad also pushing for this, I figured this was an obvious move to make (technically from the Zeebe end but also product-wise).

tmetzke commented 9 months ago

I also added this reasoning to the public issue in Zeebe.

barmac commented 9 months ago

Does this mean we'll now have 4 options for our users to choose?

Technically it's 5 different options spread among 2 groups:

nikku commented 9 months ago

Thanks @tmetzke. This rationale makes absolute sense as we have the opportunity to re-think things.

barmac commented 8 months ago

Since each of the features in the upstream libraries are merged, this is now fixed upstream.

barmac commented 8 months ago

This will complete the story: https://github.com/bpmn-io/bpmn-js-properties-panel/issues/1032