camunda / linting

Linting for the Camunda Desktop and Web Modeler
MIT License
2 stars 2 forks source link

Support zeebe user task #101

Closed barmac closed 8 months ago

barmac commented 8 months ago

The error message for implementation looks like below:

image

Screen capture (error message is outdated):

https://github.com/camunda/linting/assets/28307541/c8a74bca-301a-47b1-9709-945a17f6b2d1


Related to https://github.com/camunda/product-hub/issues/2126 Related to https://github.com/camunda/camunda-modeler/issues/4087

barmac commented 8 months ago

Requires https://github.com/camunda/bpmnlint-plugin-camunda-compat/pull/161

barmac commented 8 months ago

We are usually a little bit more descriptive than "Not supported" ⬇️:

I took this from Zeebe Properties, but can adjust ofc.

barmac commented 8 months ago

Default value is Job Worker when creating a new user task, is this desired? Potentially we want to make this dependent on the platform type selected, can we accomplish that?

It's based on the XML. For all of the legacy tasks, Job Worker is displayed. When we change the implementation to Zeebe user task, a <zeebe:userTask> extension is added. This way, we don't forcefeed new feature to the legacy diagrams (no surprises), and also avoid the unnecessary decision requirement ("Implementation must be selected").

We can change the default but it's outside of the scope of linting project, so let's discuss it via https://github.com/camunda/product-hub/issues/2126

nikku commented 8 months ago

https://github.com/camunda/linting/pull/101#issuecomment-1973064297

Agreed.

barmac commented 8 months ago

We are usually a little bit more descriptive than "Not supported" ⬇️:

I took this from Zeebe Properties, but can adjust ofc.

@nikku How about this?

image

barmac commented 8 months ago

Or:

image

nikku commented 8 months ago

Better :arrow_up: (as a general pattern). Not Form ID etc. are unsupported, but the form type (or implementation type) chosen. :muscle:

barmac commented 8 months ago

The second one, right? https://github.com/camunda/linting/pull/101#issuecomment-1973157619

barmac commented 8 months ago

I pushed https://github.com/camunda/linting/pull/101#issuecomment-1973157619

nikku commented 8 months ago

I missed the first one :see_no_evil: .

To be discussed, but I like it as it is clear (what would unsupported mean if you don't have the error panel open).

nikku commented 8 months ago

Reached out to the group. But honestly I think we should just adopt the clear "Supported only in Camunda 8.x and newer" consistently, everywhere.

In this sense I'd be fine to leave the current PR as is (following the existing pattern) and rework the call to action consistently as a follow-up.

What do you think?

barmac commented 8 months ago

I think "Zeebe user task not supported." guides the user better how to fix the problem. It's not the whole entry, but rather the current value which triggers the error. Perhaps we could make it even better: "Only job worker is supported".

On the other hand, if we want to foster migration, pointing out that migration gives more features, better value, "Supported only in Camunda 8.x" serves that better. So it's a matter of priorities.

nikku commented 8 months ago

On the other hand, if we want to foster migration, pointing out that migration gives more features, better value, "Supported only in Camunda 8.x" serves that better. So it's a matter of priorities.

I think if we focus a drop down and mark it as an error, then it is clear that the current choice is the issue. So :+1: for (1) focussing the dropdown and marking it as an error and (2) having a descriptive error message that indicates the version compatibility "Supported only in Camunda 8.x".

barmac commented 8 months ago

So now it's https://github.com/camunda/linting/pull/101#issuecomment-1973155463 :D

nikku commented 8 months ago

Great stuff. Could you create an issue to follow-up on this, consistently applying it across the properties panel?

There is other places where we currently validate in the wrong place, and I'd imagine the approach you took could work there, too?

image

barmac commented 8 months ago

I created a follow-up issue: https://github.com/camunda/linting/issues/102#issue-2163570580