From 8b550991a4dbcd7a05e08e8f600b5444d5f4004d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 20 Sep 2019 00:18:28 +0100 Subject: [PATCH] Refactored some core entity actions - Created BookChild class to share some page/chapter logic. - Gave entities the power to generate their own permissions and slugs. - Moved bits out of BaseController constructor since it was overly sticky. - Moved slug generation logic into its own class. - Created a facade for permissions due to high use. - Fixed failing test issues from last commits --- app/Actions/ActivityService.php | 1 + app/Auth/Permissions/PermissionService.php | 1 - app/Config/app.php | 1 + app/Entities/BookChild.php | 21 ++++ app/Entities/Bookshelf.php | 2 +- app/Entities/Chapter.php | 13 +-- app/Entities/Entity.php | 36 +++++- app/Entities/Page.php | 11 +- app/Entities/Repos/EntityRepo.php | 112 +++++-------------- app/Entities/Repos/PageRepo.php | 33 +++--- app/Entities/SlugGenerator.php | 64 +++++++++++ app/Facades/Permissions.php | 16 +++ app/Http/Controllers/BookController.php | 17 +-- app/Http/Controllers/BookshelfController.php | 8 +- app/Http/Controllers/ChapterController.php | 4 +- app/Http/Controllers/Controller.php | 26 ++--- app/Http/Controllers/HomeController.php | 10 +- app/Http/Controllers/PageController.php | 10 +- app/Http/Controllers/UserController.php | 2 +- app/Providers/CustomFacadeProvider.php | 13 ++- phpunit.xml | 2 +- routes/web.php | 2 +- tests/Entity/PageDraftTest.php | 15 +-- tests/Entity/TagTest.php | 7 +- tests/SharedTestHelpers.php | 2 +- 25 files changed, 242 insertions(+), 187 deletions(-) create mode 100644 app/Entities/BookChild.php create mode 100644 app/Entities/SlugGenerator.php create mode 100644 app/Facades/Permissions.php diff --git a/app/Actions/ActivityService.php b/app/Actions/ActivityService.php index 8511e4d6d..d092a35c2 100644 --- a/app/Actions/ActivityService.php +++ b/app/Actions/ActivityService.php @@ -73,6 +73,7 @@ class ActivityService */ public function removeEntity(Entity $entity) { + // TODO - Rewrite to db query. $activities = $entity->activity; foreach ($activities as $activity) { $activity->extra = $entity->name; diff --git a/app/Auth/Permissions/PermissionService.php b/app/Auth/Permissions/PermissionService.php index a5ab4ea9a..b4ea17cec 100644 --- a/app/Auth/Permissions/PermissionService.php +++ b/app/Auth/Permissions/PermissionService.php @@ -215,7 +215,6 @@ class PermissionService * @param Collection $books * @param array $roles * @param bool $deleteOld - * @throws \Throwable */ protected function buildJointPermissionsForBooks($books, $roles, $deleteOld = false) { diff --git a/app/Config/app.php b/app/Config/app.php index b4a1076a9..50e3bdb6a 100755 --- a/app/Config/app.php +++ b/app/Config/app.php @@ -182,6 +182,7 @@ return [ 'Setting' => BookStack\Facades\Setting::class, 'Views' => BookStack\Facades\Views::class, 'Images' => BookStack\Facades\Images::class, + 'Permissions' => BookStack\Facades\Permissions::class, ], diff --git a/app/Entities/BookChild.php b/app/Entities/BookChild.php new file mode 100644 index 000000000..c76baf29a --- /dev/null +++ b/app/Entities/BookChild.php @@ -0,0 +1,21 @@ +belongsTo(Book::class); + } + +} \ No newline at end of file diff --git a/app/Entities/Bookshelf.php b/app/Entities/Bookshelf.php index 1db348b6b..7ad2415ed 100644 --- a/app/Entities/Bookshelf.php +++ b/app/Entities/Bookshelf.php @@ -111,7 +111,7 @@ class Bookshelf extends Entity */ public function appendBook(Book $book) { - if (!$this->contains($book)) { + if ($this->contains($book)) { return; } diff --git a/app/Entities/Chapter.php b/app/Entities/Chapter.php index b204f1903..d121432fa 100644 --- a/app/Entities/Chapter.php +++ b/app/Entities/Chapter.php @@ -1,6 +1,8 @@ belongsTo(Book::class); - } - /** * Get the pages that this chapter contains. * @param string $dir diff --git a/app/Entities/Entity.php b/app/Entities/Entity.php index 480d7caa7..4e54a9e27 100644 --- a/app/Entities/Entity.php +++ b/app/Entities/Entity.php @@ -6,6 +6,7 @@ use BookStack\Actions\Tag; use BookStack\Actions\View; use BookStack\Auth\Permissions\EntityPermission; use BookStack\Auth\Permissions\JointPermission; +use BookStack\Facades\Permissions; use BookStack\Ownable; use Carbon\Carbon; use Illuminate\Database\Eloquent\Relations\MorphMany; @@ -91,7 +92,8 @@ class Entity extends Ownable */ public function activity() { - return $this->morphMany(Activity::class, 'entity')->orderBy('created_at', 'desc'); + return $this->morphMany(Activity::class, 'entity') + ->orderBy('created_at', 'desc'); } /** @@ -102,11 +104,6 @@ class Entity extends Ownable return $this->morphMany(View::class, 'viewable'); } - public function viewCountQuery() - { - return $this->views()->selectRaw('viewable_id, sum(views) as view_count')->groupBy('viewable_id'); - } - /** * Get the Tag models that have been user assigned to this entity. * @return \Illuminate\Database\Eloquent\Relations\MorphMany @@ -185,6 +182,14 @@ class Entity extends Ownable return strtolower(static::getClassName()); } + /** + * Get the type of this entity. + */ + public function type(): string + { + return static::getType(); + } + /** * Get an instance of an entity of the given type. * @param $type @@ -255,4 +260,23 @@ class Entity extends Ownable { return $path; } + + /** + * Rebuild the permissions for this entity. + */ + public function rebuildPermissions() + { + /** @noinspection PhpUnhandledExceptionInspection */ + Permissions::buildJointPermissionsForEntity($this); + } + + /** + * Generate and set a new URL slug for this model. + */ + public function refreshSlug(): string + { + $generator = new SlugGenerator($this); + $this->slug = $generator->generate(); + return $this->slug; + } } diff --git a/app/Entities/Page.php b/app/Entities/Page.php index c32417418..752b3c9dd 100644 --- a/app/Entities/Page.php +++ b/app/Entities/Page.php @@ -2,7 +2,7 @@ use BookStack\Uploads\Attachment; -class Page extends Entity +class Page extends BookChild { protected $fillable = ['name', 'html', 'priority', 'markdown']; @@ -30,15 +30,6 @@ class Page extends Entity return $array; } - /** - * Get the book this page sits in. - * @return \Illuminate\Database\Eloquent\Relations\BelongsTo - */ - public function book() - { - return $this->belongsTo(Book::class); - } - /** * Get the parent item * @return \Illuminate\Database\Eloquent\Relations\BelongsTo diff --git a/app/Entities/Repos/EntityRepo.php b/app/Entities/Repos/EntityRepo.php index 925e3c30d..c5fb3501e 100644 --- a/app/Entities/Repos/EntityRepo.php +++ b/app/Entities/Repos/EntityRepo.php @@ -6,6 +6,7 @@ use BookStack\Actions\ViewService; use BookStack\Auth\Permissions\PermissionService; use BookStack\Auth\User; use BookStack\Entities\Book; +use BookStack\Entities\BookChild; use BookStack\Entities\Bookshelf; use BookStack\Entities\Chapter; use BookStack\Entities\Entity; @@ -16,7 +17,6 @@ use BookStack\Exceptions\NotFoundException; use BookStack\Exceptions\NotifyException; use BookStack\Uploads\AttachmentService; use DOMDocument; -use DOMNode; use DOMXPath; use Illuminate\Contracts\Pagination\LengthAwarePaginator; use Illuminate\Database\Eloquent\Builder; @@ -465,25 +465,6 @@ class EntityRepo return $slug; } - /** - * Check if a slug already exists in the database. - * @param string $type - * @param string $slug - * @param bool|integer $currentId - * @param bool|integer $bookId - * @return bool - */ - protected function slugExists($type, $slug, $currentId = false, $bookId = false) - { - $query = $this->entityProvider->get($type)->where('slug', '=', $slug); - if (strtolower($type) === 'page' || strtolower($type) === 'chapter') { - $query = $query->where('book_id', '=', $bookId); - } - if ($currentId) { - $query = $query->where('id', '!=', $currentId); - } - return $query->count() > 0; - } /** * Updates entity restrictions from a request @@ -501,14 +482,14 @@ class EntityRepo foreach ($restrictions as $action => $value) { $entity->permissions()->create([ 'role_id' => $roleId, - 'action' => strtolower($action) + 'action' => strtolower($action), ]); } } } $entity->save(); - $this->permissionService->buildJointPermissionsForEntity($entity); + $entity->rebuildPermissions(); } @@ -519,12 +500,10 @@ class EntityRepo * @param array $input * @param Book|null $book * @return Entity - * @throws Throwable */ public function createFromInput(string $type, array $input = [], Book $book = null) { $entityModel = $this->entityProvider->get($type)->newInstance($input); - $entityModel->slug = $this->findSuitableSlug($type, $entityModel->name, false, $book ? $book->id : false); $entityModel->created_by = user()->id; $entityModel->updated_by = user()->id; @@ -532,41 +511,39 @@ class EntityRepo $entityModel->book_id = $book->id; } + $entityModel->refreshSlug(); $entityModel->save(); if (isset($input['tags'])) { $this->tagRepo->saveTagsToEntity($entityModel, $input['tags']); } - $this->permissionService->buildJointPermissionsForEntity($entityModel); + $entityModel->rebuildPermissions(); $this->searchService->indexEntity($entityModel); return $entityModel; } /** * Update entity details from request input. - * Used for books and chapters - * @param string $type - * @param Entity $entityModel - * @param array $input - * @return Entity - * @throws Throwable + * Used for books and chapters. + * TODO: Remove type param */ - public function updateFromInput(string $type, Entity $entityModel, array $input = []) + public function updateFromInput(string $type, Entity $entityModel, array $input = []): Entity { - if ($entityModel->name !== $input['name']) { - $entityModel->slug = $this->findSuitableSlug($type, $input['name'], $entityModel->id); - } - $entityModel->fill($input); $entityModel->updated_by = user()->id; + + if ($entityModel->isDirty('name')) { + $entityModel->refreshSlug(); + } + $entityModel->save(); if (isset($input['tags'])) { $this->tagRepo->saveTagsToEntity($entityModel, $input['tags']); } - $this->permissionService->buildJointPermissionsForEntity($entityModel); + $entityModel->rebuildPermissions(); $this->searchService->indexEntity($entityModel); return $entityModel; } @@ -595,62 +572,24 @@ class EntityRepo /** * Change the book that an entity belongs to. - * @param string $type - * @param integer $newBookId - * @param Entity $entity - * @param bool $rebuildPermissions - * @return Entity */ - public function changeBook($type, $newBookId, Entity $entity, $rebuildPermissions = false) + public function changeBook(BookChild $bookChild, int $newBookId): Entity { - $entity->book_id = $newBookId; + $bookChild->book_id = $newBookId; + $bookChild->refreshSlug(); + $bookChild->save(); + // Update related activity - foreach ($entity->activity as $activity) { - $activity->book_id = $newBookId; - $activity->save(); - } - $entity->slug = $this->findSuitableSlug($type, $entity->name, $entity->id, $newBookId); - $entity->save(); + $bookChild->activity()->update(['book_id' => $newBookId]); // Update all child pages if a chapter - if (strtolower($type) === 'chapter') { - foreach ($entity->pages as $page) { - $this->changeBook('page', $newBookId, $page, false); + if ($bookChild->isA('chapter')) { + foreach ($bookChild->pages as $page) { + $this->changeBook($page, $newBookId); } } - // Update permissions if applicable - if ($rebuildPermissions) { - $entity->load('book'); - $this->permissionService->buildJointPermissionsForEntity($entity->book); - } - - return $entity; - } - - /** - * Alias method to update the book jointPermissions in the PermissionService. - * @param Book $book - */ - public function buildJointPermissionsForBook(Book $book) - { - $this->permissionService->buildJointPermissionsForEntity($book); - } - - /** - * Format a name as a url slug. - * @param $name - * @return string - */ - protected function nameToSlug($name) - { - $slug = preg_replace('/[\+\/\\\?\@\}\{\.\,\=\[\]\#\&\!\*\'\;\:\$\%]/', '', mb_strtolower($name)); - $slug = preg_replace('/\s{2,}/', ' ', $slug); - $slug = str_replace(' ', '-', $slug); - if ($slug === "") { - $slug = substr(md5(rand(1, 500)), 0, 5); - } - return $slug; + return $bookChild; } /** @@ -885,6 +824,7 @@ class EntityRepo $shelfBooks = $bookshelf->books()->get(); $updatedBookCount = 0; + /** @var Book $book */ foreach ($shelfBooks as $book) { if (!userCan('restrictions-manage', $book)) { continue; @@ -893,7 +833,7 @@ class EntityRepo $book->restricted = $bookshelf->restricted; $book->permissions()->createMany($shelfPermissions); $book->save(); - $this->permissionService->buildJointPermissionsForEntity($book); + $book->rebuildPermissions(); $updatedBookCount++; } diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index ba59d1277..0e0585a85 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -9,7 +9,6 @@ use Carbon\Carbon; use DOMDocument; use DOMElement; use DOMXPath; -use Illuminate\Support\Collection; class PageRepo extends EntityRepo { @@ -60,11 +59,6 @@ class PageRepo extends EntityRepo $oldHtml = $page->html; $oldName = $page->name; - // Prevent slug being updated if no name change - if ($page->name !== $input['name']) { - $page->slug = $this->findSuitableSlug('page', $input['name'], $page->id, $book_id); - } - // Save page tags if present if (isset($input['tags'])) { $this->tagRepo->saveTagsToEntity($page, $input['tags']); @@ -79,11 +73,17 @@ class PageRepo extends EntityRepo $page->fill($input); $page->html = $this->formatHtml($input['html']); $page->text = $this->pageToPlainText($page); + $page->updated_by = $userId; + $page->revision_count++; + if (setting('app-editor') !== 'markdown') { $page->markdown = ''; } - $page->updated_by = $userId; - $page->revision_count++; + + if ($page->isDirty('name')) { + $page->refreshSlug(); + } + $page->save(); // Remove all update drafts for this user & page. @@ -242,8 +242,7 @@ class PageRepo extends EntityRepo } $book->pages()->save($page); - $page = $this->entityProvider->page->find($page->id); - $this->permissionService->buildJointPermissionsForEntity($page); + $page->refresh()->rebuildPermissions(); return $page; } @@ -310,12 +309,11 @@ class PageRepo extends EntityRepo $draftPage->template = ($input['template'] === 'true'); } - $draftPage->slug = $this->findSuitableSlug('page', $draftPage->name, false, $draftPage->book->id); $draftPage->html = $this->formatHtml($input['html']); $draftPage->text = $this->pageToPlainText($draftPage); $draftPage->draft = false; $draftPage->revision_count = 1; - + $draftPage->refreshSlug(); $draftPage->save(); $this->savePageRevision($draftPage, trans('entities.pages_initial_revision')); $this->searchService->indexEntity($draftPage); @@ -468,12 +466,14 @@ class PageRepo extends EntityRepo { $page->revision_count++; $this->savePageRevision($page); + $revision = $page->revisions()->where('id', '=', $revisionId)->first(); $page->fill($revision->toArray()); - $page->slug = $this->findSuitableSlug('page', $page->name, $page->id, $book->id); $page->text = $this->pageToPlainText($page); $page->updated_by = user()->id; + $page->refreshSlug(); $page->save(); + $this->searchService->indexEntity($page); return $page; } @@ -482,18 +482,19 @@ class PageRepo extends EntityRepo * Change the page's parent to the given entity. * @param Page $page * @param Entity $parent - * @throws \Throwable */ public function changePageParent(Page $page, Entity $parent) { $book = $parent->isA('book') ? $parent : $parent->book; $page->chapter_id = $parent->isA('chapter') ? $parent->id : 0; $page->save(); + if ($page->book->id !== $book->id) { - $page = $this->changeBook('page', $book->id, $page); + $page = $this->changeBook($page, $book->id); } + $page->load('book'); - $this->permissionService->buildJointPermissionsForEntity($book); + $book->rebuildPermissions(); } /** diff --git a/app/Entities/SlugGenerator.php b/app/Entities/SlugGenerator.php new file mode 100644 index 000000000..e68e00b06 --- /dev/null +++ b/app/Entities/SlugGenerator.php @@ -0,0 +1,64 @@ +entity = $entity; + } + + /** + * Generate a fresh slug for the given entity. + * The slug will generated so it does not conflict within the same parent item. + */ + public function generate(): string + { + $slug = $this->formatNameAsSlug($this->entity->name); + while ($this->slugInUse($slug)) { + $slug .= '-' . substr(md5(rand(1, 500)), 0, 3); + } + return $slug; + } + + /** + * Format a name as a url slug. + */ + protected function formatNameAsSlug(string $name): string + { + $slug = preg_replace('/[\+\/\\\?\@\}\{\.\,\=\[\]\#\&\!\*\'\;\:\$\%]/', '', mb_strtolower($name)); + $slug = preg_replace('/\s{2,}/', ' ', $slug); + $slug = str_replace(' ', '-', $slug); + if ($slug === "") { + $slug = substr(md5(rand(1, 500)), 0, 5); + } + return $slug; + } + + /** + * Check if a slug is already in-use for this + * type of model within the same parent. + */ + protected function slugInUse(string $slug): bool + { + $query = $this->entity->newQuery()->where('slug', '=', $slug); + + if ($this->entity instanceof BookChild) { + $query->where('book_id', '=', $this->entity->book_id); + } + + if ($this->entity->id) { + $query->where('id', '!=', $this->entity->id); + } + + return $query->count() > 0; + } + + +} \ No newline at end of file diff --git a/app/Facades/Permissions.php b/app/Facades/Permissions.php new file mode 100644 index 000000000..c552d7cdb --- /dev/null +++ b/app/Facades/Permissions.php @@ -0,0 +1,16 @@ +getUser($this->currentUser, 'books_view_type', config('app.views.books')); - $sort = setting()->getUser($this->currentUser, 'books_sort', 'name'); - $order = setting()->getUser($this->currentUser, 'books_sort_order', 'asc'); + $view = setting()->getForCurrentUser('books_view_type', config('app.views.books')); + $sort = setting()->getForCurrentUser('books_sort', 'name'); + $order = setting()->getForCurrentUser('books_sort_order', 'asc'); $books = $this->bookRepo->getAllPaginated('book', 18, $sort, $order); - $recents = $this->signedIn ? $this->bookRepo->getRecentlyViewed('book', 4, 0) : false; + $recents = $this->isSignedIn() ? $this->bookRepo->getRecentlyViewed('book', 4, 0) : false; $popular = $this->bookRepo->getPopular('book', 4, 0); $new = $this->bookRepo->getRecentlyCreated('book', 4, 0); @@ -107,7 +107,6 @@ class BookController extends Controller * @throws NotFoundException * @throws ImageUploadException * @throws ValidationException - * @throws Throwable */ public function store(Request $request, string $shelfSlug = null) { @@ -246,7 +245,7 @@ class BookController extends Controller * @return Factory|View * @throws NotFoundException */ - public function getSortItem(string $bookSlug) + public function sortItem(string $bookSlug) { $book = $this->bookRepo->getBySlug($bookSlug); $bookChildren = $this->bookRepo->getBookChildren($book); @@ -286,10 +285,12 @@ class BookController extends Controller // Get the books involved in the sort $bookIdsInvolved = $bookIdsInvolved->unique()->toArray(); $booksInvolved = $this->bookRepo->getManyById('book', $bookIdsInvolved, false, true); + // Throw permission error if invalid ids or inaccessible books given. if (count($bookIdsInvolved) !== count($booksInvolved)) { $this->showPermissionError(); } + // Check permissions of involved books $booksInvolved->each(function (Book $book) { $this->checkOwnablePermission('book-update', $book); @@ -304,7 +305,7 @@ class BookController extends Controller $chapterChanged = ($mapItem->type === 'page') && intval($model->chapter_id) !== $mapItem->parentChapter; if ($bookChanged) { - $this->bookRepo->changeBook($mapItem->type, $mapItem->book, $model); + $this->bookRepo->changeBook($model, $mapItem->book); } if ($chapterChanged) { $model->chapter_id = intval($mapItem->parentChapter); @@ -318,7 +319,7 @@ class BookController extends Controller // Rebuild permissions and add activity for involved books. $booksInvolved->each(function (Book $book) { - $this->bookRepo->buildJointPermissionsForBook($book); + $book->rebuildPermissions(); Activity::add($book, 'book_sort', $book->id); }); diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php index 9bb94484c..b5565eb57 100644 --- a/app/Http/Controllers/BookshelfController.php +++ b/app/Http/Controllers/BookshelfController.php @@ -40,9 +40,9 @@ class BookshelfController extends Controller */ public function index() { - $view = setting()->getUser($this->currentUser, 'bookshelves_view_type', config('app.views.bookshelves', 'grid')); - $sort = setting()->getUser($this->currentUser, 'bookshelves_sort', 'name'); - $order = setting()->getUser($this->currentUser, 'bookshelves_sort_order', 'asc'); + $view = setting()->getForCurrentUser('bookshelves_view_type', config('app.views.bookshelves', 'grid')); + $sort = setting()->getForCurrentUser('bookshelves_sort', 'name'); + $order = setting()->getForCurrentUser('bookshelves_sort_order', 'asc'); $sortOptions = [ 'name' => trans('common.sort_name'), 'created_at' => trans('common.sort_created_at'), @@ -54,7 +54,7 @@ class BookshelfController extends Controller $shelf->books = $this->entityRepo->getBookshelfChildren($shelf); } - $recents = $this->signedIn ? $this->entityRepo->getRecentlyViewed('bookshelf', 4, 0) : false; + $recents = $this->isSignedIn() ? $this->entityRepo->getRecentlyViewed('bookshelf', 4, 0) : false; $popular = $this->entityRepo->getPopular('bookshelf', 4, 0); $new = $this->entityRepo->getRecentlyCreated('bookshelf', 4, 0); diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index 8090556c9..7b1fb300c 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -201,7 +201,9 @@ class ChapterController extends Controller return redirect()->back(); } - $this->entityRepo->changeBook('chapter', $parent->id, $chapter, true); + $this->entityRepo->changeBook($chapter, $parent->id); + $chapter->rebuildPermissions(); + Activity::add($chapter, 'chapter_move', $chapter->book->id); $this->showSuccessNotification( trans('entities.chapter_move_success', ['bookName' => $parent->name])); diff --git a/app/Http/Controllers/Controller.php b/app/Http/Controllers/Controller.php index 7bca77495..034c852de 100644 --- a/app/Http/Controllers/Controller.php +++ b/app/Http/Controllers/Controller.php @@ -2,9 +2,6 @@ namespace BookStack\Http\Controllers; -use BookStack\Auth\User; -use BookStack\Entities\Entity; -use BookStack\Facades\Activity; use BookStack\Ownable; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Foundation\Validation\ValidatesRequests; @@ -16,23 +13,20 @@ abstract class Controller extends BaseController { use DispatchesJobs, ValidatesRequests; - /** - * @var User static - */ - protected $currentUser; - - /** - * @var bool - */ - protected $signedIn; - /** * Controller constructor. */ public function __construct() { - $this->currentUser = user(); - $this->signedIn = auth()->check(); + // + } + + /** + * Check if the current user is signed in. + */ + protected function isSignedIn(): bool + { + return auth()->check(); } /** @@ -123,7 +117,7 @@ abstract class Controller extends BaseController protected function checkPermissionOrCurrentUser(string $permissionName, int $userId) { return $this->checkPermissionOr($permissionName, function () use ($userId) { - return $userId === $this->currentUser->id; + return $userId === user()->id; }); } diff --git a/app/Http/Controllers/HomeController.php b/app/Http/Controllers/HomeController.php index d2c75f956..a37371d3a 100644 --- a/app/Http/Controllers/HomeController.php +++ b/app/Http/Controllers/HomeController.php @@ -26,9 +26,9 @@ class HomeController extends Controller public function index() { $activity = Activity::latest(10); - $draftPages = $this->signedIn ? $this->entityRepo->getUserDraftPages(6) : []; + $draftPages = $this->isSignedIn() ? $this->entityRepo->getUserDraftPages(6) : []; $recentFactor = count($draftPages) > 0 ? 0.5 : 1; - $recents = $this->signedIn ? Views::getUserRecentlyViewed(12*$recentFactor, 0) : $this->entityRepo->getRecentlyCreated('book', 12*$recentFactor); + $recents = $this->isSignedIn() ? Views::getUserRecentlyViewed(12*$recentFactor, 0) : $this->entityRepo->getRecentlyCreated('book', 12*$recentFactor); $recentlyUpdatedPages = $this->entityRepo->getRecentlyUpdated('page', 12); $homepageOptions = ['default', 'books', 'bookshelves', 'page']; @@ -47,9 +47,9 @@ class HomeController extends Controller // Add required list ordering & sorting for books & shelves views. if ($homepageOption === 'bookshelves' || $homepageOption === 'books') { $key = $homepageOption; - $view = setting()->getUser($this->currentUser, $key . '_view_type', config('app.views.' . $key)); - $sort = setting()->getUser($this->currentUser, $key . '_sort', 'name'); - $order = setting()->getUser($this->currentUser, $key . '_sort_order', 'asc'); + $view = setting()->getForCurrentUser($key . '_view_type', config('app.views.' . $key)); + $sort = setting()->getForCurrentUser($key . '_sort', 'name'); + $order = setting()->getForCurrentUser($key . '_sort_order', 'asc'); $sortOptions = [ 'name' => trans('common.sort_name'), diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index 37b575010..736fcf4f6 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -56,7 +56,7 @@ class PageController extends Controller $this->checkOwnablePermission('page-create', $parent); // Redirect to draft edit screen if signed in - if ($this->signedIn) { + if ($this->isSignedIn()) { $draft = $this->pageRepo->getDraftPage($book, $chapter); return redirect($draft->getUrl()); } @@ -111,7 +111,7 @@ class PageController extends Controller $this->checkOwnablePermission('page-create', $draft->parent); $this->setPageTitle(trans('entities.pages_edit_draft')); - $draftsEnabled = $this->signedIn; + $draftsEnabled = $this->isSignedIn(); $templates = $this->pageRepo->getPageTemplates(10); return view('pages.edit', [ @@ -230,7 +230,7 @@ class PageController extends Controller } // Check for a current draft version for this user - $userPageDraft = $this->pageRepo->getUserPageDraft($page, $this->currentUser->id); + $userPageDraft = $this->pageRepo->getUserPageDraft($page, user()->id); if ($userPageDraft !== null) { $page->name = $userPageDraft->name; $page->html = $userPageDraft->html; @@ -243,7 +243,7 @@ class PageController extends Controller $this->showWarningNotification( implode("\n", $warnings)); } - $draftsEnabled = $this->signedIn; + $draftsEnabled = $this->isSignedIn(); $templates = $this->pageRepo->getPageTemplates(10); return view('pages.edit', [ @@ -285,7 +285,7 @@ class PageController extends Controller $page = $this->pageRepo->getById('page', $pageId, true); $this->checkOwnablePermission('page-update', $page); - if (!$this->signedIn) { + if (!$this->isSignedIn()) { return response()->json([ 'status' => 'error', 'message' => trans('errors.guests_cannot_save_drafts'), diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 6984fef1e..c787e78ad 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -177,7 +177,7 @@ class UserController extends Controller } // External auth id updates - if ($this->currentUser->can('users-manage') && $request->filled('external_auth_id')) { + if (user()->can('users-manage') && $request->filled('external_auth_id')) { $user->external_auth_id = $request->get('external_auth_id'); } diff --git a/app/Providers/CustomFacadeProvider.php b/app/Providers/CustomFacadeProvider.php index e7bde5290..b4158187c 100644 --- a/app/Providers/CustomFacadeProvider.php +++ b/app/Providers/CustomFacadeProvider.php @@ -4,6 +4,7 @@ namespace BookStack\Providers; use BookStack\Actions\ActivityService; use BookStack\Actions\ViewService; +use BookStack\Auth\Permissions\PermissionService; use BookStack\Settings\SettingService; use BookStack\Uploads\ImageService; use Illuminate\Support\ServiceProvider; @@ -27,20 +28,24 @@ class CustomFacadeProvider extends ServiceProvider */ public function register() { - $this->app->bind('activity', function () { + $this->app->singleton('activity', function () { return $this->app->make(ActivityService::class); }); - $this->app->bind('views', function () { + $this->app->singleton('views', function () { return $this->app->make(ViewService::class); }); - $this->app->bind('setting', function () { + $this->app->singleton('setting', function () { return $this->app->make(SettingService::class); }); - $this->app->bind('images', function () { + $this->app->singleton('images', function () { return $this->app->make(ImageService::class); }); + + $this->app->singleton('permissions', function () { + return $this->app->make(PermissionService::class); + }); } } diff --git a/phpunit.xml b/phpunit.xml index 7de7233af..9f83e95ff 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -29,7 +29,7 @@ - + diff --git a/routes/web.php b/routes/web.php index 0277c1bb4..be729f566 100644 --- a/routes/web.php +++ b/routes/web.php @@ -40,7 +40,7 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/{slug}/edit', 'BookController@edit'); Route::put('/{slug}', 'BookController@update'); Route::delete('/{id}', 'BookController@destroy'); - Route::get('/{slug}/sort-item', 'BookController@getSortItem'); + Route::get('/{slug}/sort-item', 'BookController@sortItem'); Route::get('/{slug}', 'BookController@show'); Route::get('/{bookSlug}/permissions', 'BookController@showPermissions'); Route::put('/{bookSlug}/permissions', 'BookController@permissions'); diff --git a/tests/Entity/PageDraftTest.php b/tests/Entity/PageDraftTest.php index f29231c39..e374575f5 100644 --- a/tests/Entity/PageDraftTest.php +++ b/tests/Entity/PageDraftTest.php @@ -18,25 +18,26 @@ class PageDraftTest extends BrowserKitTest public function test_draft_content_shows_if_available() { $addedContent = '

test message content

'; - $this->asAdmin()->visit($this->page->getUrl() . '/edit') + $this->asAdmin()->visit($this->page->getUrl('/edit')) ->dontSeeInField('html', $addedContent); $newContent = $this->page->html . $addedContent; $this->pageRepo->updatePageDraft($this->page, ['html' => $newContent]); - $this->asAdmin()->visit($this->page->getUrl() . '/edit') + $this->asAdmin()->visit($this->page->getUrl('/edit')) ->seeInField('html', $newContent); } public function test_draft_not_visible_by_others() { $addedContent = '

test message content

'; - $this->asAdmin()->visit($this->page->getUrl() . '/edit') + $this->asAdmin()->visit($this->page->getUrl('/edit')) ->dontSeeInField('html', $addedContent); $newContent = $this->page->html . $addedContent; $newUser = $this->getEditor(); $this->pageRepo->updatePageDraft($this->page, ['html' => $newContent]); - $this->actingAs($newUser)->visit($this->page->getUrl() . '/edit') + + $this->actingAs($newUser)->visit($this->page->getUrl('/edit')) ->dontSeeInField('html', $newContent); } @@ -44,7 +45,7 @@ class PageDraftTest extends BrowserKitTest { $this->asAdmin(); $this->pageRepo->updatePageDraft($this->page, ['html' => 'test content']); - $this->asAdmin()->visit($this->page->getUrl() . '/edit') + $this->asAdmin()->visit($this->page->getUrl('/edit')) ->see('You are currently editing a draft'); } @@ -52,7 +53,7 @@ class PageDraftTest extends BrowserKitTest { $nonEditedPage = \BookStack\Entities\Page::take(10)->get()->last(); $addedContent = '

test message content

'; - $this->asAdmin()->visit($this->page->getUrl() . '/edit') + $this->asAdmin()->visit($this->page->getUrl('/edit')) ->dontSeeInField('html', $addedContent); $newContent = $this->page->html . $addedContent; @@ -60,7 +61,7 @@ class PageDraftTest extends BrowserKitTest $this->pageRepo->updatePageDraft($this->page, ['html' => $newContent]); $this->actingAs($newUser) - ->visit($this->page->getUrl() . '/edit') + ->visit($this->page->getUrl('/edit')) ->see('Admin has started editing this page'); $this->flushSession(); $this->visit($nonEditedPage->getUrl() . '/edit') diff --git a/tests/Entity/TagTest.php b/tests/Entity/TagTest.php index 70b8c960b..13876410a 100644 --- a/tests/Entity/TagTest.php +++ b/tests/Entity/TagTest.php @@ -3,6 +3,7 @@ use BookStack\Entities\Book; use BookStack\Entities\Chapter; use BookStack\Actions\Tag; +use BookStack\Entities\Entity; use BookStack\Entities\Page; use BookStack\Auth\Permissions\PermissionService; @@ -14,9 +15,9 @@ class TagTest extends BrowserKitTest /** * Get an instance of a page that has many tags. * @param \BookStack\Actions\Tag[]|bool $tags - * @return mixed + * @return Entity */ - protected function getEntityWithTags($class, $tags = false) + protected function getEntityWithTags($class, $tags = false): Entity { $entity = $class::first(); @@ -122,7 +123,7 @@ class TagTest extends BrowserKitTest // Set restricted permission the page $page->restricted = true; $page->save(); - $permissionService->buildJointPermissionsForEntity($page); + $page->rebuildPermissions(); $this->asAdmin()->get('/ajax/tags/suggest/names?search=co')->seeJsonEquals(['color', 'country']); $this->asEditor()->get('/ajax/tags/suggest/names?search=co')->seeJsonEquals([]); diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php index ed92fa59f..2bff27223 100644 --- a/tests/SharedTestHelpers.php +++ b/tests/SharedTestHelpers.php @@ -80,7 +80,7 @@ trait SharedTestHelpers */ protected function regenEntityPermissions(Entity $entity) { - app(PermissionService::class)->buildJointPermissionsForEntity($entity); + $entity->rebuildPermissions(); $entity->load('jointPermissions'); }