From a3fcc98d6ec2597a212b569898437d3d2d9421bd Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Wed, 9 Nov 2022 19:30:08 +0000
Subject: [PATCH] Aligned user preference endpoints in style and behaviour
Changes their endpoints and remove the user id from the URLs.
Simplifies list changes to share a single endpoint, which aligns it to
the behaviour of the existing sort preference endpoint.
Also added test to ensure user preferences are deleted on user delete.
---
app/Auth/UserRepo.php | 3 +
.../Controllers/UserPreferencesController.php | 78 ++++++-------------
app/Settings/SettingService.php | 12 +++
resources/js/components/code-editor.js | 2 +-
resources/js/components/shortcuts.js | 1 -
.../views/common/dark-mode-toggle.blade.php | 2 +-
resources/views/common/sort.blade.php | 2 +-
.../views/entities/view-toggle.blade.php | 8 +-
.../views/home/parts/expand-toggle.blade.php | 2 +-
resources/views/home/shelves.blade.php | 2 +-
resources/views/shelves/index.blade.php | 2 +-
resources/views/shelves/show.blade.php | 2 +-
routes/web.php | 12 ++-
tests/Entity/BookTest.php | 12 +--
tests/User/UserManagementTest.php | 17 ++++
tests/User/UserPreferencesTest.php | 20 ++---
16 files changed, 89 insertions(+), 88 deletions(-)
diff --git a/app/Auth/UserRepo.php b/app/Auth/UserRepo.php
index c589fd964..78bcb978e 100644
--- a/app/Auth/UserRepo.php
+++ b/app/Auth/UserRepo.php
@@ -158,6 +158,9 @@ class UserRepo
// Delete user profile images
$this->userAvatar->destroyAllForUser($user);
+ // Delete related activities
+ setting()->deleteUserSettings($user->id);
+
if (!empty($newOwnerId)) {
$newOwner = User::query()->find($newOwnerId);
if (!is_null($newOwner)) {
diff --git a/app/Http/Controllers/UserPreferencesController.php b/app/Http/Controllers/UserPreferencesController.php
index c42be0484..e5ac69818 100644
--- a/app/Http/Controllers/UserPreferencesController.php
+++ b/app/Http/Controllers/UserPreferencesController.php
@@ -38,8 +38,8 @@ class UserPreferencesController extends Controller
$providedShortcuts = $request->get('shortcut', []);
$shortcuts = new UserShortcutMap($providedShortcuts);
- setting()->putUser(user(), 'ui-shortcuts', $shortcuts->toJson());
- setting()->putUser(user(), 'ui-shortcuts-enabled', $enabled);
+ setting()->putForCurrentUser('ui-shortcuts', $shortcuts->toJson());
+ setting()->putForCurrentUser('ui-shortcuts-enabled', $enabled);
$this->showSuccessNotification('Shortcut preferences have been updated!');
@@ -47,70 +47,45 @@ class UserPreferencesController extends Controller
}
/**
- * Update the user's preferred book-list display setting.
+ * Update the preferred view format for a list view of the given type.
*/
- public function switchBooksView(Request $request, int $id)
+ public function changeView(Request $request, string $type)
{
- return $this->switchViewType($id, $request, 'books');
- }
-
- /**
- * Update the user's preferred shelf-list display setting.
- */
- public function switchShelvesView(Request $request, int $id)
- {
- return $this->switchViewType($id, $request, 'bookshelves');
- }
-
- /**
- * Update the user's preferred shelf-view book list display setting.
- */
- 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);
-
- $viewType = $request->get('view_type');
- if (!in_array($viewType, ['grid', 'list'])) {
- $viewType = 'list';
+ $valueViewTypes = ['books', 'bookshelves', 'bookshelf'];
+ if (!in_array($type, $valueViewTypes)) {
+ return redirect()->back(500);
}
- $user = $this->userRepo->getById($userId);
- $key = $listName . '_view_type';
- setting()->putUser($user, $key, $viewType);
+ $view = $request->get('view');
+ if (!in_array($view, ['grid', 'list'])) {
+ $view = 'list';
+ }
- return redirect()->back(302, [], "/settings/users/$userId");
+ $key = $type . '_view_type';
+ setting()->putForCurrentUser($key, $view);
+
+ return redirect()->back(302, [], "/");
}
/**
* Change the stored sort type for a particular view.
*/
- public function changeSort(Request $request, string $id, string $type)
+ public function changeSort(Request $request, string $type)
{
$validSortTypes = ['books', 'bookshelves', 'shelf_books', 'users', 'roles', 'webhooks', 'tags', 'page_revisions'];
if (!in_array($type, $validSortTypes)) {
return redirect()->back(500);
}
- $this->checkPermissionOrCurrentUser('users-manage', $id);
-
$sort = substr($request->get('sort') ?: 'name', 0, 50);
$order = $request->get('order') === 'desc' ? 'desc' : 'asc';
- $user = $this->userRepo->getById($id);
$sortKey = $type . '_sort';
$orderKey = $type . '_sort_order';
- setting()->putUser($user, $sortKey, $sort);
- setting()->putUser($user, $orderKey, $order);
+ setting()->putForCurrentUser($sortKey, $sort);
+ setting()->putForCurrentUser($orderKey, $order);
- return redirect()->back(302, [], "/settings/users/{$id}");
+ return redirect()->back(302, [], "/");
}
/**
@@ -119,7 +94,7 @@ class UserPreferencesController extends Controller
public function toggleDarkMode()
{
$enabled = setting()->getForCurrentUser('dark-mode-enabled', false);
- setting()->putUser(user(), 'dark-mode-enabled', $enabled ? 'false' : 'true');
+ setting()->putForCurrentUser('dark-mode-enabled', $enabled ? 'false' : 'true');
return redirect()->back();
}
@@ -127,18 +102,15 @@ class UserPreferencesController extends Controller
/**
* Update the stored section expansion preference for the given user.
*/
- public function updateExpansionPreference(Request $request, string $id, string $key)
+ public function changeExpansion(Request $request, string $type)
{
- $this->checkPermissionOrCurrentUser('users-manage', $id);
- $keyWhitelist = ['home-details'];
- if (!in_array($key, $keyWhitelist)) {
+ $typeWhitelist = ['home-details'];
+ if (!in_array($type, $typeWhitelist)) {
return response('Invalid key', 500);
}
$newState = $request->get('expand', 'false');
-
- $user = $this->userRepo->getById($id);
- setting()->putUser($user, 'section_expansion#' . $key, $newState);
+ setting()->putForCurrentUser('section_expansion#' . $type, $newState);
return response('', 204);
}
@@ -161,6 +133,6 @@ class UserPreferencesController extends Controller
array_splice($currentFavorites, $index, 1);
}
- setting()->putUser(user(), 'code-language-favourites', implode(',', $currentFavorites));
+ setting()->putForCurrentUser('code-language-favourites', implode(',', $currentFavorites));
}
}
diff --git a/app/Settings/SettingService.php b/app/Settings/SettingService.php
index f2c4c8305..9f0a41ea2 100644
--- a/app/Settings/SettingService.php
+++ b/app/Settings/SettingService.php
@@ -194,6 +194,8 @@ class SettingService
/**
* Put a user-specific setting into the database.
+ * Can only take string value types since this may use
+ * the session which is less flexible to data types.
*/
public function putUser(User $user, string $key, string $value): bool
{
@@ -206,6 +208,16 @@ class SettingService
return $this->put($this->userKey($user->id, $key), $value);
}
+ /**
+ * Put a user-specific setting into the database for the current access user.
+ * Can only take string value types since this may use
+ * the session which is less flexible to data types.
+ */
+ public function putForCurrentUser(string $key, string $value)
+ {
+ return $this->putUser(user(), $key, $value);
+ }
+
/**
* Convert a setting key into a user-specific key.
*/
diff --git a/resources/js/components/code-editor.js b/resources/js/components/code-editor.js
index 2d8031205..d0c6c432a 100644
--- a/resources/js/components/code-editor.js
+++ b/resources/js/components/code-editor.js
@@ -73,7 +73,7 @@ class CodeEditor {
isFavorite ? this.favourites.add(language) : this.favourites.delete(language);
button.setAttribute('data-favourite', isFavorite ? 'true' : 'false');
- window.$http.patch('/settings/users/update-code-language-favourite', {
+ window.$http.patch('/preferences/update-code-language-favourite', {
language: language,
active: isFavorite
});
diff --git a/resources/js/components/shortcuts.js b/resources/js/components/shortcuts.js
index cec8684c8..4efa3d42b 100644
--- a/resources/js/components/shortcuts.js
+++ b/resources/js/components/shortcuts.js
@@ -70,7 +70,6 @@ class Shortcuts {
*/
runShortcut(id) {
const el = this.container.querySelector(`[data-shortcut="${id}"]`);
- console.info('Shortcut run', el);
if (!el) {
return false;
}
diff --git a/resources/views/common/dark-mode-toggle.blade.php b/resources/views/common/dark-mode-toggle.blade.php
index 0812e487a..d6ecbc4d6 100644
--- a/resources/views/common/dark-mode-toggle.blade.php
+++ b/resources/views/common/dark-mode-toggle.blade.php
@@ -1,4 +1,4 @@
-<form action="{{ url('/settings/users/toggle-dark-mode') }}" method="post">
+<form action="{{ url('/preferences/toggle-dark-mode') }}" method="post">
{{ csrf_field() }}
{{ method_field('patch') }}
@if(setting()->getForCurrentUser('dark-mode-enabled'))
diff --git a/resources/views/common/sort.blade.php b/resources/views/common/sort.blade.php
index 996f7a837..29dfa60a1 100644
--- a/resources/views/common/sort.blade.php
+++ b/resources/views/common/sort.blade.php
@@ -9,7 +9,7 @@
action="{{ url()->current() }}"
method="get"
@else
- action="{{ url("/settings/users/". user()->id ."/change-sort/{$type}") }}"
+ action="{{ url("/preferences/change-sort/{$type}") }}"
method="post"
@endif
>
diff --git a/resources/views/entities/view-toggle.blade.php b/resources/views/entities/view-toggle.blade.php
index 9ff1b4927..e376b878d 100644
--- a/resources/views/entities/view-toggle.blade.php
+++ b/resources/views/entities/view-toggle.blade.php
@@ -1,15 +1,15 @@
<div>
- <form action="{{ url("/settings/users/". user()->id ."/switch-${type}-view") }}" method="POST" class="inline">
+ <form action="{{ url("/preferences/change-view/" . $type) }}" method="POST" class="inline">
{!! csrf_field() !!}
{!! method_field('PATCH') !!}
- <input type="hidden" value="{{ $view === 'list'? 'grid' : 'list' }}" name="view_type">
+
@if ($view === 'list')
- <button type="submit" class="icon-list-item text-primary">
+ <button type="submit" name="view" value="grid" class="icon-list-item text-primary">
<span class="icon">@icon('grid')</span>
<span>{{ trans('common.grid_view') }}</span>
</button>
@else
- <button type="submit" class="icon-list-item text-primary">
+ <button type="submit" name="view" value="list" class="icon-list-item text-primary">
<span>@icon('list')</span>
<span>{{ trans('common.list_view') }}</span>
</button>
diff --git a/resources/views/home/parts/expand-toggle.blade.php b/resources/views/home/parts/expand-toggle.blade.php
index 8ed7ff6e0..291e5db34 100644
--- a/resources/views/home/parts/expand-toggle.blade.php
+++ b/resources/views/home/parts/expand-toggle.blade.php
@@ -4,7 +4,7 @@ $key - Unique key for checking existing stored state.
--}}
<?php $isOpen = setting()->getForCurrentUser('section_expansion#'. $key); ?>
<button type="button" expand-toggle="{{ $target }}"
- expand-toggle-update-endpoint="{{ url('/settings/users/'. user()->id .'/update-expansion-preference/' . $key) }}"
+ expand-toggle-update-endpoint="{{ url('/preferences/change-expansion/' . $key) }}"
expand-toggle-is-open="{{ $isOpen ? 'yes' : 'no' }}"
class="icon-list-item {{ $classes ?? '' }}">
<span>@icon('expand-text')</span>
diff --git a/resources/views/home/shelves.blade.php b/resources/views/home/shelves.blade.php
index c525643b9..fc99b915f 100644
--- a/resources/views/home/shelves.blade.php
+++ b/resources/views/home/shelves.blade.php
@@ -18,7 +18,7 @@
<span>{{ trans('entities.shelves_new_action') }}</span>
</a>
@endif
- @include('entities.view-toggle', ['view' => $view, 'type' => 'shelves'])
+ @include('entities.view-toggle', ['view' => $view, 'type' => 'bookshelves'])
@include('home.parts.expand-toggle', ['classes' => 'text-primary', 'target' => '.entity-list.compact .entity-item-snippet', 'key' => 'home-details'])
@include('common.dark-mode-toggle', ['classes' => 'icon-list-item text-primary'])
</div>
diff --git a/resources/views/shelves/index.blade.php b/resources/views/shelves/index.blade.php
index 3de4247ce..df3ca83eb 100644
--- a/resources/views/shelves/index.blade.php
+++ b/resources/views/shelves/index.blade.php
@@ -16,7 +16,7 @@
</a>
@endif
- @include('entities.view-toggle', ['view' => $view, 'type' => 'shelves'])
+ @include('entities.view-toggle', ['view' => $view, 'type' => 'bookshelves'])
<a href="{{ url('/tags') }}" class="icon-list-item">
<span>@icon('tag')</span>
diff --git a/resources/views/shelves/show.blade.php b/resources/views/shelves/show.blade.php
index 0f6587407..1ea37992a 100644
--- a/resources/views/shelves/show.blade.php
+++ b/resources/views/shelves/show.blade.php
@@ -118,7 +118,7 @@
</a>
@endif
- @include('entities.view-toggle', ['view' => $view, 'type' => 'shelf'])
+ @include('entities.view-toggle', ['view' => $view, 'type' => 'bookshelf'])
<hr class="primary-background">
diff --git a/routes/web.php b/routes/web.php
index f9899dba6..f66f0d984 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -249,13 +249,11 @@ Route::middleware('auth')->group(function () {
Route::redirect('/preferences', '/');
Route::get('/preferences/shortcuts', [UserPreferencesController::class, 'showShortcuts']);
Route::put('/preferences/shortcuts', [UserPreferencesController::class, 'updateShortcuts']);
- Route::patch('/settings/users/{id}/switch-books-view', [UserPreferencesController::class, 'switchBooksView']);
- Route::patch('/settings/users/{id}/switch-shelves-view', [UserPreferencesController::class, 'switchShelvesView']);
- Route::patch('/settings/users/{id}/switch-shelf-view', [UserPreferencesController::class, 'switchShelfView']);
- Route::patch('/settings/users/{id}/change-sort/{type}', [UserPreferencesController::class, 'changeSort']);
- Route::patch('/settings/users/{id}/update-expansion-preference/{key}', [UserPreferencesController::class, 'updateExpansionPreference']);
- Route::patch('/settings/users/toggle-dark-mode', [UserPreferencesController::class, 'toggleDarkMode']);
- Route::patch('/settings/users/update-code-language-favourite', [UserPreferencesController::class, 'updateCodeLanguageFavourite']);
+ Route::patch('/preferences/change-view/{type}', [UserPreferencesController::class, 'changeView']);
+ Route::patch('/preferences/change-sort/{type}', [UserPreferencesController::class, 'changeSort']);
+ Route::patch('/preferences/change-expansion/{type}', [UserPreferencesController::class, 'changeExpansion']);
+ Route::patch('/preferences/toggle-dark-mode', [UserPreferencesController::class, 'toggleDarkMode']);
+ Route::patch('/preferences/update-code-language-favourite', [UserPreferencesController::class, 'updateCodeLanguageFavourite']);
// User API Tokens
Route::get('/settings/users/{userId}/create-api-token', [UserApiTokenController::class, 'create']);
diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php
index cccff3a1f..9e2750fd0 100644
--- a/tests/Entity/BookTest.php
+++ b/tests/Entity/BookTest.php
@@ -225,18 +225,18 @@ class BookTest extends TestCase
setting()->putUser($editor, 'books_view_type', 'list');
$resp = $this->actingAs($editor)->get('/books');
- $this->withHtml($resp)->assertElementContains('form[action$="/settings/users/' . $editor->id . '/switch-books-view"]', 'Grid View');
- $this->withHtml($resp)->assertElementExists('input[name="view_type"][value="grid"]');
+ $this->withHtml($resp)->assertElementContains('form[action$="/preferences/change-view/books"]', 'Grid View');
+ $this->withHtml($resp)->assertElementExists('button[name="view"][value="grid"]');
- $resp = $this->patch("/settings/users/{$editor->id}/switch-books-view", ['view_type' => 'grid']);
+ $resp = $this->patch("/preferences/change-view/books", ['view' => 'grid']);
$resp->assertRedirect();
$this->assertEquals('grid', setting()->getUser($editor, 'books_view_type'));
$resp = $this->actingAs($editor)->get('/books');
- $this->withHtml($resp)->assertElementContains('form[action$="/settings/users/' . $editor->id . '/switch-books-view"]', 'List View');
- $this->withHtml($resp)->assertElementExists('input[name="view_type"][value="list"]');
+ $this->withHtml($resp)->assertElementContains('form[action$="/preferences/change-view/books"]', 'List View');
+ $this->withHtml($resp)->assertElementExists('button[name="view"][value="list"]');
- $resp = $this->patch("/settings/users/{$editor->id}/switch-books-view", ['view_type' => 'list']);
+ $resp = $this->patch("/preferences/change-view/books", ['view_type' => 'list']);
$resp->assertRedirect();
$this->assertEquals('list', setting()->getUser($editor, 'books_view_type'));
}
diff --git a/tests/User/UserManagementTest.php b/tests/User/UserManagementTest.php
index e295034ce..4991e052a 100644
--- a/tests/User/UserManagementTest.php
+++ b/tests/User/UserManagementTest.php
@@ -160,6 +160,23 @@ class UserManagementTest extends TestCase
]);
}
+ public function test_delete_removes_user_preferences()
+ {
+ $editor = $this->getEditor();
+ setting()->putUser($editor, 'dark-mode-enabled', 'true');
+
+ $this->assertDatabaseHas('settings', [
+ 'setting_key' => 'user:' . $editor->id . ':dark-mode-enabled',
+ 'value' => 'true',
+ ]);
+
+ $this->asAdmin()->delete("settings/users/{$editor->id}");
+
+ $this->assertDatabaseMissing('settings', [
+ 'setting_key' => 'user:' . $editor->id . ':dark-mode-enabled',
+ ]);
+ }
+
public function test_guest_profile_shows_limited_form()
{
$guest = User::getDefault();
diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php
index 3ef575eab..03dad7990 100644
--- a/tests/User/UserPreferencesTest.php
+++ b/tests/User/UserPreferencesTest.php
@@ -50,7 +50,7 @@ class UserPreferencesTest extends TestCase
$editor = $this->getEditor();
$this->actingAs($editor);
- $updateRequest = $this->patch('/settings/users/' . $editor->id . '/change-sort/books', [
+ $updateRequest = $this->patch('/preferences/change-sort/books', [
'sort' => 'created_at',
'order' => 'desc',
]);
@@ -73,7 +73,7 @@ class UserPreferencesTest extends TestCase
$editor = $this->getEditor();
$this->actingAs($editor);
- $updateRequest = $this->patch('/settings/users/' . $editor->id . '/change-sort/dogs', [
+ $updateRequest = $this->patch('/preferences/change-sort/dogs', [
'sort' => 'name',
'order' => 'asc',
]);
@@ -88,7 +88,7 @@ class UserPreferencesTest extends TestCase
$editor = $this->getEditor();
$this->actingAs($editor);
- $updateRequest = $this->patch('/settings/users/' . $editor->id . '/update-expansion-preference/home-details', ['expand' => 'true']);
+ $updateRequest = $this->patch('/preferences/change-expansion/home-details', ['expand' => 'true']);
$updateRequest->assertStatus(204);
$this->assertDatabaseHas('settings', [
@@ -97,7 +97,7 @@ class UserPreferencesTest extends TestCase
]);
$this->assertEquals(true, setting()->getForCurrentUser('section_expansion#home-details'));
- $invalidKeyRequest = $this->patch('/settings/users/' . $editor->id . '/update-expansion-preference/my-home-details', ['expand' => 'true']);
+ $invalidKeyRequest = $this->patch('/preferences/change-expansion/my-home-details', ['expand' => 'true']);
$invalidKeyRequest->assertStatus(500);
}
@@ -108,7 +108,7 @@ class UserPreferencesTest extends TestCase
$this->withHtml($home)->assertElementNotExists('.dark-mode');
$this->assertEquals(false, setting()->getForCurrentUser('dark-mode-enabled', false));
- $prefChange = $this->patch('/settings/users/toggle-dark-mode');
+ $prefChange = $this->patch('/preferences/toggle-dark-mode');
$prefChange->assertRedirect();
$this->assertEquals(true, setting()->getForCurrentUser('dark-mode-enabled'));
@@ -162,7 +162,7 @@ class UserPreferencesTest extends TestCase
->assertElementNotExists('.featured-image-container')
->assertElementExists('.content-wrap .entity-list-item');
- $req = $this->patch("/settings/users/{$editor->id}/switch-shelf-view", ['view_type' => 'grid']);
+ $req = $this->patch("/preferences/change-view/bookshelf", ['view' => 'grid']);
$req->assertRedirect($shelf->getUrl());
$resp = $this->actingAs($editor)->get($shelf->getUrl())
@@ -179,14 +179,14 @@ class UserPreferencesTest extends TestCase
$page = $this->entities->page();
$this->actingAs($editor);
- $this->patch('/settings/users/update-code-language-favourite', ['language' => 'php', 'active' => true]);
- $this->patch('/settings/users/update-code-language-favourite', ['language' => 'javascript', 'active' => true]);
+ $this->patch('/preferences/update-code-language-favourite', ['language' => 'php', 'active' => true]);
+ $this->patch('/preferences/update-code-language-favourite', ['language' => 'javascript', 'active' => true]);
$resp = $this->get($page->getUrl('/edit'));
$resp->assertSee('option:code-editor:favourites="php,javascript"', false);
- $this->patch('/settings/users/update-code-language-favourite', ['language' => 'ruby', 'active' => true]);
- $this->patch('/settings/users/update-code-language-favourite', ['language' => 'php', 'active' => false]);
+ $this->patch('/preferences/update-code-language-favourite', ['language' => 'ruby', 'active' => true]);
+ $this->patch('/preferences/update-code-language-favourite', ['language' => 'php', 'active' => false]);
$resp = $this->get($page->getUrl('/edit'));
$resp->assertSee('option:code-editor:favourites="javascript,ruby"', false);