From 8b160b9fb409f1e0d32b831e44a179cd480ab1bc Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 24 Dec 2017 14:42:31 +0000 Subject: [PATCH 01/16] Updated pull-request info to be clearer Also updated dev version --- readme.md | 8 +++++++- version | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 1b3db4a56..77f1e8805 100644 --- a/readme.md +++ b/readme.md @@ -72,7 +72,13 @@ Some strings have colon-prefixed variables in such as `:userName`. Leave these v Feel free to create issues to request new features or to report bugs and problems. Just please follow the template given when creating the issue. -Pull requests are very welcome. If the scope of your pull request is very large it may be best to open the pull request early or create an issue for it to discuss how it will fit in to the project and plan out the merge. +### Pull Request + +Pull requests are very welcome. If the scope of your pull request is large it may be best to open the pull request early or create an issue for it to discuss how it will fit in to the project and plan out the merge. + +Pull requests should be created from the `master` branch and should be merged back into `master` once done. Please do not build from or request a merge into the `release` branch as this is only for publishing releases. + +If you are looking to alter CSS or JavaScript content please edit the source files found in `resources/assets`. Any CSS or JS files within `public` are built from these source files and therefore should not be edited directly. ## Website, Docs & Blog diff --git a/version b/version index de0ad7791..0507cd08e 100644 --- a/version +++ b/version @@ -1 +1 @@ -v0.18-dev +v0.20-dev From 7da8804753b38529e6add829726de2145a0059d1 Mon Sep 17 00:00:00 2001 From: Abijeet Date: Tue, 26 Dec 2017 02:22:41 +0530 Subject: [PATCH 02/16] Adds code to allow deletion of users via cmd line. Fixes #579 Command: php artisan bookstack:delete-users Signed-off-by: Abijeet --- app/Console/Commands/DeleteUsers.php | 62 ++++++++++++++++++++++++++++ app/User.php | 2 +- 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 app/Console/Commands/DeleteUsers.php diff --git a/app/Console/Commands/DeleteUsers.php b/app/Console/Commands/DeleteUsers.php new file mode 100644 index 000000000..c287dd55e --- /dev/null +++ b/app/Console/Commands/DeleteUsers.php @@ -0,0 +1,62 @@ +user = $user; + $this->userRepo = $userRepo; + parent::__construct(); + } + + public function handle() + { + $confirm = $this->ask('This will delete all users from the system that are not "admin" or system users. Are you sure you want to continue? (Type "yes" to continue)'); + $numDeleted = 0; + if (strtolower(trim($confirm)) === 'yes') + { + $totalUsers = User::count(); + $users = $this->user->where('system_name', '=', null)->with('roles')->get(); + foreach ($users as $user) + { + if ($user->hasRole('admin')) + { + // don't delete users with "admin" role + continue; + } + $this->userRepo->destroy($user); + ++$numDeleted; + } + $this->info("Deleted $numDeleted of $totalUsers total users."); + } + else + { + $this->info('Exiting...'); + } + } + +} diff --git a/app/User.php b/app/User.php index 8033557e4..fd6879ba0 100644 --- a/app/User.php +++ b/app/User.php @@ -81,7 +81,7 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon */ public function hasSystemRole($role) { - return $this->roles->pluck('system_name')->contains('admin'); + return $this->roles->pluck('system_name')->contains($role); } /** From 0d4db603a47a6cd8889bb7b591bdc2cffbcac134 Mon Sep 17 00:00:00 2001 From: Abijeet Date: Tue, 26 Dec 2017 12:38:16 +0530 Subject: [PATCH 03/16] Adds button to allow users to toggle the book view via the books list page. Closes #613 Signed-off-by: Abijeet --- app/Http/Controllers/BookController.php | 2 +- app/Http/Controllers/UserController.php | 21 +++++++++++++++++++++ resources/lang/en/entities.php | 1 + resources/views/books/index.blade.php | 7 ++++++- routes/web.php | 1 + 5 files changed, 30 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index e181aec89..aa8f89ea4 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -46,7 +46,7 @@ class BookController extends Controller 'books' => $books, 'recents' => $recents, 'popular' => $popular, - 'new' => $new, + 'new' => $new, 'booksViewType' => $booksViewType ]); } diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index fe5c7a243..5c10133a2 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -249,4 +249,25 @@ class UserController extends Controller 'assetCounts' => $assetCounts ]); } + + public function switchBookView($id, Request $request) { + $this->checkPermission('users-manage'); + $viewType = $request->get('book_view_type'); + + if (!in_array($viewType, ['grid', 'list'])) { + $viewType = 'list'; + } + + $user = $this->user->findOrFail($id); + setting()->putUser($user, 'books_view_type', $viewType); + + $previousUrl = url()->previous(); + if (empty($previousUrl)) { + // if no previous URL, redirect to settings + return redirect("/settings/users/$id"); + } else { + // redirect to the previous page. + return redirect($previousUrl); + } + } } diff --git a/resources/lang/en/entities.php b/resources/lang/en/entities.php index 4dc5ccc38..6e481d03b 100644 --- a/resources/lang/en/entities.php +++ b/resources/lang/en/entities.php @@ -99,6 +99,7 @@ return [ 'books_sort_named' => 'Sort Book :bookName', 'books_sort_show_other' => 'Show Other Books', 'books_sort_save' => 'Save New Order', + 'books_toggle_view' => 'Toggle Book View', /** * Chapters diff --git a/resources/views/books/index.blade.php b/resources/views/books/index.blade.php index d392af045..5ca01a550 100644 --- a/resources/views/books/index.blade.php +++ b/resources/views/books/index.blade.php @@ -4,6 +4,11 @@
+
id}/switch-book-view") }}" method="POST" class="inline"> + {!! csrf_field() !!} + + +
@if($currentUser->can('book-create-all')) {{ trans('entities.books_create') }} @endif @@ -52,7 +57,7 @@
@endforeach {!! $books->render() !!} - @else + @else
@foreach($books as $key => $book) @include('books/grid-item', ['book' => $book]) diff --git a/routes/web.php b/routes/web.php index f5e59f109..31de09737 100644 --- a/routes/web.php +++ b/routes/web.php @@ -146,6 +146,7 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/users', 'UserController@index'); Route::get('/users/create', 'UserController@create'); Route::get('/users/{id}/delete', 'UserController@delete'); + Route::post('/users/{id}/switch-book-view', 'UserController@switchBookView'); Route::post('/users/create', 'UserController@store'); Route::get('/users/{id}', 'UserController@edit'); Route::put('/users/{id}', 'UserController@update'); From d5a252977576c8f45e16bba51c6b5762c9152d93 Mon Sep 17 00:00:00 2001 From: Abijeet Date: Tue, 26 Dec 2017 15:46:20 +0530 Subject: [PATCH 04/16] Adds test cases and fixes an issue with the permission checking. Signed-off-by: Abijeet --- app/Http/Controllers/UserController.php | 4 +++- tests/Entity/EntityTest.php | 23 ++++++++++++++++++++++- tests/UserProfileTest.php | 2 +- 3 files changed, 26 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 5c10133a2..397bb2922 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -251,7 +251,9 @@ class UserController extends Controller } public function switchBookView($id, Request $request) { - $this->checkPermission('users-manage'); + $this->checkPermissionOr('users-manage', function () use ($id) { + return $this->currentUser->id == $id; + }); $viewType = $request->get('book_view_type'); if (!in_array($viewType, ['grid', 'list'])) { diff --git a/tests/Entity/EntityTest.php b/tests/Entity/EntityTest.php index a43f65b5e..549253417 100644 --- a/tests/Entity/EntityTest.php +++ b/tests/Entity/EntityTest.php @@ -82,6 +82,27 @@ class EntityTest extends BrowserKitTest ->see($firstChapter->name); } + public function test_toggle_book_view() + { + $editor = $this->getEditor(); + setting()->putUser($editor, 'books_view_type', 'grid'); + + $this->actingAs($editor) + ->visit('/books') + ->pageHasElement('.featured-image-container') + ->submitForm('Toggle Book View') + // Check redirection. + ->seePageIs('/books') + ->pageNotHasElement('.featured-image-container'); + + $this->actingAs($editor) + ->visit('/books') + ->submitForm('Toggle Book View') + ->seePageIs('/books') + ->pageHasElement('.featured-image-container'); + + } + public function pageCreation($chapter) { $page = factory(Page::class)->make([ @@ -155,7 +176,7 @@ class EntityTest extends BrowserKitTest ->type($book->name, '#name') ->type($book->description, '#description') ->press('Save Book'); - + $expectedPattern = '/\/books\/my-first-book-[0-9a-zA-Z]{3}/'; $this->assertRegExp($expectedPattern, $this->currentUri, "Did not land on expected page [$expectedPattern].\n"); diff --git a/tests/UserProfileTest.php b/tests/UserProfileTest.php index 0c66363f0..3262117d5 100644 --- a/tests/UserProfileTest.php +++ b/tests/UserProfileTest.php @@ -103,7 +103,7 @@ class UserProfileTest extends BrowserKitTest $this->actingAs($editor) ->visit('/books') ->pageNotHasElement('.featured-image-container') - ->pageHasElement('.entity-list-item'); + ->pageHasElement('.content .entity-list-item'); } public function test_books_view_is_grid() From afe781bc398b319b965a29ccc45ca4b6aa879d0f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 28 Dec 2017 13:19:02 +0000 Subject: [PATCH 05/16] Enabled session in 404 responses Fixes #634 --- app/Exceptions/Handler.php | 26 +++++++++++++++++++++++++ app/Http/Controllers/PageController.php | 3 ++- app/Http/Kernel.php | 8 ++++++++ resources/views/errors/404.blade.php | 3 --- resources/views/errors/500.blade.php | 2 +- tests/ErrorTest.php | 18 +++++++++++++++++ 6 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 tests/ErrorTest.php diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 12792e151..a979072e2 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -4,11 +4,14 @@ namespace BookStack\Exceptions; use Exception; use Illuminate\Auth\AuthenticationException; +use Illuminate\Http\Request; +use Illuminate\Pipeline\Pipeline; use Illuminate\Validation\ValidationException; use Illuminate\Database\Eloquent\ModelNotFoundException; use Symfony\Component\HttpKernel\Exception\HttpException; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; use Illuminate\Auth\Access\AuthorizationException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; class Handler extends ExceptionHandler { @@ -60,9 +63,32 @@ class Handler extends ExceptionHandler return response()->view('errors/' . $code, ['message' => $message], $code); } + // Handle 404 errors with a loaded session to enable showing user-specific information + if ($this->isExceptionType($e, NotFoundHttpException::class)) { + return $this->loadErrorMiddleware($request, function ($request) use ($e) { + $message = $e->getMessage() ?: trans('errors.404_page_not_found'); + return response()->view('errors/404', ['message' => $message], 404); + }); + } + return parent::render($request, $e); } + /** + * Load the middleware required to show state/session-enabled error pages. + * @param Request $request + * @param $callback + * @return mixed + */ + protected function loadErrorMiddleware(Request $request, $callback) + { + $middleware = (\Route::getMiddlewareGroups()['web_errors']); + return (new Pipeline($this->container)) + ->send($request) + ->through($middleware) + ->then($callback); + } + /** * Check the exception chain to compare against the original exception type. * @param Exception $e diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php index 13e928465..9dc7d6401 100644 --- a/app/Http/Controllers/PageController.php +++ b/app/Http/Controllers/PageController.php @@ -145,6 +145,7 @@ class PageController extends Controller * @param string $bookSlug * @param string $pageSlug * @return Response + * @throws NotFoundException */ public function show($bookSlug, $pageSlug) { @@ -152,7 +153,7 @@ class PageController extends Controller $page = $this->entityRepo->getBySlug('page', $pageSlug, $bookSlug); } catch (NotFoundException $e) { $page = $this->entityRepo->getPageByOldSlug($pageSlug, $bookSlug); - if ($page === null) abort(404); + if ($page === null) throw $e; return redirect($page->getUrl()); } diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index cd894de95..9d2871bbe 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -33,6 +33,14 @@ class Kernel extends HttpKernel \Illuminate\Routing\Middleware\SubstituteBindings::class, \BookStack\Http\Middleware\Localization::class ], + 'web_errors' => [ + \BookStack\Http\Middleware\EncryptCookies::class, + \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class, + \Illuminate\Session\Middleware\StartSession::class, + \Illuminate\View\Middleware\ShareErrorsFromSession::class, + \BookStack\Http\Middleware\VerifyCsrfToken::class, + \BookStack\Http\Middleware\Localization::class + ], 'api' => [ 'throttle:60,1', 'bindings', diff --git a/resources/views/errors/404.blade.php b/resources/views/errors/404.blade.php index f6ef850af..7cc67a677 100644 --- a/resources/views/errors/404.blade.php +++ b/resources/views/errors/404.blade.php @@ -1,8 +1,6 @@ @extends('simple-layout') @section('content') - -

 

@@ -16,7 +14,6 @@
@if (setting('app-public') || !user()->isDefault()) -
diff --git a/resources/views/errors/500.blade.php b/resources/views/errors/500.blade.php index 71fb78a35..a01234d81 100644 --- a/resources/views/errors/500.blade.php +++ b/resources/views/errors/500.blade.php @@ -6,7 +6,7 @@

{{ trans('errors.error_occurred') }}

-
{{ $message }}
+
{{ $message or 'An unknown error occurred' }}

{{ trans('errors.return_home') }}

diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php new file mode 100644 index 000000000..c9b5a0109 --- /dev/null +++ b/tests/ErrorTest.php @@ -0,0 +1,18 @@ +getEditor(); + $this->actingAs($editor); + $notFound = $this->get('/fgfdngldfnotfound'); + $notFound->assertStatus(404); + $notFound->assertDontSeeText('Log in'); + $notFound->assertSeeText($editor->getShortName(9)); + } +} \ No newline at end of file From 937d2cd55cd1916ee2125748d4cb2b6852086834 Mon Sep 17 00:00:00 2001 From: Abijeet Date: Thu, 28 Dec 2017 22:32:24 +0530 Subject: [PATCH 06/16] Adds font-size to ol to ensure that they are uniform. Fixes #643 Signed-off-by: Abijeet --- resources/assets/sass/_components.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resources/assets/sass/_components.scss b/resources/assets/sass/_components.scss index 54e109067..051d1978a 100644 --- a/resources/assets/sass/_components.scss +++ b/resources/assets/sass/_components.scss @@ -549,7 +549,7 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group { .content { padding: $-s; font-size: 0.666em; - p, ul { + p, ul, ol { font-size: $fs-m; margin: .5em 0; } From 0c1b1cd435b1b757fecbdd4122803b7fff58c1be Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 29 Dec 2017 16:14:20 +0000 Subject: [PATCH 07/16] Standardised admin role check --- app/Console/Commands/DeleteUsers.php | 4 ++-- app/Repos/UserRepo.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/Console/Commands/DeleteUsers.php b/app/Console/Commands/DeleteUsers.php index c287dd55e..8829d3992 100644 --- a/app/Console/Commands/DeleteUsers.php +++ b/app/Console/Commands/DeleteUsers.php @@ -39,11 +39,11 @@ class DeleteUsers extends Command{ $numDeleted = 0; if (strtolower(trim($confirm)) === 'yes') { - $totalUsers = User::count(); + $totalUsers = $this->user->count(); $users = $this->user->where('system_name', '=', null)->with('roles')->get(); foreach ($users as $user) { - if ($user->hasRole('admin')) + if ($user->hasSystemRole('admin')) { // don't delete users with "admin" role continue; diff --git a/app/Repos/UserRepo.php b/app/Repos/UserRepo.php index c3546a442..52ad2e47e 100644 --- a/app/Repos/UserRepo.php +++ b/app/Repos/UserRepo.php @@ -115,9 +115,9 @@ class UserRepo */ public function isOnlyAdmin(User $user) { - if (!$user->roles->pluck('name')->contains('admin')) return false; + if (!$user->hasSystemRole('admin')) return false; - $adminRole = $this->role->getRole('admin'); + $adminRole = $this->role->getSystemRole('admin'); if ($adminRole->users->count() > 1) return false; return true; } From 141bf227251287174d0a4cff833b88762031e6a1 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 29 Dec 2017 16:49:03 +0000 Subject: [PATCH 08/16] Updated book view change to PATCH + other amends Moved toggle to right of header bar and added unique text and icon for each view type. Removed old profile setting to keep things clean. --- app/Http/Controllers/UserController.php | 18 +++++++++--------- resources/lang/en/common.php | 2 ++ resources/lang/en/entities.php | 1 - resources/lang/en/settings.php | 1 - resources/views/books/index.blade.php | 16 ++++++++++++---- resources/views/users/edit.blade.php | 7 ------- routes/web.php | 2 +- 7 files changed, 24 insertions(+), 23 deletions(-) diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 397bb2922..2fe22f1e1 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -250,12 +250,18 @@ class UserController extends Controller ]); } + /** + * Update the user's preferred book-list display setting. + * @param $id + * @param Request $request + * @return \Illuminate\Http\RedirectResponse + */ public function switchBookView($id, Request $request) { $this->checkPermissionOr('users-manage', function () use ($id) { return $this->currentUser->id == $id; }); - $viewType = $request->get('book_view_type'); + $viewType = $request->get('book_view_type'); if (!in_array($viewType, ['grid', 'list'])) { $viewType = 'list'; } @@ -263,13 +269,7 @@ class UserController extends Controller $user = $this->user->findOrFail($id); setting()->putUser($user, 'books_view_type', $viewType); - $previousUrl = url()->previous(); - if (empty($previousUrl)) { - // if no previous URL, redirect to settings - return redirect("/settings/users/$id"); - } else { - // redirect to the previous page. - return redirect($previousUrl); - } + return redirect()->back(302, [], "/settings/users/$id"); } + } diff --git a/resources/lang/en/common.php b/resources/lang/en/common.php index 7cdd7c23e..26f096327 100644 --- a/resources/lang/en/common.php +++ b/resources/lang/en/common.php @@ -49,6 +49,8 @@ return [ 'toggle_details' => 'Toggle Details', 'toggle_thumbnails' => 'Toggle Thumbnails', 'details' => 'Details', + 'grid_view' => 'Grid View', + 'list_view' => 'List View', /** * Header diff --git a/resources/lang/en/entities.php b/resources/lang/en/entities.php index 6e481d03b..4dc5ccc38 100644 --- a/resources/lang/en/entities.php +++ b/resources/lang/en/entities.php @@ -99,7 +99,6 @@ return [ 'books_sort_named' => 'Sort Book :bookName', 'books_sort_show_other' => 'Show Other Books', 'books_sort_save' => 'Save New Order', - 'books_toggle_view' => 'Toggle Book View', /** * Chapters diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php index f35c486ad..f3e26fb45 100755 --- a/resources/lang/en/settings.php +++ b/resources/lang/en/settings.php @@ -96,7 +96,6 @@ return [ 'users_external_auth_id' => 'External Authentication ID', 'users_password_warning' => 'Only fill the below if you would like to change your password:', 'users_system_public' => 'This user represents any guest users that visit your instance. It cannot be used to log in but is assigned automatically.', - 'users_books_view_type' => 'Preferred layout for books viewing', 'users_delete' => 'Delete User', 'users_delete_named' => 'Delete user :userName', 'users_delete_warning' => 'This will fully delete this user with the name \':userName\' from the system.', diff --git a/resources/views/books/index.blade.php b/resources/views/books/index.blade.php index 5ca01a550..2ab819327 100644 --- a/resources/views/books/index.blade.php +++ b/resources/views/books/index.blade.php @@ -1,14 +1,22 @@ @extends('sidebar-layout') @section('toolbar') -
-
-
+
+
id}/switch-book-view") }}" method="POST" class="inline"> {!! csrf_field() !!} + {!! method_field('PATCH') !!} - + @if ($booksViewType === 'list') + + @else + + @endif
+
+
+
+
@if($currentUser->can('book-create-all')) {{ trans('entities.books_create') }} @endif diff --git a/resources/views/users/edit.blade.php b/resources/views/users/edit.blade.php index 14ab19e0e..fc75593b8 100644 --- a/resources/views/users/edit.blade.php +++ b/resources/views/users/edit.blade.php @@ -43,13 +43,6 @@ @endforeach
-
- - -
diff --git a/routes/web.php b/routes/web.php index 31de09737..5ddb3fb94 100644 --- a/routes/web.php +++ b/routes/web.php @@ -146,7 +146,7 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/users', 'UserController@index'); Route::get('/users/create', 'UserController@create'); Route::get('/users/{id}/delete', 'UserController@delete'); - Route::post('/users/{id}/switch-book-view', 'UserController@switchBookView'); + Route::patch('/users/{id}/switch-book-view', 'UserController@switchBookView'); Route::post('/users/create', 'UserController@store'); Route::get('/users/{id}', 'UserController@edit'); Route::put('/users/{id}', 'UserController@update'); From 359b1b40a2b62046109db7794e393de3d1d9be4f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 30 Dec 2017 15:50:33 +0000 Subject: [PATCH 09/16] Fixed broken table/ol/ul page includes Fixes #640 --- app/Repos/EntityRepo.php | 10 ++++++++-- tests/Entity/PageContentTest.php | 21 +++++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php index c31ddfefe..24c680234 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -690,6 +690,7 @@ class EntityRepo preg_match_all("/{{@\s?([0-9].*?)}}/", $content, $matches); if (count($matches[0]) === 0) return $content; + $topLevelTags = ['table', 'ul', 'ol']; foreach ($matches[1] as $index => $includeId) { $splitInclude = explode('#', $includeId, 2); $pageId = intval($splitInclude[0]); @@ -714,8 +715,13 @@ class EntityRepo continue; } $innerContent = ''; - foreach ($matchingElem->childNodes as $childNode) { - $innerContent .= $doc->saveHTML($childNode); + $isTopLevel = in_array(strtolower($matchingElem->nodeName), $topLevelTags); + if ($isTopLevel) { + $innerContent .= $doc->saveHTML($matchingElem); + } else { + foreach ($matchingElem->childNodes as $childNode) { + $innerContent .= $doc->saveHTML($childNode); + } } $content = str_replace($matches[0][$index], trim($innerContent), $content); } diff --git a/tests/Entity/PageContentTest.php b/tests/Entity/PageContentTest.php index cd6526aec..370514788 100644 --- a/tests/Entity/PageContentTest.php +++ b/tests/Entity/PageContentTest.php @@ -9,7 +9,7 @@ class PageContentTest extends TestCase public function test_page_includes() { $page = Page::first(); - $secondPage = Page::all()->get(2); + $secondPage = Page::where('id', '!=', $page->id)->first(); $secondPage->html = "

Hello, This is a test

This is a second block of content

"; $secondPage->save(); @@ -38,7 +38,7 @@ class PageContentTest extends TestCase public function test_saving_page_with_includes() { $page = Page::first(); - $secondPage = Page::all()->get(2); + $secondPage = Page::where('id', '!=', $page->id)->first(); $this->asEditor(); $page->html = "

{{@$secondPage->id}}

"; @@ -50,6 +50,23 @@ class PageContentTest extends TestCase $this->assertContains("{{@$secondPage->id}}", $page->html); } + public function test_page_includes_do_not_break_tables() + { + $page = Page::first(); + $secondPage = Page::where('id', '!=', $page->id)->first(); + + $content = '
test
'; + $secondPage->html = $content; + $secondPage->save(); + + $page->html = "{{@{$secondPage->id}#table}}"; + $page->save(); + + $this->asEditor(); + $pageResp = $this->get($page->getUrl()); + $pageResp->assertSee($content); + } + public function test_page_revision_views_viewable() { $this->asEditor(); From 96b8c403a8783b514abc280afd96cd3c06e900c2 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 30 Dec 2017 16:09:27 +0000 Subject: [PATCH 10/16] Fixed failing book view test Also ensured setting system localcache is cleared correctly --- app/Services/SettingService.php | 3 +++ tests/Entity/EntityTest.php | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/Services/SettingService.php b/app/Services/SettingService.php index 18a7c0d1b..ce87f5a4b 100644 --- a/app/Services/SettingService.php +++ b/app/Services/SettingService.php @@ -98,6 +98,9 @@ class SettingService { $cacheKey = $this->cachePrefix . $key; $this->cache->forget($cacheKey); + if (isset($this->localCache[$key])) { + unset($this->localCache[$key]); + } } /** diff --git a/tests/Entity/EntityTest.php b/tests/Entity/EntityTest.php index 549253417..4d4e0e6cd 100644 --- a/tests/Entity/EntityTest.php +++ b/tests/Entity/EntityTest.php @@ -90,14 +90,14 @@ class EntityTest extends BrowserKitTest $this->actingAs($editor) ->visit('/books') ->pageHasElement('.featured-image-container') - ->submitForm('Toggle Book View') + ->submitForm('List View') // Check redirection. ->seePageIs('/books') ->pageNotHasElement('.featured-image-container'); $this->actingAs($editor) ->visit('/books') - ->submitForm('Toggle Book View') + ->submitForm('Grid View') ->seePageIs('/books') ->pageHasElement('.featured-image-container'); From 4a24d1c31b7d244cb15eced098c5f8c3954f7fda Mon Sep 17 00:00:00 2001 From: Abijeet Date: Sun, 31 Dec 2017 16:25:21 +0530 Subject: [PATCH 11/16] Checks the target and the source book before performing the sort. Signed-off-by: Abijeet --- app/Http/Controllers/BookController.php | 27 +++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index aa8f89ea4..c042c502b 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -195,20 +195,43 @@ class BookController extends Controller $sortMap = json_decode($request->get('sort-tree')); $defaultBookId = $book->id; + $permissionsList = [$book->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->entityRepo->exists('book', $bookChild->book) ? intval($bookChild->book) : $defaultBookId; + $bookId = $defaultBookId; + $targetBook = $this->entityRepo->getById('book', $bookChild->book); + + // Check permission for target book + if (!empty($targetBook)) { + $bookId = $targetBook->id; + if (!in_array($bookId, $permissionsList)) { + $this->checkOwnablePermission('book-update', $targetBook); + // cache the permission for future use. + $permissionsList[] = $bookId; + } + } + $chapterId = ($isPage && $bookChild->parentChapter === false) ? 0 : intval($bookChild->parentChapter); $model = $this->entityRepo->getById($isPage?'page':'chapter', $id); + // Check permissions for the source book + $sourceBook = $model->book; + if (!in_array($sourceBook->id, $permissionsList)) { + $this->checkOwnablePermission('book-update', $sourceBook); + $permissionsList[] = $sourceBook->id; + } + // 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)) { $this->entityRepo->changeBook($isPage?'page':'chapter', $bookId, $model); $model->priority = $priority; - if ($isPage) $model->chapter_id = $chapterId; + if ($isPage) { + $model->chapter_id = $chapterId; + } $model->save(); $updatedModels->push($model); } From e13e71cbe06f42fae43d5389ba49f86315add44b Mon Sep 17 00:00:00 2001 From: Abijeet Date: Sun, 31 Dec 2017 16:44:46 +0530 Subject: [PATCH 12/16] Changed the sort view to only show books to which we have an update permission. Signed-off-by: Abijeet --- app/Http/Controllers/BookController.php | 6 ++---- app/Repos/EntityRepo.php | 11 ++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index c042c502b..700f7a06f 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -155,7 +155,7 @@ class BookController extends Controller $book = $this->entityRepo->getBySlug('book', $bookSlug); $this->checkOwnablePermission('book-update', $book); $bookChildren = $this->entityRepo->getBookChildren($book, true); - $books = $this->entityRepo->getAll('book', false); + $books = $this->entityRepo->getAll('book', false, 'update'); $this->setPageTitle(trans('entities.books_sort_named', ['bookName'=>$book->getShortName()])); return view('books/sort', ['book' => $book, 'current' => $book, 'books' => $books, 'bookChildren' => $bookChildren]); } @@ -229,9 +229,7 @@ class BookController extends Controller if ($model->priority !== $priority || $model->book_id !== $bookId || ($isPage && $model->chapter_id !== $chapterId)) { $this->entityRepo->changeBook($isPage?'page':'chapter', $bookId, $model); $model->priority = $priority; - if ($isPage) { - $model->chapter_id = $chapterId; - } + if ($isPage) $model->chapter_id = $chapterId; $model->save(); $updatedModels->push($model); } diff --git a/app/Repos/EntityRepo.php b/app/Repos/EntityRepo.php index 24c680234..2c92e1907 100644 --- a/app/Repos/EntityRepo.php +++ b/app/Repos/EntityRepo.php @@ -113,9 +113,9 @@ class EntityRepo * @param bool $allowDrafts * @return \Illuminate\Database\Query\Builder */ - protected function entityQuery($type, $allowDrafts = false) + protected function entityQuery($type, $allowDrafts = false, $permission = 'view') { - $q = $this->permissionService->enforceEntityRestrictions($type, $this->getEntity($type), 'view'); + $q = $this->permissionService->enforceEntityRestrictions($type, $this->getEntity($type), $permission); if (strtolower($type) === 'page' && !$allowDrafts) { $q = $q->where('draft', '=', false); } @@ -196,14 +196,15 @@ class EntityRepo } /** - * Get all entities of a type limited by count unless count if false. + * Get all entities of a type with the given permission, limited by count unless count is false. * @param string $type * @param integer|bool $count + * @param string $permission * @return Collection */ - public function getAll($type, $count = 20) + public function getAll($type, $count = 20, $permission = 'view') { - $q = $this->entityQuery($type)->orderBy('name', 'asc'); + $q = $this->entityQuery($type, false, $permission)->orderBy('name', 'asc'); if ($count !== false) $q = $q->take($count); return $q->get(); } From e269cc7ea71ce385fab112dcd7502fa3936582dc Mon Sep 17 00:00:00 2001 From: Abijeet Date: Sun, 31 Dec 2017 20:17:08 +0530 Subject: [PATCH 13/16] Adds test case for sorting permissions. Signed-off-by: Abijeet --- tests/BrowserKitTest.php | 1 - tests/Permissions/RestrictionsTest.php | 67 ++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/BrowserKitTest.php b/tests/BrowserKitTest.php index 1eabc7417..d5c9911f8 100644 --- a/tests/BrowserKitTest.php +++ b/tests/BrowserKitTest.php @@ -3,7 +3,6 @@ use BookStack\Entity; use BookStack\Role; use BookStack\Services\PermissionService; -use BookStack\User; use Illuminate\Contracts\Console\Kernel; use Illuminate\Foundation\Testing\DatabaseTransactions; use Laravel\BrowserKitTesting\TestCase; diff --git a/tests/Permissions/RestrictionsTest.php b/tests/Permissions/RestrictionsTest.php index 218b7a0d8..8f37b2517 100644 --- a/tests/Permissions/RestrictionsTest.php +++ b/tests/Permissions/RestrictionsTest.php @@ -3,6 +3,7 @@ use BookStack\Book; use BookStack\Services\PermissionService; use BookStack\User; +use BookStack\Repos\EntityRepo; class RestrictionsTest extends BrowserKitTest { @@ -554,4 +555,70 @@ class RestrictionsTest extends BrowserKitTest $this->dontSee(substr($bookChapter->name, 0, 15)); } + public function test_book_sort_view_permission() + { + $firstBook = Book::first(); + $secondBook = Book::find(2); + $thirdBook = Book::find(3); + + $this->setEntityRestrictions($firstBook, ['view', 'update']); + $this->setEntityRestrictions($secondBook, ['view']); + $this->setEntityRestrictions($thirdBook, ['view', 'update']); + + // Test sort page visibility + $this->actingAs($this->user)->visit($secondBook->getUrl() . '/sort') + ->see('You do not have permission') + ->seePageIs('/'); + + // Check sort page on first book + $this->actingAs($this->user)->visit($firstBook->getUrl() . '/sort') + ->see($thirdBook->name) + ->dontSee($secondBook->name); + } + + public function test_book_sort_permission() { + $firstBook = Book::first(); + $secondBook = Book::find(2); + + $this->setEntityRestrictions($firstBook, ['view', 'update']); + $this->setEntityRestrictions($secondBook, ['view']); + + $firstBookChapter = $this->app[EntityRepo::class]->createFromInput('chapter', + ['name' => 'first book chapter'], $firstBook); + $secondBookChapter = $this->app[EntityRepo::class]->createFromInput('chapter', + ['name' => 'second book chapter'], $secondBook); + + // Create request data + $reqData = [ + [ + 'id' => $firstBookChapter->id, + 'sort' => 0, + 'parentChapter' => false, + 'type' => 'chapter', + 'book' => $secondBook->id + ] + ]; + + // Move chapter from first book to a second book + $this->actingAs($this->user)->put($firstBook->getUrl() . '/sort', ['sort-tree' => json_encode($reqData)]) + ->followRedirects() + ->see('You do not have permission') + ->seePageIs('/'); + + $reqData = [ + [ + 'id' => $secondBookChapter->id, + 'sort' => 0, + 'parentChapter' => false, + 'type' => 'chapter', + 'book' => $firstBook->id + ] + ]; + + // Move chapter from second book to first book + $this->actingAs($this->user)->put($firstBook->getUrl() . '/sort', ['sort-tree' => json_encode($reqData)]) + ->followRedirects() + ->see('You do not have permission') + ->seePageIs('/'); + } } From 6988a6ff88a939a1161cb809f8157af4827c4d70 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 31 Dec 2017 16:25:58 +0000 Subject: [PATCH 14/16] Added view override support Relates to #652 --- config/view.php | 9 ++++++--- themes/.gitignore | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) create mode 100755 themes/.gitignore diff --git a/config/view.php b/config/view.php index e193ab61d..8dc2841e7 100644 --- a/config/view.php +++ b/config/view.php @@ -1,5 +1,10 @@ [ - realpath(base_path('resources/views')), - ], + 'paths' => $viewPaths, /* |-------------------------------------------------------------------------- diff --git a/themes/.gitignore b/themes/.gitignore new file mode 100755 index 000000000..d6b7ef32c --- /dev/null +++ b/themes/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore From a77756a2dad8a0766a7220d6c6d085f321233d5f Mon Sep 17 00:00:00 2001 From: Abijeet Date: Sat, 6 Jan 2018 01:04:48 +0530 Subject: [PATCH 15/16] Refactored the code to first check for the permissions before sorting the book. Signed-off-by: Abijeet --- app/Http/Controllers/BookController.php | 42 +++++++++++++++---------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index 700f7a06f..d70f9d0df 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -195,7 +195,32 @@ class BookController extends Controller $sortMap = json_decode($request->get('sort-tree')); $defaultBookId = $book->id; + // Check permissions for all target and source books $permissionsList = [$book->id]; + foreach ($sortMap as $bookChild) { + // Check permission for target book + if (!in_array($bookChild->book, $permissionsList)) { + $targetBook = $this->entityRepo->getById('book', $bookChild->book); + if (!empty($targetBook)) { + $bookId = $targetBook->id; + $this->checkOwnablePermission('book-update', $targetBook); + // cache the permission for future use. + $permissionsList[] = $bookId; + } + } + + // Check permissions for the source book + $id = intval($bookChild->id); + $isPage = $bookChild->type == 'page'; + $model = $this->entityRepo->getById($isPage?'page':'chapter', $id); + $sourceBook = $model->book; + if (!in_array($sourceBook->id, $permissionsList)) { + $this->checkOwnablePermission('book-update', $sourceBook); + + // cache the permission for future use. + $permissionsList[] = $sourceBook->id; + } + } // Loop through contents of provided map and update entities accordingly foreach ($sortMap as $bookChild) { @@ -205,26 +230,9 @@ class BookController extends Controller $bookId = $defaultBookId; $targetBook = $this->entityRepo->getById('book', $bookChild->book); - // Check permission for target book - if (!empty($targetBook)) { - $bookId = $targetBook->id; - if (!in_array($bookId, $permissionsList)) { - $this->checkOwnablePermission('book-update', $targetBook); - // cache the permission for future use. - $permissionsList[] = $bookId; - } - } - $chapterId = ($isPage && $bookChild->parentChapter === false) ? 0 : intval($bookChild->parentChapter); $model = $this->entityRepo->getById($isPage?'page':'chapter', $id); - // Check permissions for the source book - $sourceBook = $model->book; - if (!in_array($sourceBook->id, $permissionsList)) { - $this->checkOwnablePermission('book-update', $sourceBook); - $permissionsList[] = $sourceBook->id; - } - // 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)) { $this->entityRepo->changeBook($isPage?'page':'chapter', $bookId, $model); From 281da59bae04068965213c74ff91d7f92749fe34 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sat, 13 Jan 2018 16:44:47 +0000 Subject: [PATCH 16/16] Refactored book sort using collections --- app/Http/Controllers/BookController.php | 95 +++++++++++-------------- 1 file changed, 40 insertions(+), 55 deletions(-) diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php index d70f9d0df..f1645bb4b 100644 --- a/app/Http/Controllers/BookController.php +++ b/app/Http/Controllers/BookController.php @@ -190,71 +190,56 @@ class BookController extends Controller } // Sort pages and chapters - $sortedBooks = []; - $updatedModels = collect(); - $sortMap = json_decode($request->get('sort-tree')); - $defaultBookId = $book->id; + $sortMap = collect(json_decode($request->get('sort-tree'))); + $bookIdsInvolved = collect([$book->id]); - // Check permissions for all target and source books - $permissionsList = [$book->id]; - foreach ($sortMap as $bookChild) { - // Check permission for target book - if (!in_array($bookChild->book, $permissionsList)) { - $targetBook = $this->entityRepo->getById('book', $bookChild->book); - if (!empty($targetBook)) { - $bookId = $targetBook->id; - $this->checkOwnablePermission('book-update', $targetBook); - // cache the permission for future use. - $permissionsList[] = $bookId; - } - } + // Load models into map + $sortMap->each(function($mapItem) use ($bookIdsInvolved) { + $mapItem->type = ($mapItem->type === 'page' ? 'page' : 'chapter'); + $mapItem->model = $this->entityRepo->getById($mapItem->type, $mapItem->id); + // Store source and target books + $bookIdsInvolved->push(intval($mapItem->model->book_id)); + $bookIdsInvolved->push(intval($mapItem->book)); + }); - // Check permissions for the source book - $id = intval($bookChild->id); - $isPage = $bookChild->type == 'page'; - $model = $this->entityRepo->getById($isPage?'page':'chapter', $id); - $sourceBook = $model->book; - if (!in_array($sourceBook->id, $permissionsList)) { - $this->checkOwnablePermission('book-update', $sourceBook); - - // cache the permission for future use. - $permissionsList[] = $sourceBook->id; - } + // Get the books involved in the sort + $bookIdsInvolved = $bookIdsInvolved->unique()->toArray(); + $booksInvolved = $this->entityRepo->book->newQuery()->whereIn('id', $bookIdsInvolved)->get(); + // 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); + }); - // 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 = $defaultBookId; - $targetBook = $this->entityRepo->getById('book', $bookChild->book); + // Perform the sort + $sortMap->each(function($mapItem) { + $model = $mapItem->model; - $chapterId = ($isPage && $bookChild->parentChapter === false) ? 0 : intval($bookChild->parentChapter); - $model = $this->entityRepo->getById($isPage?'page':'chapter', $id); + $priorityChanged = intval($model->priority) !== intval($mapItem->sort); + $bookChanged = intval($model->book_id) !== intval($mapItem->book); + $chapterChanged = ($mapItem->type === 'page') && intval($model->chapter_id) !== $mapItem->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)) { - $this->entityRepo->changeBook($isPage?'page':'chapter', $bookId, $model); - $model->priority = $priority; - if ($isPage) $model->chapter_id = $chapterId; + if ($bookChanged) { + $this->entityRepo->changeBook($mapItem->type, $mapItem->book, $model); + } + if ($chapterChanged) { + $model->chapter_id = intval($mapItem->parentChapter); $model->save(); - $updatedModels->push($model); } - - // Store involved books to be sorted later - if (!in_array($bookId, $sortedBooks)) { - $sortedBooks[] = $bookId; + if ($priorityChanged) { + $model->priority = intval($mapItem->sort); + $model->save(); } - } + }); - // Add activity for books - foreach ($sortedBooks as $bookId) { - /** @var Book $updatedBook */ - $updatedBook = $this->entityRepo->getById('book', $bookId); - $this->entityRepo->buildJointPermissionsForBook($updatedBook); - Activity::add($updatedBook, 'book_sort', $updatedBook->id); - } + // Rebuild permissions and add activity for involved books. + $booksInvolved->each(function(Book $book) { + $this->entityRepo->buildJointPermissionsForBook($book); + Activity::add($book, 'book_sort', $book->id); + }); return redirect($book->getUrl()); }