From 896bd86cc2ae17bbab954a25ab5fbf18ce87faa4 Mon Sep 17 00:00:00 2001 From: Rias Date: Thu, 27 Aug 2026 13:43:28 +0200 Subject: [PATCH 1/3] Add activity comments backend --- resources/translations/en/app.php | 3 + src/Activity/Activities.php | 37 +++ src/Activity/ActivityComments.php | 292 ++++++++++++++++++ src/Activity/ActivityEventRecorder.php | 3 +- src/Activity/EventTypes/CommentCreated.php | 12 + src/Activity/EventTypes/CommentDeleted.php | 12 + src/Activity/EventTypes/CommentEdited.php | 12 + src/Activity/EventTypes/CommentEvent.php | 37 +++ src/Activity/Models/ActivityEvent.php | 2 + ..._25_000000_create_activityevents_table.php | 7 + ...000_create_activitynotifications_table.php | 40 +++ src/Database/Migrations/Install.php | 13 + src/Database/Table.php | 2 + .../Actions/PurgeExpiredActivity.php | 1 + src/Markdown/Markdown.php | 26 +- src/Support/Facades/Activities.php | 9 + src/SystemMessage/SystemMessageCatalog.php | 2 +- .../ActivityMentionNotification.php | 118 +++++++ .../Actions/PurgeExpiredActivityTest.php | 26 +- 19 files changed, 646 insertions(+), 8 deletions(-) create mode 100644 src/Activity/ActivityComments.php create mode 100644 src/Activity/EventTypes/CommentCreated.php create mode 100644 src/Activity/EventTypes/CommentDeleted.php create mode 100644 src/Activity/EventTypes/CommentEdited.php create mode 100644 src/Activity/EventTypes/CommentEvent.php create mode 100644 src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php create mode 100644 src/User/Notifications/ActivityMentionNotification.php diff --git a/resources/translations/en/app.php b/resources/translations/en/app.php index 4639b283145..5f6c0a5fe4c 100644 --- a/resources/translations/en/app.php +++ b/resources/translations/en/app.php @@ -2249,6 +2249,9 @@ 'by {creator}' => 'by {creator}', 'categories' => 'categories', 'category' => 'category', + 'comment_mention_body' => "Hey {{user.friendlyName|e}},\n\n{{author|e}} mentioned you in a comment on “{{subject|e}}”:\n\n{{comment}}\n\n<{{link}}>", + 'comment_mention_heading' => 'When someone mentions a user in a comment:', + 'comment_mention_subject' => 'You were mentioned in a comment', 'contains' => 'contains', 'content block' => 'content block', 'content blocks' => 'content blocks', diff --git a/src/Activity/Activities.php b/src/Activity/Activities.php index 0f0ab38d50d..518914e3240 100644 --- a/src/Activity/Activities.php +++ b/src/Activity/Activities.php @@ -6,7 +6,10 @@ use CraftCms\Cms\Activity\Contracts\ActivityEventTypeInterface; use CraftCms\Cms\Activity\Models\ActivityEvent; +use CraftCms\Cms\Element\Contracts\ElementInterface; +use CraftCms\Cms\Site\Data\Site; use CraftCms\Cms\Support\HtmlSanitizer\HtmlSanitizerManager; +use CraftCms\Cms\User\Elements\User; use Illuminate\Container\Attributes\Scoped; use Illuminate\Contracts\Support\Htmlable; use Illuminate\Database\Eloquent\Builder; @@ -21,6 +24,7 @@ class Activities public function __construct( private readonly HtmlSanitizerManager $htmlSanitizers, private readonly ActivityEventRecorder $events, + private readonly ActivityComments $comments, ) {} public function record(ActivityEventTypeInterface $event): ActivityEvent @@ -28,6 +32,39 @@ public function record(ActivityEventTypeInterface $event): ActivityEvent return $this->events->record($event); } + public function createComment( + ElementInterface $subject, + User $author, + Site $site, + string $markdown, + ): ActivityEvent { + return $this->comments->create($subject, $author, $site, $markdown); + } + + public function editComment( + ActivityEvent $comment, + User $author, + string $markdown, + ?ElementInterface $subject = null, + ): ActivityEvent { + return $this->comments->edit($comment, $author, $markdown, $subject); + } + + public function deleteComment(ActivityEvent $comment, User $actor): ActivityEvent + { + return $this->comments->delete($comment, $actor); + } + + public function canMention(User $user, ElementInterface $subject): bool + { + return $this->comments->canMention($user, $subject); + } + + public function renderComment(ActivityEvent $version): HtmlString + { + return $this->comments->render($version); + } + /** @return Builder */ public function query(): Builder { diff --git a/src/Activity/ActivityComments.php b/src/Activity/ActivityComments.php new file mode 100644 index 00000000000..5c03cbfe6a2 --- /dev/null +++ b/src/Activity/ActivityComments.php @@ -0,0 +1,292 @@ +validate($markdown); + + return DB::transaction(function () use ($subject, $author, $site, $markdown): ActivityEvent { + $event = $this->events->record(new CommentCreated( + subject: $subject, + actor: $author, + site: $site, + markdown: $markdown, + authorId: $author->id, + authorLabel: $author->name, + mentions: $this->resolveMentions($markdown, $subject), + )); + + $this->scheduleMentionNotifications($event, $event); + + return $event; + }); + } + + public function edit( + ActivityEvent $comment, + User $author, + string $markdown, + ?ElementInterface $subject = null, + ): ActivityEvent { + $this->validate($markdown); + + return $this->mutate($comment, $author, CommentEdited::class, $markdown, $subject); + } + + public function delete(ActivityEvent $comment, User $actor): ActivityEvent + { + return $this->mutate($comment, $actor, CommentDeleted::class); + } + + public function canMention(User $user, ElementInterface $subject): bool + { + return $user->getStatus() === User::STATUS_ACTIVE + && $user->can('accessCp') + && Gate::forUser($user)->allows('view', $subject); + } + + public function render(ActivityEvent $version): HtmlString + { + $mentionData = $version->data['mentions'] ?? []; + + if (! is_array($mentionData)) { + throw new UnexpectedValueException('Activity comment mentions must be an array.'); + } + + $mentions = collect($mentionData)->keyBy('id'); + $users = User::find() + ->id($mentions->keys()->all()) + ->status(null) + ->collect() + ->keyBy('id'); + $html = $this->markdown->transform( + $version->data['markdown'], + function (Document $document) use ($mentions, $users): void { + foreach ($this->mentionLinks($document) as [$node, $reference]) { + if (! ctype_digit($reference)) { + continue; + } + + $id = (int) $reference; + $mention = $mentions->get($id); + + if ($mention === null) { + continue; + } + + $user = $users->get($id); + $canView = $user !== null && Gate::check('view', $user); + $username = $canView ? ($user->username ?? $mention['username']) : $mention['username']; + + $node->replaceWith($canView && $user->getCpEditUrl() !== null + ? new Link($user->getCpEditUrl(), "@$username") + : new Text("@$username")); + } + }, + Markdown::FLAVOR_GFM_COMMENT, + ); + + return new HtmlString($this->htmlSanitizers->sanitize($html)); + } + + /** @param class-string $eventType */ + private function mutate( + ActivityEvent $comment, + User $actor, + string $eventType, + ?string $markdown = null, + ?ElementInterface $liveSubject = null, + ): ActivityEvent { + return DB::transaction(function () use ($comment, $actor, $eventType, $markdown, $liveSubject): ActivityEvent { + $root = ActivityEvent::query() + ->whereKey($comment->id) + ->where('eventType', CommentCreated::class) + ->whereNull('rootEventId') + ->lockForUpdate() + ->firstOrFail(); + $current = ActivityEvent::query() + ->where('rootEventId', $root->id) + ->newestFirst() + ->first() ?? $root; + + if ($current->eventType === CommentDeleted::class) { + throw ValidationException::withMessages([ + 'commentId' => t('This comment has been removed.'), + ]); + } + + $subject = new ActivitySubject( + $root->subjectType, + $root->subjectId, + $root->snapshots['subject']['label'], + ); + $site = $root->siteId === null ? null : Site::get($root->siteId); + + if ($site === null) { + throw new LogicException('Activity comments require a current site.'); + } + + $event = $this->events->record(new $eventType( + subject: $subject, + actor: $actor, + site: $site, + markdown: $markdown ?? $current->data['markdown'], + authorId: $root->data['author']['id'], + authorLabel: $root->data['author']['label'], + mentions: $markdown === null + ? ($current->data['mentions'] ?? []) + : $this->resolveMentions($markdown, $liveSubject), + ), rootEventId: $root->id); + + if ($markdown !== null) { + $this->scheduleMentionNotifications($root, $event); + } + + return $event; + }); + } + + private function scheduleMentionNotifications(ActivityEvent $comment, ActivityEvent $version): void + { + foreach ($version->data['mentions'] as $mention) { + $pair = [ + 'activityEventId' => $comment->id, + 'userId' => $mention['id'], + ]; + + if (DB::table(Table::ACTIVITYNOTIFICATIONS)->where($pair)->exists()) { + continue; + } + + DB::table(Table::ACTIVITYNOTIFICATIONS)->insert([ + ...$pair, + 'versionEventId' => $version->id, + ]); + + DB::afterCommit(function () use ($mention, $pair, $version): void { + try { + UserModel::query() + ->findOrFail($mention['id']) + ->notify(new ActivityMentionNotification($version->id)); + } catch (Throwable $exception) { + DB::table(Table::ACTIVITYNOTIFICATIONS) + ->where($pair) + ->where('versionEventId', $version->id) + ->delete(); + report($exception); + } + }); + } + } + + /** @return list */ + private function resolveMentions(string $markdown, ?ElementInterface $subject): array + { + $references = []; + $this->markdown->transform( + $markdown, + function (Document $document) use (&$references): void { + foreach ($this->mentionLinks($document) as [, $reference]) { + $references[] = $reference; + } + }, + Markdown::FLAVOR_GFM_COMMENT, + ); + + if ($references === []) { + return []; + } + + $ids = collect($references) + ->map(function (string $id): int { + if (! ctype_digit($id)) { + throw ValidationException::withMessages([ + 'markdown' => t('Comment contains an invalid user mention.'), + ]); + } + + return (int) $id; + }) + ->unique() + ->values(); + $users = User::find() + ->id($ids->all()) + ->status(User::STATUS_ACTIVE) + ->collect() + ->keyBy('id'); + + return $ids->map(function (int $id) use ($subject, $users): array { + $user = $users->get($id); + + if ($subject === null || $user === null || ! $this->canMention($user, $subject)) { + throw ValidationException::withMessages([ + 'markdown' => t('Comment contains an ineligible user mention.'), + ]); + } + + return ['id' => $user->id, 'username' => $user->username]; + })->all(); + } + + /** @return iterable */ + private function mentionLinks(Document $document): iterable + { + foreach (new NodeIterator($document) as $node) { + if ($node instanceof Link && str_starts_with($node->getUrl(), 'craft-user:')) { + yield [$node, substr($node->getUrl(), strlen('craft-user:'))]; + } + } + } + + private function validate(string $markdown): void + { + if (blank($markdown)) { + throw ValidationException::withMessages([ + 'markdown' => t('Comment cannot be blank.'), + ]); + } + } +} diff --git a/src/Activity/ActivityEventRecorder.php b/src/Activity/ActivityEventRecorder.php index 0416144db32..300dd7207d7 100644 --- a/src/Activity/ActivityEventRecorder.php +++ b/src/Activity/ActivityEventRecorder.php @@ -20,7 +20,7 @@ public function __construct( private readonly Impersonation $impersonation, ) {} - public function record(ActivityEventTypeInterface $event): ActivityEvent + public function record(ActivityEventTypeInterface $event, ?string $rootEventId = null): ActivityEvent { $data = $event->data(); @@ -56,6 +56,7 @@ public function record(ActivityEventTypeInterface $event): ActivityEvent 'subjectType' => $subject?->type, 'subjectId' => $subject?->id, 'siteId' => $site?->id, + 'rootEventId' => $rootEventId, 'payload' => [ 'snapshots' => $snapshots, 'changes' => collect($event->changes())->toArray(), diff --git a/src/Activity/EventTypes/CommentCreated.php b/src/Activity/EventTypes/CommentCreated.php new file mode 100644 index 00000000000..1541f4c5129 --- /dev/null +++ b/src/Activity/EventTypes/CommentCreated.php @@ -0,0 +1,12 @@ + $mentions */ + public function __construct( + ElementInterface|ActivitySubject $subject, + User|ActivityActor $actor, + Site $site, + private readonly string $markdown, + private readonly int $authorId, + private readonly string $authorLabel, + private readonly array $mentions, + ) { + parent::__construct($subject, $actor, $site); + } + + public function data(): array + { + return [ + 'markdown' => $this->markdown, + 'author' => ['id' => $this->authorId, 'label' => $this->authorLabel], + 'mentions' => $this->mentions, + ]; + } +} diff --git a/src/Activity/Models/ActivityEvent.php b/src/Activity/Models/ActivityEvent.php index 5194f9530e5..dfbc31d0140 100644 --- a/src/Activity/Models/ActivityEvent.php +++ b/src/Activity/Models/ActivityEvent.php @@ -26,6 +26,7 @@ * @property string|null $subjectType * @property string|null $subjectId * @property int|null $siteId + * @property string|null $rootEventId * @property array{snapshots: array>, changes: list>, data: array} $payload * @property array> $snapshots * @property list $changes @@ -47,6 +48,7 @@ protected function casts(): array 'id' => 'string', 'actorId' => 'integer', 'siteId' => 'integer', + 'rootEventId' => 'string', 'payload' => 'array', 'occurredAt' => 'immutable_datetime', ]; diff --git a/src/Database/Migrations/2026_08_25_000000_create_activityevents_table.php b/src/Database/Migrations/2026_08_25_000000_create_activityevents_table.php index f6cc7f45a81..a01084dce1b 100644 --- a/src/Database/Migrations/2026_08_25_000000_create_activityevents_table.php +++ b/src/Database/Migrations/2026_08_25_000000_create_activityevents_table.php @@ -24,6 +24,7 @@ public function up(): void $table->string('subjectType')->nullable(); $table->string('subjectId')->nullable(); $table->unsignedBigInteger('siteId')->nullable(); + $table->unsignedBigInteger('rootEventId')->nullable(); $table->jsonb('payload'); $table->dateTime('occurredAt'); }); @@ -31,6 +32,12 @@ public function up(): void Schema::createIndex(Table::ACTIVITYEVENTS, ['actorType', 'actorId']); Schema::createIndex(Table::ACTIVITYEVENTS, ['subjectType', 'subjectId', 'siteId', 'occurredAt', 'id']); Schema::createIndex(Table::ACTIVITYEVENTS, ['occurredAt', 'id']); + Schema::createIndex(Table::ACTIVITYEVENTS, ['rootEventId', 'occurredAt', 'id']); + + Schema::table(Table::ACTIVITYEVENTS, fn (Blueprint $table) => $table->foreign('rootEventId') + ->references('id') + ->on(Table::ACTIVITYEVENTS) + ->cascadeOnDelete()); } public function down(): void diff --git a/src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php b/src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php new file mode 100644 index 00000000000..38c2d46cbc3 --- /dev/null +++ b/src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php @@ -0,0 +1,40 @@ +unsignedBigInteger('activityEventId'); + $table->unsignedBigInteger('userId'); + $table->unsignedBigInteger('versionEventId'); + }); + + Schema::createIndex(Table::ACTIVITYNOTIFICATIONS, ['activityEventId', 'userId'], unique: true); + + Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('activityEventId') + ->references('id') + ->on(Table::ACTIVITYEVENTS) + ->cascadeOnDelete()); + Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('versionEventId') + ->references('id') + ->on(Table::ACTIVITYEVENTS) + ->cascadeOnDelete()); + } + + public function down(): void + { + Schema::dropIfExists(Table::ACTIVITYNOTIFICATIONS); + } +}; diff --git a/src/Database/Migrations/Install.php b/src/Database/Migrations/Install.php index ef868bdb90b..2922ee47d97 100644 --- a/src/Database/Migrations/Install.php +++ b/src/Database/Migrations/Install.php @@ -219,10 +219,18 @@ public function createTables(?Logger $logger = null): void $table->string('subjectType')->nullable(); $table->string('subjectId')->nullable(); $table->unsignedBigInteger('siteId')->nullable(); + $table->unsignedBigInteger('rootEventId')->nullable(); $table->jsonb('payload'); $table->dateTime('occurredAt'); }); + $logger?->subLabel('activitynotifications'); + Schema::create(Table::ACTIVITYNOTIFICATIONS, function (Blueprint $table) { + $table->unsignedBigInteger('activityEventId'); + $table->unsignedBigInteger('userId'); + $table->unsignedBigInteger('versionEventId'); + }); + $logger?->subLabel('addresses'); Schema::create('addresses', function (Blueprint $table) { $table->integer('id', true); @@ -1037,6 +1045,8 @@ public function createIndexes(): void Schema::createIndex(Table::ACTIVITYEVENTS, ['actorType', 'actorId']); Schema::createIndex(Table::ACTIVITYEVENTS, ['subjectType', 'subjectId', 'siteId', 'occurredAt', 'id']); Schema::createIndex(Table::ACTIVITYEVENTS, ['occurredAt', 'id']); + Schema::createIndex(Table::ACTIVITYEVENTS, ['rootEventId', 'occurredAt', 'id']); + Schema::createIndex(Table::ACTIVITYNOTIFICATIONS, ['activityEventId', 'userId'], unique: true); Schema::createIndex(Table::ASSETINDEXDATA, ['sessionId', 'volumeId']); Schema::createIndex(Table::ASSETINDEXDATA, ['sessionId', 'status', 'id']); Schema::createIndex(Table::ASSETINDEXDATA, ['volumeId']); @@ -1182,6 +1192,9 @@ public function createIndexes(): void public function addForeignKeys(): void { + Schema::table(Table::ACTIVITYEVENTS, fn (Blueprint $table) => $table->foreign('rootEventId')->references('id')->on(Table::ACTIVITYEVENTS)->cascadeOnDelete()); + Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('activityEventId')->references('id')->on(Table::ACTIVITYEVENTS)->cascadeOnDelete()); + Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('versionEventId')->references('id')->on(Table::ACTIVITYEVENTS)->cascadeOnDelete()); Schema::table(Table::ADDRESSES, fn (Blueprint $table) => $table->foreign('id')->references('id')->on(Table::ELEMENTS)->cascadeOnDelete()); Schema::table(Table::ADDRESSES, fn (Blueprint $table) => $table->foreign('primaryOwnerId')->references('id')->on(Table::ELEMENTS)->cascadeOnDelete()); Schema::table(Table::ASSETINDEXDATA, fn (Blueprint $table) => $table->foreign('volumeId')->references('id')->on(Table::VOLUMES)->cascadeOnDelete()); diff --git a/src/Database/Table.php b/src/Database/Table.php index e3fd7c75e2c..aa517aa0191 100644 --- a/src/Database/Table.php +++ b/src/Database/Table.php @@ -11,6 +11,8 @@ { public const string ACTIVITYEVENTS = 'activityevents'; + public const string ACTIVITYNOTIFICATIONS = 'activitynotifications'; + public const string ADDRESSES = 'addresses'; public const string ASSETINDEXDATA = 'assetindexdata'; diff --git a/src/GarbageCollection/Actions/PurgeExpiredActivity.php b/src/GarbageCollection/Actions/PurgeExpiredActivity.php index b76d978bfc0..3ac07ca8eee 100644 --- a/src/GarbageCollection/Actions/PurgeExpiredActivity.php +++ b/src/GarbageCollection/Actions/PurgeExpiredActivity.php @@ -21,6 +21,7 @@ public function __invoke(): void function () { DB::table(Table::ACTIVITYEVENTS) ->select('id') + ->whereNull('rootEventId') ->where('occurredAt', '<', now()->subSeconds($this->generalConfig->activityRetentionDuration)) ->orderBy('id') ->chunkById( diff --git a/src/Markdown/Markdown.php b/src/Markdown/Markdown.php index 42d2640134c..9223aa6c85f 100644 --- a/src/Markdown/Markdown.php +++ b/src/Markdown/Markdown.php @@ -11,6 +11,9 @@ use Illuminate\Container\Attributes\Singleton; use InvalidArgumentException; use League\CommonMark\MarkdownConverter; +use League\CommonMark\Node\Block\Document; +use League\CommonMark\Parser\MarkdownParser; +use League\CommonMark\Renderer\HtmlRenderer; #[Singleton] class Markdown @@ -79,6 +82,22 @@ public function parseParagraph(string $markdown, ?string $flavor = null, bool $a )), "\n"); } + /** @param callable(Document): void $transform */ + public function transform(string $markdown, callable $transform, ?string $flavor = null): string + { + if (ltrim($markdown) === '') { + return ''; + } + + $options = new MarkdownOptions(flavor: $flavor); + $environment = $this->converter($options)->getEnvironment(); + $document = new MarkdownParser($environment)->parse($this->normalize($markdown)); + + $transform($document); + + return new HtmlRenderer($environment)->renderDocument($document)->getContent(); + } + public function convert(string $markdown, MarkdownOptions $options): string { if (ltrim($markdown) === '') { @@ -86,10 +105,15 @@ public function convert(string $markdown, MarkdownOptions $options): string } return $this->converter($options) - ->convert(str_replace(["\r\n", "\n\r", "\r"], "\n", $markdown)) + ->convert($this->normalize($markdown)) ->getContent(); } + private function normalize(string $markdown): string + { + return str_replace(["\r\n", "\n\r", "\r"], "\n", $markdown); + } + private function converter(MarkdownOptions $options): MarkdownConverter { $cacheKey = $options->cacheKey(); diff --git a/src/Support/Facades/Activities.php b/src/Support/Facades/Activities.php index 02d963a625b..b9417c9411d 100644 --- a/src/Support/Facades/Activities.php +++ b/src/Support/Facades/Activities.php @@ -6,12 +6,21 @@ use CraftCms\Cms\Activity\Contracts\ActivityEventTypeInterface; use CraftCms\Cms\Activity\Models\ActivityEvent; +use CraftCms\Cms\Element\Contracts\ElementInterface; +use CraftCms\Cms\Site\Data\Site; +use CraftCms\Cms\User\Elements\User; use Illuminate\Contracts\Support\Htmlable; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Facades\Facade; +use Illuminate\Support\HtmlString; /** * @method static ActivityEvent record(ActivityEventTypeInterface $event) + * @method static ActivityEvent createComment(ElementInterface $subject, User $author, Site $site, string $markdown) + * @method static ActivityEvent editComment(ActivityEvent $comment, User $author, string $markdown, ?ElementInterface $subject = null) + * @method static ActivityEvent deleteComment(ActivityEvent $comment, User $actor) + * @method static bool canMention(User $user, ElementInterface $subject) + * @method static HtmlString renderComment(ActivityEvent $version) * @method static Builder query() * @method static string|Htmlable format(ActivityEvent $event) * @method static string icon(ActivityEvent $event) diff --git a/src/SystemMessage/SystemMessageCatalog.php b/src/SystemMessage/SystemMessageCatalog.php index 8743182dc2b..d551c74bf0d 100644 --- a/src/SystemMessage/SystemMessageCatalog.php +++ b/src/SystemMessage/SystemMessageCatalog.php @@ -29,7 +29,7 @@ class SystemMessageCatalog public function __construct( private readonly Container $container, ) { - foreach (['account_activation', 'verify_new_email', 'forgot_password', 'test_email'] as $key) { + foreach (['account_activation', 'comment_mention', 'verify_new_email', 'forgot_password', 'test_email'] as $key) { $this->register($key, fn () => new SystemMessage([ 'key' => $key, 'heading' => t("{$key}_heading"), diff --git a/src/User/Notifications/ActivityMentionNotification.php b/src/User/Notifications/ActivityMentionNotification.php new file mode 100644 index 00000000000..7fc8796e319 --- /dev/null +++ b/src/User/Notifications/ActivityMentionNotification.php @@ -0,0 +1,118 @@ +queue = Cms::config()->queueName; + } + + /** @return class-string[] */ + public function via(mixed $notifiable): array + { + return [MailChannel::class]; + } + + public function shouldSend(CraftUser $notifiable, string $channel): bool + { + $recipient = User::find() + ->id($notifiable->getCraftUserId()) + ->status(User::STATUS_ACTIVE) + ->one(); + $subject = $this->subject(); + + return $recipient !== null + && $subject !== null + && Gate::forUser($recipient)->allows('view', $subject); + } + + public function toMail(CraftUser $notifiable): SystemMessageMailable + { + $version = $this->version(); + $subject = $this->subject($version); + $editUrl = $subject?->getCpEditUrl(); + + if ($subject === null || $editUrl === null) { + throw new LogicException('Activity mention notification subjects must have a control panel edit URL.'); + } + + $mailable = app(SystemMessages::class)->mailable( + key: 'comment_mention', + user: $notifiable->asElement(), + variables: [ + 'author' => $version->data['author']['label'], + 'subject' => $version->snapshots['subject']['label'], + 'comment' => $this->notificationComment($version), + 'link' => Template::raw(Url::cpUrl($editUrl)), + ], + ); + $mailable->siteId = $version->siteId; + + return $mailable; + } + + private function version(): ActivityEvent + { + return ActivityEvent::query()->findOrFail($this->versionEventId); + } + + private function subject(?ActivityEvent $version = null): ?ElementInterface + { + $version ??= $this->version(); + + if ($version->subjectId === null) { + return null; + } + + return Elements::getElementByUid( + $version->subjectId, + $version->subjectType, + $version->siteId, + ); + } + + private function notificationComment(ActivityEvent $version): string + { + $mentionData = $version->data['mentions'] ?? []; + + if (! is_array($mentionData)) { + throw new UnexpectedValueException('Activity comment mentions must be an array.'); + } + + $mentions = collect($mentionData)->keyBy('id'); + + return preg_replace_callback( + '/\[((?:\\\\.|[^]\\\\])*)]\(craft-user:(\d+)\)/', + fn (array $match): string => isset($mentions[$match[2]]) + ? "@{$mentions[$match[2]]['username']}" + : $match[0], + $version->data['markdown'], + ) ?? $version->data['markdown']; + } +} diff --git a/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php b/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php index e7160daa847..75c3c6c6e63 100644 --- a/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php +++ b/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php @@ -8,8 +8,14 @@ use CraftCms\Cms\Activity\EventTypes\ElementUpdated; use CraftCms\Cms\Activity\Models\ActivityEvent; use CraftCms\Cms\Cms; +use CraftCms\Cms\Database\Table; +use CraftCms\Cms\Entry\Models\Entry; use CraftCms\Cms\GarbageCollection\Actions\PurgeExpiredActivity; +use CraftCms\Cms\Site\Models\Site; +use CraftCms\Cms\Support\Facades\Sites; +use CraftCms\Cms\User\Models\User; use Illuminate\Support\Facades\Date; +use Illuminate\Support\Facades\DB; afterEach(fn () => Date::setTestNow()); @@ -25,20 +31,30 @@ expect(ActivityEvent::query()->whereKey($event->id)->exists())->toBeTrue(); }); -it('purges activity older than the retention duration', function () { +it('purges eligible standalone events and complete comment groups', function () { Cms::config()->activityRetentionDuration(3600); $activities = app(Activities::class); - $subject = new ActivitySubject('document', 'one', 'Document one'); + $author = User::factory()->createElement(); + $entry = Entry::factory()->createElement(); + $site = Sites::getSiteById(Site::factory()->create()->id); Date::setTestNow('2026-08-26 10:00:00'); - $expired = $activities->record(new ElementCreated(subject: $subject)); + $expired = $activities->record(new ElementCreated(subject: $entry)); + $comment = $activities->createComment($entry, $author, $site, 'Original comment'); Date::setTestNow('2026-08-26 12:00:00'); - $retained = $activities->record(new ElementUpdated(subject: $subject)); + $edit = $activities->editComment($comment, $author, 'Edited comment', $entry); + $retained = $activities->record(new ElementUpdated(subject: $entry)); + DB::table(Table::ACTIVITYNOTIFICATIONS)->insert([ + 'activityEventId' => $comment->id, + 'userId' => $author->id, + 'versionEventId' => $edit->id, + ]); app(PurgeExpiredActivity::class)(); - expect(ActivityEvent::query()->whereKey($retained->id)->exists())->toBeTrue() + expect(ActivityEvent::query()->pluck('id')->all())->toBe([$retained->id]) + ->and(DB::table(Table::ACTIVITYNOTIFICATIONS)->count())->toBe(0) ->and(ActivityEvent::query()->whereKey($expired->id)->exists())->toBeFalse(); }); From a0d836636e9caf67967ffa63bf1437ac8b2d65e0 Mon Sep 17 00:00:00 2001 From: Rias Date: Thu, 27 Aug 2026 14:12:26 +0200 Subject: [PATCH 2/3] Address activity comments review feedback --- src/Activity/Activities.php | 37 ---- src/Activity/ActivityComments.php | 121 +++++++------ ...000_create_activitynotifications_table.php | 40 ----- src/Database/Migrations/Install.php | 10 -- src/Database/Table.php | 2 - .../Extensions/UserMentionExtension.php | 36 ++++ src/Markdown/Flavors/GfmFlavor.php | 2 + src/Markdown/Markdown.php | 9 +- src/Support/Facades/Activities.php | 9 - .../ActivityMentionNotification.php | 33 +--- .../Feature/Activity/ActivityCommentsTest.php | 160 ++++++++++++++++++ .../Actions/PurgeExpiredActivityTest.php | 14 +- 12 files changed, 269 insertions(+), 204 deletions(-) delete mode 100644 src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php create mode 100644 src/Markdown/CommonMark/Extensions/UserMentionExtension.php create mode 100644 tests/Feature/Activity/ActivityCommentsTest.php diff --git a/src/Activity/Activities.php b/src/Activity/Activities.php index 518914e3240..0f0ab38d50d 100644 --- a/src/Activity/Activities.php +++ b/src/Activity/Activities.php @@ -6,10 +6,7 @@ use CraftCms\Cms\Activity\Contracts\ActivityEventTypeInterface; use CraftCms\Cms\Activity\Models\ActivityEvent; -use CraftCms\Cms\Element\Contracts\ElementInterface; -use CraftCms\Cms\Site\Data\Site; use CraftCms\Cms\Support\HtmlSanitizer\HtmlSanitizerManager; -use CraftCms\Cms\User\Elements\User; use Illuminate\Container\Attributes\Scoped; use Illuminate\Contracts\Support\Htmlable; use Illuminate\Database\Eloquent\Builder; @@ -24,7 +21,6 @@ class Activities public function __construct( private readonly HtmlSanitizerManager $htmlSanitizers, private readonly ActivityEventRecorder $events, - private readonly ActivityComments $comments, ) {} public function record(ActivityEventTypeInterface $event): ActivityEvent @@ -32,39 +28,6 @@ public function record(ActivityEventTypeInterface $event): ActivityEvent return $this->events->record($event); } - public function createComment( - ElementInterface $subject, - User $author, - Site $site, - string $markdown, - ): ActivityEvent { - return $this->comments->create($subject, $author, $site, $markdown); - } - - public function editComment( - ActivityEvent $comment, - User $author, - string $markdown, - ?ElementInterface $subject = null, - ): ActivityEvent { - return $this->comments->edit($comment, $author, $markdown, $subject); - } - - public function deleteComment(ActivityEvent $comment, User $actor): ActivityEvent - { - return $this->comments->delete($comment, $actor); - } - - public function canMention(User $user, ElementInterface $subject): bool - { - return $this->comments->canMention($user, $subject); - } - - public function renderComment(ActivityEvent $version): HtmlString - { - return $this->comments->render($version); - } - /** @return Builder */ public function query(): Builder { diff --git a/src/Activity/ActivityComments.php b/src/Activity/ActivityComments.php index 5c03cbfe6a2..4da239f9175 100644 --- a/src/Activity/ActivityComments.php +++ b/src/Activity/ActivityComments.php @@ -10,7 +10,6 @@ use CraftCms\Cms\Activity\EventTypes\CommentEdited; use CraftCms\Cms\Activity\EventTypes\CommentEvent; use CraftCms\Cms\Activity\Models\ActivityEvent; -use CraftCms\Cms\Database\Table; use CraftCms\Cms\Element\Contracts\ElementInterface; use CraftCms\Cms\Markdown\Markdown; use CraftCms\Cms\Site\Data\Site; @@ -18,16 +17,17 @@ use CraftCms\Cms\User\Elements\User; use CraftCms\Cms\User\Models\User as UserModel; use CraftCms\Cms\User\Notifications\ActivityMentionNotification; +use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; use Illuminate\Support\HtmlString; use Illuminate\Validation\ValidationException; use League\CommonMark\Extension\CommonMark\Node\Inline\Link; +use League\CommonMark\Extension\Mention\Mention; use League\CommonMark\Node\Block\Document; use League\CommonMark\Node\Inline\Text; use League\CommonMark\Node\NodeIterator; use LogicException; -use Throwable; use UnexpectedValueException; use function CraftCms\Cms\t; @@ -59,7 +59,7 @@ public function create( mentions: $this->resolveMentions($markdown, $subject), )); - $this->scheduleMentionNotifications($event, $event); + $this->scheduleMentionNotifications($event, $event->data['mentions']); return $event; }); @@ -69,7 +69,7 @@ public function edit( ActivityEvent $comment, User $author, string $markdown, - ?ElementInterface $subject = null, + ElementInterface $subject, ): ActivityEvent { $this->validate($markdown); @@ -88,15 +88,9 @@ public function canMention(User $user, ElementInterface $subject): bool && Gate::forUser($user)->allows('view', $subject); } - public function render(ActivityEvent $version): HtmlString + public function render(ActivityEvent $version, User $viewer): HtmlString { - $mentionData = $version->data['mentions'] ?? []; - - if (! is_array($mentionData)) { - throw new UnexpectedValueException('Activity comment mentions must be an array.'); - } - - $mentions = collect($mentionData)->keyBy('id'); + $mentions = $this->mentions($version); $users = User::find() ->id($mentions->keys()->all()) ->status(null) @@ -104,21 +98,20 @@ public function render(ActivityEvent $version): HtmlString ->keyBy('id'); $html = $this->markdown->transform( $version->data['markdown'], - function (Document $document) use ($mentions, $users): void { - foreach ($this->mentionLinks($document) as [$node, $reference]) { - if (! ctype_digit($reference)) { - continue; - } - - $id = (int) $reference; - $mention = $mentions->get($id); + function (Document $document) use ($mentions, $users, $viewer): void { + foreach ($this->mentionNodes($document) as $node) { + $reference = $node->getIdentifier(); + $mention = ctype_digit($reference) ? $mentions->get((int) $reference) : null; if ($mention === null) { + $node->replaceWith(new Text($node->getLabel() ?? "@$reference")); + continue; } + $id = (int) $reference; $user = $users->get($id); - $canView = $user !== null && Gate::check('view', $user); + $canView = $user !== null && Gate::forUser($viewer)->allows('view', $user); $username = $canView ? ($user->username ?? $mention['username']) : $mention['username']; $node->replaceWith($canView && $user->getCpEditUrl() !== null @@ -132,6 +125,13 @@ function (Document $document) use ($mentions, $users): void { return new HtmlString($this->htmlSanitizers->sanitize($html)); } + public function notificationText(ActivityEvent $version, User $viewer): string + { + $html = $this->render($version, $viewer)->toHtml(); + + return trim(html_entity_decode(strip_tags($html), ENT_QUOTES | ENT_HTML5, 'UTF-8')); + } + /** @param class-string $eventType */ private function mutate( ActivityEvent $comment, @@ -142,11 +142,10 @@ private function mutate( ): ActivityEvent { return DB::transaction(function () use ($comment, $actor, $eventType, $markdown, $liveSubject): ActivityEvent { $root = ActivityEvent::query() - ->whereKey($comment->id) - ->where('eventType', CommentCreated::class) + ->eventTypes(CommentCreated::class) ->whereNull('rootEventId') ->lockForUpdate() - ->firstOrFail(); + ->findOrFail($comment->id); $current = ActivityEvent::query() ->where('rootEventId', $root->id) ->newestFirst() @@ -169,6 +168,9 @@ private function mutate( throw new LogicException('Activity comments require a current site.'); } + $mentions = $markdown === null + ? ($current->data['mentions'] ?? []) + : $this->resolveMentions($markdown, $liveSubject); $event = $this->events->record(new $eventType( subject: $subject, actor: $actor, @@ -176,49 +178,30 @@ private function mutate( markdown: $markdown ?? $current->data['markdown'], authorId: $root->data['author']['id'], authorLabel: $root->data['author']['label'], - mentions: $markdown === null - ? ($current->data['mentions'] ?? []) - : $this->resolveMentions($markdown, $liveSubject), + mentions: $mentions, ), rootEventId: $root->id); if ($markdown !== null) { - $this->scheduleMentionNotifications($root, $event); + $previousMentionIds = array_column($current->data['mentions'] ?? [], 'id'); + $addedMentions = array_values(array_filter( + $mentions, + fn (array $mention): bool => ! in_array($mention['id'], $previousMentionIds, true), + )); + + $this->scheduleMentionNotifications($event, $addedMentions); } return $event; }); } - private function scheduleMentionNotifications(ActivityEvent $comment, ActivityEvent $version): void + /** @param list $mentions */ + private function scheduleMentionNotifications(ActivityEvent $version, array $mentions): void { - foreach ($version->data['mentions'] as $mention) { - $pair = [ - 'activityEventId' => $comment->id, - 'userId' => $mention['id'], - ]; - - if (DB::table(Table::ACTIVITYNOTIFICATIONS)->where($pair)->exists()) { - continue; - } - - DB::table(Table::ACTIVITYNOTIFICATIONS)->insert([ - ...$pair, - 'versionEventId' => $version->id, - ]); - - DB::afterCommit(function () use ($mention, $pair, $version): void { - try { - UserModel::query() - ->findOrFail($mention['id']) - ->notify(new ActivityMentionNotification($version->id)); - } catch (Throwable $exception) { - DB::table(Table::ACTIVITYNOTIFICATIONS) - ->where($pair) - ->where('versionEventId', $version->id) - ->delete(); - report($exception); - } - }); + foreach ($mentions as $mention) { + UserModel::query() + ->findOrFail($mention['id']) + ->notify(new ActivityMentionNotification($version->id)); } } @@ -229,8 +212,8 @@ private function resolveMentions(string $markdown, ?ElementInterface $subject): $this->markdown->transform( $markdown, function (Document $document) use (&$references): void { - foreach ($this->mentionLinks($document) as [, $reference]) { - $references[] = $reference; + foreach ($this->mentionNodes($document) as $node) { + $references[] = $node->getIdentifier(); } }, Markdown::FLAVOR_GFM_COMMENT, @@ -271,16 +254,28 @@ function (Document $document) use (&$references): void { })->all(); } - /** @return iterable */ - private function mentionLinks(Document $document): iterable + /** @return iterable */ + private function mentionNodes(Document $document): iterable { foreach (new NodeIterator($document) as $node) { - if ($node instanceof Link && str_starts_with($node->getUrl(), 'craft-user:')) { - yield [$node, substr($node->getUrl(), strlen('craft-user:'))]; + if ($node instanceof Mention) { + yield $node; } } } + /** @return Collection */ + private function mentions(ActivityEvent $version): Collection + { + $mentions = $version->data['mentions'] ?? []; + + if (! is_array($mentions)) { + throw new UnexpectedValueException('Activity comment mentions must be an array.'); + } + + return collect($mentions)->keyBy('id'); + } + private function validate(string $markdown): void { if (blank($markdown)) { diff --git a/src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php b/src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php deleted file mode 100644 index 38c2d46cbc3..00000000000 --- a/src/Database/Migrations/2026_08_26_000000_create_activitynotifications_table.php +++ /dev/null @@ -1,40 +0,0 @@ -unsignedBigInteger('activityEventId'); - $table->unsignedBigInteger('userId'); - $table->unsignedBigInteger('versionEventId'); - }); - - Schema::createIndex(Table::ACTIVITYNOTIFICATIONS, ['activityEventId', 'userId'], unique: true); - - Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('activityEventId') - ->references('id') - ->on(Table::ACTIVITYEVENTS) - ->cascadeOnDelete()); - Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('versionEventId') - ->references('id') - ->on(Table::ACTIVITYEVENTS) - ->cascadeOnDelete()); - } - - public function down(): void - { - Schema::dropIfExists(Table::ACTIVITYNOTIFICATIONS); - } -}; diff --git a/src/Database/Migrations/Install.php b/src/Database/Migrations/Install.php index 2922ee47d97..2ff6ebbb023 100644 --- a/src/Database/Migrations/Install.php +++ b/src/Database/Migrations/Install.php @@ -224,13 +224,6 @@ public function createTables(?Logger $logger = null): void $table->dateTime('occurredAt'); }); - $logger?->subLabel('activitynotifications'); - Schema::create(Table::ACTIVITYNOTIFICATIONS, function (Blueprint $table) { - $table->unsignedBigInteger('activityEventId'); - $table->unsignedBigInteger('userId'); - $table->unsignedBigInteger('versionEventId'); - }); - $logger?->subLabel('addresses'); Schema::create('addresses', function (Blueprint $table) { $table->integer('id', true); @@ -1046,7 +1039,6 @@ public function createIndexes(): void Schema::createIndex(Table::ACTIVITYEVENTS, ['subjectType', 'subjectId', 'siteId', 'occurredAt', 'id']); Schema::createIndex(Table::ACTIVITYEVENTS, ['occurredAt', 'id']); Schema::createIndex(Table::ACTIVITYEVENTS, ['rootEventId', 'occurredAt', 'id']); - Schema::createIndex(Table::ACTIVITYNOTIFICATIONS, ['activityEventId', 'userId'], unique: true); Schema::createIndex(Table::ASSETINDEXDATA, ['sessionId', 'volumeId']); Schema::createIndex(Table::ASSETINDEXDATA, ['sessionId', 'status', 'id']); Schema::createIndex(Table::ASSETINDEXDATA, ['volumeId']); @@ -1193,8 +1185,6 @@ public function createIndexes(): void public function addForeignKeys(): void { Schema::table(Table::ACTIVITYEVENTS, fn (Blueprint $table) => $table->foreign('rootEventId')->references('id')->on(Table::ACTIVITYEVENTS)->cascadeOnDelete()); - Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('activityEventId')->references('id')->on(Table::ACTIVITYEVENTS)->cascadeOnDelete()); - Schema::table(Table::ACTIVITYNOTIFICATIONS, fn (Blueprint $table) => $table->foreign('versionEventId')->references('id')->on(Table::ACTIVITYEVENTS)->cascadeOnDelete()); Schema::table(Table::ADDRESSES, fn (Blueprint $table) => $table->foreign('id')->references('id')->on(Table::ELEMENTS)->cascadeOnDelete()); Schema::table(Table::ADDRESSES, fn (Blueprint $table) => $table->foreign('primaryOwnerId')->references('id')->on(Table::ELEMENTS)->cascadeOnDelete()); Schema::table(Table::ASSETINDEXDATA, fn (Blueprint $table) => $table->foreign('volumeId')->references('id')->on(Table::VOLUMES)->cascadeOnDelete()); diff --git a/src/Database/Table.php b/src/Database/Table.php index aa517aa0191..e3fd7c75e2c 100644 --- a/src/Database/Table.php +++ b/src/Database/Table.php @@ -11,8 +11,6 @@ { public const string ACTIVITYEVENTS = 'activityevents'; - public const string ACTIVITYNOTIFICATIONS = 'activitynotifications'; - public const string ADDRESSES = 'addresses'; public const string ASSETINDEXDATA = 'assetindexdata'; diff --git a/src/Markdown/CommonMark/Extensions/UserMentionExtension.php b/src/Markdown/CommonMark/Extensions/UserMentionExtension.php new file mode 100644 index 00000000000..0e206ab0b5e --- /dev/null +++ b/src/Markdown/CommonMark/Extensions/UserMentionExtension.php @@ -0,0 +1,36 @@ +addEventListener(DocumentParsedEvent::class, $this(...)); + } + + public function __invoke(DocumentParsedEvent $event): void + { + foreach ($event->getDocument()->iterator() as $node) { + if (! $node instanceof Link || ! str_starts_with($node->getUrl(), self::URL_PREFIX)) { + continue; + } + + $mention = new Mention('user', '@', substr($node->getUrl(), strlen(self::URL_PREFIX))); + $mention->setUrl($node->getUrl()); + $mention->setTitle($node->getTitle()); + $mention->replaceChildren($node->children()); + $node->replaceWith($mention); + } + } +} diff --git a/src/Markdown/Flavors/GfmFlavor.php b/src/Markdown/Flavors/GfmFlavor.php index b74040155f2..d6fea7a2aca 100644 --- a/src/Markdown/Flavors/GfmFlavor.php +++ b/src/Markdown/Flavors/GfmFlavor.php @@ -4,6 +4,7 @@ namespace CraftCms\Cms\Markdown\Flavors; +use CraftCms\Cms\Markdown\CommonMark\Extensions\UserMentionExtension; use CraftCms\Cms\Markdown\MarkdownOptions; use League\CommonMark\Environment\Environment; use League\CommonMark\Extension\Autolink\AutolinkExtension; @@ -21,6 +22,7 @@ public function __construct(private readonly string $softBreak = "\n") {} public function __invoke(MarkdownOptions $options): MarkdownConverter { $environment = $this->environment($options, $this->softBreak); + $environment->addExtension(new UserMentionExtension); if ($options->inlineOnly) { $environment diff --git a/src/Markdown/Markdown.php b/src/Markdown/Markdown.php index 9223aa6c85f..761f9d41c2f 100644 --- a/src/Markdown/Markdown.php +++ b/src/Markdown/Markdown.php @@ -91,7 +91,7 @@ public function transform(string $markdown, callable $transform, ?string $flavor $options = new MarkdownOptions(flavor: $flavor); $environment = $this->converter($options)->getEnvironment(); - $document = new MarkdownParser($environment)->parse($this->normalize($markdown)); + $document = new MarkdownParser($environment)->parse($markdown); $transform($document); @@ -105,15 +105,10 @@ public function convert(string $markdown, MarkdownOptions $options): string } return $this->converter($options) - ->convert($this->normalize($markdown)) + ->convert($markdown) ->getContent(); } - private function normalize(string $markdown): string - { - return str_replace(["\r\n", "\n\r", "\r"], "\n", $markdown); - } - private function converter(MarkdownOptions $options): MarkdownConverter { $cacheKey = $options->cacheKey(); diff --git a/src/Support/Facades/Activities.php b/src/Support/Facades/Activities.php index b9417c9411d..02d963a625b 100644 --- a/src/Support/Facades/Activities.php +++ b/src/Support/Facades/Activities.php @@ -6,21 +6,12 @@ use CraftCms\Cms\Activity\Contracts\ActivityEventTypeInterface; use CraftCms\Cms\Activity\Models\ActivityEvent; -use CraftCms\Cms\Element\Contracts\ElementInterface; -use CraftCms\Cms\Site\Data\Site; -use CraftCms\Cms\User\Elements\User; use Illuminate\Contracts\Support\Htmlable; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Facades\Facade; -use Illuminate\Support\HtmlString; /** * @method static ActivityEvent record(ActivityEventTypeInterface $event) - * @method static ActivityEvent createComment(ElementInterface $subject, User $author, Site $site, string $markdown) - * @method static ActivityEvent editComment(ActivityEvent $comment, User $author, string $markdown, ?ElementInterface $subject = null) - * @method static ActivityEvent deleteComment(ActivityEvent $comment, User $actor) - * @method static bool canMention(User $user, ElementInterface $subject) - * @method static HtmlString renderComment(ActivityEvent $version) * @method static Builder query() * @method static string|Htmlable format(ActivityEvent $event) * @method static string icon(ActivityEvent $event) diff --git a/src/User/Notifications/ActivityMentionNotification.php b/src/User/Notifications/ActivityMentionNotification.php index 7fc8796e319..ffc72ad7bd8 100644 --- a/src/User/Notifications/ActivityMentionNotification.php +++ b/src/User/Notifications/ActivityMentionNotification.php @@ -4,6 +4,7 @@ namespace CraftCms\Cms\User\Notifications; +use CraftCms\Cms\Activity\ActivityComments; use CraftCms\Cms\Activity\Models\ActivityEvent; use CraftCms\Cms\Cms; use CraftCms\Cms\Element\Contracts\ElementInterface; @@ -15,14 +16,12 @@ use CraftCms\Cms\User\Contracts\CraftUser; use CraftCms\Cms\User\Elements\User; use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Contracts\Queue\ShouldQueueAfterCommit; use Illuminate\Notifications\Channels\MailChannel; use Illuminate\Notifications\Notification; -use Illuminate\Support\Facades\Gate; use LogicException; -use UnexpectedValueException; -class ActivityMentionNotification extends Notification implements ShouldQueue +class ActivityMentionNotification extends Notification implements ShouldQueueAfterCommit { use Queueable; @@ -49,7 +48,7 @@ public function shouldSend(CraftUser $notifiable, string $channel): bool return $recipient !== null && $subject !== null - && Gate::forUser($recipient)->allows('view', $subject); + && app(ActivityComments::class)->canMention($recipient, $subject); } public function toMail(CraftUser $notifiable): SystemMessageMailable @@ -62,13 +61,14 @@ public function toMail(CraftUser $notifiable): SystemMessageMailable throw new LogicException('Activity mention notification subjects must have a control panel edit URL.'); } + $recipient = $notifiable->asElement(); $mailable = app(SystemMessages::class)->mailable( key: 'comment_mention', - user: $notifiable->asElement(), + user: $recipient, variables: [ 'author' => $version->data['author']['label'], 'subject' => $version->snapshots['subject']['label'], - 'comment' => $this->notificationComment($version), + 'comment' => app(ActivityComments::class)->notificationText($version, $recipient), 'link' => Template::raw(Url::cpUrl($editUrl)), ], ); @@ -96,23 +96,4 @@ private function subject(?ActivityEvent $version = null): ?ElementInterface $version->siteId, ); } - - private function notificationComment(ActivityEvent $version): string - { - $mentionData = $version->data['mentions'] ?? []; - - if (! is_array($mentionData)) { - throw new UnexpectedValueException('Activity comment mentions must be an array.'); - } - - $mentions = collect($mentionData)->keyBy('id'); - - return preg_replace_callback( - '/\[((?:\\\\.|[^]\\\\])*)]\(craft-user:(\d+)\)/', - fn (array $match): string => isset($mentions[$match[2]]) - ? "@{$mentions[$match[2]]['username']}" - : $match[0], - $version->data['markdown'], - ) ?? $version->data['markdown']; - } } diff --git a/tests/Feature/Activity/ActivityCommentsTest.php b/tests/Feature/Activity/ActivityCommentsTest.php new file mode 100644 index 00000000000..f49cbc8c3d1 --- /dev/null +++ b/tests/Feature/Activity/ActivityCommentsTest.php @@ -0,0 +1,160 @@ +create(); + Sites::refreshSites(); + Notification::fake(); + + $this->comments = app(ActivityComments::class); + $this->author = User::findOne(); + $this->entry = Entry::factory()->createElement(['title' => 'Release notes']); + $this->site = Sites::getSiteById($this->entry->siteId); + $this->mentionPermissions = [ + 'accessCp', + "editSite:{$this->site->uid}", + "viewEntries:{$this->entry->getSection()->uid}", + "viewPeerEntries:{$this->entry->getSection()->uid}", + ]; + $this->mentioned = UserModel::factory() + ->withPermissions($this->mentionPermissions) + ->createElement(['admin' => false, 'username' => 'grace']); + + $this->actingAs($this->author); + DB::table(Table::ACTIVITYEVENTS)->delete(); +}); + +it('records immutable comment lifecycle versions', function () { + $created = $this->comments->create($this->entry, $this->author, $this->site, 'First version'); + $edited = $this->comments->edit($created, $this->author, 'Second version', $this->entry); + $deleted = $this->comments->delete($created, $this->author); + + expect($created->eventType)->toBe(CommentCreated::class) + ->and($created->rootEventId)->toBeNull() + ->and($edited->eventType)->toBe(CommentEdited::class) + ->and($edited->rootEventId)->toBe($created->id) + ->and($deleted->eventType)->toBe(CommentDeleted::class) + ->and($deleted->rootEventId)->toBe($created->id) + ->and($deleted->data['markdown'])->toBe('Second version'); + + expect(fn () => $this->comments->edit($created, $this->author, 'Resurrected', $this->entry)) + ->toThrow(ValidationException::class); +}); + +it('stores, validates, and renders structured mentions', function () { + $comment = $this->comments->create( + $this->entry, + $this->author, + $this->site, + "Hello [@grace](craft-user:{$this->mentioned->id}) and @plain.", + ); + $document = new DOMDocument; + $document->loadHTML( + $this->comments->render($comment, $this->author)->toHtml(), + LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD, + ); + + expect($comment->data['mentions'])->toBe([[ + 'id' => $this->mentioned->id, + 'username' => 'grace', + ]]) + ->and($document->textContent)->toBe('Hello @grace and @plain.'); + + UserModel::query()->whereKey($this->mentioned->id)->update(['username' => 'hopper']); + $document->loadHTML( + $this->comments->render($comment, $this->author)->toHtml(), + LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD, + ); + + expect($document->textContent)->toBe('Hello @hopper and @plain.'); + + $ineligible = UserModel::factory() + ->withPermissions(['accessCp']) + ->createElement(['admin' => false, 'username' => 'ineligible']); + + expect(fn () => $this->comments->create( + $this->entry, + $this->author, + $this->site, + "Hello [@ineligible](craft-user:{$ineligible->id}).", + ))->toThrow(ValidationException::class) + ->and(fn () => $this->comments->create( + $this->entry, + $this->author, + $this->site, + " \n\t ", + ))->toThrow(ValidationException::class); +}); + +it('normalizes CommonMark mention links in notification comments', function (Closure $mention) { + $comment = $this->comments->create( + $this->entry, + $this->author, + $this->site, + sprintf('Hello **A & B**, %s.', $mention($this->mentioned->id)), + ); + $notifiable = UserModel::query()->findOrFail($this->mentioned->id); + + expect(new ActivityMentionNotification($comment->id)->toMail($notifiable)->variables['comment']) + ->toBe('Hello A & B, @grace.'); +})->with([ + 'plain destination' => fn (int $id): string => "[@grace](craft-user:$id)", + 'angle destination' => fn (int $id): string => "[@grace]()", + 'destination with title' => fn (int $id): string => "[@grace](craft-user:$id \"Grace Hopper\")", +]); + +it('rechecks complete mention eligibility before sending', function () { + $comment = $this->comments->create( + $this->entry, + $this->author, + $this->site, + "Hello [@grace](craft-user:{$this->mentioned->id}).", + ); + $notification = new ActivityMentionNotification($comment->id); + $notifiable = UserModel::query()->findOrFail($this->mentioned->id); + + expect($notification->shouldSend($notifiable, 'mail'))->toBeTrue(); + + UserPermissions::saveUserPermissions( + $this->mentioned->id, + array_values(array_diff($this->mentionPermissions, ['accessCp'])), + ); + UserPermissions::reset(); + + expect($notification->shouldSend($notifiable, 'mail'))->toBeFalse(); +}); + +it('notifies users added by comment edits', function () { + $added = UserModel::factory() + ->withPermissions($this->mentionPermissions) + ->createElement(['admin' => false, 'username' => 'ada']); + $markdown = "Hello [@grace](craft-user:{$this->mentioned->id})."; + $comment = $this->comments->create($this->entry, $this->author, $this->site, $markdown); + $editedMarkdown = "$markdown And [@ada](craft-user:{$added->id})."; + + $this->comments->edit($comment, $this->author, $editedMarkdown, $this->entry); + $this->comments->edit($comment, $this->author, $editedMarkdown, $this->entry); + + Notification::assertSentTimes(ActivityMentionNotification::class, 2); + Notification::assertSentTo(UserModel::query()->findOrFail($this->mentioned->id), ActivityMentionNotification::class); + Notification::assertSentTo(UserModel::query()->findOrFail($added->id), ActivityMentionNotification::class); +}); diff --git a/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php b/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php index 75c3c6c6e63..38478f9002c 100644 --- a/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php +++ b/tests/Feature/GarbageCollection/Actions/PurgeExpiredActivityTest.php @@ -3,19 +3,18 @@ declare(strict_types=1); use CraftCms\Cms\Activity\Activities; +use CraftCms\Cms\Activity\ActivityComments; use CraftCms\Cms\Activity\Data\ActivitySubject; use CraftCms\Cms\Activity\EventTypes\ElementCreated; use CraftCms\Cms\Activity\EventTypes\ElementUpdated; use CraftCms\Cms\Activity\Models\ActivityEvent; use CraftCms\Cms\Cms; -use CraftCms\Cms\Database\Table; use CraftCms\Cms\Entry\Models\Entry; use CraftCms\Cms\GarbageCollection\Actions\PurgeExpiredActivity; use CraftCms\Cms\Site\Models\Site; use CraftCms\Cms\Support\Facades\Sites; use CraftCms\Cms\User\Models\User; use Illuminate\Support\Facades\Date; -use Illuminate\Support\Facades\DB; afterEach(fn () => Date::setTestNow()); @@ -34,27 +33,22 @@ it('purges eligible standalone events and complete comment groups', function () { Cms::config()->activityRetentionDuration(3600); $activities = app(Activities::class); + $comments = app(ActivityComments::class); $author = User::factory()->createElement(); $entry = Entry::factory()->createElement(); $site = Sites::getSiteById(Site::factory()->create()->id); Date::setTestNow('2026-08-26 10:00:00'); $expired = $activities->record(new ElementCreated(subject: $entry)); - $comment = $activities->createComment($entry, $author, $site, 'Original comment'); + $comment = $comments->create($entry, $author, $site, 'Original comment'); Date::setTestNow('2026-08-26 12:00:00'); - $edit = $activities->editComment($comment, $author, 'Edited comment', $entry); + $comments->edit($comment, $author, 'Edited comment', $entry); $retained = $activities->record(new ElementUpdated(subject: $entry)); - DB::table(Table::ACTIVITYNOTIFICATIONS)->insert([ - 'activityEventId' => $comment->id, - 'userId' => $author->id, - 'versionEventId' => $edit->id, - ]); app(PurgeExpiredActivity::class)(); expect(ActivityEvent::query()->pluck('id')->all())->toBe([$retained->id]) - ->and(DB::table(Table::ACTIVITYNOTIFICATIONS)->count())->toBe(0) ->and(ActivityEvent::query()->whereKey($expired->id)->exists())->toBeFalse(); }); From 484ca441ed0054f266f3d9c86fe5c3f39394f26c Mon Sep 17 00:00:00 2001 From: Rias Date: Thu, 3 Sep 2026 13:13:07 +0200 Subject: [PATCH 3/3] Refine activity comments and notifications --- src/Activity/ActivityComments.php | 74 ++++++------ src/Activity/EventTypes/CommentDeleted.php | 2 +- src/Activity/EventTypes/CommentEdited.php | 2 +- src/Activity/EventTypes/CommentEvent.php | 2 +- src/Activity/Models/ActivityEvent.php | 10 ++ src/Cp/Notifications/CpNotification.php | 105 ++++++++++++------ .../ActivityMentionNotification.php | 49 ++++---- .../Feature/Activity/ActivityCommentsTest.php | 36 ++++-- .../Notifications/NotificationCenterTest.php | 14 +++ 9 files changed, 180 insertions(+), 114 deletions(-) diff --git a/src/Activity/ActivityComments.php b/src/Activity/ActivityComments.php index 4da239f9175..89ec0bdfaf8 100644 --- a/src/Activity/ActivityComments.php +++ b/src/Activity/ActivityComments.php @@ -20,6 +20,7 @@ use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Gate; +use Illuminate\Support\Facades\Notification; use Illuminate\Support\HtmlString; use Illuminate\Validation\ValidationException; use League\CommonMark\Extension\CommonMark\Node\Inline\Link; @@ -27,7 +28,6 @@ use League\CommonMark\Node\Block\Document; use League\CommonMark\Node\Inline\Text; use League\CommonMark\Node\NodeIterator; -use LogicException; use UnexpectedValueException; use function CraftCms\Cms\t; @@ -43,26 +43,24 @@ public function __construct( public function create( ElementInterface $subject, User $author, - Site $site, + ?Site $site, string $markdown, ): ActivityEvent { $this->validate($markdown); - return DB::transaction(function () use ($subject, $author, $site, $markdown): ActivityEvent { - $event = $this->events->record(new CommentCreated( - subject: $subject, - actor: $author, - site: $site, - markdown: $markdown, - authorId: $author->id, - authorLabel: $author->name, - mentions: $this->resolveMentions($markdown, $subject), - )); + $event = $this->events->record(new CommentCreated( + subject: $subject, + actor: $author, + site: $site, + markdown: $markdown, + authorId: $author->id, + authorLabel: $author->name, + mentions: $this->resolveMentions($markdown, $subject), + )); - $this->scheduleMentionNotifications($event, $event->data['mentions']); + $this->scheduleMentionNotifications($event, $event->data['mentions']); - return $event; - }); + return $event; } public function edit( @@ -147,7 +145,7 @@ private function mutate( ->lockForUpdate() ->findOrFail($comment->id); $current = ActivityEvent::query() - ->where('rootEventId', $root->id) + ->rootEvent($root) ->newestFirst() ->first() ?? $root; @@ -164,10 +162,6 @@ private function mutate( ); $site = $root->siteId === null ? null : Site::get($root->siteId); - if ($site === null) { - throw new LogicException('Activity comments require a current site.'); - } - $mentions = $markdown === null ? ($current->data['mentions'] ?? []) : $this->resolveMentions($markdown, $liveSubject); @@ -198,11 +192,10 @@ private function mutate( /** @param list $mentions */ private function scheduleMentionNotifications(ActivityEvent $version, array $mentions): void { - foreach ($mentions as $mention) { - UserModel::query() - ->findOrFail($mention['id']) - ->notify(new ActivityMentionNotification($version->id)); - } + Notification::send( + UserModel::query()->findMany(array_column($mentions, 'id')), + new ActivityMentionNotification($version), + ); } /** @return list */ @@ -224,15 +217,8 @@ function (Document $document) use (&$references): void { } $ids = collect($references) - ->map(function (string $id): int { - if (! ctype_digit($id)) { - throw ValidationException::withMessages([ - 'markdown' => t('Comment contains an invalid user mention.'), - ]); - } - - return (int) $id; - }) + ->filter(fn (string $id): bool => ctype_digit($id)) + ->map(fn (string $id): int => (int) $id) ->unique() ->values(); $users = User::find() @@ -241,17 +227,19 @@ function (Document $document) use (&$references): void { ->collect() ->keyBy('id'); - return $ids->map(function (int $id) use ($subject, $users): array { - $user = $users->get($id); + return $ids + ->map(function (int $id) use ($subject, $users): ?array { + $user = $users->get($id); - if ($subject === null || $user === null || ! $this->canMention($user, $subject)) { - throw ValidationException::withMessages([ - 'markdown' => t('Comment contains an ineligible user mention.'), - ]); - } + if ($subject === null || $user === null || ! $this->canMention($user, $subject)) { + return null; + } - return ['id' => $user->id, 'username' => $user->username]; - })->all(); + return ['id' => $user->id, 'username' => $user->username]; + }) + ->filter() + ->values() + ->all(); } /** @return iterable */ diff --git a/src/Activity/EventTypes/CommentDeleted.php b/src/Activity/EventTypes/CommentDeleted.php index 29b52125a4d..972bb9796b5 100644 --- a/src/Activity/EventTypes/CommentDeleted.php +++ b/src/Activity/EventTypes/CommentDeleted.php @@ -6,7 +6,7 @@ class CommentDeleted extends CommentEvent { - protected const string LABEL = 'Removed comment'; + protected const string LABEL = 'Comment removed'; protected const string ICON = 'comment-slash'; } diff --git a/src/Activity/EventTypes/CommentEdited.php b/src/Activity/EventTypes/CommentEdited.php index 6f89b0effd1..fda8a2382cb 100644 --- a/src/Activity/EventTypes/CommentEdited.php +++ b/src/Activity/EventTypes/CommentEdited.php @@ -6,7 +6,7 @@ class CommentEdited extends CommentEvent { - protected const string LABEL = 'Edited comment'; + protected const string LABEL = 'Comment edited'; protected const string ICON = 'comment'; } diff --git a/src/Activity/EventTypes/CommentEvent.php b/src/Activity/EventTypes/CommentEvent.php index 28c068ddc39..ebbb8ab8dfa 100644 --- a/src/Activity/EventTypes/CommentEvent.php +++ b/src/Activity/EventTypes/CommentEvent.php @@ -17,7 +17,7 @@ abstract class CommentEvent extends ActivityEventType public function __construct( ElementInterface|ActivitySubject $subject, User|ActivityActor $actor, - Site $site, + ?Site $site, private readonly string $markdown, private readonly int $authorId, private readonly string $authorLabel, diff --git a/src/Activity/Models/ActivityEvent.php b/src/Activity/Models/ActivityEvent.php index dfbc31d0140..df7f81d25c0 100644 --- a/src/Activity/Models/ActivityEvent.php +++ b/src/Activity/Models/ActivityEvent.php @@ -134,6 +134,16 @@ protected function actor(Builder $query, ActivityActor $actor): Builder ->where('actorId', $actor->id); } + /** + * @param Builder $query + * @return Builder + */ + #[Scope] + protected function rootEvent(Builder $query, self|string $rootEvent): Builder + { + return $query->where('rootEventId', $rootEvent instanceof self ? $rootEvent->id : $rootEvent); + } + /** * @param Builder $query * @return Builder diff --git a/src/Cp/Notifications/CpNotification.php b/src/Cp/Notifications/CpNotification.php index 1bf9c885b2c..62d91d142b3 100644 --- a/src/Cp/Notifications/CpNotification.php +++ b/src/Cp/Notifications/CpNotification.php @@ -6,9 +6,12 @@ use Closure; use CraftCms\Cms\Cp\Data\NotificationButtonData; +use CraftCms\Cms\User\Contracts\CraftUser; use Illuminate\Notifications\Channels\DatabaseChannel; use Illuminate\Notifications\Notification; use Illuminate\Support\Arr; +use Laravel\SerializableClosure\SerializableClosure; +use UnexpectedValueException; class CpNotification extends Notification { @@ -16,51 +19,53 @@ class CpNotification extends Notification protected string $kind; - protected string|Closure|null $title = null; + protected string|SerializableClosure|null $title = null; - protected string|Closure|null $byline = null; + protected string|SerializableClosure|null $byline = null; - protected string|Closure|null $icon = null; + protected string|SerializableClosure|null $icon = null; - protected string|Closure|null $image = null; + protected string|SerializableClosure|null $image = null; - protected string|Closure|null $imageAlt = null; + protected string|SerializableClosure|null $imageAlt = null; - protected string|Closure|null $url = null; + protected string|SerializableClosure|null $url = null; - /** @var list|Closure(object): list */ - protected array|Closure $buttons = []; + /** @var list|SerializableClosure */ + protected array|SerializableClosure $buttons = []; - /** @param string|Closure(object): string $message */ - public function __construct( - protected string|Closure $message, - ) { + protected string|SerializableClosure $message; + + /** @param string|Closure(CraftUser): string $message */ + public function __construct(string|Closure $message) + { $this->kind = static::class; + $this->message = $this->serializable($message); } /** @return class-string[] */ - public function via(object $notifiable): array + public function via(CraftUser $notifiable): array { return [DatabaseChannel::class]; } /** @return array */ - public function toDatabase(object $notifiable): array + public function toDatabase(CraftUser $notifiable): array { return Arr::whereNotNull([ 'kind' => $this->kind, - 'title' => value($this->title, $notifiable), - 'message' => value($this->message, $notifiable), - 'byline' => value($this->byline, $notifiable), - 'icon' => value($this->icon, $notifiable), - 'image' => value($this->image, $notifiable), - 'imageAlt' => value($this->imageAlt, $notifiable), - 'url' => value($this->url, $notifiable), - 'buttons' => collect(value($this->buttons, $notifiable))->toArray(), + 'title' => $this->resolve($this->title, $notifiable), + 'message' => $this->resolve($this->message, $notifiable), + 'byline' => $this->resolve($this->byline, $notifiable), + 'icon' => $this->resolve($this->icon, $notifiable), + 'image' => $this->resolve($this->image, $notifiable), + 'imageAlt' => $this->resolve($this->imageAlt, $notifiable), + 'url' => $this->resolve($this->url, $notifiable), + 'buttons' => collect($this->resolveButtons($notifiable))->toArray(), ]); } - public function databaseType(object $notifiable): string + public function databaseType(CraftUser $notifiable): string { return self::TYPE; } @@ -72,55 +77,81 @@ public function kind(string $kind): static return $this; } - /** @param string|Closure(object): string|null $title */ + /** @param string|Closure(CraftUser): string|null $title */ public function title(string|Closure|null $title): static { - $this->title = $title; + $this->title = $this->serializable($title); return $this; } - /** @param string|Closure(object): string|null $byline */ + /** @param string|Closure(CraftUser): string|null $byline */ public function byline(string|Closure|null $byline): static { - $this->byline = $byline; + $this->byline = $this->serializable($byline); return $this; } - /** @param string|Closure(object): string|null $icon */ + /** @param string|Closure(CraftUser): string|null $icon */ public function icon(string|Closure|null $icon): static { - $this->icon = $icon; + $this->icon = $this->serializable($icon); return $this; } /** - * @param string|Closure(object): string $url - * @param string|Closure(object): string $alt + * @param string|Closure(CraftUser): string $url + * @param string|Closure(CraftUser): string $alt */ public function image(string|Closure $url, string|Closure $alt): static { - $this->image = $url; - $this->imageAlt = $alt; + $this->image = $this->serializable($url); + $this->imageAlt = $this->serializable($alt); return $this; } - /** @param string|Closure(object): string|null $url */ + /** @param string|Closure(CraftUser): string|null $url */ public function url(string|Closure|null $url): static { - $this->url = $url; + $this->url = $this->serializable($url); return $this; } - /** @param list|Closure(object): list $buttons */ + /** @param list|Closure(CraftUser): list $buttons */ public function buttons(array|Closure $buttons): static { - $this->buttons = $buttons; + $this->buttons = $buttons instanceof Closure ? new SerializableClosure($buttons) : $buttons; return $this; } + + private function serializable(string|Closure|null $value): string|SerializableClosure|null + { + return $value instanceof Closure ? new SerializableClosure($value) : $value; + } + + private function resolve(string|SerializableClosure|null $value, CraftUser $notifiable): mixed + { + return $value instanceof SerializableClosure ? $value($notifiable) : $value; + } + + /** @return array */ + private function resolveButtons(CraftUser $notifiable): array + { + if (is_array($this->buttons)) { + return $this->buttons; + } + + $buttons = ($this->buttons)($notifiable); + + if (! is_array($buttons)) { + throw new UnexpectedValueException('CP notification button callbacks must return an array.'); + } + + return $buttons; + } } diff --git a/src/User/Notifications/ActivityMentionNotification.php b/src/User/Notifications/ActivityMentionNotification.php index ffc72ad7bd8..6e596b960a9 100644 --- a/src/User/Notifications/ActivityMentionNotification.php +++ b/src/User/Notifications/ActivityMentionNotification.php @@ -7,6 +7,7 @@ use CraftCms\Cms\Activity\ActivityComments; use CraftCms\Cms\Activity\Models\ActivityEvent; use CraftCms\Cms\Cms; +use CraftCms\Cms\Cp\Notifications\CpNotification; use CraftCms\Cms\Element\Contracts\ElementInterface; use CraftCms\Cms\Support\Facades\Elements; use CraftCms\Cms\Support\Template; @@ -18,24 +19,34 @@ use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueueAfterCommit; use Illuminate\Notifications\Channels\MailChannel; -use Illuminate\Notifications\Notification; use LogicException; -class ActivityMentionNotification extends Notification implements ShouldQueueAfterCommit +class ActivityMentionNotification extends CpNotification implements ShouldQueueAfterCommit { use Queueable; public int $tries = 3; - public function __construct(public string $versionEventId) + public function __construct(public ActivityEvent $event) { + parent::__construct( + static fn (CraftUser $notifiable): string => app(ActivityComments::class) + ->notificationText($event, $notifiable->asElement()), + ); + $this->queue = Cms::config()->queueName; + $this + ->title('comment_mention_subject') + ->byline($event->data['author']['label']) + ->icon('comment') + ->url($this->subject()?->getCpEditUrl()); } /** @return class-string[] */ - public function via(mixed $notifiable): array + #[\Override] + public function via(CraftUser $notifiable): array { - return [MailChannel::class]; + return [...parent::via($notifiable), MailChannel::class]; } public function shouldSend(CraftUser $notifiable, string $channel): bool @@ -53,8 +64,7 @@ public function shouldSend(CraftUser $notifiable, string $channel): bool public function toMail(CraftUser $notifiable): SystemMessageMailable { - $version = $this->version(); - $subject = $this->subject($version); + $subject = $this->subject(); $editUrl = $subject?->getCpEditUrl(); if ($subject === null || $editUrl === null) { @@ -66,34 +76,27 @@ public function toMail(CraftUser $notifiable): SystemMessageMailable key: 'comment_mention', user: $recipient, variables: [ - 'author' => $version->data['author']['label'], - 'subject' => $version->snapshots['subject']['label'], - 'comment' => app(ActivityComments::class)->notificationText($version, $recipient), + 'author' => $this->event->data['author']['label'], + 'subject' => $this->event->snapshots['subject']['label'], + 'comment' => app(ActivityComments::class)->notificationText($this->event, $recipient), 'link' => Template::raw(Url::cpUrl($editUrl)), ], ); - $mailable->siteId = $version->siteId; + $mailable->siteId = $this->event->siteId; return $mailable; } - private function version(): ActivityEvent + private function subject(): ?ElementInterface { - return ActivityEvent::query()->findOrFail($this->versionEventId); - } - - private function subject(?ActivityEvent $version = null): ?ElementInterface - { - $version ??= $this->version(); - - if ($version->subjectId === null) { + if ($this->event->subjectId === null) { return null; } return Elements::getElementByUid( - $version->subjectId, - $version->subjectType, - $version->siteId, + $this->event->subjectId, + $this->event->subjectType, + $this->event->siteId, ); } } diff --git a/tests/Feature/Activity/ActivityCommentsTest.php b/tests/Feature/Activity/ActivityCommentsTest.php index f49cbc8c3d1..5229fd47e33 100644 --- a/tests/Feature/Activity/ActivityCommentsTest.php +++ b/tests/Feature/Activity/ActivityCommentsTest.php @@ -6,6 +6,7 @@ use CraftCms\Cms\Activity\EventTypes\CommentCreated; use CraftCms\Cms\Activity\EventTypes\CommentDeleted; use CraftCms\Cms\Activity\EventTypes\CommentEdited; +use CraftCms\Cms\Cp\Notifications\CpNotification; use CraftCms\Cms\Database\Table; use CraftCms\Cms\Edition; use CraftCms\Cms\Entry\Models\Entry; @@ -15,6 +16,8 @@ use CraftCms\Cms\User\Elements\User; use CraftCms\Cms\User\Models\User as UserModel; use CraftCms\Cms\User\Notifications\ActivityMentionNotification; +use Illuminate\Notifications\Channels\DatabaseChannel; +use Illuminate\Notifications\Channels\MailChannel; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Notification; use Illuminate\Validation\ValidationException; @@ -44,12 +47,13 @@ }); it('records immutable comment lifecycle versions', function () { - $created = $this->comments->create($this->entry, $this->author, $this->site, 'First version'); + $created = $this->comments->create($this->entry, $this->author, null, 'First version'); $edited = $this->comments->edit($created, $this->author, 'Second version', $this->entry); $deleted = $this->comments->delete($created, $this->author); expect($created->eventType)->toBe(CommentCreated::class) ->and($created->rootEventId)->toBeNull() + ->and($created->siteId)->toBeNull() ->and($edited->eventType)->toBe(CommentEdited::class) ->and($edited->rootEventId)->toBe($created->id) ->and($deleted->eventType)->toBe(CommentDeleted::class) @@ -60,7 +64,7 @@ ->toThrow(ValidationException::class); }); -it('stores, validates, and renders structured mentions', function () { +it('stores and renders eligible mentions and ignores invalid mentions', function () { $comment = $this->comments->create( $this->entry, $this->author, @@ -91,12 +95,19 @@ ->withPermissions(['accessCp']) ->createElement(['admin' => false, 'username' => 'ineligible']); - expect(fn () => $this->comments->create( + $ignored = $this->comments->create( $this->entry, $this->author, $this->site, - "Hello [@ineligible](craft-user:{$ineligible->id}).", - ))->toThrow(ValidationException::class) + "Hello [@ineligible](craft-user:{$ineligible->id}) and [@invalid](craft-user:not-a-number).", + ); + $document->loadHTML( + $this->comments->render($ignored, $this->author)->toHtml(), + LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD, + ); + + expect($ignored->data['mentions'])->toBe([]) + ->and($document->textContent)->toBe('Hello @ineligible and @invalid.') ->and(fn () => $this->comments->create( $this->entry, $this->author, @@ -114,7 +125,7 @@ ); $notifiable = UserModel::query()->findOrFail($this->mentioned->id); - expect(new ActivityMentionNotification($comment->id)->toMail($notifiable)->variables['comment']) + expect(new ActivityMentionNotification($comment)->toMail($notifiable)->variables['comment']) ->toBe('Hello A & B, @grace.'); })->with([ 'plain destination' => fn (int $id): string => "[@grace](craft-user:$id)", @@ -129,10 +140,19 @@ $this->site, "Hello [@grace](craft-user:{$this->mentioned->id}).", ); - $notification = new ActivityMentionNotification($comment->id); + $notification = unserialize(serialize(new ActivityMentionNotification($comment))); $notifiable = UserModel::query()->findOrFail($this->mentioned->id); - expect($notification->shouldSend($notifiable, 'mail'))->toBeTrue(); + expect($notification)->toBeInstanceOf(CpNotification::class) + ->and($notification->via($notifiable))->toBe([DatabaseChannel::class, MailChannel::class]) + ->and($notification->toDatabase($notifiable))->toMatchArray([ + 'title' => 'comment_mention_subject', + 'message' => 'Hello @grace.', + 'byline' => $this->author->name, + 'icon' => 'comment', + 'url' => $this->entry->getCpEditUrl(), + ]) + ->and($notification->shouldSend($notifiable, 'mail'))->toBeTrue(); UserPermissions::saveUserPermissions( $this->mentioned->id, diff --git a/tests/Feature/Cp/Notifications/NotificationCenterTest.php b/tests/Feature/Cp/Notifications/NotificationCenterTest.php index f3ce425e0f4..54568df9e69 100644 --- a/tests/Feature/Cp/Notifications/NotificationCenterTest.php +++ b/tests/Feature/Cp/Notifications/NotificationCenterTest.php @@ -71,6 +71,20 @@ expect(app(NotificationCenter::class)->get())->toBe([]); }); +it('serializes closure values', function () { + $user = User::query()->firstOrFail(); + $notification = new CpNotification( + static fn (CraftUser $notifiable): string => "Hello {$notifiable->asElement()->username}", + )->title(static fn (CraftUser $notifiable): string => "For {$notifiable->asElement()->email}"); + + $notification = unserialize(serialize($notification)); + + expect($notification->toDatabase($user))->toMatchArray([ + 'message' => "Hello {$user->username}", + 'title' => "For {$user->email}", + ]); +}); + class ConfiguredNotificationUser extends User { #[Override]