Make menu blocks (block.settings.system_menu_block:*) fully validatable

Validate level: must be between 0 and $this->menuTree->maxDepth()→ Range constraint?

Can a range constraint be variable like this, or should we fall back to the same thing with did for weight and say it is somewhere between 0 and PHP_INT_MAX?

$this->menuTree->maxDepth()

Fun fact: \Drupal\Core\Menu\MenuLinkTree::maxDepth() calls \Drupal\Core\Menu\MenuTreeStorage::maxDepth(), which does no computation at all, but merely returns...a constant (\Drupal\Core\Menu\MenuTreeStorage::MAX_DEPTH).

That constant is older than two of my nieces. It was created in #2301239: MenuLinkNG part1 (no UI or conversions): plugins (static + MenuLinkContent) + MenuLinkManager + MenuTreeStorage → and appears to have been completely unchanged since.

So, to get this issue done faster, I propose that we don't even need to worry about having a special constraint to determine the max depth dynamically. Let's just hard-code it, at least for now, based on the value of that constant, which has not been changed in core since it was introduced.

That way, we'll get pretty strong validation done quickly, without a ton of effort, and we could make it a more flexible constraint later in a follow-up.

Wouldn't it be better to make depth optional, aka nullable: true? Then we don't need to assign a special meaning to 0

Not sure we should do that, as it would constitute an API change for...what benefit, exactly? Maybe a separate issue would be better here.

The analysis in #4 looks great! That means we can do it easily with the same flexibility of constraint we have today. +1 to this approach

Wrote the update path. But it does need an explicit validation test.

Don't we also need a constraint adding to depth?

Tests written. Just need to test that ol' update path!

And, added an update path test. I think this is reviewable now.

Tests are green, that's great! I think the new constraint also makes a lot of sense.
There's an upgrade path and the changes to tests also look good.

Just one minor comment about the deprecation notice linking to a CR instead of an issue nid
Fine to self RTBC

Wrote a CR and linked to it from the deprecation message, so back to RTBC per #13.

Was going to ask why we are not calling this Depth instead of MaxLinkDepth and coupling to the menu. But seems getting maxDepth in some other way it rather hard. So guess my thoughts are a no-op.

Minor issue with the post-update hook.

Moving the update path around was a very minor lift, so restoring RTBC here on the assumption that tests will pass.

#4: 🤣🤯 Truly ROFL when you wrote 🤣

MenuLinkDepthConstraintValidator and its test coverage look excellent! 👏

Committed df9baa9 and pushed to 11.x. Thanks!

longwave → closed merge request !11933

@longwave the MR was closed but it doesn't look like this was committed/pushed - was that by mistake?

Gah, crosspost