From f5f96f84e7061dc2356510760c200ccae415fee5 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Wed, 28 Feb 2024 13:07:34 +0000
Subject: [PATCH] 404: Fixed entity list issue with entity with non-visible
 parent

Adds our mixed entity list loader to popular queries for more efficient
loading.
---
 app/Entities/Queries/QueryPopular.php | 26 +++++++-----------------
 tests/ErrorTest.php                   | 29 +++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/app/Entities/Queries/QueryPopular.php b/app/Entities/Queries/QueryPopular.php
index b2ca565eb..2b46ebfbc 100644
--- a/app/Entities/Queries/QueryPopular.php
+++ b/app/Entities/Queries/QueryPopular.php
@@ -4,10 +4,8 @@ namespace BookStack\Entities\Queries;
 
 use BookStack\Activity\Models\View;
 use BookStack\Entities\EntityProvider;
-use BookStack\Entities\Models\BookChild;
-use BookStack\Entities\Models\Entity;
+use BookStack\Entities\Tools\MixedEntityListLoader;
 use BookStack\Permissions\PermissionApplicator;
-use Illuminate\Database\Eloquent\Relations\BelongsTo;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\DB;
 
@@ -16,10 +14,11 @@ class QueryPopular
     public function __construct(
         protected PermissionApplicator $permissions,
         protected EntityProvider $entityProvider,
+        protected MixedEntityListLoader $listLoader,
     ) {
     }
 
-    public function run(int $count, int $page, array $filterModels = null)
+    public function run(int $count, int $page, array $filterModels = null): Collection
     {
         $query = $this->permissions
             ->restrictEntityRelationQuery(View::query(), 'views', 'viewable_id', 'viewable_type')
@@ -31,24 +30,13 @@ class QueryPopular
             $query->whereIn('viewable_type', $this->entityProvider->getMorphClasses($filterModels));
         }
 
-        $entities = $query->with('viewable')
+        $views = $query
             ->skip($count * ($page - 1))
             ->take($count)
-            ->get()
-            ->pluck('viewable')
-            ->filter();
+            ->get();
 
-        $this->loadBooksForChildren($entities);
+        $this->listLoader->loadIntoRelations($views->all(), 'viewable', false);
 
-        return $entities;
-    }
-
-    protected function loadBooksForChildren(Collection $entities): void
-    {
-        $bookChildren = $entities->filter(fn(Entity $entity) => $entity instanceof BookChild);
-        $eloquent = (new \Illuminate\Database\Eloquent\Collection($bookChildren));
-        $eloquent->load(['book' => function (BelongsTo $query) {
-            $query->scopes('visible');
-        }]);
+        return $views->pluck('viewable')->filter();
     }
 }
diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php
index 0d2ef808c..642945d43 100644
--- a/tests/ErrorTest.php
+++ b/tests/ErrorTest.php
@@ -23,6 +23,35 @@ class ErrorTest extends TestCase
         $notFound->assertSeeText('tester');
     }
 
+    public function test_404_page_does_not_non_visible_content()
+    {
+        $editor = $this->users->editor();
+        $book = $this->entities->book();
+
+        $this->actingAs($editor)->get($book->getUrl())->assertOk();
+
+        $this->permissions->disableEntityInheritedPermissions($book);
+
+        $this->actingAs($editor)->get($book->getUrl())->assertNotFound();
+    }
+
+    public function test_404_page_shows_visible_content_within_non_visible_parent()
+    {
+        $editor = $this->users->editor();
+        $book = $this->entities->book();
+        $page = $book->pages()->first();
+
+        $this->actingAs($editor)->get($page->getUrl())->assertOk();
+
+        $this->permissions->disableEntityInheritedPermissions($book);
+        $this->permissions->addEntityPermission($page, ['view'], $editor->roles()->first());
+
+        $resp = $this->actingAs($editor)->get($book->getUrl());
+        $resp->assertNotFound();
+        $resp->assertSee($page->name);
+        $resp->assertDontSee($book->name);
+    }
+
     public function test_item_not_found_does_not_get_logged_to_file()
     {
         $this->actingAs($this->users->viewer());