Notifications: Review of PR to include path path #4629

- Merged book and chapter name items to a single page path list item
  which has links to parent page/chapter.
- Added permission filtering to page path elements.
- Added page path to also be on comment notifications.
- Updated testing to cover.
- Added new Message Line objects to support.

Done during review of #4629

diff --git a/app/Activity/Notifications/MessageParts/EntityLinkMessageLine.php b/app/Activity/Notifications/MessageParts/EntityLinkMessageLine.php
new file mode 100644 (file)
index 0000000..599833c
--- /dev/null
+++ b/app/Activity/Notifications/MessageParts/EntityLinkMessageLine.php
@@ -0,0 +1,29 @@
+<?php
+
+namespace BookStack\Activity\Notifications\MessageParts;
+
+use BookStack\Entities\Models\Entity;
+use Illuminate\Contracts\Support\Htmlable;
+use Stringable;
+
+/**
+ * A link to a specific entity in the system, with the text showing its name.
+ */
+class EntityLinkMessageLine implements Htmlable, Stringable
+{
+    public function __construct(
+        protected Entity $entity,
+        protected int $nameLength = 120,
+    ) {
+    }
+
+    public function toHtml(): string
+    {
+        return '<a href="' . e($this->entity->getUrl()) . '">' . e($this->entity->getShortName($this->nameLength)) . '</a>';
+    }
+
+    public function __toString(): string
+    {
+        return "{$this->entity->getShortName($this->nameLength)} ({$this->entity->getUrl()})";
+    }
+}

diff --git a/app/Activity/Notifications/MessageParts/EntityPathMessageLine.php b/app/Activity/Notifications/MessageParts/EntityPathMessageLine.php
new file mode 100644 (file)
index 0000000..4b0f6e6
--- /dev/null
+++ b/app/Activity/Notifications/MessageParts/EntityPathMessageLine.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace BookStack\Activity\Notifications\MessageParts;
+
+use BookStack\Entities\Models\Entity;
+use Illuminate\Contracts\Support\Htmlable;
+use Stringable;
+
+/**
+ * A link to a specific entity in the system, with the text showing its name.
+ */
+class EntityPathMessageLine implements Htmlable, Stringable
+{
+    /**
+     * @var EntityLinkMessageLine[]
+     */
+    protected array $entityLinks;
+
+    public function __construct(
+        protected array $entities
+    ) {
+        $this->entityLinks = array_map(fn (Entity $entity) => new EntityLinkMessageLine($entity, 24), $this->entities);
+    }
+
+    public function toHtml(): string
+    {
+        $entityHtmls = array_map(fn (EntityLinkMessageLine $line) => $line->toHtml(), $this->entityLinks);
+        return implode(' &gt; ', $entityHtmls);
+    }
+
+    public function __toString(): string
+    {
+        return implode(' > ', $this->entityLinks);
+    }
+}

diff --git a/app/Activity/Notifications/Messages/BaseActivityNotification.php b/app/Activity/Notifications/Messages/BaseActivityNotification.php
index 322df5d94f652b6dbfd4bcb75538cf796af06fb8..ca86eb81bf842f0b5c675c51993ba3db3d18ccd4 100644 (file)
--- a/app/Activity/Notifications/Messages/BaseActivityNotification.php
+++ b/app/Activity/Notifications/Messages/BaseActivityNotification.php
@@ -3,8 +3,12 @@
 namespace BookStack\Activity\Notifications\Messages;
 
 use BookStack\Activity\Models\Loggable;
+use BookStack\Activity\Notifications\MessageParts\EntityPathMessageLine;
 use BookStack\Activity\Notifications\MessageParts\LinkedMailMessageLine;
 use BookStack\App\MailNotification;
+use BookStack\Entities\Models\Entity;
+use BookStack\Entities\Models\Page;
+use BookStack\Permissions\PermissionApplicator;
 use BookStack\Translation\LocaleDefinition;
 use BookStack\Users\Models\User;
 use Illuminate\Bus\Queueable;
@@ -44,4 +48,20 @@ abstract class BaseActivityNotification extends MailNotification
             $locale->trans('notifications.footer_reason_link'),
         );
     }
+
+    /**
+     * Build a line which provides the book > chapter path to a page.
+     * Takes into account visibility of these parent items.
+     * Returns null if no path items can be used.
+     */
+    protected function buildPagePathLine(Page $page, User $notifiable): ?EntityPathMessageLine
+    {
+        $permissions = new PermissionApplicator($notifiable);
+
+        $path = array_filter([$page->book, $page->chapter], function (?Entity $entity) use ($permissions) {
+            return !is_null($entity) && $permissions->checkOwnableUserAccess($entity, 'view');
+        });
+
+        return empty($path) ? null : new EntityPathMessageLine($path);
+    }
 }

diff --git a/app/Activity/Notifications/Messages/CommentCreationNotification.php b/app/Activity/Notifications/Messages/CommentCreationNotification.php
index 094ab30b74e2972baa24212bd5f0fd8b40334b40..30d0ffa2be4c280cea5af66a004106fd5f062054 100644 (file)
--- a/app/Activity/Notifications/Messages/CommentCreationNotification.php
+++ b/app/Activity/Notifications/Messages/CommentCreationNotification.php
@@ -3,6 +3,7 @@
 namespace BookStack\Activity\Notifications\Messages;
 
 use BookStack\Activity\Models\Comment;
+use BookStack\Activity\Notifications\MessageParts\EntityLinkMessageLine;
 use BookStack\Activity\Notifications\MessageParts\ListMessageLine;
 use BookStack\Entities\Models\Page;
 use BookStack\Users\Models\User;
@@ -19,14 +20,17 @@ class CommentCreationNotification extends BaseActivityNotification
 
         $locale = $notifiable->getLocale();
 
+        $listLines = array_filter([
+            $locale->trans('notifications.detail_page_name') => new EntityLinkMessageLine($page),
+            $locale->trans('notifications.detail_page_path') => $this->buildPagePathLine($page, $notifiable),
+            $locale->trans('notifications.detail_commenter') => $this->user->name,
+            $locale->trans('notifications.detail_comment') => strip_tags($comment->html),
+        ]);
+
         return $this->newMailMessage($locale)
             ->subject($locale->trans('notifications.new_comment_subject', ['pageName' => $page->getShortName()]))
             ->line($locale->trans('notifications.new_comment_intro', ['appName' => setting('app-name')]))
-            ->line(new ListMessageLine([
-                $locale->trans('notifications.detail_page_name') => $page->name,
-                $locale->trans('notifications.detail_commenter') => $this->user->name,
-                $locale->trans('notifications.detail_comment') => strip_tags($comment->html),
-            ]))
+            ->line(new ListMessageLine($listLines))
             ->action($locale->trans('notifications.action_view_comment'), $page->getUrl('#comment' . $comment->local_id))
             ->line($this->buildReasonFooterLine($locale));
     }

diff --git a/app/Activity/Notifications/Messages/PageCreationNotification.php b/app/Activity/Notifications/Messages/PageCreationNotification.php
index e98f0c20c320bead0237d5f414253c6274a7c377..0b98ad30ce4147ba44c488f8782f00a37ce0c411 100644 (file)
--- a/app/Activity/Notifications/Messages/PageCreationNotification.php
+++ b/app/Activity/Notifications/Messages/PageCreationNotification.php
@@ -2,9 +2,9 @@
 
 namespace BookStack\Activity\Notifications\Messages;
 
+use BookStack\Activity\Notifications\MessageParts\EntityLinkMessageLine;
 use BookStack\Activity\Notifications\MessageParts\ListMessageLine;
 use BookStack\Entities\Models\Page;
-use BookStack\Entities\Models\Chapter;
 use BookStack\Users\Models\User;
 use Illuminate\Notifications\Messages\MailMessage;
 
@@ -14,32 +14,19 @@ class PageCreationNotification extends BaseActivityNotification
     {
         /** @var Page $page */
         $page = $this->detail;
-        $book = $page->book;
-        $chapterId = $page->chapter_id;
-        $chapter = $chapterId ? Chapter::find($chapterId) : null;
 
         $locale = $notifiable->getLocale();
 
-        $listMessageData = [
-            $locale->trans('notifications.detail_page_name') => $page->name,
-            '' => '',
-        ];
-
-        if ($chapter) {
-            $listMessageData += [
-                $locale->trans('notifications.detail_chapter_name') => $chapter->name,
-            ];
-        }
-    
-        $listMessageData += [
-            $locale->trans('notifications.detail_book_name') => $book->name,
+        $listLines = array_filter([
+            $locale->trans('notifications.detail_page_name') => new EntityLinkMessageLine($page),
+            $locale->trans('notifications.detail_page_path') => $this->buildPagePathLine($page, $notifiable),
             $locale->trans('notifications.detail_created_by') => $this->user->name,
-        ];
+        ]);
 
         return $this->newMailMessage($locale)
             ->subject($locale->trans('notifications.new_page_subject', ['pageName' => $page->getShortName()]))
-            ->line($locale->trans('notifications.new_page_intro', ['appName' => setting('app-name')], $locale))
-            ->line(new ListMessageLine($listMessageData))
+            ->line($locale->trans('notifications.new_page_intro', ['appName' => setting('app-name')]))
+            ->line(new ListMessageLine($listLines))
             ->action($locale->trans('notifications.action_view_page'), $page->getUrl())
             ->line($this->buildReasonFooterLine($locale));
     }

diff --git a/app/Activity/Notifications/Messages/PageUpdateNotification.php b/app/Activity/Notifications/Messages/PageUpdateNotification.php
index a303a7883d92cfadbfd73773b2f7718145684029..80ee378ccd60d6490a1e288d37f157bd3260638e 100644 (file)
--- a/app/Activity/Notifications/Messages/PageUpdateNotification.php
+++ b/app/Activity/Notifications/Messages/PageUpdateNotification.php
@@ -2,9 +2,9 @@
 
 namespace BookStack\Activity\Notifications\Messages;
 
+use BookStack\Activity\Notifications\MessageParts\EntityLinkMessageLine;
 use BookStack\Activity\Notifications\MessageParts\ListMessageLine;
 use BookStack\Entities\Models\Page;
-use BookStack\Entities\Models\Chapter;
 use BookStack\Users\Models\User;
 use Illuminate\Notifications\Messages\MailMessage;
 
@@ -14,32 +14,19 @@ class PageUpdateNotification extends BaseActivityNotification
     {
         /** @var Page $page */
         $page = $this->detail;
-        $book = $page->book;
-        $chapterId = $page->chapter_id;
-        $chapter = $chapterId ? Chapter::find($chapterId) : null;
 
         $locale = $notifiable->getLocale();
 
-        $listMessageData = [
-            $locale->trans('notifications.detail_page_name') => $page->name,
-            '' => '',
-        ];
-    
-        if ($chapter) {
-            $listMessageData += [
-                $locale->trans('notifications.detail_chapter_name') => $chapter->name,
-            ];
-        }
-    
-        $listMessageData += [
-            $locale->trans('notifications.detail_book_name') => $book->name,
+        $listLines = array_filter([
+            $locale->trans('notifications.detail_page_name') => new EntityLinkMessageLine($page),
+            $locale->trans('notifications.detail_page_path') => $this->buildPagePathLine($page, $notifiable),
             $locale->trans('notifications.detail_updated_by') => $this->user->name,
-        ];
+        ]);
 
         return $this->newMailMessage($locale)
             ->subject($locale->trans('notifications.updated_page_subject', ['pageName' => $page->getShortName()]))
             ->line($locale->trans('notifications.updated_page_intro', ['appName' => setting('app-name')]))
-            ->line(new ListMessageLine($listMessageData))
+            ->line(new ListMessageLine($listLines))
             ->line($locale->trans('notifications.updated_page_debounce'))
             ->action($locale->trans('notifications.action_view_page'), $page->getUrl())
             ->line($this->buildReasonFooterLine($locale));

diff --git a/lang/de/notifications.php b/lang/de/notifications.php
index c1691f89afd4757586862078ab8083952c67e863..314f0bfe39c0b0b2585c3fbff80d38685d361c69 100644 (file)
--- a/lang/de/notifications.php
+++ b/lang/de/notifications.php
@@ -12,8 +12,6 @@ return [
     'updated_page_intro' => 'Eine Seite wurde in :appName aktualisiert:',
     'updated_page_debounce' => 'Um eine Flut von Benachrichtigungen zu vermeiden, werden Sie für eine gewisse Zeit keine Benachrichtigungen für weitere Bearbeitungen dieser Seite durch denselben Bearbeiter erhalten.',
 
-    'detail_book_name' => 'Name des Buches:',
-    'detail_chapter_name' => 'Name des Kapitels:',
     'detail_page_name' => 'Name der Seite:',
     'detail_commenter' => 'Kommentator:',
     'detail_comment' => 'Kommentar:',

diff --git a/lang/de_informal/notifications.php b/lang/de_informal/notifications.php
index 7b01bccd16e20045fbfa92669ca87692259fd4f8..fc6204d501830f4112805270e1fcac7225a477bb 100644 (file)
--- a/lang/de_informal/notifications.php
+++ b/lang/de_informal/notifications.php
@@ -12,8 +12,6 @@ return [
     'updated_page_intro' => 'Eine Seite wurde in :appName aktualisiert:',
     'updated_page_debounce' => 'Um eine Flut von Benachrichtigungen zu vermeiden, wirst du für eine gewisse Zeit keine Benachrichtigungen für weitere Bearbeitungen dieser Seite durch denselben Bearbeiter erhalten.',
 
-    'detail_book_name' => 'Buchname:',
-    'detail_chapter_name' => 'Kapitelname:',
     'detail_page_name' => 'Seitenname:',
     'detail_commenter' => 'Kommentator:',
     'detail_comment' => 'Kommentar:',

diff --git a/lang/en/notifications.php b/lang/en/notifications.php
index f476ee5fc73776d92618e6f6417249cea36d17b9..1afd23f1dc4b0bfbb208f923075a86b14547a801 100644 (file)
--- a/lang/en/notifications.php
+++ b/lang/en/notifications.php
@@ -12,9 +12,8 @@ return [
     '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_book_name' => 'Book Name:',
-    'detail_chapter_name' => 'Chapter Name:',
     'detail_page_name' => 'Page Name:',
+    'detail_page_path' => 'Page Path:',
     'detail_commenter' => 'Commenter:',
     'detail_comment' => 'Comment:',
     'detail_created_by' => 'Created By:',

diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php
index 5b9ae5a4c9e3d1996b652cab41f00675d7d9962a..42216a37960239c655c7e156e9199236e84f9b8e 100644 (file)
--- a/tests/Activity/WatchTest.php
+++ b/tests/Activity/WatchTest.php
@@ -12,7 +12,6 @@ use BookStack\Activity\Tools\ActivityLogger;
 use BookStack\Activity\Tools\UserEntityWatchOptions;
 use BookStack\Activity\WatchLevels;
 use BookStack\Entities\Models\Entity;
-use BookStack\Entities\Tools\TrashCan;
 use BookStack\Settings\UserNotificationPreferences;
 use Illuminate\Support\Facades\Notification;
 use Tests\TestCase;
@@ -268,6 +267,7 @@ class WatchTest extends TestCase
             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, 'Page Path: ' . $entities['book']->getShortName(24) . ' > ' . $entities['chapter']->getShortName(24))
                 && str_contains($mailContent, 'Commenter: ' . $admin->name)
                 && str_contains($mailContent, 'Comment: My new comment response');
         });
@@ -285,12 +285,13 @@ class WatchTest extends TestCase
         $this->actingAs($admin);
         $this->entities->updatePage($entities['page'], ['name' => 'Updated page', 'html' => 'new page content']);
 
-        $notifications->assertSentTo($editor, function (PageUpdateNotification $notification) use ($editor, $admin) {
+        $notifications->assertSentTo($editor, function (PageUpdateNotification $notification) use ($editor, $admin, $entities) {
             $mail = $notification->toMail($editor);
             $mailContent = html_entity_decode(strip_tags($mail->render()), ENT_QUOTES);
             return $mail->subject === 'Updated page: Updated page'
                 && str_contains($mailContent, 'View Page')
                 && str_contains($mailContent, 'Page Name: Updated page')
+                && str_contains($mailContent, 'Page Path: ' . $entities['book']->getShortName(24) . ' > ' . $entities['chapter']->getShortName(24))
                 && 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');
         });
@@ -314,12 +315,13 @@ class WatchTest extends TestCase
         $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) {
+        $notifications->assertSentTo($editor, function (PageCreationNotification $notification) use ($editor, $admin, $entities) {
             $mail = $notification->toMail($editor);
             $mailContent = html_entity_decode(strip_tags($mail->render()), ENT_QUOTES);
             return $mail->subject === 'New page: My new page'
                 && str_contains($mailContent, 'View Page')
                 && str_contains($mailContent, 'Page Name: My new page')
+                && str_contains($mailContent, 'Page Path: ' . $entities['book']->getShortName(24) . ' > ' . $entities['chapter']->getShortName(24))
                 && str_contains($mailContent, 'Created By: ' . $admin->name);
         });
     }
@@ -405,4 +407,32 @@ class WatchTest extends TestCase
 
         $this->assertDatabaseMissing('watches', ['watchable_type' => 'page', 'watchable_id' => $page->id]);
     }
+
+    public function test_page_path_in_notifications_limited_by_permissions()
+    {
+        $chapter = $this->entities->chapterHasPages();
+        $page = $chapter->pages()->first();
+        $book = $chapter->book;
+        $notification = new PageCreationNotification($page, $this->users->editor());
+
+        $viewer = $this->users->viewer();
+        $viewerRole = $viewer->roles()->first();
+
+        $content = html_entity_decode(strip_tags($notification->toMail($viewer)->render()), ENT_QUOTES);
+        $this->assertStringContainsString('Page Path: ' . $book->getShortName(24) . ' > ' . $chapter->getShortName(24), $content);
+
+        $this->permissions->setEntityPermissions($page, ['view'], [$viewerRole]);
+        $this->permissions->setEntityPermissions($chapter, [], [$viewerRole]);
+
+        $content = html_entity_decode(strip_tags($notification->toMail($viewer)->render()), ENT_QUOTES);
+        $this->assertStringContainsString('Page Path: ' . $book->getShortName(24), $content);
+        $this->assertStringNotContainsString(' > ' . $chapter->getShortName(24), $content);
+
+        $this->permissions->setEntityPermissions($book, [], [$viewerRole]);
+
+        $content = html_entity_decode(strip_tags($notification->toMail($viewer)->render()), ENT_QUOTES);
+        $this->assertStringNotContainsString('Page Path:', $content);
+        $this->assertStringNotContainsString($book->getShortName(24), $content);
+        $this->assertStringNotContainsString($chapter->getShortName(24), $content);
+    }
 }