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.

diff --git a/app/Config/clockwork.php b/app/Config/clockwork.php
index 394af845172af8d5356f14b97a8c75d0cb856a1b..bd59eaf71d858583f61d0346aeb4e5e657211725 100644 (file)
--- 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 955bd707bc6f5ceeb84440a71742b24fcf6dc549..15e67a0f7231256869f05248e090b33973e1cb74 100644 (file)
--- 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 9170226a5e80b8828330d066bf9b7562784a3d44..a665bcb6bab7d314c35939f1c7e6aa3e4e902c28 100644 (file)
--- 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 fb484b85d72db13ae35f0eaca97de7388b026b35..430654330f36b97d5ec16dbaf22c12a56b337738 100644 (file)
--- 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 2e36a84b94da39363ef72e803ad0a4f23a35e920..4274589e26055c5d1e378156f7d7b0ccd76f87bc 100644 (file)
--- 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 d2a5a3ee3514416d2d9df9611a9f1d901134a1be..40598e2098375afc564f35a371eb22048ef50925 100644 (file)
--- 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 a5938f754ef8a418ba63e04134522a9e07dc0b9c..06298f470b902f8abfef4257c108a0fa42f1ecb5 100644 (file)
--- 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 b31bd7d37ee06e66de9940a1a710bc5ab2eab7ec..b8c2b613387ef79a156ecc783e5e9dd067083bca 100644 (file)
--- 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 e2d6cfc81f0a8ce5e12172c7f4c793787feff2e4..9698d4dd9c254dea3ba5d3e1a6390bd61033b84d 100644 (file)
--- 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 0d084472d1a43e0caf78136a78b07801b8c816ed..22659d5bb726bf011403ce14cc728d810bde861a 100644 (file)
--- 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 f1b8ed98553cce61f099273f52dd7c1784d9511d..be276e110040cd156c68e7d996ee26b458a1373c 100644 (file)
--- 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,
             ],
         ]]);
     }