From 053cbbd5b6cd98788ae4f0c56fc220ae6b020736 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 10 Apr 2020 12:49:16 +0100 Subject: [PATCH] Updated view-change endpoints to be clearer, separated books and shelf - Separated books-list and shelf-show view types to be saved separately. During review of #1755 --- app/Http/Controllers/BookshelfController.php | 2 +- app/Http/Controllers/UserController.php | 65 +++++-------------- resources/views/books/index.blade.php | 2 +- resources/views/common/home-book.blade.php | 2 +- resources/views/common/home-shelves.blade.php | 2 +- resources/views/shelves/index.blade.php | 2 +- resources/views/shelves/show.blade.php | 2 +- routes/web.php | 3 +- tests/User/UserProfileTest.php | 35 ++++++++-- 9 files changed, 54 insertions(+), 61 deletions(-) diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php index c46cac277..f2cc11c7b 100644 --- a/app/Http/Controllers/BookshelfController.php +++ b/app/Http/Controllers/BookshelfController.php @@ -103,11 +103,11 @@ class BookshelfController extends Controller public function show(string $slug) { $shelf = $this->bookshelfRepo->getBySlug($slug); - $view = setting()->getForCurrentUser('books_view_type', config('app.views.books')); $this->checkOwnablePermission('book-view', $shelf); Views::add($shelf); $this->entityContextManager->setShelfContext($shelf->id); + $view = setting()->getForCurrentUser('bookshelf_view_type', config('app.views.books')); $this->setPageTitle($shelf->getShortName()); return view('shelves.show', [ diff --git a/app/Http/Controllers/UserController.php b/app/Http/Controllers/UserController.php index 55a4610bc..43cbad1fb 100644 --- a/app/Http/Controllers/UserController.php +++ b/app/Http/Controllers/UserController.php @@ -7,7 +7,6 @@ use BookStack\Auth\UserRepo; use BookStack\Exceptions\UserUpdateException; use BookStack\Uploads\ImageRepo; use Illuminate\Http\Request; -use Illuminate\Http\Response; use Illuminate\Support\Str; class UserController extends Controller @@ -20,10 +19,6 @@ class UserController extends Controller /** * UserController constructor. - * @param User $user - * @param UserRepo $userRepo - * @param UserInviteService $inviteService - * @param ImageRepo $imageRepo */ public function __construct(User $user, UserRepo $userRepo, UserInviteService $inviteService, ImageRepo $imageRepo) { @@ -36,8 +31,6 @@ class UserController extends Controller /** * Display a listing of the users. - * @param Request $request - * @return Response */ public function index(Request $request) { @@ -55,7 +48,6 @@ class UserController extends Controller /** * Show the form for creating a new user. - * @return Response */ public function create() { @@ -67,9 +59,8 @@ class UserController extends Controller /** * Store a newly created user in storage. - * @param Request $request - * @return Response * @throws UserUpdateException + * @throws \Illuminate\Validation\ValidationException */ public function store(Request $request) { @@ -138,13 +129,11 @@ class UserController extends Controller /** * Update the specified user in storage. - * @param Request $request - * @param int $id - * @return Response * @throws UserUpdateException * @throws \BookStack\Exceptions\ImageUploadException + * @throws \Illuminate\Validation\ValidationException */ - public function update(Request $request, $id) + public function update(Request $request, int $id) { $this->preventAccessInDemoMode(); $this->checkPermissionOrCurrentUser('users-manage', $id); @@ -212,10 +201,8 @@ class UserController extends Controller /** * Show the user delete page. - * @param int $id - * @return \Illuminate\View\View */ - public function delete($id) + public function delete(int $id) { $this->checkPermissionOrCurrentUser('users-manage', $id); @@ -226,11 +213,9 @@ class UserController extends Controller /** * Remove the specified user from storage. - * @param int $id - * @return Response * @throws \Exception */ - public function destroy($id) + public function destroy(int $id) { $this->preventAccessInDemoMode(); $this->checkPermissionOrCurrentUser('users-manage', $id); @@ -255,8 +240,6 @@ class UserController extends Controller /** * Show the user profile page - * @param $id - * @return \Illuminate\Contracts\View\Factory|\Illuminate\View\View */ public function showProfilePage($id) { @@ -276,34 +259,32 @@ class UserController extends Controller /** * Update the user's preferred book-list display setting. - * @param Request $request - * @param $id - * @return \Illuminate\Http\RedirectResponse */ - public function switchBookView(Request $request, $id) + public function switchBooksView(Request $request, int $id) { return $this->switchViewType($id, $request, 'books'); } /** * Update the user's preferred shelf-list display setting. - * @param Request $request - * @param $id - * @return \Illuminate\Http\RedirectResponse */ - public function switchShelfView(Request $request, $id) + public function switchShelvesView(Request $request, int $id) { return $this->switchViewType($id, $request, 'bookshelves'); } /** - * For a type of list, switch with stored view type for a user. - * @param integer $userId - * @param Request $request - * @param string $listName - * @return \Illuminate\Http\RedirectResponse + * Update the user's preferred shelf-view book list display setting. */ - protected function switchViewType($userId, Request $request, string $listName) + public function switchShelfView(Request $request, int $id) + { + return $this->switchViewType($id, $request, 'bookshelf'); + } + + /** + * For a type of list, switch with stored view type for a user. + */ + protected function switchViewType(int $userId, Request $request, string $listName) { $this->checkPermissionOrCurrentUser('users-manage', $userId); @@ -321,10 +302,6 @@ class UserController extends Controller /** * Change the stored sort type for a particular view. - * @param Request $request - * @param string $id - * @param string $type - * @return \Illuminate\Http\RedirectResponse */ public function changeSort(Request $request, string $id, string $type) { @@ -337,10 +314,6 @@ class UserController extends Controller /** * Update the stored section expansion preference for the given user. - * @param Request $request - * @param string $id - * @param string $key - * @return \Illuminate\Contracts\Routing\ResponseFactory|\Symfony\Component\HttpFoundation\Response */ public function updateExpansionPreference(Request $request, string $id, string $key) { @@ -359,10 +332,6 @@ class UserController extends Controller /** * Changed the stored preference for a list sort order. - * @param int $userId - * @param Request $request - * @param string $listName - * @return \Illuminate\Http\RedirectResponse */ protected function changeListSort(int $userId, Request $request, string $listName) { diff --git a/resources/views/books/index.blade.php b/resources/views/books/index.blade.php index b9bd987a9..f3c3ee34b 100644 --- a/resources/views/books/index.blade.php +++ b/resources/views/books/index.blade.php @@ -43,7 +43,7 @@ @endif - @include('partials.view-toggle', ['view' => $view, 'type' => 'book']) + @include('partials.view-toggle', ['view' => $view, 'type' => 'books']) diff --git a/resources/views/common/home-book.blade.php b/resources/views/common/home-book.blade.php index 7644eeb88..8caae814a 100644 --- a/resources/views/common/home-book.blade.php +++ b/resources/views/common/home-book.blade.php @@ -12,7 +12,7 @@
{{ trans('common.actions') }}
- @include('partials.view-toggle', ['view' => $view, 'type' => 'book']) + @include('partials.view-toggle', ['view' => $view, 'type' => 'books']) @include('components.expand-toggle', ['target' => '.entity-list.compact .entity-item-snippet', 'key' => 'home-details'])
diff --git a/resources/views/common/home-shelves.blade.php b/resources/views/common/home-shelves.blade.php index a9c585893..bac6fa154 100644 --- a/resources/views/common/home-shelves.blade.php +++ b/resources/views/common/home-shelves.blade.php @@ -12,7 +12,7 @@
{{ trans('common.actions') }}
- @include('partials.view-toggle', ['view' => $view, 'type' => 'shelf']) + @include('partials.view-toggle', ['view' => $view, 'type' => 'shelves']) @include('components.expand-toggle', ['target' => '.entity-list.compact .entity-item-snippet', 'key' => 'home-details'])
diff --git a/resources/views/shelves/index.blade.php b/resources/views/shelves/index.blade.php index 98f97f133..56b76f96f 100644 --- a/resources/views/shelves/index.blade.php +++ b/resources/views/shelves/index.blade.php @@ -15,7 +15,7 @@ {{ trans('entities.shelves_new_action') }} @endif - @include('partials.view-toggle', ['view' => $view, 'type' => 'shelf']) + @include('partials.view-toggle', ['view' => $view, 'type' => 'shelves']) diff --git a/resources/views/shelves/show.blade.php b/resources/views/shelves/show.blade.php index 56df61c91..6fee6f45d 100644 --- a/resources/views/shelves/show.blade.php +++ b/resources/views/shelves/show.blade.php @@ -95,7 +95,7 @@ @endif - @include('partials.view-toggle', ['view' => $view, 'type' => 'book']) + @include('partials.view-toggle', ['view' => $view, 'type' => 'shelf'])
diff --git a/routes/web.php b/routes/web.php index 90261e1ac..4dfccdf36 100644 --- a/routes/web.php +++ b/routes/web.php @@ -178,7 +178,8 @@ Route::group(['middleware' => 'auth'], function () { Route::get('/users', 'UserController@index'); Route::get('/users/create', 'UserController@create'); Route::get('/users/{id}/delete', 'UserController@delete'); - Route::patch('/users/{id}/switch-book-view', 'UserController@switchBookView'); + Route::patch('/users/{id}/switch-books-view', 'UserController@switchBooksView'); + Route::patch('/users/{id}/switch-shelves-view', 'UserController@switchShelvesView'); Route::patch('/users/{id}/switch-shelf-view', 'UserController@switchShelfView'); Route::patch('/users/{id}/change-sort/{type}', 'UserController@changeSort'); Route::patch('/users/{id}/update-expansion-preference/{key}', 'UserController@updateExpansionPreference'); diff --git a/tests/User/UserProfileTest.php b/tests/User/UserProfileTest.php index 979064baa..0a3a1a6b2 100644 --- a/tests/User/UserProfileTest.php +++ b/tests/User/UserProfileTest.php @@ -1,5 +1,8 @@ user = \BookStack\Auth\User::all()->last(); + $this->user = User::all()->last(); } public function test_profile_page_shows_name() @@ -57,8 +60,8 @@ class UserProfileTest extends BrowserKitTest $newUser = $this->getNewBlankUser(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); - \Activity::add($entities['book'], 'book_update', $entities['book']->id); - \Activity::add($entities['page'], 'page_create', $entities['book']->id); + Activity::add($entities['book'], 'book_update', $entities['book']->id); + Activity::add($entities['page'], 'page_create', $entities['book']->id); $this->asAdmin()->visit('/user/' . $newUser->id) ->seeInElement('#recent-user-activity', 'updated book') @@ -71,8 +74,8 @@ class UserProfileTest extends BrowserKitTest $newUser = $this->getNewBlankUser(); $this->actingAs($newUser); $entities = $this->createEntityChainBelongingToUser($newUser, $newUser); - \Activity::add($entities['book'], 'book_update', $entities['book']->id); - \Activity::add($entities['page'], 'page_create', $entities['book']->id); + Activity::add($entities['book'], 'book_update', $entities['book']->id); + Activity::add($entities['page'], 'page_create', $entities['book']->id); $this->asAdmin()->visit('/')->clickInElement('#recent-activity', $newUser->name) ->seePageIs('/user/' . $newUser->id) @@ -89,7 +92,7 @@ class UserProfileTest extends BrowserKitTest public function test_guest_profile_cannot_be_deleted() { - $guestUser = \BookStack\Auth\User::getDefault(); + $guestUser = User::getDefault(); $this->asAdmin()->visit('/settings/users/' . $guestUser->id . '/delete') ->see('Delete User')->see('Guest') ->press('Confirm') @@ -118,4 +121,24 @@ class UserProfileTest extends BrowserKitTest ->pageHasElement('.featured-image-container'); } + public function test_shelf_view_type_change() + { + $editor = $this->getEditor(); + $shelf = Bookshelf::query()->first(); + setting()->putUser($editor, 'bookshelf_view_type', 'list'); + + $this->actingAs($editor)->visit($shelf->getUrl()) + ->pageNotHasElement('.featured-image-container') + ->pageHasElement('.content-wrap .entity-list-item') + ->see('Grid View'); + + $req = $this->patch("/settings/users/{$editor->id}/switch-shelf-view", ['view_type' => 'grid']); + $req->assertRedirectedTo($shelf->getUrl()); + + $this->actingAs($editor)->visit($shelf->getUrl()) + ->pageHasElement('.featured-image-container') + ->pageNotHasElement('.content-wrap .entity-list-item') + ->see('List View'); + } + }