It seems that when the analysis is done on an entity that has the yoast seo field, validation errors are still displayed to the user. This creates issues when creating new entities for example, where the title is marked as required, but there is no title yet, and also when adding paragraphs etc.

It seems that the code in /src/Form/AnalysisFormHandler.php calls:
$this->messenger->all();

When it seems to me that it should call:

$this->messenger->deleteAll();

This fixes it for us.

CommentFileSizeAuthor
#2 fix_form_validation_errors-2980480-2.patch970 bytesvermario
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vermario created an issue. See original summary.

vermario’s picture

vermario CreditAttribution: vermario at Wunder commented
Assigned: vermario » Unassigned
Status: Active » Needs review
FileSize
fix_form_validation_errors-2980480-2.patch970 bytes

Attaching patch.

K3vin_nl’s picture

K3vin_nl CreditAttribution: K3vin_nl as a volunteer commented

I have tested the patch.
It does seem to fix some issues, where it would give (outdated) errors and prevent the entity from being saved correctly.

However I still have an issue that prevents changes to a paragraph from being saved.

I'm not sure (yet) if that is related to issue. If I remove the Real time SEO field from the form the paragraph changes do save, so it does at least seem to be related to the yoast_seo module.

j3ll3nl’s picture

j3ll3nl
he / him
Primary language Dutch
Location Rotterdam
CreditAttribution: j3ll3nl at FRMWRK commented

I don't get why `$this->messenger->all()` is used.

In my opinion
`'#limit_validation_errors' => [], should be used.

The only problem is that it doesn't work for nested entities (paragraphs in this case)

j3ll3nl’s picture

j3ll3nl
he / him
Primary language Dutch
Location Rotterdam
CreditAttribution: j3ll3nl at FRMWRK commented
Priority: Normal » Major
j3ll3nl’s picture

j3ll3nl
he / him
Primary language Dutch
Location Rotterdam
CreditAttribution: j3ll3nl at FRMWRK commented
Status: Needs review » Needs work
Kingdutch’s picture

Kingdutch
he/him
Primary language English
CreditAttribution: Kingdutch at Open Social commented
Priority: Major » Normal
Status: Needs work » Fixed

I have committed the patch from #2 as it's indeed correct. This is an issue that was introduced with the conversion from the old drupal_get_messages() calls in #2971920: Deprecated drupal_get_messages() where I missed this change in behaviour.

I'll leave #2992284: Paragraphs fields not updated on node saving open for the other issue which seems to be unrelated from the error messages showing.

I agree that in an ideal world using the #limit_validation_errors key would be used but unfortunately this has not yet proven to e implementable in a reliable manner so I'm considering that part of this story a won't fix. Of course if someone can show a proper method to use it that supports Paragraphs then they can open a new issue.

Thanks! :)

  • Kingdutch committed 9fb7e37 on 8.x-2.x authored by vermario 
    Issue #2980480 by vermario: Validation errors in forms are not really...

Status: Fixed » Closed (fixed)

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

damontgomery’s picture

damontgomery CreditAttribution: damontgomery at Palantir.net for Tableau commented

I'm not sure what better options there are, but we are having a number of issues related to this. The general idea that we preview a node and then "clear" the errors before the users sees them I think is the issue.

We are getting that "Title field is required." if you try to add a paragraph while this module is in the middle of doing its preview. I haven't confirmed that it's a race condition, but I believe this is what is happening.

With metatags that use the title token on our site, metatags is also throwing a 500 error.