Merge pull request #1738 from nextcloud/comments-provide-displaynames-with-mentions

comment mentions: show displayname not uid

apps/comments/appinfo/app.php

@@ -71,3 +71,14 @@ $commentsManager->registerEventHandler(function () {
 	$handler = $application->getContainer()->query(\OCA\Comments\EventHandler::class);
 	return $handler;
 });
+$commentsManager->registerDisplayNameResolver('user', function($id) {
+	$manager = \OC::$server->getUserManager();
+	$user = $manager->get($id);
+	if(is_null($user)) {
+		$l = \OC::$server->getL10N('comments');
+		$displayName = $l->t('Unknown user');
+	} else {
+		$displayName = $user->getDisplayName();
+	}
+	return $displayName;
+});

apps/comments/css/comments.css

@@ -64,6 +64,10 @@
 	line-height: 32px;
 }
 
+#commentsTabView .comment .message .avatar {
+	display: inline-block;
+}
+
 #activityTabView li.comment.collapsed .activitymessage,
 #commentsTabView .comment.collapsed .message {
 	white-space: pre-wrap;

apps/comments/js/commentmodel.js

@@ -35,7 +35,8 @@
 			'creationDateTime': '{' + NS_OWNCLOUD + '}creationDateTime',
 			'objectType': '{' + NS_OWNCLOUD + '}objectType',
 			'objectId': '{' + NS_OWNCLOUD + '}objectId',
-			'isUnread': '{' + NS_OWNCLOUD + '}isUnread'
+			'isUnread': '{' + NS_OWNCLOUD + '}isUnread',
+			'mentions': '{' + NS_OWNCLOUD + '}mentions'
 		},
 
 		parse: function(data) {
@@ -48,8 +49,30 @@
 				creationDateTime: data.creationDateTime,
 				objectType: data.objectType,
 				objectId: data.objectId,
-				isUnread: (data.isUnread === 'true')
+				isUnread: (data.isUnread === 'true'),
+				mentions: this._parseMentions(data.mentions)
 			};
+		},
+
+		_parseMentions: function(mentions) {
+			if(_.isUndefined(mentions)) {
+				return {};
+			}
+			var result = {};
+			for(var i in mentions) {
+				var mention = mentions[i];
+				if(_.isUndefined(mention.localName) || mention.localName !== 'mention') {
+					continue;
+				}
+				result[i] = {};
+				for (var child = mention.firstChild; child; child = child.nextSibling) {
+					if(_.isUndefined(child.localName) || !child.localName.startsWith('mention')) {
+						continue;
+					}
+					result[i][child.localName] = child.textContent;
+				}
+			}
+			return result;
 		}
 	});
 

apps/comments/js/commentstabview.js

@@ -184,7 +184,7 @@
 				timestamp: timestamp,
 				date: OC.Util.relativeModifiedDate(timestamp),
 				altDate: OC.Util.formatDate(timestamp),
-				formattedMessage: this._formatMessage(commentModel.get('message'))
+				formattedMessage: this._formatMessage(commentModel.get('message'), commentModel.get('mentions'))
 			}, commentModel.attributes);
 			return data;
 		},
@@ -251,8 +251,35 @@
 		 * Convert a message to be displayed in HTML,
 		 * converts newlines to <br> tags.
 		 */
-		_formatMessage: function(message) {
+		_formatMessage: function(message, mentions) {
-			return escapeHTML(message).replace(/\n/g, '<br/>');
+			message = escapeHTML(message).replace(/\n/g, '<br/>');
+
+			for(var i in mentions) {
+				var mention = '@' + mentions[i].mentionId;
+
+				var avatar = '';
+				if(this._avatarsEnabled) {
+					avatar = '<div class="avatar" '
+						+ 'data-user="' + _.escape(mentions[i].mentionId) + '"'
+						+' data-user-display-name="'
+						+ _.escape(mentions[i].mentionDisplayName) + '"></div>';
+				}
+
+				// escape possible regex characters in the name
+				mention = mention.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+				var displayName = avatar + ' <strong>'+ _.escape(mentions[i].mentionDisplayName)+'</strong>';
+
+				// replace every mention either at the start of the input or after a whitespace
+				// followed by a non-word character.
+				message = message.replace(new RegExp("(^|\\s)(" + mention + ")\\b", 'g'),
+					function(match, p1) {
+						// to  get number of whitespaces (0 vs 1) right
+						return p1+displayName;
+					}
+				);
+			}
+
+			return message;
 		},
 
 		nextPage: function() {
@@ -280,7 +307,7 @@
 			$formRow.find('textarea').on('keydown input change', this._onTypeComment);
 
 			// copy avatar element from original to avoid flickering
-			$formRow.find('.avatar').replaceWith($comment.find('.avatar').clone());
+			$formRow.find('.avatar:first').replaceWith($comment.find('.avatar:first').clone());
 			$formRow.find('.has-tooltip').tooltip();
 
 			// Enable autosize
@@ -359,6 +386,48 @@
 			this.nextPage();
 		},
 
+		/**
+		 * takes care of updating comment elements after submit (either new
+		 * comment or edit).
+		 *
+		 * @param {OC.Backbone.Model} model
+		 * @param {jQuery} $form
+		 * @param {string|undefined} commentId
+		 * @private
+		 */
+		_onSubmitSuccess: function(model, $form, commentId) {
+			var self = this;
+			var $submit = $form.find('.submit');
+			var $loading = $form.find('.submitLoading');
+			var $textArea = $form.find('.message');
+
+			model.fetch({
+				success: function(model) {
+					$submit.removeClass('hidden');
+					$loading.addClass('hidden');
+					var $target;
+
+					if(!_.isUndefined(commentId)) {
+						var $row = $form.closest('.comment');
+						$target = $row.data('commentEl');
+						$target.removeClass('hidden');
+						$row.remove();
+					} else {
+						$target = $('.commentsTabView .comments').find('li:first');
+						$textArea.val('').prop('disabled', false);
+					}
+
+					$target.find('.message')
+						.html(self._formatMessage(model.get('message'), model.get('mentions')))
+						.find('.avatar')
+						.each(function () { $(this).avatar(); });
+				},
+				error: function () {
+					self._onSubmitError($form, commentId);
+				}
+			});
+		},
+
 		_onSubmitComment: function(e) {
 			var self = this;
 			var $form = $(e.target);
@@ -385,21 +454,10 @@
 					message: $textArea.val()
 				}, {
 					success: function(model) {
-						var $row = $form.closest('.comment');
+						self._onSubmitSuccess(model, $form, commentId);
-						$submit.removeClass('hidden');
-						$loading.addClass('hidden');
-						$row.data('commentEl')
-							.removeClass('hidden')
-							.find('.message')
-							.html(self._formatMessage(model.get('message')));
-						$row.remove();
 					},
 					error: function() {
-						$submit.removeClass('hidden');
+						self._onSubmitError($form, commentId);
-						$loading.addClass('hidden');
-						$textArea.prop('disabled', false);
-
-						OC.Notification.showTemporary(t('comments', 'Error occurred while updating comment with id {id}', {id: commentId}));
 					}
 				});
 			} else {
@@ -414,17 +472,11 @@
 					at: 0,
 					// wait for real creation before adding
 					wait: true,
-					success: function() {
+					success: function(model) {
-						$submit.removeClass('hidden');
+						self._onSubmitSuccess(model, $form);
-						$loading.addClass('hidden');
-						$textArea.val('').prop('disabled', false);
 					},
 					error: function() {
-						$submit.removeClass('hidden');
+						self._onSubmitError($form);
-						$loading.addClass('hidden');
-						$textArea.prop('disabled', false);
-
-						OC.Notification.showTemporary(t('comments', 'Error occurred while posting comment'));
 					}
 				});
 			}
@@ -432,6 +484,26 @@
 			return false;
 		},
 
+		/**
+		 * takes care of updating the UI after an error on submit (either new
+		 * comment or edit).
+		 *
+		 * @param {jQuery} $form
+		 * @param {string|undefined} commentId
+		 * @private
+		 */
+		_onSubmitError: function($form, commentId) {
+			$form.find('.submit').removeClass('hidden');
+			$form.find('.submitLoading').addClass('hidden');
+			$form.find('.message').prop('disabled', false);
+
+			if(!_.isUndefined(commentId)) {
+				OC.Notification.showTemporary(t('comments', 'Error occurred while updating comment with id {id}', {id: commentId}));
+			} else {
+				OC.Notification.showTemporary(t('comments', 'Error occurred while posting comment'));
+			}
+		},
+
 		/**
 		 * Returns whether the given message is long and needs
 		 * collapsing

apps/comments/lib/Notification/Listener.php

@@ -61,7 +61,7 @@ class Listener {
 	public function evaluate(CommentsEvent $event) {
 		$comment = $event->getComment();
 
-		$mentions = $this->extractMentions($comment->getMessage());
+		$mentions = $this->extractMentions($comment->getMentions());
 		if(empty($mentions)) {
 			// no one to notify
 			return;
@@ -69,16 +69,15 @@ class Listener {
 
 		$notification = $this->instantiateNotification($comment);
 
-		foreach($mentions as $mention) {
+		foreach($mentions as $uid) {
-			$user = substr($mention, 1); // @username → username
+			if( ($comment->getActorType() === 'users' && $uid === $comment->getActorId())
-			if( ($comment->getActorType() === 'users' && $user === $comment->getActorId())
+				|| !$this->userManager->userExists($uid)
-				|| !$this->userManager->userExists($user)
 			) {
 				// do not notify unknown users or yourself
 				continue;
 			}
 
-			$notification->setUser($user);
+			$notification->setUser($uid);
 			if(    $event->getEvent() === CommentsEvent::EVENT_DELETE
 				|| $event->getEvent() === CommentsEvent::EVENT_PRE_UPDATE)
 			{
@@ -111,16 +110,21 @@ class Listener {
 	}
 
 	/**
-	 * extracts @-mentions out of a message body.
+	 * flattens the mention array returned from comments to a list of user ids.
 	 *
-	 * @param string $message
+	 * @param array $mentions
-	 * @return string[] containing the mentions, e.g. ['@alice', '@bob']
+	 * @return string[] containing the mentions, e.g. ['alice', 'bob']
 	 */
-	public function extractMentions($message) {
+	public function extractMentions(array $mentions) {
-		$ok = preg_match_all('/\B@[a-z0-9_\-@\.\']+/i', $message, $mentions);
+		if(empty($mentions)) {
-		if(!$ok || !isset($mentions[0]) || !is_array($mentions[0])) {
 			return [];
 		}
-		return array_unique($mentions[0]);
+		$uids = [];
+		foreach($mentions as $mention) {
+			if($mention['type'] === 'user') {
+				$uids[] = $mention['id'];
+			}
+		}
+		return $uids;
 	}
 }

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

@@ -72,10 +72,6 @@ class ListenerTest extends TestCase {
 	 * @param string $notificationMethod
 	 */
 	public function testEvaluate($eventType, $notificationMethod) {
-		$message = '@foobar and @barfoo you should know, @foo@bar.com is valid' .
-			' and so is @bar@foo.org@foobar.io I hope that clarifies everything.' .
-			' cc @23452-4333-54353-2342 @yolo!';
-
 		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
 		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
 		$comment->expects($this->any())
@@ -85,8 +81,15 @@ class ListenerTest extends TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue(new \DateTime()));
 		$comment->expects($this->once())
-			->method('getMessage')
+			->method('getMentions')
-			->will($this->returnValue($message));
+			->willReturn([
+				[ 'type' => 'user', 'id' => 'foobar'],
+				[ 'type' => 'user', 'id' => 'barfoo'],
+				[ 'type' => 'user', 'id' => 'foo@bar.com'],
+				[ 'type' => 'user', 'id' => 'bar@foo.org@foobar.io'],
+				[ 'type' => 'user', 'id' => '23452-4333-54353-2342'],
+				[ 'type' => 'user', 'id' => 'yolo'],
+			]);
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
@@ -134,8 +137,6 @@ class ListenerTest extends TestCase {
 	 * @param string $eventType
 	 */
 	public function testEvaluateNoMentions($eventType) {
-		$message = 'a boring comment without mentions';
-
 		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
 		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
 		$comment->expects($this->any())
@@ -145,8 +146,8 @@ class ListenerTest extends TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue(new \DateTime()));
 		$comment->expects($this->once())
-			->method('getMessage')
+			->method('getMentions')
-			->will($this->returnValue($message));
+			->willReturn([]);
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
@@ -173,8 +174,6 @@ class ListenerTest extends TestCase {
 	}
 
 	public function testEvaluateUserDoesNotExist() {
-		$message = '@foobar bla bla bla';
-
 		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
 		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
 		$comment->expects($this->any())
@@ -184,8 +183,8 @@ class ListenerTest extends TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue(new \DateTime()));
 		$comment->expects($this->once())
-			->method('getMessage')
+			->method('getMentions')
-			->will($this->returnValue($message));
+			->willReturn([[ 'type' => 'user', 'id' => 'foobar']]);
 
 		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
 		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
@@ -221,119 +220,4 @@ class ListenerTest extends TestCase {
 
 		$this->listener->evaluate($event);
 	}
-
-	/**
-	 * @dataProvider eventProvider
-	 * @param string $eventType
-	 * @param string $notificationMethod
-	 */
-	public function testEvaluateOneMentionPerUser($eventType, $notificationMethod) {
-		$message = '@foobar bla bla bla @foobar';
-
-		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
-		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
-		$comment->expects($this->any())
-			->method('getObjectType')
-			->will($this->returnValue('files'));
-		$comment->expects($this->any())
-			->method('getCreationDateTime')
-			->will($this->returnValue(new \DateTime()));
-		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
-
-		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
-		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
-			->disableOriginalConstructor()
-			->getMock();
-		$event->expects($this->once())
-			->method('getComment')
-			->will($this->returnValue($comment));
-		$event->expects(($this->any()))
-			->method(('getEvent'))
-			->will($this->returnValue($eventType));
-
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
-		$notification = $this->getMockBuilder('\OCP\Notification\INotification')->getMock();
-		$notification->expects($this->any())
-			->method($this->anything())
-			->will($this->returnValue($notification));
-		$notification->expects($this->once())
-			->method('setUser');
-
-		$this->notificationManager->expects($this->once())
-			->method('createNotification')
-			->will($this->returnValue($notification));
-		$this->notificationManager->expects($this->once())
-			->method($notificationMethod)
-			->with($this->isInstanceOf('\OCP\Notification\INotification'));
-
-		$this->userManager->expects($this->once())
-			->method('userExists')
-			->withConsecutive(
-				['foobar']
-			)
-			->will($this->returnValue(true));
-
-		$this->listener->evaluate($event);
-	}
-
-	/**
-	 * @dataProvider eventProvider
-	 * @param string $eventType
-	 */
-	public function testEvaluateNoSelfMention($eventType) {
-		$message = '@foobar bla bla bla';
-
-		/** @var IComment|\PHPUnit_Framework_MockObject_MockObject $comment */
-		$comment = $this->getMockBuilder('\OCP\Comments\IComment')->getMock();
-		$comment->expects($this->any())
-			->method('getObjectType')
-			->will($this->returnValue('files'));
-		$comment->expects($this->any())
-			->method('getActorType')
-			->will($this->returnValue('users'));
-		$comment->expects($this->any())
-			->method('getActorId')
-			->will($this->returnValue('foobar'));
-		$comment->expects($this->any())
-			->method('getCreationDateTime')
-			->will($this->returnValue(new \DateTime()));
-		$comment->expects($this->once())
-			->method('getMessage')
-			->will($this->returnValue($message));
-
-		/** @var CommentsEvent|\PHPUnit_Framework_MockObject_MockObject $event */
-		$event = $this->getMockBuilder('\OCP\Comments\CommentsEvent')
-			->disableOriginalConstructor()
-			->getMock();
-		$event->expects($this->once())
-			->method('getComment')
-			->will($this->returnValue($comment));
-		$event->expects(($this->any()))
-			->method(('getEvent'))
-			->will($this->returnValue($eventType));
-
-		/** @var INotification|\PHPUnit_Framework_MockObject_MockObject $notification */
-		$notification = $this->getMockBuilder('\OCP\Notification\INotification')->getMock();
-		$notification->expects($this->any())
-			->method($this->anything())
-			->will($this->returnValue($notification));
-		$notification->expects($this->never())
-			->method('setUser');
-
-		$this->notificationManager->expects($this->once())
-			->method('createNotification')
-			->will($this->returnValue($notification));
-		$this->notificationManager->expects($this->never())
-			->method('notify');
-		$this->notificationManager->expects($this->never())
-			->method('markProcessed');
-
-		$this->userManager->expects($this->never())
-			->method('userExists');
-
-		$this->listener->evaluate($event);
-	}
-
 }

apps/comments/tests/js/commentstabviewSpec.js

@@ -43,6 +43,7 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 		clock = sinon.useFakeTimers(Date.UTC(2016, 1, 3, 10, 5, 9));
 		fetchStub = sinon.stub(OCA.Comments.CommentCollection.prototype, 'fetchNext');
 		view = new OCA.Comments.CommentsTabView();
+		view._avatarsEnabled = false;
 		fileInfoModel = new OCA.Files.FileInfoModel({
 			id: 5,
 			name: 'One.txt',
@@ -74,8 +75,29 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 			message: 'Second\nNewline',
 			creationDateTime: new Date(Date.UTC(2016, 1, 3, 10, 0, 0)).toUTCString()
 		});
+		var comment3 = new OCA.Comments.CommentModel({
+			id: 3,
+			actorId: 'anotheruser',
+			actorDisplayName: 'Another User',
+			actorType: 'users',
+			verb: 'comment',
+			message: 'Hail to thee, @macbeth. Yours faithfully, @banquo',
+			creationDateTime: new Date(Date.UTC(2016, 1, 3, 10, 5, 9)).toUTCString(),
+			mentions: {
+				0: {
+					mentionDisplayName: "Thane of Cawdor",
+					mentionId: "macbeth",
+					mentionTye: "user"
+				},
+				1: {
+					mentionDisplayName: "Lord Banquo",
+					mentionId: "banquo",
+					mentionTye: "user"
+				}
+			}
+		});
 
-		testComments = [comment1, comment2];
+		testComments = [comment1, comment2, comment3];
 	});
 	afterEach(function() {
 		view.remove();
@@ -102,7 +124,7 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 			view.collection.set(testComments);
 
 			var $comments = view.$el.find('.comments>li');
-			expect($comments.length).toEqual(2);
+			expect($comments.length).toEqual(3);
 			var $item = $comments.eq(0);
 			expect($item.find('.author').text()).toEqual('User One');
 			expect($item.find('.date').text()).toEqual('seconds ago');
@@ -122,6 +144,32 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 			expect($item.find('.author').text()).toEqual('[Deleted user]');
 			expect($item.find('.avatar').attr('data-username')).not.toBeDefined();
 		});
+
+		it('renders mentioned user id to avatar and displayname', function() {
+			view._avatarsEnabled = true;
+			view.collection.set(testComments);
+
+			var $comment = view.$el.find('.comment[data-id=3] .message');
+			expect($comment.length).toEqual(1);
+			expect($comment.find('.avatar[data-user=macbeth]').length).toEqual(1);
+			expect($comment.find('strong:first').text()).toEqual('Thane of Cawdor');
+
+			expect($comment.find('.avatar[data-user=banquo]').length).toEqual(1);
+			expect($comment.find('strong:last-child').text()).toEqual('Lord Banquo');
+		});
+
+		it('renders mentioned user id to displayname, avatars disabled', function() {
+			view.collection.set(testComments);
+
+			var $comment = view.$el.find('.comment[data-id=3] .message');
+			expect($comment.length).toEqual(1);
+			expect($comment.find('.avatar[data-user=macbeth]').length).toEqual(0);
+			expect($comment.find('strong:first-child').text()).toEqual('Thane of Cawdor');
+
+			expect($comment.find('.avatar[data-user=banquo]').length).toEqual(0);
+			expect($comment.find('strong:last-child').text()).toEqual('Lord Banquo');
+		});
+
 	});
 	describe('more comments', function() {
 		var hasMoreResultsStub;
@@ -156,8 +204,8 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 			expect(fetchStub.calledOnce).toEqual(true);
 		});
 		it('appends comment to the list when added to collection', function() {
-			var comment3 = new OCA.Comments.CommentModel({
+			var comment4 = new OCA.Comments.CommentModel({
-				id: 3,
+				id: 4,
 				actorType: 'users',
 				actorId: 'user3',
 				actorDisplayName: 'User Three',
@@ -167,11 +215,11 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 				creationDateTime: new Date(Date.UTC(2016, 1, 3, 5, 0, 0)).toUTCString()
 			});
 
-			view.collection.add(comment3);
+			view.collection.add(comment4);
 
-			expect(view.$el.find('.comments>li').length).toEqual(3);
+			expect(view.$el.find('.comments>li').length).toEqual(4);
 
-			var $item = view.$el.find('.comments>li').eq(2);
+			var $item = view.$el.find('.comments>li').eq(3);
 			expect($item.find('.author').text()).toEqual('User Three');
 			expect($item.find('.date').text()).toEqual('5 hours ago');
 			expect($item.find('.message').html()).toEqual('Third');
@@ -267,10 +315,12 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 	});
 	describe('editing comments', function() {
 		var saveStub;
+		var fetchStub;
 		var currentUserStub;
 
 		beforeEach(function() {
 			saveStub = sinon.stub(OCA.Comments.CommentModel.prototype, 'save');
+			fetchStub = sinon.stub(OCA.Comments.CommentModel.prototype, 'fetch');
 			currentUserStub = sinon.stub(OC, 'getCurrentUser');
 			currentUserStub.returns({
 				uid: 'testuser',
@@ -292,11 +342,12 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 				actorType: 'users',
 				verb: 'comment',
 				message: 'New message from another user',
-				creationDateTime: new Date(Date.UTC(2016, 1, 3, 10, 5, 9)).toUTCString()
+				creationDateTime: new Date(Date.UTC(2016, 1, 3, 10, 5, 9)).toUTCString(),
 			});
 		});
 		afterEach(function() {
 			saveStub.restore();
+			fetchStub.restore();
 			currentUserStub.restore();
 		});
 
@@ -341,6 +392,9 @@ describe('OCA.Comments.CommentsTabView tests', function() {
 			model.set('message', 'modified\nmessage');
 			saveStub.yieldTo('success', model);
 
+			expect(fetchStub.calledOnce).toEqual(true);
+			fetchStub.yieldTo('success', model);
+
 			// original comment element is visible again
 			expect($comment.hasClass('hidden')).toEqual(false);
 			// and its message was updated

apps/dav/lib/Comments/CommentNode.php

@@ -41,6 +41,11 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 	const PROPERTY_NAME_UNREAD = '{http://owncloud.org/ns}isUnread';
 	const PROPERTY_NAME_MESSAGE = '{http://owncloud.org/ns}message';
 	const PROPERTY_NAME_ACTOR_DISPLAYNAME = '{http://owncloud.org/ns}actorDisplayName';
+	const PROPERTY_NAME_MENTIONS = '{http://owncloud.org/ns}mentions';
+	const PROPERTY_NAME_MENTION = '{http://owncloud.org/ns}mention';
+	const PROPERTY_NAME_MENTION_TYPE = '{http://owncloud.org/ns}mentionType';
+	const PROPERTY_NAME_MENTION_ID = '{http://owncloud.org/ns}mentionId';
+	const PROPERTY_NAME_MENTION_DISPLAYNAME = '{http://owncloud.org/ns}mentionDisplayName';
 
 	/** @var  IComment */
 	public $comment;
@@ -85,6 +90,9 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			return strpos($name, 'get') === 0;
 		});
 		foreach($methods as $getter) {
+			if($getter === 'getMentions') {
+				continue;	// special treatment
+			}
 			$name = '{'.self::NS_OWNCLOUD.'}' . lcfirst(substr($getter, 3));
 			$this->properties[$name] = $getter;
 		}
@@ -113,7 +121,12 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			// re-used property names are defined as constants
 			self::PROPERTY_NAME_MESSAGE,
 			self::PROPERTY_NAME_ACTOR_DISPLAYNAME,
-			self::PROPERTY_NAME_UNREAD
+			self::PROPERTY_NAME_UNREAD,
+			self::PROPERTY_NAME_MENTIONS,
+			self::PROPERTY_NAME_MENTION,
+			self::PROPERTY_NAME_MENTION_TYPE,
+			self::PROPERTY_NAME_MENTION_ID,
+			self::PROPERTY_NAME_MENTION_DISPLAYNAME,
 		];
 	}
 
@@ -240,6 +253,8 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 			$result[self::PROPERTY_NAME_ACTOR_DISPLAYNAME] = $displayName;
 		}
 
+		$result[self::PROPERTY_NAME_MENTIONS] = $this->composeMentionsPropertyValue();
+
 		$unread = null;
 		$user =  $this->userSession->getUser();
 		if(!is_null($user)) {
@@ -260,4 +275,31 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties {
 
 		return $result;
 	}
+
+	/**
+	 * transforms a mentions array as returned from IComment->getMentions to an
+	 * array with DAV-compatible structure that can be assigned to the
+	 * PROPERTY_NAME_MENTION property.
+	 *
+	 * @return array
+	 */
+	protected function composeMentionsPropertyValue() {
+		return array_map(function($mention) {
+			try {
+				$displayName = $this->commentsManager->resolveDisplayName($mention['type'], $mention['id']);
+			} catch (\OutOfBoundsException $e) {
+				$this->logger->logException($e);
+				// No displayname, upon client's discretion what to display.
+				$displayName = '';
+			}
+
+			return [
+				self::PROPERTY_NAME_MENTION => [
+					self::PROPERTY_NAME_MENTION_TYPE        => $mention['type'],
+					self::PROPERTY_NAME_MENTION_ID          => $mention['id'],
+					self::PROPERTY_NAME_MENTION_DISPLAYNAME => $displayName,
+				]
+			];
+		}, $this->comment->getMentions());
+	}
 }

apps/dav/tests/unit/Comments/CommentsNodeTest.php

@@ -27,11 +27,14 @@ namespace OCA\DAV\Tests\unit\Comments;
 
 use OCA\DAV\Comments\CommentNode;
 use OCP\Comments\IComment;
+use OCP\Comments\ICommentsManager;
 use OCP\Comments\MessageTooLongException;
 
 class CommentsNodeTest extends \Test\TestCase {
 
+	/** @var  ICommentsManager|\PHPUnit_Framework_MockObject_MockObject */
 	protected $commentsManager;
+
 	protected $comment;
 	protected $node;
 	protected $userManager;
@@ -373,6 +376,18 @@ class CommentsNodeTest extends \Test\TestCase {
 			$ns . 'topmostParentId' => '2',
 			$ns . 'childrenCount' => 3,
 			$ns . 'message' => 'such a nice file you have…',
+			$ns . 'mentions' => [
+				[ $ns . 'mention' => [
+					$ns . 'mentionType' => 'user',
+					$ns . 'mentionId' => 'alice',
+					$ns . 'mentionDisplayName' => 'Alice Al-Isson',
+				] ],
+				[ $ns . 'mention' => [
+					$ns . 'mentionType' => 'user',
+					$ns . 'mentionId' => 'bob',
+					$ns . 'mentionDisplayName' => 'Unknown user',
+				] ],
+			],
 			$ns . 'verb' => 'comment',
 			$ns . 'actorType' => 'users',
 			$ns . 'actorId' => 'alice',
@@ -384,6 +399,14 @@ class CommentsNodeTest extends \Test\TestCase {
 			$ns . 'isUnread' => null,
 		];
 
+		$this->commentsManager->expects($this->exactly(2))
+			->method('resolveDisplayName')
+			->withConsecutive(
+				[$this->equalTo('user'), $this->equalTo('alice')],
+				[$this->equalTo('user'), $this->equalTo('bob')]
+			)
+			->willReturnOnConsecutiveCalls('Alice Al-Isson', 'Unknown user');
+
 		$this->comment->expects($this->once())
 			->method('getId')
 			->will($this->returnValue($expected[$ns . 'id']));
@@ -404,6 +427,13 @@ class CommentsNodeTest extends \Test\TestCase {
 			->method('getMessage')
 			->will($this->returnValue($expected[$ns . 'message']));
 
+		$this->comment->expects($this->once())
+			->method('getMentions')
+			->willReturn([
+				['type' => 'user', 'id' => 'alice'],
+				['type' => 'user', 'id' => 'bob'],
+			]);
+
 		$this->comment->expects($this->once())
 			->method('getVerb')
 			->will($this->returnValue($expected[$ns . 'verb']));
@@ -475,6 +505,10 @@ class CommentsNodeTest extends \Test\TestCase {
 			->method('getCreationDateTime')
 			->will($this->returnValue($creationDT));
 
+		$this->comment->expects($this->any())
+			->method('getMentions')
+			->willReturn([]);
+
 		$this->commentsManager->expects($this->once())
 			->method('getReadMark')
 			->will($this->returnValue($readDT));

lib/private/Comments/Comment.php

@@ -203,6 +203,43 @@ class Comment implements IComment {
 		return $this;
 	}
 
+	/**
+	 * returns an array containing mentions that are included in the comment
+	 *
+	 * @return array each mention provides a 'type' and an 'id', see example below
+	 * @since 9.2.0
+	 *
+	 * The return array looks like:
+	 * [
+	 *   [
+	 *     'type' => 'user',
+	 *     'id' => 'citizen4'
+	 *   ],
+	 *   [
+	 *     'type' => 'group',
+	 *     'id' => 'media'
+	 *   ],
+	 *   …
+	 * ]
+	 *
+	 */
+	public function getMentions() {
+		$ok = preg_match_all('/\B@[a-z0-9_\-@\.\']+/i', $this->getMessage(), $mentions);
+		if(!$ok || !isset($mentions[0]) || !is_array($mentions[0])) {
+			return [];
+		}
+		$uids = array_unique($mentions[0]);
+		$result = [];
+		foreach ($uids as $uid) {
+			// exclude author, no self-mentioning
+			if($uid === '@' . $this->getActorId()) {
+				continue;
+			}
+			$result[] = ['type' => 'user', 'id' => substr($uid, 1)];
+		}
+		return $result;
+	}
+
 	/**
 	 * returns the verb of the comment
 	 *

lib/private/Comments/Manager.php

@@ -55,6 +55,9 @@ class Manager implements ICommentsManager {
 	/** @var  ICommentsEventHandler[] */
 	protected $eventHandlers = [];
 
+	/** @var \Closure[] */
+	protected $displayNameResolvers = [];
+
 	/**
 	 * Manager constructor.
 	 *
@@ -759,6 +762,50 @@ class Manager implements ICommentsManager {
 		$this->eventHandlers = [];
 	}
 
+	/**
+	 * registers a method that resolves an ID to a display name for a given type
+	 *
+	 * @param string $type
+	 * @param \Closure $closure
+	 * @throws \OutOfBoundsException
+	 * @since 9.2.0
+	 *
+	 * Only one resolver shall be registered per type. Otherwise a
+	 * \OutOfBoundsException has to thrown.
+	 */
+	public function registerDisplayNameResolver($type, \Closure $closure) {
+		if(!is_string($type)) {
+			throw new \InvalidArgumentException('String expected.');
+		}
+		if(isset($this->displayNameResolvers[$type])) {
+			throw new \OutOfBoundsException('Displayname resolver for this type already registered');
+		}
+		$this->displayNameResolvers[$type] = $closure;
+	}
+
+	/**
+	 * resolves a given ID of a given Type to a display name.
+	 *
+	 * @param string $type
+	 * @param string $id
+	 * @return string
+	 * @throws \OutOfBoundsException
+	 * @since 9.2.0
+	 *
+	 * If a provided type was not registered, an \OutOfBoundsException shall
+	 * be thrown. It is upon the resolver discretion what to return of the
+	 * provided ID is unknown. It must be ensured that a string is returned.
+	 */
+	public function resolveDisplayName($type, $id) {
+		if(!is_string($type)) {
+			throw new \InvalidArgumentException('String expected.');
+		}
+		if(!isset($this->displayNameResolvers[$type])) {
+			throw new \OutOfBoundsException('No Displayname resolver for this type registered');
+		}
+		return (string)$this->displayNameResolvers[$type]($id);
+	}
+
 	/**
 	 * returns valid, registered entities
 	 *

lib/public/Comments/IComment.php

@@ -132,6 +132,28 @@ interface IComment {
 	 */
 	public function setMessage($message);
 
+	/**
+	 * returns an array containing mentions that are included in the comment
+	 *
+	 * @return array each mention provides a 'type' and an 'id', see example below
+	 * @since 9.2.0
+	 *
+	 * The return array looks like:
+	 * [
+	 *   [
+	 *     'type' => 'user',
+	 *     'id' => 'citizen4'
+	 *   ],
+	 *   [
+	 *     'type' => 'group',
+	 *     'id' => 'media'
+	 *   ],
+	 *   …
+	 * ]
+	 *
+	 */
+	public function getMentions();
+
 	/**
 	 * returns the verb of the comment
 	 *

lib/public/Comments/ICommentsManager.php

@@ -246,4 +246,32 @@ interface ICommentsManager {
 	 */
 	public function registerEventHandler(\Closure $closure);
 
+	/**
+	 * registers a method that resolves an ID to a display name for a given type
+	 *
+	 * @param string $type
+	 * @param \Closure $closure
+	 * @throws \OutOfBoundsException
+	 * @since 9.2.0
+	 *
+	 * Only one resolver shall be registered per type. Otherwise a
+	 * \OutOfBoundsException has to thrown.
+	 */
+	public function registerDisplayNameResolver($type, \Closure $closure);
+
+	/**
+	 * resolves a given ID of a given Type to a display name.
+	 *
+	 * @param string $type
+	 * @param string $id
+	 * @return string
+	 * @throws \OutOfBoundsException
+	 * @since 9.2.0
+	 *
+	 * If a provided type was not registered, an \OutOfBoundsException shall
+	 * be thrown. It is upon the resolver discretion what to return of the
+	 * provided ID is unknown. It must be ensured that a string is returned.
+	 */
+	public function resolveDisplayName($type, $id);
+
 }

tests/lib/Comments/CommentTest.php

@@ -2,13 +2,14 @@
 
 namespace Test\Comments;
 
+use OC\Comments\Comment;
 use OCP\Comments\IComment;
 use Test\TestCase;
 
 class CommentTest extends TestCase {
 
 	public function testSettersValidInput() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 
 		$id = 'comment23';
 		$parentId = 'comment11.5';
@@ -51,14 +52,14 @@ class CommentTest extends TestCase {
 	 * @expectedException \OCP\Comments\IllegalIDChangeException
 	 */
 	public function testSetIdIllegalInput() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 
 		$comment->setId('c23');
 		$comment->setId('c17');
 	}
 
 	public function testResetId() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$comment->setId('c23');
 		$comment->setId('');
 
@@ -82,7 +83,7 @@ class CommentTest extends TestCase {
 	 * @expectedException \InvalidArgumentException
 	 */
 	public function testSimpleSetterInvalidInput($field, $input) {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$setter = 'set' . $field;
 
 		$comment->$setter($input);
@@ -106,7 +107,7 @@ class CommentTest extends TestCase {
 	 * @expectedException \InvalidArgumentException
 	 */
 	public function testSetRoleInvalidInput($role, $type, $id){
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$setter = 'set' . $role;
 		$comment->$setter($type, $id);
 	}
@@ -115,11 +116,55 @@ class CommentTest extends TestCase {
 	 * @expectedException \OCP\Comments\MessageTooLongException
 	 */
 	public function testSetUberlongMessage() {
-		$comment = new \OC\Comments\Comment();
+		$comment = new Comment();
 		$msg = str_pad('', IComment::MAX_MESSAGE_LENGTH + 1, 'x');
 		$comment->setMessage($msg);
 	}
 
+	public function mentionsProvider() {
+		return [
+			[
+				'@alice @bob look look, a cook!', ['alice', 'bob']
+			],
+			[
+				'no mentions in this message', []
+			],
+			[
+				'@alice @bob look look, a duplication @alice test @bob!', ['alice', 'bob']
+			],
+			[
+				'@alice is the author, but notify @bob!', ['bob'], 'alice'
+			],
+			[
+				'@foobar and @barfoo you should know, @foo@bar.com is valid' .
+					' and so is @bar@foo.org@foobar.io I hope that clarifies everything.' .
+					' cc @23452-4333-54353-2342 @yolo!',
+				['foobar', 'barfoo', 'foo@bar.com', 'bar@foo.org@foobar.io', '23452-4333-54353-2342', 'yolo']
+			]
+
+		];
+	}
+
+	/**
+	 * @dataProvider mentionsProvider
+	 */
+	public function testMentions($message, $expectedUids, $author = null) {
+		$comment = new Comment();
+		$comment->setMessage($message);
+		if(!is_null($author)) {
+			$comment->setActor('user', $author);
+		}
+		$mentions = $comment->getMentions();
+		while($mention = array_shift($mentions)) {
+			$uid = array_shift($expectedUids);
+			$this->assertSame('user', $mention['type']);
+			$this->assertSame($uid, $mention['id']);
+			$this->assertNotSame($author, $mention['id']);
+		}
+		$this->assertEmpty($mentions);
+		$this->assertEmpty($expectedUids);
+	}
+
 
 
 }

tests/lib/Comments/FakeManager.php

@@ -40,4 +40,8 @@ class FakeManager implements \OCP\Comments\ICommentsManager {
 	public function deleteReadMarksOnObject($objectType, $objectId) {}
 
 	public function registerEventHandler(\Closure $closure) {}
+
+	public function registerDisplayNameResolver($type, \Closure $closure) {}
+
+	public function resolveDisplayName($type, $id) {}
 }

tests/lib/Comments/ManagerTest.php

@@ -664,4 +664,82 @@ class ManagerTest extends TestCase {
 		$manager->delete($comment->getId());
 	}
 
+	public function testResolveDisplayName() {
+		$manager = $this->getManager();
+
+		$planetClosure = function($name) {
+			return ucfirst($name);
+		};
+
+		$galaxyClosure = function($name) {
+			return strtoupper($name);
+		};
+
+		$manager->registerDisplayNameResolver('planet', $planetClosure);
+		$manager->registerDisplayNameResolver('galaxy', $galaxyClosure);
+
+		$this->assertSame('Neptune', $manager->resolveDisplayName('planet', 'neptune'));
+		$this->assertSame('SOMBRERO', $manager->resolveDisplayName('galaxy', 'sombrero'));
+	}
+
+	/**
+	 * @expectedException \OutOfBoundsException
+	 */
+	public function testRegisterResolverDuplicate() {
+		$manager = $this->getManager();
+
+		$planetClosure = function($name) {
+			return ucfirst($name);
+		};
+		$manager->registerDisplayNameResolver('planet', $planetClosure);
+		$manager->registerDisplayNameResolver('planet', $planetClosure);
+	}
+
+	/**
+	 * @expectedException \InvalidArgumentException
+	 */
+	public function testRegisterResolverInvalidType() {
+		$manager = $this->getManager();
+
+		$planetClosure = function($name) {
+			return ucfirst($name);
+		};
+		$manager->registerDisplayNameResolver(1337, $planetClosure);
+	}
+
+	/**
+	 * @expectedException \OutOfBoundsException
+	 */
+	public function testResolveDisplayNameUnregisteredType() {
+		$manager = $this->getManager();
+
+		$planetClosure = function($name) {
+			return ucfirst($name);
+		};
+
+		$manager->registerDisplayNameResolver('planet', $planetClosure);
+		$manager->resolveDisplayName('galaxy', 'sombrero');
+	}
+
+	public function testResolveDisplayNameDirtyResolver() {
+		$manager = $this->getManager();
+
+		$planetClosure = function() { return null; };
+
+		$manager->registerDisplayNameResolver('planet', $planetClosure);
+		$this->assertTrue(is_string($manager->resolveDisplayName('planet', 'neptune')));
+	}
+
+	/**
+	 * @expectedException \InvalidArgumentException
+	 */
+	public function testResolveDisplayNameInvalidType() {
+		$manager = $this->getManager();
+
+		$planetClosure = function() { return null; };
+
+		$manager->registerDisplayNameResolver('planet', $planetClosure);
+		$this->assertTrue(is_string($manager->resolveDisplayName(1337, 'neptune')));
+	}
+
 }