EntityStorageException when trying to save a link over the maximum depth

Thank you @Upchuk for the patch and test and sorry for the delay. Here's my review:

1. +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php
   @@ -356,8 +377,19 @@ public function save(array $form, FormStateInterface $form_state) {
   +      if ($parent_tree[$parent_id]->depth == 9) {
   +        $form_state->setErrorByName('menu_parent', $this->t("The parent menu link <em>@parent_link</em> has reached the maximum depth of 9.", array('@parent_link' => $form['menu_parent']['#options'][$form_state->getValue('menu_parent')])));
   

   I think it would be nice to get the value of 9 from the relevant class, because as far as I know it would be possible for an alternate menu storage to allow for a greater depth.

2. +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -597,6 +600,48 @@ function addInvalidMenuLink() {
   +        'weight[0][value]' => '0'
   ...
   +      'weight[0][value]' => '0'
   

   Minor: These should have a trailing comma per https://www.drupal.org/coding-standards#array.

Awesome! I think from the test you should be able to do this:

$menu_link_tree = \Drupal::service('menu.link_tree');
$menu_link_tree->maxDepth();

So assuming I'm right the test needs a bit more work :) Thanks @Upchuk!

Thanks @Upchuk! Nitpicks alert:

1. +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php
   @@ -340,8 +361,19 @@ public function save(array $form, FormStateInterface $form_state) {
   +    // Check whether the selected parent has reached its maximum depth of 9.
   
   +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -442,6 +442,9 @@ function doMenuTests() {
   +    // Verify the maximum depth of 9 limitation.
   

   I think these should just say maximum depth instead of referring to 9.

2. +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php
   @@ -340,8 +361,19 @@ public function save(array $form, FormStateInterface $form_state) {
      protected function doValidate(array $form, FormStateInterface $form_state) {
   -    $extracted = $this->pathValidator->getUrlIfValid($form_state->getValue('url'));
   +    // Check whether the selected parent has reached its maximum depth of 9.
   +    $parent_id = strstr($form_state->getValue('menu_parent'), 'menu_link_content');
   +    if ($parent_id) {
   +      $parent_plugin = $this->menuLinkManager->getDefinition($parent_id);
   +      $menu_tree_params = new MenuTreeParameters();
   +      $menu_tree_params->setRoot($parent_plugin['id']);
   +      $parent_tree = $this->menuLinkTree->load($parent_plugin['menu_name'], $menu_tree_params);
   +      if ($parent_tree[$parent_id]->depth == $this->menuLinkTree->maxDepth()) {
   +        $form_state->setErrorByName('menu_parent', $this->t("The parent menu link <em>@parent_link</em> has reached the maximum depth of @depth.", array('@parent_link' => $form['menu_parent']['#options'][$form_state->getValue('menu_parent')], '@depth' => $this->menuLinkTree->maxDepth())));
   +      }
   +    }
    
   +    $extracted = $this->pathValidator->getUrlIfValid($form_state->getValue('url'));
        if (!$extracted) {
          $form_state->setErrorByName('url', $this->t("The path '@link_path' is either invalid or you do not have access to it.", array('@link_path' => $form_state->getValue('url'))));
        }
   

   Minor but adding the blank line underneath $extracted back would leave that part alone and probably make the patch easier to read too :)

3. +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -628,6 +631,52 @@ function addInvalidMenuLink() {
   +   * Attempts to add a menu link which exceeds the maxiumum depth.
   

   Minor: typo - maximum.

4. +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -628,6 +631,52 @@ function addInvalidMenuLink() {
   +  function addInvalidDepthMenuLink() {
   +
   +    $last_link = null;
   

   Minor, extra blank line here at the start of the method.

5. +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -628,6 +631,52 @@ function addInvalidMenuLink() {
   +    // Try to add a tenth level of menu links.
   

   Maybe something along the lines of "Try to add another level of menu links beyond the maximum."?

Thanks for your patience on this one @Upchuk :)

With that out of the way I have a couple thoughts:

1. +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php
   @@ -340,6 +361,18 @@ public function save(array $form, FormStateInterface $form_state) {
   +    $parent_id = strstr($form_state->getValue('menu_parent'), 'menu_link_content');
   +    if ($parent_id) {
   

   These lines look a bit suspect to me now, what if you tried to add a menu_link_content child menu link to a menu structure that already had a depth of 9 but where the parent link wasn't generated by the menu_link_content plugin? I think the validation needs to be flexible enough to handle that case otherwise it would still fail. This may also mean we're missing some test coverage for that case.

2. +++ b/core/modules/menu_link_content/src/Form/MenuLinkContentForm.php
   @@ -340,6 +361,18 @@ public function save(array $form, FormStateInterface $form_state) {
   +        $form_state->setErrorByName('menu_parent', $this->t("The parent menu link <em>@parent_link</em> has reached the maximum depth of @depth.", array('@parent_link' => $form['menu_parent']['#options'][$form_state->getValue('menu_parent')], '@depth' => $this->menuLinkTree->maxDepth())));
   

   When manually testing I noticed that the error message contains the dashes that show depth, I think we probably don't want that. For example:

   The parent menu link ------------------ Yay has reached the maximum depth of 9.

Thanks @pwolanin! That looks promising, and for tests I think they could be built on top of the tests in #28.

I just tested D7 HEAD and the depth is limited on the add menu link form, not just the edit menu link form.

Nice, looking good to me! This is the only thing that jumped out at me, just a docs/coding standard thing:

+++ b/core/modules/menu_ui/src/Tests/MenuTest.php
@@ -628,6 +631,39 @@ function addInvalidMenuLink() {
   /**
+   * Checks to see if new menu links can be added to parents who have reached the
+   * maximum depth.
+   */

This summary line needs to fit in 80 characters per https://www.drupal.org/node/1354#drupal.

Here's a suggestion: "Tests that parent options are limited by depth when adding menu links."

Glad to see this moving :) I would mention we're not supposed to use t() in assert messages:

+++ b/core/modules/menu_ui/src/Tests/MenuTest.php
@@ -648,6 +651,48 @@ function addInvalidMenuLink() {
+    $this->assertNoOption('edit-menu-parent', $value, t('The invalid option is not there.'));
...
+      $this->assertOption('edit-menu-parent', $link, t('The valid option number @number is there.', array('@number' => $key + 1)));

https://www.drupal.org/simpletest-tutorial-drupal7#t

I'm happy to sign off on this, just a couple more small docs/standards things:

1. +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -648,6 +651,48 @@ function addInvalidMenuLink() {
   +    // Check that the last link cannot be selected as parent in the new menu link form.
   ...
   +    // Check that all links but the last can be selected as parent in the new menu link form.
   

   Please wrap these at 80 characters per https://www.drupal.org/node/1354#drupal.

2. +++ b/core/modules/menu_ui/src/Tests/MenuTest.php
   @@ -648,6 +651,48 @@ function addInvalidMenuLink() {
   +    $this->assertNoOption('edit-menu-parent', $value, 'The invalid option is not there');
   

   I can't find a source but I think assertion messages need to end in a period too.

Just for future reference those >80 char comments could be wrapped rather than truncated/reworded :)