From fec44452cb67819b594bdfca4ca37e4a20c0f42e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 3 Dec 2024 13:47:45 +0000 Subject: [PATCH] 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. --- app/Api/ApiEntityListFormatter.php | 32 ++---- app/Search/SearchApiController.php | 15 +-- dev/api/responses/search-all.json | 172 ++++++++++++++--------------- tests/Api/SearchApiTest.php | 44 +++++++- 4 files changed, 142 insertions(+), 121 deletions(-) diff --git a/app/Api/ApiEntityListFormatter.php b/app/Api/ApiEntityListFormatter.php index 7c2d09d4f..3c94d96ee 100644 --- 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. */ diff --git a/app/Search/SearchApiController.php b/app/Search/SearchApiController.php index 28a3b53e6..79cd8cfab 100644 --- 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'), diff --git a/dev/api/responses/search-all.json b/dev/api/responses/search-all.json index f60a12f75..2ad896416 100644 --- 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 cats", + "content": "...once a bunch of cats named tony...behaviour of cats 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 cats", + "content": "...people ask why cats? but there are...the reason that cats 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 cats", - "content": "...once a bunch of cats named tony...behaviour of cats 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 cats", - "content": "...people ask why cats? but there are...the reason that cats 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 cats?", - "content": "cats 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 cats?", + "content": "cats are some of the most advanced animals in the world." + }, + "tags": [] + } + ], + "total": 3 } diff --git a/tests/Api/SearchApiTest.php b/tests/Api/SearchApiTest.php index 3f2eb395c..9da7900ca 100644 --- 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'); + } }