diff options
24 files changed, 604 insertions, 198 deletions
diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index d2315946ee7..75d64667761 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -19,7 +19,7 @@ If you have questions about how to install or use Nextcloud, please direct these Help us to maximize the effort we can spend fixing issues and adding new features, by not reporting duplicate issues. -[template]: https://raw.github.com/nextcloud/core/master/issue_template.md +[template]: https://raw.githubusercontent.com/nextcloud/server/master/.github/issue_template.md [forum]: https://help.nextcloud.com/ [irc]: https://webchat.freenode.net/?channels=nextcloud diff --git a/.gitignore b/.gitignore index c3f97ff504d..f25fb52ce2d 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ /config/mount.php /apps/inc.php /assets +/.htaccess # ignore all apps except core ones /apps*/* diff --git a/apps/comments/appinfo/app.php b/apps/comments/appinfo/app.php index 771b35d9c6a..66e60dbbd85 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 667f32871bb..796a550227b 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 89492707b61..e75c79b3f08 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 fe3695569bf..8387e527f4a 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 <br> tags. */ - _formatMessage: function(message) { - return escapeHTML(message).replace(/\n/g, '<br/>'); + _formatMessage: function(message, mentions) { + 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'); - $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); } }); } @@ -433,6 +485,26 @@ }, /** + * 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 426e85cac83..d30c59c93d5 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 12f388fcff9..3007b78cb3d 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 470ff0d2217..9e4bf4f0533 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 f247921be79..1fa8e057b99 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 1c7bd782496..94eaea01d56 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'])); @@ -405,6 +428,13 @@ class CommentsNodeTest extends \Test\TestCase { ->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/apps/files/js/file-upload.js b/apps/files/js/file-upload.js index 30784528700..8fec7d5c04e 100644 --- a/apps/files/js/file-upload.js +++ b/apps/files/js/file-upload.js @@ -1071,6 +1071,7 @@ OC.Uploader.prototype = _.extend({ self.clear(); self._hideProgressBar(); + self.trigger('stop', e, data); }); fileupload.on('fileuploadfail', function(e, data) { self.log('progress handle fileuploadfail', e, data); diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index bf4fd75d4cc..18534db3ee9 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -2810,8 +2810,8 @@ $.when.apply($, promises).then(function() { // highlight uploaded files self.highlightFiles(fileNames); + self.updateStorageStatistics(); }); - self.updateStorageStatistics(); var uploadText = self.$fileList.find('tr .uploadtext'); self.showFileBusyState(uploadText.closest('tr'), false); diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js index 3b0e0b83b82..15dab3b9882 100644 --- a/apps/files/tests/js/filelistSpec.js +++ b/apps/files/tests/js/filelistSpec.js @@ -2794,13 +2794,22 @@ describe('OCA.Files.FileList tests', function() { highlightStub.restore(); }); - it('queries storage stats', function() { + it('queries storage stats after all fetches are done', function() { var statStub = sinon.stub(fileList, 'updateStorageStatistics'); - addFile(createUpload('upload.txt', '/subdir')); - expect(statStub.notCalled).toEqual(true); + var highlightStub = sinon.stub(fileList, 'highlightFiles'); + var def1 = addFile(createUpload('upload.txt', '/subdir')); + var def2 = addFile(createUpload('upload2.txt', '/subdir')); + var def3 = addFile(createUpload('upload3.txt', '/another')); uploader.trigger('stop', {}); + + expect(statStub.notCalled).toEqual(true); + def1.resolve(); + expect(statStub.notCalled).toEqual(true); + def2.resolve(); + def3.resolve(); expect(statStub.calledOnce).toEqual(true); - statStub.restore(); + + highlightStub.restore(); }); }); }); diff --git a/lib/base.php b/lib/base.php index 883c1f54b17..e7bedb69596 100644 --- a/lib/base.php +++ b/lib/base.php @@ -217,12 +217,7 @@ class OC { // set the right include path set_include_path( - OC::$SERVERROOT . '/lib/private' . PATH_SEPARATOR . - self::$configDir . PATH_SEPARATOR . - OC::$SERVERROOT . '/3rdparty' . PATH_SEPARATOR . - implode(PATH_SEPARATOR, $paths) . PATH_SEPARATOR . - get_include_path() . PATH_SEPARATOR . - OC::$SERVERROOT + implode(PATH_SEPARATOR, $paths) ); } diff --git a/lib/private/Comments/Comment.php b/lib/private/Comments/Comment.php index f6f0801c683..b5f063be323 100644 --- a/lib/private/Comments/Comment.php +++ b/lib/private/Comments/Comment.php @@ -204,6 +204,43 @@ class Comment implements IComment { } /** + * 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 * * @return string diff --git a/lib/private/Comments/Manager.php b/lib/private/Comments/Manager.php index b3ecab731e1..001f4f9441c 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. * @@ -760,6 +763,50 @@ class Manager implements ICommentsManager { } /** + * 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 * * @return \OCP\Comments\ICommentsEventHandler[] diff --git a/lib/public/Comments/IComment.php b/lib/public/Comments/IComment.php index bb997a07223..8210d4c8c7e 100644 --- a/lib/public/Comments/IComment.php +++ b/lib/public/Comments/IComment.php @@ -133,6 +133,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 * * @return string diff --git a/lib/public/Comments/ICommentsManager.php b/lib/public/Comments/ICommentsManager.php index 98169fb335f..6a32cfd803d 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/settings/Controller/ChangePasswordController.php b/settings/Controller/ChangePasswordController.php index f709a8dd431..e43d0d8f343 100644 --- a/settings/Controller/ChangePasswordController.php +++ b/settings/Controller/ChangePasswordController.php @@ -84,6 +84,7 @@ class ChangePasswordController extends Controller { /** * @NoAdminRequired + * @NoSubadminRequired * * @param string $oldpassword * @param string $newpassword diff --git a/settings/css/settings.css b/settings/css/settings.css index 2f0f4b23515..0dadf401c04 100644 --- a/settings/css/settings.css +++ b/settings/css/settings.css @@ -435,12 +435,22 @@ span.version { #apps-list { position: relative; height: 100%; + display: flex; + flex-wrap: wrap; + align-content: flex-start; } -.section { +#apps-list .section { position: relative; + flex: 1 0 330px; + margin: 0; + padding-right: 50px; +} +#apps-list .section.apps-experimental { + flex-basis: 90%; } .section h2.app-name { margin-bottom: 8px; + display: inline; } .followupsection { display: block; diff --git a/tests/lib/Comments/CommentTest.php b/tests/lib/Comments/CommentTest.php index ea10c52ae17..10ec4bae7d5 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 7cd146e7cb2..dfb8f21b64b 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 5bacc794ba7..a320366f29e 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'))); + } + } |