From 2fbf5527c70d5d3eadb2767ca5301ad05f7f28c8 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 10 Sep 2023 15:18:31 +0100
Subject: [PATCH] Simplified and aligned handling of mixed entity endpoints
Fixes #4444
---
.../Controllers/FavouriteController.php | 53 +++++--------------
app/Activity/Controllers/WatchController.php | 43 ++-------------
.../Tools/MixedEntityRequestHelper.php | 39 ++++++++++++++
.../views/entities/favourite-action.blade.php | 2 +-
.../views/entities/watch-action.blade.php | 2 +-
.../views/entities/watch-controls.blade.php | 2 +-
tests/Activity/WatchTest.php | 6 +--
tests/FavouriteTest.php | 8 +--
8 files changed, 67 insertions(+), 88 deletions(-)
create mode 100644 app/Entities/Tools/MixedEntityRequestHelper.php
diff --git a/app/Activity/Controllers/FavouriteController.php b/app/Activity/Controllers/FavouriteController.php
index 1b88ffd64..d2534ddfe 100644
--- 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\Entities\Tools\MixedEntityRequestHelper;
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.
*/
@@ -36,13 +42,14 @@ class FavouriteController extends Controller
*/
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', [
- 'name' => $favouritable->name,
+ 'name' => $entity->name,
]));
return redirect()->back();
@@ -53,48 +60,16 @@ class FavouriteController extends Controller
*/
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', [
- 'name' => $favouritable->name,
+ 'name' => $entity->name,
]));
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 3d7e18116..d63918fb3 100644
--- 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;
-use BookStack\App\Model;
-use BookStack\Entities\Models\Entity;
+use BookStack\Entities\Tools\MixedEntityRequestHelper;
use BookStack\Http\Controller;
-use Exception;
use Illuminate\Http\Request;
-use Illuminate\Validation\ValidationException;
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'],
+ ...$entityHelper->validationRules()
]);
- $watchable = $this->getValidatedModelFromRequest($request);
+ $watchable = $entityHelper->getVisibleEntityFromRequestData($requestData);
$watchOptions = new UserEntityWatchOptions(user(), $watchable);
$watchOptions->updateLevelByName($requestData['level']);
@@ -29,37 +27,4 @@ class WatchController extends Controller
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
index 000000000..8319f6aa0
--- /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 35189044b..e596fbdce 100644
--- 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() }}
- <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>
diff --git a/resources/views/entities/watch-action.blade.php b/resources/views/entities/watch-action.blade.php
index 34e287804..c87bc2b2b 100644
--- 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') }}
- <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"
diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php
index d24e12018..9389a6c5a 100644
--- 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() }}
- <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">
diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php
index fa50d8c79..5db0067cc 100644
--- 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', [
- 'type' => get_class($book),
+ 'type' => $book->getMorphClass(),
'id' => $book->id,
'level' => 'comments'
]);
@@ -81,7 +81,7 @@ class WatchTest extends TestCase
]);
$resp = $this->put('/watching/update', [
- 'type' => get_class($book),
+ 'type' => $book->getMorphClass(),
'id' => $book->id,
'level' => 'default'
]);
@@ -101,7 +101,7 @@ class WatchTest extends TestCase
$book = $this->entities->book();
$resp = $this->put('/watching/update', [
- 'type' => get_class($book),
+ 'type' => $book->getMorphClass(),
'id' => $book->id,
'level' => 'comments'
]);
diff --git a/tests/FavouriteTest.php b/tests/FavouriteTest.php
index 0e30cbd58..48048e284 100644
--- 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');
- $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', [
- 'type' => get_class($page),
+ 'type' => $page->getMorphClass(),
'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', [
- 'type' => get_class($page),
+ 'type' => $page->getMorphClass(),
'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', [
- 'type' => get_class($book),
+ 'type' => $book->getMorphClass(),
'id' => $book->id,
]);
$resp->assertRedirect($book->getUrl());