Follow-up for #3382510: FormStateInterface::setErrorByName() needs not #name but a variation

Arguably this needs tests. OTOH, this is such a trivial change/fix that that seems … excessive.

Why wasn't this caught? Doesn't this have test coverage? Well, we were relying on the test coverage that ✨ Add optional validation constraint support to ConfigFormBase Fixed had introduced: UpdateSettingsFormTest. That explicitly tests this:

    $this->assertTrue($this->assertSession()->fieldExists('update_notify_emails')->hasClass('error'));

But … that just happens to be a case where there is no nesting (no #tree ⇒ TRUE).

seems the suggestion in the MR is to remove the property. Not sure if that deserves it's own follow up or if it's simple enough here.

I've merged this into 📌 ConfigFormBase + validation constraints: support composite form elements Needs review as they are extremely related and really need fixing together.

👍🙏

Well it turns out the other part of 📌 ConfigFormBase + validation constraints: support composite form elements Needs review is super complex. Let's get this done first.

I don't fully understand the entire scope of the slack discussion, but this patch is not very complicated and it looks great. I can't run the test only changes on the MR, but the second MR fails where it is expected to fail. +1

Opened 📌 Replace calling FormState::setErrorByName() with an empty string with a new method Active at catch's request

It took me a few minutes to understand the

if ($property_path === '') {
          // There is a map to a non-existing config key. Try to work backwards.
          $property_path = $violation->getParameters()['@key'] ?? '';
        }

dark magic 🧙

I think we can make it less mysterious 😄

@Wim Leers nice idea - I agree and applied your suggestion.

Pushed a better solution than what I suggested on the MR 🤓

I just committed/pushed this already. Please open a follow-up or maybe we can quick fix it here.

Okay, on it!

This change makes sense to me, and I think it makes the error messages a little clearer. That's not a bad thing. No objections.

Committed and pushed f9109dc8600 to 11.x and 56aeb82270c to 10.2.x. Thanks!

Thanks @Wim Leers

Thanks!

See y'all in 📌 ConfigFormBase + validation constraints: support composite form elements Needs review 😊

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