Upstream conversion of ECA models not created by bpmn_io

Created on 28 October 2024, about 2 months ago

Problem/Motivation

If ECA models have been created by different modellers or even programmatically, as it happens by AI modules, then they can't be edited in bpmn_io as the canvas information is missing and bpmn_io doesn't know about the objects, their locations and relationships.

Proposed resolution

An upstream conversion is required to build an initial bpmn_io model out from a pure ECA model. There have been Slack discussions about this here and here.

Thanks to @lammensj who found the tool bpmn-js-cli which seems to allow building the model in the canvas from code. When used together with the bpmn-auto-layout package, we may even be able to provide a somehow properly laid out model.

The way to implement this could be, to provide a new controller "Edit with bpmn.io" for models that are owned by a different modeller. That controller would do the same as the current edit controller but also provide the ECA model data in a json variable and also load the two javascript packages so that they can do their work in building the initial model from that data.

This would be like a clone feature, where an ECA model from a different modeller would be cloned into a model owned by the bpmn_io modeller.

Feature request
Status

Active

Version

2.1

Component

Code

Created by

🇩🇪Germany jurgenhaas Gottmadingen

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jurgenhaas
  • 🇧🇪Belgium lammensj

    Looking at it further, it might even be that we don't need that cli-plugin but use the modeler-instance directly. See example: https://github.com/bpmn-io/bpmn-js-examples/blob/main/modeling-api/src/snippets/introduction.js

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Oh, nice, that looks even more promos sing. The CLI part made me feel a bit uncomfortable, but talking to the modeller instance directly like this makes so much sense.

  • 🇧🇪Belgium lammensj

    I just got a working POC where I was able to "clone" an existing ECA-config by opening it in the BPMN.io-modeler and use underlying modeler to create the necessary elements and connect them to each other.
    Now focussing on the auto-layout... Would be cool to show this on DrupalCamp Berlin :-D

  • 🇧🇪Belgium lammensj

    Using bpmn-auto-layout is also working! Now for some more testing...

  • 🇧🇪Belgium lammensj

    Summary

    - Added an additional route and controller which preprocesses the given the ECA-config entity and passes certain mappings via drupalSettings to Javascript
    - Registered this new route as entity operation of ECA
    - Created an additional Javascript class which processes the mapping by creating "Elements" (events, gateways, tasks and conditions) for the root (the Process) and then properly orders them in the BPMN.io-UI

    QA

    - Use the fork of this module
    - Download the package of this ECA-sample: https://ecaguide.org/library/simple/add_role_to_inserted_or_updated_user/
    - Unzip it
    - Alter the "eca.eca.eca_lib_0007.yml"-file (this the ECA-config file, not the model) by changing the modeller to "core"
    - Install the dependencies of this sample (see "dependencies.yml") and the ECA Classic Modeller ( https://www.drupal.org/project/eca_cm )
    - Import the altered config file via the UI
    - Go to the overview of ECA-models, you should notice that it was added
    - On the right, select "Edit with BPMN.io"

    TODO

    - e2e-test with Nightwatch

  • 🇩🇪Germany jurgenhaas Gottmadingen

    I've reviewed the code and this is impressive! Great work @lammensj, very exciting. I will give this a try in a minute but wanted to leave to small but general thoughts:

    • Is "clone" the correct key for what's happening here?
    • This keeps the ID of the original model, which is from a different modeller. That means it overrides a config entity of unknown source. For the context of AI-generated ECA models, that's exactly what we want, but maybe not for others?

    That brought me to the following idea: we do have a fallback modeller plugin. How about a convention that AI tools should generate models with that fallback ID and those models will be converted and overridden. All others will really be cloned so that we keep the original.

    Again, thank you so much. This won't take long to get merged. And the community will love it.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Wow, it works!!!

    Just a small issue: it doesn't change the modeller ID when saving the updated ECA model, and it remains at the path /admin/config/workflow/eca/eca_lib_0029/clone/bpmn-io which will continue to use that new route which triggers an upstream conversion again.

    The expected behavior should be that it updates the modeller ID and redirects to /admin/config/workflow/eca/eca_lib_0029/edit after the initial save that's following the auto-layout.

    And an additional idea, but then for a separate issue: the auto-layout feature is probably worth exposing as a function in the canvas in general. What do you think?

  • 🇧🇪Belgium lammensj

    @jurgenhaas,

    1. Do you prefer "convert"? Might capture the intention better... I guess it's a matter of semantics
    2. I wasn't aware of the fallback-modeller, so yes, I will definitely make sure that the AI Agent is is generating one that uses that fallback. I wasn't planning on giving the LLM the option to choose.
    3. Now that were working towards multiple modellers being able to alter the same ECA-config, the question rises if the admin UI should be altered/optimised? Because you would have multiple rows, concerning the same ECA-config, but different modellers which aren't kept in sync.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    1. Do you prefer "convert"? Might capture the intention better... I guess it's a matter of semantics

    That sounds good, yes.

    3. Now that were working towards multiple modellers being able to alter the same ECA-config, the question rises if the admin UI should be altered/optimised? Because you would have multiple rows, concerning the same ECA-config, but different modellers which aren't kept in sync.

    We probably don't need that. I see 2 different use cases:

    The first one is for models that are not created by bpmn_io and not by fallback, they should be converted and saved under a new ID. That way, the original modeller continues ownership and maintenance of its own model and bpmn_io gets a copy of it under it's own regime.

    The second one is for models that have been created by fallback, i.e. not really a modeller that has an intent for continued maintenance. For those, we only need to convert, change the modeller to bpmn_io and that model is now owned by bpmn_io.

    That solution assumes that if the AI agent creates a model it has no intend to ever update that model, does it?

  • Merge request !77Resolve #3484063 "Upstream conversion of" → (Merged) created by lammensj
  • Pipeline finished with Failed
    about 1 month ago
    Total: 145s
    #336897
  • Pipeline finished with Failed
    about 1 month ago
    Total: 145s
    #338346
  • 🇧🇪Belgium lammensj

    @jurgenhaas, I was able to adjust the Converter and clone the original ECA-entity when it doesn't use the fallback modeller. I'm using the "createDuplicate"-method of the ConfigEntityBase-class. However... The protected $id-property of the ECA-entity doesn't allow null-values, hence the failure in the pipeline. Can this be changed in the ECA-module or should I look for another way of cloning it?

    Btw, this can be manually QA'd already, just follow the same steps as above. Just don't forget to temporarily allow null-values for that $id-property.

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Nice work, again.

    The good news is, the ModellerInterfaces comes with a clone method that is really helpful here. Each modeller will handle the cloning and return a prepared ECA entity. And the Fallback modeller returns the ECA entity as is without cloning. So, that gives you everything you need.

    I've committed the necessary modifications to the MR. Please feel free to revert that commit, if you don't like it.

  • Pipeline finished with Failed
    about 1 month ago
    Total: 149s
    #342425
  • Pipeline finished with Failed
    about 1 month ago
    Total: 150s
    #342435
  • Pipeline finished with Failed
    about 1 month ago
    Total: 164s
    #342573
  • Pipeline finished with Failed
    about 1 month ago
    Total: 171s
    #342617
  • Pipeline finished with Failed
    about 1 month ago
    Total: 258s
    #342620
  • Pipeline finished with Failed
    about 1 month ago
    Total: 177s
    #342627
  • Pipeline finished with Failed
    about 1 month ago
    Total: 170s
    #342643
  • 🇧🇪Belgium lammensj

    @jurgenhaas, I've adjusted the existing kernel test and added a couple of nightwatch tests. You can see the results of the conversion and the auto-layout here: https://git.drupalcode.org/issue/bpmn_io-3484063/-/jobs/3400559/artifacts/browse/web/core/reports/nightwatch/screenshots/Tests/bpmn_io/.

    Is there anything else? Do you require more manual testing?

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Thank you @lammensj for all your hard work on this. It looks really great and I've also fixed the remaining PHPCS and PHPStan tests. I don't think we need any further tests at this point.

    But I found one odd ECA model, that causes an issue: https://ecaguide.org/library/simple/combined_conditions/

    The problem seems to be that there are event objects that have no template associated to them. Do you think that could be fixed?

  • Pipeline finished with Skipped
    about 1 month ago
    #344856
  • 🇩🇪Germany jurgenhaas Gottmadingen

    Turns out, the model above doesn't contain any more elements, so the conversion does exactly what's possible and we merge this now.

    Any potential follow-up issues can then be addressed in separate issues.

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024