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.
Comments
Comment #2
berdirComment #3
miro_dietikerAnd 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.
Comment #4
miro_dietikerFor now, promoting for better prioritization as such validation errors are really annoying when you hit them as a user.
Comment #5
robin.ingelbrecht commentedAfter 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.
Comment #6
robin.ingelbrecht commentedSo 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...
Comment #7
robin.ingelbrecht commentedMade a little woopsie :)
Comment #8
miro_dietikerComment #9
berdirPatch doesn't apply.
Comment #10
marcoscanoTrying 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.
Comment #12
marcoscanoSome minor test tweaks and wording improvement.
Comment #13
nkamala commentedI have rerolled this patch with the latest version.
Comment #15
nkamala commentedUpdated the patch.
Comment #16
nikolaatI 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.
Comment #17
kala4ekA little bit adjusted patch from #16.
Now it also tries to display the actual field name from nested paragraph.
Comment #18
kekkisMarked NR to initiate testing on #17.
Comment #21
gfbarbosa commented#17 works fine here when a required field inside a collapsed paragraph have a null value
Comment #22
maico de jongComment #23
maico de jongComment #24
imaniwat commentedTested 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
Comment #27
berdirConverted 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.
Comment #28
broonJust 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".
Comment #29
berdirThat's unfortunate, missed that, I'd accept a patch to make it conditional with fallback in a new issue.