From 1ea2ac864aaa7e4ee3995ec675fd92db7b2722cd Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 11 Feb 2024 15:42:37 +0000 Subject: [PATCH] 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. --- app/Config/clockwork.php | 2 ++ .../Controllers/BookApiController.php | 4 +++- .../Controllers/BookshelfApiController.php | 4 +++- .../Controllers/ChapterApiController.php | 23 +++++++++++-------- .../Controllers/ChapterController.php | 3 ++- .../Controllers/PageApiController.php | 3 ++- app/Entities/Queries/PageQueries.php | 8 +++++++ tests/Api/BooksApiTest.php | 3 +++ tests/Api/ChaptersApiTest.php | 13 +++++++++++ tests/Api/PagesApiTest.php | 4 ++++ tests/Api/ShelvesApiTest.php | 3 +++ 11 files changed, 56 insertions(+), 14 deletions(-) diff --git a/app/Config/clockwork.php b/app/Config/clockwork.php index 394af8451..bd59eaf71 100644 --- a/app/Config/clockwork.php +++ b/app/Config/clockwork.php @@ -173,6 +173,8 @@ return [ // List of URIs that should not be collected 'except' => [ + '/uploads/images/.*', // BookStack image requests + '/horizon/.*', // Laravel Horizon requests '/telescope/.*', // Laravel Telescope requests '/_debugbar/.*', // Laravel DebugBar requests diff --git a/app/Entities/Controllers/BookApiController.php b/app/Entities/Controllers/BookApiController.php index 955bd707b..15e67a0f7 100644 --- a/app/Entities/Controllers/BookApiController.php +++ b/app/Entities/Controllers/BookApiController.php @@ -26,7 +26,9 @@ class BookApiController extends ApiController */ public function list() { - $books = $this->queries->visibleForList(); + $books = $this->queries + ->visibleForList() + ->addSelect(['created_by', 'updated_by']); return $this->apiListingResponse($books, [ 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', diff --git a/app/Entities/Controllers/BookshelfApiController.php b/app/Entities/Controllers/BookshelfApiController.php index 9170226a5..a665bcb6b 100644 --- a/app/Entities/Controllers/BookshelfApiController.php +++ b/app/Entities/Controllers/BookshelfApiController.php @@ -24,7 +24,9 @@ class BookshelfApiController extends ApiController */ public function list() { - $shelves = $this->queries->visibleForList(); + $shelves = $this->queries + ->visibleForList() + ->addSelect(['created_by', 'updated_by']); return $this->apiListingResponse($shelves, [ 'id', 'name', 'slug', 'description', 'created_at', 'updated_at', 'created_by', 'updated_by', 'owned_by', diff --git a/app/Entities/Controllers/ChapterApiController.php b/app/Entities/Controllers/ChapterApiController.php index fb484b85d..430654330 100644 --- a/app/Entities/Controllers/ChapterApiController.php +++ b/app/Entities/Controllers/ChapterApiController.php @@ -3,8 +3,8 @@ namespace BookStack\Entities\Controllers; use BookStack\Entities\Models\Chapter; -use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Queries\ChapterQueries; +use BookStack\Entities\Queries\EntityQueries; use BookStack\Entities\Repos\ChapterRepo; use BookStack\Exceptions\PermissionsException; use BookStack\Http\ApiController; @@ -38,7 +38,7 @@ class ChapterApiController extends ApiController public function __construct( protected ChapterRepo $chapterRepo, protected ChapterQueries $queries, - protected BookQueries $bookQueries, + protected EntityQueries $entityQueries, ) { } @@ -47,7 +47,8 @@ class ChapterApiController extends ApiController */ public function list() { - $chapters = $this->queries->visibleForList(); + $chapters = $this->queries->visibleForList() + ->addSelect(['created_by', 'updated_by']); return $this->apiListingResponse($chapters, [ 'id', 'book_id', 'name', 'slug', 'description', 'priority', @@ -63,7 +64,7 @@ class ChapterApiController extends ApiController $requestData = $this->validate($request, $this->rules['create']); $bookId = $request->get('book_id'); - $book = $this->bookQueries->findVisibleByIdOrFail(intval($bookId)); + $book = $this->entityQueries->books->findVisibleByIdOrFail(intval($bookId)); $this->checkOwnablePermission('chapter-create', $book); $chapter = $this->chapterRepo->create($requestData, $book); @@ -79,12 +80,14 @@ class ChapterApiController extends ApiController $chapter = $this->queries->findVisibleByIdOrFail(intval($id)); $chapter = $this->forJsonDisplay($chapter); - $chapter->load([ - 'createdBy', 'updatedBy', 'ownedBy', - 'pages' => function (HasMany $query) { - $query->scopes('visible')->get(['id', 'name', 'slug']); - } - ]); + $chapter->load(['createdBy', 'updatedBy', 'ownedBy']); + + // Note: More fields than usual here, for backwards compatibility, + // 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); } diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index 2e36a84b9..4274589e2 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -79,7 +79,8 @@ class ChapterController extends Controller $this->checkOwnablePermission('chapter-view', $chapter); $sidebarTree = (new BookContents($chapter->book))->getTree(); - $pages = $chapter->getVisiblePages(); + $pages = $this->entityQueries->pages->visibleForChapterList($chapter->id)->get(); + $nextPreviousLocator = new NextPreviousContentLocator($chapter, $sidebarTree); View::incrementFor($chapter); diff --git a/app/Entities/Controllers/PageApiController.php b/app/Entities/Controllers/PageApiController.php index d2a5a3ee3..40598e209 100644 --- a/app/Entities/Controllers/PageApiController.php +++ b/app/Entities/Controllers/PageApiController.php @@ -45,7 +45,8 @@ class PageApiController extends ApiController */ public function list() { - $pages = $this->queries->visibleForList(); + $pages = $this->queries->visibleForList() + ->addSelect(['created_by', 'updated_by', 'revision_count', 'editor']); return $this->apiListingResponse($pages, [ 'id', 'book_id', 'chapter_id', 'name', 'slug', 'priority', diff --git a/app/Entities/Queries/PageQueries.php b/app/Entities/Queries/PageQueries.php index a5938f754..06298f470 100644 --- a/app/Entities/Queries/PageQueries.php +++ b/app/Entities/Queries/PageQueries.php @@ -73,6 +73,14 @@ class PageQueries implements ProvidesEntityQueries ->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 { return $this->start() diff --git a/tests/Api/BooksApiTest.php b/tests/Api/BooksApiTest.php index b31bd7d37..b8c2b6133 100644 --- a/tests/Api/BooksApiTest.php +++ b/tests/Api/BooksApiTest.php @@ -24,6 +24,9 @@ class BooksApiTest extends TestCase 'id' => $firstBook->id, 'name' => $firstBook->name, 'slug' => $firstBook->slug, + 'owned_by' => $firstBook->owned_by, + 'created_by' => $firstBook->created_by, + 'updated_by' => $firstBook->updated_by, ], ]]); } diff --git a/tests/Api/ChaptersApiTest.php b/tests/Api/ChaptersApiTest.php index e2d6cfc81..9698d4dd9 100644 --- a/tests/Api/ChaptersApiTest.php +++ b/tests/Api/ChaptersApiTest.php @@ -28,6 +28,9 @@ class ChaptersApiTest extends TestCase 'book_id' => $firstChapter->book->id, 'priority' => $firstChapter->priority, '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, 'slug' => $page->slug, '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, diff --git a/tests/Api/PagesApiTest.php b/tests/Api/PagesApiTest.php index 0d084472d..22659d5bb 100644 --- a/tests/Api/PagesApiTest.php +++ b/tests/Api/PagesApiTest.php @@ -27,6 +27,10 @@ class PagesApiTest extends TestCase 'slug' => $firstPage->slug, 'book_id' => $firstPage->book->id, 'priority' => $firstPage->priority, + 'owned_by' => $firstPage->owned_by, + 'created_by' => $firstPage->created_by, + 'updated_by' => $firstPage->updated_by, + 'revision_count' => $firstPage->revision_count, ], ]]); } diff --git a/tests/Api/ShelvesApiTest.php b/tests/Api/ShelvesApiTest.php index f1b8ed985..be276e110 100644 --- a/tests/Api/ShelvesApiTest.php +++ b/tests/Api/ShelvesApiTest.php @@ -25,6 +25,9 @@ class ShelvesApiTest extends TestCase 'id' => $firstBookshelf->id, 'name' => $firstBookshelf->name, 'slug' => $firstBookshelf->slug, + 'owned_by' => $firstBookshelf->owned_by, + 'created_by' => $firstBookshelf->created_by, + 'updated_by' => $firstBookshelf->updated_by, ], ]]); }