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

Property bindings of type "camunda:executionListener" ignores name property #13

Closed
AlexanderSkrock opened this issue Aug 24, 2023 · 5 comments · Fixed by #17
Closed
Labels
enhancement New feature or request spring cleaning Could be cleaned up one day

Comments

@AlexanderSkrock
Copy link
Contributor

AlexanderSkrock commented Aug 24, 2023

Describe the Bug

Element templates including an execution listener binding do not respect the configured property name. In consequence, it currently is not possible to define a class-based execution listener as template.

Per documentation (https://unpkg.com/@camunda/element-templates-json-schema/resources/schema.json) the name attribute at properties/property/binding/name is not specific to any type. In additional this currently breaks the properties panel.

Steps to Reproduce

  1. Create an element template for service task
  2. Add a property binding of type "executionListener"
  3. Set the property name to bind to "class"
  4. Create new task element within Modeler
  5. Apply the configured template
  6. Now the properties panel is frozen and the console shows an error.
  7. The configured class name is not set as "class" property but as "value" which would not work on execution. This leads to the afore mentioned error.

Mine looks similar to this:

{
  "$schema": "https://unpkg.com/@camunda/element-templates-json-schema/resources/schema.json",
  "name": "Name",
  "id": "id",
  "version": 1,
  "appliesTo": [
    "bpmn:Task"
  ],
  "elementType": {
    "value": "bpmn:ServiceTask"
  },
  "properties": [
    {
      "label": "Implementation Type",
      "type": "Hidden",
      "value": "path.to.class",
      "binding": {
        "type": "camunda:executionListener",
        "event": "start",
        "name": "class"
      }
    }
  ]
}

Expected Behavior

The "name" property should be evaluated for execution listeners.

I discovered the behaviour is defined here: CreateHelper.js#L153. Whatever is calculated as parater value is assgined to "value". The default should not point to "value" but rather to "binding.name". If that is not configured "value" seems okay. In theory only the following line would need a change: CreateHelper.js#L173

Environment

  • OS: Windows 10
  • Camunda Desktop Modeler 5.0.0

I would happily try to create a pull request, if that is okay to you.

Kind regards
Alexander Skrock

@philippfromme
Copy link
Contributor

@pinussilvestrus Can you confirm that this was the intended behavior? I can't find a test verifying this.

@pinussilvestrus
Copy link
Contributor

The schema rules seem to not strictly validate the binding properties for each binding type. This was reported via camunda/element-templates-json-schema#109.

According to the docs, name in combination with type=executionListener is indeed not supported. So we can either support it or add stricter validation to avoid confusion.

@AlexanderSkrock
Copy link
Contributor Author

AlexanderSkrock commented Sep 15, 2023

Hi @philippfromme and @pinussilvestrus ,

you arguments sound reasonable. Maybe the name attribute was not originally made for this use case. Still, I would appreciate if your decision includes that we enable class-based execution listeners for element templates.

Another approach would be to use a more specific attribute such as class. However I thought using value in combination with name would be a good idea because it is very similar how script-based execution listeners work. They also use value for the code to execute and scriptFormat to define we have a script and the scripting format.

Kind regards
Alexander Skrock

@philippfromme philippfromme added enhancement New feature or request spring cleaning Could be cleaned up one day and removed bug Something isn't working labels Sep 19, 2023
@barmac barmac added the backlog Queued in backlog label Sep 26, 2023
@barmac
Copy link
Member

barmac commented Sep 26, 2023

I'm moving this to backlog as a potential enhancement. We are also happy to accept external contributions.

@AlexanderSkrock
Copy link
Contributor Author

Thank you for your evaluation and I would be happy to contribute, @barmac .

I just realized I deleted my fork, when cleaning up my repos and forgot about the open pull request. Luckily I still had my changes locally and was able to open a new pull request.

barmac added a commit to camunda/element-templates-json-schema that referenced this issue Oct 10, 2023
…ripts

This adds support for `camunda:executionListener` binding of each supported implementation type.
Backwards compatibility is maintained while ensuring the new property is of correct value.

Related to bpmn-io/bpmn-js-element-templates#13

---------

Co-authored-by: Maciej Barelkowski <[email protected]>
barmac added a commit that referenced this issue Oct 12, 2023
…roperty binding

This also updates `@bpmn-io/element-templates-validator` to v1.2.0.

Closes #13

Co-authored-by: Maciej Barelkowski <[email protected]>
barmac added a commit that referenced this issue Oct 12, 2023
This also updates `@bpmn-io/element-templates-validator` to v1.2.0.

Closes #13

Co-authored-by: Maciej Barelkowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request spring cleaning Could be cleaned up one day
Projects
None yet
4 participants