Queries: Update API to align data with previous versions

Ensures fields returned match API docs and previous versions of
BookStack where we were accidentally returning more fields than
expected.
Updates tests to cover many of these.
Also updated clockwork to ignore image requests for less noisy
debugging.
Also updated chapter page query to not be loading all page data, via new
query in PageQueries.
This commit is contained in:
Dan Brown 2024-02-11 15:42:37 +00:00
parent ed9c013f6e
commit 1ea2ac864a
No known key found for this signature in database
GPG key ID: 46D9F943C24A2EF9
11 changed files with 56 additions and 14 deletions

View file

@ -173,6 +173,8 @@ return [
// List of URIs that should not be collected // List of URIs that should not be collected
'except' => [ 'except' => [
'/uploads/images/.*', // BookStack image requests
'/horizon/.*', // Laravel Horizon requests '/horizon/.*', // Laravel Horizon requests
'/telescope/.*', // Laravel Telescope requests '/telescope/.*', // Laravel Telescope requests
'/_debugbar/.*', // Laravel DebugBar requests '/_debugbar/.*', // Laravel DebugBar requests

View file

@ -26,7 +26,9 @@ class BookApiController extends ApiController
*/ */
public function list() public function list()
{ {
$books = $this->queries->visibleForList(); $books = $this->queries
->visibleForList()
->addSelect(['created_by', 'updated_by']);
return $this->apiListingResponse($books, [ return $this->apiListingResponse($books, [
'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by',

View file

@ -24,7 +24,9 @@ class BookshelfApiController extends ApiController
*/ */
public function list() public function list()
{ {
$shelves = $this->queries->visibleForList(); $shelves = $this->queries
->visibleForList()
->addSelect(['created_by', 'updated_by']);
return $this->apiListingResponse($shelves, [ return $this->apiListingResponse($shelves, [
'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by',

View file

@ -3,8 +3,8 @@
namespace BookStack\Entities\Controllers; namespace BookStack\Entities\Controllers;
use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Chapter;
use BookStack\Entities\Queries\BookQueries;
use BookStack\Entities\Queries\ChapterQueries; use BookStack\Entities\Queries\ChapterQueries;
use BookStack\Entities\Queries\EntityQueries;
use BookStack\Entities\Repos\ChapterRepo; use BookStack\Entities\Repos\ChapterRepo;
use BookStack\Exceptions\PermissionsException; use BookStack\Exceptions\PermissionsException;
use BookStack\Http\ApiController; use BookStack\Http\ApiController;
@ -38,7 +38,7 @@ class ChapterApiController extends ApiController
public function __construct( public function __construct(
protected ChapterRepo $chapterRepo, protected ChapterRepo $chapterRepo,
protected ChapterQueries $queries, protected ChapterQueries $queries,
protected BookQueries $bookQueries, protected EntityQueries $entityQueries,
) { ) {
} }
@ -47,7 +47,8 @@ class ChapterApiController extends ApiController
*/ */
public function list() public function list()
{ {
$chapters = $this->queries->visibleForList(); $chapters = $this->queries->visibleForList()
->addSelect(['created_by', 'updated_by']);
return $this->apiListingResponse($chapters, [ return $this->apiListingResponse($chapters, [
'id', 'book_id', 'name', 'slug', 'description', 'priority', 'id', 'book_id', 'name', 'slug', 'description', 'priority',
@ -63,7 +64,7 @@ class ChapterApiController extends ApiController
$requestData = $this->validate($request, $this->rules['create']); $requestData = $this->validate($request, $this->rules['create']);
$bookId = $request->get('book_id'); $bookId = $request->get('book_id');
$book = $this->bookQueries->findVisibleByIdOrFail(intval($bookId)); $book = $this->entityQueries->books->findVisibleByIdOrFail(intval($bookId));
$this->checkOwnablePermission('chapter-create', $book); $this->checkOwnablePermission('chapter-create', $book);
$chapter = $this->chapterRepo->create($requestData, $book); $chapter = $this->chapterRepo->create($requestData, $book);
@ -79,12 +80,14 @@ class ChapterApiController extends ApiController
$chapter = $this->queries->findVisibleByIdOrFail(intval($id)); $chapter = $this->queries->findVisibleByIdOrFail(intval($id));
$chapter = $this->forJsonDisplay($chapter); $chapter = $this->forJsonDisplay($chapter);
$chapter->load([ $chapter->load(['createdBy', 'updatedBy', 'ownedBy']);
'createdBy', 'updatedBy', 'ownedBy',
'pages' => function (HasMany $query) { // Note: More fields than usual here, for backwards compatibility,
$query->scopes('visible')->get(['id', 'name', 'slug']); // due to previously accidentally including more fields that desired.
} $pages = $this->entityQueries->pages->visibleForChapterList($chapter->id)
]); ->addSelect(['created_by', 'updated_by', 'revision_count', 'editor'])
->get();
$chapter->setRelation('pages', $pages);
return response()->json($chapter); return response()->json($chapter);
} }

View file

@ -79,7 +79,8 @@ class ChapterController extends Controller
$this->checkOwnablePermission('chapter-view', $chapter); $this->checkOwnablePermission('chapter-view', $chapter);
$sidebarTree = (new BookContents($chapter->book))->getTree(); $sidebarTree = (new BookContents($chapter->book))->getTree();
$pages = $chapter->getVisiblePages(); $pages = $this->entityQueries->pages->visibleForChapterList($chapter->id)->get();
$nextPreviousLocator = new NextPreviousContentLocator($chapter, $sidebarTree); $nextPreviousLocator = new NextPreviousContentLocator($chapter, $sidebarTree);
View::incrementFor($chapter); View::incrementFor($chapter);

View file

@ -45,7 +45,8 @@ class PageApiController extends ApiController
*/ */
public function list() public function list()
{ {
$pages = $this->queries->visibleForList(); $pages = $this->queries->visibleForList()
->addSelect(['created_by', 'updated_by', 'revision_count', 'editor']);
return $this->apiListingResponse($pages, [ return $this->apiListingResponse($pages, [
'id', 'book_id', 'chapter_id', 'name', 'slug', 'priority', 'id', 'book_id', 'chapter_id', 'name', 'slug', 'priority',

View file

@ -73,6 +73,14 @@ class PageQueries implements ProvidesEntityQueries
->select($this->mergeBookSlugForSelect(static::$listAttributes)); ->select($this->mergeBookSlugForSelect(static::$listAttributes));
} }
public function visibleForChapterList(int $chapterId): Builder
{
return $this->visibleForList()
->where('chapter_id', '=', $chapterId)
->orderBy('draft', 'desc')
->orderBy('priority', 'asc');
}
public function visibleWithContents(): Builder public function visibleWithContents(): Builder
{ {
return $this->start() return $this->start()

View file

@ -24,6 +24,9 @@ class BooksApiTest extends TestCase
'id' => $firstBook->id, 'id' => $firstBook->id,
'name' => $firstBook->name, 'name' => $firstBook->name,
'slug' => $firstBook->slug, 'slug' => $firstBook->slug,
'owned_by' => $firstBook->owned_by,
'created_by' => $firstBook->created_by,
'updated_by' => $firstBook->updated_by,
], ],
]]); ]]);
} }

View file

@ -28,6 +28,9 @@ class ChaptersApiTest extends TestCase
'book_id' => $firstChapter->book->id, 'book_id' => $firstChapter->book->id,
'priority' => $firstChapter->priority, 'priority' => $firstChapter->priority,
'book_slug' => $firstChapter->book->slug, 'book_slug' => $firstChapter->book->slug,
'owned_by' => $firstChapter->owned_by,
'created_by' => $firstChapter->created_by,
'updated_by' => $firstChapter->updated_by,
], ],
]]); ]]);
} }
@ -149,6 +152,16 @@ class ChaptersApiTest extends TestCase
'id' => $page->id, 'id' => $page->id,
'slug' => $page->slug, 'slug' => $page->slug,
'name' => $page->name, 'name' => $page->name,
'owned_by' => $page->owned_by,
'created_by' => $page->created_by,
'updated_by' => $page->updated_by,
'book_id' => $page->id,
'chapter_id' => $chapter->id,
'priority' => $page->priority,
'book_slug' => $chapter->book->slug,
'draft' => $page->draft,
'template' => $page->template,
'editor' => $page->editor,
], ],
], ],
'default_template_id' => null, 'default_template_id' => null,

View file

@ -27,6 +27,10 @@ class PagesApiTest extends TestCase
'slug' => $firstPage->slug, 'slug' => $firstPage->slug,
'book_id' => $firstPage->book->id, 'book_id' => $firstPage->book->id,
'priority' => $firstPage->priority, 'priority' => $firstPage->priority,
'owned_by' => $firstPage->owned_by,
'created_by' => $firstPage->created_by,
'updated_by' => $firstPage->updated_by,
'revision_count' => $firstPage->revision_count,
], ],
]]); ]]);
} }

View file

@ -25,6 +25,9 @@ class ShelvesApiTest extends TestCase
'id' => $firstBookshelf->id, 'id' => $firstBookshelf->id,
'name' => $firstBookshelf->name, 'name' => $firstBookshelf->name,
'slug' => $firstBookshelf->slug, 'slug' => $firstBookshelf->slug,
'owned_by' => $firstBookshelf->owned_by,
'created_by' => $firstBookshelf->created_by,
'updated_by' => $firstBookshelf->updated_by,
], ],
]]); ]]);
} }