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

Trailing whitespaces lead to failures during the execution #2385

Closed
saig0 opened this issue Aug 2, 2021 · 11 comments
Closed

Trailing whitespaces lead to failures during the execution #2385

saig0 opened this issue Aug 2, 2021 · 11 comments
Assignees
Labels
BPMN Camunda 7 Flags an issue as related to Camunda Automation Platform 7 Camunda 8 Flags an issue as related to Camunda 8 channel:customer enhancement New feature or request spring cleaning Could be cleaned up one day ux
Milestone

Comments

@saig0
Copy link
Member

saig0 commented Aug 2, 2021

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 bpmn-io/properties-panel#309

@saig0 saig0 added BPMN enhancement New feature or request Camunda 8 Flags an issue as related to Camunda 8 labels Aug 2, 2021
@ingorichtsmeier
Copy link

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

@ingorichtsmeier ingorichtsmeier added the Camunda 7 Flags an issue as related to Camunda Automation Platform 7 label Aug 3, 2021
@pinussilvestrus
Copy link
Contributor

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

/cc @andreasgeier

@pinussilvestrus pinussilvestrus added the backlog Queued in backlog label Aug 5, 2021
@andreasgeier
Copy link

andreasgeier commented Aug 5, 2021

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
Copy link
Member

nikku commented Aug 16, 2021

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
Copy link
Contributor

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
Copy link
Member

nikku commented Aug 17, 2021

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

@nikku
Copy link
Member

nikku commented Nov 12, 2021

Confirmed again by internal feedback (@mschoe).

@hkupitz
Copy link
Contributor

hkupitz commented Oct 21, 2022

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 CatalinaMoisuc added ready Ready to be worked on and removed backlog Queued in backlog labels Jan 23, 2023
@CatalinaMoisuc
Copy link
Member

CatalinaMoisuc commented Jan 23, 2023

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.

@nikku nikku added the ux label Jan 23, 2023
@CatalinaMoisuc
Copy link
Member

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

@CatalinaMoisuc CatalinaMoisuc added channel:customer backlog Queued in backlog spring cleaning Could be cleaned up one day and removed ready Ready to be worked on labels Apr 4, 2023
@nikku nikku added the fixed upstream Requires integration of upstream change label Feb 25, 2025 — with bpmn-io-tasks
@nikku nikku removed the backlog Queued in backlog label Feb 25, 2025
@nikku
Copy link
Member

nikku commented Feb 25, 2025

Fixed upstream via bpmn-io/properties-panel#309. We now automatically trim leading and trailing whitespace for anything but expressions.

@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending in progress Currently worked on and removed fixed upstream Requires integration of upstream change needs review Review pending in progress Currently worked on labels Mar 4, 2025
nikku pushed a commit that referenced this issue Mar 4, 2025
@nikku nikku closed this as completed in 43d1e2a Mar 4, 2025
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Mar 4, 2025
@github-actions github-actions bot added this to the M87 milestone Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BPMN Camunda 7 Flags an issue as related to Camunda Automation Platform 7 Camunda 8 Flags an issue as related to Camunda 8 channel:customer enhancement New feature or request spring cleaning Could be cleaned up one day ux
Projects
None yet
Development

No branches or pull requests

9 participants