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

[FEAT] Only infer incoming/outgoing ids #2852

Closed
tbouffard opened this issue Sep 12, 2023 · 0 comments · Fixed by #2856
Closed

[FEAT] Only infer incoming/outgoing ids #2852

tbouffard opened this issue Sep 12, 2023 · 0 comments · Fixed by #2856
Assignees
Labels
BPMN support Something about the BPMN specification that the lib is already supporting or will support enhancement New feature or request
Milestone

Comments

@tbouffard
Copy link
Member

tbouffard commented Sep 12, 2023

Suggested refactoring

I suggest to refactor how incoming/outgoing ids are retrieved from the BPMN source:

  • not to consider incoming/outoing properties
  • only infer them from the flows source and target references

Rationale

I implemented the properties retrieval as a first implementation #2520.
As incoming/outgoing properties are not mandatory in the model, we then add an "inferring" step to compute the ids that could miss (by checking the source and target attributes of a flow). That way, we ensure we are not missing values. We did this in #2638 which also allow to get information not available in the model (incoming/outoing values for participant/pool and text annotation).

The incoming/outgoing properties can be wrong for instance if an initial diagram with such values is modified with a modeler that don't support them. After some diagram modifications of flow, the ids previously referenced incoming/outgoing may not reference anymore flows, but they could stay in the model if the modeler decide to not touch them.
There is currently no filtering of such wrong values in bpmn-visualization.

However, "retrieving the properties" is useless as "inferring" computes the sames values without the extra wrong values. That's why I suggest to only keep the "inferring" step.

Side effects of the current implementation

The PathResolver provided by bv-experimental-add-ons and all our future path-related APIs will rely on incomings/outoings values, so they must be exact.
In process-analytics/bpmn-visualization-addons#119, we already see that wrong edges/flows can be inferred because of that.

@tbouffard tbouffard added this to the 0.39.1 milestone Sep 12, 2023
@tbouffard tbouffard added the refactoring Code refactoring label Sep 12, 2023
@tbouffard tbouffard moved this to Todo in Roadmap 2023 Sep 12, 2023
@tbouffard tbouffard self-assigned this Sep 15, 2023
@tbouffard tbouffard moved this from Todo to In Progress in Roadmap 2023 Sep 15, 2023
@tbouffard tbouffard changed the title [REFACTOR] Only infer incoming/outgoing ids [FEAT] Only infer incoming/outgoing ids Sep 15, 2023
@tbouffard tbouffard added enhancement New feature or request BPMN support Something about the BPMN specification that the lib is already supporting or will support and removed refactoring Code refactoring labels Sep 15, 2023
@github-project-automation github-project-automation bot moved this from In Progress to Done in Roadmap 2023 Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BPMN support Something about the BPMN specification that the lib is already supporting or will support enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant