Search API: Updated handling of parent detail, added testing

Review of #5280. - Removed additional non-needed loads which could ignore permissions. - Updated new formatter method name to be more specific on use. - Added test case to cover changes. - Updated API examples to align parent id/info in info to be representative.
2025-01-31 12:11:37 +01:00 · 2024-12-03 13:47:45 +00:00 · 2024-12-03 13:47:45 +00:00 · fec44452cb
commit fec44452cb
parent f606711463
4 changed files with 142 additions and 121 deletions
--- a/app/Api/ApiEntityListFormatter.php
+++ b/app/Api/ApiEntityListFormatter.php
@ -2,6 +2,7 @@

 namespace BookStack\Api;

+use BookStack\Entities\Models\BookChild;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Models\Page;

@ -72,20 +73,20 @@ class ApiEntityListFormatter
    }

    /**
-     * Enable the inclusion of related book and chapter titles in the response.
+     * Include parent book/chapter info in the formatted data.
     */
-    public function withRelatedData(): self
+    public function withParents(): self
    {
        $this->withField('book', function (Entity $entity) {
-            if (method_exists($entity, 'book')) {
-                return $entity->book()->select(['id', 'name', 'slug'])->first();
+            if ($entity instanceof BookChild && $entity->book) {
+                return $entity->book->only(['id', 'name', 'slug']);
            }
            return null;
        });

        $this->withField('chapter', function (Entity $entity) {
-            if ($entity instanceof Page && $entity->chapter_id) {
-                return $entity->chapter()->select(['id', 'name', 'slug'])->first();
+            if ($entity instanceof Page && $entity->chapter) {
+                return $entity->chapter->only(['id', 'name', 'slug']);
            }
            return null;
        });
@ -99,8 +100,6 @@ class ApiEntityListFormatter
     */
    public function format(): array
    {
-        $this->loadRelatedData();
-
        $results = [];

        foreach ($this->list as $item) {
@ -110,23 +109,6 @@ class ApiEntityListFormatter
        return $results;
    }

-    /**
-     * Eager load the related book and chapter data when needed.
-     */
-    protected function loadRelatedData(): void
-    {
-        $pages = collect($this->list)->filter(fn($item) => $item instanceof Page);
-
-        foreach ($this->list as $entity) {
-            if (method_exists($entity, 'book')) {
-                $entity->load('book');
-            }
-            if ($entity instanceof Page && $entity->chapter_id) {
-                $entity->load('chapter');
-            }
-        }
-    }
-
    /**
     * Format a single entity item to a plain array.
     */
--- a/app/Search/SearchApiController.php
+++ b/app/Search/SearchApiController.php
@ -9,21 +9,18 @@ use Illuminate\Http\Request;

 class SearchApiController extends ApiController
 {
-    protected SearchRunner $searchRunner;
-    protected SearchResultsFormatter $resultsFormatter;
-
    protected $rules = [
        'all' => [
            'query' => ['required'],
-            'page' => ['integer', 'min:1'],
+            'page'  => ['integer', 'min:1'],
            'count' => ['integer', 'min:1', 'max:100'],
        ],
    ];

-    public function __construct(SearchRunner $searchRunner, SearchResultsFormatter $resultsFormatter)
-    {
-        $this->searchRunner = $searchRunner;
-        $this->resultsFormatter = $resultsFormatter;
+    public function __construct(
+        protected SearchRunner $searchRunner,
+        protected SearchResultsFormatter $resultsFormatter
+    ) {
    }

    /**
@ -50,7 +47,7 @@ class SearchApiController extends ApiController
        $this->resultsFormatter->format($results['results']->all(), $options);

        $data = (new ApiEntityListFormatter($results['results']->all()))
-            ->withType()->withTags()->withRelatedData()
+            ->withType()->withTags()->withParents()
            ->withField('preview_html', function (Entity $entity) {
                return [
                    'name' => (string) $entity->getAttribute('preview_name'),
--- a/dev/api/responses/search-all.json
+++ b/dev/api/responses/search-all.json
@ -1,92 +1,92 @@
 {
-    "data": [
+  "data": [
+    {
+      "id": 84,
+      "book_id": 1,
+      "slug": "a-chapter-for-cats",
+      "name": "A chapter for cats",
+      "created_at": "2021-11-14T15:57:35.000000Z",
+      "updated_at": "2021-11-14T15:57:35.000000Z",
+      "type": "chapter",
+      "url": "https://example.com/books/cats/chapter/a-chapter-for-cats",
+      "book": {
+        "id": 1,
+        "name": "Cats",
+        "slug": "cats"
+      },
+      "preview_html": {
+        "name": "A chapter for <strong>cats</strong>",
+        "content": "...once a bunch of <strong>cats</strong> named tony...behaviour of <strong>cats</strong> is unsuitable"
+      },
+      "tags": []
+    },
+    {
+      "name": "The hows and whys of cats",
+      "id": 396,
+      "slug": "the-hows-and-whys-of-cats",
+      "book_id": 1,
+      "chapter_id": 75,
+      "draft": false,
+      "template": false,
+      "created_at": "2021-05-15T16:28:10.000000Z",
+      "updated_at": "2021-11-14T15:56:49.000000Z",
+      "type": "page",
+      "url": "https://example.com/books/cats/page/the-hows-and-whys-of-cats",
+      "book": {
+        "id": 1,
+        "name": "Cats",
+        "slug": "cats"
+      },
+      "chapter": {
+        "id": 75,
+        "name": "A chapter for cats",
+        "slug": "a-chapter-for-cats"
+      },
+      "preview_html": {
+        "name": "The hows and whys of <strong>cats</strong>",
+        "content": "...people ask why <strong>cats</strong>? but there are...the reason that <strong>cats</strong> are fast are due to..."
+      },
+      "tags": [
        {
-            "id": 84,
-            "book_id": 1,
-            "slug": "a-chapter-for-cats",
-            "name": "A chapter for cats",
-            "created_at": "2021-11-14T15:57:35.000000Z",
-            "updated_at": "2021-11-14T15:57:35.000000Z",
-            "type": "chapter",
-            "url": "https://example.com/books/my-book/chapter/a-chapter-for-cats",
-            "book": {
-                "id": 1,
-                "name": "Cats",
-                "slug": "cats"
-            },
-            "preview_html": {
-                "name": "A chapter for <strong>cats</strong>",
-                "content": "...once a bunch of <strong>cats</strong> named tony...behaviour of <strong>cats</strong> is unsuitable"
-            },
-            "tags": []
+          "name": "Animal",
+          "value": "Cat",
+          "order": 0
        },
        {
-            "name": "The hows and whys of cats",
-            "id": 396,
-            "slug": "the-hows-and-whys-of-cats",
-            "book_id": 1,
-            "chapter_id": 75,
-            "draft": false,
-            "template": false,
-            "created_at": "2021-05-15T16:28:10.000000Z",
-            "updated_at": "2021-11-14T15:56:49.000000Z",
-            "type": "page",
-            "url": "https://example.com/books/my-book/page/the-hows-and-whys-of-cats",
-            "book": {
-                "id": 1,
-                "name": "Cats",
-                "slug": "cats"
-            },
-            "chapter": {
-                "id": 84,
-                "name": "A chapter for cats",
-                "slug": "a-chapter-for-cats"
-            },
-            "preview_html": {
-                "name": "The hows and whys of <strong>cats</strong>",
-                "content": "...people ask why <strong>cats</strong>? but there are...the reason that <strong>cats</strong> are fast are due to..."
-            },
-            "tags": [
-                {
-                    "name": "Animal",
-                    "value": "Cat",
-                    "order": 0
-                },
-                {
-                    "name": "Category",
-                    "value": "Top Content",
-                    "order": 0
-                }
-            ]
-        },
-        {
-            "name": "How advanced are cats?",
-            "id": 362,
-            "slug": "how-advanced-are-cats",
-            "book_id": 13,
-            "chapter_id": 73,
-            "draft": false,
-            "template": false,
-            "created_at": "2020-11-29T21:55:07.000000Z",
-            "updated_at": "2021-11-14T16:02:39.000000Z",
-            "type": "page",
-            "url": "https://example.com/books/my-book/page/how-advanced-are-cats",
-            "book": {
-                "id": 1,
-                "name": "Cats",
-                "slug": "cats"
-            },
-            "chapter": {
-                "id": 84,
-                "name": "A chapter for cats",
-                "slug": "a-chapter-for-cats"
-            },
-            "preview_html": {
-                "name": "How advanced are <strong>cats</strong>?",
-                "content": "<strong>cats</strong> are some of the most advanced animals in the world."
-            },
-            "tags": []
+          "name": "Category",
+          "value": "Top Content",
+          "order": 0
        }
-    ],
-    "total": 3
+      ]
+    },
+    {
+      "name": "How advanced are cats?",
+      "id": 362,
+      "slug": "how-advanced-are-cats",
+      "book_id": 13,
+      "chapter_id": 73,
+      "draft": false,
+      "template": false,
+      "created_at": "2020-11-29T21:55:07.000000Z",
+      "updated_at": "2021-11-14T16:02:39.000000Z",
+      "type": "page",
+      "url": "https://example.com/books/big-cats/page/how-advanced-are-cats",
+      "book": {
+        "id": 13,
+        "name": "Big Cats",
+        "slug": "big-cats"
+      },
+      "chapter": {
+        "id": 73,
+        "name": "A chapter for bigger cats",
+        "slug": "a-chapter-for-bigger-cats"
+      },
+      "preview_html": {
+        "name": "How advanced are <strong>cats</strong>?",
+        "content": "<strong>cats</strong> are some of the most advanced animals in the world."
+      },
+      "tags": []
+    }
+  ],
+  "total": 3
 }
--- a/tests/Api/SearchApiTest.php
+++ b/tests/Api/SearchApiTest.php
@ -13,7 +13,7 @@ class SearchApiTest extends TestCase
 {
    use TestsApi;

-    protected $baseEndpoint = '/api/search';
+    protected string $baseEndpoint = '/api/search';

    public function test_all_endpoint_returns_search_filtered_results_with_query()
    {
@ -74,4 +74,46 @@ class SearchApiTest extends TestCase
        $resp = $this->actingAsApiEditor()->get($this->baseEndpoint . '?query=myqueryvalue');
        $resp->assertOk();
    }
+
+    public function test_all_endpoint_includes_parent_details_where_visible()
+    {
+        $page = $this->entities->pageWithinChapter();
+        $chapter = $page->chapter;
+        $book = $page->book;
+
+        $page->update(['name' => 'name with superextrauniquevalue within']);
+        $page->indexForSearch();
+
+        $editor = $this->users->editor();
+        $this->actingAsApiEditor();
+        $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertJsonFragment([
+            'id' => $page->id,
+            'type' => 'page',
+            'book' => [
+                'id' => $book->id,
+                'name' => $book->name,
+                'slug' => $book->slug,
+            ],
+            'chapter' => [
+                'id' => $chapter->id,
+                'name' => $chapter->name,
+                'slug' => $chapter->slug,
+            ],
+        ]);
+
+        $this->permissions->disableEntityInheritedPermissions($chapter);
+        $this->permissions->setEntityPermissions($page, ['view'], [$editor->roles()->first()]);
+
+        $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertJsonPath('data.0.id', $page->id);
+        $resp->assertJsonPath('data.0.book.name', $book->name);
+        $resp->assertJsonMissingPath('data.0.chapter');
+
+        $this->permissions->disableEntityInheritedPermissions($book);
+
+        $resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
+        $resp->assertJsonPath('data.0.id', $page->id);
+        $resp->assertJsonMissingPath('data.0.book.name');
+    }
 }