Make block.settings.inline_block:* fully validatable

Implemented suggestions

Related javascript tests seem to be failing but I can't reproduce the issues manually. Any suggestions?

For the test failures.

One way I like to debug JavaScript tests locally is placing createScreenshots throughout the test to make sure the page is showing what I expected

https://www.drupal.org/docs/develop/automated-testing/phpunit-in-drupal/... and more specifically https://github.com/ddev/ddev-selenium-standalone-chrome?tab=readme-ov-fi... helped a lot with getting insight into the failures.

Back to NR

The lone failure was a build test failure and seems unrelated.

Left some comments on the MR, thanks!

Responded to feedback.

1 small comment but can mark after!

Posted on the PR but I'm not sure about the extra hardening. Other feedback addressed.

Went ahead and added the suggestion as it's what was done for DatabaseStorage. Rest of the feedback appears to be addressed

Current failure is broken HEAD for StatusTest

For the feedback from @larowlan

(was pointed to this issue by @penyaskito at https://drupal.slack.com/archives/C072JMEPUS1/p1759664391938989?thread_t...)

I have a use case for this:

I want to create an custom block type to display remote video and expose it to Canvas. Obviously core Media supports remote video out of the box, so this should be easy. However, I cannot get this inline block into Canvas because

Block plugin settings must opt into strict validation. Use the FullyValidatable constraint. See https://www.drupal.org/node/3404425

Enabling this would 1) solve my specific use case, and 2) open up a world of possibilities with other use cases.

I'll work on this this week

Added some thoughts on how this could work. I feal this would be possible, but before i dive into solutions, rather have some validation on my thoughts.

I think I've found a way to use the Collection constraint from symfony to do this, but since im using new Composite constraints this is kinda blocked on #3521131: Add support for Sequentially constraint since that simplifies how we use composites a lot.

#3521131: Add support for Sequentially constraint has been merged. ^^

Ok, think i got a working implementation, might need some cleaning up. But locally tests make sense and error where they should.

Few things.

Update layout_builder.schema.yml to new constraints. We need NotBlank and NotNull since it could be "null". If we want to only check for existence of the key that wouldnt be required, but i assume NULL is comparable to not there, and the RequiredConstraint doesnt do that but only checks for existence of the key in the value.

Unfortunately we need a MappingCollectionConstraintValidator because of how we pass around typed data.

Failures when other fields are invalid. Seems like something related to the context. Will investigate and extend test coverage.

Simplified a lot, no more overwriting.

Did also fix an issue where Composite constraints trigged validation on all child properties, which is not correct.

Think this works like this, powerfull new tool!

Will wait for OK to make a changerecord.

Couple more comments on the MR.

Thanks for the review. Adjusted to most of the feedback.

Change records made for the new constraints.

Some layout builder tests are failing now that probably need to be investigated to see if they're related to the changes here.

Yeah just came back to see that and set to nw :P

Think the translation issue is unrelated. it also runs green locally.

The other error i had earlier, which was a missing constaint: key on the Constraint, which is deprecated sometime soon. Bad copy and paste there.

green

lgtm

Needs some work on nested validations.

Well that makes tests green, which also tests a subset of the issue we ran into.

We need to do a few more cases though. Noting here what alex proposed so we don't need to look at slack.

I would also want to check whether this has anything to do composite constraints. I wonder what happens if you just double the constraints on a root config schema

^^ Just test that and it recurses twice… so the whole fix for double messages is wrong

The reason you ran into this is that you’re the first to write explicit tests for what happens when you put more that one constraint on a mapping

Another example from Alex

    cat:
      type: mapping
      constraints:
        NotBlank: ~
      mapping:
        type:
          type: string
          constraints:
            Callback:
              callback: [\Drupal\config_test\ConfigValidation, validateCats]
        count:
          type: integer
          constraints:
            Callback:
              callback: [\Drupal\config_test\ConfigValidation, validateCatCount]

So there is a constraint on cat and cat.type

Mostly the issue is, we don't want to recurse into the Mapping (or complexdata really) if we are just validating a constraint. The change in the test branch validates it works for the cases I tested, but we want to make sure other cases go well also.

We can probable test that in ComplexDataConstraintValidator and see if we can break (when just running those on 11.x)

My comment got eaten. :(

Added a new test. Without the changes to RecursiveContextualValidator it fails on call count to constraints.

If i remove the is_root_call condition from RecursiveContextualValidator then ./core/tests/Drupal/Tests/Core/TypedData/RecursiveContextualValidatorTest.php will fail because it is not getting all violations.

Not sure what is next to really check. Seems this (combined with green pipeline) is good enough?

The added counter test looks good, and I think this is pretty close. Have a few comments/questions, mostly about docs.

Fixed feedback, kept one thread open so it is easy to find to know if the change is satisfactory ;)

lgtm

Changes all make sense and as @larowlan pointed out in Slack this will help open a Layout Builder to Canvas upgrade path.

Committed and pushed 1de50d63dcb to 11.x and 774f29f6033 to 11.3.x. Thanks!