From b8d03614ff1befbb40a003e43252034ceaf18bbb Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Thu, 21 Sep 2023 18:07:03 -0400
Subject: [PATCH 01/25] =?UTF-8?q?Whoops,=20this=20isn=E2=80=99t=20needed?=
 =?UTF-8?q?=20anymore.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 tests/CP/Navigation/NavTest.php | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/tests/CP/Navigation/NavTest.php b/tests/CP/Navigation/NavTest.php
index 0fe2d54bde4..108276f535f 100644
--- a/tests/CP/Navigation/NavTest.php
+++ b/tests/CP/Navigation/NavTest.php
@@ -485,30 +485,7 @@ public function it_checks_if_has_active_children()
     }
 
     /** @test */
-    public function it_can_get_has_active_children_status_with_custom_resolve_children_pattern()
-    {
-        $collections = Nav::content('Custom Collections Url')
-            ->url('http://localhost/cp/custom/url')
-            ->active('collections*')
-            ->children(function () use (&$pages, &$articles) {
-                return [
-                    $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                    $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-                ];
-            });
-
-        Request::swap(Request::create('http://localhost/cp/collections/articles'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-    }
-
-    /**
-     * @deprecated
-     *
-     * @test
-     */
-    public function it_can_get_has_active_children_status_with_deprecated_active_pattern()
+    public function it_can_get_has_active_children_status_with_custom_is_active_pattern()
     {
         $collections = Nav::content('Custom Collections Url')
             ->url('http://localhost/cp/custom/url')

From 09d5b48f12db99b0536c96a1408362e7e8f7d530 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 20 Oct 2023 21:24:59 -0400
Subject: [PATCH 02/25] Ensure we cache explicit cp nav urls for `isActive()`
 checks on unresolved closures.

---
 src/CP/Navigation/NavBuilder.php | 38 ++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/src/CP/Navigation/NavBuilder.php b/src/CP/Navigation/NavBuilder.php
index 659114ac7d8..57aa43b3f4a 100644
--- a/src/CP/Navigation/NavBuilder.php
+++ b/src/CP/Navigation/NavBuilder.php
@@ -3,6 +3,7 @@
 namespace Statamic\CP\Navigation;
 
 use Exception;
+use Illuminate\Support\Facades\Cache;
 use Statamic\Facades\Preference;
 use Statamic\Facades\User;
 use Statamic\Support\Arr;
@@ -10,6 +11,9 @@
 
 class NavBuilder
 {
+    const ALL_URLS_CACHE_KEY = 'cp-nav-urls-all';
+    const CHILDREN_URLS_CACHE_KEY = 'cp-nav-urls-children';
+
     protected $items = [];
     protected $pendingItems = [];
     protected $withHidden = false;
@@ -55,6 +59,7 @@ public function build($preferences = true)
             ->trackOriginalSectionItems()
             ->applyPreferenceOverrides($preferences)
             ->buildSections()
+            ->ensureCachedUrls()
             ->get();
     }
 
@@ -884,6 +889,39 @@ protected function generateNewItemId($section, $name)
         return (new NavItem)->display($name)->section($section)->id();
     }
 
+    /**
+     * Ensure urls are cached for `isActive()` checks.
+     *
+     * @return $this
+     */
+    protected function ensureCachedUrls()
+    {
+        if (Cache::has(static::ALL_URLS_CACHE_KEY) && Cache::has(static::CHILDREN_URLS_CACHE_KEY)) {
+            return $this;
+        }
+
+        $items = $this->built
+            ->flatMap(fn ($section) => $section['items'])
+            ->each(fn ($item) => $item->resolveChildren());
+
+        $allUrls = $items
+            ->flatMap(function ($item) {
+                return array_merge([$item->url()], $item->children()?->map->url()->all() ?? []);
+            })
+            ->unique()
+            ->values();
+
+        $childrenUrls = $items
+            ->mapWithKeys(function ($item) {
+                return [$item->id() => $item->children()?->map->url()->all() ?? []];
+            });
+
+        Cache::put(static::ALL_URLS_CACHE_KEY, $allUrls);
+        Cache::put(static::CHILDREN_URLS_CACHE_KEY, $childrenUrls);
+
+        return $this;
+    }
+
     /**
      * Get built nav.
      *

From 85462c37ffdda7687ec352d11b3ae2a4c6f1aed1 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 20 Oct 2023 21:25:47 -0400
Subject: [PATCH 03/25] Refactor `isActive()` logic.

---
 src/CP/Navigation/NavItem.php | 37 +++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php
index 37f029a180a..3bf20cf4cd0 100644
--- a/src/CP/Navigation/NavItem.php
+++ b/src/CP/Navigation/NavItem.php
@@ -2,7 +2,7 @@
 
 namespace Statamic\CP\Navigation;
 
-use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Cache;
 use Statamic\Facades\CP\Nav;
 use Statamic\Facades\URL;
 use Statamic\Statamic;
@@ -343,7 +343,7 @@ public function can($ability = null, $arguments = [])
     }
 
     /**
-     * Get whether the nav item is currently active.
+     * Determine whether the nav item is currently active.
      *
      * @return bool
      */
@@ -353,28 +353,45 @@ public function isActive()
             return true;
         }
 
+        // If the current url is not explicitly referenced in CP nav,
+        // check if active descendant using regex pattern instead.
+        if (! Cache::get(NavBuilder::ALL_URLS_CACHE_KEY)?->contains(request()->url())) {
+            return $this->isActiveByPattern();
+        }
+
         return request()->url() === URL::removeQueryAndFragment($this->url);
     }
 
     /**
-     * Get whether the nav item has a currently active child.
+     * Determine whether the nav item has a currently active child.
      *
      * @return bool
      */
     protected function hasActiveChild()
     {
-        if ($this->shouldResolveChildren()) {
-            $this->resolveChildren();
+        if (! $childrenUrls = Cache::get(NavBuilder::CHILDREN_URLS_CACHE_KEY)?->get($this->id())) {
+            return false;
         }
 
-        if (! $this->children() instanceof Collection) {
+        return collect($childrenUrls)
+            ->map(fn ($url) => URL::removeQueryAndFragment($url))
+            ->contains(request()->url());
+    }
+
+    /**
+     * Determine whether the nav item is currently active using the regex technique for deeply nested hierarchical urls.
+     *
+     * @return bool
+     */
+    public function isActiveByPattern()
+    {
+        if (! $this->active) {
             return false;
         }
 
-        return $this
-            ->children()
-            ->filter(fn ($item) => $item->isActive())
-            ->isNotEmpty();
+        $pattern = preg_quote(config('statamic.cp.route'), '#').'/'.$this->active;
+
+        return preg_match('#'.$pattern.'#', request()->decodedPath()) === 1;
     }
 
     /**

From 483aaa0dc173fab5eaec0fdb53d9b27f6a3387b4 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 20 Oct 2023 21:26:11 -0400
Subject: [PATCH 04/25] =?UTF-8?q?This=20isn=E2=80=99t=20being=20used=20any?=
 =?UTF-8?q?more.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/CP/Navigation/NavItem.php | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php
index 3bf20cf4cd0..75718ab3428 100644
--- a/src/CP/Navigation/NavItem.php
+++ b/src/CP/Navigation/NavItem.php
@@ -266,22 +266,6 @@ public function active($pattern = null)
         return $this->fluentlyGetOrSet('active')->value($pattern);
     }
 
-    /**
-     * Determine when to resolve children for `hasActiveChild()` checks.
-     *
-     * @return bool
-     */
-    protected function shouldResolveChildren()
-    {
-        if (! $this->active) {
-            return false;
-        }
-
-        $pattern = preg_quote(config('statamic.cp.route'), '#').'/'.$this->active;
-
-        return preg_match('#'.$pattern.'#', request()->decodedPath()) === 1;
-    }
-
     /**
      * Resolve children closure.
      *

From e767756addc55c5b25608bddbc94598b846af08b Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 20 Oct 2023 21:28:20 -0400
Subject: [PATCH 05/25] Ensure we bust caches when updating cp nav preferences.

---
 src/CP/Navigation/Nav.php                                | 8 ++++++++
 src/CP/Navigation/NavBuilder.php                         | 9 +++++++++
 .../CP/Preferences/Nav/DefaultNavController.php          | 4 ++++
 .../Controllers/CP/Preferences/Nav/RoleNavController.php | 4 ++++
 .../Controllers/CP/Preferences/Nav/UserNavController.php | 5 +++++
 5 files changed, 30 insertions(+)

diff --git a/src/CP/Navigation/Nav.php b/src/CP/Navigation/Nav.php
index e2567a4d705..a3a8e7fe949 100644
--- a/src/CP/Navigation/Nav.php
+++ b/src/CP/Navigation/Nav.php
@@ -102,6 +102,14 @@ public function buildWithoutPreferences($withHidden = false)
         return $this->build(false, $withHidden);
     }
 
+    /**
+     * Clear cached urls.
+     */
+    public function clearCachedUrls()
+    {
+        return NavBuilder::clearCachedUrls();
+    }
+
     /**
      * Make base items.
      *
diff --git a/src/CP/Navigation/NavBuilder.php b/src/CP/Navigation/NavBuilder.php
index 57aa43b3f4a..d957058020c 100644
--- a/src/CP/Navigation/NavBuilder.php
+++ b/src/CP/Navigation/NavBuilder.php
@@ -931,4 +931,13 @@ protected function get()
     {
         return $this->built;
     }
+
+    /**
+     * Clear cached urls.
+     */
+    public static function clearCachedUrls()
+    {
+        Cache::forget(static::ALL_URLS_CACHE_KEY);
+        Cache::forget(static::CHILDREN_URLS_CACHE_KEY);
+    }
 }
diff --git a/src/Http/Controllers/CP/Preferences/Nav/DefaultNavController.php b/src/Http/Controllers/CP/Preferences/Nav/DefaultNavController.php
index f71e1b2d05a..b0fcedc41af 100644
--- a/src/Http/Controllers/CP/Preferences/Nav/DefaultNavController.php
+++ b/src/Http/Controllers/CP/Preferences/Nav/DefaultNavController.php
@@ -41,6 +41,8 @@ public function update(Request $request)
 
         Preference::default()->set('nav', $nav)->save();
 
+        Nav::clearCachedUrls();
+
         $this->success(__('Saved'));
 
         return true;
@@ -50,6 +52,8 @@ public function destroy()
     {
         Preference::default()->remove('nav')->save();
 
+        Nav::clearCachedUrls();
+
         return true;
     }
 }
diff --git a/src/Http/Controllers/CP/Preferences/Nav/RoleNavController.php b/src/Http/Controllers/CP/Preferences/Nav/RoleNavController.php
index 14bb65f0174..ced54f5354d 100644
--- a/src/Http/Controllers/CP/Preferences/Nav/RoleNavController.php
+++ b/src/Http/Controllers/CP/Preferences/Nav/RoleNavController.php
@@ -46,6 +46,8 @@ public function update(Request $request, $handle)
 
         $role->setPreference('nav', $nav)->save();
 
+        Nav::clearCachedUrls();
+
         $this->success(__('Saved'));
 
         return true;
@@ -57,6 +59,8 @@ public function destroy($handle)
 
         $role->removePreference('nav')->save();
 
+        Nav::clearCachedUrls();
+
         return true;
     }
 }
diff --git a/src/Http/Controllers/CP/Preferences/Nav/UserNavController.php b/src/Http/Controllers/CP/Preferences/Nav/UserNavController.php
index a55c9319d33..6adbee28480 100644
--- a/src/Http/Controllers/CP/Preferences/Nav/UserNavController.php
+++ b/src/Http/Controllers/CP/Preferences/Nav/UserNavController.php
@@ -3,6 +3,7 @@
 namespace Statamic\Http\Controllers\CP\Preferences\Nav;
 
 use Illuminate\Http\Request;
+use Statamic\Facades\CP\Nav;
 use Statamic\Facades\User;
 use Statamic\Http\Controllers\Controller;
 
@@ -26,6 +27,8 @@ public function update(Request $request)
 
         User::current()->setPreference('nav', $nav)->save();
 
+        Nav::clearCachedUrls();
+
         $this->success(__('Saved'));
 
         return true;
@@ -35,6 +38,8 @@ public function destroy()
     {
         User::current()->removePreference('nav')->save();
 
+        Nav::clearCachedUrls();
+
         return true;
     }
 }

From 34d8fef9261fa8cb11b1529c682a3180faf97035 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 20 Oct 2023 21:28:33 -0400
Subject: [PATCH 06/25] Tests wip.

---
 tests/CP/Navigation/NavPreferencesTest.php |   8 +-
 tests/CP/Navigation/NavTest.php            | 150 ++++++++++++++++++---
 2 files changed, 136 insertions(+), 22 deletions(-)

diff --git a/tests/CP/Navigation/NavPreferencesTest.php b/tests/CP/Navigation/NavPreferencesTest.php
index 5b3b64510e1..e5389da5820 100644
--- a/tests/CP/Navigation/NavPreferencesTest.php
+++ b/tests/CP/Navigation/NavPreferencesTest.php
@@ -503,8 +503,8 @@ public function it_can_alias_items_within_a_section()
             ],
         ]);
         $this->assertEquals(['Collections', 'Navigation', 'Taxonomies', 'Assets', 'Globals', 'Pages'], $nav->get('Content')->map->display()->all());
-        $this->assertArrayHasKey('Pages', $nav->get('Content')->keyBy->display()->get('Collections')->children()->keyBy->display()->all());
-        $this->assertArrayHasKey('Articles', $nav->get('Content')->keyBy->display()->get('Collections')->children()->keyBy->display()->all());
+        $this->assertArrayHasKey('Pages', $nav->get('Content')->keyBy->display()->get('Collections')->resolveChildren()->children()->keyBy->display()->all());
+        $this->assertArrayHasKey('Articles', $nav->get('Content')->keyBy->display()->get('Collections')->resolveChildren()->children()->keyBy->display()->all());
     }
 
     /** @test */
@@ -596,8 +596,8 @@ public function it_can_alias_items_into_another_section()
             ],
         ]);
         $this->assertEquals(['Dashboard', 'Pages'], $nav->get('Top Level')->map->display()->all());
-        $this->assertArrayHasKey('Pages', $nav->get('Content')->keyBy->display()->get('Collections')->children()->keyBy->display()->all());
-        $this->assertArrayHasKey('Articles', $nav->get('Content')->keyBy->display()->get('Collections')->children()->keyBy->display()->all());
+        $this->assertArrayHasKey('Pages', $nav->get('Content')->keyBy->display()->get('Collections')->resolveChildren()->children()->keyBy->display()->all());
+        $this->assertArrayHasKey('Articles', $nav->get('Content')->keyBy->display()->get('Collections')->resolveChildren()->children()->keyBy->display()->all());
 
         // Aliasing in same section should just copy the item...
         $nav = $this->buildNavWithPreferences([
diff --git a/tests/CP/Navigation/NavTest.php b/tests/CP/Navigation/NavTest.php
index 108276f535f..4c80d55d91e 100644
--- a/tests/CP/Navigation/NavTest.php
+++ b/tests/CP/Navigation/NavTest.php
@@ -401,8 +401,11 @@ public function it_can_use_extend_to_remove_a_default_statamic_nav_item()
     }
 
     /** @test */
-    public function it_checks_if_active()
+    public function it_checks_various_active_patterns()
     {
+        // Ensure urls are not cached so that we can test regex based isActive() checks
+        Nav::clearCachedUrls();
+
         $hello = Nav::create('hello')->url('http://localhost/cp/hello');
         $helloWithQueryParams = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello?params');
         $helloWithAnchor = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello#anchor');
@@ -433,15 +436,15 @@ public function it_checks_if_active()
         $this->assertFalse($hello->isActive());
         $this->assertFalse($helloWithQueryParams->isActive());
         $this->assertFalse($helloWithAnchor->isActive());
-        $this->assertFalse($hell->isActive());
+        $this->assertTrue($hell->isActive());
         $this->assertFalse($localNotCp->isActive());
         $this->assertFalse($external->isActive());
         $this->assertFalse($externalSecure->isActive());
 
         Request::swap(Request::create('http://localhost/cp/hello/test'));
-        $this->assertFalse($hello->isActive());
-        $this->assertFalse($helloWithQueryParams->isActive());
-        $this->assertFalse($helloWithAnchor->isActive());
+        $this->assertTrue($hello->isActive());
+        $this->assertTrue($helloWithQueryParams->isActive());
+        $this->assertTrue($helloWithAnchor->isActive());
         $this->assertFalse($hell->isActive());
         $this->assertFalse($localNotCp->isActive());
         $this->assertFalse($external->isActive());
@@ -471,12 +474,10 @@ public function it_checks_if_has_active_children()
     {
         $collections = Nav::content('Collections')
             ->url('http://localhost/cp/collections')
-            ->children(function () use (&$pages, &$articles) {
-                return [
-                    $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                    $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-                ];
-            });
+            ->children([
+                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
+                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
+            ]);
 
         Request::swap(Request::create('http://localhost/cp/collections/articles'));
         $this->assertTrue($collections->isActive());
@@ -484,23 +485,136 @@ public function it_checks_if_has_active_children()
         $this->assertTrue($articles->isActive());
     }
 
+    /** @test */
+    public function it_can_get_has_active_children_status_with_nested_url_hierarchy()
+    {
+        $collections = Nav::content('Custom Collections Url')
+            ->url('http://localhost/cp/collections')
+            ->children([
+                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
+                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
+            ]);
+
+        Request::swap(Request::create('http://localhost/cp/collections/articles/nested-article'));
+        $this->assertTrue($collections->isActive());
+        $this->assertFalse($pages->isActive());
+        $this->assertTrue($articles->isActive());
+    }
+
     /** @test */
     public function it_can_get_has_active_children_status_with_custom_is_active_pattern()
     {
         $collections = Nav::content('Custom Collections Url')
             ->url('http://localhost/cp/custom/url')
-            ->active('collections*')
-            ->children(function () use (&$pages, &$articles) {
-                return [
-                    $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                    $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-                ];
-            });
+            ->active('collections/.*')
+            ->children([
+                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
+                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
+            ]);
+
+        Request::swap(Request::create('http://localhost/cp/collections/articles'));
+        $this->assertTrue($collections->isActive());
+        $this->assertFalse($pages->isActive());
+        $this->assertTrue($articles->isActive());
+    }
+
+    /** @test */
+    public function it_can_get_has_active_children_status_with_multiple_custom_is_active_patterns()
+    {
+        // TODO
+        $this->markTestSkipped();
+
+        $collections = Nav::content('Custom Collections Url')
+            ->url('http://localhost/cp/custom/url')
+            ->active([
+                'collections',
+            ])
+            ->children([
+                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
+                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
+                $categories = Nav::item('Categories')->url('/cp/taxonomies/categories'),
+            ]);
+
+        Request::swap(Request::create('http://localhost/cp/collections/pages'));
+        $this->assertTrue($collections->isActive());
+        $this->assertTrue($pages->isActive());
+        $this->assertFalse($articles->isActive());
+        $this->assertFalse($categories->isActive());
 
         Request::swap(Request::create('http://localhost/cp/collections/articles'));
         $this->assertTrue($collections->isActive());
         $this->assertFalse($pages->isActive());
         $this->assertTrue($articles->isActive());
+        $this->assertFalse($categories->isActive());
+
+        Request::swap(Request::create('http://localhost/cp/taxonomies/categories'));
+        $this->assertTrue($collections->isActive());
+        $this->assertFalse($pages->isActive());
+        $this->assertFalse($articles->isActive());
+        $this->assertTrue($categories->isActive());
+    }
+
+    /** @test */
+    public function it_can_get_has_active_children_status_with_custom_is_active_url()
+    {
+        // TODO
+        $this->markTestSkipped();
+
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        $this
+            ->actingAs(tap(User::make()->makeSuper())->save())
+            ->get('/cp/collections/articles')
+            ->assertStatus(200);
+
+        $collections = Nav::content('Custom Collections Url')
+            ->url('http://localhost/cp/custom/url')
+            ->active('http://localhost/cp/collections')
+            ->children([
+                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
+                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
+            ]);
+
+        $this->assertTrue($collections->isActive());
+        $this->assertFalse($pages->isActive());
+        $this->assertTrue($articles->isActive());
+    }
+
+    /** @test */
+    public function it_can_get_has_active_children_status_with_multiple_custom_is_active_urls()
+    {
+        // TODO
+        $this->markTestSkipped();
+
+        $collections = Nav::content('Custom Collections Url')
+            ->url('http://localhost/cp/custom/url')
+            ->active([
+                'collections/.*',
+            ])
+            ->children([
+                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
+                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
+                $categories = Nav::item('Categories')->url('/cp/taxonomies/categories'),
+            ]);
+
+        Request::swap(Request::create('http://localhost/cp/collections/pages'));
+        $this->assertTrue($collections->isActive());
+        $this->assertTrue($pages->isActive());
+        $this->assertFalse($articles->isActive());
+        $this->assertFalse($categories->isActive());
+
+        Request::swap(Request::create('http://localhost/cp/collections/articles'));
+        $this->assertTrue($collections->isActive());
+        $this->assertFalse($pages->isActive());
+        $this->assertTrue($articles->isActive());
+        $this->assertFalse($categories->isActive());
+
+        Request::swap(Request::create('http://localhost/cp/taxonomies/categories'));
+        $this->assertTrue($collections->isActive());
+        $this->assertFalse($pages->isActive());
+        $this->assertFalse($articles->isActive());
+        $this->assertTrue($categories->isActive());
     }
 
     /** @test */

From 2c66d0c8f80b5e545488bccae4ff519458cd58ef Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 20 Oct 2023 21:47:28 -0400
Subject: [PATCH 07/25] Cache before applying preference overrides, so that
 cache is applicable to every user/role/etc.

---
 src/CP/Navigation/NavBuilder.php |  5 ++---
 src/CP/Navigation/NavItem.php    | 20 +++++++++++++++-----
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/CP/Navigation/NavBuilder.php b/src/CP/Navigation/NavBuilder.php
index d957058020c..7affe04ef30 100644
--- a/src/CP/Navigation/NavBuilder.php
+++ b/src/CP/Navigation/NavBuilder.php
@@ -57,9 +57,9 @@ public function build($preferences = true)
             ->syncOriginal()
             ->trackCoreSections()
             ->trackOriginalSectionItems()
+            ->ensureCachedUrls()
             ->applyPreferenceOverrides($preferences)
             ->buildSections()
-            ->ensureCachedUrls()
             ->get();
     }
 
@@ -900,8 +900,7 @@ protected function ensureCachedUrls()
             return $this;
         }
 
-        $items = $this->built
-            ->flatMap(fn ($section) => $section['items'])
+        $items = collect($this->items)
             ->each(fn ($item) => $item->resolveChildren());
 
         $allUrls = $items
diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php
index 75718ab3428..86a529bb71f 100644
--- a/src/CP/Navigation/NavItem.php
+++ b/src/CP/Navigation/NavItem.php
@@ -2,6 +2,7 @@
 
 namespace Statamic\CP\Navigation;
 
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Cache;
 use Statamic\Facades\CP\Nav;
 use Statamic\Facades\URL;
@@ -353,13 +354,22 @@ public function isActive()
      */
     protected function hasActiveChild()
     {
-        if (! $childrenUrls = Cache::get(NavBuilder::CHILDREN_URLS_CACHE_KEY)?->get($this->id())) {
-            return false;
+        // If children are already resolved to a collection, just check `isActive()` on each child item.
+        if ($this->children() instanceof Collection) {
+            return $this
+                ->children()
+                ->filter(fn ($item) => $item->isActive())
+                ->isNotEmpty();
+        }
+
+        // If children closure has not been resolved, and children urls are cached, check against cached children.
+        if ($childrenUrls = Cache::get(NavBuilder::CHILDREN_URLS_CACHE_KEY)?->get($this->id())) {
+            return collect($childrenUrls)
+                ->map(fn ($url) => URL::removeQueryAndFragment($url))
+                ->contains(request()->url());
         }
 
-        return collect($childrenUrls)
-            ->map(fn ($url) => URL::removeQueryAndFragment($url))
-            ->contains(request()->url());
+        return false;
     }
 
     /**

From 2e8db19465865454067042051cd2c082d3368949 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 24 Oct 2023 14:08:32 -0400
Subject: [PATCH 08/25] Let `NavBuilder` handle caching.

---
 src/CP/Navigation/NavItem.php | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php
index 86a529bb71f..0d6922ec9d6 100644
--- a/src/CP/Navigation/NavItem.php
+++ b/src/CP/Navigation/NavItem.php
@@ -3,7 +3,6 @@
 namespace Statamic\CP\Navigation;
 
 use Illuminate\Support\Collection;
-use Illuminate\Support\Facades\Cache;
 use Statamic\Facades\CP\Nav;
 use Statamic\Facades\URL;
 use Statamic\Statamic;
@@ -340,7 +339,7 @@ public function isActive()
 
         // If the current url is not explicitly referenced in CP nav,
         // check if active descendant using regex pattern instead.
-        if (! Cache::get(NavBuilder::ALL_URLS_CACHE_KEY)?->contains(request()->url())) {
+        if (! NavBuilder::getAllUrls()->contains(request()->url())) {
             return $this->isActiveByPattern();
         }
 
@@ -363,7 +362,7 @@ protected function hasActiveChild()
         }
 
         // If children closure has not been resolved, and children urls are cached, check against cached children.
-        if ($childrenUrls = Cache::get(NavBuilder::CHILDREN_URLS_CACHE_KEY)?->get($this->id())) {
+        if ($childrenUrls = NavBuilder::getUnresolvedChildrenUrlsForItem($this)) {
             return collect($childrenUrls)
                 ->map(fn ($url) => URL::removeQueryAndFragment($url))
                 ->contains(request()->url());

From 620f03bc733070cd12ae011220574b53b85d278e Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 24 Oct 2023 14:14:50 -0400
Subject: [PATCH 09/25] Fix edge case `isActive` bug when moving an item out of
 a core nav children closure.

---
 src/CP/Navigation/NavBuilder.php | 99 +++++++++++++++++++++++++-------
 1 file changed, 79 insertions(+), 20 deletions(-)

diff --git a/src/CP/Navigation/NavBuilder.php b/src/CP/Navigation/NavBuilder.php
index 7affe04ef30..94963c13885 100644
--- a/src/CP/Navigation/NavBuilder.php
+++ b/src/CP/Navigation/NavBuilder.php
@@ -4,6 +4,7 @@
 
 use Exception;
 use Illuminate\Support\Facades\Cache;
+use Statamic\Facades\Blink;
 use Statamic\Facades\Preference;
 use Statamic\Facades\User;
 use Statamic\Support\Arr;
@@ -11,8 +12,8 @@
 
 class NavBuilder
 {
+    const UNRESOLVED_CHILDREN_URLS_CACHE_KEY = 'cp-nav-urls-unresolved-children';
     const ALL_URLS_CACHE_KEY = 'cp-nav-urls-all';
-    const CHILDREN_URLS_CACHE_KEY = 'cp-nav-urls-children';
 
     protected $items = [];
     protected $pendingItems = [];
@@ -22,6 +23,8 @@ class NavBuilder
     protected $sectionsManipulations = [];
     protected $sectionsOrder = [];
     protected $sectionsWithReorderedItems = [];
+    protected $urlsUnresolvedChildren = [];
+    protected $urlsAll = [];
     protected $built;
 
     /**
@@ -57,9 +60,10 @@ public function build($preferences = true)
             ->syncOriginal()
             ->trackCoreSections()
             ->trackOriginalSectionItems()
-            ->ensureCachedUrls()
+            ->trackUrls()
             ->applyPreferenceOverrides($preferences)
             ->buildSections()
+            ->blinkUrls()
             ->get();
     }
 
@@ -810,6 +814,12 @@ protected function userRemoveItemFromChildren($item)
             return;
         }
 
+        if ($this->urlsUnresolvedChildren->has($parent->id())) {
+            $this->urlsUnresolvedChildren[$parent->id()] = collect($this->urlsUnresolvedChildren[$parent->id()])
+                ->reject(fn ($url) => $url === $item->url())
+                ->all();
+        }
+
         if ($parent->resolveChildren()->children()) {
             $parent->children(
                 $parent->children()->reject(function ($child) use ($item) {
@@ -890,53 +900,102 @@ protected function generateNewItemId($section, $name)
     }
 
     /**
-     * Ensure urls are cached for `isActive()` checks.
+     * Track URLs for `isActive` checks on nav items.
      *
      * @return $this
      */
-    protected function ensureCachedUrls()
+    protected function trackUrls()
     {
-        if (Cache::has(static::ALL_URLS_CACHE_KEY) && Cache::has(static::CHILDREN_URLS_CACHE_KEY)) {
+        // If URLs are already cached, get them from cache so that we don't have to
+        // resolve children closures on every request for performance reasons.
+        if ($this->hasCachedUrls()) {
+            $this->urlsUnresolvedChildren = Cache::get(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY);
+            $this->urlsAll = Cache::get(static::ALL_URLS_CACHE_KEY);
+
             return $this;
         }
 
-        $items = collect($this->items)
-            ->each(fn ($item) => $item->resolveChildren());
+        $this->urlsUnresolvedChildren = collect($this->items)
+            ->filter(fn ($item) => is_callable($item->children()))
+            ->mapWithKeys(function ($item) {
+                return [$item->id() => $item->resolveChildren()->children()?->map->url()->all() ?? []];
+            });
 
-        $allUrls = $items
+        $this->urlsAll = collect($this->items)
             ->flatMap(function ($item) {
-                return array_merge([$item->url()], $item->children()?->map->url()->all() ?? []);
+                return array_merge([$item->url()], $item->resolveChildren()->children()?->map->url()->all() ?? []);
             })
             ->unique()
             ->values();
 
-        $childrenUrls = $items
-            ->mapWithKeys(function ($item) {
-                return [$item->id() => $item->children()?->map->url()->all() ?? []];
-            });
+        Cache::put(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY, $this->urlsUnresolvedChildren);
+        Cache::put(static::ALL_URLS_CACHE_KEY, $this->urlsAll);
+
+        return $this;
+    }
 
-        Cache::put(static::ALL_URLS_CACHE_KEY, $allUrls);
-        Cache::put(static::CHILDREN_URLS_CACHE_KEY, $childrenUrls);
+    /**
+     * Check if cache has URLs.
+     *
+     * @return bool
+     */
+    protected function hasCachedUrls()
+    {
+        return Cache::has(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)
+            && Cache::has(static::ALL_URLS_CACHE_KEY);
+    }
+
+    /**
+     * Blink URLs for `isActive` checks during this request.
+     *
+     * @return $this
+     */
+    protected function blinkUrls()
+    {
+        Blink::put(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY, $this->urlsUnresolvedChildren);
+        Blink::put(static::ALL_URLS_CACHE_KEY, $this->urlsAll);
 
         return $this;
     }
 
     /**
-     * Get built nav.
+     * Get all unresolved children URLs for an item's `isActive` checks.
      *
      * @return \Illuminate\Support\Collection
      */
-    protected function get()
+    public static function getUnresolvedChildrenUrlsForItem($item)
     {
-        return $this->built;
+        return Blink::get(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)?->get($item->id());
+    }
+
+    /**
+     * Get all URLs explicitly used in nav for `isActive` checks.
+     *
+     * @return \Illuminate\Support\Collection
+     */
+    public static function getAllUrls()
+    {
+        return Blink::get(static::ALL_URLS_CACHE_KEY) ?? collect();
     }
 
     /**
-     * Clear cached urls.
+     * Clear cached urls. Important when saving/deleting CP nav preferences, etc.
      */
     public static function clearCachedUrls()
     {
+        Cache::forget(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY);
+        Blink::forget(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY);
         Cache::forget(static::ALL_URLS_CACHE_KEY);
-        Cache::forget(static::CHILDREN_URLS_CACHE_KEY);
+        Blink::forget(static::ALL_URLS_CACHE_KEY);
+    }
+
+    /**
+     * Get built nav.
+     *
+     * @return \Illuminate\Support\Collection
+     */
+    protected function get()
+    {
+        return $this->built;
     }
 }

From 974e8437932978cc623b5776f20d0e34b8940c01 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Wed, 25 Oct 2023 11:36:18 -0400
Subject: [PATCH 10/25] Be smarter about updating closure based children URL
 caches when item is active.

---
 src/CP/Navigation/NavBuilder.php | 57 ++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/src/CP/Navigation/NavBuilder.php b/src/CP/Navigation/NavBuilder.php
index 94963c13885..53b37c261f7 100644
--- a/src/CP/Navigation/NavBuilder.php
+++ b/src/CP/Navigation/NavBuilder.php
@@ -18,6 +18,7 @@ class NavBuilder
     protected $items = [];
     protected $pendingItems = [];
     protected $withHidden = false;
+    protected $itemsWithChildrenClosures = [];
     protected $sections = [];
     protected $sectionsOriginalItemIds = [];
     protected $sectionsManipulations = [];
@@ -52,7 +53,8 @@ public function build($preferences = true)
         }
 
         return $this
-            ->buildChildren()
+            ->trackChildrenClosures()
+            ->resolveChildrenClosures()
             ->validateNesting()
             ->validateViews()
             ->authorizeItems()
@@ -68,11 +70,25 @@ public function build($preferences = true)
     }
 
     /**
-     * Build children closures.
+     * Track children closures.
      *
      * @return $this
      */
-    protected function buildChildren()
+    protected function trackChildrenClosures()
+    {
+        collect($this->items)
+            ->filter(fn ($item) => is_callable($item->children()))
+            ->each(fn ($item) => $this->itemsWithChildrenClosures[] = $item->id());
+
+        return $this;
+    }
+
+    /**
+     * Resolve children closures.
+     *
+     * @return $this
+     */
+    protected function resolveChildrenClosures()
     {
         collect($this->items)
             ->filter(fn ($item) => $item->isActive() || $this->withHidden)
@@ -911,12 +927,13 @@ protected function trackUrls()
         if ($this->hasCachedUrls()) {
             $this->urlsUnresolvedChildren = Cache::get(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY);
             $this->urlsAll = Cache::get(static::ALL_URLS_CACHE_KEY);
+            $this->ensureUrlCachesAreUpToDate();
 
             return $this;
         }
 
         $this->urlsUnresolvedChildren = collect($this->items)
-            ->filter(fn ($item) => is_callable($item->children()))
+            ->filter(fn ($item) => collect($this->itemsWithChildrenClosures)->contains($item->id()))
             ->mapWithKeys(function ($item) {
                 return [$item->id() => $item->resolveChildren()->children()?->map->url()->all() ?? []];
             });
@@ -928,10 +945,38 @@ protected function trackUrls()
             ->unique()
             ->values();
 
+        $this->cacheUrls();
+
+        return $this;
+    }
+
+    /**
+     * Cache tracked URLs.
+     */
+    protected function cacheUrls()
+    {
         Cache::put(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY, $this->urlsUnresolvedChildren);
         Cache::put(static::ALL_URLS_CACHE_KEY, $this->urlsAll);
+    }
 
-        return $this;
+    /**
+     * Ensure URL caches are up to date.
+     */
+    protected function ensureUrlCachesAreUpToDate()
+    {
+        $needsUpdating = collect($this->items)
+            ->filter(fn ($item) => collect($this->itemsWithChildrenClosures)->contains($item->id()))
+            ->filter(fn ($item) => $item->isActive() || $this->withHidden)
+            ->mapWithKeys(fn ($item) => [$item->id() => $item->children()?->map->url()->all() ?? []])
+            ->filter(fn ($urls, $id) => $this->urlsUnresolvedChildren->get($id) != $urls)
+            ->each(function ($urls, $id) {
+                $this->urlsUnresolvedChildren->put($id, $urls);
+            })
+            ->isNotEmpty();
+
+        if ($needsUpdating) {
+            $this->cacheUrls();
+        }
     }
 
     /**
@@ -959,7 +1004,7 @@ protected function blinkUrls()
     }
 
     /**
-     * Get all unresolved children URLs for an item's `isActive` checks.
+     * Get unresolved children URLs for an item's `isActive` checks.
      *
      * @return \Illuminate\Support\Collection
      */

From 4489397fb0db8dc80ddab14d9fdbbcae9f666c90 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Wed, 25 Oct 2023 12:06:53 -0400
Subject: [PATCH 11/25] =?UTF-8?q?Deprecate=20this,=20because=20it=E2=80=99?=
 =?UTF-8?q?s=20confusing=20all=20around.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/CP/Navigation/NavItem.php | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php
index 0d6922ec9d6..cad18dc786c 100644
--- a/src/CP/Navigation/NavItem.php
+++ b/src/CP/Navigation/NavItem.php
@@ -259,6 +259,12 @@ public function isChild($isChild = null)
     /**
      * Active URL pattern to determine when to resolve children for `hasActiveChild()` checks.
      *
+     * Though we still check active patterns for nested URLs internally, having to manually
+     * use this method should not be needed anymore, not to mention it is confusing for
+     * addon devs to know when they even need to use it, thus we are deprecating it.
+     *
+     * @deprecated
+     *
      * @return $this
      */
     public function active($pattern = null)

From dd00e79e581a6a19d853bf0322f220e678ea038d Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Wed, 25 Oct 2023 14:37:59 -0400
Subject: [PATCH 12/25] Add arg to easily remove child CP nav item.

---
 src/CP/Navigation/Nav.php       | 49 +++++++++++++++++++++++---
 tests/CP/Navigation/NavTest.php | 62 +++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+), 4 deletions(-)

diff --git a/src/CP/Navigation/Nav.php b/src/CP/Navigation/Nav.php
index e2567a4d705..d8908393489 100644
--- a/src/CP/Navigation/Nav.php
+++ b/src/CP/Navigation/Nav.php
@@ -45,12 +45,13 @@ public function item($name)
     }
 
     /**
-     * Find or create nav item.
+     * Find nav item.
      *
      * @param  string  $section
      * @param  string  $name
+     * @return NavItem|null
      */
-    public function findOrCreate($section, $name)
+    public function find($section, $name)
     {
         $item = collect($this->items)->first(function ($item) use ($section, $name) {
             return $item->section() === $section
@@ -58,7 +59,19 @@ public function findOrCreate($section, $name)
                 && ! $item->isChild();
         });
 
-        return $item ?: $this->create($name)->section($section);
+        return $item;
+    }
+
+    /**
+     * Find or create nav item.
+     *
+     * @param  string  $section
+     * @param  string  $name
+     * @return NavItem
+     */
+    public function findOrCreate($section, $name)
+    {
+        return $this->find($section, $name) ?: $this->create($name)->section($section);
     }
 
     /**
@@ -66,10 +79,15 @@ public function findOrCreate($section, $name)
      *
      * @param  string  $section
      * @param  string|null  $name
+     * @param  string|null  $childName
      * @return $this
      */
-    public function remove($section, $name = null)
+    public function remove($section, $name = null, $childName = null)
     {
+        if ($childName) {
+            return $this->removeChildItem($section, $name, $childName);
+        }
+
         $this->items = collect($this->items)
             ->reject(function ($item) use ($section, $name) {
                 return $name
@@ -81,6 +99,29 @@ public function remove($section, $name = null)
         return $this;
     }
 
+    /**
+     * Remove nav item.
+     *
+     * @param  string  $section
+     * @param  string|null  $name
+     * @param  string|null  $childName
+     * @return $this
+     */
+    protected function removeChildItem($section, $name, $childName)
+    {
+        if (! $parent = $this->find($section, $name)) {
+            return $this;
+        }
+
+        if (! $children = $parent->resolveChildren()->children()) {
+            return $this;
+        }
+
+        $parent->children($children->reject(fn ($child) => $child->display() === $childName));
+
+        return $this;
+    }
+
     /**
      * Build navigation.
      *
diff --git a/tests/CP/Navigation/NavTest.php b/tests/CP/Navigation/NavTest.php
index 491a8f3c83d..f399b662302 100644
--- a/tests/CP/Navigation/NavTest.php
+++ b/tests/CP/Navigation/NavTest.php
@@ -344,6 +344,41 @@ public function it_can_remove_a_specific_nav_item()
         $this->assertEquals('A-Wing', $ships->first()->display());
     }
 
+    /** @test */
+    public function it_can_remove_a_specific_nav_child_item()
+    {
+        $this->actingAs(tap(User::make()->makeSuper())->save());
+
+        Nav::ships('Y-Wing')
+            ->url('/y-wing')
+            ->icon('y-wing')
+            ->children(function () {
+                return [
+                    Nav::item('Foo'),
+                    Nav::item('Bar'),
+                ];
+            });
+
+        Nav::ships('A-Wing')
+            ->url('/a-wing')
+            ->icon('a-wing')
+            ->children(function () {
+                return [
+                    Nav::item('Foo'),
+                    Nav::item('Bar'),
+                ];
+            });
+
+        $this->assertCount(2, $this->build()->get('Ships'));
+
+        Nav::remove('Ships', 'Y-Wing', 'Foo');
+
+        $this->assertCount(2, $ships = $this->build()->get('Ships'));
+
+        $this->assertEquals(['Bar'], $ships->first()->resolveChildren()->children()->map->display()->all());
+        $this->assertEquals(['Foo', 'Bar'], $ships->last()->resolveChildren()->children()->map->display()->all());
+    }
+
     /** @test */
     public function it_can_use_extend_to_defer_until_after_statamic_core_nav_items_are_built()
     {
@@ -378,6 +413,33 @@ public function it_can_use_extend_to_remove_a_default_statamic_nav_item()
         $this->assertNotContains('Collections', $this->build()->get('Content')->map->display());
     }
 
+    /** @test */
+    public function it_can_use_extend_to_remove_a_default_statamic_child_nav_item()
+    {
+        Facades\Collection::make('articles')->save();
+        Facades\Collection::make('pages')->save();
+
+        $this->actingAs(tap(User::make()->makeSuper())->save());
+
+        $nav = Nav::build();
+
+        $collectionsChildren = function () {
+            return $this->build()
+                ->get('Content')
+                ->first(fn ($item) => $item->display() === 'Collections')
+                ->resolveChildren()
+                ->children();
+        };
+
+        $this->assertEquals(['Articles', 'Pages'], $collectionsChildren()->map->display()->all());
+
+        Nav::extend(function ($nav) {
+            $nav->remove('Content', 'Collections', 'Articles');
+        });
+
+        $this->assertEquals(['Pages'], $collectionsChildren()->map->display()->all());
+    }
+
     /** @test */
     public function it_checks_if_active()
     {

From 8ef2e15f466c6213d2f46e94b42eb5f61bdbf9bc Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Wed, 25 Oct 2023 14:38:24 -0400
Subject: [PATCH 13/25] Test new public `find()` method as well.

---
 tests/CP/Navigation/NavTest.php | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tests/CP/Navigation/NavTest.php b/tests/CP/Navigation/NavTest.php
index f399b662302..3b9c9f885b6 100644
--- a/tests/CP/Navigation/NavTest.php
+++ b/tests/CP/Navigation/NavTest.php
@@ -124,7 +124,7 @@ public function it_can_create_a_nav_item_with_a_custom_svg_icon()
     }
 
     /** @test */
-    public function it_can_get_and_modify_an_existing_item()
+    public function it_can_find_and_modify_an_existing_item()
     {
         $this->actingAs(tap(User::make()->makeSuper())->save());
 
@@ -132,6 +132,27 @@ public function it_can_get_and_modify_an_existing_item()
             ->url('/pit-droid')
             ->icon('<svg>...</svg>');
 
+        Nav::find('Droids', 'WAC-47')
+            ->url('/d-squad');
+
+        $item = $this->build()->get('Droids')->first();
+
+        $this->assertEquals('Droids', $item->section());
+        $this->assertEquals('WAC-47', $item->display());
+        $this->assertEquals('<svg>...</svg>', $item->icon());
+        $this->assertEquals('http://localhost/d-squad', $item->url());
+    }
+
+    /** @test */
+    public function it_can_find_and_modify_an_existing_item_using_magic_constructor()
+    {
+        $this->actingAs(tap(User::make()->makeSuper())->save());
+
+        Nav::droids('WAC-47')
+            ->url('/pit-droid')
+            ->icon('<svg>...</svg>');
+
+        // Callign the same constructor does a `findOrCreate()` under the hood...
         Nav::droids('WAC-47')
             ->url('/d-squad');
 

From 7774a56102df3ad3c96d81fe6c5e93fcc9296cb3 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 27 Oct 2023 14:27:53 -0400
Subject: [PATCH 14/25] Fix more edge cases.

---
 src/CP/Navigation/NavBuilder.php | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/CP/Navigation/NavBuilder.php b/src/CP/Navigation/NavBuilder.php
index 53b37c261f7..4219e56c2e5 100644
--- a/src/CP/Navigation/NavBuilder.php
+++ b/src/CP/Navigation/NavBuilder.php
@@ -964,21 +964,32 @@ protected function cacheUrls()
      */
     protected function ensureUrlCachesAreUpToDate()
     {
-        $needsUpdating = collect($this->items)
+        $updated = collect($this->items)
             ->filter(fn ($item) => collect($this->itemsWithChildrenClosures)->contains($item->id()))
             ->filter(fn ($item) => $item->isActive() || $this->withHidden)
             ->mapWithKeys(fn ($item) => [$item->id() => $item->children()?->map->url()->all() ?? []])
             ->filter(fn ($urls, $id) => $this->urlsUnresolvedChildren->get($id) != $urls)
-            ->each(function ($urls, $id) {
-                $this->urlsUnresolvedChildren->put($id, $urls);
-            })
+            ->each(fn ($urls, $id) => $this->trackChangedChildren($id, $urls))
             ->isNotEmpty();
 
-        if ($needsUpdating) {
+        if ($updated) {
             $this->cacheUrls();
         }
     }
 
+    /**
+     * Track changed children URLs.
+     */
+    protected function trackChangedChildren($id, $urls)
+    {
+        $this->urlsUnresolvedChildren->put($id, $urls);
+
+        $this->urlsAll = $this->urlsAll
+            ->merge($urls)
+            ->unique()
+            ->values();
+    }
+
     /**
      * Check if cache has URLs.
      *
@@ -1010,7 +1021,8 @@ protected function blinkUrls()
      */
     public static function getUnresolvedChildrenUrlsForItem($item)
     {
-        return Blink::get(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)?->get($item->id());
+        return Blink::get(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)?->get($item->id())
+            ?? Cache::get(static::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)?->get($item->id());
     }
 
     /**
@@ -1020,7 +1032,9 @@ public static function getUnresolvedChildrenUrlsForItem($item)
      */
     public static function getAllUrls()
     {
-        return Blink::get(static::ALL_URLS_CACHE_KEY) ?? collect();
+        return Blink::get(static::ALL_URLS_CACHE_KEY)
+            ?? Cache::get(static::ALL_URLS_CACHE_KEY)
+            ?? collect();
     }
 
     /**

From 423af5d8af248a5a4f1d617e68f062807191107e Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 27 Oct 2023 16:39:11 -0400
Subject: [PATCH 15/25] Fix more edge cases around URL hierarchy.

---
 src/CP/Navigation/NavItem.php | 70 ++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 9 deletions(-)

diff --git a/src/CP/Navigation/NavItem.php b/src/CP/Navigation/NavItem.php
index cad18dc786c..ec931f7b9c8 100644
--- a/src/CP/Navigation/NavItem.php
+++ b/src/CP/Navigation/NavItem.php
@@ -21,6 +21,7 @@ class NavItem
     protected $icon;
     protected $children;
     protected $isChild;
+    protected $wasOriginallyChild;
     protected $authorization;
     protected $active;
     protected $view;
@@ -148,6 +149,21 @@ protected function generateActivePatternForCpUrl($url)
         return $relativeUrl.'(/(.*)?|$)';
     }
 
+    /**
+     * Generate active URL patterns for this item's children.
+     *
+     * @return Collection
+     */
+    protected function generateActivePatternsForChildren()
+    {
+        if (! $this->children()) {
+            return collect();
+        }
+
+        return collect(NavBuilder::getUnresolvedChildrenUrlsForItem($this) ?? [])
+            ->map(fn ($url) => $this->generateActivePatternForCpUrl($url));
+    }
+
     /**
      * Get editable url for nav builder UI.
      */
@@ -253,9 +269,25 @@ public function isChild($isChild = null)
             ->getter(function ($value) {
                 return (bool) $value;
             })
+            ->afterSetter(function ($value) {
+                if ($value === true && ! isset($this->wasOriginallyChild)) {
+                    $this->wasOriginallyChild = $value;
+                }
+            })
             ->value($isChild);
     }
 
+    /**
+     * Check if this nav item was ever a child before user preferences were applied.
+     *
+     * @param  bool|null  $isChild
+     * @return mixed
+     */
+    protected function wasOriginallyChild()
+    {
+        return (bool) $this->wasOriginallyChild;
+    }
+
     /**
      * Active URL pattern to determine when to resolve children for `hasActiveChild()` checks.
      *
@@ -343,10 +375,11 @@ public function isActive()
             return true;
         }
 
-        // If the current url is not explicitly referenced in CP nav,
-        // check if active descendant using regex pattern instead.
-        if (! NavBuilder::getAllUrls()->contains(request()->url())) {
-            return $this->isActiveByPattern();
+        // If the current URL is not explicitly referenced in the CP nav,
+        // and if this item is/was ever a child nav item,
+        // then check against URL heirarchy conventions using regex pattern.
+        if ($this->currentUrlIsNotExplicitlyReferencedInNav() && $this->wasOriginallyChild()) {
+            return $this->isActiveByPattern($this->active);
         }
 
         return request()->url() === URL::removeQueryAndFragment($this->url);
@@ -367,6 +400,13 @@ protected function hasActiveChild()
                 ->isNotEmpty();
         }
 
+        // If the current URL is not explicitly referenced in the CP nav,
+        // and if this item has children to check against,
+        // then check against URL heirarchy conventions using regex pattern.
+        if ($this->currentUrlIsNotExplicitlyReferencedInNav() && $this->children()) {
+            return $this->isActiveByPattern($this->generateActivePatternsForChildren());
+        }
+
         // If children closure has not been resolved, and children urls are cached, check against cached children.
         if ($childrenUrls = NavBuilder::getUnresolvedChildrenUrlsForItem($this)) {
             return collect($childrenUrls)
@@ -377,20 +417,32 @@ protected function hasActiveChild()
         return false;
     }
 
+    /**
+     * Determine whether the current URL is explicitly referenced in nav.
+     *
+     * @return bool
+     */
+    protected function currentUrlIsNotExplicitlyReferencedInNav()
+    {
+        return ! NavBuilder::getAllUrls()->contains(request()->url());
+    }
+
     /**
      * Determine whether the nav item is currently active using the regex technique for deeply nested hierarchical urls.
      *
+     * @param  string|array  $active
      * @return bool
      */
-    public function isActiveByPattern()
+    protected function isActiveByPattern($active)
     {
-        if (! $this->active) {
+        if (! $active) {
             return false;
         }
 
-        $pattern = preg_quote(config('statamic.cp.route'), '#').'/'.$this->active;
-
-        return preg_match('#'.$pattern.'#', request()->decodedPath()) === 1;
+        return collect($active)
+            ->map(fn ($pattern) => preg_quote(config('statamic.cp.route'), '#').'/'.$pattern)
+            ->filter(fn ($pattern) => preg_match('#'.$pattern.'#', request()->decodedPath()) === 1)
+            ->isNotEmpty();
     }
 
     /**

From 7cdc675dde8206afbd23c6a52c9b201c735c7dc3 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Fri, 27 Oct 2023 16:53:52 -0400
Subject: [PATCH 16/25] These patterns are only intended to check against
 descendants of child items, since we have explicit child URLs.

---
 tests/CP/Navigation/NavTest.php | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/CP/Navigation/NavTest.php b/tests/CP/Navigation/NavTest.php
index b0109364adc..d02c24163d1 100644
--- a/tests/CP/Navigation/NavTest.php
+++ b/tests/CP/Navigation/NavTest.php
@@ -384,13 +384,16 @@ public function it_checks_various_active_patterns()
         // Ensure urls are not cached so that we can test regex based isActive() checks
         Nav::clearCachedUrls();
 
-        $hello = Nav::create('hello')->url('http://localhost/cp/hello');
-        $helloWithQueryParams = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello?params');
-        $helloWithAnchor = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello#anchor');
-        $hell = Nav::create('hell')->url('http://localhost/cp/hell');
-        $localNotCp = Nav::create('localNotCp')->url('/dashboard');
-        $external = Nav::create('external')->url('http://external.com');
-        $externalSecure = Nav::create('externalSecure')->url('https://external.com');
+        // These patterns are only intended to check against descendants of child items, since we have explicit child URLs.
+        Nav::create('parent')->section('test')->children([
+            $hello = Nav::create('hello')->url('http://localhost/cp/hello'),
+            $helloWithQueryParams = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello?params'),
+            $helloWithAnchor = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello#anchor'),
+            $hell = Nav::create('hell')->url('http://localhost/cp/hell'),
+            $localNotCp = Nav::create('localNotCp')->url('/dashboard'),
+            $external = Nav::create('external')->url('http://external.com'),
+            $externalSecure = Nav::create('externalSecure')->url('https://external.com'),
+        ]);
 
         Request::swap(Request::create('http://localhost/cp/hell'));
         $this->assertFalse($hello->isActive());

From 38cf8f92604c07b939dcaf17f228d1372ab97a77 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 17:01:36 -0400
Subject: [PATCH 17/25] Active nav item tests wip.

---
 tests/CP/Navigation/ActiveNavItemTest.php | 265 ++++++++++++++++++++++
 tests/CP/Navigation/NavTest.php           | 221 ------------------
 2 files changed, 265 insertions(+), 221 deletions(-)
 create mode 100644 tests/CP/Navigation/ActiveNavItemTest.php

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
new file mode 100644
index 00000000000..2493030923d
--- /dev/null
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -0,0 +1,265 @@
+<?php
+
+namespace Tests\CP\Navigation;
+
+use Closure;
+use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Request;
+use Statamic\CP\Navigation\NavBuilder;
+use Statamic\Facades;
+use Statamic\Facades\Blink;
+use Statamic\Facades\CP\Nav;
+use Statamic\Facades\User;
+use Tests\FakesRoles;
+use Tests\PreventSavingStacheItemsToDisk;
+use Tests\TestCase;
+
+class ActiveNavItemTest extends TestCase
+{
+    use FakesRoles;
+    use PreventSavingStacheItemsToDisk;
+
+    protected $shouldPreventNavBeingBuilt = false;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        // Always act as super user for these tests
+        $this->actingAs(tap(User::make()->makeSuper())->save());
+
+        // TODO: Other tests are leaving behind forms without titles that are causing failures here?
+        Facades\Form::shouldReceive('all')->andReturn(collect());
+    }
+
+    /** @test */
+    public function it_renders_core_children_closure_when_not_active()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/dashboard')
+            ->assertStatus(200);
+
+        $collections = $this->buildAndGetItem('Content', 'Collections');
+
+        $this->assertFalse($collections->isActive());
+        $this->assertInstanceOf(Closure::class, $collections->children());
+    }
+
+    /** @test */
+    public function it_resolves_core_children_closure_and_can_check_when_parent_item_is_active()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections')
+            ->assertStatus(200);
+
+        $collections = $this->buildAndGetItem('Content', 'Collections');
+
+        $this->assertTrue($collections->isActive());
+        $this->assertInstanceOf(Collection::class, $collections->children());
+        $this->assertFalse($this->getItemByDisplay($collections->children(), 'Pages')->isActive());
+        $this->assertFalse($this->getItemByDisplay($collections->children(), 'Articles')->isActive());
+    }
+
+    /** @test */
+    public function it_resolves_core_children_closure_and_can_check_when_parent_and_child_item_are_active()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections/articles')
+            ->assertStatus(200);
+
+        $collections = $this->buildAndGetItem('Content', 'Collections');
+
+        $this->assertTrue($collections->isActive());
+        $this->assertInstanceOf(Collection::class, $collections->children());
+        $this->assertFalse($this->getItemByDisplay($collections->children(), 'Pages')->isActive());
+        $this->assertTrue($this->getItemByDisplay($collections->children(), 'Articles')->isActive());
+    }
+
+    /** @test */
+    public function it_resolves_core_children_closure_and_can_check_when_parent_and_descendant_of_child_item_is_active()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections/articles/entries/create/en')
+            ->assertStatus(200);
+
+        $collections = $this->buildAndGetItem('Content', 'Collections');
+
+        $this->assertTrue($collections->isActive());
+        $this->assertInstanceOf(Collection::class, $collections->children());
+        $this->assertFalse($collections->children()->keyBy->display()->get('Pages')->isActive());
+        $this->assertTrue($collections->children()->keyBy->display()->get('Articles')->isActive());
+    }
+
+    /** @test */
+    public function it_properly_handles_various_descendant_edge_cases_when_checking_is_active_on_nav_children()
+    {
+        // Ensure urls are not cached so that we can test regex based isActive() checks
+        Nav::clearCachedUrls();
+
+        // These patterns are only intended to check against descendants of child items, since we have explicit child URLs.
+        $parent = Nav::create('parent')
+            ->section('test')
+            ->url('http://localhost/cp/parent')
+            ->children([
+                $hello = Nav::create('hello')->url('http://localhost/cp/hello'),
+                $helloWithQueryParams = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello?params'),
+                $helloWithAnchor = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello#anchor'),
+                $hell = Nav::create('hell')->url('http://localhost/cp/hell'),
+                $localNotCp = Nav::create('localNotCp')->url('/dashboard'),
+                $external = Nav::create('external')->url('http://external.com'),
+                $externalSecure = Nav::create('externalSecure')->url('https://external.com'),
+            ]);
+
+        // Test active status on an explicit item
+        Request::swap(Request::create('http://localhost/cp/hell'));
+        $this->assertTrue($parent->isActive());
+        $this->assertFalse($hello->isActive());
+        $this->assertFalse($helloWithQueryParams->isActive());
+        $this->assertFalse($helloWithAnchor->isActive());
+        $this->assertTrue($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Test active status on an explicit item where url params or anchors were set on url
+        Request::swap(Request::create('http://localhost/cp/hello'));
+        $this->assertTrue($parent->isActive());
+        $this->assertTrue($hello->isActive());
+        $this->assertTrue($helloWithQueryParams->isActive());
+        $this->assertTrue($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Test active status on a descendant of an explicit item
+        Request::swap(Request::create('http://localhost/cp/hell/test'));
+        $this->assertTrue($parent->isActive());
+        $this->assertFalse($hello->isActive());
+        $this->assertFalse($helloWithQueryParams->isActive());
+        $this->assertFalse($helloWithAnchor->isActive());
+        $this->assertTrue($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Test active status on a descendant of an explicit item where url params or anchors were set on url
+        Request::swap(Request::create('http://localhost/cp/hello/test'));
+        $this->assertTrue($parent->isActive());
+        $this->assertTrue($hello->isActive());
+        $this->assertTrue($helloWithQueryParams->isActive());
+        $this->assertTrue($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Test active status on a descendant of an explicit item where url param is part of current url
+        Request::swap(Request::create('http://localhost/cp/hello?params'));
+        $this->assertTrue($parent->isActive());
+        $this->assertTrue($hello->isActive());
+        $this->assertTrue($helloWithQueryParams->isActive());
+        $this->assertTrue($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Test active status on a descendant of an explicit item where anchor is part of current url
+        Request::swap(Request::create('http://localhost/cp/hello#anchor'));
+        $this->assertTrue($parent->isActive());
+        $this->assertTrue($hello->isActive());
+        $this->assertTrue($helloWithQueryParams->isActive());
+        $this->assertTrue($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Test active status on a deeper descendant of an explicit item where url params and anchors were set on url
+        Request::swap(Request::create('http://localhost/cp/hello/this/is/super/nested?params#anchor'));
+        $this->assertTrue($parent->isActive());
+        $this->assertTrue($hello->isActive());
+        $this->assertTrue($helloWithQueryParams->isActive());
+        $this->assertTrue($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Ensure regex check is not used when checking is active on explicit parent item
+        Request::swap(Request::create('http://localhost/cp/parent'));
+        $this->assertTrue($parent->isActive());
+        $this->assertFalse($hello->isActive());
+        $this->assertFalse($helloWithQueryParams->isActive());
+        $this->assertFalse($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+
+        // Ensure regex check is not used when checking is active on descendant of parent item
+        Request::swap(Request::create('http://localhost/cp/parent/nested/item'));
+        $this->assertFalse($parent->isActive());
+        $this->assertFalse($hello->isActive());
+        $this->assertFalse($helloWithQueryParams->isActive());
+        $this->assertFalse($helloWithAnchor->isActive());
+        $this->assertFalse($hell->isActive());
+        $this->assertFalse($localNotCp->isActive());
+        $this->assertFalse($external->isActive());
+        $this->assertFalse($externalSecure->isActive());
+    }
+
+    protected function prepareNavCaches()
+    {
+        // Clear caches
+        Nav::clearCachedUrls();
+        $this->assertFalse(Cache::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertFalse(Blink::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertFalse(Cache::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertFalse(Blink::has(NavBuilder::ALL_URLS_CACHE_KEY));
+
+        // Ensure the nav is built and cached so that tests can check `isActive()` on children in unresolved closures
+        Nav::build();
+        $this->assertTrue(Cache::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertTrue(Blink::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertTrue(Cache::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertTrue(Blink::has(NavBuilder::ALL_URLS_CACHE_KEY));
+
+        return $this;
+    }
+
+    protected function build()
+    {
+        return Nav::build()->pluck('items', 'display');
+    }
+
+    protected function buildAndGetItem($sectionDisplay, $itemDisplay)
+    {
+        $sectionItems = $this->build()->get($sectionDisplay);
+
+        return $this->getItemByDisplay($sectionItems, $itemDisplay);
+    }
+
+    protected function getItemByDisplay($items, $display)
+    {
+        return $items->keyBy->display()->get($display);
+    }
+}
diff --git a/tests/CP/Navigation/NavTest.php b/tests/CP/Navigation/NavTest.php
index d02c24163d1..8930d47ad16 100644
--- a/tests/CP/Navigation/NavTest.php
+++ b/tests/CP/Navigation/NavTest.php
@@ -2,7 +2,6 @@
 
 namespace Tests\CP\Navigation;
 
-use Illuminate\Support\Facades\Request;
 use Illuminate\Support\Facades\Route;
 use Statamic\CP\Navigation\NavItem;
 use Statamic\Facades;
@@ -378,226 +377,6 @@ public function it_can_use_extend_to_remove_a_default_statamic_nav_item()
         $this->assertNotContains('Collections', $this->build()->get('Content')->map->display());
     }
 
-    /** @test */
-    public function it_checks_various_active_patterns()
-    {
-        // Ensure urls are not cached so that we can test regex based isActive() checks
-        Nav::clearCachedUrls();
-
-        // These patterns are only intended to check against descendants of child items, since we have explicit child URLs.
-        Nav::create('parent')->section('test')->children([
-            $hello = Nav::create('hello')->url('http://localhost/cp/hello'),
-            $helloWithQueryParams = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello?params'),
-            $helloWithAnchor = Nav::create('helloWithAnchor')->url('http://localhost/cp/hello#anchor'),
-            $hell = Nav::create('hell')->url('http://localhost/cp/hell'),
-            $localNotCp = Nav::create('localNotCp')->url('/dashboard'),
-            $external = Nav::create('external')->url('http://external.com'),
-            $externalSecure = Nav::create('externalSecure')->url('https://external.com'),
-        ]);
-
-        Request::swap(Request::create('http://localhost/cp/hell'));
-        $this->assertFalse($hello->isActive());
-        $this->assertFalse($helloWithQueryParams->isActive());
-        $this->assertFalse($helloWithAnchor->isActive());
-        $this->assertTrue($hell->isActive());
-        $this->assertFalse($localNotCp->isActive());
-        $this->assertFalse($external->isActive());
-        $this->assertFalse($externalSecure->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/hello'));
-        $this->assertTrue($hello->isActive());
-        $this->assertTrue($helloWithQueryParams->isActive());
-        $this->assertTrue($helloWithAnchor->isActive());
-        $this->assertFalse($hell->isActive());
-        $this->assertFalse($localNotCp->isActive());
-        $this->assertFalse($external->isActive());
-        $this->assertFalse($externalSecure->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/hell/test'));
-        $this->assertFalse($hello->isActive());
-        $this->assertFalse($helloWithQueryParams->isActive());
-        $this->assertFalse($helloWithAnchor->isActive());
-        $this->assertTrue($hell->isActive());
-        $this->assertFalse($localNotCp->isActive());
-        $this->assertFalse($external->isActive());
-        $this->assertFalse($externalSecure->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/hello/test'));
-        $this->assertTrue($hello->isActive());
-        $this->assertTrue($helloWithQueryParams->isActive());
-        $this->assertTrue($helloWithAnchor->isActive());
-        $this->assertFalse($hell->isActive());
-        $this->assertFalse($localNotCp->isActive());
-        $this->assertFalse($external->isActive());
-        $this->assertFalse($externalSecure->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/hello?params'));
-        $this->assertTrue($hello->isActive());
-        $this->assertTrue($helloWithQueryParams->isActive());
-        $this->assertTrue($helloWithAnchor->isActive());
-        $this->assertFalse($hell->isActive());
-        $this->assertFalse($localNotCp->isActive());
-        $this->assertFalse($external->isActive());
-        $this->assertFalse($externalSecure->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/hello#anchor'));
-        $this->assertTrue($hello->isActive());
-        $this->assertTrue($helloWithQueryParams->isActive());
-        $this->assertTrue($helloWithAnchor->isActive());
-        $this->assertFalse($hell->isActive());
-        $this->assertFalse($localNotCp->isActive());
-        $this->assertFalse($external->isActive());
-        $this->assertFalse($externalSecure->isActive());
-    }
-
-    /** @test */
-    public function it_checks_if_has_active_children()
-    {
-        $collections = Nav::content('Collections')
-            ->url('http://localhost/cp/collections')
-            ->children([
-                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-            ]);
-
-        Request::swap(Request::create('http://localhost/cp/collections/articles'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-    }
-
-    /** @test */
-    public function it_can_get_has_active_children_status_with_nested_url_hierarchy()
-    {
-        $collections = Nav::content('Custom Collections Url')
-            ->url('http://localhost/cp/collections')
-            ->children([
-                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-            ]);
-
-        Request::swap(Request::create('http://localhost/cp/collections/articles/nested-article'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-    }
-
-    /** @test */
-    public function it_can_get_has_active_children_status_with_custom_is_active_pattern()
-    {
-        $collections = Nav::content('Custom Collections Url')
-            ->url('http://localhost/cp/custom/url')
-            ->active('collections/.*')
-            ->children([
-                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-            ]);
-
-        Request::swap(Request::create('http://localhost/cp/collections/articles'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-    }
-
-    /** @test */
-    public function it_can_get_has_active_children_status_with_multiple_custom_is_active_patterns()
-    {
-        // TODO
-        $this->markTestSkipped();
-
-        $collections = Nav::content('Custom Collections Url')
-            ->url('http://localhost/cp/custom/url')
-            ->active([
-                'collections',
-            ])
-            ->children([
-                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-                $categories = Nav::item('Categories')->url('/cp/taxonomies/categories'),
-            ]);
-
-        Request::swap(Request::create('http://localhost/cp/collections/pages'));
-        $this->assertTrue($collections->isActive());
-        $this->assertTrue($pages->isActive());
-        $this->assertFalse($articles->isActive());
-        $this->assertFalse($categories->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/collections/articles'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-        $this->assertFalse($categories->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/taxonomies/categories'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertFalse($articles->isActive());
-        $this->assertTrue($categories->isActive());
-    }
-
-    /** @test */
-    public function it_can_get_has_active_children_status_with_custom_is_active_url()
-    {
-        // TODO
-        $this->markTestSkipped();
-
-        Facades\Collection::make('pages')->title('Pages')->save();
-        Facades\Collection::make('articles')->title('Articles')->save();
-
-        $this
-            ->actingAs(tap(User::make()->makeSuper())->save())
-            ->get('/cp/collections/articles')
-            ->assertStatus(200);
-
-        $collections = Nav::content('Custom Collections Url')
-            ->url('http://localhost/cp/custom/url')
-            ->active('http://localhost/cp/collections')
-            ->children([
-                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-            ]);
-
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-    }
-
-    /** @test */
-    public function it_can_get_has_active_children_status_with_multiple_custom_is_active_urls()
-    {
-        // TODO
-        $this->markTestSkipped();
-
-        $collections = Nav::content('Custom Collections Url')
-            ->url('http://localhost/cp/custom/url')
-            ->active([
-                'collections/.*',
-            ])
-            ->children([
-                $pages = Nav::item('Pages')->url('/cp/collections/pages'),
-                $articles = Nav::item('Articles')->url('/cp/collections/articles'),
-                $categories = Nav::item('Categories')->url('/cp/taxonomies/categories'),
-            ]);
-
-        Request::swap(Request::create('http://localhost/cp/collections/pages'));
-        $this->assertTrue($collections->isActive());
-        $this->assertTrue($pages->isActive());
-        $this->assertFalse($articles->isActive());
-        $this->assertFalse($categories->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/collections/articles'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertTrue($articles->isActive());
-        $this->assertFalse($categories->isActive());
-
-        Request::swap(Request::create('http://localhost/cp/taxonomies/categories'));
-        $this->assertTrue($collections->isActive());
-        $this->assertFalse($pages->isActive());
-        $this->assertFalse($articles->isActive());
-        $this->assertTrue($categories->isActive());
-    }
-
     /** @test */
     public function it_sets_the_url()
     {

From 73a57825bada9efa871f8775e6f9de6ea9c53e6a Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 17:35:19 -0400
Subject: [PATCH 18/25] Rename

---
 tests/CP/Navigation/ActiveNavItemTest.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index 2493030923d..b613db70aa8 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -108,12 +108,12 @@ public function it_resolves_core_children_closure_and_can_check_when_parent_and_
     }
 
     /** @test */
-    public function it_properly_handles_various_descendant_edge_cases_when_checking_is_active_on_nav_children()
+    public function it_properly_handles_various_edge_cases_when_checking_is_active_on_descendants_of_nav_children()
     {
         // Ensure urls are not cached so that we can test regex based isActive() checks
         Nav::clearCachedUrls();
 
-        // These patterns are only intended to check against descendants of child items, since we have explicit child URLs.
+        // These patterns are only intended to check against descendants of child items, since we have explicit child URLs
         $parent = Nav::create('parent')
             ->section('test')
             ->url('http://localhost/cp/parent')

From 564451e9879157a690957281a0bc12e2eae62a6d Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 18:23:16 -0400
Subject: [PATCH 19/25] Add test coverage for extension nav items and children.

---
 tests/CP/Navigation/ActiveNavItemTest.php | 198 +++++++++++++++++++++-
 1 file changed, 197 insertions(+), 1 deletion(-)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index b613db70aa8..abf14e5706a 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -6,6 +6,7 @@
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Request;
+use Illuminate\Support\Facades\Route;
 use Statamic\CP\Navigation\NavBuilder;
 use Statamic\Facades;
 use Statamic\Facades\Blink;
@@ -33,8 +34,21 @@ public function setUp(): void
         Facades\Form::shouldReceive('all')->andReturn(collect());
     }
 
+    protected function resolveApplicationConfiguration($app)
+    {
+        parent::resolveApplicationConfiguration($app);
+
+        // Set up test routes for fake SEO Pro extension
+        $app->booted(function () {
+            Route::get('cp/seo-pro', fn () => 'test');
+            Route::get('cp/seo-pro/section-defaults', fn () => 'test');
+            Route::get('cp/seo-pro/section-defaults/pages', fn () => 'test');
+            Route::get('cp/seo-pro/section-defaults/articles', fn () => 'test');
+        });
+    }
+
     /** @test */
-    public function it_renders_core_children_closure_when_not_active()
+    public function it_builds_core_children_closure_when_not_active()
     {
         Facades\Collection::make('pages')->title('Pages')->save();
         Facades\Collection::make('articles')->title('Articles')->save();
@@ -107,6 +121,188 @@ public function it_resolves_core_children_closure_and_can_check_when_parent_and_
         $this->assertTrue($collections->children()->keyBy->display()->get('Articles')->isActive());
     }
 
+    /** @test */
+    public function it_can_check_if_parent_extension_item_is_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children([
+                    $nav->item('Reports')->url('/cp/seo-pro/reports')->can('view seo reports'),
+                    $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                ]);
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/seo-pro')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertTrue($seoPro->isActive());
+        $this->assertInstanceOf(Collection::class, $seoPro->children());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Reports')->isActive());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Section Defaults')->isActive());
+    }
+
+    /** @test */
+    public function it_can_check_when_parent_and_child_extension_items_are_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children([
+                    $nav->item('Reports')->url('/cp/seo-pro/reports')->can('view seo reports'),
+                    $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                ]);
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/seo-pro/section-defaults')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertTrue($seoPro->isActive());
+        $this->assertInstanceOf(Collection::class, $seoPro->children());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Reports')->isActive());
+        $this->assertTrue($this->getItemByDisplay($seoPro->children(), 'Section Defaults')->isActive());
+    }
+
+    /** @test */
+    public function it_can_check_when_parent_and_descendant_of_child_extension_item_is_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children([
+                    $nav->item('Reports')->url('/cp/seo-pro/reports')->can('view seo reports'),
+                    $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                ]);
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/seo-pro/section-defaults/pages')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertTrue($seoPro->isActive());
+        $this->assertInstanceOf(Collection::class, $seoPro->children());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Reports')->isActive());
+        $this->assertTrue($this->getItemByDisplay($seoPro->children(), 'Section Defaults')->isActive());
+    }
+
+    /** @test */
+    public function it_builds_extension_children_closure_when_not_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Reports')->url('/cp/seo-pro/')->can('view seo reports'),
+                        $nav->item('Site Defaults')->url('/cp/seo-pro/site-defaults')->can('edit seo site defaults'),
+                        $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/dashboard')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertFalse($seoPro->isActive());
+        $this->assertInstanceOf(Closure::class, $seoPro->children());
+    }
+
+    /** @test */
+    public function it_resolves_extension_children_closure_and_can_check_when_parent_item_is_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Reports')->url('/cp/seo-pro/reports')->can('view seo reports'),
+                        $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/seo-pro')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertTrue($seoPro->isActive());
+        $this->assertInstanceOf(Collection::class, $seoPro->children());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Reports')->isActive());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Section Defaults')->isActive());
+    }
+
+    /** @test */
+    public function it_resolves_extension_children_closure_and_can_check_when_parent_and_child_item_are_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Reports')->url('/cp/seo-pro/reports')->can('view seo reports'),
+                        $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/seo-pro/section-defaults')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertTrue($seoPro->isActive());
+        $this->assertInstanceOf(Collection::class, $seoPro->children());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Reports')->isActive());
+        $this->assertTrue($this->getItemByDisplay($seoPro->children(), 'Section Defaults')->isActive());
+    }
+
+    /** @test */
+    public function it_resolves_extension_children_closure_and_can_check_when_parent_and_descendant_of_child_item_is_active()
+    {
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->tools('SEO Pro')
+                ->url('/cp/seo-pro')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Reports')->url('/cp/seo-pro/reports')->can('view seo reports'),
+                        $nav->item('Section Defaults')->url('/cp/seo-pro/section-defaults')->can('edit seo section defaults'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/seo-pro/section-defaults/pages')
+            ->assertStatus(200);
+
+        $seoPro = $this->buildAndGetItem('Tools', 'SEO Pro');
+
+        $this->assertTrue($seoPro->isActive());
+        $this->assertInstanceOf(Collection::class, $seoPro->children());
+        $this->assertFalse($this->getItemByDisplay($seoPro->children(), 'Reports')->isActive());
+        $this->assertTrue($this->getItemByDisplay($seoPro->children(), 'Section Defaults')->isActive());
+    }
+
     /** @test */
     public function it_properly_handles_various_edge_cases_when_checking_is_active_on_descendants_of_nav_children()
     {

From be9c6d1fda59f9c164fa23b97198aabbf8bee3ae Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 18:41:24 -0400
Subject: [PATCH 20/25] =?UTF-8?q?Test=20rad-pack/shopify=20case=20where=20?=
 =?UTF-8?q?we=E2=80=99re=20hijacking=20core=20item=20children.?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 tests/CP/Navigation/ActiveNavItemTest.php | 94 +++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index abf14e5706a..d6133646a28 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -423,6 +423,100 @@ public function it_properly_handles_various_edge_cases_when_checking_is_active_o
         $this->assertFalse($externalSecure->isActive());
     }
 
+    /** @test */
+    public function active_nav_check_still_functions_properly_when_custom_nav_extension_hijacks_a_core_item_child()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+        Facades\Collection::make('products')->title('Products')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        // Remove `Products` and `Categories` from core parents, and add to `Schopify` extension item as children
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->remove('Content', 'Collections', 'Products');
+            $nav->remove('Content', 'Taxonomies', 'Categories');
+
+            $nav->tools('Schopify')
+                ->url('/cp/collections/products')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Products')->url('/cp/collections/products'),
+                        $nav->item('Categories')->url('/cp/taxonomies/categories'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections/products')
+            ->assertStatus(200);
+
+        $nav = $this->build();
+
+        $collections = $this->getItemByDisplay($nav->get('Content'), 'Collections');
+        $taxonomies = $this->getItemByDisplay($nav->get('Content'), 'Taxonomies');
+        $schopify = $this->getItemByDisplay($nav->get('Tools'), 'Schopify');
+
+        // Ensure only the `Schopify` nav item is active, since we moved the current url (ie. `Products`) to this item
+        $this->assertFalse($collections->isActive());
+        $this->assertFalse($taxonomies->isActive());
+        $this->assertTrue($schopify->isActive());
+        $this->assertInstanceOf(Collection::class, $schopify->children());
+
+        // Ensure the new `Products` child under `Schopify` is active
+        $this->assertTrue($this->getItemByDisplay($schopify->children(), 'Products')->isActive());
+        $this->assertFalse($this->getItemByDisplay($schopify->children(), 'Categories')->isActive());
+    }
+
+    /** @test */
+    public function active_nav_descendant_check_still_functions_properly_when_custom_nav_extension_hijacks_a_core_item_child()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+        Facades\Collection::make('products')->title('Products')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        // Remove `Products` and `Categories` from core parents, and add to `Schopify` extension item as children
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->remove('Content', 'Collections', 'Products');
+            $nav->remove('Content', 'Taxonomies', 'Categories');
+
+            $nav->tools('Schopify')
+                ->url('/cp/collections/products')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Products')->url('/cp/collections/products'),
+                        $nav->item('Categories')->url('/cp/taxonomies/categories'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections/products/entries/create/en')
+            ->assertStatus(200);
+
+        $nav = $this->build();
+
+        $collections = $this->getItemByDisplay($nav->get('Content'), 'Collections');
+        $taxonomies = $this->getItemByDisplay($nav->get('Content'), 'Taxonomies');
+        $schopify = $this->getItemByDisplay($nav->get('Tools'), 'Schopify');
+
+        // Ensure only the `Schopify` nav item is active, since we moved the current url (ie. `Products`) to this item
+        $this->assertFalse($collections->isActive());
+        $this->assertFalse($taxonomies->isActive());
+        $this->assertTrue($schopify->isActive());
+        $this->assertInstanceOf(Collection::class, $schopify->children());
+
+        // Ensure the new `Products` child under `Schopify` is active, because the current URL is a descendant of this item
+        $this->assertTrue($this->getItemByDisplay($schopify->children(), 'Products')->isActive());
+        $this->assertFalse($this->getItemByDisplay($schopify->children(), 'Categories')->isActive());
+    }
+
     protected function prepareNavCaches()
     {
         // Clear caches

From fd4acaf5f26757998cb0a97952eb9598db14ceef Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 18:48:28 -0400
Subject: [PATCH 21/25] Rename.

---
 tests/CP/Navigation/ActiveNavItemTest.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index d6133646a28..6f0eae0802a 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -122,7 +122,7 @@ public function it_resolves_core_children_closure_and_can_check_when_parent_and_
     }
 
     /** @test */
-    public function it_can_check_if_parent_extension_item_is_active()
+    public function it_can_check_if_parent_extension_with_array_based_children_item_is_active()
     {
         Facades\CP\Nav::extend(function ($nav) {
             $nav->tools('SEO Pro')
@@ -147,7 +147,7 @@ public function it_can_check_if_parent_extension_item_is_active()
     }
 
     /** @test */
-    public function it_can_check_when_parent_and_child_extension_items_are_active()
+    public function it_can_check_when_parent_and_array_based_child_extension_items_are_active()
     {
         Facades\CP\Nav::extend(function ($nav) {
             $nav->tools('SEO Pro')
@@ -172,7 +172,7 @@ public function it_can_check_when_parent_and_child_extension_items_are_active()
     }
 
     /** @test */
-    public function it_can_check_when_parent_and_descendant_of_child_extension_item_is_active()
+    public function it_can_check_when_parent_and_array_based_descendant_of_child_extension_item_is_active()
     {
         Facades\CP\Nav::extend(function ($nav) {
             $nav->tools('SEO Pro')

From 4f8cc9d1a4d5d1e4abdcb20075eba92ede40ca43 Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 18:56:55 -0400
Subject: [PATCH 22/25] Test that descendant `isActive()` check works on URLs
 unrelated to parent item.

---
 tests/CP/Navigation/ActiveNavItemTest.php | 45 +++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index 6f0eae0802a..38932c2dd0a 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -44,6 +44,8 @@ protected function resolveApplicationConfiguration($app)
             Route::get('cp/seo-pro/section-defaults', fn () => 'test');
             Route::get('cp/seo-pro/section-defaults/pages', fn () => 'test');
             Route::get('cp/seo-pro/section-defaults/articles', fn () => 'test');
+            Route::get('cp/totally-custom-url', fn () => 'test');
+            Route::get('cp/totally-custom-url/deeper/descendant', fn () => 'test');
         });
     }
 
@@ -517,6 +519,49 @@ public function active_nav_descendant_check_still_functions_properly_when_custom
         $this->assertFalse($this->getItemByDisplay($schopify->children(), 'Categories')->isActive());
     }
 
+    /** @test */
+    public function active_nav_descendant_with_unrelated_url_still_functions_properly_when_custom_nav_extension_hijacks_a_core_item_child()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+        Facades\Collection::make('products')->title('Products')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        // Remove `Products` and `Categories` from core parents, and add to `Schopify` extension item as children
+        Facades\CP\Nav::extend(function ($nav) {
+            $nav->remove('Content', 'Collections', 'Products');
+            $nav->remove('Content', 'Taxonomies', 'Categories');
+
+            $nav->tools('Schopify')
+                ->url('/cp/collections/products')
+                ->children(function () use ($nav) {
+                    return [
+                        $nav->item('Products')->url('/cp/collections/products'),
+                        $nav->item('Categories')->url('/cp/taxonomies/categories'),
+                        $nav->item('Unrelated')->url('/cp/totally-custom-url'),
+                    ];
+                });
+        });
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/totally-custom-url/deeper/descendant')
+            ->assertStatus(200);
+
+        $schopify = $this->buildAndGetItem('Tools', 'Schopify');
+
+        // Ensure only the `Schopify` nav item is active and children are resolved
+        $this->assertTrue($schopify->isActive());
+        $this->assertInstanceOf(Collection::class, $schopify->children());
+
+        // Ensure our `Unrelated` totally custom URL item is considered active as well, based on URL hierarchy
+        $this->assertFalse($this->getItemByDisplay($schopify->children(), 'Products')->isActive());
+        $this->assertFalse($this->getItemByDisplay($schopify->children(), 'Categories')->isActive());
+        $this->assertTrue($this->getItemByDisplay($schopify->children(), 'Unrelated')->isActive());
+    }
+
     protected function prepareNavCaches()
     {
         // Clear caches

From de54d6e0a5c1aed056b6bf274132273136c9386e Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 19:03:58 -0400
Subject: [PATCH 23/25] Ensure hijacked items were properly removed from
 original parents

---
 tests/CP/Navigation/ActiveNavItemTest.php | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index 38932c2dd0a..8feb0076ccb 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -470,6 +470,12 @@ public function active_nav_check_still_functions_properly_when_custom_nav_extens
         // Ensure the new `Products` child under `Schopify` is active
         $this->assertTrue($this->getItemByDisplay($schopify->children(), 'Products')->isActive());
         $this->assertFalse($this->getItemByDisplay($schopify->children(), 'Categories')->isActive());
+
+        // Ensure hijacked items were properly removed from original parents
+        $this->assertInstanceOf(Collection::class, $collections->children());
+        $this->assertEquals(['Articles', 'Pages'], $collections->children()->map->display()->all());
+        $this->assertInstanceOf(Collection::class, $taxonomies->children());
+        $this->assertEquals(['Tags'], $taxonomies->children()->map->display()->all());
     }
 
     /** @test */

From 948e7f5d86c2683705bb522adf8687556467ed4e Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 20:17:26 -0400
Subject: [PATCH 24/25] Add better test coverage for the use case mentioned by
 the original fix in #8273.

---
 tests/CP/Navigation/ActiveNavItemTest.php | 168 +++++++++++++++++++++-
 1 file changed, 166 insertions(+), 2 deletions(-)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index 8feb0076ccb..863cbad6130 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -568,6 +568,170 @@ public function active_nav_descendant_with_unrelated_url_still_functions_properl
         $this->assertTrue($this->getItemByDisplay($schopify->children(), 'Unrelated')->isActive());
     }
 
+    /** @test */
+    public function active_nav_check_still_functions_properly_on_moved_items()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections/articles')
+            ->assertStatus(200);
+
+        $nav = $this->build([
+            'top_level' => [
+                'content::collections::articles' => [
+                    'action' => '@move',
+                    'children' => [
+                        'content::taxonomies::categories' => '@move',
+                    ],
+                ],
+            ],
+        ]);
+
+        $articles = $this->getItemByDisplay($nav->get('Top Level'), 'Articles');
+        $categories = $this->getItemByDisplay($articles->children(), 'Categories');
+        $collections = $this->getItemByDisplay($nav->get('Content'), 'Collections');
+        $taxonomies = $this->getItemByDisplay($nav->get('Content'), 'Taxonomies');
+
+        // Ensure old parents are not active
+        $this->assertFalse($collections->isActive());
+        $this->assertFalse($taxonomies->isActive());
+
+        // Ensure moved item is active
+        $this->assertTrue($articles->isActive());
+
+        // Child should not be active in this case though
+        $this->assertFalse($categories->isActive());
+    }
+
+    /** @test */
+    public function active_nav_check_still_functions_properly_on_explicit_child_within_moved_items()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/taxonomies/categories')
+            ->assertStatus(200);
+
+        $nav = $this->build([
+            'top_level' => [
+                'content::collections::articles' => [
+                    'action' => '@move',
+                    'children' => [
+                        'content::taxonomies::categories' => '@move',
+                    ],
+                ],
+            ],
+        ]);
+
+        $articles = $this->getItemByDisplay($nav->get('Top Level'), 'Articles');
+        $categories = $this->getItemByDisplay($articles->children(), 'Categories');
+        $collections = $this->getItemByDisplay($nav->get('Content'), 'Collections');
+        $taxonomies = $this->getItemByDisplay($nav->get('Content'), 'Taxonomies');
+
+        // Ensure old parents are not active
+        $this->assertFalse($collections->isActive());
+        $this->assertFalse($taxonomies->isActive());
+
+        // Ensure moved item is active
+        $this->assertTrue($articles->isActive());
+
+        // Ensure child of moved item is now active
+        $this->assertTrue($categories->isActive());
+    }
+
+    /** @test */
+    public function active_nav_check_still_functions_properly_on_descendant_of_moved_items()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/collections/articles/entries/create/en')
+            ->assertStatus(200);
+
+        $nav = $this->build([
+            'top_level' => [
+                'content::collections::articles' => [
+                    'action' => '@move',
+                    'children' => [
+                        'content::taxonomies::categories' => '@move',
+                    ],
+                ],
+            ],
+        ]);
+
+        $articles = $this->getItemByDisplay($nav->get('Top Level'), 'Articles');
+        $categories = $this->getItemByDisplay($articles->children(), 'Categories');
+        $collections = $this->getItemByDisplay($nav->get('Content'), 'Collections');
+        $taxonomies = $this->getItemByDisplay($nav->get('Content'), 'Taxonomies');
+
+        // Ensure old parents are not active
+        $this->assertFalse($collections->isActive());
+        $this->assertFalse($taxonomies->isActive());
+
+        // Ensure moved item is active, due to URL hierarchy of current URL being a descendant
+        $this->assertTrue($articles->isActive());
+
+        // Child should not be active in this case though
+        $this->assertFalse($categories->isActive());
+    }
+
+    /** @test */
+    public function active_nav_check_still_functions_properly_on_descendant_of_child_within_moved_item()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        $this
+            ->prepareNavCaches()
+            ->get('http://localhost/cp/taxonomies/categories/terms/create/en')
+            ->assertStatus(200);
+
+        $nav = $this->build([
+            'top_level' => [
+                'content::collections::articles' => [
+                    'action' => '@move',
+                    'children' => [
+                        'content::taxonomies::categories' => '@move',
+                    ],
+                ],
+            ],
+        ]);
+
+        $articles = $this->getItemByDisplay($nav->get('Top Level'), 'Articles');
+        $categories = $this->getItemByDisplay($articles->children(), 'Categories');
+        $collections = $this->getItemByDisplay($nav->get('Content'), 'Collections');
+        $taxonomies = $this->getItemByDisplay($nav->get('Content'), 'Taxonomies');
+
+        // Ensure old parents are not active
+        $this->assertFalse($collections->isActive());
+        $this->assertFalse($taxonomies->isActive());
+
+        // Ensure moved item is active
+        $this->assertTrue($articles->isActive());
+
+        // Child should not be active in this case, due to URL hierarchy of current URL being a descendant
+        $this->assertTrue($categories->isActive());
+    }
+
     protected function prepareNavCaches()
     {
         // Clear caches
@@ -587,9 +751,9 @@ protected function prepareNavCaches()
         return $this;
     }
 
-    protected function build()
+    protected function build($preferences = null)
     {
-        return Nav::build()->pluck('items', 'display');
+        return Nav::build($preferences)->pluck('items', 'display');
     }
 
     protected function buildAndGetItem($sectionDisplay, $itemDisplay)

From 0ee1e0e6e16c9cf4cef3260b747aa9f858cb6abd Mon Sep 17 00:00:00 2001
From: Jesse Leite <jesseleite@gmail.com>
Date: Tue, 31 Oct 2023 21:04:54 -0400
Subject: [PATCH 25/25] Test that caches get updated when new children are
 resolved.

---
 tests/CP/Navigation/ActiveNavItemTest.php | 91 +++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/tests/CP/Navigation/ActiveNavItemTest.php b/tests/CP/Navigation/ActiveNavItemTest.php
index 863cbad6130..ce26c7fc03a 100644
--- a/tests/CP/Navigation/ActiveNavItemTest.php
+++ b/tests/CP/Navigation/ActiveNavItemTest.php
@@ -49,6 +49,97 @@ protected function resolveApplicationConfiguration($app)
         });
     }
 
+    /** @test */
+    public function it_resolves_all_children_only_once_to_build_caches_for_is_active_checks()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        Facades\Taxonomy::make('tags')->title('Tags')->save();
+        Facades\Taxonomy::make('categories')->title('Categories')->save();
+
+        // Clear caches
+        Nav::clearCachedUrls();
+        $this->assertFalse(Cache::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertFalse(Blink::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertFalse(Cache::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertFalse(Blink::has(NavBuilder::ALL_URLS_CACHE_KEY));
+
+        // Ensure that all children are resolved and URLs are cached for `isActive()` checks on first build
+        $nav = Nav::build()->pluck('items', 'display');
+        $this->assertTrue(Cache::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertTrue(Blink::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertTrue(Cache::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertTrue(Blink::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertInstanceOf(Collection::class, $this->getItemByDisplay($nav->get('Content'), 'Collections')->children());
+        $this->assertInstanceOf(Collection::class, $this->getItemByDisplay($nav->get('Content'), 'Taxonomies')->children());
+
+        // Ensure that it builds children as unresolved closures on second build
+        $nav = Nav::build()->pluck('items', 'display');
+        $this->assertTrue(Cache::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertTrue(Blink::has(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY));
+        $this->assertTrue(Cache::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertTrue(Blink::has(NavBuilder::ALL_URLS_CACHE_KEY));
+        $this->assertInstanceOf(Closure::class, $this->getItemByDisplay($nav->get('Content'), 'Collections')->children());
+        $this->assertInstanceOf(Closure::class, $this->getItemByDisplay($nav->get('Content'), 'Taxonomies')->children());
+    }
+
+    /** @test */
+    public function it_updates_caches_when_new_child_urls_are_detected_after_resolving_children()
+    {
+        Facades\Collection::make('pages')->title('Pages')->save();
+        Facades\Collection::make('articles')->title('Articles')->save();
+
+        // Ensure we clear cached URLs and build nav cache
+        Nav::clearCachedUrls();
+        Nav::build();
+
+        // Assert that our collection children are properly cached
+        $collectionsChildrenUrls = [
+            'http://localhost/cp/collections/articles',
+            'http://localhost/cp/collections/pages',
+        ];
+        $this->assertEquals($collectionsChildrenUrls, Cache::get(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)->get('content::collections'));
+        $this->assertEquals($collectionsChildrenUrls, Blink::get(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)->get('content::collections'));
+        collect($collectionsChildrenUrls)->each(function ($url) {
+            $this->assertTrue(Cache::get(NavBuilder::ALL_URLS_CACHE_KEY)->contains($url));
+            $this->assertTrue(Blink::get(NavBuilder::ALL_URLS_CACHE_KEY)->contains($url));
+        });
+
+        // Now let's create a new collection
+        Facades\Collection::make('products')->title('Products')->save();
+
+        // Simply building the nav should change what is cached
+        $collectionsChildrenUrls = [
+            'http://localhost/cp/collections/articles',
+            'http://localhost/cp/collections/pages',
+        ];
+        $this->assertEquals($collectionsChildrenUrls, Cache::get(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)->get('content::collections'));
+        $this->assertEquals($collectionsChildrenUrls, Blink::get(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)->get('content::collections'));
+        collect($collectionsChildrenUrls)->each(function ($url) {
+            $this->assertTrue(Cache::get(NavBuilder::ALL_URLS_CACHE_KEY)->contains($url));
+            $this->assertTrue(Blink::get(NavBuilder::ALL_URLS_CACHE_KEY)->contains($url));
+        });
+
+        // But if we build the nav again by hitting collections url to resolve its' children, the caches should get updated
+        $this
+            ->get('http://localhost/cp/collections')
+            ->assertStatus(200);
+
+        // Assert that our collection children caches are properly updated
+        $updatedChildrenUrls = [
+            'http://localhost/cp/collections/articles',
+            'http://localhost/cp/collections/pages',
+            'http://localhost/cp/collections/products',
+        ];
+        $this->assertEquals($updatedChildrenUrls, Cache::get(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)->get('content::collections'));
+        $this->assertEquals($updatedChildrenUrls, Blink::get(NavBuilder::UNRESOLVED_CHILDREN_URLS_CACHE_KEY)->get('content::collections'));
+        collect($updatedChildrenUrls)->each(function ($url) {
+            $this->assertTrue(Cache::get(NavBuilder::ALL_URLS_CACHE_KEY)->contains($url));
+            $this->assertTrue(Blink::get(NavBuilder::ALL_URLS_CACHE_KEY)->contains($url));
+        });
+    }
+
     /** @test */
     public function it_builds_core_children_closure_when_not_active()
     {