Merge pull request #34807 from nextcloud/fix/compute-notification-parsed-subject

Compute notification parsed subject from rich subject when possible

3 files changed, 12 insertions, 45 deletions

diff --git a/apps/comments/lib/Activity/Provider.php b/apps/comments/lib/Activity/Provider.php
index c6e55326580..f9a5971c7f3 100644
--- a/apps/comments/lib/Activity/Provider.php
+++ b/apps/comments/lib/Activity/Provider.php
@@ -31,12 +31,10 @@ use OCP\Comments\ICommentsManager;
 use OCP\Comments\NotFoundException;
 use OCP\IL10N;
 use OCP\IURLGenerator;
-use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 
 class Provider implements IProvider {
-
 	protected IFactory $languageFactory;
 	protected ?IL10N $l = null;
 	protected IUrlGenerator $url;
@@ -97,14 +95,12 @@ class Provider implements IProvider {
 
 		if ($event->getSubject() === 'add_comment_subject') {
 			if ($subjectParameters['actor'] === $this->activityManager->getCurrentUserId()) {
-				$event->setParsedSubject($this->l->t('You commented'))
-					->setRichSubject($this->l->t('You commented'), []);
+				$event->setRichSubject($this->l->t('You commented'), []);
 			} else {
 				$author = $this->generateUserParameter($subjectParameters['actor']);
-				$event->setParsedSubject($this->l->t('%1$s commented', [$author['name']]))
-					->setRichSubject($this->l->t('{author} commented'), [
-						'author' => $author,
-					]);
+				$event->setRichSubject($this->l->t('{author} commented'), [
+					'author' => $author,
+				]);
 			}
 		} else {
 			throw new \InvalidArgumentException();
diff --git a/apps/comments/lib/Notification/Notifier.php b/apps/comments/lib/Notification/Notifier.php
index 4ddb7295bfe..59f1ad5da5f 100644
--- a/apps/comments/lib/Notification/Notifier.php
+++ b/apps/comments/lib/Notification/Notifier.php
@@ -29,7 +29,6 @@ use OCP\Comments\ICommentsManager;
 use OCP\Comments\NotFoundException;
 use OCP\Files\IRootFolder;
 use OCP\IURLGenerator;
-use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Notification\AlreadyProcessedException;
@@ -37,7 +36,6 @@ use OCP\Notification\INotification;
 use OCP\Notification\INotifier;
 
 class Notifier implements INotifier {
-
 	protected IFactory $l10nFactory;
 	protected IRootFolder $rootFolder;
 	protected ICommentsManager $commentsManager;
@@ -147,9 +145,7 @@ class Notifier implements INotifier {
 				}
 				[$message, $messageParameters] = $this->commentToRichMessage($comment);
 				$notification->setRichSubject($subject, $subjectParameters)
-					->setParsedSubject($this->richToParsed($subject, $subjectParameters))
 					->setRichMessage($message, $messageParameters)
-					->setParsedMessage($this->richToParsed($message, $messageParameters))
 					->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/comment.svg')))
 					->setLink($this->url->linkToRouteAbsolute(
 						'comments.Notifications.view',
@@ -205,19 +201,4 @@ class Notifier implements INotifier {
 		}
 		return [$message, $messageParameters];
 	}
-
-	public function richToParsed(string $message, array $parameters): string {
-		$placeholders = $replacements = [];
-		foreach ($parameters as $placeholder => $parameter) {
-			$placeholders[] = '{' . $placeholder . '}';
-			if ($parameter['type'] === 'user') {
-				$replacements[] = '@' . $parameter['name'];
-			} elseif ($parameter['type'] === 'file') {
-				$replacements[] = $parameter['path'];
-			} else {
-				$replacements[] = $parameter['name'];
-			}
-		}
-		return str_replace($placeholders, $replacements, $message);
-	}
 }
diff --git a/apps/comments/tests/Unit/Notification/NotifierTest.php b/apps/comments/tests/Unit/Notification/NotifierTest.php
index ecd22ffd9e3..9281bb99fe8 100644
--- a/apps/comments/tests/Unit/Notification/NotifierTest.php
+++ b/apps/comments/tests/Unit/Notification/NotifierTest.php
@@ -35,7 +35,6 @@ use OCP\Files\IRootFolder;
 use OCP\Files\Node;
 use OCP\IL10N;
 use OCP\IURLGenerator;
-use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Notification\INotification;
@@ -43,7 +42,6 @@ use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class NotifierTest extends TestCase {
-
 	/** @var Notifier */
 	protected $notifier;
 	/** @var IFactory|MockObject */
@@ -135,10 +133,8 @@ class NotifierTest extends TestCase {
 			->method('getSubjectParameters')
 			->willReturn(['files', '678']);
 		$this->notification
-			->expects($this->once())
-			->method('setParsedSubject')
-			->with($message)
-			->willReturnSelf();
+			->expects($this->never())
+			->method('setParsedSubject');
 		$this->notification
 			->expects($this->once())
 			->method('setRichSubject')
@@ -150,10 +146,8 @@ class NotifierTest extends TestCase {
 			->with('Hi {mention-user1}!', ['mention-user1' => ['type' => 'user', 'id' => 'you', 'name' => 'Your name']])
 			->willReturnSelf();
 		$this->notification
-			->expects($this->once())
-			->method('setParsedMessage')
-			->with('Hi @Your name!')
-			->willReturnSelf();
+			->expects($this->never())
+			->method('setParsedMessage');
 		$this->notification
 			->expects($this->once())
 			->method('setIcon')
@@ -256,10 +250,8 @@ class NotifierTest extends TestCase {
 			->method('getSubjectParameters')
 			->willReturn(['files', '678']);
 		$this->notification
-			->expects($this->once())
-			->method('setParsedSubject')
-			->with($message)
-			->willReturnSelf();
+			->expects($this->never())
+			->method('setParsedSubject');
 		$this->notification
 			->expects($this->once())
 			->method('setRichSubject')
@@ -271,10 +263,8 @@ class NotifierTest extends TestCase {
 			->with('Hi {mention-user1}!', ['mention-user1' => ['type' => 'user', 'id' => 'you', 'name' => 'Your name']])
 			->willReturnSelf();
 		$this->notification
-			->expects($this->once())
-			->method('setParsedMessage')
-			->with('Hi @Your name!')
-			->willReturnSelf();
+			->expects($this->never())
+			->method('setParsedMessage');
 		$this->notification
 			->expects($this->once())
 			->method('setIcon')