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

Add process applications feature #4762

Draft
wants to merge 11 commits into
base: develop
Choose a base branch
from
Draft

Conversation

philippfromme
Copy link
Contributor

@philippfromme philippfromme commented Dec 16, 2024

Proposed Changes

Backend

  • adds file-context feature
    • all open files are added to file context
    • metadata is extracted from every supported file type (.bpmn, .dmn, .form, .process-application)
    • process application is opened/closed automatically if opened/closed file is part of process application
    • opening process application watches process application directory

Frontend

  • ProcessApplications feature to open and close process applications automatically
  • process applications is a plugin

✨ New application menu entry

  • to create process application
  • choose directory, empty .process-application file will be created

✨ New (➕) entry

  • to create process application
  • works the same as new application menu entry

✨ New button in status bar

  • shown when open file is part of process application

✨ New overlay

  • shows all files that are part of same process application as open file

✨ Tab groups

  • active tab now has darker background instead of bottom border
  • bottom border for files that are part of process application
  • different border colors for different process applications

Try it out

I've added an example process application: a354d21

Open any file in this directory to see the process application.

To run the Camunda Modeler try npm install && npm start

Follow-ups

Closes #4666
Closes #4675
Closes #4687
Related to #4676
Related to #4664

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Dec 16, 2024
@@ -1136,8 +1134,7 @@ export class App extends PureComponent {
if (
activeTab !== prevState.activeTab ||
tabs !== prevState.tabs ||
layout !== prevState.layout ||
endpoints !== prevState.endpoints
Copy link
Member

@nikku nikku Dec 16, 2024

Choose a reason for hiding this comment

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

This will be used (again) once we support "endpoints" for deployment.

Why would we remove it here?

Copy link
Member

Choose a reason for hiding this comment

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

(Of course fine, if we consider to store the endpoints in a different place, once supported).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed it because it's not used at all at the moment and was probably meant to be removed. I would rather reintroduce it again in the context of adding a feature that requires it.

Copy link
Member

Choose a reason for hiding this comment

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

We just have to be aware that this is "the data model" that was previously used. We can remove it, but then we'll not remember the data model once we re-introduce an endpoints feature.

@nikku
Copy link
Member

nikku commented Jan 6, 2025

@philippfromme Please add "steps to try out" to this PR, so @lmbateman and others can give it a run (to understand the UX we propose).

@philippfromme philippfromme force-pushed the process-applications branch 4 times, most recently from 5350340 to cbc64dc Compare January 10, 2025 15:15
@philippfromme philippfromme marked this pull request as ready for review January 10, 2025 15:21
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jan 10, 2025
@philippfromme philippfromme changed the title Process applications App process applications feature Jan 10, 2025
@philippfromme philippfromme changed the title App process applications feature Add process applications feature Jan 10, 2025
@jarekdanielak
Copy link
Contributor

jarekdanielak commented Jan 15, 2025

Overall works fine on macOS! 👏

Perhaps if we can now create and open an application process, we could use an option to close it? Similar to close active/all/other tabs:
image

@philippfromme
Copy link
Contributor Author

Perhaps if we can now create and open an application process, we could use an option to close it? Similar to close active/all/other tabs:

You mean Close process application would close all files belonging to the open process application? Interesting idea I haven't thought about. 🤔

@jarekdanielak
Copy link
Contributor

You mean Close process application would close all files belonging to the open process application?

Exactly. 😊

@jarekdanielak
Copy link
Contributor

jarekdanielak commented Jan 15, 2025

When I try to deploy a file from a process application, the modeler just reloads.

Same thing happens when I save a file. Both only happens to files in a process application, the rest works normally.

Screen.Recording.2025-01-15.at.11.56.30.mov

@philippfromme
Copy link
Contributor Author

When I try to deploy a file from a process application, the modeler just reloads.

Same thing happens when I save a file. Both only happens to files in a process application, the rest works normally.

Screen.Recording.2025-01-15.at.11.56.30.mov

This happens becasue the modeler reloads when something in resources changes. 🙈 Opening a process application outside will prevent this.

@barmac
Copy link
Collaborator

barmac commented Jan 15, 2025

I've added the bpmn/dmn-moddle libraries to the app so that a built application can run: 70f0555

@barmac barmac force-pushed the process-applications branch from 70f0555 to 1653967 Compare January 15, 2025 15:45
@barmac

This comment was marked as resolved.

@barmac
Copy link
Collaborator

barmac commented Jan 15, 2025

I'll try to help with this one.

@barmac

This comment was marked as resolved.

@barmac
Copy link
Collaborator

barmac commented Jan 15, 2025

@philippfromme were you able to run a local build of this? So the one from npm run build.

On my machine, the build job results in the source code of dmn-moddle being included in the asar. This is not coming from node_modules where the package is installed correctly.

To verify the build, besides running the app, you can execute this command for MacOS:
npx asar list dist/mac-arm64/Camunda\ Modeler.app/Contents/Resources/app.asar | grep dmn-moddle
This should show dist in the list. Not sure how to do it on Windows.

@jarekdanielak
Copy link
Contributor

Creating a variable on any task in BPMN model with Variables panel open (or opening it after) crashes.

Screen.Recording.2025-01-15.at.17.36.50.mov

@barmac
Copy link
Collaborator

barmac commented Feb 20, 2025

Also, we assume that a form json contains id property. We should try-catch this as null is a valid JSON.

@barmac
Copy link
Collaborator

barmac commented Feb 20, 2025

^ - these are small changes

@barmac
Copy link
Collaborator

barmac commented Feb 20, 2025

Tests pending...

@barmac barmac force-pushed the process-applications branch 2 times, most recently from cb030fb to 6517a80 Compare February 20, 2025 17:21
@nikku nikku added the in progress Currently worked on label Feb 21, 2025 — with bpmn-io-tasks
@nikku nikku removed the needs review Review pending label Feb 21, 2025
@barmac barmac marked this pull request as draft February 21, 2025 09:56
@barmac
Copy link
Collaborator

barmac commented Feb 24, 2025

Symlink issue can only be partially resolved with configuration. Even with followSymlink we report a file twice. There's no loop though.

@barmac barmac force-pushed the process-applications branch from 6517a80 to 6ee48b8 Compare February 24, 2025 13:58
@barmac barmac force-pushed the process-applications branch from 6ee48b8 to 52a094e Compare February 24, 2025 14:00
@barmac barmac force-pushed the process-applications branch from 52a094e to 71c247a Compare February 24, 2025 14:03
@barmac barmac force-pushed the process-applications branch from 71c247a to 63b099c Compare February 24, 2025 14:05
@barmac barmac force-pushed the process-applications branch from 63b099c to 13dfaf6 Compare February 24, 2025 16:01
@barmac
Copy link
Collaborator

barmac commented Feb 25, 2025

Symlink issue can only be partially resolved with configuration. Even with followSymlink we report a file twice. There's no loop though.

OK somehow it's good now o.O

@philippfromme
Copy link
Contributor Author

philippfromme commented Feb 26, 2025

Also, we assume that a form json contains id property. We should try-catch this as null is a valid JSON.

A form always has an ID by default and you cannot remove it through the properties panel. If we want to account for cases that cannot be modeled we should also account for processes and decisions with no ID and add the same error handling to the BPMN and DMN processor. 🤔

try {
  const form = JSON.parse(item.file.contents); // if the form is null
  formId = form.id; // then this will throw

  assert(formId, 'Form must have an id'); // else this assumes that the form is not null but has no ID which cannot be modeled
} catch (error) {
  return {
    type: 'form',
    error: error.message,
    ids: []
  };
}

@philippfromme
Copy link
Contributor Author

With the new RPA tab merged in the meantime should we add these as recognized process application items?

@jarekdanielak
Copy link
Contributor

With the new RPA tab merged in the meantime should we add these as recognized process application items?

Perhaps only if RPA is enabled with the flag?

@barmac
Copy link
Collaborator

barmac commented Feb 26, 2025

With the new RPA tab merged in the meantime should we add these as recognized process application items?

Eventually, yes. But it's also OK to add the support later, when we understand the story of RPA linking. AFAIK it was done via element templates so far which indicates that we may be missing something (@marstamm correct me if I'm wrong).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Currently worked on
Projects
None yet
5 participants