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: introduce CasePathResolver #142

Merged
merged 4 commits into from
Oct 11, 2023
Merged

feat: introduce CasePathResolver #142

merged 4 commits into from
Oct 11, 2023

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Oct 8, 2023

This new class is dedicated to path resolution of a single instance/case of a process.

Given a set of elements considered as completed, it is currently able to compute the edges between the provided shapes and the shapes around the provided edges.
This is the first step towards the implementation of more intelligent computing in the future.

In addition

  • refactor the constructor of PathResolver: it now only accepts a ElementsRegistry parameter Previously, it required a BpmnElementsRegistry object which is not needed as the implementation only needs to access to the methods related to the model.
  • main README: add link to the sources of the demo

Covers #16
Closes #22
Covers #24

Notes about the types introduced in this PR

The types of the parameter and the returned value are defined to correspond to the state of the final implementation as it is known today. There should be no discussion of types in this PR as the API is not finalized.
What follows is the current proposal with an overview of the structure. It does not mean that the final implementation will be like this (in particular, do not debate now about a pseudo duplication in the type definition, it is too early as the types may change).

export type CasePathResolverInput = {
    /**
     * The IDs of elements (flowNodes/shapes and flows/edges) that are already completed. Non-existing ids will be silently ignored.
     *
     * `Completed` means that the element has been fully executed or definitively cancelled (for BPM engine that supports it and allow instance continuation of cancelled elements).
     * No further user action or automation will update the element.
     */
    completedIds: string[];

    // see see https://github.com/process-analytics/bv-experimental-add-ons/issues/16
    pendingIds?: string[];
};
export type CasePathResolverOutput = {
    /**
     * The `BpmnSemantic` objects retrieved from the model that relate to the ids passed in {@link CasePathResolverInput}.
     */
    provided: {
        completed: {
            shapes: ShapeBpmnSemantic[];
            edges: EdgeBpmnSemantic[];
        };
        // It is not sure that we will let configure pending edges, see https://github.com/process-analytics/bv-experimental-add-ons/issues/16
        pending?: {
            shapes: ShapeBpmnSemantic[];
            edges: EdgeBpmnSemantic[];
        };
    };
    computed: {
        completed: {
            shapes: ShapeBpmnSemantic[];
            edges: EdgeBpmnSemantic[];
        };
        // This may require a more complex type if we want to provide information about why the elements are considered as candidates only
        // We may also split pending and completed elements
        // See https://github.com/process-analytics/bv-experimental-add-ons/issues/24
        candidates: {
            shapes: ShapeBpmnSemantic[];
            edges: EdgeBpmnSemantic[];
        };
    };
};

It is dedicated to path resolution of a single instance/case of a process.
Given a set of elements considered as completed, it is currently able to compute the edges between
the provided shapes and the shapes around the provided edges.

In addition
  - refactor the constructor of `PathResolver`: it now only accepts a `ElementsRegistry` parameter
  Previously, it required a `BpmnElementsRegistry` object which is not needed as the implementation
  only needs to access to the methods related to the model.
  - main README: add link to the sources of the demo
@tbouffard tbouffard added the enhancement New feature or request label Oct 8, 2023
@github-actions
Copy link

github-actions bot commented Oct 8, 2023

♻️ PR Preview d065c70 has been successfully destroyed since this PR has been closed.

🤖 By surge-preview

[no ci]
import type { EdgeBpmnSemantic, ElementsRegistry, ShapeBpmnSemantic } from 'bpmn-visualization';

// bpmn-visualization does not filter duplicates when passing an ids several times
const filterDuplicates = (ids: string[]): string[] => [...new Set(ids)];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: IMHO this is a bug in bpmn-visualization that we should fix. It applies to all methods of ElementsRegistry taking ids or kinds.
What do you think @csouchet?
If so, I will create an issue.

Copy link
Member

@csouchet csouchet Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, the bpmn-visualization APIs should check the uniqueness of the parameter

Copy link
Member Author

@tbouffard tbouffard Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine, before merging this PR, I am going to create a dedicated issue in bpmn-visualization and another one in bv-experimental-add-ons to update the implementation when the fix is available in a new release of bpmn-visualization.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug created: process-analytics/bpmn-visualization-js#2921
Refactor task created: #150

@tbouffard
Copy link
Member Author

tbouffard commented Oct 10, 2023

Putting in draft, waiting for creating issues related to duplicated results returned by the bpmn-visualization API.
[UPDATE] issues created, so ready for merge.

@tbouffard tbouffard marked this pull request as ready for review October 11, 2023 16:28
@sonarcloud
Copy link

sonarcloud bot commented Oct 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@tbouffard tbouffard merged commit 2c951c8 into main Oct 11, 2023
5 checks passed
@tbouffard tbouffard deleted the feat/add_CasePathResolver branch October 11, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] CasePathResolver: add a method that returns the "full path" i.e compute both Shapes and Edges
2 participants