Add the message to the mention notifications

The chat message can be much longer than the maximum length allowed for notification messages, so the notification message is trimmed if needed. When the notification is being prepared to be displayed it is taken into account if the message was trimmed, and in that case an ellipsis is added at the start and/or the end (depending on the case) of the message. Note that the original notification message does not contain the ellipsis characters, as depending on the language a whitespace should be added or not before and after an ellipsis, and the language in which the notification will be displayed is only known at the time of displaying it, but not when it is saved. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
2017-11-02 02:20:01 +01:00 · 2017-11-02 02:20:01 +01:00 · 1e0bcabdeb
--- a/lib/Chat/Notifier.php
+++ b/lib/Chat/Notifier.php
@ -134,9 +134,61 @@ class Notifier {
 				])
 			->setDateTime($comment->getCreationDateTime());

+		$notificationMessage = $this->getNotificationMessage($comment, $mentionedUserId);
+		if (count($notificationMessage) === 1) {
+			$notification->setMessage($notificationMessage[0]);
+		} else {
+			$notification->setMessage($notificationMessage[0], $notificationMessage[1]);
+		}
+
 		return $notification;
 	}

+	/**
+	 * Returns the message for a notification from the message of the comment.
+	 *
+	 * The message is returned as an array; the first element is the message
+	 * itself, and the second element is another array that contains the
+	 * parameters for the message. If no parameters are needed then the returned
+	 * array has a single element.
+	 *
+	 * The message of a comment can be much longer than the maximum allowed
+	 * length for the message of a notification so, if needed, the comment
+	 * message is trimmed around the first mention to the user. In that case
+	 * the "ellipsisStart" and/or "ellipsisEnd" (depending on the case) are
+	 * returned as the parameters.
+	 *
+	 * @param IComment $comment
+	 * @param string $mentionedUserId
+	 * @return array the first element is a message suitable to be stored in a
+	 *         notification, and the second are the parameters, if any.
+	 */
+	private function getNotificationMessage(IComment $comment, $mentionedUserId) {
+		$maximumLength = 64;
+
+		$message = $comment->getMessage();
+
+		$messageLength = strlen($message);
+		if ($messageLength <= $maximumLength) {
+			return [$message];
+		}
+
+		$mention = '@' . $mentionedUserId;
+		$mentionLength = strlen($mention);
+		// Only the first mention is taken into account
+		$mentionMiddleIndex = strpos($message, $mention) + $mentionLength / 2;
+
+		if ($mentionMiddleIndex <= $maximumLength / 2) {
+			return [substr($message, 0, $maximumLength), ['ellipsisEnd']];
+		}
+
+		if ($mentionMiddleIndex >= ($messageLength - $maximumLength / 2)) {
+			return [substr($message, -$maximumLength), ['ellipsisStart']];
+		}
+
+		return [substr($message, $mentionMiddleIndex - ($maximumLength / 2), $maximumLength), ['ellipsisStart', 'ellipsisEnd']];
+	}
+
 	private function shouldUserBeNotified($userId, IComment $comment) {
 		if ($userId === $comment->getActorId()) {
 			// Do not notify the user if she mentioned herself
--- a/lib/Notification/Notifier.php
+++ b/lib/Notification/Notifier.php
@ -147,6 +147,18 @@ class Notifier implements INotifier {
 			];
 		}

+		$messageParameters = $notification->getMessageParameters();
+
+		$parsedMessage = $notification->getMessage();
+		if (in_array('ellipsisStart', $messageParameters) && !in_array('ellipsisEnd', $messageParameters)) {
+			$parsedMessage = $l->t('… %s', $parsedMessage);
+		} else if (!in_array('ellipsisStart', $messageParameters) && in_array('ellipsisEnd', $messageParameters)) {
+			$parsedMessage = $l->t('%s …', $parsedMessage);
+		} else if (in_array('ellipsisStart', $messageParameters) && in_array('ellipsisEnd', $messageParameters)) {
+			$parsedMessage = $l->t('… %s …', $parsedMessage);
+		}
+		$notification->setParsedMessage($parsedMessage);
+
 		if ($room->getType() === Room::ONE_TO_ONE_CALL) {
 			$notification
 				->setParsedSubject(
--- a/tests/php/Chat/NotifierTest.php
+++ b/tests/php/Chat/NotifierTest.php
@ -125,6 +125,11 @@ class NotifierTest extends \Test\TestCase {
 			->with('anotherUser')
 			->willReturnSelf();

+		$notification->expects($this->once())
+			->method('setMessage')
+			->with($comment->getMessage())
+			->willReturnSelf();
+
 		$this->notificationManager->expects($this->once())
 			->method('notify')
 			->with($notification);
@ -146,6 +151,92 @@ class NotifierTest extends \Test\TestCase {
 			->with('anotherUser')
 			->willReturnSelf();

+		$notification->expects($this->once())
+			->method('setMessage')
+			->with($comment->getMessage())
+			->willReturnSelf();
+
+		$this->notificationManager->expects($this->once())
+			->method('notify')
+			->with($notification);
+
+		$this->notifier->notifyMentionedUsers($comment);
+	}
+
+	public function testNotifyMentionedUsersWithLongMessageStartMention() {
+		$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016),
+			'123456789 @anotherUserWithOddLengthName 123456789-123456789-123456789-123456789-123456789-123456789');
+
+		$notification = $this->newNotification($comment);
+
+		$this->notificationManager->expects($this->once())
+			->method('createNotification')
+			->willReturn($notification);
+
+		$notification->expects($this->once())
+			->method('setUser')
+			->with('anotherUserWithOddLengthName')
+			->willReturnSelf();
+
+		$notification->expects($this->once())
+			->method('setMessage')
+			->with('123456789 @anotherUserWithOddLengthName 123456789-123456789-1234', ['ellipsisEnd'])
+			->willReturnSelf();
+
+		$this->notificationManager->expects($this->once())
+			->method('notify')
+			->with($notification);
+
+		$this->notifier->notifyMentionedUsers($comment);
+	}
+
+	public function testNotifyMentionedUsersWithLongMessageMiddleMention() {
+		$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016),
+			'123456789-123456789-123456789-1234 @anotherUserWithOddLengthName 6789-123456789-123456789-123456789');
+
+		$notification = $this->newNotification($comment);
+
+		$this->notificationManager->expects($this->once())
+			->method('createNotification')
+			->willReturn($notification);
+
+		$notification->expects($this->once())
+			->method('setUser')
+			->with('anotherUserWithOddLengthName')
+			->willReturnSelf();
+
+		$notification->expects($this->once())
+			->method('setMessage')
+			->with('89-123456789-1234 @anotherUserWithOddLengthName 6789-123456789-1', ['ellipsisStart', 'ellipsisEnd'])
+			->willReturnSelf();
+
+		$this->notificationManager->expects($this->once())
+			->method('notify')
+			->with($notification);
+
+		$this->notifier->notifyMentionedUsers($comment);
+	}
+
+	public function testNotifyMentionedUsersWithLongMessageEndMention() {
+		$comment = $this->newComment(108, 'users', 'testUser', new \DateTime('@' . 1000000016),
+			'123456789-123456789-123456789-123456789-123456789-123456789 @anotherUserWithOddLengthName 123456789');
+
+		$notification = $this->newNotification($comment);
+
+		$this->notificationManager->expects($this->once())
+			->method('createNotification')
+			->willReturn($notification);
+
+		$notification->expects($this->once())
+			->method('setUser')
+			->with('anotherUserWithOddLengthName')
+			->willReturnSelf();
+
+		$notification->expects($this->once())
+			->method('setMessage')
+			->with('6789-123456789-123456789 @anotherUserWithOddLengthName 123456789', ['ellipsisStart'])
+			->willReturnSelf();
+
 		$this->notificationManager->expects($this->once())
 			->method('notify')
 			->with($notification);
@ -207,11 +298,21 @@ class NotifierTest extends \Test\TestCase {
 			->with('anotherUser')
 			->willReturnSelf();

+		$anotherUserNotification->expects($this->once())
+			->method('setMessage')
+			->with('Mention @anotherUser, and @unknownUser, and @testUser, and @user')
+			->willReturnSelf();
+
 		$userAbleToJoinNotification->expects($this->once())
 			->method('setUser')
 			->with('userAbleToJoin')
 			->willReturnSelf();

+		$userAbleToJoinNotification->expects($this->once())
+			->method('setMessage')
+			->with('notherUser, and @unknownUser, and @testUser, and @userAbleToJoin')
+			->willReturnSelf();
+
 		$this->notificationManager->expects($this->exactly(2))
 			->method('notify')
 			->withConsecutive(
--- a/tests/php/Notification/NotifierTest.php
+++ b/tests/php/Notification/NotifierTest.php
@ -262,97 +262,104 @@ class NotifierTest extends \Test\TestCase {
 	public function dataPrepareMention() {
 		return [
 			[
-				Room::ONE_TO_ONE_CALL, ['userType' => 'users', 'userId' => 'testUser'],           'Test user', '',
+				Room::ONE_TO_ONE_CALL, ['userType' => 'users', 'userId' => 'testUser'],           ['ellipsisStart', 'ellipsisEnd'], 'Test user', '',
 				'Test user mentioned you in a private chat',
 				['{user} mentioned you in a private chat',
 					['user' => ['type' => 'user', 'id' => 'testUser', 'name' => 'Test user']]
-				]
+				],
+				'… message …'
 			],
 			// If the user is deleted in a one to one chat the chat is also
 			// deleted, and that in turn would delete the pending notification.
 			[
-				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           'Test user', '',
+				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           [], 'Test user', '',
 				'Test user mentioned you in a group chat',
 				['{user} mentioned you in a group chat',
 					['user' => ['type' => 'user', 'id' => 'testUser', 'name' => 'Test user']]
-				]
+				],
+				'message'
 			],
 			[
-				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           null,        '',
+				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           ['ellipsisStart'], null,        '',
 				'A (now) deleted user mentioned you in a group chat',
 				['A (now) deleted user mentioned you in a group chat',
 					[]
 				],
-				true
-			],
+				'… message',
+				true],
 			[
-				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           'Test user', 'Room name',
+				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           ['ellipsisEnd'], 'Test user', 'Room name',
 				'Test user mentioned you in a group chat: Room name',
 				['{user} mentioned you in a group chat: {call}',
 					[
 						'user' => ['type' => 'user', 'id' => 'testUser', 'name' => 'Test user'],
 						'call' => ['type' => 'call', 'id' => 'testRoomId', 'name' => 'Room name', 'call-type' => 'group']
 					]
-				]
+				],
+				'message …'
 			],
 			[
-				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           null,        'Room name',
+				Room::GROUP_CALL,      ['userType' => 'users', 'userId' => 'testUser'],           ['ellipsisStart', 'ellipsisEnd'], null,        'Room name',
 				'A (now) deleted user mentioned you in a group chat: Room name',
 				['A (now) deleted user mentioned you in a group chat: {call}',
 					[
 						'call' => ['type' => 'call', 'id' => 'testRoomId', 'name' => 'Room name', 'call-type' => 'group']
 					]
 				],
-				true
-			],
+				'… message …',
+				true],
 			[
-				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           'Test user', '',
+				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           [], 'Test user', '',
 				'Test user mentioned you in a group chat',
 				['{user} mentioned you in a group chat',
 					['user' => ['type' => 'user', 'id' => 'testUser', 'name' => 'Test user']]
-				]
+				],
+				'message'
 			],
 			[
-				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           null,        '',
+				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           ['ellipsisStart'], null,        '',
 				'A (now) deleted user mentioned you in a group chat',
 				['A (now) deleted user mentioned you in a group chat',
 					[]
 				],
-				true
-			],
+				'… message',
+				true],
 			[
-				Room::PUBLIC_CALL,     ['userType' => 'guests', 'userId' => 'testSpreedSession'], null,        '',
+				Room::PUBLIC_CALL,     ['userType' => 'guests', 'userId' => 'testSpreedSession'], ['ellipsisEnd'], null,        '',
 				'A guest mentioned you in a group chat',
 				['A guest mentioned you in a group chat',
 					[]
-				]
+				],
+				'message …'
 			],
 			[
-				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           'Test user', 'Room name',
+				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           ['ellipsisStart', 'ellipsisEnd'], 'Test user', 'Room name',
 				'Test user mentioned you in a group chat: Room name',
 				['{user} mentioned you in a group chat: {call}',
 					[
 						'user' => ['type' => 'user', 'id' => 'testUser', 'name' => 'Test user'],
 						'call' => ['type' => 'call', 'id' => 'testRoomId', 'name' => 'Room name', 'call-type' => 'public']
 					]
-				]
+				],
+				'… message …'
 			],
 			[
-				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           null,    'Room name',
+				Room::PUBLIC_CALL,     ['userType' => 'users', 'userId' => 'testUser'],           [], null,    'Room name',
 				'A (now) deleted user mentioned you in a group chat: Room name',
 				['A (now) deleted user mentioned you in a group chat: {call}',
 					[
 						'call' => ['type' => 'call', 'id' => 'testRoomId', 'name' => 'Room name', 'call-type' => 'public']
 					]
 				],
-				true
-			],
+				'message',
+				true],
 			[
-				Room::PUBLIC_CALL,     ['userType' => 'guests', 'userId' => 'testSpreedSession'], null,    'Room name',
+				Room::PUBLIC_CALL,     ['userType' => 'guests', 'userId' => 'testSpreedSession'], ['ellipsisStart', 'ellipsisEnd'], null,    'Room name',
 				'A guest mentioned you in a group chat: Room name',
 				['A guest mentioned you in a group chat: {call}',
 					['call' => ['type' => 'call', 'id' => 'testRoomId', 'name' => 'Room name', 'call-type' => 'public']]
-				]
+				],
+				'… message …'
 			]
 		];
 	}
@ -361,16 +368,18 @@ class NotifierTest extends \Test\TestCase {
 	 * @dataProvider dataPrepareMention
 	 * @param int $roomType
 	 * @param array $subjectParameters
+	 * @param array $messageParameters
 	 * @param string $displayName
 	 * @param string $roomName
 	 * @param string $parsedSubject
 	 * @param array $richSubject
+	 * @param string $parsedMessage
 	 * @param bool $deletedUser
 	 */
-	public function testPrepareMention($roomType, $subjectParameters, $displayName, $roomName, $parsedSubject, $richSubject, $deletedUser = false) {
+	public function testPrepareMention($roomType, $subjectParameters, $messageParameters, $displayName, $roomName, $parsedSubject, $richSubject, $parsedMessage, $deletedUser = false) {
 		$notification = $this->createMock(INotification::class);
 		$l = $this->createMock(IL10N::class);
-		$l->expects($this->exactly(2))
+		$l->expects($this->atLeast(2))
 			->method('t')
 			->will($this->returnCallback(function($text, $parameters = []) {
 				return vsprintf($text, $parameters);
@ -434,6 +443,10 @@ class NotifierTest extends \Test\TestCase {
 			->method('setRichSubject')
 			->with($richSubject[0], $richSubject[1])
 			->willReturnSelf();
+		$notification->expects($this->once())
+			->method('setParsedMessage')
+			->with($parsedMessage)
+			->willReturnSelf();

 		$notification->expects($this->once())
 			->method('getApp')
@ -447,6 +460,12 @@ class NotifierTest extends \Test\TestCase {
 		$notification->expects($this->once())
 			->method('getObjectType')
 			->willReturn('room');
+		$notification->expects($this->once())
+			->method('getMessage')
+			->willReturn('message');
+		$notification->expects($this->once())
+			->method('getMessageParameters')
+			->willReturn($messageParameters);

 		$this->assertEquals($notification, $this->notifier->prepare($notification, 'de'));
 	}