Problem/Motivation

With a microsite enabled, adding a child node at a depth of 9 or more triggers the following error:

Drupal\Core\Entity\EntityStorageException: The link with ID entity_hierarchy_microsite:88540548-85f7-4ff2-b7db-324a5dbac241 or its children exceeded the maximum depth of 9 in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save()

Steps to reproduce

  1. Create a microsite
  2. Add children down to a depth of 8
  3. Adding more children triggers the above error

Proposed resolution

The microsite module should perhaps not create more menus after the depth limit is reached. Perhaps even alert the user that the maximum depth has been reached. I'll dig into this and have a patch soon-ish.

CommentFileSizeAuthor
#6 3212448-6.patch6.87 KBlarowlan
#6 3212448-6-interdiff.txt3.38 KBlarowlan
#5 interdiff_5.txt3.39 KBmarkgp
#5 3212448-max-depth-5.patch6.95 KBmarkgp
#3 3212448-max-depth.patch4.64 KBmarkgp

Comments

markgp created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

The microsite module should perhaps not create more menus

+1 to that approach

Great find, thanks

markgp’s picture

markgp commented
StatusFileSize
new 3212448-max-depth.patch4.64 KB
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Active » Needs review

Couple of minor things, but this looks solid - thanks

  1. +++ b/modules/entity_hierarchy_microsite/src/EntityHooks.php
@@ -182,13 +182,22 @@ class EntityHooks implements ContainerInjectionInterface {
+    $menu_link_tree = \Drupal::service('menu.link_tree');

    This class uses dependency injection, can we inject this instead of using the global \Drupal singleton 

  2. +++ b/modules/entity_hierarchy_microsite/src/EntityHooks.php
@@ -182,13 +182,22 @@ class EntityHooks implements ContainerInjectionInterface {
-        $this->menuLinkManager->updateDefinition($plugin_id, $definition, FALSE);
-        continue;
+        if ($definition['metadata']['entity_hierarchy_depth'] < $menu_max_depth) {
+          $this->menuLinkManager->updateDefinition($plugin_id, $definition, FALSE);
+        } else {
+          $this->menuLinkManager->removeDefinition($plugin_id);
+        }
+      } else {
+        if ($definition['metadata']['entity_hierarchy_depth'] < $menu_max_depth) {
+          $this->menuLinkManager->addDefinition($plugin_id, $definition);
+        }
       }
-      $this->menuLinkManager->addDefinition($plugin_id, $definition);

    can we keep the continues here, and avoid using else

markgp’s picture

markgp commented
StatusFileSize
new 3212448-max-depth-5.patch6.95 KB
new interdiff_5.txt3.39 KB
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
StatusFileSize
new 3212448-6-interdiff.txt3.38 KB
new 3212448-6.patch6.87 KB

Fixing a whitespace issue and keeping the argument order the same just in case

We can avoid another else here too.

If this passes will cut a new release

  • larowlan committed a74eeea on 3.x 
    Issue #3212448 by markgp, larowlan: Error when adding children below a...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Fixed

This will go out as 3.0.2 shortly

Thanks for your contributions

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

https://www.drupal.org/project/entity_hierarchy/releases/3.0.2

Status: Fixed » Closed (fixed)

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