fix: keep menu items in parent cycles, tighten menu write tests

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7

src/Integrations/WordPress/Menus/MenuTree.php

@@ -21,20 +21,55 @@ final class MenuTree
     public static function build(array $items): array
     {
         $nodes = [];
+        $parentOf = [];
         foreach ($items as $item) {
-            $nodes[(int) $item->ID] = self::node($item);
+            $id = (int) $item->ID;
+            $nodes[$id] = self::node($item);
+            $parentOf[$id] = (int) $item->menu_item_parent;
         }
 
         $children = [];
-        foreach ($items as $item) {
+        foreach (array_keys($nodes) as $id) {
-            $id = (int) $item->ID;
+            $children[self::topLevelParent($id, $parentOf, $nodes)][] = $id;
-            $parent = (int) $item->menu_item_parent;
-            $children[$parent !== $id && isset($nodes[$parent]) ? $parent : 0][] = $id;
         }
 
         return self::attach(0, $nodes, $children);
     }
 
+    /**
+     * The parent to attach $id under: its own menu_item_parent, unless that would
+     * make $id unreachable from the root – a self-parent, a parent missing from
+     * this menu, or a chain of parents that cycles back on itself instead of
+     * reaching the root (0) within count($nodes) steps (e.g. A's parent is B and
+     * B's parent is A). Any of these fall back to the top level (0): a broken or
+     * cyclic parent link must never make an item disappear from the tree.
+     *
+     * @param array<int, int>                  $parentOf
+     * @param array<int, array<string, mixed>> $nodes
+     */
+    private static function topLevelParent(int $id, array $parentOf, array $nodes): int
+    {
+        $parent = $parentOf[$id];
+        if ($parent === $id || ! isset($nodes[$parent])) {
+            return 0;
+        }
+
+        $current = $parent;
+        for ($steps = 0; $steps < count($nodes); $steps++) {
+            if ($current === 0) {
+                return $parent;
+            }
+
+            if (! isset($nodes[$current])) {
+                return 0;
+            }
+
+            $current = $parentOf[$current];
+        }
+
+        return 0;
+    }
+
     /** @return array{id: int, title: string, type: string, object: string, object_id: int, url: string} */
     public static function node(object $item): array
     {

tests/Unit/Integrations/WordPress/Menus/MenuAssignLocationAbilityTest.php

@@ -26,23 +26,39 @@ final class MenuAssignLocationAbilityTest extends TestCase
     {
         Functions\expect('set_theme_mod')->once()->with('nav_menu_locations', ['primary-menu' => 8]);
         $log = Mockery::mock(ChangeLog::class);
-        $log->expects('record')->andReturn(6);
+        $log->expects('record')
+            ->with(
+                'lit/menu-assign-location',
+                'menu_location',
+                'primary-menu',
+                'Menü 8 der Position primary-menu zugewiesen'
+            )
+            ->andReturn(6);
 
         $result = (new MenuAssignLocationAbility($log))->execute(['location' => 'primary-menu', 'menu_id' => 8]);
 
         self::assertSame(3, $result['previous_menu_id']);
         self::assertSame('assigned', $result['changed'][0]['action']);
+        self::assertSame(6, $result['log_id']);
     }
 
     public function testZeroRemovesTheAssignment(): void
     {
         Functions\expect('set_theme_mod')->once()->with('nav_menu_locations', []);
         $log = Mockery::mock(ChangeLog::class);
-        $log->expects('record')->andReturn(7);
+        $log->expects('record')
+            ->with(
+                'lit/menu-assign-location',
+                'menu_location',
+                'primary-menu',
+                'Zuweisung der Position primary-menu entfernt'
+            )
+            ->andReturn(7);
 
         $result = (new MenuAssignLocationAbility($log))->execute(['location' => 'primary-menu', 'menu_id' => 0]);
 
         self::assertSame('unassigned', $result['changed'][0]['action']);
+        self::assertSame(7, $result['log_id']);
     }
 
     public function testUnknownMenuIsNotFound(): void
@@ -54,4 +70,14 @@ final class MenuAssignLocationAbilityTest extends TestCase
 
         self::assertSame('lit_not_found', $result->get_error_code());
     }
+
+    public function testPermissionNeedsThemeOptionRights(): void
+    {
+        Functions\when('current_user_can')->justReturn(false);
+
+        $ability = new MenuAssignLocationAbility(Mockery::mock(ChangeLog::class));
+        $input = ['location' => 'primary-menu', 'menu_id' => 8];
+
+        self::assertSame('lit_forbidden', $ability->permission($input)->get_error_code());
+    }
 }

tests/Unit/Integrations/WordPress/Menus/MenuCreateAbilityTest.php

@@ -54,5 +54,15 @@ final class MenuCreateAbilityTest extends TestCase
 
         self::assertSame(8, $result['menu_id']);
         self::assertSame('created', $result['changed'][0]['action']);
+        self::assertSame(5, $result['log_id']);
+    }
+
+    public function testPermissionNeedsThemeOptionRights(): void
+    {
+        Functions\when('current_user_can')->justReturn(false);
+
+        $ability = new MenuCreateAbility(Mockery::mock(ChangeLog::class));
+
+        self::assertSame('lit_forbidden', $ability->permission(['name' => 'Neu'])->get_error_code());
     }
 }

tests/Unit/Integrations/WordPress/Menus/MenuTreeTest.php

@@ -34,4 +34,21 @@ final class MenuTreeTest extends TestCase
 
         self::assertSame(['Waise', 'Zirkel'], array_column($tree, 'title'));
     }
+
+    public function testParentNotPresentInTheMenuLandsOnTheTopLevel(): void
+    {
+        $tree = MenuTree::build([self::item(40, 12345, 'Verwaist')]);
+
+        self::assertSame(['Verwaist'], array_column($tree, 'title'));
+        self::assertSame([], $tree[0]['children']);
+    }
+
+    public function testATwoNodeParentCycleKeepsBothItemsOnTheTopLevel(): void
+    {
+        $tree = MenuTree::build([self::item(30, 31, 'A'), self::item(31, 30, 'B')]);
+
+        self::assertSame(['A', 'B'], array_column($tree, 'title'));
+        self::assertSame([], $tree[0]['children']);
+        self::assertSame([], $tree[1]['children']);
+    }
 }