Skip to content

Commit fec4445

Browse files
ssddanbrownssddanbrown
committed
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.
1 parent f606711 commit fec4445

File tree

4 files changed

+142
-121
lines changed

4 files changed

+142
-121
lines changed

‎app/Api/ApiEntityListFormatter.php

+7-25
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace BookStack\Api;
44

5+
use BookStack\Entities\Models\BookChild;
56
use BookStack\Entities\Models\Entity;
67
use BookStack\Entities\Models\Page;
78

@@ -72,20 +73,20 @@ public function withTags(): self
7273
}
7374

7475
/**
75-
* Enable the inclusion of related book and chapter titles in the response.
76+
* Include parent book/chapter info in the formatted data.
7677
*/
77-
public function withRelatedData(): self
78+
public function withParents(): self
7879
{
7980
$this->withField('book', function (Entity $entity) {
80-
if (method_exists($entity, 'book')) {
81-
return $entity->book()->select(['id', 'name', 'slug'])->first();
81+
if ($entity instanceof BookChild && $entity->book) {
82+
return $entity->book->only(['id', 'name', 'slug']);
8283
}
8384
return null;
8485
});
8586

8687
$this->withField('chapter', function (Entity $entity) {
87-
if ($entity instanceof Page && $entity->chapter_id) {
88-
return $entity->chapter()->select(['id', 'name', 'slug'])->first();
88+
if ($entity instanceof Page && $entity->chapter) {
89+
return $entity->chapter->only(['id', 'name', 'slug']);
8990
}
9091
return null;
9192
});
@@ -99,8 +100,6 @@ public function withRelatedData(): self
99100
*/
100101
public function format(): array
101102
{
102-
$this->loadRelatedData();
103-
104103
$results = [];
105104

106105
foreach ($this->list as $item) {
@@ -110,23 +109,6 @@ public function format(): array
110109
return $results;
111110
}
112111

113-
/**
114-
* Eager load the related book and chapter data when needed.
115-
*/
116-
protected function loadRelatedData(): void
117-
{
118-
$pages = collect($this->list)->filter(fn($item) => $item instanceof Page);
119-
120-
foreach ($this->list as $entity) {
121-
if (method_exists($entity, 'book')) {
122-
$entity->load('book');
123-
}
124-
if ($entity instanceof Page && $entity->chapter_id) {
125-
$entity->load('chapter');
126-
}
127-
}
128-
}
129-
130112
/**
131113
* Format a single entity item to a plain array.
132114
*/

‎app/Search/SearchApiController.php

+6-9
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,18 @@
99

1010
class SearchApiController extends ApiController
1111
{
12-
protected SearchRunner $searchRunner;
13-
protected SearchResultsFormatter $resultsFormatter;
14-
1512
protected $rules = [
1613
'all' => [
1714
'query' => ['required'],
18-
'page' => ['integer', 'min:1'],
15+
'page' => ['integer', 'min:1'],
1916
'count' => ['integer', 'min:1', 'max:100'],
2017
],
2118
];
2219

23-
public function __construct(SearchRunner $searchRunner, SearchResultsFormatter $resultsFormatter)
24-
{
25-
$this->searchRunner = $searchRunner;
26-
$this->resultsFormatter = $resultsFormatter;
20+
public function __construct(
21+
protected SearchRunner $searchRunner,
22+
protected SearchResultsFormatter $resultsFormatter
23+
) {
2724
}
2825

2926
/**
@@ -50,7 +47,7 @@ public function all(Request $request)
5047
$this->resultsFormatter->format($results['results']->all(), $options);
5148

5249
$data = (new ApiEntityListFormatter($results['results']->all()))
53-
->withType()->withTags()->withRelatedData()
50+
->withType()->withTags()->withParents()
5451
->withField('preview_html', function (Entity $entity) {
5552
return [
5653
'name' => (string) $entity->getAttribute('preview_name'),

‎dev/api/responses/search-all.json

+86-86
Original file line numberDiff line numberDiff line change
@@ -1,92 +1,92 @@
11
{
2-
"data": [
2+
"data": [
3+
{
4+
"id": 84,
5+
"book_id": 1,
6+
"slug": "a-chapter-for-cats",
7+
"name": "A chapter for cats",
8+
"created_at": "2021-11-14T15:57:35.000000Z",
9+
"updated_at": "2021-11-14T15:57:35.000000Z",
10+
"type": "chapter",
11+
"url": "https://example.com/books/cats/chapter/a-chapter-for-cats",
12+
"book": {
13+
"id": 1,
14+
"name": "Cats",
15+
"slug": "cats"
16+
},
17+
"preview_html": {
18+
"name": "A chapter for <strong>cats</strong>",
19+
"content": "...once a bunch of <strong>cats</strong> named tony...behaviour of <strong>cats</strong> is unsuitable"
20+
},
21+
"tags": []
22+
},
23+
{
24+
"name": "The hows and whys of cats",
25+
"id": 396,
26+
"slug": "the-hows-and-whys-of-cats",
27+
"book_id": 1,
28+
"chapter_id": 75,
29+
"draft": false,
30+
"template": false,
31+
"created_at": "2021-05-15T16:28:10.000000Z",
32+
"updated_at": "2021-11-14T15:56:49.000000Z",
33+
"type": "page",
34+
"url": "https://example.com/books/cats/page/the-hows-and-whys-of-cats",
35+
"book": {
36+
"id": 1,
37+
"name": "Cats",
38+
"slug": "cats"
39+
},
40+
"chapter": {
41+
"id": 75,
42+
"name": "A chapter for cats",
43+
"slug": "a-chapter-for-cats"
44+
},
45+
"preview_html": {
46+
"name": "The hows and whys of <strong>cats</strong>",
47+
"content": "...people ask why <strong>cats</strong>? but there are...the reason that <strong>cats</strong> are fast are due to..."
48+
},
49+
"tags": [
350
{
4-
"id": 84,
5-
"book_id": 1,
6-
"slug": "a-chapter-for-cats",
7-
"name": "A chapter for cats",
8-
"created_at": "2021-11-14T15:57:35.000000Z",
9-
"updated_at": "2021-11-14T15:57:35.000000Z",
10-
"type": "chapter",
11-
"url": "https://example.com/books/my-book/chapter/a-chapter-for-cats",
12-
"book": {
13-
"id": 1,
14-
"name": "Cats",
15-
"slug": "cats"
16-
},
17-
"preview_html": {
18-
"name": "A chapter for <strong>cats</strong>",
19-
"content": "...once a bunch of <strong>cats</strong> named tony...behaviour of <strong>cats</strong> is unsuitable"
20-
},
21-
"tags": []
51+
"name": "Animal",
52+
"value": "Cat",
53+
"order": 0
2254
},
2355
{
24-
"name": "The hows and whys of cats",
25-
"id": 396,
26-
"slug": "the-hows-and-whys-of-cats",
27-
"book_id": 1,
28-
"chapter_id": 75,
29-
"draft": false,
30-
"template": false,
31-
"created_at": "2021-05-15T16:28:10.000000Z",
32-
"updated_at": "2021-11-14T15:56:49.000000Z",
33-
"type": "page",
34-
"url": "https://example.com/books/my-book/page/the-hows-and-whys-of-cats",
35-
"book": {
36-
"id": 1,
37-
"name": "Cats",
38-
"slug": "cats"
39-
},
40-
"chapter": {
41-
"id": 84,
42-
"name": "A chapter for cats",
43-
"slug": "a-chapter-for-cats"
44-
},
45-
"preview_html": {
46-
"name": "The hows and whys of <strong>cats</strong>",
47-
"content": "...people ask why <strong>cats</strong>? but there are...the reason that <strong>cats</strong> are fast are due to..."
48-
},
49-
"tags": [
50-
{
51-
"name": "Animal",
52-
"value": "Cat",
53-
"order": 0
54-
},
55-
{
56-
"name": "Category",
57-
"value": "Top Content",
58-
"order": 0
59-
}
60-
]
61-
},
62-
{
63-
"name": "How advanced are cats?",
64-
"id": 362,
65-
"slug": "how-advanced-are-cats",
66-
"book_id": 13,
67-
"chapter_id": 73,
68-
"draft": false,
69-
"template": false,
70-
"created_at": "2020-11-29T21:55:07.000000Z",
71-
"updated_at": "2021-11-14T16:02:39.000000Z",
72-
"type": "page",
73-
"url": "https://example.com/books/my-book/page/how-advanced-are-cats",
74-
"book": {
75-
"id": 1,
76-
"name": "Cats",
77-
"slug": "cats"
78-
},
79-
"chapter": {
80-
"id": 84,
81-
"name": "A chapter for cats",
82-
"slug": "a-chapter-for-cats"
83-
},
84-
"preview_html": {
85-
"name": "How advanced are <strong>cats</strong>?",
86-
"content": "<strong>cats</strong> are some of the most advanced animals in the world."
87-
},
88-
"tags": []
56+
"name": "Category",
57+
"value": "Top Content",
58+
"order": 0
8959
}
90-
],
91-
"total": 3
60+
]
61+
},
62+
{
63+
"name": "How advanced are cats?",
64+
"id": 362,
65+
"slug": "how-advanced-are-cats",
66+
"book_id": 13,
67+
"chapter_id": 73,
68+
"draft": false,
69+
"template": false,
70+
"created_at": "2020-11-29T21:55:07.000000Z",
71+
"updated_at": "2021-11-14T16:02:39.000000Z",
72+
"type": "page",
73+
"url": "https://example.com/books/big-cats/page/how-advanced-are-cats",
74+
"book": {
75+
"id": 13,
76+
"name": "Big Cats",
77+
"slug": "big-cats"
78+
},
79+
"chapter": {
80+
"id": 73,
81+
"name": "A chapter for bigger cats",
82+
"slug": "a-chapter-for-bigger-cats"
83+
},
84+
"preview_html": {
85+
"name": "How advanced are <strong>cats</strong>?",
86+
"content": "<strong>cats</strong> are some of the most advanced animals in the world."
87+
},
88+
"tags": []
89+
}
90+
],
91+
"total": 3
9292
}

‎tests/Api/SearchApiTest.php

+43-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class SearchApiTest extends TestCase
1313
{
1414
use TestsApi;
1515

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

1818
public function test_all_endpoint_returns_search_filtered_results_with_query()
1919
{
@@ -74,4 +74,46 @@ public function test_all_endpoint_requires_query_parameter()
7474
$resp = $this->actingAsApiEditor()->get($this->baseEndpoint . '?query=myqueryvalue');
7575
$resp->assertOk();
7676
}
77+
78+
public function test_all_endpoint_includes_parent_details_where_visible()
79+
{
80+
$page = $this->entities->pageWithinChapter();
81+
$chapter = $page->chapter;
82+
$book = $page->book;
83+
84+
$page->update(['name' => 'name with superextrauniquevalue within']);
85+
$page->indexForSearch();
86+
87+
$editor = $this->users->editor();
88+
$this->actingAsApiEditor();
89+
$resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
90+
$resp->assertJsonFragment([
91+
'id' => $page->id,
92+
'type' => 'page',
93+
'book' => [
94+
'id' => $book->id,
95+
'name' => $book->name,
96+
'slug' => $book->slug,
97+
],
98+
'chapter' => [
99+
'id' => $chapter->id,
100+
'name' => $chapter->name,
101+
'slug' => $chapter->slug,
102+
],
103+
]);
104+
105+
$this->permissions->disableEntityInheritedPermissions($chapter);
106+
$this->permissions->setEntityPermissions($page, ['view'], [$editor->roles()->first()]);
107+
108+
$resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
109+
$resp->assertJsonPath('data.0.id', $page->id);
110+
$resp->assertJsonPath('data.0.book.name', $book->name);
111+
$resp->assertJsonMissingPath('data.0.chapter');
112+
113+
$this->permissions->disableEntityInheritedPermissions($book);
114+
115+
$resp = $this->getJson($this->baseEndpoint . '?query=superextrauniquevalue');
116+
$resp->assertJsonPath('data.0.id', $page->id);
117+
$resp->assertJsonMissingPath('data.0.book.name');
118+
}
77119
}

0 commit comments

Comments
 (0)