diff --git a/apps/comments/appinfo/app.php b/apps/comments/appinfo/app.php index 771b35d9c6..66e60dbbd8 100644 --- a/apps/comments/appinfo/app.php +++ b/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; +}); diff --git a/apps/comments/css/comments.css b/apps/comments/css/comments.css index 667f32871b..796a550227 100644 --- a/apps/comments/css/comments.css +++ b/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; diff --git a/apps/comments/js/commentmodel.js b/apps/comments/js/commentmodel.js index 89492707b6..e75c79b3f0 100644 --- a/apps/comments/js/commentmodel.js +++ b/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; } }); diff --git a/apps/comments/js/commentstabview.js b/apps/comments/js/commentstabview.js index fe3695569b..8387e527f4 100644 --- a/apps/comments/js/commentstabview.js +++ b/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
tags. */ - _formatMessage: function(message) { - return escapeHTML(message).replace(/\n/g, '
'); + _formatMessage: function(message, mentions) { + message = escapeHTML(message).replace(/\n/g, '
'); + + for(var i in mentions) { + var mention = '@' + mentions[i].mentionId; + + var avatar = ''; + if(this._avatarsEnabled) { + avatar = '
'; + } + + // escape possible regex characters in the name + mention = mention.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + var displayName = avatar + ' '+ _.escape(mentions[i].mentionDisplayName)+''; + + // 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'); - $submit.removeClass('hidden'); - $loading.addClass('hidden'); - $row.data('commentEl') - .removeClass('hidden') - .find('.message') - .html(self._formatMessage(model.get('message'))); - $row.remove(); + self._onSubmitSuccess(model, $form, commentId); }, error: function() { - $submit.removeClass('hidden'); - $loading.addClass('hidden'); - $textArea.prop('disabled', false); - - OC.Notification.showTemporary(t('comments', 'Error occurred while updating comment with id {id}', {id: commentId})); + self._onSubmitError($form, commentId); } }); } else { @@ -414,17 +472,11 @@ at: 0, // wait for real creation before adding wait: true, - success: function() { - $submit.removeClass('hidden'); - $loading.addClass('hidden'); - $textArea.val('').prop('disabled', false); + success: function(model) { + self._onSubmitSuccess(model, $form); }, error: function() { - $submit.removeClass('hidden'); - $loading.addClass('hidden'); - $textArea.prop('disabled', false); - - OC.Notification.showTemporary(t('comments', 'Error occurred while posting comment')); + self._onSubmitError($form); } }); } @@ -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 diff --git a/apps/comments/lib/Notification/Listener.php b/apps/comments/lib/Notification/Listener.php index 426e85cac8..d30c59c93d 100644 --- a/apps/comments/lib/Notification/Listener.php +++ b/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) { - $user = substr($mention, 1); // @username → username - if( ($comment->getActorType() === 'users' && $user === $comment->getActorId()) - || !$this->userManager->userExists($user) + foreach($mentions as $uid) { + if( ($comment->getActorType() === 'users' && $uid === $comment->getActorId()) + || !$this->userManager->userExists($uid) ) { // 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 - * @return string[] containing the mentions, e.g. ['@alice', '@bob'] + * @param array $mentions + * @return string[] containing the mentions, e.g. ['alice', 'bob'] */ - public function extractMentions($message) { - $ok = preg_match_all('/\B@[a-z0-9_\-@\.\']+/i', $message, $mentions); - if(!$ok || !isset($mentions[0]) || !is_array($mentions[0])) { + public function extractMentions(array $mentions) { + if(empty($mentions)) { return []; } - return array_unique($mentions[0]); + $uids = []; + foreach($mentions as $mention) { + if($mention['type'] === 'user') { + $uids[] = $mention['id']; + } + } + return $uids; } } diff --git a/apps/comments/tests/Unit/Notification/ListenerTest.php b/apps/comments/tests/Unit/Notification/ListenerTest.php index 12f388fcff..3007b78cb3 100644 --- a/apps/comments/tests/Unit/Notification/ListenerTest.php +++ b/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') - ->will($this->returnValue($message)); + ->method('getMentions') + ->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') - ->will($this->returnValue($message)); + ->method('getMentions') + ->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') - ->will($this->returnValue($message)); + ->method('getMentions') + ->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); - } - } diff --git a/apps/comments/tests/js/commentstabviewSpec.js b/apps/comments/tests/js/commentstabviewSpec.js index 470ff0d221..9e4bf4f053 100644 --- a/apps/comments/tests/js/commentstabviewSpec.js +++ b/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({ - id: 3, + var comment4 = new OCA.Comments.CommentModel({ + 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 diff --git a/apps/dav/lib/Comments/CommentNode.php b/apps/dav/lib/Comments/CommentNode.php index f247921be7..1fa8e057b9 100644 --- a/apps/dav/lib/Comments/CommentNode.php +++ b/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()); + } } diff --git a/apps/dav/tests/unit/Comments/CommentsNodeTest.php b/apps/dav/tests/unit/Comments/CommentsNodeTest.php index 1c7bd78249..94eaea01d5 100644 --- a/apps/dav/tests/unit/Comments/CommentsNodeTest.php +++ b/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)); diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index f6f0801c68..b5f063be32 100644 --- a/lib/private/Comments/Comment.php +++ b/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 * diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index b3ecab731e..001f4f9441 100644 --- a/lib/private/Comments/Manager.php +++ b/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 * diff --git a/lib/public/Comments/IComment.php b/lib/public/Comments/IComment.php index bb997a0722..8210d4c8c7 100644 --- a/lib/public/Comments/IComment.php +++ b/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 * diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index 98169fb335..6a32cfd803 100644 --- a/lib/public/Comments/ICommentsManager.php +++ b/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); + } diff --git a/tests/lib/Comments/CommentTest.php b/tests/lib/Comments/CommentTest.php index ea10c52ae1..10ec4bae7d 100644 --- a/tests/lib/Comments/CommentTest.php +++ b/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); + } + } diff --git a/tests/lib/Comments/FakeManager.php b/tests/lib/Comments/FakeManager.php index 7cd146e7cb..dfb8f21b64 100644 --- a/tests/lib/Comments/FakeManager.php +++ b/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) {} } diff --git a/tests/lib/Comments/ManagerTest.php b/tests/lib/Comments/ManagerTest.php index 5bacc794ba..a320366f29 100644 --- a/tests/lib/Comments/ManagerTest.php +++ b/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'))); + } + }