Problem/Motivation

When a closed paragraph fails validation, the error message is shown on the parent entity form being saved.

In normal operation, this does not happen. But when, for example, a non-required field is made required, existing content cannot be edited and saved without filling the field first. It is hard to figure out what field is causing the validation error. You will have to open every paragraph (and nested paragraphs) to find it out.

Proposed resolution

Show what paragraph and field triggered the error.

I've attached a patch which resolves the issue for our use-case.

Issue fork paragraphs-3027525

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:

Comments

jsst created an issue. See original summary.

berdir’s picture

Status: Active » Needs review
miro_dietiker’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

And we definitively want to test cover this improvement.
If i remember right, we also already have some tests that cover validation and should be easy to extend.

miro_dietiker’s picture

Priority: Normal » Major

For now, promoting for better prioritization as such validation errors are really annoying when you hit them as a user.

robin.ingelbrecht’s picture

After applying this patch, I get an error when the address module tries to validate a field:

Error: Call to a member function getEntity() on string in Drupal\paragraphs\Plugin\Field\FieldWidget\ParagraphsWidget->massageFormValues() (line 2248 of modules/contrib/paragraphs/src/Plugin/Field/FieldWidget/ParagraphsWidget.php).

The property on which "getEntity()" is executed, is indeed a string which contains the invalid postal code of my address field.

robin.ingelbrecht’s picture

So this is a new patch which does some extra checks... seems to work for me. Don't know if the approach is fool proof though...

robin.ingelbrecht’s picture

Made a little woopsie :)

miro_dietiker’s picture

Status: Needs work » Needs review
berdir’s picture

Status: Needs review » Needs work

Patch doesn't apply.

marcoscano’s picture

Status: Needs work » Needs review
Related issues: +#3002809: Add a visual effect for paragraph fields with errors
StatusFileSize
new4.91 KB

Trying to move this forward.

Earlier patches seem to no longer be relevant since we are not including error messages directly in the widget anymore, we are now relying on the form display form validation helper. Because of this there's no interdiff, it's a totally different patch.

I tried to think different approaches for this, and ended up with the idea that it might just be easier/safer to post-process the error messages and prefix labels when appropriate. Admittedly not the cleanest/most efficient approach possible, but it seemed a good compromise to not having to deal with all different possible ways violation messages are added to the form.

I updated one test but there might be others that need updating too, let's see what the testbot says.

Status: Needs review » Needs work

The last submitted patch, 10: 3027525-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

marcoscano’s picture

Status: Needs work » Needs review
StatusFileSize
new3.28 KB
new4.9 KB

Some minor test tweaks and wording improvement.

nkamala’s picture

StatusFileSize
new4.87 KB

I have rerolled this patch with the latest version.

Status: Needs review » Needs work

The last submitted patch, 13: 3027525-13.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

nkamala’s picture

StatusFileSize
new4.9 KB

Updated the patch.

nikolaat’s picture

StatusFileSize
new2.75 KB

I created a patch that handles errors only for collapsed paragraphs, because I found the previous patch to be buggy handling all $form_state errors.

Labels structure/logic is the same as the patch above, I just replaced default basic collapsed error.

kala4ek’s picture

StatusFileSize
new5.99 KB

A little bit adjusted patch from #16.
Now it also tries to display the actual field name from nested paragraph.

kekkis’s picture

Status: Needs work » Needs review

Marked NR to initiate testing on #17.

The last submitted patch, 15: 3027525-14.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

Status: Needs review » Needs work

The last submitted patch, 17: 3027525_17.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

gfbarbosa’s picture

StatusFileSize
new25.53 KB

#17 works fine here when a required field inside a collapsed paragraph have a null value

maico de jong’s picture

StatusFileSize
new7.12 KB
  • Encountered an error when validation fails on a field without a label property. Fixed by using the getLabel() method.
  • Update test to check for the improved error message contents.
maico de jong’s picture

Status: Needs work » Needs review
imaniwat’s picture

Status: Needs review » Reviewed & tested by the community

Tested on Drupal 10.3.0-dev and Paragraphs 8.x-1.17

Paragraph label is displayed in the error message. Collapsing a required paragraph also triggers validation. We can confirm that the error message is triggered at both the node level and paragraph level.

RTBC

  • Berdir committed 3d8a748c on 8.x-1.x
    Issue #3027525 by Berdir, marcoscano, robin.ingelbrecht, nkamala, jsst,...
berdir’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs tests

Converted to a MR, did manual testing, fixed a fatal error that could happen if the target type is no longer allowed by loading the paragraph type directly to get its label.

There's still room for improvement, specifically when you have multiple nested paragraphs, then the delta is not unique, but it's obviously a big improvement. Merged.

broon’s picture

Just as a note for anyone looking into that change: By adding a sixth parameter to the widget class, this breaks functionality for modules extending Paragraphs, for example "Default Paragraphs".

berdir’s picture

That's unfortunate, missed that, I'd accept a patch to make it conditional with fallback in a new issue.

Status: Fixed » Closed (fixed)

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