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.
CommentFileSizeAuthor
successful_save.png391.19 KBrkoller

Issue fork eca-3302292

Command icon 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:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rkoller created an issue. See original summary.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
CreditAttribution: rkoller commented
Issue summary: View changes
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
CreditAttribution: jurgenhaas at LakeDrops commented
Component: User interface » Code
Assigned: Unassigned » jurgenhaas
Category: Task » Bug report
Issue tags: +backport

This sounds like a bug and I'll have a look.

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
CreditAttribution: jurgenhaas at LakeDrops commented
Title: Improve the admin messages - prevent the "successfully saved the model" status message if error messages are returned alongside » Do not show success message when trying to save a model that contains errors

Confirmed, it's a bug and changing the title accordingly.

jurgenhaas opened merge request !212

jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
CreditAttribution: jurgenhaas at LakeDrops commented
Assigned: jurgenhaas » Unassigned
Status: Active » Needs review

This 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.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
CreditAttribution: rkoller commented
Status: Needs review » Needs work

thanks for the MR!

  • i can confirm that the successfully saved model message is gone and an error message is shown instead - works
  • i've also tested to change the name of the model after adding a regex sequenceflow, the reload was also prevented as stated - works
  • one problem i've noticed. i've added one regex sequenceflow and one error message was shown for the action in question. then i've added a second regex sequenceflow and two error messages were shown. but when i've added a third sequenceflow still only two error messages for actions were shown. adding a fourth regex flow lead to three error messages for actions were shown. looks like it is one off as soon as more than two actions have errors somehow? - needs work
  • and i am not sure if it would be possible to change the visual representation. but currently the error message for "the model contains errors and can not be saved" is on the same level like the error messages for the individual action errors. but basically those action errors are child elements of parent "model contains errors" error message. having an hierarchy makes it way easier to process and comprehend for the user.
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
CreditAttribution: jurgenhaas at LakeDrops commented
Status: Needs work » Needs review

one problem i've noticed. i've added one regex sequenceflow and one error message was shown for the action in question. then i've added a second regex sequenceflow and two error messages were shown. but when i've added a third sequenceflow still only two error messages for actions were shown. adding a fourth regex flow lead to three error messages for actions were shown. looks like it is one off as soon as more than two actions have errors somehow? - needs work

I 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.

and i am not sure if it would be possible to change the visual representation. but currently the error message for "the model contains errors and can not be saved" is on the same level like the error messages for the individual action errors. but basically those action errors are child elements of parent "model contains errors" error message. having an hierarchy makes it way easier to process and comprehend for the user.

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.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
CreditAttribution: rkoller commented
Status: Needs review » Reviewed & tested by the community

I 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 :/

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.

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.

  • jurgenhaas committed 8359cb1 on 1.1.x 
    Issue #3302292 by jurgenhaas, rkoller: Do not show success message when...

  • jurgenhaas committed 6a21040 on 1.0.x 
    Issue #3302292 by jurgenhaas, rkoller: Do not show success message when...
jurgenhaas’s picture

jurgenhaas
he/him
Primary language German
Location Gottmadingen
CreditAttribution: jurgenhaas at LakeDrops commented
Status: Reviewed & tested by the community » Fixed

Thanks @rkoller

I've merged this fix into 1.1.x and also backported it to 1.0.x

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.