Chapters API: Allowed move via book_id property

Aligns it with pages and with the book_id property already being part of
the API.
For #4272.

diff --git a/app/Entities/Controllers/ChapterApiController.php b/app/Entities/Controllers/ChapterApiController.php
index ce20e6b96c38fe1c89e16d81ca934fb0b8cb8d31..403c58de3c5a1a346c3e6f778a0b336c0ba1b23d 100644 (file)
--- a/app/Entities/Controllers/ChapterApiController.php
+++ b/app/Entities/Controllers/ChapterApiController.php
@@ -5,14 +5,14 @@ namespace BookStack\Entities\Controllers;
 use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Chapter;
 use BookStack\Entities\Repos\ChapterRepo;
+use BookStack\Exceptions\PermissionsException;
 use BookStack\Http\ApiController;
+use Exception;
 use Illuminate\Database\Eloquent\Relations\HasMany;
 use Illuminate\Http\Request;
 
 class ChapterApiController extends ApiController
 {
-    protected $chapterRepo;
-
     protected $rules = [
         'create' => [
             'book_id'     => ['required', 'integer'],
@@ -28,12 +28,9 @@ class ChapterApiController extends ApiController
         ],
     ];
 
-    /**
-     * ChapterController constructor.
-     */
-    public function __construct(ChapterRepo $chapterRepo)
-    {
-        $this->chapterRepo = $chapterRepo;
+    public function __construct(
+        protected ChapterRepo $chapterRepo
+    ) {
     }
 
     /**
@@ -54,13 +51,13 @@ class ChapterApiController extends ApiController
      */
     public function create(Request $request)
     {
-        $this->validate($request, $this->rules['create']);
+        $requestData = $this->validate($request, $this->rules['create']);
 
         $bookId = $request->get('book_id');
         $book = Book::visible()->findOrFail($bookId);
         $this->checkOwnablePermission('chapter-create', $book);
 
-        $chapter = $this->chapterRepo->create($request->all(), $book);
+        $chapter = $this->chapterRepo->create($requestData, $book);
 
         return response()->json($chapter->load(['tags']));
     }
@@ -79,13 +76,30 @@ class ChapterApiController extends ApiController
 
     /**
      * Update the details of a single chapter.
+     * Providing a 'book_id' property will essentially move the chapter
+     * into that parent element if you have permissions to do so.
      */
     public function update(Request $request, string $id)
     {
+        $requestData = $this->validate($request, $this->rules()['update']);
         $chapter = Chapter::visible()->findOrFail($id);
         $this->checkOwnablePermission('chapter-update', $chapter);
 
-        $updatedChapter = $this->chapterRepo->update($chapter, $request->all());
+        if ($request->has('book_id') && $chapter->book_id !== intval($requestData['book_id'])) {
+            $this->checkOwnablePermission('chapter-delete', $chapter);
+
+            try {
+                $this->chapterRepo->move($chapter, "book:{$requestData['book_id']}");
+            } catch (Exception $exception) {
+                if ($exception instanceof  PermissionsException) {
+                    $this->showPermissionError();
+                }
+
+                return $this->jsonError(trans('errors.selected_book_not_found'));
+            }
+        }
+
+        $updatedChapter = $this->chapterRepo->update($chapter, $requestData);
 
         return response()->json($updatedChapter->load(['tags']));
     }

diff --git a/tests/Api/ChaptersApiTest.php b/tests/Api/ChaptersApiTest.php
index a48e3b02626f5a80a47031cb71acbf942f707696..713d8bba4bab6156b1c5b326ec5df2cc7efe26b6 100644 (file)
--- a/tests/Api/ChaptersApiTest.php
+++ b/tests/Api/ChaptersApiTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Api;
 
+use BookStack\Entities\Models\Book;
 use BookStack\Entities\Models\Chapter;
 use Carbon\Carbon;
 use Illuminate\Support\Facades\DB;
@@ -163,6 +164,33 @@ class ChaptersApiTest extends TestCase
         $this->assertGreaterThan(Carbon::now()->subDay()->unix(), $chapter->updated_at->unix());
     }
 
+    public function test_update_with_book_id_moves_chapter()
+    {
+        $this->actingAsApiEditor();
+        $chapter = $this->entities->chapterHasPages();
+        $page = $chapter->pages()->first();
+        $newBook = Book::query()->where('id', '!=', $chapter->book_id)->first();
+
+        $resp = $this->putJson($this->baseEndpoint . "/{$chapter->id}", ['book_id' => $newBook->id]);
+        $resp->assertOk();
+        $chapter->refresh();
+
+        $this->assertDatabaseHas('chapters', ['id' => $chapter->id, 'book_id' => $newBook->id]);
+        $this->assertDatabaseHas('pages', ['id' => $page->id, 'book_id' => $newBook->id, 'chapter_id' => $chapter->id]);
+    }
+
+    public function test_update_with_new_book_id_requires_delete_permission()
+    {
+        $editor = $this->users->editor();
+        $this->permissions->removeUserRolePermissions($editor, ['chapter-delete-all', 'chapter-delete-own']);
+        $this->actingAs($editor);
+        $chapter = $this->entities->chapterHasPages();
+        $newBook = Book::query()->where('id', '!=', $chapter->book_id)->first();
+
+        $resp = $this->putJson($this->baseEndpoint . "/{$chapter->id}", ['book_id' => $newBook->id]);
+        $this->assertPermissionError($resp);
+    }
+
     public function test_delete_endpoint()
     {
         $this->actingAsApiEditor();

diff --git a/tests/TestCase.php b/tests/TestCase.php
index abee1d3b3429756cd4173783eab2af35e6dabf5f..322ab037032f76ea9adb7b3002f7c2f845f1309d 100644 (file)
--- a/tests/TestCase.php
+++ b/tests/TestCase.php
@@ -213,18 +213,14 @@ abstract class TestCase extends BaseTestCase
      */
     private function isPermissionError($response): bool
     {
+        if ($response->status() === 403 && $response instanceof JsonResponse) {
+            $errMessage = $response->getData(true)['error']['message'] ?? '';
+            return $errMessage === 'You do not have permission to perform the requested action.';
+        }
+
         return $response->status() === 302
-            && (
-                (
-                    $response->headers->get('Location') === url('/')
-                    && strpos(session()->pull('error', ''), 'You do not have permission to access') === 0
-                )
-                ||
-                (
-                    $response instanceof JsonResponse &&
-                    $response->json(['error' => 'You do not have permission to perform the requested action.'])
-                )
-            );
+            && $response->headers->get('Location') === url('/')
+            && str_starts_with(session()->pull('error', ''), 'You do not have permission to access');
     }
 
     /**