From 45e75edf0544217e1a46f5628037770a6713b23b Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 19 Jul 2023 11:03:05 +0100 Subject: [PATCH 01/20] Notifications: Started activity->notification core framework --- .../Handlers/CommentCreationNotificationHandler.php | 13 +++++++++++++ .../Notifications/Handlers/NotificationHandler.php | 13 +++++++++++++ .../Handlers/PageCreationNotificationHandler.php | 13 +++++++++++++ .../Handlers/PageUpdateNotificationHandler.php | 13 +++++++++++++ app/Activity/Tools/ActivityLogger.php | 11 +++++++++-- 5 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php create mode 100644 app/Activity/Notifications/Handlers/NotificationHandler.php create mode 100644 app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php create mode 100644 app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php diff --git a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php new file mode 100644 index 000000000..5f2e1c770 --- /dev/null +++ b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php @@ -0,0 +1,13 @@ +notifications->loadDefaultHandlers(); + } + /** * Add a generic activity event to the database. */ - public function add(string $type, $detail = '') + public function add(string $type, string|Loggable $detail = ''): void { $detailToStore = ($detail instanceof Loggable) ? $detail->logDescriptor() : $detail; @@ -34,6 +40,7 @@ class ActivityLogger $this->setNotification($type); $this->dispatchWebhooks($type, $detail); + $this->notifications->handle($type, $detail); Theme::dispatch(ThemeEvents::ACTIVITY_LOGGED, $type, $detail); } From 100b28707cbda87a57d5f1d31c6e33f88642caa6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 25 Jul 2023 17:06:48 +0100 Subject: [PATCH 02/20] Notifications: added user preference UI & logic Includes testing to cover. Also added file missing from previous commit. --- .../Notifications/NotificationManager.php | 49 +++++++++++++++++++ app/Settings/UserNotificationPreferences.php | 46 +++++++++++++++++ .../Controllers/UserPreferencesController.php | 40 ++++++++++++--- lang/en/preferences.php | 10 +++- .../users/preferences/notifications.blade.php | 45 +++++++++++++++++ routes/web.php | 2 + tests/User/UserPreferencesTest.php | 25 ++++++++++ 7 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 app/Activity/Notifications/NotificationManager.php create mode 100644 app/Settings/UserNotificationPreferences.php create mode 100644 resources/views/users/preferences/notifications.blade.php diff --git a/app/Activity/Notifications/NotificationManager.php b/app/Activity/Notifications/NotificationManager.php new file mode 100644 index 000000000..5864b8456 --- /dev/null +++ b/app/Activity/Notifications/NotificationManager.php @@ -0,0 +1,49 @@ +[] + */ + protected array $handlers = []; + + public function handle(string $activityType, string|Loggable $detail): void + { + $handlersToRun = $this->handlers[$activityType] ?? []; + foreach ($handlersToRun as $handlerClass) { + /** @var NotificationHandler $handler */ + $handler = app()->make($handlerClass); + $handler->handle($activityType, $detail); + } + } + + /** + * @param class-string $handlerClass + */ + public function registerHandler(string $activityType, string $handlerClass): void + { + if (!isset($this->handlers[$activityType])) { + $this->handlers[$activityType] = []; + } + + if (!in_array($handlerClass, $this->handlers[$activityType])) { + $this->handlers[$activityType][] = $handlerClass; + } + } + + public function loadDefaultHandlers(): void + { + $this->registerHandler(ActivityType::PAGE_CREATE, PageCreationNotificationHandler::class); + $this->registerHandler(ActivityType::PAGE_UPDATE, PageUpdateNotificationHandler::class); + $this->registerHandler(ActivityType::COMMENT_CREATE, CommentCreationNotificationHandler::class); + } +} diff --git a/app/Settings/UserNotificationPreferences.php b/app/Settings/UserNotificationPreferences.php new file mode 100644 index 000000000..5b267b533 --- /dev/null +++ b/app/Settings/UserNotificationPreferences.php @@ -0,0 +1,46 @@ +getNotificationSetting('own-page-changes'); + } + + public function notifyOnOwnPageComments(): bool + { + return $this->getNotificationSetting('own-page-comments'); + } + + public function notifyOnCommentReplies(): bool + { + return $this->getNotificationSetting('comment-replies'); + } + + public function updateFromSettingsArray(array $settings) + { + $allowList = ['own-page-changes', 'own-page-comments', 'comment-replies']; + foreach ($settings as $setting => $status) { + if (!in_array($setting, $allowList)) { + continue; + } + + $value = $status === 'true' ? 'true' : 'false'; + setting()->putUser($this->user, 'notifications#' . $setting, $value); + } + } + + protected function getNotificationSetting(string $key): bool + { + return setting()->getUser($this->user, 'notifications#' . $key); + } +} diff --git a/app/Users/Controllers/UserPreferencesController.php b/app/Users/Controllers/UserPreferencesController.php index b20a8aa37..faa99629b 100644 --- a/app/Users/Controllers/UserPreferencesController.php +++ b/app/Users/Controllers/UserPreferencesController.php @@ -3,17 +3,16 @@ namespace BookStack\Users\Controllers; use BookStack\Http\Controller; +use BookStack\Settings\UserNotificationPreferences; use BookStack\Settings\UserShortcutMap; use BookStack\Users\UserRepo; use Illuminate\Http\Request; class UserPreferencesController extends Controller { - protected UserRepo $userRepo; - - public function __construct(UserRepo $userRepo) - { - $this->userRepo = $userRepo; + public function __construct( + protected UserRepo $userRepo + ) { } /** @@ -47,6 +46,35 @@ class UserPreferencesController extends Controller return redirect('/preferences/shortcuts'); } + /** + * Show the notification preferences for the current user. + */ + public function showNotifications() + { + $preferences = (new UserNotificationPreferences(user())); + + return view('users.preferences.notifications', [ + 'preferences' => $preferences, + ]); + } + + /** + * Update the notification preferences for the current user. + */ + public function updateNotifications(Request $request) + { + $data = $this->validate($request, [ + 'preferences' => ['required', 'array'], + 'preferences.*' => ['required', 'string'], + ]); + + $preferences = (new UserNotificationPreferences(user())); + $preferences->updateFromSettingsArray($data['preferences']); + $this->showSuccessNotification(trans('preferences.notifications_update_success')); + + return redirect('/preferences/notifications'); + } + /** * Update the preferred view format for a list view of the given type. */ @@ -123,7 +151,7 @@ class UserPreferencesController extends Controller { $validated = $this->validate($request, [ 'language' => ['required', 'string', 'max:20'], - 'active' => ['required', 'bool'], + 'active' => ['required', 'bool'], ]); $currentFavoritesStr = setting()->getForCurrentUser('code-language-favourites', ''); diff --git a/lang/en/preferences.php b/lang/en/preferences.php index e9a47461b..2ade3a0e4 100644 --- a/lang/en/preferences.php +++ b/lang/en/preferences.php @@ -15,4 +15,12 @@ return [ 'shortcuts_save' => 'Save Shortcuts', 'shortcuts_overlay_desc' => 'Note: When shortcuts are enabled a helper overlay is available via pressing "?" which will highlight the available shortcuts for actions currently visible on the screen.', 'shortcuts_update_success' => 'Shortcut preferences have been updated!', -]; \ No newline at end of file + + 'notifications' => 'Notification Preferences', + 'notifications_desc' => 'Control the email notifications you receive when certain activity is performed within the system.', + 'notifications_opt_own_page_changes' => 'Notify upon changes to pages I own', + 'notifications_opt_own_page_comments' => 'Notify upon comments on pages I own', + 'notifications_opt_comment_replies' => 'Notify upon replies to my comments', + 'notifications_save' => 'Save Preferences', + 'notifications_update_success' => 'Notification preferences have been updated!', +]; diff --git a/resources/views/users/preferences/notifications.blade.php b/resources/views/users/preferences/notifications.blade.php new file mode 100644 index 000000000..3a301aeb6 --- /dev/null +++ b/resources/views/users/preferences/notifications.blade.php @@ -0,0 +1,45 @@ +@extends('layouts.simple') + +@section('body') +
+ +
+
+ {{ method_field('put') }} + {{ csrf_field() }} + +

{{ trans('preferences.notifications') }}

+

{{ trans('preferences.notifications_desc') }}

+ +
+
+ @include('form.toggle-switch', [ + 'name' => 'preferences[own-page-changes]', + 'value' => $preferences->notifyOnOwnPageChanges(), + 'label' => trans('preferences.notifications_opt_own_page_changes'), + ]) +
+
+ @include('form.toggle-switch', [ + 'name' => 'preferences[own-page-comments]', + 'value' => $preferences->notifyOnOwnPageComments(), + 'label' => trans('preferences.notifications_opt_own_page_comments'), + ]) +
+
+ @include('form.toggle-switch', [ + 'name' => 'preferences[comment-replies]', + 'value' => $preferences->notifyOnCommentReplies(), + 'label' => trans('preferences.notifications_opt_comment_replies'), + ]) +
+
+ +
+ +
+
+
+ +
+@stop diff --git a/routes/web.php b/routes/web.php index 74ee74a2c..9ea44f03c 100644 --- a/routes/web.php +++ b/routes/web.php @@ -231,6 +231,8 @@ Route::middleware('auth')->group(function () { Route::redirect('/preferences', '/'); Route::get('/preferences/shortcuts', [UserControllers\UserPreferencesController::class, 'showShortcuts']); Route::put('/preferences/shortcuts', [UserControllers\UserPreferencesController::class, 'updateShortcuts']); + Route::get('/preferences/notifications', [UserControllers\UserPreferencesController::class, 'showNotifications']); + Route::put('/preferences/notifications', [UserControllers\UserPreferencesController::class, 'updateNotifications']); Route::patch('/preferences/change-view/{type}', [UserControllers\UserPreferencesController::class, 'changeView']); Route::patch('/preferences/change-sort/{type}', [UserControllers\UserPreferencesController::class, 'changeSort']); Route::patch('/preferences/change-expansion/{type}', [UserControllers\UserPreferencesController::class, 'changeExpansion']); diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index e47a259a5..e83df5731 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -45,6 +45,31 @@ class UserPreferencesTest extends TestCase $this->withHtml($this->get('/'))->assertElementExists('body[component="shortcuts"]'); } + public function test_notification_preferences_updating() + { + $this->asEditor(); + + // View preferences with defaults + $resp = $this->get('/preferences/notifications'); + $resp->assertSee('Notification Preferences'); + + $html = $this->withHtml($resp); + $html->assertFieldHasValue('preferences[comment-replies]', 'false'); + + // Update preferences + $resp = $this->put('/preferences/notifications', [ + 'preferences' => ['comment-replies' => 'true'], + ]); + + $resp->assertRedirect('/preferences/notifications'); + $resp->assertSessionHas('success', 'Notification preferences have been updated!'); + + // View updates to preferences page + $resp = $this->get('/preferences/notifications'); + $html = $this->withHtml($resp); + $html->assertFieldHasValue('preferences[comment-replies]', 'true'); + } + public function test_update_sort_preference() { $editor = $this->users->editor(); From ff2674c464994408b6fc359c5b93ca5633211c5d Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 25 Jul 2023 17:59:04 +0100 Subject: [PATCH 03/20] Notifications: Added role receive-notifications permission --- app/Permissions/PermissionsRepo.php | 7 ++- app/Users/Controllers/RoleController.php | 8 ++- ...receive_notifications_role_permissions.php | 51 +++++++++++++++++++ lang/en/settings.php | 1 + .../views/settings/roles/parts/form.blade.php | 1 + 5 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 database/migrations/2023_07_25_124945_add_receive_notifications_role_permissions.php diff --git a/app/Permissions/PermissionsRepo.php b/app/Permissions/PermissionsRepo.php index 889a6ea08..b41612968 100644 --- a/app/Permissions/PermissionsRepo.php +++ b/app/Permissions/PermissionsRepo.php @@ -12,12 +12,11 @@ use Illuminate\Database\Eloquent\Collection; class PermissionsRepo { - protected JointPermissionBuilder $permissionBuilder; protected array $systemRoles = ['admin', 'public']; - public function __construct(JointPermissionBuilder $permissionBuilder) - { - $this->permissionBuilder = $permissionBuilder; + public function __construct( + protected JointPermissionBuilder $permissionBuilder + ) { } /** diff --git a/app/Users/Controllers/RoleController.php b/app/Users/Controllers/RoleController.php index f6472e4de..0052d829d 100644 --- a/app/Users/Controllers/RoleController.php +++ b/app/Users/Controllers/RoleController.php @@ -13,11 +13,9 @@ use Illuminate\Http\Request; class RoleController extends Controller { - protected PermissionsRepo $permissionsRepo; - - public function __construct(PermissionsRepo $permissionsRepo) - { - $this->permissionsRepo = $permissionsRepo; + public function __construct( + protected PermissionsRepo $permissionsRepo + ) { } /** diff --git a/database/migrations/2023_07_25_124945_add_receive_notifications_role_permissions.php b/database/migrations/2023_07_25_124945_add_receive_notifications_role_permissions.php new file mode 100644 index 000000000..4872e421e --- /dev/null +++ b/database/migrations/2023_07_25_124945_add_receive_notifications_role_permissions.php @@ -0,0 +1,51 @@ +insertGetId([ + 'name' => 'receive-notifications', + 'display_name' => 'Receive & Manage Notifications', + 'created_at' => Carbon::now()->toDateTimeString(), + 'updated_at' => Carbon::now()->toDateTimeString(), + ]); + + $adminRoleId = DB::table('roles')->where('system_name', '=', 'admin')->first()->id; + DB::table('permission_role')->insert([ + 'role_id' => $adminRoleId, + 'permission_id' => $permissionId, + ]); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + $permission = DB::table('role_permissions') + ->where('name', '=', 'receive-notifications') + ->first(); + + if ($permission) { + DB::table('permission_role')->where([ + 'permission_id' => $permission->id, + ])->delete(); + } + + DB::table('role_permissions') + ->where('name', '=', 'receive-notifications') + ->delete(); + } +}; diff --git a/lang/en/settings.php b/lang/en/settings.php index c110e8992..8821c77f0 100644 --- a/lang/en/settings.php +++ b/lang/en/settings.php @@ -163,6 +163,7 @@ return [ 'role_manage_settings' => 'Manage app settings', 'role_export_content' => 'Export content', 'role_editor_change' => 'Change page editor', + 'role_notifications' => 'Receive & manage notifications', 'role_asset' => 'Asset Permissions', 'roles_system_warning' => 'Be aware that access to any of the above three permissions can allow a user to alter their own privileges or the privileges of others in the system. Only assign roles with these permissions to trusted users.', 'role_asset_desc' => 'These permissions control default access to the assets within the system. Permissions on Books, Chapters and Pages will override these permissions.', diff --git a/resources/views/settings/roles/parts/form.blade.php b/resources/views/settings/roles/parts/form.blade.php index ac5c320d2..9fa76f2bf 100644 --- a/resources/views/settings/roles/parts/form.blade.php +++ b/resources/views/settings/roles/parts/form.blade.php @@ -38,6 +38,7 @@
@include('settings.roles.parts.checkbox', ['permission' => 'access-api', 'label' => trans('settings.role_access_api')])
@include('settings.roles.parts.checkbox', ['permission' => 'content-export', 'label' => trans('settings.role_export_content')])
@include('settings.roles.parts.checkbox', ['permission' => 'editor-change', 'label' => trans('settings.role_editor_change')])
+
@include('settings.roles.parts.checkbox', ['permission' => 'receive-notifications', 'label' => trans('settings.role_notifications')])
@include('settings.roles.parts.checkbox', ['permission' => 'settings-manage', 'label' => trans('settings.role_manage_settings')])
From 730f539029385109e3e3b9557bca6f7a8b8a8855 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 27 Jul 2023 14:27:45 +0100 Subject: [PATCH 04/20] Notifications: Started entity watch UI --- resources/icons/{view.svg => watch.svg} | 0 resources/sass/_lists.scss | 3 + resources/sass/_text.scss | 1 + resources/views/books/show.blade.php | 2 +- resources/views/entities/meta.blade.php | 8 +++ .../views/entities/watch-controls.blade.php | 71 +++++++++++++++++++ 6 files changed, 84 insertions(+), 1 deletion(-) rename resources/icons/{view.svg => watch.svg} (100%) create mode 100644 resources/views/entities/watch-controls.blade.php diff --git a/resources/icons/view.svg b/resources/icons/watch.svg similarity index 100% rename from resources/icons/view.svg rename to resources/icons/watch.svg diff --git a/resources/sass/_lists.scss b/resources/sass/_lists.scss index ad0803e71..64411014d 100644 --- a/resources/sass/_lists.scss +++ b/resources/sass/_lists.scss @@ -681,6 +681,9 @@ ul.pagination { &.wide { min-width: 220px; } + &.xl-limited { + width: 280px; + } .text-muted { color: #999; fill: #999; diff --git a/resources/sass/_text.scss b/resources/sass/_text.scss index 7cade9607..a3e6f09ac 100644 --- a/resources/sass/_text.scss +++ b/resources/sass/_text.scss @@ -365,6 +365,7 @@ li.checkbox-item, li.task-list-item { } .break-text { + white-space: normal; word-wrap: break-word; overflow-wrap: break-word; } diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index 8bb41c18b..f126c6219 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -67,7 +67,7 @@ @stop @section('right') -
+
{{ trans('common.details') }}
@include('entities.meta', ['entity' => $book]) diff --git a/resources/views/entities/meta.blade.php b/resources/views/entities/meta.blade.php index 7ad61f1b1..b657d82de 100644 --- a/resources/views/entities/meta.blade.php +++ b/resources/views/entities/meta.blade.php @@ -68,4 +68,12 @@
@endif + +
+
+ @icon('watch') + Watching with default preferences +
+ @include('entities.watch-controls', ['entity' => $entity]) +
\ No newline at end of file diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php new file mode 100644 index 000000000..ab4af0f77 --- /dev/null +++ b/resources/views/entities/watch-controls.blade.php @@ -0,0 +1,71 @@ +
+{{-- {{ method_field('PUT') }}--}} + +
    +
  • + +
    • +
    • +
  • + +
    • +
    • +
  • + +
    • +
    • +
  • + +
    • +
    • +
  • + +
    • +
    • +
  • +
    + Change default notification preferences +
    +
    • +
+
\ No newline at end of file From 6100b99828153a1b712e73da84d90b583d833b01 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 31 Jul 2023 15:23:28 +0100 Subject: [PATCH 05/20] Notifications: Extracted watch options, updated UI further --- app/Users/UserWatchOptions.php | 22 ++++++ lang/en/entities.php | 14 ++++ resources/js/components/dropdown.js | 1 + resources/sass/_layout.scss | 10 ++- resources/sass/_lists.scss | 8 +- resources/sass/_variables.scss | 1 + resources/views/books/show.blade.php | 3 +- resources/views/entities/meta.blade.php | 7 +- .../views/entities/watch-action.blade.php | 9 +++ .../views/entities/watch-controls.blade.php | 73 +++++-------------- 10 files changed, 86 insertions(+), 62 deletions(-) create mode 100644 app/Users/UserWatchOptions.php create mode 100644 resources/views/entities/watch-action.blade.php diff --git a/app/Users/UserWatchOptions.php b/app/Users/UserWatchOptions.php new file mode 100644 index 000000000..a835bcfc7 --- /dev/null +++ b/app/Users/UserWatchOptions.php @@ -0,0 +1,22 @@ + -1, + 'ignore' => 0, + 'new' => 1, + 'updates' => 2, + 'comments' => 3, + ]; + + /** + * @return string[] + */ + public static function getAvailableOptionNames(): array + { + return array_keys(static::$levelByOption); + } +} diff --git a/lang/en/entities.php b/lang/en/entities.php index 4fb043aa9..80b9142f5 100644 --- a/lang/en/entities.php +++ b/lang/en/entities.php @@ -403,4 +403,18 @@ return [ 'references' => 'References', 'references_none' => 'There are no tracked references to this item.', 'references_to_desc' => 'Shown below are all the known pages in the system that link to this item.', + + // Watch Options + 'watch' => 'Watch', + 'watch_title_default' => 'Default Preferences', + 'watch_desc_default' => 'Revert watching to just your default notification preferences.', + 'watch_title_ignore' => 'Ignore', + 'watch_desc_ignore' => 'Ignore all notifications, including those from user-level preferences.', + 'watch_title_new' => 'New Pages', + 'watch_desc_new' => 'Notify when any new page is created within this item.', + 'watch_title_updates' => 'All Page Updates', + 'watch_desc_updates' => 'Notify upon all new pages and page changes.', + 'watch_title_comments' => 'All Page Updates & Comments', + 'watch_desc_comments' => 'Notify upon all new pages, page changes and new comments.', + 'watch_change_default' => 'Change default notification preferences', ]; diff --git a/resources/js/components/dropdown.js b/resources/js/components/dropdown.js index b68f332b6..2c5919a37 100644 --- a/resources/js/components/dropdown.js +++ b/resources/js/components/dropdown.js @@ -132,6 +132,7 @@ export class Dropdown extends Component { onSelect(this.toggle, event => { event.stopPropagation(); + event.preventDefault(); this.show(event); if (event instanceof KeyboardEvent) { keyboardNavHandler.focusNext(); diff --git a/resources/sass/_layout.scss b/resources/sass/_layout.scss index a8604b81b..50776ea28 100644 --- a/resources/sass/_layout.scss +++ b/resources/sass/_layout.scss @@ -353,7 +353,7 @@ body.flexbox { margin-inline-end: $-xl; grid-template-columns: 1fr 4fr 1fr; grid-template-areas: "a b c"; - grid-column-gap: $-xxl; + grid-column-gap: $-xl; .tri-layout-right { grid-area: c; min-width: 0; @@ -378,6 +378,14 @@ body.flexbox { padding-inline-end: $-l; } } +@include between($xxl, $xxxl) { + .tri-layout-container { + grid-template-columns: 1fr calc(940px + (2 * $-m)) 1fr; + grid-column-gap: $-s; + margin-inline-start: $-m; + margin-inline-end: $-m; + } +} @include between($l, $xxl) { .tri-layout-left { position: sticky; diff --git a/resources/sass/_lists.scss b/resources/sass/_lists.scss index 64411014d..323551196 100644 --- a/resources/sass/_lists.scss +++ b/resources/sass/_lists.scss @@ -672,7 +672,7 @@ ul.pagination { @include lightDark(color, #555, #eee); fill: currentColor; text-align: start !important; - max-height: 500px; + max-height: 80vh; overflow-y: auto; &.anchor-left { inset-inline-end: auto; @@ -683,6 +683,7 @@ ul.pagination { } &.xl-limited { width: 280px; + max-width: 100%; } .text-muted { color: #999; @@ -708,6 +709,11 @@ ul.pagination { white-space: nowrap; line-height: 1.4; cursor: pointer; + &.break-text { + white-space: normal; + word-wrap: break-word; + overflow-wrap: break-word; + } &:hover, &:focus { text-decoration: none; background-color: var(--color-primary-light); diff --git a/resources/sass/_variables.scss b/resources/sass/_variables.scss index a3598e29c..35586bf58 100644 --- a/resources/sass/_variables.scss +++ b/resources/sass/_variables.scss @@ -2,6 +2,7 @@ /////////////// // Screen breakpoints +$xxxl: 1700px; $xxl: 1400px; $xl: 1100px; $l: 1000px; diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index f126c6219..305a65132 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -67,7 +67,7 @@ @stop @section('right') -
+
{{ trans('common.details') }}
@include('entities.meta', ['entity' => $book]) @@ -142,6 +142,7 @@ @if(signedInUser()) @include('entities.favourite-action', ['entity' => $book]) @endif + @include('entities.watch-action', ['entity' => $book]) @if(userCan('content-export')) @include('entities.export-menu', ['entity' => $book]) @endif diff --git a/resources/views/entities/meta.blade.php b/resources/views/entities/meta.blade.php index b657d82de..0bda12938 100644 --- a/resources/views/entities/meta.blade.php +++ b/resources/views/entities/meta.blade.php @@ -69,11 +69,12 @@ @endif -
-
+
+ @icon('watch') Watching with default preferences -
+ @include('entities.watch-controls', ['entity' => $entity])
\ No newline at end of file diff --git a/resources/views/entities/watch-action.blade.php b/resources/views/entities/watch-action.blade.php new file mode 100644 index 000000000..dd626a0ef --- /dev/null +++ b/resources/views/entities/watch-action.blade.php @@ -0,0 +1,9 @@ +
+ {{ csrf_field() }} + + + +
\ No newline at end of file diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php index ab4af0f77..b6b16a607 100644 --- a/resources/views/entities/watch-controls.blade.php +++ b/resources/views/entities/watch-controls.blade.php @@ -1,71 +1,32 @@
{{-- {{ method_field('PUT') }}--}} + {{ csrf_field() }} + + -
    +
      + @foreach(\BookStack\Users\UserWatchOptions::getAvailableOptionNames() as $option)
    • -
      • + @endforeach
    • - -
      • -
      • -
    • - -
      • -
      • -
    • - -
      • -
      • -
    • - -
      • -
      • -
    • -
      - Change default notification preferences -
      + {{ trans('entities.watch_change_default') }}
    \ No newline at end of file From 8cdf3203ef67b01ad359699c5c6a5ea8d890e4f8 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 31 Jul 2023 16:08:29 +0100 Subject: [PATCH 06/20] Notifications: Started back-end for watch system Added DB and started controller method. --- app/Activity/Controllers/WatchController.php | 63 +++++++++++++++++++ app/Activity/Models/Watch.php | 52 +++++++++++++++ app/Users/UserWatchOptions.php | 22 ------- ...2023_07_31_104430_create_watches_table.php | 37 +++++++++++ .../views/entities/watch-controls.blade.php | 2 +- 5 files changed, 153 insertions(+), 23 deletions(-) create mode 100644 app/Activity/Controllers/WatchController.php create mode 100644 app/Activity/Models/Watch.php delete mode 100644 app/Users/UserWatchOptions.php create mode 100644 database/migrations/2023_07_31_104430_create_watches_table.php diff --git a/app/Activity/Controllers/WatchController.php b/app/Activity/Controllers/WatchController.php new file mode 100644 index 000000000..f9e8a4e3d --- /dev/null +++ b/app/Activity/Controllers/WatchController.php @@ -0,0 +1,63 @@ +validate($request, [ + 'level' => ['required', 'string'], + ]); + + $watchable = $this->getValidatedModelFromRequest($request); + $newLevel = Watch::optionNameToLevel($requestData['level']); + + if ($newLevel < 0) { + // TODO - Delete + } else { + // TODO - Upsert + } + } + + /** + * @throws ValidationException + * @throws Exception + */ + protected function getValidatedModelFromRequest(Request $request): Entity + { + $modelInfo = $this->validate($request, [ + 'type' => ['required', 'string'], + 'id' => ['required', 'integer'], + ]); + + if (!class_exists($modelInfo['type'])) { + throw new Exception('Model not found'); + } + + /** @var Model $model */ + $model = new $modelInfo['type'](); + if (!$model instanceof Entity) { + throw new Exception('Model not an entity'); + } + + $modelInstance = $model->newQuery() + ->where('id', '=', $modelInfo['id']) + ->first(['id', 'name', 'owned_by']); + + $inaccessibleEntity = ($modelInstance instanceof Entity && !userCan('view', $modelInstance)); + if (is_null($modelInstance) || $inaccessibleEntity) { + throw new Exception('Model instance not found'); + } + + return $modelInstance; + } +} diff --git a/app/Activity/Models/Watch.php b/app/Activity/Models/Watch.php new file mode 100644 index 000000000..6e0e1f787 --- /dev/null +++ b/app/Activity/Models/Watch.php @@ -0,0 +1,52 @@ + -1, + 'ignore' => 0, + 'new' => 1, + 'updates' => 2, + 'comments' => 3, + ]; + + public function watchable() + { + $this->morphTo(); + } + + public function jointPermissions(): HasMany + { + return $this->hasMany(JointPermission::class, 'entity_id', 'watchable_id') + ->whereColumn('favourites.watchable_type', '=', 'joint_permissions.entity_type'); + } + + /** + * @return string[] + */ + public static function getAvailableOptionNames(): array + { + return array_keys(static::$levelByOption); + } + + public static function optionNameToLevel(string $option): int + { + return static::$levelByOption[$option] ?? -1; + } +} diff --git a/app/Users/UserWatchOptions.php b/app/Users/UserWatchOptions.php deleted file mode 100644 index a835bcfc7..000000000 --- a/app/Users/UserWatchOptions.php +++ /dev/null @@ -1,22 +0,0 @@ - -1, - 'ignore' => 0, - 'new' => 1, - 'updates' => 2, - 'comments' => 3, - ]; - - /** - * @return string[] - */ - public static function getAvailableOptionNames(): array - { - return array_keys(static::$levelByOption); - } -} diff --git a/database/migrations/2023_07_31_104430_create_watches_table.php b/database/migrations/2023_07_31_104430_create_watches_table.php new file mode 100644 index 000000000..e2a5c20d0 --- /dev/null +++ b/database/migrations/2023_07_31_104430_create_watches_table.php @@ -0,0 +1,37 @@ +increments('id'); + $table->integer('user_id')->index(); + $table->integer('watchable_id'); + $table->string('watchable_type', 100); + $table->tinyInteger('level', false, true)->index(); + $table->timestamps(); + + $table->index(['watchable_id', 'watchable_type'], 'watchable_index'); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::dropIfExists('watches'); + } +}; diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php index b6b16a607..8d6bfed00 100644 --- a/resources/views/entities/watch-controls.blade.php +++ b/resources/views/entities/watch-controls.blade.php @@ -5,7 +5,7 @@
      - @foreach(\BookStack\Users\UserWatchOptions::getAvailableOptionNames() as $option) + @foreach(\BookStack\Activity\Models\Watch::getAvailableOptionNames() as $option)
\ No newline at end of file diff --git a/resources/views/entities/watch-action.blade.php b/resources/views/entities/watch-action.blade.php index dd626a0ef..34e287804 100644 --- a/resources/views/entities/watch-action.blade.php +++ b/resources/views/entities/watch-action.blade.php @@ -1,8 +1,12 @@ -
+ {{ csrf_field() }} + {{ method_field('PUT') }} - diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php index 8d6bfed00..e02db5800 100644 --- a/resources/views/entities/watch-controls.blade.php +++ b/resources/views/entities/watch-controls.blade.php @@ -1,27 +1,30 @@ - -{{-- {{ method_field('PUT') }}--}} + + {{ method_field('PUT') }} {{ csrf_field() }}
    - @foreach(\BookStack\Activity\Models\Watch::getAvailableOptionNames() as $option) -
  • -
- - -
    • + + +
  • +
    +
    • @endforeach
  • group(function () { Route::post('/favourites/add', [ActivityControllers\FavouriteController::class, 'add']); Route::post('/favourites/remove', [ActivityControllers\FavouriteController::class, 'remove']); + // Watching + Route::put('/watching/update', [ActivityControllers\WatchController::class, 'update']); + // Other Pages Route::get('/', [HomeController::class, 'index']); Route::get('/home', [HomeController::class, 'index']); From 9779c1a357d49654856ceb1b1f30d91cde8fa2fd Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 4 Aug 2023 12:27:29 +0100 Subject: [PATCH 08/20] Notifications: Started core user notification logic Put together an initial notification. Started logic to query and identify watchers. --- .../CommentCreationNotificationHandler.php | 3 +- .../Handlers/NotificationHandler.php | 5 +- .../PageCreationNotificationHandler.php | 23 ++++++- .../PageUpdateNotificationHandler.php | 3 +- .../Notifications/LinkedMailMessageLine.php | 26 ++++++++ .../Messages/BaseActivityNotification.php | 50 +++++++++++++++ .../Messages/PageCreationNotification.php | 28 +++++++++ .../Notifications/NotificationManager.php | 5 +- app/Activity/Tools/ActivityLogger.php | 2 +- app/Activity/Tools/EntityWatchers.php | 55 +++++++++++++++++ app/Activity/Tools/UserWatchOptions.php | 37 +---------- app/Activity/WatchLevels.php | 61 +++++++++++++++++++ .../views/entities/watch-controls.blade.php | 2 +- 13 files changed, 258 insertions(+), 42 deletions(-) create mode 100644 app/Activity/Notifications/LinkedMailMessageLine.php create mode 100644 app/Activity/Notifications/Messages/BaseActivityNotification.php create mode 100644 app/Activity/Notifications/Messages/PageCreationNotification.php create mode 100644 app/Activity/Tools/EntityWatchers.php create mode 100644 app/Activity/WatchLevels.php diff --git a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php index 5f2e1c770..67c304339 100644 --- a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php @@ -3,10 +3,11 @@ namespace BookStack\Activity\Notifications\Handlers; use BookStack\Activity\Models\Loggable; +use BookStack\Users\Models\User; class CommentCreationNotificationHandler implements NotificationHandler { - public function handle(string $activityType, Loggable|string $detail): void + public function handle(string $activityType, Loggable|string $detail, User $user): void { // TODO } diff --git a/app/Activity/Notifications/Handlers/NotificationHandler.php b/app/Activity/Notifications/Handlers/NotificationHandler.php index fdf97eb79..fecca2181 100644 --- a/app/Activity/Notifications/Handlers/NotificationHandler.php +++ b/app/Activity/Notifications/Handlers/NotificationHandler.php @@ -3,11 +3,14 @@ namespace BookStack\Activity\Notifications\Handlers; use BookStack\Activity\Models\Loggable; +use BookStack\Users\Models\User; interface NotificationHandler { /** * Run this handler. + * Provides the activity type, related activity detail/model + * along with the user that triggered the activity. */ - public function handle(string $activityType, string|Loggable $detail): void; + public function handle(string $activityType, string|Loggable $detail, User $user): void; } diff --git a/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php b/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php index 20189fc1e..a61df48ae 100644 --- a/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php @@ -3,11 +3,32 @@ namespace BookStack\Activity\Notifications\Handlers; use BookStack\Activity\Models\Loggable; +use BookStack\Activity\Models\Watch; +use BookStack\Activity\Tools\EntityWatchers; +use BookStack\Activity\WatchLevels; +use BookStack\Users\Models\User; class PageCreationNotificationHandler implements NotificationHandler { - public function handle(string $activityType, Loggable|string $detail): void + public function handle(string $activityType, Loggable|string $detail, User $user): void { // TODO + + // No user-level preferences to care about here. + // Possible Scenarios: + // ✅ User watching parent chapter + // ✅ User watching parent book + // ❌ User ignoring parent book + // ❌ User ignoring parent chapter + // ❌ User watching parent book, ignoring chapter + // ✅ User watching parent book, watching chapter + // ❌ User ignoring parent book, ignoring chapter + // ✅ User ignoring parent book, watching chapter + + // Get all relevant watchers + $watchers = new EntityWatchers($detail, WatchLevels::NEW); + + // TODO - need to check entity visibility and receive-notifications permissions. + // Maybe abstract this to a generic late-stage filter? } } diff --git a/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php b/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php index ef71dccbf..bbd189d52 100644 --- a/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php @@ -3,10 +3,11 @@ namespace BookStack\Activity\Notifications\Handlers; use BookStack\Activity\Models\Loggable; +use BookStack\Users\Models\User; class PageUpdateNotificationHandler implements NotificationHandler { - public function handle(string $activityType, Loggable|string $detail): void + public function handle(string $activityType, Loggable|string $detail, User $user): void { // TODO } diff --git a/app/Activity/Notifications/LinkedMailMessageLine.php b/app/Activity/Notifications/LinkedMailMessageLine.php new file mode 100644 index 000000000..224d8e87c --- /dev/null +++ b/app/Activity/Notifications/LinkedMailMessageLine.php @@ -0,0 +1,26 @@ +url) . '">' . e($this->linkText) . ''; + return str_replace(':link', $link, e($this->line)); + } +} diff --git a/app/Activity/Notifications/Messages/BaseActivityNotification.php b/app/Activity/Notifications/Messages/BaseActivityNotification.php new file mode 100644 index 000000000..285e2803e --- /dev/null +++ b/app/Activity/Notifications/Messages/BaseActivityNotification.php @@ -0,0 +1,50 @@ + $this->detail, + 'activity_creator' => $this->user, + ]; + } +} diff --git a/app/Activity/Notifications/Messages/PageCreationNotification.php b/app/Activity/Notifications/Messages/PageCreationNotification.php new file mode 100644 index 000000000..2e9a6debc --- /dev/null +++ b/app/Activity/Notifications/Messages/PageCreationNotification.php @@ -0,0 +1,28 @@ +detail; + + return (new MailMessage()) + ->subject("New Page: " . $page->getShortName()) + ->line("A new page has been created in " . setting('app-name') . ':') + ->line("Page Name: " . $page->name) + ->line("Created By: " . $this->user->name) + ->action('View Page', $page->getUrl()) + ->line(new LinkedMailMessageLine( + url('/preferences/notifications'), + 'This notification was sent to you because :link cover this type of activity for this item.', + 'your notification preferences', + )); + } +} diff --git a/app/Activity/Notifications/NotificationManager.php b/app/Activity/Notifications/NotificationManager.php index 5864b8456..01361c1ee 100644 --- a/app/Activity/Notifications/NotificationManager.php +++ b/app/Activity/Notifications/NotificationManager.php @@ -8,6 +8,7 @@ use BookStack\Activity\Notifications\Handlers\CommentCreationNotificationHandler use BookStack\Activity\Notifications\Handlers\NotificationHandler; use BookStack\Activity\Notifications\Handlers\PageCreationNotificationHandler; use BookStack\Activity\Notifications\Handlers\PageUpdateNotificationHandler; +use BookStack\Users\Models\User; class NotificationManager { @@ -16,13 +17,13 @@ class NotificationManager */ protected array $handlers = []; - public function handle(string $activityType, string|Loggable $detail): void + public function handle(string $activityType, string|Loggable $detail, User $user): void { $handlersToRun = $this->handlers[$activityType] ?? []; foreach ($handlersToRun as $handlerClass) { /** @var NotificationHandler $handler */ $handler = app()->make($handlerClass); - $handler->handle($activityType, $detail); + $handler->handle($activityType, $detail, $user); } } diff --git a/app/Activity/Tools/ActivityLogger.php b/app/Activity/Tools/ActivityLogger.php index 3135f57a7..e8ea7c293 100644 --- a/app/Activity/Tools/ActivityLogger.php +++ b/app/Activity/Tools/ActivityLogger.php @@ -40,7 +40,7 @@ class ActivityLogger $this->setNotification($type); $this->dispatchWebhooks($type, $detail); - $this->notifications->handle($type, $detail); + $this->notifications->handle($type, $detail, user()); Theme::dispatch(ThemeEvents::ACTIVITY_LOGGED, $type, $detail); } diff --git a/app/Activity/Tools/EntityWatchers.php b/app/Activity/Tools/EntityWatchers.php new file mode 100644 index 000000000..20375ef45 --- /dev/null +++ b/app/Activity/Tools/EntityWatchers.php @@ -0,0 +1,55 @@ +build(); + } + + protected function build(): void + { + $watches = $this->getRelevantWatches(); + + // TODO - De-dupe down watches per-user across entity types + // so we end up with [user_id => status] values + // then filter to current watch level, considering ignores, + // then populate the class watchers/ignores with ids. + } + + protected function getRelevantWatches(): array + { + /** @var Entity[] $entitiesInvolved */ + $entitiesInvolved = array_filter([ + $this->entity, + $this->entity instanceof BookChild ? $this->entity->book : null, + $this->entity instanceof Page ? $this->entity->chapter : null, + ]); + + $query = Watch::query()->where(function (Builder $query) use ($entitiesInvolved) { + foreach ($entitiesInvolved as $entity) { + $query->orWhere(function (Builder $query) use ($entity) { + $query->where('watchable_type', '=', $entity->getMorphClass()) + ->where('watchable_id', '=', $entity->id); + }); + } + }); + + return $query->get([ + 'level', 'watchable_id', 'watchable_type', 'user_id' + ])->all(); + } +} diff --git a/app/Activity/Tools/UserWatchOptions.php b/app/Activity/Tools/UserWatchOptions.php index 0607d60a3..64c5f317f 100644 --- a/app/Activity/Tools/UserWatchOptions.php +++ b/app/Activity/Tools/UserWatchOptions.php @@ -3,20 +3,13 @@ namespace BookStack\Activity\Tools; use BookStack\Activity\Models\Watch; +use BookStack\Activity\WatchLevels; use BookStack\Entities\Models\Entity; use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Builder; class UserWatchOptions { - protected static array $levelByName = [ - 'default' => -1, - 'ignore' => 0, - 'new' => 1, - 'updates' => 2, - 'comments' => 3, - ]; - public function __construct( protected User $user, ) { @@ -30,7 +23,7 @@ class UserWatchOptions public function getEntityWatchLevel(Entity $entity): string { $levelValue = $this->entityQuery($entity)->first(['level'])->level ?? -1; - return $this->levelValueToName($levelValue); + return WatchLevels::levelValueToName($levelValue); } public function isWatching(Entity $entity): bool @@ -40,7 +33,7 @@ class UserWatchOptions public function updateEntityWatchLevel(Entity $entity, string $level): void { - $levelValue = $this->levelNameToValue($level); + $levelValue = WatchLevels::levelNameToValue($level); if ($levelValue < 0) { $this->removeForEntity($entity); return; @@ -71,28 +64,4 @@ class UserWatchOptions ->where('watchable_type', '=', $entity->getMorphClass()) ->where('user_id', '=', $this->user->id); } - - /** - * @return string[] - */ - public static function getAvailableLevelNames(): array - { - return array_keys(static::$levelByName); - } - - protected static function levelNameToValue(string $level): int - { - return static::$levelByName[$level] ?? -1; - } - - protected static function levelValueToName(int $level): string - { - foreach (static::$levelByName as $name => $value) { - if ($level === $value) { - return $name; - } - } - - return 'default'; - } } diff --git a/app/Activity/WatchLevels.php b/app/Activity/WatchLevels.php new file mode 100644 index 000000000..2951bc7a8 --- /dev/null +++ b/app/Activity/WatchLevels.php @@ -0,0 +1,61 @@ + value array. + */ + public static function all(): array + { + $options = []; + foreach ((new \ReflectionClass(static::class))->getConstants() as $name => $value) { + $options[strtolower($name)] = $value; + } + + return $options; + } + + public static function levelNameToValue(string $level): int + { + return static::all()[$level] ?? -1; + } + + public static function levelValueToName(int $level): string + { + foreach (static::all() as $name => $value) { + if ($level === $value) { + return $name; + } + } + + return 'default'; + } +} diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php index e02db5800..adde98998 100644 --- a/resources/views/entities/watch-controls.blade.php +++ b/resources/views/entities/watch-controls.blade.php @@ -5,7 +5,7 @@
      - @foreach(\BookStack\Activity\Tools\UserWatchOptions::getAvailableLevelNames() as $option) + @foreach(\BookStack\Activity\WatchLevels::all() as $option)
    • \ No newline at end of file diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php index 5ad6108a0..4fdda5347 100644 --- a/resources/views/entities/watch-controls.blade.php +++ b/resources/views/entities/watch-controls.blade.php @@ -1,35 +1,42 @@ - - {{ method_field('PUT') }} - {{ csrf_field() }} - - +
    + + @icon(($ignoring ? 'watch-ignore' : 'watch')) + {{ $label }} + + + {{ method_field('PUT') }} + {{ csrf_field() }} + + -
      - @foreach(\BookStack\Activity\WatchLevels::all() as $option => $value) -
    • -
    - - + + +
  • +
    +
    • + @endforeach
  • -
    + {{ trans('entities.watch_change_default') }}
    • - @endforeach -
  • - {{ trans('entities.watch_change_default') }} -
    • - - \ No newline at end of file + + +
    \ No newline at end of file diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index aefcb96e1..7d44f677d 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -185,7 +185,7 @@
    - @if($watchOptions->canWatch() && !$watchOptions->isWatching($page)) + @if($watchOptions->canWatch() && !$watchOptions->isWatching()) @include('entities.watch-action', ['entity' => $page]) @endif @if(signedInUser()) From d9fdecd902c3542160e1a21a4cb3833e494baaa0 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 14 Aug 2023 13:11:18 +0100 Subject: [PATCH 12/20] Notifications: User watch list and differnt page watch options - Adds option filtering and alternative text for page watch options. - Adds "Watched & Ignored Items" list to user notification preferences page to show existing watched items. --- app/Activity/Models/Watch.php | 18 ++++++++-- app/Activity/WatchLevels.php | 34 +++++++++++++++++-- app/Entities/Queries/TopFavourites.php | 2 +- .../Controllers/UserPreferencesController.php | 9 ++++- lang/en/entities.php | 2 ++ lang/en/preferences.php | 2 ++ resources/views/entities/icon-link.blade.php | 7 ++++ .../views/entities/watch-controls.blade.php | 8 +++-- resources/views/settings/audit.blade.php | 8 +---- .../users/preferences/notifications.blade.php | 25 ++++++++++++++ 10 files changed, 99 insertions(+), 16 deletions(-) create mode 100644 resources/views/entities/icon-link.blade.php diff --git a/app/Activity/Models/Watch.php b/app/Activity/Models/Watch.php index 6637c9655..dfb72cc0a 100644 --- a/app/Activity/Models/Watch.php +++ b/app/Activity/Models/Watch.php @@ -2,10 +2,12 @@ namespace BookStack\Activity\Models; +use BookStack\Activity\WatchLevels; use BookStack\Permissions\Models\JointPermission; use Carbon\Carbon; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\HasMany; +use Illuminate\Database\Eloquent\Relations\MorphTo; /** * @property int $id @@ -20,14 +22,24 @@ class Watch extends Model { protected $guarded = []; - public function watchable() + public function watchable(): MorphTo { - $this->morphTo(); + return $this->morphTo(); } public function jointPermissions(): HasMany { return $this->hasMany(JointPermission::class, 'entity_id', 'watchable_id') - ->whereColumn('favourites.watchable_type', '=', 'joint_permissions.entity_type'); + ->whereColumn('watches.watchable_type', '=', 'joint_permissions.entity_type'); + } + + public function getLevelName(): string + { + return WatchLevels::levelValueToName($this->level); + } + + public function ignoring(): bool + { + return $this->level === WatchLevels::IGNORE; } } diff --git a/app/Activity/WatchLevels.php b/app/Activity/WatchLevels.php index 2951bc7a8..de3c5e122 100644 --- a/app/Activity/WatchLevels.php +++ b/app/Activity/WatchLevels.php @@ -2,6 +2,10 @@ namespace BookStack\Activity; +use BookStack\Entities\Models\Bookshelf; +use BookStack\Entities\Models\Entity; +use BookStack\Entities\Models\Page; + class WatchLevels { /** @@ -32,6 +36,7 @@ class WatchLevels /** * Get all the possible values as an option_name => value array. + * @returns array */ public static function all(): array { @@ -43,11 +48,36 @@ class WatchLevels return $options; } - public static function levelNameToValue(string $level): int + /** + * Get the watch options suited for the given entity. + * @returns array + */ + public static function allSuitedFor(Entity $entity): array { - return static::all()[$level] ?? -1; + $options = static::all(); + + if ($entity instanceof Page) { + unset($options['new']); + } elseif ($entity instanceof Bookshelf) { + return []; + } + + return $options; } + /** + * Convert the given name to a level value. + * Defaults to default value if the level does not exist. + */ + public static function levelNameToValue(string $level): int + { + return static::all()[$level] ?? static::DEFAULT; + } + + /** + * Convert the given int level value to a level name. + * Defaults to 'default' level name if not existing. + */ public static function levelValueToName(int $level): string { foreach (static::all() as $name => $value) { diff --git a/app/Entities/Queries/TopFavourites.php b/app/Entities/Queries/TopFavourites.php index 3f8d2e62e..cbccf35b0 100644 --- a/app/Entities/Queries/TopFavourites.php +++ b/app/Entities/Queries/TopFavourites.php @@ -10,7 +10,7 @@ class TopFavourites extends EntityQuery public function run(int $count, int $skip = 0) { $user = user(); - if (is_null($user) || $user->isDefault()) { + if ($user->isDefault()) { return collect(); } diff --git a/app/Users/Controllers/UserPreferencesController.php b/app/Users/Controllers/UserPreferencesController.php index faa99629b..999115e7b 100644 --- a/app/Users/Controllers/UserPreferencesController.php +++ b/app/Users/Controllers/UserPreferencesController.php @@ -2,7 +2,9 @@ namespace BookStack\Users\Controllers; +use BookStack\Activity\Models\Watch; use BookStack\Http\Controller; +use BookStack\Permissions\PermissionApplicator; use BookStack\Settings\UserNotificationPreferences; use BookStack\Settings\UserShortcutMap; use BookStack\Users\UserRepo; @@ -49,12 +51,17 @@ class UserPreferencesController extends Controller /** * Show the notification preferences for the current user. */ - public function showNotifications() + public function showNotifications(PermissionApplicator $permissions) { $preferences = (new UserNotificationPreferences(user())); + $query = Watch::query()->where('user_id', '=', user()->id); + $query = $permissions->restrictEntityRelationQuery($query, 'watches', 'watchable_id', 'watchable_type'); + $watches = $query->with('watchable')->paginate(20); + return view('users.preferences.notifications', [ 'preferences' => $preferences, + 'watches' => $watches, ]); } diff --git a/lang/en/entities.php b/lang/en/entities.php index f1e7dcc01..b1b0e5236 100644 --- a/lang/en/entities.php +++ b/lang/en/entities.php @@ -414,8 +414,10 @@ return [ 'watch_desc_new' => 'Notify when any new page is created within this item.', 'watch_title_updates' => 'All Page Updates', 'watch_desc_updates' => 'Notify upon all new pages and page changes.', + 'watch_desc_updates_page' => 'Notify upon all page changes.', 'watch_title_comments' => 'All Page Updates & Comments', 'watch_desc_comments' => 'Notify upon all new pages, page changes and new comments.', + 'watch_desc_comments_page' => 'Notify upon page changes and new comments.', 'watch_change_default' => 'Change default notification preferences', 'watch_detail_ignore' => 'Ignoring notifications', 'watch_detail_new' => 'Watching for new pages', diff --git a/lang/en/preferences.php b/lang/en/preferences.php index 2ade3a0e4..97968f8b1 100644 --- a/lang/en/preferences.php +++ b/lang/en/preferences.php @@ -23,4 +23,6 @@ return [ 'notifications_opt_comment_replies' => 'Notify upon replies to my comments', 'notifications_save' => 'Save Preferences', 'notifications_update_success' => 'Notification preferences have been updated!', + 'notifications_watched' => 'Watched & Ignored Items', + 'notifications_watched_desc' => ' Below are the items that have custom watch preferences applied. To update your preferences for these, view the item then find the watch options in the sidebar.', ]; diff --git a/resources/views/entities/icon-link.blade.php b/resources/views/entities/icon-link.blade.php new file mode 100644 index 000000000..a3e95fda8 --- /dev/null +++ b/resources/views/entities/icon-link.blade.php @@ -0,0 +1,7 @@ + + @icon($entity->getType()) +
    + {{ $entity->name }} +
    +     \ No newline at end of file diff --git a/resources/views/entities/watch-controls.blade.php b/resources/views/entities/watch-controls.blade.php index 4fdda5347..d24e12018 100644 --- a/resources/views/entities/watch-controls.blade.php +++ b/resources/views/entities/watch-controls.blade.php @@ -11,7 +11,7 @@
      - @foreach(\BookStack\Activity\WatchLevels::all() as $option => $value) + @foreach(\BookStack\Activity\WatchLevels::allSuitedFor($entity) as $option => $value)
    • diff --git a/resources/views/settings/audit.blade.php b/resources/views/settings/audit.blade.php index dd0874c92..9f0572c1a 100644 --- a/resources/views/settings/audit.blade.php +++ b/resources/views/settings/audit.blade.php @@ -95,13 +95,7 @@ : {{ $activity->type }}
    @if($activity->entity) - - @icon($activity->entity->getType()) -
    - {{ $activity->entity->name }} -
    -     + @include('entities.icon-link', ['entity' => $activity->entity]) @elseif($activity->detail && $activity->isForEntity())
    {{ trans('settings.audit_deleted_item') }}
    diff --git a/resources/views/users/preferences/notifications.blade.php b/resources/views/users/preferences/notifications.blade.php index 3a301aeb6..3bbf78280 100644 --- a/resources/views/users/preferences/notifications.blade.php +++ b/resources/views/users/preferences/notifications.blade.php @@ -41,5 +41,30 @@ +
    +

    {{ trans('preferences.notifications_watched') }}

    +

    {{ trans('preferences.notifications_watched_desc') }}

    + + @if($watches->isEmpty()) +

    {{ trans('common.no_items') }}

    + @else +
    + @foreach($watches as $watch) +
    +
    + @include('entities.icon-link', ['entity' => $watch->watchable]) +
    +
    + @icon('watch' . ($watch->ignoring() ? '-ignore' : '')) + {{ trans('entities.watch_title_' . $watch->getLevelName()) }} +
    +
    + @endforeach +
    + @endif + +
    {{ $watches->links() }}
    +
    +
    @stop From 371779205a29816b90fcf31bb6dba609aeb192fc Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Mon, 14 Aug 2023 17:29:12 +0100 Subject: [PATCH 13/20] Notifications: Added new preferences view and access control - Added general user preferences view and updated link in profile menu to suit. - Made notification permission required for notification preferences view, added test to cover. --- .../Controllers/UserPreferencesController.php | 11 +++++ lang/en/common.php | 1 + lang/en/preferences.php | 5 ++ resources/icons/user-preferences.svg | 1 + resources/views/common/header.blade.php | 6 +-- .../views/users/preferences/index.blade.php | 41 ++++++++++++++++ .../users/preferences/notifications.blade.php | 49 ++++++++++--------- routes/web.php | 2 +- tests/User/UserPreferencesTest.php | 36 +++++++++++++- 9 files changed, 123 insertions(+), 29 deletions(-) create mode 100644 resources/icons/user-preferences.svg create mode 100644 resources/views/users/preferences/index.blade.php diff --git a/app/Users/Controllers/UserPreferencesController.php b/app/Users/Controllers/UserPreferencesController.php index 999115e7b..d9ee50ca7 100644 --- a/app/Users/Controllers/UserPreferencesController.php +++ b/app/Users/Controllers/UserPreferencesController.php @@ -17,6 +17,14 @@ class UserPreferencesController extends Controller ) { } + /** + * Show the overview for user preferences. + */ + public function index() + { + return view('users.preferences.index'); + } + /** * Show the user-specific interface shortcuts. */ @@ -53,6 +61,8 @@ class UserPreferencesController extends Controller */ public function showNotifications(PermissionApplicator $permissions) { + $this->checkPermission('receive-notifications'); + $preferences = (new UserNotificationPreferences(user())); $query = Watch::query()->where('user_id', '=', user()->id); @@ -70,6 +80,7 @@ class UserPreferencesController extends Controller */ public function updateNotifications(Request $request) { + $this->checkPermission('receive-notifications'); $data = $this->validate($request, [ 'preferences' => ['required', 'array'], 'preferences.*' => ['required', 'string'], diff --git a/lang/en/common.php b/lang/en/common.php index de7937b2b..47b74d5b6 100644 --- a/lang/en/common.php +++ b/lang/en/common.php @@ -42,6 +42,7 @@ return [ 'remove' => 'Remove', 'add' => 'Add', 'configure' => 'Configure', + 'manage' => 'Manage', 'fullscreen' => 'Fullscreen', 'favourite' => 'Favourite', 'unfavourite' => 'Unfavourite', diff --git a/lang/en/preferences.php b/lang/en/preferences.php index 97968f8b1..118e8ba82 100644 --- a/lang/en/preferences.php +++ b/lang/en/preferences.php @@ -5,6 +5,8 @@ */ return [ + 'preferences' => 'Preferences', + 'shortcuts' => 'Shortcuts', 'shortcuts_interface' => 'Interface Keyboard Shortcuts', 'shortcuts_toggle_desc' => 'Here you can enable or disable keyboard system interface shortcuts, used for navigation and actions.', @@ -15,6 +17,7 @@ return [ 'shortcuts_save' => 'Save Shortcuts', 'shortcuts_overlay_desc' => 'Note: When shortcuts are enabled a helper overlay is available via pressing "?" which will highlight the available shortcuts for actions currently visible on the screen.', 'shortcuts_update_success' => 'Shortcut preferences have been updated!', + 'shortcuts_overview_desc' => 'Manage keyboard shortcuts you can use to navigate the system user interface.', 'notifications' => 'Notification Preferences', 'notifications_desc' => 'Control the email notifications you receive when certain activity is performed within the system.', @@ -25,4 +28,6 @@ return [ 'notifications_update_success' => 'Notification preferences have been updated!', 'notifications_watched' => 'Watched & Ignored Items', 'notifications_watched_desc' => ' Below are the items that have custom watch preferences applied. To update your preferences for these, view the item then find the watch options in the sidebar.', + + 'profile_overview_desc' => ' Manage your user profile details including preferred language and authentication options.', ]; diff --git a/resources/icons/user-preferences.svg b/resources/icons/user-preferences.svg new file mode 100644 index 000000000..5ae1773ca --- /dev/null +++ b/resources/icons/user-preferences.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/resources/views/common/header.blade.php b/resources/views/common/header.blade.php index a8b711595..97a411d84 100644 --- a/resources/views/common/header.blade.php +++ b/resources/views/common/header.blade.php @@ -104,9 +104,9 @@
  • - - @icon('shortcuts') -
    {{ trans('preferences.shortcuts') }}
    +     + @icon('user-preferences') +
    {{ trans('preferences.preferences') }}
  • diff --git a/resources/views/users/preferences/index.blade.php b/resources/views/users/preferences/index.blade.php new file mode 100644 index 000000000..a79245acb --- /dev/null +++ b/resources/views/users/preferences/index.blade.php @@ -0,0 +1,41 @@ +@extends('layouts.simple') + +@section('body') +
    + +
    +
    +

    {{ trans('preferences.shortcuts_interface') }}

    +

    {{ trans('preferences.shortcuts_overview_desc') }}

    +
    +
    + {{ trans('common.manage') }} +
    +
    + + @if(userCan('receive-notifications')) +
    +
    +

    {{ trans('preferences.notifications') }}

    +

    {{ trans('preferences.notifications_desc') }}

    +
    +
    + {{ trans('common.manage') }} +
    +
    + @endif + + @if(signedInUser()) +
    +
    +

    {{ trans('settings.users_edit_profile') }}

    +

    {{ trans('preferences.profile_overview_desc') }}

    +
    +
    + {{ trans('common.manage') }} +
    +
    + @endif + +
    +@stop diff --git a/resources/views/users/preferences/notifications.blade.php b/resources/views/users/preferences/notifications.blade.php index 3bbf78280..ae89c087e 100644 --- a/resources/views/users/preferences/notifications.blade.php +++ b/resources/views/users/preferences/notifications.blade.php @@ -11,33 +11,36 @@

    {{ trans('preferences.notifications') }}

    {{ trans('preferences.notifications_desc') }}

    -
    -
    - @include('form.toggle-switch', [ - 'name' => 'preferences[own-page-changes]', - 'value' => $preferences->notifyOnOwnPageChanges(), - 'label' => trans('preferences.notifications_opt_own_page_changes'), - ]) +
    +
    +
    + @include('form.toggle-switch', [ + 'name' => 'preferences[own-page-changes]', + 'value' => $preferences->notifyOnOwnPageChanges(), + 'label' => trans('preferences.notifications_opt_own_page_changes'), + ]) +
    +
    + @include('form.toggle-switch', [ + 'name' => 'preferences[own-page-comments]', + 'value' => $preferences->notifyOnOwnPageComments(), + 'label' => trans('preferences.notifications_opt_own_page_comments'), + ]) +
    +
    + @include('form.toggle-switch', [ + 'name' => 'preferences[comment-replies]', + 'value' => $preferences->notifyOnCommentReplies(), + 'label' => trans('preferences.notifications_opt_comment_replies'), + ]) +
    -
    - @include('form.toggle-switch', [ - 'name' => 'preferences[own-page-comments]', - 'value' => $preferences->notifyOnOwnPageComments(), - 'label' => trans('preferences.notifications_opt_own_page_comments'), - ]) -
    -
    - @include('form.toggle-switch', [ - 'name' => 'preferences[comment-replies]', - 'value' => $preferences->notifyOnCommentReplies(), - 'label' => trans('preferences.notifications_opt_comment_replies'), - ]) + +
    +
    -
    - -
    diff --git a/routes/web.php b/routes/web.php index 27a54f8b4..c7fc92fc7 100644 --- a/routes/web.php +++ b/routes/web.php @@ -231,7 +231,7 @@ Route::middleware('auth')->group(function () { Route::delete('/settings/users/{id}', [UserControllers\UserController::class, 'destroy']); // User Preferences - Route::redirect('/preferences', '/'); + Route::get('/preferences', [UserControllers\UserPreferencesController::class, 'index']); Route::get('/preferences/shortcuts', [UserControllers\UserPreferencesController::class, 'showShortcuts']); Route::put('/preferences/shortcuts', [UserControllers\UserPreferencesController::class, 'updateShortcuts']); Route::get('/preferences/notifications', [UserControllers\UserPreferencesController::class, 'showNotifications']); diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index e83df5731..a30484bd2 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -6,6 +6,22 @@ use Tests\TestCase; class UserPreferencesTest extends TestCase { + public function test_index_view() + { + $resp = $this->asEditor()->get('/preferences'); + $resp->assertOk(); + $resp->assertSee('Interface Keyboard Shortcuts'); + $resp->assertSee('Edit Profile'); + } + + public function test_index_view_accessible_but_without_profile_for_guest_user() + { + $this->setSettings(['app-public' => 'true']); + $resp = $this->get('/preferences'); + $resp->assertOk(); + $resp->assertSee('Interface Keyboard Shortcuts'); + $resp->assertDontSee('Edit Profile'); + } public function test_interface_shortcuts_updating() { $this->asEditor(); @@ -45,12 +61,28 @@ class UserPreferencesTest extends TestCase $this->withHtml($this->get('/'))->assertElementExists('body[component="shortcuts"]'); } + public function test_notification_routes_requires_notification_permission() + { + $viewer = $this->users->viewer(); + $resp = $this->actingAs($viewer)->get('/preferences/notifications'); + $this->assertPermissionError($resp); + + $resp = $this->put('/preferences/notifications'); + $this->assertPermissionError($resp); + + $this->permissions->grantUserRolePermissions($viewer, ['receive-notifications']); + $resp = $this->get('/preferences/notifications'); + $resp->assertOk(); + $resp->assertSee('Notification Preferences'); + } + public function test_notification_preferences_updating() { - $this->asEditor(); + $editor = $this->users->editor(); + $this->permissions->grantUserRolePermissions($editor, ['receive-notifications']); // View preferences with defaults - $resp = $this->get('/preferences/notifications'); + $resp = $this->actingAs($editor)->get('/preferences/notifications'); $resp->assertSee('Notification Preferences'); $html = $this->withHtml($resp); From 615741af9d36c725299cc8fc9c0ee012bd3d5759 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 15 Aug 2023 14:39:39 +0100 Subject: [PATCH 14/20] Notifications: Cleaned up mails, added debounce for updates - Updated mail notification design to be a bit prettier, and extracted text to new lang file for translation. - Added debounce logic for page update notifications. - Fixed watch options not being filtered to current user. --- app/Activity/Models/Comment.php | 11 +++----- .../Handlers/BaseNotificationHandler.php | 5 ++-- .../CommentCreationNotificationHandler.php | 11 ++++---- .../Handlers/NotificationHandler.php | 5 ++-- .../PageCreationNotificationHandler.php | 5 ++-- .../PageUpdateNotificationHandler.php | 22 ++++++++++++++-- .../LinkedMailMessageLine.php | 2 +- .../MessageParts/ListMessageLine.php | 26 +++++++++++++++++++ .../Messages/BaseActivityNotification.php | 13 ++++++++++ .../Messages/CommentCreationNotification.php | 22 +++++++--------- .../Messages/PageCreationNotification.php | 20 +++++++------- .../Messages/PageUpdateNotification.php | 22 +++++++--------- .../Notifications/NotificationManager.php | 6 +++-- app/Activity/Tools/ActivityLogger.php | 2 +- app/Activity/Tools/UserEntityWatchOptions.php | 16 +++++++----- lang/en/notifications.php | 26 +++++++++++++++++++ .../vendor/notifications/email.blade.php | 8 +++--- 17 files changed, 152 insertions(+), 70 deletions(-) rename app/Activity/Notifications/{ => MessageParts}/LinkedMailMessageLine.php (91%) create mode 100644 app/Activity/Notifications/MessageParts/ListMessageLine.php create mode 100644 lang/en/notifications.php diff --git a/app/Activity/Models/Comment.php b/app/Activity/Models/Comment.php index 72098a3c3..bcbed6c56 100644 --- a/app/Activity/Models/Comment.php +++ b/app/Activity/Models/Comment.php @@ -5,6 +5,7 @@ namespace BookStack\Activity\Models; use BookStack\App\Model; use BookStack\Users\Models\HasCreatorAndUpdater; use Illuminate\Database\Eloquent\Factories\HasFactory; +use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\MorphTo; /** @@ -35,7 +36,7 @@ class Comment extends Model implements Loggable /** * Get the parent comment this is in reply to (if existing). */ - public function parent() + public function parent(): BelongsTo { return $this->belongsTo(Comment::class); } @@ -50,20 +51,16 @@ class Comment extends Model implements Loggable /** * Get created date as a relative diff. - * - * @return mixed */ - public function getCreatedAttribute() + public function getCreatedAttribute(): string { return $this->created_at->diffForHumans(); } /** * Get updated date as a relative diff. - * - * @return mixed */ - public function getUpdatedAttribute() + public function getUpdatedAttribute(): string { return $this->updated_at->diffForHumans(); } diff --git a/app/Activity/Notifications/Handlers/BaseNotificationHandler.php b/app/Activity/Notifications/Handlers/BaseNotificationHandler.php index e0b3f3ceb..f1742592e 100644 --- a/app/Activity/Notifications/Handlers/BaseNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/BaseNotificationHandler.php @@ -2,6 +2,7 @@ namespace BookStack\Activity\Notifications\Handlers; +use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\Messages\BaseActivityNotification; use BookStack\Entities\Models\Entity; use BookStack\Permissions\PermissionApplicator; @@ -18,7 +19,7 @@ abstract class BaseNotificationHandler implements NotificationHandler * @param class-string $notification * @param int[] $userIds */ - protected function sendNotificationToUserIds(string $notification, array $userIds, User $initiator, Entity $relatedModel): void + protected function sendNotificationToUserIds(string $notification, array $userIds, User $initiator, string|Loggable $detail, Entity $relatedModel): void { $users = User::query()->whereIn('id', array_unique($userIds))->get(); @@ -39,7 +40,7 @@ abstract class BaseNotificationHandler implements NotificationHandler } // Send the notification - $user->notify(new $notification($relatedModel, $initiator)); + $user->notify(new $notification($detail, $initiator)); } } } diff --git a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php index 27d61307a..112852cf9 100644 --- a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php @@ -2,6 +2,7 @@ namespace BookStack\Activity\Notifications\Handlers; +use BookStack\Activity\Models\Activity; use BookStack\Activity\Models\Comment; use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\Messages\CommentCreationNotification; @@ -12,7 +13,7 @@ use BookStack\Users\Models\User; class CommentCreationNotificationHandler extends BaseNotificationHandler { - public function handle(string $activityType, Loggable|string $detail, User $user): void + public function handle(Activity $activity, Loggable|string $detail, User $user): void { if (!($detail instanceof Comment)) { throw new \InvalidArgumentException("Detail for comment creation notifications must be a comment"); @@ -24,10 +25,10 @@ class CommentCreationNotificationHandler extends BaseNotificationHandler $watcherIds = $watchers->getWatcherUserIds(); // Page owner if user preferences allow - if (!$watchers->isUserIgnoring($detail->owned_by) && $detail->ownedBy) { - $userNotificationPrefs = new UserNotificationPreferences($detail->ownedBy); + if (!$watchers->isUserIgnoring($detail->created_by) && $detail->createdBy) { + $userNotificationPrefs = new UserNotificationPreferences($detail->createdBy); if ($userNotificationPrefs->notifyOnOwnPageComments()) { - $watcherIds[] = $detail->owned_by; + $watcherIds[] = $detail->created_by; } } @@ -40,6 +41,6 @@ class CommentCreationNotificationHandler extends BaseNotificationHandler } } - $this->sendNotificationToUserIds(CommentCreationNotification::class, $watcherIds, $user, $page); + $this->sendNotificationToUserIds(CommentCreationNotification::class, $watcherIds, $user, $detail, $page); } } diff --git a/app/Activity/Notifications/Handlers/NotificationHandler.php b/app/Activity/Notifications/Handlers/NotificationHandler.php index fecca2181..8c5498664 100644 --- a/app/Activity/Notifications/Handlers/NotificationHandler.php +++ b/app/Activity/Notifications/Handlers/NotificationHandler.php @@ -2,6 +2,7 @@ namespace BookStack\Activity\Notifications\Handlers; +use BookStack\Activity\Models\Activity; use BookStack\Activity\Models\Loggable; use BookStack\Users\Models\User; @@ -9,8 +10,8 @@ interface NotificationHandler { /** * Run this handler. - * Provides the activity type, related activity detail/model + * Provides the activity, related activity detail/model * along with the user that triggered the activity. */ - public function handle(string $activityType, string|Loggable $detail, User $user): void; + public function handle(Activity $activity, string|Loggable $detail, User $user): void; } diff --git a/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php b/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php index e9aca2f23..2492021e2 100644 --- a/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/PageCreationNotificationHandler.php @@ -2,6 +2,7 @@ namespace BookStack\Activity\Notifications\Handlers; +use BookStack\Activity\Models\Activity; use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\Messages\PageCreationNotification; use BookStack\Activity\Tools\EntityWatchers; @@ -11,13 +12,13 @@ use BookStack\Users\Models\User; class PageCreationNotificationHandler extends BaseNotificationHandler { - public function handle(string $activityType, Loggable|string $detail, User $user): void + public function handle(Activity $activity, Loggable|string $detail, User $user): void { if (!($detail instanceof Page)) { throw new \InvalidArgumentException("Detail for page create notifications must be a page"); } $watchers = new EntityWatchers($detail, WatchLevels::NEW); - $this->sendNotificationToUserIds(PageCreationNotification::class, $watchers->getWatcherUserIds(), $user, $detail); + $this->sendNotificationToUserIds(PageCreationNotification::class, $watchers->getWatcherUserIds(), $user, $detail, $detail); } } diff --git a/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php b/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php index 5a2bf4e9c..744aba18f 100644 --- a/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/PageUpdateNotificationHandler.php @@ -2,6 +2,8 @@ namespace BookStack\Activity\Notifications\Handlers; +use BookStack\Activity\ActivityType; +use BookStack\Activity\Models\Activity; use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\Messages\PageUpdateNotification; use BookStack\Activity\Tools\EntityWatchers; @@ -12,15 +14,31 @@ use BookStack\Users\Models\User; class PageUpdateNotificationHandler extends BaseNotificationHandler { - public function handle(string $activityType, Loggable|string $detail, User $user): void + public function handle(Activity $activity, Loggable|string $detail, User $user): void { if (!($detail instanceof Page)) { throw new \InvalidArgumentException("Detail for page update notifications must be a page"); } + // Get last update from activity + $lastUpdate = $detail->activity() + ->where('type', '=', ActivityType::PAGE_UPDATE) + ->where('id', '!=', $activity->id) + ->latest('created_at') + ->first(); + + // Return if the same user has already updated the page in the last 15 mins + if ($lastUpdate && $lastUpdate->user_id === $user->id) { + if ($lastUpdate->created_at->gt(now()->subMinutes(15))) { + return; + } + } + + // Get active watchers $watchers = new EntityWatchers($detail, WatchLevels::UPDATES); $watcherIds = $watchers->getWatcherUserIds(); + // Add page owner if preferences allow if (!$watchers->isUserIgnoring($detail->owned_by) && $detail->ownedBy) { $userNotificationPrefs = new UserNotificationPreferences($detail->ownedBy); if ($userNotificationPrefs->notifyOnOwnPageChanges()) { @@ -28,6 +46,6 @@ class PageUpdateNotificationHandler extends BaseNotificationHandler } } - $this->sendNotificationToUserIds(PageUpdateNotification::class, $watcherIds, $user, $detail); + $this->sendNotificationToUserIds(PageUpdateNotification::class, $watcherIds, $user, $detail, $detail); } } diff --git a/app/Activity/Notifications/LinkedMailMessageLine.php b/app/Activity/Notifications/MessageParts/LinkedMailMessageLine.php similarity index 91% rename from app/Activity/Notifications/LinkedMailMessageLine.php rename to app/Activity/Notifications/MessageParts/LinkedMailMessageLine.php index 224d8e87c..8f6a4e2b9 100644 --- a/app/Activity/Notifications/LinkedMailMessageLine.php +++ b/app/Activity/Notifications/MessageParts/LinkedMailMessageLine.php @@ -1,6 +1,6 @@ list as $header => $content) { + $list[] = '' . e($header) . ' ' . e($content); + } + return implode("
    \n", $list); + } +} diff --git a/app/Activity/Notifications/Messages/BaseActivityNotification.php b/app/Activity/Notifications/Messages/BaseActivityNotification.php index 285e2803e..eb6eb0cc8 100644 --- a/app/Activity/Notifications/Messages/BaseActivityNotification.php +++ b/app/Activity/Notifications/Messages/BaseActivityNotification.php @@ -3,6 +3,7 @@ namespace BookStack\Activity\Notifications\Messages; use BookStack\Activity\Models\Loggable; +use BookStack\Activity\Notifications\MessageParts\LinkedMailMessageLine; use BookStack\Users\Models\User; use Illuminate\Bus\Queueable; use Illuminate\Notifications\Messages\MailMessage; @@ -47,4 +48,16 @@ abstract class BaseActivityNotification extends Notification 'activity_creator' => $this->user, ]; } + + /** + * Build the common reason footer line used in mail messages. + */ + protected function buildReasonFooterLine(): LinkedMailMessageLine + { + return new LinkedMailMessageLine( + url('/preferences/notifications'), + trans('notifications.footer_reason'), + trans('notifications.footer_reason_link'), + ); + } } diff --git a/app/Activity/Notifications/Messages/CommentCreationNotification.php b/app/Activity/Notifications/Messages/CommentCreationNotification.php index 817eb7b84..ce358724b 100644 --- a/app/Activity/Notifications/Messages/CommentCreationNotification.php +++ b/app/Activity/Notifications/Messages/CommentCreationNotification.php @@ -3,7 +3,7 @@ namespace BookStack\Activity\Notifications\Messages; use BookStack\Activity\Models\Comment; -use BookStack\Activity\Notifications\LinkedMailMessageLine; +use BookStack\Activity\Notifications\MessageParts\ListMessageLine; use BookStack\Entities\Models\Page; use Illuminate\Notifications\Messages\MailMessage; @@ -17,16 +17,14 @@ class CommentCreationNotification extends BaseActivityNotification $page = $comment->entity; return (new MailMessage()) - ->subject("New Comment on Page: " . $page->getShortName()) - ->line("A user has commented on a page in " . setting('app-name') . ':') - ->line("Page Name: " . $page->name) - ->line("Commenter: " . $this->user->name) - ->line("Comment: " . strip_tags($comment->html)) - ->action('View Comment', $page->getUrl('#comment' . $comment->local_id)) - ->line(new LinkedMailMessageLine( - url('/preferences/notifications'), - 'This notification was sent to you because :link cover this type of activity for this item.', - 'your notification preferences', - )); + ->subject(trans('notifications.new_comment_subject', ['pageName' => $page->getShortName()])) + ->line(trans('notifications.new_comment_intro', ['appName' => setting('app-name')])) + ->line(new ListMessageLine([ + trans('notifications.detail_page_name') => $page->name, + trans('notifications.detail_commenter') => $this->user->name, + trans('notifications.detail_comment') => strip_tags($comment->html), + ])) + ->action(trans('notifications.action_view_comment'), $page->getUrl('#comment' . $comment->local_id)) + ->line($this->buildReasonFooterLine()); } } diff --git a/app/Activity/Notifications/Messages/PageCreationNotification.php b/app/Activity/Notifications/Messages/PageCreationNotification.php index 2e9a6debc..068f95acc 100644 --- a/app/Activity/Notifications/Messages/PageCreationNotification.php +++ b/app/Activity/Notifications/Messages/PageCreationNotification.php @@ -2,7 +2,7 @@ namespace BookStack\Activity\Notifications\Messages; -use BookStack\Activity\Notifications\LinkedMailMessageLine; +use BookStack\Activity\Notifications\MessageParts\ListMessageLine; use BookStack\Entities\Models\Page; use Illuminate\Notifications\Messages\MailMessage; @@ -14,15 +14,13 @@ class PageCreationNotification extends BaseActivityNotification $page = $this->detail; return (new MailMessage()) - ->subject("New Page: " . $page->getShortName()) - ->line("A new page has been created in " . setting('app-name') . ':') - ->line("Page Name: " . $page->name) - ->line("Created By: " . $this->user->name) - ->action('View Page', $page->getUrl()) - ->line(new LinkedMailMessageLine( - url('/preferences/notifications'), - 'This notification was sent to you because :link cover this type of activity for this item.', - 'your notification preferences', - )); + ->subject(trans('notifications.new_page_subject', ['pageName' => $page->getShortName()])) + ->line(trans('notifications.new_page_intro', ['appName' => setting('app-name')])) + ->line(new ListMessageLine([ + trans('notifications.detail_page_name') => $page->name, + trans('notifications.detail_created_by') => $this->user->name, + ])) + ->action(trans('notifications.action_view_page'), $page->getUrl()) + ->line($this->buildReasonFooterLine()); } } diff --git a/app/Activity/Notifications/Messages/PageUpdateNotification.php b/app/Activity/Notifications/Messages/PageUpdateNotification.php index f29f50dde..c4a6de0bd 100644 --- a/app/Activity/Notifications/Messages/PageUpdateNotification.php +++ b/app/Activity/Notifications/Messages/PageUpdateNotification.php @@ -2,7 +2,7 @@ namespace BookStack\Activity\Notifications\Messages; -use BookStack\Activity\Notifications\LinkedMailMessageLine; +use BookStack\Activity\Notifications\MessageParts\ListMessageLine; use BookStack\Entities\Models\Page; use Illuminate\Notifications\Messages\MailMessage; @@ -14,16 +14,14 @@ class PageUpdateNotification extends BaseActivityNotification $page = $this->detail; return (new MailMessage()) - ->subject("Updated Page: " . $page->getShortName()) - ->line("A page has been updated in " . setting('app-name') . ':') - ->line("Page Name: " . $page->name) - ->line("Updated By: " . $this->user->name) - ->line("To prevent a mass of notifications, for a while you won't be sent notifications for further edits to this page by the same editor.") - ->action('View Page', $page->getUrl()) - ->line(new LinkedMailMessageLine( - url('/preferences/notifications'), - 'This notification was sent to you because :link cover this type of activity for this item.', - 'your notification preferences', - )); + ->subject(trans('notifications.updated_page_subject', ['pageName' => $page->getShortName()])) + ->line(trans('notifications.updated_page_intro', ['appName' => setting('app-name')])) + ->line(new ListMessageLine([ + trans('notifications.detail_page_name') => $page->name, + trans('notifications.detail_updated_by') => $this->user->name, + ])) + ->line(trans('notifications.updated_page_debounce')) + ->action(trans('notifications.action_view_page'), $page->getUrl()) + ->line($this->buildReasonFooterLine()); } } diff --git a/app/Activity/Notifications/NotificationManager.php b/app/Activity/Notifications/NotificationManager.php index 01361c1ee..fc6a5f57c 100644 --- a/app/Activity/Notifications/NotificationManager.php +++ b/app/Activity/Notifications/NotificationManager.php @@ -3,6 +3,7 @@ namespace BookStack\Activity\Notifications; use BookStack\Activity\ActivityType; +use BookStack\Activity\Models\Activity; use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\Handlers\CommentCreationNotificationHandler; use BookStack\Activity\Notifications\Handlers\NotificationHandler; @@ -17,13 +18,14 @@ class NotificationManager */ protected array $handlers = []; - public function handle(string $activityType, string|Loggable $detail, User $user): void + public function handle(Activity $activity, string|Loggable $detail, User $user): void { + $activityType = $activity->type; $handlersToRun = $this->handlers[$activityType] ?? []; foreach ($handlersToRun as $handlerClass) { /** @var NotificationHandler $handler */ $handler = app()->make($handlerClass); - $handler->handle($activityType, $detail, $user); + $handler->handle($activity, $detail, $user); } } diff --git a/app/Activity/Tools/ActivityLogger.php b/app/Activity/Tools/ActivityLogger.php index e8ea7c293..adda36c1b 100644 --- a/app/Activity/Tools/ActivityLogger.php +++ b/app/Activity/Tools/ActivityLogger.php @@ -40,7 +40,7 @@ class ActivityLogger $this->setNotification($type); $this->dispatchWebhooks($type, $detail); - $this->notifications->handle($type, $detail, user()); + $this->notifications->handle($activity, $detail, user()); Theme::dispatch(ThemeEvents::ACTIVITY_LOGGED, $type, $detail); } diff --git a/app/Activity/Tools/UserEntityWatchOptions.php b/app/Activity/Tools/UserEntityWatchOptions.php index 26d830851..08fbe2e8d 100644 --- a/app/Activity/Tools/UserEntityWatchOptions.php +++ b/app/Activity/Tools/UserEntityWatchOptions.php @@ -76,14 +76,16 @@ class UserEntityWatchOptions $entities[] = $this->entity->chapter; } - $query = Watch::query()->where(function (Builder $subQuery) use ($entities) { - foreach ($entities as $entity) { - $subQuery->orWhere(function (Builder $whereQuery) use ($entity) { - $whereQuery->where('watchable_type', '=', $entity->getMorphClass()) + $query = Watch::query() + ->where('user_id', '=', $this->user->id) + ->where(function (Builder $subQuery) use ($entities) { + foreach ($entities as $entity) { + $subQuery->orWhere(function (Builder $whereQuery) use ($entity) { + $whereQuery->where('watchable_type', '=', $entity->getMorphClass()) ->where('watchable_id', '=', $entity->id); - }); - } - }); + }); + } + }); $this->watchMap = $query->get(['watchable_type', 'level']) ->pluck('level', 'watchable_type') diff --git a/lang/en/notifications.php b/lang/en/notifications.php new file mode 100644 index 000000000..5539ae9a9 --- /dev/null +++ b/lang/en/notifications.php @@ -0,0 +1,26 @@ + 'New comment on page: :pageName', + 'new_comment_intro' => 'A user has commented on a page in :appName:', + 'new_page_subject' => 'New page: :pageName', + 'new_page_intro' => 'A new page has been created in :appName:', + 'updated_page_subject' => 'Updated page: :pageName', + 'updated_page_intro' => 'A page has been updated in :appName:', + 'updated_page_debounce' => 'To prevent a mass of notifications, for a while you won\'t be sent notifications for further edits to this page by the same editor.', + + 'detail_page_name' => 'Page Name:', + 'detail_commenter' => 'Commenter:', + 'detail_comment' => 'Comment:', + 'detail_created_by' => 'Created By:', + 'detail_updated_by' => 'Updated By:', + + 'action_view_comment' => 'View Comment', + 'action_view_page' => 'View Page', + + 'footer_reason' => 'This notification was sent to you because :link cover this type of activity for this item.', + 'footer_reason_link' => 'your notification preferences', +]; diff --git a/resources/views/vendor/notifications/email.blade.php b/resources/views/vendor/notifications/email.blade.php index f73b87b59..88cdbd890 100644 --- a/resources/views/vendor/notifications/email.blade.php +++ b/resources/views/vendor/notifications/email.blade.php @@ -30,7 +30,7 @@ $style = [ /* Layout ------------------------------ */ - 'body' => 'margin: 0; padding: 0; width: 100%; background-color: #F2F4F6;', + 'body' => 'margin: 0; padding: 0; width: 100%; background-color: #F2F4F6;color:#444444;', 'email-wrapper' => 'width: 100%; margin: 0; padding: 0; background-color: #F2F4F6;', /* Masthead ----------------------- */ @@ -54,8 +54,8 @@ $style = [ 'anchor' => 'color: '.setting('app-color').';overflow-wrap: break-word;word-wrap: break-word;word-break: break-all;word-break:break-word;', 'header-1' => 'margin-top: 0; color: #2F3133; font-size: 19px; font-weight: bold; text-align: left;', - 'paragraph' => 'margin-top: 0; color: #74787E; font-size: 16px; line-height: 1.5em;', - 'paragraph-sub' => 'margin-top: 0; color: #74787E; font-size: 12px; line-height: 1.5em;', + 'paragraph' => 'margin-top: 0; color: #444444; font-size: 16px; line-height: 1.5em;', + 'paragraph-sub' => 'margin-top: 0; color: #444444; font-size: 12px; line-height: 1.5em;', 'paragraph-center' => 'text-align: center;', /* Buttons ------------------------------ */ @@ -147,7 +147,7 @@ $style = [ @foreach ($outroLines as $line) -

    +

    {{ $line }}

    @endforeach From bc6e19b2a1ef424d6901cf07c179e445deede08e Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Tue, 15 Aug 2023 20:08:27 +0100 Subject: [PATCH 15/20] Notifications: Added testing to cover controls --- app/Activity/Controllers/WatchController.php | 1 + database/seeders/DummyContentSeeder.php | 2 + tests/Activity/WatchTest.php | 152 +++++++++++++++++++ tests/User/UserPreferencesTest.php | 21 ++- 4 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 tests/Activity/WatchTest.php diff --git a/app/Activity/Controllers/WatchController.php b/app/Activity/Controllers/WatchController.php index e0596864c..e2e27ca4a 100644 --- a/app/Activity/Controllers/WatchController.php +++ b/app/Activity/Controllers/WatchController.php @@ -15,6 +15,7 @@ class WatchController extends Controller { public function update(Request $request) { + // TODO - Require notification permission $requestData = $this->validate($request, [ 'level' => ['required', 'string'], ]); diff --git a/database/seeders/DummyContentSeeder.php b/database/seeders/DummyContentSeeder.php index 0f030d671..f0d3ffcdb 100644 --- a/database/seeders/DummyContentSeeder.php +++ b/database/seeders/DummyContentSeeder.php @@ -27,6 +27,8 @@ class DummyContentSeeder extends Seeder // Create an editor user $editorUser = User::factory()->create(); $editorRole = Role::getRole('editor'); + $additionalEditorPerms = ['receive-notifications']; + $editorRole->permissions()->syncWithoutDetaching(RolePermission::whereIn('name', $additionalEditorPerms)->pluck('id')); $editorUser->attachRole($editorRole); // Create a viewer user diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php new file mode 100644 index 000000000..919e52608 --- /dev/null +++ b/tests/Activity/WatchTest.php @@ -0,0 +1,152 @@ +users->editor(); + $this->actingAs($editor); + + $entities = [$this->entities->book(), $this->entities->chapter(), $this->entities->page()]; + /** @var Entity $entity */ + foreach ($entities as $entity) { + $resp = $this->get($entity->getUrl()); + $this->withHtml($resp)->assertElementContains('form[action$="/watching/update"] button.icon-list-item', 'Watch'); + + $watchOptions = new UserEntityWatchOptions($editor, $entity); + $watchOptions->updateWatchLevel('comments'); + + $resp = $this->get($entity->getUrl()); + $this->withHtml($resp)->assertElementNotExists('form[action$="/watching/update"] button.icon-list-item'); + } + } + + public function test_watch_action_only_shows_with_permission() + { + $viewer = $this->users->viewer(); + $this->actingAs($viewer); + + $entities = [$this->entities->book(), $this->entities->chapter(), $this->entities->page()]; + /** @var Entity $entity */ + foreach ($entities as $entity) { + $resp = $this->get($entity->getUrl()); + $this->withHtml($resp)->assertElementNotExists('form[action$="/watching/update"] button.icon-list-item'); + } + + $this->permissions->grantUserRolePermissions($viewer, ['receive-notifications']); + + /** @var Entity $entity */ + foreach ($entities as $entity) { + $resp = $this->get($entity->getUrl()); + $this->withHtml($resp)->assertElementExists('form[action$="/watching/update"] button.icon-list-item'); + } + } + + public function test_watch_update() + { + $editor = $this->users->editor(); + $book = $this->entities->book(); + + $this->actingAs($editor)->get($book->getUrl()); + $resp = $this->put('/watching/update', [ + 'type' => get_class($book), + 'id' => $book->id, + 'level' => 'comments' + ]); + + $resp->assertRedirect($book->getUrl()); + $this->assertSessionHas('success'); + $this->assertDatabaseHas('watches', [ + 'watchable_id' => $book->id, + 'watchable_type' => $book->getMorphClass(), + 'user_id' => $editor->id, + 'level' => WatchLevels::COMMENTS, + ]); + + $resp = $this->put('/watching/update', [ + 'type' => get_class($book), + 'id' => $book->id, + 'level' => 'default' + ]); + $resp->assertRedirect($book->getUrl()); + $this->assertDatabaseMissing('watches', [ + 'watchable_id' => $book->id, + 'watchable_type' => $book->getMorphClass(), + 'user_id' => $editor->id, + ]); + } + + public function test_watch_detail_display_reflects_state() + { + $editor = $this->users->editor(); + $book = $this->entities->bookHasChaptersAndPages(); + $chapter = $book->chapters()->first(); + $page = $chapter->pages()->first(); + + (new UserEntityWatchOptions($editor, $book))->updateWatchLevel('updates'); + + $this->actingAs($editor)->get($book->getUrl())->assertSee('Watching new pages and updates'); + $this->get($chapter->getUrl())->assertSee('Watching via parent book'); + $this->get($page->getUrl())->assertSee('Watching via parent book'); + + (new UserEntityWatchOptions($editor, $chapter))->updateWatchLevel('comments'); + $this->get($chapter->getUrl())->assertSee('Watching new pages, updates & comments'); + $this->get($page->getUrl())->assertSee('Watching via parent chapter'); + + (new UserEntityWatchOptions($editor, $page))->updateWatchLevel('updates'); + $this->get($page->getUrl())->assertSee('Watching new pages and updates'); + } + + public function test_watch_detail_ignore_indicator_cascades() + { + $editor = $this->users->editor(); + $book = $this->entities->bookHasChaptersAndPages(); + (new UserEntityWatchOptions($editor, $book))->updateWatchLevel('ignore'); + + $this->actingAs($editor)->get($book->getUrl())->assertSee('Ignoring notifications'); + $this->get($book->chapters()->first()->getUrl())->assertSee('Ignoring via parent book'); + $this->get($book->pages()->first()->getUrl())->assertSee('Ignoring via parent book'); + } + + public function test_watch_option_menu_shows_current_active_state() + { + $editor = $this->users->editor(); + $book = $this->entities->book(); + $options = new UserEntityWatchOptions($editor, $book); + + $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); + $respHtml->assertElementNotExists('form[action$="/watching/update"] svg[data-icon="check-circle"]'); + + $options->updateWatchLevel('comments'); + $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); + $respHtml->assertElementExists('form[action$="/watching/update"] button[value="comments"] svg[data-icon="check-circle"]'); + + $options->updateWatchLevel('ignore'); + $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); + $respHtml->assertElementExists('form[action$="/watching/update"] button[value="ignore"] svg[data-icon="check-circle"]'); + } + + public function test_watch_option_menu_limits_options_for_pages() + { + $editor = $this->users->editor(); + $book = $this->entities->bookHasChaptersAndPages(); + (new UserEntityWatchOptions($editor, $book))->updateWatchLevel('ignore'); + + $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); + $respHtml->assertElementExists('form[action$="/watching/update"] button[name="level"][value="new"]'); + + $respHtml = $this->withHtml($this->get($book->pages()->first()->getUrl())); + $respHtml->assertElementExists('form[action$="/watching/update"] button[name="level"][value="updates"]'); + $respHtml->assertElementNotExists('form[action$="/watching/update"] button[name="level"][value="new"]'); + } + + // TODO - Guest user cannot see/set notifications + // TODO - Actual notification testing +} diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index a30484bd2..a0e7e063f 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -2,6 +2,7 @@ namespace Tests\User; +use BookStack\Activity\Tools\UserEntityWatchOptions; use Tests\TestCase; class UserPreferencesTest extends TestCase @@ -79,7 +80,6 @@ class UserPreferencesTest extends TestCase public function test_notification_preferences_updating() { $editor = $this->users->editor(); - $this->permissions->grantUserRolePermissions($editor, ['receive-notifications']); // View preferences with defaults $resp = $this->actingAs($editor)->get('/preferences/notifications'); @@ -102,6 +102,25 @@ class UserPreferencesTest extends TestCase $html->assertFieldHasValue('preferences[comment-replies]', 'true'); } + public function test_notification_preferences_show_watches() + { + $editor = $this->users->editor(); + $book = $this->entities->book(); + + $options = new UserEntityWatchOptions($editor, $book); + $options->updateWatchLevel('comments'); + + $resp = $this->actingAs($editor)->get('/preferences/notifications'); + $resp->assertSee($book->name); + $resp->assertSee('All Page Updates & Comments'); + + $options->updateWatchLevel('default'); + + $resp = $this->actingAs($editor)->get('/preferences/notifications'); + $resp->assertDontSee($book->name); + $resp->assertDontSee('All Page Updates & Comments'); + } + public function test_update_sort_preference() { $editor = $this->users->editor(); From 565908ef52f640f8028f71c7448abea939f31c1f Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 16 Aug 2023 16:02:00 +0100 Subject: [PATCH 16/20] Notifications: Add phpunit test for notification sending Covers core case scenarios, and check of notification content. --- app/Activity/Controllers/WatchController.php | 5 +- .../CommentCreationNotificationHandler.php | 8 +- app/Http/Controller.php | 10 ++ .../Controllers/UserPreferencesController.php | 2 + database/seeders/DummyContentSeeder.php | 2 +- tests/Activity/WatchTest.php | 166 +++++++++++++++++- tests/Helpers/UserRoleProvider.php | 8 + tests/User/UserPreferencesTest.php | 15 ++ 8 files changed, 208 insertions(+), 8 deletions(-) diff --git a/app/Activity/Controllers/WatchController.php b/app/Activity/Controllers/WatchController.php index e2e27ca4a..43908812b 100644 --- a/app/Activity/Controllers/WatchController.php +++ b/app/Activity/Controllers/WatchController.php @@ -2,7 +2,6 @@ namespace BookStack\Activity\Controllers; -use BookStack\Activity\Models\Watch; use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\App\Model; use BookStack\Entities\Models\Entity; @@ -15,7 +14,9 @@ class WatchController extends Controller { public function update(Request $request) { - // TODO - Require notification permission + $this->checkPermission('receive-notifications'); + $this->preventGuestAccess(); + $requestData = $this->validate($request, [ 'level' => ['required', 'string'], ]); diff --git a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php index 112852cf9..bc12c8566 100644 --- a/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/CommentCreationNotificationHandler.php @@ -8,6 +8,7 @@ use BookStack\Activity\Models\Loggable; use BookStack\Activity\Notifications\Messages\CommentCreationNotification; use BookStack\Activity\Tools\EntityWatchers; use BookStack\Activity\WatchLevels; +use BookStack\Entities\Models\Page; use BookStack\Settings\UserNotificationPreferences; use BookStack\Users\Models\User; @@ -20,15 +21,16 @@ class CommentCreationNotificationHandler extends BaseNotificationHandler } // Main watchers + /** @var Page $page */ $page = $detail->entity; $watchers = new EntityWatchers($page, WatchLevels::COMMENTS); $watcherIds = $watchers->getWatcherUserIds(); // Page owner if user preferences allow - if (!$watchers->isUserIgnoring($detail->created_by) && $detail->createdBy) { - $userNotificationPrefs = new UserNotificationPreferences($detail->createdBy); + if (!$watchers->isUserIgnoring($page->owned_by) && $page->ownedBy) { + $userNotificationPrefs = new UserNotificationPreferences($page->ownedBy); if ($userNotificationPrefs->notifyOnOwnPageComments()) { - $watcherIds[] = $detail->created_by; + $watcherIds[] = $page->owned_by; } } diff --git a/app/Http/Controller.php b/app/Http/Controller.php index 78b899d25..584cea3aa 100644 --- a/app/Http/Controller.php +++ b/app/Http/Controller.php @@ -66,6 +66,16 @@ abstract class Controller extends BaseController } } + /** + * Prevent access for guest users beyond this point. + */ + protected function preventGuestAccess(): void + { + if (!signedInUser()) { + $this->showPermissionError(); + } + } + /** * Check the current user's permissions against an ownable item otherwise throw an exception. */ diff --git a/app/Users/Controllers/UserPreferencesController.php b/app/Users/Controllers/UserPreferencesController.php index d9ee50ca7..d73bb2d0c 100644 --- a/app/Users/Controllers/UserPreferencesController.php +++ b/app/Users/Controllers/UserPreferencesController.php @@ -62,6 +62,7 @@ class UserPreferencesController extends Controller public function showNotifications(PermissionApplicator $permissions) { $this->checkPermission('receive-notifications'); + $this->preventGuestAccess(); $preferences = (new UserNotificationPreferences(user())); @@ -81,6 +82,7 @@ class UserPreferencesController extends Controller public function updateNotifications(Request $request) { $this->checkPermission('receive-notifications'); + $this->preventGuestAccess(); $data = $this->validate($request, [ 'preferences' => ['required', 'array'], 'preferences.*' => ['required', 'string'], diff --git a/database/seeders/DummyContentSeeder.php b/database/seeders/DummyContentSeeder.php index f0d3ffcdb..47e8d1d7c 100644 --- a/database/seeders/DummyContentSeeder.php +++ b/database/seeders/DummyContentSeeder.php @@ -27,7 +27,7 @@ class DummyContentSeeder extends Seeder // Create an editor user $editorUser = User::factory()->create(); $editorRole = Role::getRole('editor'); - $additionalEditorPerms = ['receive-notifications']; + $additionalEditorPerms = ['receive-notifications', 'comment-create-all']; $editorRole->permissions()->syncWithoutDetaching(RolePermission::whereIn('name', $additionalEditorPerms)->pluck('id')); $editorUser->attachRole($editorRole); diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php index 919e52608..d68bd271f 100644 --- a/tests/Activity/WatchTest.php +++ b/tests/Activity/WatchTest.php @@ -2,9 +2,14 @@ namespace Tests\Activity; +use BookStack\Activity\Notifications\Messages\CommentCreationNotification; +use BookStack\Activity\Notifications\Messages\PageCreationNotification; +use BookStack\Activity\Notifications\Messages\PageUpdateNotification; use BookStack\Activity\Tools\UserEntityWatchOptions; use BookStack\Activity\WatchLevels; use BookStack\Entities\Models\Entity; +use BookStack\Settings\UserNotificationPreferences; +use Illuminate\Support\Facades\Notification; use Tests\TestCase; class WatchTest extends TestCase @@ -83,6 +88,22 @@ class WatchTest extends TestCase ]); } + public function test_watch_update_fails_for_guest() + { + $this->setSettings(['app-public' => 'true']); + $guest = $this->users->guest(); + $this->permissions->grantUserRolePermissions($guest, ['receive-notifications']); + $book = $this->entities->book(); + + $resp = $this->put('/watching/update', [ + 'type' => get_class($book), + 'id' => $book->id, + 'level' => 'comments' + ]); + + $this->assertPermissionError($resp); + } + public function test_watch_detail_display_reflects_state() { $editor = $this->users->editor(); @@ -147,6 +168,147 @@ class WatchTest extends TestCase $respHtml->assertElementNotExists('form[action$="/watching/update"] button[name="level"][value="new"]'); } - // TODO - Guest user cannot see/set notifications - // TODO - Actual notification testing + public function test_notify_own_page_changes() + { + $editor = $this->users->editor(); + $entities = $this->entities->createChainBelongingToUser($editor); + $prefs = new UserNotificationPreferences($editor); + $prefs->updateFromSettingsArray(['own-page-changes' => 'true']); + + $notifications = Notification::fake(); + + $this->asAdmin(); + $this->entities->updatePage($entities['page'], ['name' => 'My updated page', 'html' => 'Hello']); + $notifications->assertSentTo($editor, PageUpdateNotification::class); + } + + public function test_notify_own_page_comments() + { + $editor = $this->users->editor(); + $entities = $this->entities->createChainBelongingToUser($editor); + $prefs = new UserNotificationPreferences($editor); + $prefs->updateFromSettingsArray(['own-page-comments' => 'true']); + + $notifications = Notification::fake(); + + $this->asAdmin()->post("/comment/{$entities['page']->id}", [ + 'text' => 'My new comment' + ]); + $notifications->assertSentTo($editor, CommentCreationNotification::class); + } + + public function test_notify_comment_replies() + { + $editor = $this->users->editor(); + $entities = $this->entities->createChainBelongingToUser($editor); + $prefs = new UserNotificationPreferences($editor); + $prefs->updateFromSettingsArray(['comment-replies' => 'true']); + + $notifications = Notification::fake(); + + $this->actingAs($editor)->post("/comment/{$entities['page']->id}", [ + 'text' => 'My new comment' + ]); + $comment = $entities['page']->comments()->first(); + + $this->asAdmin()->post("/comment/{$entities['page']->id}", [ + 'text' => 'My new comment response', + 'parent_id' => $comment->id, + ]); + $notifications->assertSentTo($editor, CommentCreationNotification::class); + } + + public function test_notify_watch_parent_book_ignore() + { + $editor = $this->users->editor(); + $entities = $this->entities->createChainBelongingToUser($editor); + $watches = new UserEntityWatchOptions($editor, $entities['book']); + $prefs = new UserNotificationPreferences($editor); + $watches->updateWatchLevel('ignore'); + $prefs->updateFromSettingsArray(['own-page-changes' => 'true', 'own-page-comments' => true]); + + $notifications = Notification::fake(); + + $this->asAdmin()->post("/comment/{$entities['page']->id}", [ + 'text' => 'My new comment response', + ]); + $this->entities->updatePage($entities['page'], ['name' => 'My updated page', 'html' => 'Hello']); + $notifications->assertNothingSent(); + } + + public function test_notify_watch_parent_book_comments() + { + $notifications = Notification::fake(); + $editor = $this->users->editor(); + $admin = $this->users->admin(); + $entities = $this->entities->createChainBelongingToUser($editor); + $watches = new UserEntityWatchOptions($editor, $entities['book']); + $watches->updateWatchLevel('comments'); + + // Comment post + $this->actingAs($admin)->post("/comment/{$entities['page']->id}", [ + 'text' => 'My new comment response', + ]); + + $notifications->assertSentTo($editor, function (CommentCreationNotification $notification) use ($editor, $admin, $entities) { + $mail = $notification->toMail($editor); + $mailContent = html_entity_decode(strip_tags($mail->render())); + return $mail->subject === 'New comment on page: ' . $entities['page']->getShortName() + && str_contains($mailContent, 'View Comment') + && str_contains($mailContent, 'Page Name: ' . $entities['page']->name) + && str_contains($mailContent, 'Commenter: ' . $admin->name) + && str_contains($mailContent, 'Comment: My new comment response'); + }); + } + + public function test_notify_watch_parent_book_updates() + { + $notifications = Notification::fake(); + $editor = $this->users->editor(); + $admin = $this->users->admin(); + $entities = $this->entities->createChainBelongingToUser($editor); + $watches = new UserEntityWatchOptions($editor, $entities['book']); + $watches->updateWatchLevel('updates'); + + $this->actingAs($admin); + $this->entities->updatePage($entities['page'], ['name' => 'Updated page', 'html' => 'new page content']); + + $notifications->assertSentTo($editor, function (PageUpdateNotification $notification) use ($editor, $admin) { + $mail = $notification->toMail($editor); + $mailContent = html_entity_decode(strip_tags($mail->render())); + return $mail->subject === 'Updated page: Updated page' + && str_contains($mailContent, 'View Page') + && str_contains($mailContent, 'Page Name: Updated page') + && str_contains($mailContent, 'Updated By: ' . $admin->name) + && str_contains($mailContent, 'you won\'t be sent notifications for further edits to this page by the same editor'); + }); + + // Test debounce + $notifications = Notification::fake(); + $this->entities->updatePage($entities['page'], ['name' => 'Updated page', 'html' => 'new page content']); + $notifications->assertNothingSentTo($editor); + } + + public function test_notify_watch_parent_book_new() + { + $notifications = Notification::fake(); + $editor = $this->users->editor(); + $admin = $this->users->admin(); + $entities = $this->entities->createChainBelongingToUser($editor); + $watches = new UserEntityWatchOptions($editor, $entities['book']); + $watches->updateWatchLevel('new'); + + $this->actingAs($admin)->get($entities['chapter']->getUrl('/create-page')); + $page = $entities['chapter']->pages()->where('draft', '=', true)->first(); + $this->post($page->getUrl(), ['name' => 'My new page', 'html' => 'My new page content']); + + $notifications->assertSentTo($editor, function (PageCreationNotification $notification) use ($editor, $admin) { + $mail = $notification->toMail($editor); + $mailContent = html_entity_decode(strip_tags($mail->render())); + return $mail->subject === 'New page: My new page' + && str_contains($mailContent, 'View Page') + && str_contains($mailContent, 'Page Name: My new page') + && str_contains($mailContent, 'Created By: ' . $admin->name); + }); + } } diff --git a/tests/Helpers/UserRoleProvider.php b/tests/Helpers/UserRoleProvider.php index b86e90394..2b7a3623d 100644 --- a/tests/Helpers/UserRoleProvider.php +++ b/tests/Helpers/UserRoleProvider.php @@ -50,6 +50,14 @@ class UserRoleProvider return $user; } + /** + * Get the system "guest" user. + */ + public function guest(): User + { + return User::where('system_name', '=', 'public')->firstOrFail(); + } + /** * Create a new fresh user without any relations. */ diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index a0e7e063f..bc023b4cd 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -121,6 +121,21 @@ class UserPreferencesTest extends TestCase $resp->assertDontSee('All Page Updates & Comments'); } + public function test_notification_preferences_not_accessible_to_guest() + { + $this->setSettings(['app-public' => 'true']); + $guest = $this->users->guest(); + $this->permissions->grantUserRolePermissions($guest, ['receive-notifications']); + + $resp = $this->get('/preferences/notifications'); + $this->assertPermissionError($resp); + + $resp = $this->put('/preferences/notifications', [ + 'preferences' => ['comment-replies' => 'true'], + ]); + $this->assertPermissionError($resp); + } + public function test_update_sort_preference() { $editor = $this->users->editor(); From 79470ea4b7a93e2aca5c6081c0dbdb3c5ef4d1a8 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 16 Aug 2023 20:15:49 +0100 Subject: [PATCH 17/20] Notifications: Made improvements from manual testing - Added titles for preference pages. - Added extra check for non-guest for notifications on preferences page. --- app/Users/Controllers/UserPreferencesController.php | 3 +++ resources/views/users/preferences/index.blade.php | 2 +- tests/User/UserPreferencesTest.php | 4 +++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/app/Users/Controllers/UserPreferencesController.php b/app/Users/Controllers/UserPreferencesController.php index d73bb2d0c..503aeaeb0 100644 --- a/app/Users/Controllers/UserPreferencesController.php +++ b/app/Users/Controllers/UserPreferencesController.php @@ -33,6 +33,8 @@ class UserPreferencesController extends Controller $shortcuts = UserShortcutMap::fromUserPreferences(); $enabled = setting()->getForCurrentUser('ui-shortcuts-enabled', false); + $this->setPageTitle(trans('preferences.shortcuts_interface')); + return view('users.preferences.shortcuts', [ 'shortcuts' => $shortcuts, 'enabled' => $enabled, @@ -70,6 +72,7 @@ class UserPreferencesController extends Controller $query = $permissions->restrictEntityRelationQuery($query, 'watches', 'watchable_id', 'watchable_type'); $watches = $query->with('watchable')->paginate(20); + $this->setPageTitle(trans('preferences.notifications')); return view('users.preferences.notifications', [ 'preferences' => $preferences, 'watches' => $watches, diff --git a/resources/views/users/preferences/index.blade.php b/resources/views/users/preferences/index.blade.php index a79245acb..689628370 100644 --- a/resources/views/users/preferences/index.blade.php +++ b/resources/views/users/preferences/index.blade.php @@ -13,7 +13,7 @@
    - @if(userCan('receive-notifications')) + @if(signedInUser() && userCan('receive-notifications'))

    {{ trans('preferences.notifications') }}

    diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index bc023b4cd..4ee04ea67 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -15,13 +15,15 @@ class UserPreferencesTest extends TestCase $resp->assertSee('Edit Profile'); } - public function test_index_view_accessible_but_without_profile_for_guest_user() + public function test_index_view_accessible_but_without_profile_and_notifications_for_guest_user() { $this->setSettings(['app-public' => 'true']); + $this->permissions->grantUserRolePermissions($this->users->guest(), ['receive-notifications']); $resp = $this->get('/preferences'); $resp->assertOk(); $resp->assertSee('Interface Keyboard Shortcuts'); $resp->assertDontSee('Edit Profile'); + $resp->assertDontSee('Notification'); } public function test_interface_shortcuts_updating() { From ee9e342b58a57ae909f0635dde26b96c37f8583c Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 17 Aug 2023 14:59:28 +0100 Subject: [PATCH 18/20] Notifications: Fixed issues causing failing tests - Ensured watch options passed in all meta template usage to fix failing scenarios where watch options did not exist. - Fixed testing issue caused by guest user permission caching. --- app/Users/Models/User.php | 7 +++++-- resources/views/books/show.blade.php | 2 +- resources/views/chapters/show.blade.php | 2 +- resources/views/pages/revision.blade.php | 2 +- resources/views/pages/show.blade.php | 2 +- resources/views/shelves/show.blade.php | 2 +- tests/Activity/WatchTest.php | 1 + tests/Helpers/UserRoleProvider.php | 2 +- tests/TestCase.php | 2 ++ 9 files changed, 14 insertions(+), 8 deletions(-) diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php index 08cab69fb..be3e9b9b3 100644 --- a/app/Users/Models/User.php +++ b/app/Users/Models/User.php @@ -88,8 +88,6 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon /** * This holds the default user when loaded. - * - * @var null|User */ protected static ?User $defaultUser = null; @@ -107,6 +105,11 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon return static::$defaultUser; } + public static function clearDefault(): void + { + static::$defaultUser = null; + } + /** * Check if the user is the default public user. */ diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php index 0fb98e304..75b01a379 100644 --- a/resources/views/books/show.blade.php +++ b/resources/views/books/show.blade.php @@ -70,7 +70,7 @@
    {{ trans('common.details') }}
    - @include('entities.meta', ['entity' => $book]) + @include('entities.meta', ['entity' => $book, 'watchOptions' => $watchOptions]) @if($book->hasPermissions())
    @if(userCan('restrictions-manage', $book)) diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php index 67a8b9fa5..3cb512ccc 100644 --- a/resources/views/chapters/show.blade.php +++ b/resources/views/chapters/show.blade.php @@ -67,7 +67,7 @@
    {{ trans('common.details') }}
    - @include('entities.meta', ['entity' => $chapter]) + @include('entities.meta', ['entity' => $chapter, 'watchOptions' => $watchOptions]) @if($book->hasPermissions())
    diff --git a/resources/views/pages/revision.blade.php b/resources/views/pages/revision.blade.php index b3208c211..c0831ef59 100644 --- a/resources/views/pages/revision.blade.php +++ b/resources/views/pages/revision.blade.php @@ -4,7 +4,7 @@
    {{ trans('common.details') }}
    - @include('entities.meta', ['entity' => $revision]) + @include('entities.meta', ['entity' => $revision, 'watchOptions' => null])
    @stop diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php index 7d44f677d..1cbb81980 100644 --- a/resources/views/pages/show.blade.php +++ b/resources/views/pages/show.blade.php @@ -81,7 +81,7 @@
    {{ trans('common.details') }}
    - @include('entities.meta', ['entity' => $page]) + @include('entities.meta', ['entity' => $page, 'watchOptions' => $watchOptions]) @if($book->hasPermissions())
    diff --git a/resources/views/shelves/show.blade.php b/resources/views/shelves/show.blade.php index 25b7a14fc..8019a557f 100644 --- a/resources/views/shelves/show.blade.php +++ b/resources/views/shelves/show.blade.php @@ -79,7 +79,7 @@
    {{ trans('common.details') }}
    - @include('entities.meta', ['entity' => $shelf]) + @include('entities.meta', ['entity' => $shelf, 'watchOptions' => null]) @if($shelf->hasPermissions())
    @if(userCan('restrictions-manage', $shelf)) diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php index d68bd271f..e8cb3cbd8 100644 --- a/tests/Activity/WatchTest.php +++ b/tests/Activity/WatchTest.php @@ -102,6 +102,7 @@ class WatchTest extends TestCase ]); $this->assertPermissionError($resp); + $guest->unsetRelations(); } public function test_watch_detail_display_reflects_state() diff --git a/tests/Helpers/UserRoleProvider.php b/tests/Helpers/UserRoleProvider.php index 2b7a3623d..3b2da369d 100644 --- a/tests/Helpers/UserRoleProvider.php +++ b/tests/Helpers/UserRoleProvider.php @@ -55,7 +55,7 @@ class UserRoleProvider */ public function guest(): User { - return User::where('system_name', '=', 'public')->firstOrFail(); + return User::getDefault(); } /** diff --git a/tests/TestCase.php b/tests/TestCase.php index 322ab0370..0ab0792bd 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -5,6 +5,7 @@ namespace Tests; use BookStack\Entities\Models\Entity; use BookStack\Settings\SettingService; use BookStack\Uploads\HttpFetcher; +use BookStack\Users\Models\User; use GuzzleHttp\Client; use GuzzleHttp\Handler\MockHandler; use GuzzleHttp\HandlerStack; @@ -46,6 +47,7 @@ abstract class TestCase extends BaseTestCase $this->permissions = new PermissionsProvider($this->users); $this->files = new FileProvider(); + User::clearDefault(); parent::setUp(); // We can uncomment the below to run tests with failings upon deprecations. From 38829f8a38a5bd90f9cce2535c79131c04ae01d6 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 17 Aug 2023 17:57:31 +0100 Subject: [PATCH 19/20] Notifications: Fixed send content permission checking Added test and changed logic to properly check the view permissions for the notification receiver before sending. Required change to permissions applicator to allow the user to be manually determined, and a service provider update to provide the class as a singleton without a specific user, so it checks the current logged in user on demand. --- .../Handlers/BaseNotificationHandler.php | 8 ++------ .../Notifications/NotificationManager.php | 2 +- app/App/Providers/AppServiceProvider.php | 5 +++++ app/Permissions/PermissionApplicator.php | 8 ++++++-- tests/Activity/WatchTest.php | 17 +++++++++++++++++ 5 files changed, 31 insertions(+), 9 deletions(-) diff --git a/app/Activity/Notifications/Handlers/BaseNotificationHandler.php b/app/Activity/Notifications/Handlers/BaseNotificationHandler.php index f1742592e..b5f339b2c 100644 --- a/app/Activity/Notifications/Handlers/BaseNotificationHandler.php +++ b/app/Activity/Notifications/Handlers/BaseNotificationHandler.php @@ -10,11 +10,6 @@ use BookStack\Users\Models\User; abstract class BaseNotificationHandler implements NotificationHandler { - public function __construct( - protected PermissionApplicator $permissionApplicator - ) { - } - /** * @param class-string $notification * @param int[] $userIds @@ -35,7 +30,8 @@ abstract class BaseNotificationHandler implements NotificationHandler } // Prevent sending if the user does not have access to the related content - if (!$this->permissionApplicator->checkOwnableUserAccess($relatedModel, 'view')) { + $permissions = new PermissionApplicator($user); + if (!$permissions->checkOwnableUserAccess($relatedModel, 'view')) { continue; } diff --git a/app/Activity/Notifications/NotificationManager.php b/app/Activity/Notifications/NotificationManager.php index fc6a5f57c..294f56ebb 100644 --- a/app/Activity/Notifications/NotificationManager.php +++ b/app/Activity/Notifications/NotificationManager.php @@ -24,7 +24,7 @@ class NotificationManager $handlersToRun = $this->handlers[$activityType] ?? []; foreach ($handlersToRun as $handlerClass) { /** @var NotificationHandler $handler */ - $handler = app()->make($handlerClass); + $handler = new $handlerClass(); $handler->handle($activity, $detail, $user); } } diff --git a/app/App/Providers/AppServiceProvider.php b/app/App/Providers/AppServiceProvider.php index 0c0895bf4..deb664ba6 100644 --- a/app/App/Providers/AppServiceProvider.php +++ b/app/App/Providers/AppServiceProvider.php @@ -9,6 +9,7 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Models\Chapter; use BookStack\Entities\Models\Page; use BookStack\Exceptions\BookStackExceptionHandlerPage; +use BookStack\Permissions\PermissionApplicator; use BookStack\Settings\SettingService; use BookStack\Util\CspService; use GuzzleHttp\Client; @@ -79,5 +80,9 @@ class AppServiceProvider extends ServiceProvider 'timeout' => 3, ]); }); + + $this->app->singleton(PermissionApplicator::class, function ($app) { + return new PermissionApplicator(null); + }); } } diff --git a/app/Permissions/PermissionApplicator.php b/app/Permissions/PermissionApplicator.php index b4fafaa9e..a796bdaee 100644 --- a/app/Permissions/PermissionApplicator.php +++ b/app/Permissions/PermissionApplicator.php @@ -8,7 +8,6 @@ use BookStack\Entities\Models\Page; use BookStack\Permissions\Models\EntityPermission; use BookStack\Users\Models\HasCreatorAndUpdater; use BookStack\Users\Models\HasOwner; -use BookStack\Users\Models\Role; use BookStack\Users\Models\User; use Illuminate\Database\Eloquent\Builder; use Illuminate\Database\Query\Builder as QueryBuilder; @@ -16,6 +15,11 @@ use InvalidArgumentException; class PermissionApplicator { + public function __construct( + protected ?User $user = null + ) { + } + /** * Checks if an entity has a restriction set upon it. * @@ -173,7 +177,7 @@ class PermissionApplicator */ protected function currentUser(): User { - return user(); + return $this->user ?? user(); } /** diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php index e8cb3cbd8..00f3f5e4d 100644 --- a/tests/Activity/WatchTest.php +++ b/tests/Activity/WatchTest.php @@ -312,4 +312,21 @@ class WatchTest extends TestCase && str_contains($mailContent, 'Created By: ' . $admin->name); }); } + + public function test_notifications_not_sent_if_lacking_view_permission_for_related_item() + { + $notifications = Notification::fake(); + $editor = $this->users->editor(); + $page = $this->entities->page(); + + $watches = new UserEntityWatchOptions($editor, $page); + $watches->updateWatchLevel('comments'); + $this->permissions->disableEntityInheritedPermissions($page); + + $this->asAdmin()->post("/comment/{$page->id}", [ + 'text' => 'My new comment response', + ])->assertOk(); + + $notifications->assertNothingSentTo($editor); + } } From e709caa0050d74e8cb4940760f5e5e21c49000a5 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Thu, 17 Aug 2023 18:10:34 +0100 Subject: [PATCH 20/20] Notifications: Switched testing from string to reference levels --- app/Activity/Controllers/WatchController.php | 2 +- app/Activity/Tools/UserEntityWatchOptions.php | 11 +++++--- tests/Activity/WatchTest.php | 26 +++++++++---------- tests/User/UserPreferencesTest.php | 5 ++-- 4 files changed, 25 insertions(+), 19 deletions(-) diff --git a/app/Activity/Controllers/WatchController.php b/app/Activity/Controllers/WatchController.php index 43908812b..3d7e18116 100644 --- a/app/Activity/Controllers/WatchController.php +++ b/app/Activity/Controllers/WatchController.php @@ -23,7 +23,7 @@ class WatchController extends Controller $watchable = $this->getValidatedModelFromRequest($request); $watchOptions = new UserEntityWatchOptions(user(), $watchable); - $watchOptions->updateWatchLevel($requestData['level']); + $watchOptions->updateLevelByName($requestData['level']); $this->showSuccessNotification(trans('activities.watch_update_level_notification')); diff --git a/app/Activity/Tools/UserEntityWatchOptions.php b/app/Activity/Tools/UserEntityWatchOptions.php index 08fbe2e8d..231204fdf 100644 --- a/app/Activity/Tools/UserEntityWatchOptions.php +++ b/app/Activity/Tools/UserEntityWatchOptions.php @@ -51,15 +51,20 @@ class UserEntityWatchOptions return null; } - public function updateWatchLevel(string $level): void + public function updateLevelByName(string $level): void { $levelValue = WatchLevels::levelNameToValue($level); - if ($levelValue < 0) { + $this->updateLevelByValue($levelValue); + } + + public function updateLevelByValue(int $level): void + { + if ($level < 0) { $this->remove(); return; } - $this->updateLevel($levelValue); + $this->updateLevel($level); } public function getWatchMap(): array diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php index 00f3f5e4d..fd86029d3 100644 --- a/tests/Activity/WatchTest.php +++ b/tests/Activity/WatchTest.php @@ -26,7 +26,7 @@ class WatchTest extends TestCase $this->withHtml($resp)->assertElementContains('form[action$="/watching/update"] button.icon-list-item', 'Watch'); $watchOptions = new UserEntityWatchOptions($editor, $entity); - $watchOptions->updateWatchLevel('comments'); + $watchOptions->updateLevelByValue(WatchLevels::COMMENTS); $resp = $this->get($entity->getUrl()); $this->withHtml($resp)->assertElementNotExists('form[action$="/watching/update"] button.icon-list-item'); @@ -112,17 +112,17 @@ class WatchTest extends TestCase $chapter = $book->chapters()->first(); $page = $chapter->pages()->first(); - (new UserEntityWatchOptions($editor, $book))->updateWatchLevel('updates'); + (new UserEntityWatchOptions($editor, $book))->updateLevelByValue(WatchLevels::UPDATES); $this->actingAs($editor)->get($book->getUrl())->assertSee('Watching new pages and updates'); $this->get($chapter->getUrl())->assertSee('Watching via parent book'); $this->get($page->getUrl())->assertSee('Watching via parent book'); - (new UserEntityWatchOptions($editor, $chapter))->updateWatchLevel('comments'); + (new UserEntityWatchOptions($editor, $chapter))->updateLevelByValue(WatchLevels::COMMENTS); $this->get($chapter->getUrl())->assertSee('Watching new pages, updates & comments'); $this->get($page->getUrl())->assertSee('Watching via parent chapter'); - (new UserEntityWatchOptions($editor, $page))->updateWatchLevel('updates'); + (new UserEntityWatchOptions($editor, $page))->updateLevelByValue(WatchLevels::UPDATES); $this->get($page->getUrl())->assertSee('Watching new pages and updates'); } @@ -130,7 +130,7 @@ class WatchTest extends TestCase { $editor = $this->users->editor(); $book = $this->entities->bookHasChaptersAndPages(); - (new UserEntityWatchOptions($editor, $book))->updateWatchLevel('ignore'); + (new UserEntityWatchOptions($editor, $book))->updateLevelByValue(WatchLevels::IGNORE); $this->actingAs($editor)->get($book->getUrl())->assertSee('Ignoring notifications'); $this->get($book->chapters()->first()->getUrl())->assertSee('Ignoring via parent book'); @@ -146,11 +146,11 @@ class WatchTest extends TestCase $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); $respHtml->assertElementNotExists('form[action$="/watching/update"] svg[data-icon="check-circle"]'); - $options->updateWatchLevel('comments'); + $options->updateLevelByValue(WatchLevels::COMMENTS); $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); $respHtml->assertElementExists('form[action$="/watching/update"] button[value="comments"] svg[data-icon="check-circle"]'); - $options->updateWatchLevel('ignore'); + $options->updateLevelByValue(WatchLevels::IGNORE); $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); $respHtml->assertElementExists('form[action$="/watching/update"] button[value="ignore"] svg[data-icon="check-circle"]'); } @@ -159,7 +159,7 @@ class WatchTest extends TestCase { $editor = $this->users->editor(); $book = $this->entities->bookHasChaptersAndPages(); - (new UserEntityWatchOptions($editor, $book))->updateWatchLevel('ignore'); + (new UserEntityWatchOptions($editor, $book))->updateLevelByValue(WatchLevels::IGNORE); $respHtml = $this->withHtml($this->actingAs($editor)->get($book->getUrl())); $respHtml->assertElementExists('form[action$="/watching/update"] button[name="level"][value="new"]'); @@ -225,7 +225,7 @@ class WatchTest extends TestCase $entities = $this->entities->createChainBelongingToUser($editor); $watches = new UserEntityWatchOptions($editor, $entities['book']); $prefs = new UserNotificationPreferences($editor); - $watches->updateWatchLevel('ignore'); + $watches->updateLevelByValue(WatchLevels::IGNORE); $prefs->updateFromSettingsArray(['own-page-changes' => 'true', 'own-page-comments' => true]); $notifications = Notification::fake(); @@ -244,7 +244,7 @@ class WatchTest extends TestCase $admin = $this->users->admin(); $entities = $this->entities->createChainBelongingToUser($editor); $watches = new UserEntityWatchOptions($editor, $entities['book']); - $watches->updateWatchLevel('comments'); + $watches->updateLevelByValue(WatchLevels::COMMENTS); // Comment post $this->actingAs($admin)->post("/comment/{$entities['page']->id}", [ @@ -269,7 +269,7 @@ class WatchTest extends TestCase $admin = $this->users->admin(); $entities = $this->entities->createChainBelongingToUser($editor); $watches = new UserEntityWatchOptions($editor, $entities['book']); - $watches->updateWatchLevel('updates'); + $watches->updateLevelByValue(WatchLevels::UPDATES); $this->actingAs($admin); $this->entities->updatePage($entities['page'], ['name' => 'Updated page', 'html' => 'new page content']); @@ -297,7 +297,7 @@ class WatchTest extends TestCase $admin = $this->users->admin(); $entities = $this->entities->createChainBelongingToUser($editor); $watches = new UserEntityWatchOptions($editor, $entities['book']); - $watches->updateWatchLevel('new'); + $watches->updateLevelByValue(WatchLevels::NEW); $this->actingAs($admin)->get($entities['chapter']->getUrl('/create-page')); $page = $entities['chapter']->pages()->where('draft', '=', true)->first(); @@ -320,7 +320,7 @@ class WatchTest extends TestCase $page = $this->entities->page(); $watches = new UserEntityWatchOptions($editor, $page); - $watches->updateWatchLevel('comments'); + $watches->updateLevelByValue(WatchLevels::COMMENTS); $this->permissions->disableEntityInheritedPermissions($page); $this->asAdmin()->post("/comment/{$page->id}", [ diff --git a/tests/User/UserPreferencesTest.php b/tests/User/UserPreferencesTest.php index 4ee04ea67..9d72f4e14 100644 --- a/tests/User/UserPreferencesTest.php +++ b/tests/User/UserPreferencesTest.php @@ -3,6 +3,7 @@ namespace Tests\User; use BookStack\Activity\Tools\UserEntityWatchOptions; +use BookStack\Activity\WatchLevels; use Tests\TestCase; class UserPreferencesTest extends TestCase @@ -110,13 +111,13 @@ class UserPreferencesTest extends TestCase $book = $this->entities->book(); $options = new UserEntityWatchOptions($editor, $book); - $options->updateWatchLevel('comments'); + $options->updateLevelByValue(WatchLevels::COMMENTS); $resp = $this->actingAs($editor)->get('/preferences/notifications'); $resp->assertSee($book->name); $resp->assertSee('All Page Updates & Comments'); - $options->updateWatchLevel('default'); + $options->updateLevelByValue(WatchLevels::DEFAULT); $resp = $this->actingAs($editor)->get('/preferences/notifications'); $resp->assertDontSee($book->name);