From f39938c4e3b8750668442a6a17e1006953b5cef2 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 12 Jun 2023 16:45:30 +0100 Subject: [PATCH] Added activity text for each activity type Ensures some sensible text is always in webhook text data. Also aligned some notification reporting to use centralised activity system instead of custom success events. For #4216 --- app/Api/UserApiTokenController.php | 3 -- .../Controllers/ChapterController.php | 2 - app/Entities/Controllers/PageController.php | 4 +- .../Controllers/PageRevisionController.php | 9 ++--- .../Controllers/RecycleBinController.php | 2 +- app/Settings/SettingController.php | 2 - app/Users/Controllers/UserController.php | 11 ++--- lang/en/activities.php | 40 ++++++++++++++++++- lang/en/entities.php | 3 -- lang/en/settings.php | 4 -- tests/LanguageTest.php | 10 +++++ tests/Permissions/RolesTest.php | 2 +- 12 files changed, 59 insertions(+), 33 deletions(-) diff --git a/app/Api/UserApiTokenController.php b/app/Api/UserApiTokenController.php index d8fc1171c..8357420ee 100644 --- a/app/Api/UserApiTokenController.php +++ b/app/Api/UserApiTokenController.php @@ -58,7 +58,6 @@ class UserApiTokenController extends Controller $token->save(); session()->flash('api-token-secret:' . $token->id, $secret); - $this->showSuccessNotification(trans('settings.user_api_token_create_success')); $this->logActivity(ActivityType::API_TOKEN_CREATE, $token); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); @@ -96,7 +95,6 @@ class UserApiTokenController extends Controller 'expires_at' => $request->get('expires_at') ?: ApiToken::defaultExpiry(), ])->save(); - $this->showSuccessNotification(trans('settings.user_api_token_update_success')); $this->logActivity(ActivityType::API_TOKEN_UPDATE, $token); return redirect($user->getEditUrl('/api-tokens/' . $token->id)); @@ -123,7 +121,6 @@ class UserApiTokenController extends Controller [$user, $token] = $this->checkPermissionAndFetchUserToken($userId, $tokenId); $token->delete(); - $this->showSuccessNotification(trans('settings.user_api_token_delete_success')); $this->logActivity(ActivityType::API_TOKEN_DELETE, $token); return redirect($user->getEditUrl('#api_tokens')); diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index cf7611685..7dcb66903 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -191,8 +191,6 @@ class ChapterController extends Controller return redirect()->back(); } - $this->showSuccessNotification(trans('entities.chapter_move_success', ['bookName' => $newBook->name])); - return redirect($chapter->getUrl()); } diff --git a/app/Entities/Controllers/PageController.php b/app/Entities/Controllers/PageController.php index e0444ecd2..3187e6486 100644 --- a/app/Entities/Controllers/PageController.php +++ b/app/Entities/Controllers/PageController.php @@ -389,7 +389,7 @@ class PageController extends Controller } try { - $parent = $this->pageRepo->move($page, $entitySelection); + $this->pageRepo->move($page, $entitySelection); } catch (PermissionsException $exception) { $this->showPermissionError(); } catch (Exception $exception) { @@ -398,8 +398,6 @@ class PageController extends Controller return redirect()->back(); } - $this->showSuccessNotification(trans('entities.pages_move_success', ['parentName' => $parent->name])); - return redirect($page->getUrl()); } diff --git a/app/Entities/Controllers/PageRevisionController.php b/app/Entities/Controllers/PageRevisionController.php index a723513a8..9e6a90477 100644 --- a/app/Entities/Controllers/PageRevisionController.php +++ b/app/Entities/Controllers/PageRevisionController.php @@ -15,11 +15,9 @@ use Ssddanbrown\HtmlDiff\Diff; class PageRevisionController extends Controller { - protected PageRepo $pageRepo; - - public function __construct(PageRepo $pageRepo) - { - $this->pageRepo = $pageRepo; + public function __construct( + protected PageRepo $pageRepo + ) { } /** @@ -153,7 +151,6 @@ class PageRevisionController extends Controller $revision->delete(); Activity::add(ActivityType::REVISION_DELETE, $revision); - $this->showSuccessNotification(trans('entities.revision_delete_success')); return redirect($page->getUrl('/revisions')); } diff --git a/app/Entities/Controllers/RecycleBinController.php b/app/Entities/Controllers/RecycleBinController.php index 30b184bbe..78f86a5ae 100644 --- a/app/Entities/Controllers/RecycleBinController.php +++ b/app/Entities/Controllers/RecycleBinController.php @@ -11,7 +11,7 @@ use BookStack\Http\Controller; class RecycleBinController extends Controller { - protected $recycleBinBaseUrl = '/settings/recycle-bin'; + protected string $recycleBinBaseUrl = '/settings/recycle-bin'; /** * On each request to a method of this controller check permissions diff --git a/app/Settings/SettingController.php b/app/Settings/SettingController.php index ffdd7545e..bd55222f2 100644 --- a/app/Settings/SettingController.php +++ b/app/Settings/SettingController.php @@ -52,9 +52,7 @@ class SettingController extends Controller ]); $store->storeFromUpdateRequest($request, $category); - $this->logActivity(ActivityType::SETTINGS_UPDATE, $category); - $this->showSuccessNotification(trans('settings.settings_save_success')); return redirect("/settings/{$category}"); } diff --git a/app/Users/Controllers/UserController.php b/app/Users/Controllers/UserController.php index b185f0856..1c1b7ba23 100644 --- a/app/Users/Controllers/UserController.php +++ b/app/Users/Controllers/UserController.php @@ -19,13 +19,10 @@ use Illuminate\Validation\ValidationException; class UserController extends Controller { - protected UserRepo $userRepo; - protected ImageRepo $imageRepo; - - public function __construct(UserRepo $userRepo, ImageRepo $imageRepo) - { - $this->userRepo = $userRepo; - $this->imageRepo = $imageRepo; + public function __construct( + protected UserRepo $userRepo, + protected ImageRepo $imageRepo + ) { } /** diff --git a/lang/en/activities.php b/lang/en/activities.php index e89b8eab2..e71a490de 100644 --- a/lang/en/activities.php +++ b/lang/en/activities.php @@ -15,6 +15,7 @@ return [ 'page_restore' => 'restored page', 'page_restore_notification' => 'Page successfully restored', 'page_move' => 'moved page', + 'page_move_notification' => 'Page successfully moved', // Chapters 'chapter_create' => 'created chapter', @@ -24,6 +25,7 @@ return [ 'chapter_delete' => 'deleted chapter', 'chapter_delete_notification' => 'Chapter successfully deleted', 'chapter_move' => 'moved chapter', + 'chapter_move_notification' => 'Chapter successfully moved', // Books 'book_create' => 'created book', @@ -47,14 +49,30 @@ return [ 'bookshelf_delete' => 'deleted shelf', 'bookshelf_delete_notification' => 'Shelf successfully deleted', + // Revisions + 'revision_restore' => 'restored revision', + 'revision_delete' => 'deleted revision', + 'revision_delete_notification' => 'Revision successfully deleted', + // Favourites 'favourite_add_notification' => '":name" has been added to your favourites', 'favourite_remove_notification' => '":name" has been removed from your favourites', - // MFA + // Auth + 'auth_login' => 'logged in', + 'auth_register' => 'registered as new user', + 'auth_password_reset_request' => 'requested user password reset', + 'auth_password_reset_update' => 'reset user password', + 'mfa_setup_method' => 'configured MFA method', 'mfa_setup_method_notification' => 'Multi-factor method successfully configured', + 'mfa_remove_method' => 'removed MFA method', 'mfa_remove_method_notification' => 'Multi-factor method successfully removed', + // Settings + 'settings_update' => 'updated settings', + 'settings_update_notification' => 'Settings successfully updated', + 'maintenance_action_run' => 'ran maintenance action', + // Webhooks 'webhook_create' => 'created webhook', 'webhook_create_notification' => 'Webhook successfully created', @@ -64,14 +82,34 @@ return [ 'webhook_delete_notification' => 'Webhook successfully deleted', // Users + 'user_create' => 'created user', + 'user_create_notification' => 'User successfully created', + 'user_update' => 'updated user', 'user_update_notification' => 'User successfully updated', + 'user_delete' => 'deleted user', 'user_delete_notification' => 'User successfully removed', + // API Tokens + 'api_token_create' => 'created api token', + 'api_token_create_notification' => 'API token successfully created', + 'api_token_update' => 'updated api token', + 'api_token_update_notification' => 'API token successfully updated', + 'api_token_delete' => 'deleted api token', + 'api_token_delete_notification' => 'API token successfully deleted', + // Roles + 'role_create' => 'created role', 'role_create_notification' => 'Role successfully created', + 'role_update' => 'updated role', 'role_update_notification' => 'Role successfully updated', + 'role_delete' => 'deleted role', 'role_delete_notification' => 'Role successfully deleted', + // Recycle Bin + 'recycle_bin_empty' => 'emptied recycle bin', + 'recycle_bin_restore' => 'restored from recycle bin', + 'recycle_bin_destroy' => 'removed from recycle bin', + // Other 'commented_on' => 'commented on', 'permissions_update' => 'updated permissions', diff --git a/lang/en/entities.php b/lang/en/entities.php index caf9e2361..92903ed1f 100644 --- a/lang/en/entities.php +++ b/lang/en/entities.php @@ -180,7 +180,6 @@ return [ 'chapters_save' => 'Save Chapter', 'chapters_move' => 'Move Chapter', 'chapters_move_named' => 'Move Chapter :chapterName', - 'chapter_move_success' => 'Chapter moved to :bookName', 'chapters_copy' => 'Copy Chapter', 'chapters_copy_success' => 'Chapter successfully copied', 'chapters_permissions' => 'Chapter Permissions', @@ -240,7 +239,6 @@ return [ 'pages_md_sync_scroll' => 'Sync preview scroll', 'pages_not_in_chapter' => 'Page is not in a chapter', 'pages_move' => 'Move Page', - 'pages_move_success' => 'Page moved to ":parentName"', 'pages_copy' => 'Copy Page', 'pages_copy_desination' => 'Copy Destination', 'pages_copy_success' => 'Page successfully copied', @@ -375,7 +373,6 @@ return [ // Revision 'revision_delete_confirm' => 'Are you sure you want to delete this revision?', 'revision_restore_confirm' => 'Are you sure you want to restore this revision? The current page contents will be replaced.', - 'revision_delete_success' => 'Revision deleted', 'revision_cannot_delete_latest' => 'Cannot delete the latest revision.', // Copy view diff --git a/lang/en/settings.php b/lang/en/settings.php index 38d817915..c110e8992 100644 --- a/lang/en/settings.php +++ b/lang/en/settings.php @@ -9,7 +9,6 @@ return [ // Common Messages 'settings' => 'Settings', 'settings_save' => 'Save Settings', - 'settings_save_success' => 'Settings saved', 'system_version' => 'System Version', 'categories' => 'Categories', @@ -232,8 +231,6 @@ return [ 'user_api_token_expiry' => 'Expiry Date', 'user_api_token_expiry_desc' => 'Set a date at which this token expires. After this date, requests made using this token will no longer work. Leaving this field blank will set an expiry 100 years into the future.', 'user_api_token_create_secret_message' => 'Immediately after creating this token a "Token ID" & "Token Secret" will be generated and displayed. The secret will only be shown a single time so be sure to copy the value to somewhere safe and secure before proceeding.', - 'user_api_token_create_success' => 'API token successfully created', - 'user_api_token_update_success' => 'API token successfully updated', 'user_api_token' => 'API Token', 'user_api_token_id' => 'Token ID', 'user_api_token_id_desc' => 'This is a non-editable system generated identifier for this token which will need to be provided in API requests.', @@ -244,7 +241,6 @@ return [ 'user_api_token_delete' => 'Delete Token', 'user_api_token_delete_warning' => 'This will fully delete this API token with the name \':tokenName\' from the system.', 'user_api_token_delete_confirm' => 'Are you sure you want to delete this API token?', - 'user_api_token_delete_success' => 'API token successfully deleted', // Webhooks 'webhooks' => 'Webhooks', diff --git a/tests/LanguageTest.php b/tests/LanguageTest.php index b65227dd8..a66227ff2 100644 --- a/tests/LanguageTest.php +++ b/tests/LanguageTest.php @@ -2,6 +2,8 @@ namespace Tests; +use BookStack\Activity\ActivityType; + class LanguageTest extends TestCase { protected array $langs; @@ -90,4 +92,12 @@ class LanguageTest extends TestCase $loginReq->assertOk(); $loginReq->assertSee('Log In'); } + + public function test_all_activity_types_have_activity_text() + { + foreach (ActivityType::all() as $activityType) { + $langKey = 'activities.' . $activityType; + $this->assertNotEquals($langKey, trans($langKey, [], 'en')); + } + } } diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php index dafa1f2bb..4c8ea1ab4 100644 --- a/tests/Permissions/RolesTest.php +++ b/tests/Permissions/RolesTest.php @@ -301,7 +301,7 @@ class RolesTest extends TestCase $resp = $this->post('/settings/features', []); $resp->assertRedirect('/settings/features'); $resp = $this->get('/settings/features'); - $resp->assertSee('Settings saved'); + $resp->assertSee('Settings successfully updated'); } public function test_restrictions_manage_all_permission()