camunda / camunda-modeler

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

Trailing whitespaces lead to failures during the execution #2385

Open saig0 opened 3 years ago

saig0 commented 3 years ago

Is your feature request related to a problem? Please describe.

In the Camunda Modeler, I created a BPMN process for Camunda Cloud. The process had a service task with an error boundary event attached to it. By accident, the error code of the error event had trailing whitespace.

The process was deployed successfully and process instances were created. While executing the process instances, error events were thrown from the job worker. These error events were not caught because of the trailing whitespace in the model. As a result, it created incidents.

Besides error events, similar problems can occur on other technical attributes (e.g. message name, job type, etc.).

Describe the solution you'd like

The Camunda Modeler detects trailing whitespace and removes them. I see no valid use case for having trailing whitespace.

Describe alternatives you've considered

Instead of removing trailing whitespace, the Camunda Modeler could show warnings that highlight the problem.

Additional context

Camunda Modeler for Camunda Cloud:

image

We found this behavior in a Gameday.


Similar to #720 Related to https://github.com/bpmn-io/properties-panel/issues/309

ingorichtsmeier commented 3 years ago

Hi. this is a issue that I see on trainings for Camunda Platform from time to time. It's very confusing for beginners.

pinussilvestrus commented 3 years ago

Thanks for reporting. That's something we can likely detect once we have proper (execution platform) linting.

/cc @andreasgeier

andreasgeier commented 3 years ago

IMHO, linting is the second-best option (from a UX perspective). If there is no specific case fur such whitespace, I would prefer to automatically remove them (for instance, onblur or onfocusout). Same for whitespace at the beginning?

Do we already have examples of such "smart behavior" in the Modeler or would that be new?

From a development perspective, of course, linting could be a quick fix since we already plan to provide this feature.

nikku commented 3 years ago

If there is no specific case fur such whitespace, I would prefer to automatically remove them (for instance, onblur or onfocusout).

Agreed. How hard would it be to trim whitespace on blur?

philippfromme commented 3 years ago

I'm curious how that would work. Trimming the whitespace on blur would result in a command, that the user doesn't notice but can then undo. 🤔

nikku commented 3 years ago

@philippfromme https://github.com/bpmn-io/diagram-js/pull/535 would proof useful here. Alternatively we could always keep the current text and trim for saving only (if that is feasible).

nikku commented 3 years ago

Confirmed again by internal feedback (@mschoe).

hkupitz commented 2 years ago

To add to this and bring up the topic again:

In a recent developer training, a trailing whitespace at the end of a message name for a catching message event led to an error that took quite long to resolve. Participants were frustrated not finding the problem and when actually figuring it out, they wondered why trailing spaces are not automatically removed for technical IDs.

Generally participants like to copy IDs and message names from websites by double clicking on them. In that case, very often the browser selects the ID/name with a trailing whitespace which is not noticed and copied straight into the modeler property text field.

image

CatalinaMoisuc commented 1 year ago

A similar issue was reported by a customer via this jira ticket.

This time, due to the trailing whitespaces, the order of the processes in Cockpit is not transparent, the user has no clue that there is a space and the space is saved in the database, thus the processes starting with a whitespace are always the first ones in the list.

Added it to ready to discuss.

CatalinaMoisuc commented 1 year ago

Options we discussed today during planning:

  1. trim on blur or focus leave
  2. have a dedicated lint rule to warn the user about this issue