Currently the admin messages give a wrong impression if a model was successfully saved or not:
I've set up a model with two StartEvents, one Task, and saved. After that I've added two Flows. each with the Tamper: Find replace REGEX
template. If i save the model i get the admin notices shown in the screenshot above. From a usability standpoint those messages are misleading. The overall dominant message is the green status message "successfully saved the model". Me as a user assume, in case i even read any further after i'Ve scanned the first green status message that the model was successfully saved, that i could tackle the two errors at a later point, and the other changes to the model are applied and saved. but as i've initially learned in here https://www.drupal.org/project/bpmn_io/issues/3302271 as soon as the action mentioned is in and isn't properly set up no changes are saved at all in general. Which is a potential source for data loss for the user in the worst case.
Proposed resolution
- Remove the "successfully saved the model" status message.
- Perhaps unify the admin notice into a single message? A suggestion for the error text could be:
singular:
Error message
The model could not be saved. The following problem that prevents saving needs your attention:
- action "Tamper": Find replace REGEX" (Flow_1vue8i3): Invalid regular expression.
plural:
Error message
The model could not be saved. The following problems that prevent saving need your attention:
- action "Tamper": Find replace REGEX" (Flow_1vue8i3): Invalid regular expression.
- action "Tamper": Find replace REGEX" (Flow_1fx5wc2): Invalid regular expression.
Comment | File | Size | Author |
---|---|---|---|
successful_save.png | 391.19 KB | rkoller |
Issue fork eca-3302292
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
rkollerComment #3
jurgenhaasThis sounds like a bug and I'll have a look.
Comment #4
jurgenhaasConfirmed, it's a bug and changing the title accordingly.
Comment #6
jurgenhaasThis MR fixes the issue: when the model validation identities one or more errors, the success message will not be printed anymore. Instead, an extra error message is being printed.
Also, if the save command identifies a need for a page reload, e.g. because of a title change, this page reload will now also prevented when the model contains an error.
Comment #7
rkollerthanks for the MR!
successfully saved model
message is gone and an error message is shown instead - worksComment #8
jurgenhaasI can't reproduce this. I have just added 3 conditions with empty regex and I get 3 error messages. I would assume that maybe the third one is just outside of your screen canvas, if the screen is too small? As we are using the Drupal core mechanism to display messages, I think it's out of scope here to handle a bigger number of messages. That's probably something for a theme then.
The Drupal message system does not know any hierarchy. It knows status, warning and error messages. And that's what we use here. If we wanted to make message handling more sophisticated, then that's an issue of its own. Here we should focus on fixing the original bug.
Comment #9
rkollerI can't reproduce this. I have just added 3 conditions with empty regex and I get 3 error messages. I would assume that maybe the third one is just outside of your screen canvas, if the screen is too small? As we are using the Drupal core mechanism to display messages, I think it's out of scope here to handle a bigger number of messages. That's probably something for a theme then.
uh that was my fault. no idea how it happened but one of the sequenceflows had a correct regex string // entered. that way it was the one off. no idea how that happened. but then everything is fine. sorry for the false alarm :/
ah thanks for the confirmation! i would vote against to roll something bespoke for eca. just wanted to make sure there isnt any way for structuring those status messages in core. i will discuss that over in the ux channel and perhaps open a feature request for core based on the feedback. to discuss and build it into core is the better choice in the long run imho. or perhaps there is already an open issue about it. will do some digging before i bring up the topic in the ux channel.
Comment #12
jurgenhaasThanks @rkoller
I've merged this fix into 1.1.x and also backported it to 1.0.x