camunda / camunda-modeler

An integrated modeling solution for BPMN, DMN and Forms based on bpmn.io.
https://camunda.com/products/modeler
MIT License
1.49k stars 477 forks source link

Run instance plugin does not detect changes in Executable option #1671

Closed oguzeroglu closed 4 years ago

oguzeroglu commented 4 years ago

Describe the Bug

Run instance plugin does not detect changes made in Executable option. It allows deploying/running Non-Executable processes.

Steps to Reproduce

  1. Try deploying/running a executable process
  2. Change the process to non executable
  3. Try to deploy/run again
  4. It still works

Expected Behavior

It should throw an error

Environment

pinussilvestrus commented 4 years ago

So this bug includes two basic scenarios?

Scenario actual behavior expected behavior
Changing Non executable --> Executable throwing start instance error (no executable process) no error
Changing Executable --> Non executable no error throwing start instance error (no executable process)

@oguzeroglu can you verify that?

I was not able to verify the first one. For me, it deployed the newest version of the diagram and then tries to run it.

Kapture 2020-01-28 at 12 53 54

The second scenario is caused by the fact we use the saved processDefinition since the engine returns no process definition if the process is not executable. That's the one from the last successful deployment. We have to do that because the engine also returns no process definition if the diagram has no changes, cf. https://github.com/camunda/camunda-modeler/blob/develop/client/src/plugins/camunda-plugin/shared/CamundaAPI.js#L39 (deploy-changed-only flag is set to true).

If we change the deploy-changed-only to false, it would work, since we would always use the newest process definition directly, and if there's none, e.g. if it's not executable, it would not run at all. That would mean we would have a new process definition version for every deployment. Do we want that?

Another solution for the second scenario would be to verify the executable property beforehand and show the error before it got deployed.

From my perspective, the second scenario is not urgent enough to investigate time on that. (Why should I want to execute a process which I marked as not executable)?

The first scenario we should fix, but I was not able to reproduce it.

pinussilvestrus commented 4 years ago

Updated the issue description since we were not able to reproduce the first scenario.

pinussilvestrus commented 4 years ago

Closed via https://github.com/camunda/camunda-modeler/commit/d3500fed63617e2fc792124ac3a078520fda5e10