diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 5b97fbdaf..8ada59433 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -3,7 +3,6 @@ use Activity; use BookStack\Repos\UserRepo; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Auth; use BookStack\Http\Requests; use BookStack\Repos\BookRepo; use BookStack\Repos\ChapterRepo; @@ -180,21 +179,31 @@ class BookController extends Controller return redirect($book->getUrl()); } - $sortedBooks = []; // Sort pages and chapters + $sortedBooks = []; + $updatedModels = collect(); $sortMap = json_decode($request->get('sort-tree')); $defaultBookId = $book->id; - foreach ($sortMap as $index => $bookChild) { - $id = $bookChild->id; + + // Loop through contents of provided map and update entities accordingly + foreach ($sortMap as $bookChild) { + $priority = $bookChild->sort; + $id = intval($bookChild->id); $isPage = $bookChild->type == 'page'; - $bookId = $this->bookRepo->exists($bookChild->book) ? $bookChild->book : $defaultBookId; + $bookId = $this->bookRepo->exists($bookChild->book) ? intval($bookChild->book) : $defaultBookId; + $chapterId = ($isPage && $bookChild->parentChapter === false) ? 0 : intval($bookChild->parentChapter); $model = $isPage ? $this->pageRepo->getById($id) : $this->chapterRepo->getById($id); - $isPage ? $this->pageRepo->changeBook($bookId, $model) : $this->chapterRepo->changeBook($bookId, $model); - $model->priority = $index; - if ($isPage) { - $model->chapter_id = ($bookChild->parentChapter === false) ? 0 : $bookChild->parentChapter; + + // Update models only if there's a change in parent chain or ordering. + if ($model->priority !== $priority || $model->book_id !== $bookId || ($isPage && $model->chapter_id !== $chapterId)) { + $isPage ? $this->pageRepo->changeBook($bookId, $model) : $this->chapterRepo->changeBook($bookId, $model); + $model->priority = $priority; + if ($isPage) $model->chapter_id = $chapterId; + $model->save(); + $updatedModels->push($model); } - $model->save(); + + // Store involved books to be sorted later if (!in_array($bookId, $sortedBooks)) { $sortedBooks[] = $bookId; } @@ -203,10 +212,12 @@ class BookController extends Controller // Add activity for books foreach ($sortedBooks as $bookId) { $updatedBook = $this->bookRepo->getById($bookId); - $this->bookRepo->updateBookPermissions($updatedBook); Activity::add($updatedBook, 'book_sort', $updatedBook->id); } + // Update permissions on changed models + $this->bookRepo->buildJointPermissions($updatedModels); + return redirect($book->getUrl()); } diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php index 3c9050bf6..03ec2c110 100644 --- a/app/Http/Controllers/ChapterController.php +++ b/app/Http/Controllers/ChapterController.php @@ -204,7 +204,7 @@ class ChapterController extends Controller return redirect()->back(); } - $this->chapterRepo->changeBook($parent->id, $chapter); + $this->chapterRepo->changeBook($parent->id, $chapter, true); Activity::add($chapter, 'chapter_move', $chapter->book->id); session()->flash('success', sprintf('Chapter moved to "%s"', $parent->name)); diff --git a/app/Repos/BookRepo.php b/app/Repos/BookRepo.php index 58816d738..fdc4dd8d4 100644 --- a/app/Repos/BookRepo.php +++ b/app/Repos/BookRepo.php @@ -2,6 +2,7 @@ use Alpha\B; use BookStack\Exceptions\NotFoundException; +use Illuminate\Database\Eloquent\Collection; use Illuminate\Support\Str; use BookStack\Book; use Views; @@ -173,15 +174,6 @@ class BookRepo extends EntityRepo $book->delete(); } - /** - * Alias method to update the book jointPermissions in the PermissionService. - * @param Book $book - */ - public function updateBookPermissions(Book $book) - { - $this->permissionService->buildJointPermissionsForEntity($book); - } - /** * Get the next child element priority. * @param Book $book diff --git a/app/Repos/ChapterRepo.php b/app/Repos/ChapterRepo.php index 1a8cbdf0f..c12a9f0f2 100644 --- a/app/Repos/ChapterRepo.php +++ b/app/Repos/ChapterRepo.php @@ -195,11 +195,12 @@ class ChapterRepo extends EntityRepo /** * Changes the book relation of this chapter. - * @param $bookId + * @param $bookId * @param Chapter $chapter + * @param bool $rebuildPermissions * @return Chapter */ - public function changeBook($bookId, Chapter $chapter) + public function changeBook($bookId, Chapter $chapter, $rebuildPermissions = false) { $chapter->book_id = $bookId; // Update related activity @@ -213,9 +214,12 @@ class ChapterRepo extends EntityRepo foreach ($chapter->pages as $page) { $this->pageRepo->changeBook($bookId, $page); } - // Update permissions - $chapter->load('book'); - $this->permissionService->buildJointPermissionsForEntity($chapter->book); + + // Update permissions if applicable + if ($rebuildPermissions) { + $chapter->load('book'); + $this->permissionService->buildJointPermissionsForEntity($chapter->book); + } return $chapter; } diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php index 28f4c0ae8..c94601738 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -6,6 +6,7 @@ use BookStack\Entity; use BookStack\Page; use BookStack\Services\PermissionService; use BookStack\User; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; class EntityRepo @@ -260,6 +261,15 @@ class EntityRepo return $query; } + /** + * Alias method to update the book jointPermissions in the PermissionService. + * @param Collection $collection collection on entities + */ + public function buildJointPermissions(Collection $collection) + { + $this->permissionService->buildJointPermissionsForEntities($collection); + } + } diff --git a/app/Services/PermissionService.php b/app/Services/PermissionService.php index 0fffe60f2..cee074cd7 100644 --- a/app/Services/PermissionService.php +++ b/app/Services/PermissionService.php @@ -8,7 +8,7 @@ use BookStack\Ownable; use BookStack\Page; use BookStack\Role; use BookStack\User; -use Illuminate\Database\Eloquent\Collection; +use Illuminate\Support\Collection; class PermissionService { @@ -25,6 +25,8 @@ class PermissionService protected $jointPermission; protected $role; + protected $entityCache; + /** * PermissionService constructor. * @param JointPermission $jointPermission @@ -48,6 +50,57 @@ class PermissionService $this->page = $page; } + /** + * Prepare the local entity cache and ensure it's empty + */ + protected function readyEntityCache() + { + $this->entityCache = [ + 'books' => collect(), + 'chapters' => collect() + ]; + } + + /** + * Get a book via ID, Checks local cache + * @param $bookId + * @return Book + */ + protected function getBook($bookId) + { + if (isset($this->entityCache['books']) && $this->entityCache['books']->has($bookId)) { + return $this->entityCache['books']->get($bookId); + } + + $book = $this->book->find($bookId); + if ($book === null) $book = false; + if (isset($this->entityCache['books'])) { + $this->entityCache['books']->put($bookId, $book); + } + + return $book; + } + + /** + * Get a chapter via ID, Checks local cache + * @param $chapterId + * @return Book + */ + protected function getChapter($chapterId) + { + if (isset($this->entityCache['chapters']) && $this->entityCache['chapters']->has($chapterId)) { + return $this->entityCache['chapters']->get($chapterId); + } + + $chapter = $this->chapter->find($chapterId); + if ($chapter === null) $chapter = false; + if (isset($this->entityCache['chapters'])) { + $this->entityCache['chapters']->put($chapterId, $chapter); + } + + return $chapter; + } + /** * Get the roles for the current user; * @return array|bool @@ -76,6 +129,7 @@ class PermissionService public function buildJointPermissions() { $this->jointPermission->truncate(); + $this->readyEntityCache(); // Get all roles (Should be the most limited dimension) $roles = $this->role->with('permissions')->get(); @@ -97,7 +151,7 @@ class PermissionService } /** - * Create the entity jointPermissions for a particular entity. + * Rebuild the entity jointPermissions for a particular entity. * @param Entity $entity */ public function buildJointPermissionsForEntity(Entity $entity) @@ -116,6 +170,17 @@ class PermissionService $this->createManyJointPermissions($entities, $roles); } + /** + * Rebuild the entity jointPermissions for a collection of entities. + * @param Collection $entities + */ + public function buildJointPermissionsForEntities(Collection $entities) + { + $roles = $this->role->with('jointPermissions')->get(); + $this->deleteManyJointPermissionsForEntities($entities); + $this->createManyJointPermissions($entities, $roles); + } + /** * Build the entity jointPermissions for a particular role. * @param Role $role @@ -177,9 +242,14 @@ class PermissionService */ protected function deleteManyJointPermissionsForEntities($entities) { + $query = $this->jointPermission->newQuery(); foreach ($entities as $entity) { - $entity->jointPermissions()->delete(); + $query->orWhere(function($query) use ($entity) { + $query->where('entity_id', '=', $entity->id) + ->where('entity_type', '=', $entity->getMorphClass()); + }); } + $query->delete(); } /** @@ -189,6 +259,7 @@ class PermissionService */ protected function createManyJointPermissions($entities, $roles) { + $this->readyEntityCache(); $jointPermissions = []; foreach ($entities as $entity) { foreach ($roles as $role) { @@ -248,8 +319,9 @@ class PermissionService } elseif ($entity->isA('chapter')) { if (!$entity->restricted) { - $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $restrictionAction); - $hasPermissiveAccessToBook = !$entity->book->restricted; + $book = $this->getBook($entity->book_id); + $hasExplicitAccessToBook = $book->hasActiveRestriction($role->id, $restrictionAction); + $hasPermissiveAccessToBook = !$book->restricted; return $this->createJointPermissionDataArray($entity, $role, $action, ($hasExplicitAccessToBook || ($roleHasPermission && $hasPermissiveAccessToBook)), ($hasExplicitAccessToBook || ($roleHasPermissionOwn && $hasPermissiveAccessToBook))); @@ -261,11 +333,14 @@ class PermissionService } elseif ($entity->isA('page')) { if (!$entity->restricted) { - $hasExplicitAccessToBook = $entity->book->hasActiveRestriction($role->id, $restrictionAction); - $hasPermissiveAccessToBook = !$entity->book->restricted; - $hasExplicitAccessToChapter = $entity->chapter && $entity->chapter->hasActiveRestriction($role->id, $restrictionAction); - $hasPermissiveAccessToChapter = $entity->chapter && !$entity->chapter->restricted; - $acknowledgeChapter = ($entity->chapter && $entity->chapter->restricted); + $book = $this->getBook($entity->book_id); + $hasExplicitAccessToBook = $book->hasActiveRestriction($role->id, $restrictionAction); + $hasPermissiveAccessToBook = !$book->restricted; + + $chapter = $this->getChapter($entity->chapter_id); + $hasExplicitAccessToChapter = $chapter && $chapter->hasActiveRestriction($role->id, $restrictionAction); + $hasPermissiveAccessToChapter = $chapter && !$chapter->restricted; + $acknowledgeChapter = ($chapter && $chapter->restricted); $hasExplicitAccessToParents = $acknowledgeChapter ? $hasExplicitAccessToChapter : $hasExplicitAccessToBook; $hasPermissiveAccessToParents = $acknowledgeChapter ? $hasPermissiveAccessToChapter : $hasPermissiveAccessToBook; diff --git a/resources/views/books/sort.blade.php b/resources/views/books/sort.blade.php index 3849dfcfc..984db0ce6 100644 --- a/resources/views/books/sort.blade.php +++ b/resources/views/books/sort.blade.php @@ -50,7 +50,7 @@ var sortableOptions = { group: 'serialization', onDrop: function($item, container, _super) { - var pageMap = buildPageMap(); + var pageMap = buildEntityMap(); $('#sort-tree-input').val(JSON.stringify(pageMap)); _super($item, container); }, @@ -74,29 +74,42 @@ $link.remove(); }); - function buildPageMap() { - var pageMap = []; + /** + * Build up a mapping of entities with their ordering and nesting. + * @returns {Array} + */ + function buildEntityMap() { + var entityMap = []; var $lists = $('.sort-list'); $lists.each(function(listIndex) { var list = $(this); var bookId = list.closest('[data-type="book"]').attr('data-id'); - var $childElements = list.find('[data-type="page"], [data-type="chapter"]'); - $childElements.each(function(childIndex) { + var $directChildren = list.find('> [data-type="page"], > [data-type="chapter"]'); + $directChildren.each(function(directChildIndex) { var $childElem = $(this); var type = $childElem.attr('data-type'); var parentChapter = false; - if(type === 'page' && $childElem.closest('[data-type="chapter"]').length === 1) { - parentChapter = $childElem.closest('[data-type="chapter"]').attr('data-id'); - } - pageMap.push({ - id: $childElem.attr('data-id'), + var childId = $childElem.attr('data-id'); + entityMap.push({ + id: childId, + sort: directChildIndex, parentChapter: parentChapter, type: type, book: bookId }); + $chapterChildren = $childElem.find('[data-type="page"]').each(function(pageIndex) { + var $chapterChild = $(this); + entityMap.push({ + id: $chapterChild.attr('data-id'), + sort: pageIndex, + parentChapter: childId, + type: 'page', + book: bookId + }); + }); }); }); - return pageMap; + return entityMap; } });