Simplified and aligned handling of mixed entity endpoints

Fixes #4444

diff --git a/app/Activity/Controllers/FavouriteController.php b/app/Activity/Controllers/FavouriteController.php
index 1b88ffd643a463095ba0adb02a67647190ba6065..d2534ddfe090b9370f0b4f7b9a51eae512bb43f3 100644 (file)
--- a/app/Activity/Controllers/FavouriteController.php
+++ b/app/Activity/Controllers/FavouriteController.php
@@ -6,11 +6,17 @@ use BookStack\Activity\Models\Favouritable;
 use BookStack\App\Model;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Queries\TopFavourites;
 use BookStack\App\Model;
 use BookStack\Entities\Models\Entity;
 use BookStack\Entities\Queries\TopFavourites;

+use BookStack\Entities\Tools\MixedEntityRequestHelper;

 use BookStack\Http\Controller;
 use Illuminate\Http\Request;
 
 class FavouriteController extends Controller
 {
 use BookStack\Http\Controller;
 use Illuminate\Http\Request;
 
 class FavouriteController extends Controller
 {

+    public function __construct(
+        protected MixedEntityRequestHelper $entityHelper,
+    ) {
+    }
+

     /**
      * Show a listing of all favourite items for the current user.
      */
     /**
      * Show a listing of all favourite items for the current user.
      */


@@ -36,13 +42,14 @@ class FavouriteController extends Controller
      */
     public function add(Request $request)
     {
      */
     public function add(Request $request)
     {

-        $favouritable = $this->getValidatedModelFromRequest($request);
-        $favouritable->favourites()->firstOrCreate([
+        $modelInfo = $this->validate($request, $this->entityHelper->validationRules());
+        $entity = $this->entityHelper->getVisibleEntityFromRequestData($modelInfo);
+        $entity->favourites()->firstOrCreate([

             'user_id' => user()->id,
         ]);
 
         $this->showSuccessNotification(trans('activities.favourite_add_notification', [
             'user_id' => user()->id,
         ]);
 
         $this->showSuccessNotification(trans('activities.favourite_add_notification', [

-            'name' => $favouritable->name,
+            'name' => $entity->name,

         ]));
 
         return redirect()->back();
         ]));
 
         return redirect()->back();


@@ -53,48 +60,16 @@ class FavouriteController extends Controller
      */
     public function remove(Request $request)
     {
      */
     public function remove(Request $request)
     {

-        $favouritable = $this->getValidatedModelFromRequest($request);
-        $favouritable->favourites()->where([
+        $modelInfo = $this->validate($request, $this->entityHelper->validationRules());
+        $entity = $this->entityHelper->getVisibleEntityFromRequestData($modelInfo);
+        $entity->favourites()->where([

             'user_id' => user()->id,
         ])->delete();
 
         $this->showSuccessNotification(trans('activities.favourite_remove_notification', [
             'user_id' => user()->id,
         ])->delete();
 
         $this->showSuccessNotification(trans('activities.favourite_remove_notification', [

-            'name' => $favouritable->name,
+            'name' => $entity->name,

         ]));
 
         return redirect()->back();
     }
         ]));
 
         return redirect()->back();
     }

-
-    /**
-     * @throws \Illuminate\Validation\ValidationException
-     * @throws \Exception
-     */
-    protected function getValidatedModelFromRequest(Request $request): Entity
-    {
-        $modelInfo = $this->validate($request, [
-            'type' => ['required', 'string'],
-            'id'   => ['required', 'integer'],
-        ]);
-
-        if (!class_exists($modelInfo['type'])) {
-            throw new \Exception('Model not found');
-        }
-
-        /** @var Model $model */
-        $model = new $modelInfo['type']();
-        if (!$model instanceof Favouritable) {
-            throw new \Exception('Model not favouritable');
-        }
-
-        $modelInstance = $model->newQuery()
-            ->where('id', '=', $modelInfo['id'])
-            ->first(['id', 'name', 'owned_by']);
-
-        $inaccessibleEntity = ($modelInstance instanceof Entity && !userCan('view', $modelInstance));
-        if (is_null($modelInstance) || $inaccessibleEntity) {
-            throw new \Exception('Model instance not found');
-        }
-
-        return $modelInstance;
-    }

 }
 }

diff --git a/app/Activity/Controllers/WatchController.php b/app/Activity/Controllers/WatchController.php
index 3d7e18116bec0b404bd8dd804480c23ab150376d..d63918fb35044d01c807d5641126d0e465434d37 100644 (file)
--- a/app/Activity/Controllers/WatchController.php
+++ b/app/Activity/Controllers/WatchController.php
@@ -3,25 +3,23 @@
 namespace BookStack\Activity\Controllers;
 
 use BookStack\Activity\Tools\UserEntityWatchOptions;
 namespace BookStack\Activity\Controllers;
 
 use BookStack\Activity\Tools\UserEntityWatchOptions;

-use BookStack\App\Model;
-use BookStack\Entities\Models\Entity;
+use BookStack\Entities\Tools\MixedEntityRequestHelper;

 use BookStack\Http\Controller;
 use BookStack\Http\Controller;

-use Exception;

 use Illuminate\Http\Request;
 use Illuminate\Http\Request;

-use Illuminate\Validation\ValidationException;

 
 class WatchController extends Controller
 {
 
 class WatchController extends Controller
 {

-    public function update(Request $request)
+    public function update(Request $request, MixedEntityRequestHelper $entityHelper)

     {
         $this->checkPermission('receive-notifications');
         $this->preventGuestAccess();
 
         $requestData = $this->validate($request, [
             'level' => ['required', 'string'],
     {
         $this->checkPermission('receive-notifications');
         $this->preventGuestAccess();
 
         $requestData = $this->validate($request, [
             'level' => ['required', 'string'],

+            ...$entityHelper->validationRules()

         ]);
 
         ]);
 

-        $watchable = $this->getValidatedModelFromRequest($request);
+        $watchable = $entityHelper->getVisibleEntityFromRequestData($requestData);

         $watchOptions = new UserEntityWatchOptions(user(), $watchable);
         $watchOptions->updateLevelByName($requestData['level']);
 
         $watchOptions = new UserEntityWatchOptions(user(), $watchable);
         $watchOptions->updateLevelByName($requestData['level']);
 


@@ -29,37 +27,4 @@ class WatchController extends Controller
 
         return redirect()->back();
     }
 
         return redirect()->back();
     }

-
-    /**
-     * @throws ValidationException
-     * @throws Exception
-     */
-    protected function getValidatedModelFromRequest(Request $request): Entity
-    {
-        $modelInfo = $this->validate($request, [
-            'type' => ['required', 'string'],
-            'id'   => ['required', 'integer'],
-        ]);
-
-        if (!class_exists($modelInfo['type'])) {
-            throw new Exception('Model not found');
-        }
-
-        /** @var Model $model */
-        $model = new $modelInfo['type']();
-        if (!$model instanceof Entity) {
-            throw new Exception('Model not an entity');
-        }
-
-        $modelInstance = $model->newQuery()
-            ->where('id', '=', $modelInfo['id'])
-            ->first(['id', 'name', 'owned_by']);
-
-        $inaccessibleEntity = ($modelInstance instanceof Entity && !userCan('view', $modelInstance));
-        if (is_null($modelInstance) || $inaccessibleEntity) {
-            throw new Exception('Model instance not found');
-        }
-
-        return $modelInstance;
-    }

 }
 }

diff --git a/app/Entities/Tools/MixedEntityRequestHelper.php b/app/Entities/Tools/MixedEntityRequestHelper.php
new file mode 100644 (file)
index 0000000..8319f6a
--- /dev/null
+++ b/app/Entities/Tools/MixedEntityRequestHelper.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace BookStack\Entities\Tools;
+
+use BookStack\Entities\EntityProvider;
+use BookStack\Entities\Models\Entity;
+
+class MixedEntityRequestHelper
+{
+    public function __construct(
+        protected EntityProvider $entities,
+    ) {
+    }
+
+    /**
+     * Query out an entity, visible to the current user, for the given
+     * entity request details (this provided in a request validated by
+     * this classes' validationRules method).
+     * @param array{type: string, id: string} $requestData
+     */
+    public function getVisibleEntityFromRequestData(array $requestData): Entity
+    {
+        $entityType = $this->entities->get($requestData['type']);
+
+        return $entityType->newQuery()->scopes(['visible'])->findOrFail($requestData['id']);
+    }
+
+    /**
+     * Get the validation rules for an abstract entity request.
+     * @return array{type: string[], id: string[]}
+     */
+    public function validationRules(): array
+    {
+        return [
+                'type' => ['required', 'string'],
+                'id'   => ['required', 'integer'],
+        ];
+    }
+}

diff --git a/resources/views/entities/favourite-action.blade.php b/resources/views/entities/favourite-action.blade.php
index 35189044bda5a28497c3dd0a622c4b1475e41baa..e596fbdce4329d4fda466d386fa36a1699c8ef1a 100644 (file)
--- a/resources/views/entities/favourite-action.blade.php
+++ b/resources/views/entities/favourite-action.blade.php
@@ -3,7 +3,7 @@
 @endphp
 <form action="{{ url('/favourites/' . ($isFavourite ? 'remove' : 'add')) }}" method="POST">
     {{ csrf_field() }}
 @endphp
 <form action="{{ url('/favourites/' . ($isFavourite ? 'remove' : 'add')) }}" method="POST">
     {{ csrf_field() }}

-    <input type="hidden" name="type" value="{{ get_class($entity) }}">
+    <input type="hidden" name="type" value="{{ $entity->getMorphClass() }}">

     <input type="hidden" name="id" value="{{ $entity->id }}">
     <button type="submit" data-shortcut="favourite" class="icon-list-item text-link">
         <span>@icon($isFavourite ? 'star' : 'star-outline')</span>
     <input type="hidden" name="id" value="{{ $entity->id }}">
     <button type="submit" data-shortcut="favourite" class="icon-list-item text-link">
         <span>@icon($isFavourite ? 'star' : 'star-outline')</span>

diff --git a/resources/views/entities/watch-action.blade.php b/resources/views/entities/watch-action.blade.php
index 34e287804cc62b0174058bd3c5a54b3dbe6b0a18..c87bc2b2b0c6e06a83ccd0518cc065c5f6527915 100644 (file)
--- a/resources/views/entities/watch-action.blade.php
+++ b/resources/views/entities/watch-action.blade.php
@@ -1,7 +1,7 @@
 <form action="{{ url('/watching/update') }}" method="POST">
     {{ csrf_field() }}
     {{ method_field('PUT') }}
 <form action="{{ url('/watching/update') }}" method="POST">
     {{ csrf_field() }}
     {{ method_field('PUT') }}

-    <input type="hidden" name="type" value="{{ get_class($entity) }}">
+    <input type="hidden" name="type" value="{{ $entity->getMorphClass() }}">

     <input type="hidden" name="id" value="{{ $entity->id }}">
     <button type="submit"
             name="level"
     <input type="hidden" name="id" value="{{ $entity->id }}">
     <button type="submit"
             name="level"

diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php
index d24e120182166aec6dc0d895ae67ade32476cf8f..9389a6c5a6df44ee6bc7b1201d0e51265572915f 100644 (file)
--- a/resources/views/entities/watch-controls.blade.php
+++ b/resources/views/entities/watch-controls.blade.php
@@ -7,7 +7,7 @@
     <form action="{{ url('/watching/update') }}" method="POST">
         {{ method_field('PUT') }}
         {{ csrf_field() }}
     <form action="{{ url('/watching/update') }}" method="POST">
         {{ method_field('PUT') }}
         {{ csrf_field() }}

-        <input type="hidden" name="type" value="{{ get_class($entity) }}">
+        <input type="hidden" name="type" value="{{ $entity->getMorphClass() }}">

         <input type="hidden" name="id" value="{{ $entity->id }}">
 
         <ul refs="dropdown@menu" class="dropdown-menu xl-limited anchor-left pb-none">
         <input type="hidden" name="id" value="{{ $entity->id }}">
 
         <ul refs="dropdown@menu" class="dropdown-menu xl-limited anchor-left pb-none">

diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php
index fa50d8c79f1d5e0d584784210460571fcaf28348..5db0067cc565b9789f6a9ce46db42832e46d15a9 100644 (file)
--- a/tests/Activity/WatchTest.php
+++ b/tests/Activity/WatchTest.php
@@ -66,7 +66,7 @@ class WatchTest extends TestCase
 
         $this->actingAs($editor)->get($book->getUrl());
         $resp = $this->put('/watching/update', [
 
         $this->actingAs($editor)->get($book->getUrl());
         $resp = $this->put('/watching/update', [

-            'type' => get_class($book),
+            'type' => $book->getMorphClass(),

             'id' => $book->id,
             'level' => 'comments'
         ]);
             'id' => $book->id,
             'level' => 'comments'
         ]);


@@ -81,7 +81,7 @@ class WatchTest extends TestCase
         ]);
 
         $resp = $this->put('/watching/update', [
         ]);
 
         $resp = $this->put('/watching/update', [

-            'type' => get_class($book),
+            'type' => $book->getMorphClass(),

             'id' => $book->id,
             'level' => 'default'
         ]);
             'id' => $book->id,
             'level' => 'default'
         ]);


@@ -101,7 +101,7 @@ class WatchTest extends TestCase
         $book = $this->entities->book();
 
         $resp = $this->put('/watching/update', [
         $book = $this->entities->book();
 
         $resp = $this->put('/watching/update', [

-            'type' => get_class($book),
+            'type' => $book->getMorphClass(),

             'id' => $book->id,
             'level' => 'comments'
         ]);
             'id' => $book->id,
             'level' => 'comments'
         ]);

diff --git a/tests/FavouriteTest.php b/tests/FavouriteTest.php
index 0e30cbd58cadc608e5150cc47207dbffb606f8ee..48048e2845d64a4fbe503e394d69502186239918 100644 (file)
--- a/tests/FavouriteTest.php
+++ b/tests/FavouriteTest.php
@@ -14,10 +14,10 @@ class FavouriteTest extends TestCase
 
         $resp = $this->actingAs($editor)->get($page->getUrl());
         $this->withHtml($resp)->assertElementContains('button', 'Favourite');
 
         $resp = $this->actingAs($editor)->get($page->getUrl());
         $this->withHtml($resp)->assertElementContains('button', 'Favourite');

-        $this->withHtml($resp)->assertElementExists('form[method="POST"][action$="/favourites/add"]');
+        $this->withHtml($resp)->assertElementExists('form[method="POST"][action$="/favourites/add"] input[name="type"][value="page"]');

 
         $resp = $this->post('/favourites/add', [
 
         $resp = $this->post('/favourites/add', [

-            'type' => get_class($page),
+            'type' => $page->getMorphClass(),

             'id'   => $page->id,
         ]);
         $resp->assertRedirect($page->getUrl());
             'id'   => $page->id,
         ]);
         $resp->assertRedirect($page->getUrl());


@@ -45,7 +45,7 @@ class FavouriteTest extends TestCase
         $this->withHtml($resp)->assertElementExists('form[method="POST"][action$="/favourites/remove"]');
 
         $resp = $this->post('/favourites/remove', [
         $this->withHtml($resp)->assertElementExists('form[method="POST"][action$="/favourites/remove"]');
 
         $resp = $this->post('/favourites/remove', [

-            'type' => get_class($page),
+            'type' => $page->getMorphClass(),

             'id'   => $page->id,
         ]);
         $resp->assertRedirect($page->getUrl());
             'id'   => $page->id,
         ]);
         $resp->assertRedirect($page->getUrl());


@@ -67,7 +67,7 @@ class FavouriteTest extends TestCase
 
         $this->actingAs($user)->get($book->getUrl());
         $resp = $this->post('/favourites/add', [
 
         $this->actingAs($user)->get($book->getUrl());
         $resp = $this->post('/favourites/add', [

-            'type' => get_class($book),
+            'type' => $book->getMorphClass(),

             'id'   => $book->id,
         ]);
         $resp->assertRedirect($book->getUrl());
             'id'   => $book->id,
         ]);
         $resp->assertRedirect($book->getUrl());