Merge pull request #2237 from nextcloud/rich-strings-and-icons-for-mention-notifications

Rich strings and icons for mention notifications

apps/comments/lib/Notification/Notifier.php

@@ -24,6 +24,7 @@ namespace OCA\Comments\Notification;
 use OCP\Comments\ICommentsManager;
 use OCP\Comments\NotFoundException;
 use OCP\Files\Folder;
+use OCP\IURLGenerator;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
 use OCP\Notification\INotification;
@@ -40,18 +41,23 @@ class Notifier implements INotifier {
 	/** @var ICommentsManager  */
 	protected $commentsManager;
 
-	/** @var IUserManager  */
+	/** @var IURLGenerator */
+	protected $url;
+
+	/** @var IUserManager */
 	protected $userManager;
 
 	public function __construct(
 		IFactory $l10nFactory,
 		Folder $userFolder,
 		ICommentsManager $commentsManager,
+		IURLGenerator $url,
 		IUserManager $userManager
 	) {
 		$this->l10nFactory = $l10nFactory;
 		$this->userFolder = $userFolder;
 		$this->commentsManager = $commentsManager;
+		$this->url = $url;
 		$this->userManager = $userManager;
 	}
 
@@ -63,7 +69,7 @@ class Notifier implements INotifier {
 	 */
 	public function prepare(INotification $notification, $languageCode) {
 		if($notification->getApp() !== 'comments') {
-			throw  new \InvalidArgumentException();
+			throw new \InvalidArgumentException();
 		}
 		try {
 			$comment = $this->commentsManager->get($notification->getObjectId());
@@ -80,6 +86,7 @@ class Notifier implements INotifier {
 				$displayName = $commenter->getDisplayName();
 			}
 		}
+
 		switch($notification->getSubject()) {
 			case 'mention':
 				$parameters = $notification->getSubjectParameters();
@@ -90,19 +97,49 @@ class Notifier implements INotifier {
 				if(empty($nodes)) {
 					throw new \InvalidArgumentException('Cannot resolve file id to Node instance');
 				}
-				$fileName = $nodes[0]->getName();
-				if($isDeletedActor) {
-					$subject = (string) $l->t(
-						'A (now) deleted user mentioned you in a comment on "%s".',
-						[ $fileName ]
-					);
+				$node = $nodes[0];
+
+				if ($isDeletedActor) {
+					$notification->setParsedSubject($l->t(
+							'A (now) deleted user mentioned you in a comment on “%s”',
+							[$node->getName()]
+						))
+						->setRichSubject(
+							$l->t('A (now) deleted user mentioned you in a comment on “{file}”'),
+							[
+								'file' => [
+									'type' => 'file',
+									'id' => $comment->getObjectId(),
+									'name' => $node->getName(),
+									'path' => $node->getPath(),
+									'link' => $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $comment->getObjectId()]),
+								],
+							]
+						);
 				} else {
-					$subject = (string) $l->t(
-						'%s mentioned you in a comment on "%s".',
-						[ $displayName, $fileName ]
-					);
+					$notification->setParsedSubject($l->t(
+							'%1$s mentioned you in a comment on “%2$s”',
+							[$displayName, $node->getName()]
+						))
+						->setRichSubject(
+							$l->t('{user} mentioned you in a comment on “{file}”'),
+							[
+								'user' => [
+									'type' => 'user',
+									'id' => $comment->getActorId(),
+									'name' => $displayName,
+								],
+								'file' => [
+									'type' => 'file',
+									'id' => $comment->getObjectId(),
+									'name' => $node->getName(),
+									'path' => $node->getPath(),
+									'link' => $this->url->linkToRouteAbsolute('files.viewcontroller.showFile', ['fileid' => $comment->getObjectId()]),
+								],
+							]
+						);
 				}
-				$notification->setParsedSubject($subject);
+				$notification->setIcon($this->url->getAbsoluteURL($this->url->imagePath('core', 'actions/comment.svg')));
 
 				return $notification;
 				break;

apps/comments/tests/Unit/Notification/NotifierTest.php

@@ -30,6 +30,7 @@ use OCP\Comments\NotFoundException;
 use OCP\Files\Folder;
 use OCP\Files\Node;
 use OCP\IL10N;
+use OCP\IURLGenerator;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\L10N\IFactory;
@@ -49,7 +50,8 @@ class NotifierTest extends TestCase {
 
 	/** @var  ICommentsManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $commentsManager;
-
+	/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
+	protected $url;
 	/** @var  IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $userManager;
 
@@ -72,16 +74,24 @@ class NotifierTest extends TestCase {
 		$this->l10nFactory = $this->getMockBuilder('OCP\L10N\IFactory')->getMock();
 		$this->folder = $this->getMockBuilder('OCP\Files\Folder')->getMock();
 		$this->commentsManager = $this->getMockBuilder('OCP\Comments\ICommentsManager')->getMock();
+		$this->url = $this->createMock(IURLGenerator::class);
 		$this->userManager = $this->getMockBuilder('OCP\IUserManager')->getMock();
 
 		$this->notifier = new Notifier(
 			$this->l10nFactory,
 			$this->folder,
 			$this->commentsManager,
+			$this->url,
 			$this->userManager
 		);
 
-		$this->l = $this->getMockBuilder('OCP\IL10N')->getMock();
+		$this->l = $this->createMock(IL10N::class);
+		$this->l->expects($this->any())
+			->method('t')
+			->will($this->returnCallback(function ($text, $parameters = []) {
+				return vsprintf($text, $parameters);
+			}));
+
 		$this->notification = $this->getMockBuilder('OCP\Notification\INotification')->getMock();
 		$this->comment = $this->getMockBuilder('OCP\Comments\IComment')->getMock();
 	}
@@ -89,7 +99,7 @@ class NotifierTest extends TestCase {
 	public function testPrepareSuccess() {
 		$fileName = 'Gre\'thor.odp';
 		$displayName = 'Huraga';
-		$message = 'Hurage mentioned you in a comment on "Gre\'thor.odp".';
+		$message = 'Huraga mentioned you in a comment on “Gre\'thor.odp”';
 
 		/** @var IUser|\PHPUnit_Framework_MockObject_MockObject $user */
 		$user = $this->getMockBuilder('OCP\IUser')->getMock();
@@ -100,7 +110,7 @@ class NotifierTest extends TestCase {
 		/** @var Node|\PHPUnit_Framework_MockObject_MockObject */
 		$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
 		$node
-			->expects($this->once())
+			->expects($this->atLeastOnce())
 			->method('getName')
 			->willReturn($fileName);
 
@@ -125,13 +135,27 @@ class NotifierTest extends TestCase {
 		$this->notification
 			->expects($this->once())
 			->method('setParsedSubject')
-			->with($message);
-
-		$this->l
+			->with($message)
+			->willReturnSelf();
+		$this->notification
 			->expects($this->once())
-			->method('t')
-			->with('%s mentioned you in a comment on "%s".', [$displayName, $fileName])
-			->willReturn($message);
+			->method('setRichSubject')
+			->with('{user} mentioned you in a comment on “{file}”', $this->anything())
+			->willReturnSelf();
+		$this->notification
+			->expects($this->once())
+			->method('setIcon')
+			->with('absolute-image-path')
+			->willReturnSelf();
+
+		$this->url->expects($this->once())
+			->method('imagePath')
+			->with('core', 'actions/comment.svg')
+			->willReturn('image-path');
+		$this->url->expects($this->once())
+			->method('getAbsoluteURL')
+			->with('image-path')
+			->willReturn('absolute-image-path');
 
 		$this->l10nFactory
 			->expects($this->once())
@@ -163,12 +187,12 @@ class NotifierTest extends TestCase {
 
 	public function testPrepareSuccessDeletedUser() {
 		$fileName = 'Gre\'thor.odp';
-		$message = 'A (now) deleted user mentioned you in a comment on "Gre\'thor.odp".';
+		$message = 'A (now) deleted user mentioned you in a comment on “Gre\'thor.odp”';
 
 		/** @var Node|\PHPUnit_Framework_MockObject_MockObject */
 		$node = $this->getMockBuilder('OCP\Files\Node')->getMock();
 		$node
-			->expects($this->once())
+			->expects($this->atLeastOnce())
 			->method('getName')
 			->willReturn($fileName);
 
@@ -193,13 +217,27 @@ class NotifierTest extends TestCase {
 		$this->notification
 			->expects($this->once())
 			->method('setParsedSubject')
-			->with($message);
-
-		$this->l
+			->with($message)
+			->willReturnSelf();
+		$this->notification
 			->expects($this->once())
-			->method('t')
-			->with('A (now) deleted user mentioned you in a comment on "%s".', [ $fileName ])
-			->willReturn($message);
+			->method('setRichSubject')
+			->with('A (now) deleted user mentioned you in a comment on “{file}”', $this->anything())
+			->willReturnSelf();
+		$this->notification
+			->expects($this->once())
+			->method('setIcon')
+			->with('absolute-image-path')
+			->willReturnSelf();
+
+		$this->url->expects($this->once())
+			->method('imagePath')
+			->with('core', 'actions/comment.svg')
+			->willReturn('image-path');
+		$this->url->expects($this->once())
+			->method('getAbsoluteURL')
+			->with('image-path')
+			->willReturn('absolute-image-path');
 
 		$this->l10nFactory
 			->expects($this->once())

core/img/actions/comment.svg

@@ -1 +1 @@
-<svg xmlns="http://www.w3.org/2000/svg" height="16" width="16"><path d="M8 1.5C3.582 1.5 0 3.962 0 7s3.582 5.5 8 5.5c.25 0 .49-.016.734-.03L13 16v-4.703c1.83-1.008 3-2.56 3-4.297 0-3.038-3.582-5.5-8-5.5z"/></svg>
+<svg xmlns="http://www.w3.org/2000/svg" height="16" width="16" viewbox="0 0 16 16"><path d="M8 1.5C3.582 1.5 0 3.962 0 7s3.582 5.5 8 5.5c.25 0 .49-.016.734-.03L13 16v-4.703c1.83-1.008 3-2.56 3-4.297 0-3.038-3.582-5.5-8-5.5z"/></svg>