From dcfd7bf7fffb6b7c946011870f1ae8a292937ddd Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sun, 19 Nov 2017 16:37:37 +0100 Subject: fix avatars in file rows of outgoing shares Signed-off-by: Arthur Schiwon --- core/js/share.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'core/js') diff --git a/core/js/share.js b/core/js/share.js index 25d59b46fb4..44f4f12f833 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -197,6 +197,12 @@ OC.Share = _.extend(OC.Share || {}, { delete OC.Share.statuses[itemSource]; } }, + _formatRegularShare: function(shareWith, shareWithDisplayName, message) { + // display avatar of the user + var avatar = ''; + var hidden = '' + message + ' ' + escapeHTML(shareWithDisplayName) + ' '; + return avatar + hidden; + }, /** * Format a remote address * @@ -207,7 +213,7 @@ OC.Share = _.extend(OC.Share || {}, { var parts = this._REMOTE_OWNER_REGEXP.exec(remoteAddress); if (!parts) { // display avatar of the user - var avatar = ''; + var avatar = ''; var hidden = '' + message + ' ' + escapeHTML(remoteAddress) + ' '; return avatar + hidden; } @@ -243,9 +249,8 @@ OC.Share = _.extend(OC.Share || {}, { */ _formatShareList: function(recipients) { var _parent = this; - return $.map(recipients, function(recipient) { - recipient = _parent._formatRemoteShare(recipient, t('core', 'Shared with')); - return recipient; + return $.map(recipients, function(shareWithDisplayName, shareWith) { + return _parent._formatRegularShare(shareWith, shareWithDisplayName, t('core', 'Shared with')); }); }, /** @@ -291,7 +296,7 @@ OC.Share = _.extend(OC.Share || {}, { } // update share action text / icon if (hasShares || owner) { - recipients = $tr.attr('data-share-recipients'); + recipients = $tr.data('share-recipient-data'); action.addClass('shared-style'); avatars = '' + t('core', 'Shared') + ''; @@ -300,13 +305,15 @@ OC.Share = _.extend(OC.Share || {}, { message = t('core', 'Shared by'); avatars = this._formatRemoteShare(owner, message); } else if (recipients) { - avatars = this._formatShareList(recipients.split(', ')).join(''); + avatars = this._formatShareList(recipients); } action.html(avatars).prepend(icon); if (owner || recipients) { var avatarElement = action.find('.avatar'); - avatarElement.avatar(avatarElement.data('username'), 32); + avatarElement.each(function () { + $(this).avatar($(this).data('username'), 32); + }); action.find('.icon-shared + span').tooltip({placement: 'top'}); } -- cgit v1.2.3 From 4247936dd6d5ea2fa60586e0f4aa4acbc7d902f1 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sun, 19 Nov 2017 16:38:04 +0100 Subject: Fix avatars in file rows of incoming shares Signed-off-by: Arthur Schiwon --- apps/files_sharing/js/share.js | 1 + apps/files_sharing/js/sharedfilelist.js | 1 + core/js/share.js | 11 ++++++----- 3 files changed, 8 insertions(+), 5 deletions(-) (limited to 'core/js') diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js index dbfce6e83b5..a7eefe43dbe 100644 --- a/apps/files_sharing/js/share.js +++ b/apps/files_sharing/js/share.js @@ -46,6 +46,7 @@ tr.attr('data-share-permissions', sharePermissions); if (fileData.shareOwner) { tr.attr('data-share-owner', fileData.shareOwner); + tr.attr('data-share-owner-id', fileData.shareOwnerId); // user should always be able to rename a mount point if (fileData.mountType === 'shared-root') { tr.attr('data-permissions', fileData.permissions | OC.PERMISSION_UPDATE); diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index b32ee97f716..c7b872772b3 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -297,6 +297,7 @@ }; if (self._sharedWithUser) { file.shareOwner = share.displayname_owner; + file.shareOwnerId = share.uid_owner; file.name = OC.basename(share.file_target); file.path = OC.dirname(share.file_target); file.permissions = share.permissions; diff --git a/core/js/share.js b/core/js/share.js index 44f4f12f833..281c414b900 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -266,12 +266,13 @@ OC.Share = _.extend(OC.Share || {}, { var type = $tr.data('type'); var icon = action.find('.icon'); var message, recipients, avatars; + var ownerId = $tr.attr('data-share-owner-id'); var owner = $tr.attr('data-share-owner'); var shareFolderIcon; var iconClass = 'icon-shared'; action.removeClass('shared-style'); // update folder icon - if (type === 'dir' && (hasShares || hasLink || owner)) { + if (type === 'dir' && (hasShares || hasLink || ownerId)) { if (hasLink) { shareFolderIcon = OC.MimeType.getIconUrl('dir-public'); } @@ -295,21 +296,21 @@ OC.Share = _.extend(OC.Share || {}, { $tr.find('.filename .thumbnail').css('background-image', 'url(' + shareFolderIcon + ')'); } // update share action text / icon - if (hasShares || owner) { + if (hasShares || ownerId) { recipients = $tr.data('share-recipient-data'); action.addClass('shared-style'); avatars = '' + t('core', 'Shared') + ''; // even if reshared, only show "Shared by" - if (owner) { + if (ownerId) { message = t('core', 'Shared by'); - avatars = this._formatRemoteShare(owner, message); + avatars = this._formatRegularShare(ownerId, owner, message); } else if (recipients) { avatars = this._formatShareList(recipients); } action.html(avatars).prepend(icon); - if (owner || recipients) { + if (ownerId || recipients) { var avatarElement = action.find('.avatar'); avatarElement.each(function () { $(this).avatar($(this).data('username'), 32); -- cgit v1.2.3 From 3a1d8fa45f22503ba9841d71b7ea630d400b2ae5 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sun, 19 Nov 2017 22:41:28 +0100 Subject: adjust, fix and extend tests Signed-off-by: Arthur Schiwon --- core/js/share.js | 22 +++++++---------- core/js/tests/specs/shareSpec.js | 51 ++++++++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 25 deletions(-) (limited to 'core/js') diff --git a/core/js/share.js b/core/js/share.js index 281c414b900..86954b2ced2 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -197,24 +197,20 @@ OC.Share = _.extend(OC.Share || {}, { delete OC.Share.statuses[itemSource]; } }, - _formatRegularShare: function(shareWith, shareWithDisplayName, message) { - // display avatar of the user - var avatar = ''; - var hidden = '' + message + ' ' + escapeHTML(shareWithDisplayName) + ' '; - return avatar + hidden; - }, /** * Format a remote address * - * @param {String} remoteAddress full remote share + * @param {String} shareWith userid, full remote share, or whatever + * @param {String} shareWithDisplayName + * @param {String} message * @return {String} HTML code to display */ - _formatRemoteShare: function(remoteAddress, message) { - var parts = this._REMOTE_OWNER_REGEXP.exec(remoteAddress); + _formatRemoteShare: function(shareWith, shareWithDisplayName, message) { + var parts = this._REMOTE_OWNER_REGEXP.exec(shareWith); if (!parts) { // display avatar of the user - var avatar = ''; - var hidden = '' + message + ' ' + escapeHTML(remoteAddress) + ' '; + var avatar = ''; + var hidden = '' + message + ' ' + escapeHTML(shareWithDisplayName) + ' '; return avatar + hidden; } @@ -250,7 +246,7 @@ OC.Share = _.extend(OC.Share || {}, { _formatShareList: function(recipients) { var _parent = this; return $.map(recipients, function(shareWithDisplayName, shareWith) { - return _parent._formatRegularShare(shareWith, shareWithDisplayName, t('core', 'Shared with')); + return _parent._formatRemoteShare(shareWith, shareWithDisplayName, t('core', 'Shared with')); }); }, /** @@ -304,7 +300,7 @@ OC.Share = _.extend(OC.Share || {}, { // even if reshared, only show "Shared by" if (ownerId) { message = t('core', 'Shared by'); - avatars = this._formatRegularShare(ownerId, owner, message); + avatars = this._formatRemoteShare(ownerId, owner, message); } else if (recipients) { avatars = this._formatShareList(recipients); } diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js index 70c698c99a2..19e9a92ca9e 100644 --- a/core/js/tests/specs/shareSpec.js +++ b/core/js/tests/specs/shareSpec.js @@ -45,6 +45,7 @@ describe('OC.Share tests', function() { var $action; $file.attr('data-share-owner', input); + $file.attr('data-share-owner-id', input); OC.Share.markFileAsShared($file); $action = $file.find('.action-share>span').parent(); @@ -119,6 +120,7 @@ describe('OC.Share tests', function() { it('shows a shared folder icon for folders shared with the current user', function() { $file.attr('data-type', 'dir'); $file.attr('data-share-owner', 'someoneelse'); + $file.attr('data-share-owner-id', 'someoneelse'); OC.Share.markFileAsShared($file); checkIcon('filetypes/folder-shared'); @@ -155,7 +157,9 @@ describe('OC.Share tests', function() { function checkRecipients(input, output, title) { var $action; - $file.attr('data-share-recipients', input); + var concatenated = _.values(input).join(', '); + $file.attr('data-share-recipients', concatenated); + $file.attr('data-share-recipient-data', JSON.stringify(input)); OC.Share.markFileAsShared($file, true); $action = $file.find('.action-share>span').parent(); @@ -177,66 +181,89 @@ describe('OC.Share tests', function() { } it('displays the local share owner as is', function() { - checkRecipients('User One', 'Shared with User One', null); + checkRecipients({'User One': 'User One'}, 'Shared with User One', null); }); it('displays the user name part of a remote recipient', function() { checkRecipients( - 'User One@someserver.com', + {'User One@someserver.com': 'User One@someserver.com'}, 'User One@…', 'Shared with User One@someserver.com' ); checkRecipients( - 'User One@someserver.com/', + '{User One@someserver.com/: User One@someserver.com/}', 'User One@…', 'Shared with User One@someserver.com' ); checkRecipients( - 'User One@someserver.com/root/of/owncloud', + {'User One@someserver.com/root/of/owncloud': 'User One@someserver.com/root/of/owncloud'}, 'User One@…', 'Shared with User One@someserver.com' ); }); it('displays the user name part with domain of a remote share owner', function() { checkRecipients( - 'User One@example.com@someserver.com', + {'User One@example.com@someserver.com': 'User One@example.com@someserver.com'}, 'User One@example.com', 'Shared with User One@example.com@someserver.com' ); checkRecipients( - 'User One@example.com@someserver.com/', + {'User One@example.com@someserver.com/': 'User One@example.com@someserver.com/'}, 'User One@example.com', 'Shared with User One@example.com@someserver.com' ); checkRecipients( - 'User One@example.com@someserver.com/root/of/owncloud', + {'User One@example.com@someserver.com/root/of/nextcloud': 'User One@example.com@someserver.com/root/of/nextcloud'}, 'User One@example.com', 'Shared with User One@example.com@someserver.com' ); }); it('display multiple remote recipients', function() { checkRecipients( - 'One@someserver.com, two@otherserver.com', + { + 'One@someserver.com': 'One@someserver.com', + 'two@otherserver.com': 'two@otherserver.com' + }, 'One@… two@…', ['Shared with One@someserver.com', 'Shared with two@otherserver.com'] ); checkRecipients( - 'One@someserver.com/, two@otherserver.com', + { + 'One@someserver.com/': 'One@someserver.com/', + 'two@otherserver.com': 'two@otherserver.com' + }, 'One@… two@…', ['Shared with One@someserver.com', 'Shared with two@otherserver.com'] ); checkRecipients( - 'One@someserver.com/root/of/owncloud, two@otherserver.com', + { + 'One@someserver.com/root/of/owncloud': 'One@someserver.com/root/of/owncloud', + 'two@otherserver.com': 'two@otherserver.com' + }, 'One@… two@…', ['Shared with One@someserver.com', 'Shared with two@otherserver.com'] ); }); it('display mixed recipients', function() { checkRecipients( - 'One, two@otherserver.com', + { + 'One': 'One', + 'two@otherserver.com': 'two@otherserver.com' + }, 'Shared with One two@…', ['Shared with two@otherserver.com'] ); }); + it('display multiple with divergent displaynames', function() { + checkRecipients( + { + 'One': 'Yoko Ono', + 'two@otherserver.com': 'two@otherserver.com', + 'Three': 'Green, Mina' + }, + 'Shared with Yoko Ono two@… Shared with Green, Mina', + ['Shared with two@otherserver.com'] + ); + }); }); }); }); -- cgit v1.2.3 From 9d95391ff14a1c753e8fda4ecaacc007572552f4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 21 Nov 2017 11:29:42 +0100 Subject: adjust tests and apply sorting Signed-off-by: Arthur Schiwon --- apps/files_sharing/js/share.js | 5 ++ apps/files_sharing/js/sharedfilelist.js | 5 +- apps/files_sharing/tests/js/shareSpec.js | 32 +++++--- core/js/share.js | 7 +- core/js/tests/specs/shareSpec.js | 132 +++++++++++++++++++++++++------ 5 files changed, 144 insertions(+), 37 deletions(-) (limited to 'core/js') diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js index a7eefe43dbe..9b794ca2d13 100644 --- a/apps/files_sharing/js/share.js +++ b/apps/files_sharing/js/share.js @@ -220,9 +220,14 @@ // note: we only update the data attribute because updateIcon() if (recipients.length) { $tr.attr('data-share-recipients', OCA.Sharing.Util.formatRecipients(recipients)); + var recipientData = _.mapObject(shareModel.get('shares'), function (share) { + return {shareWith: share.share_with, shareWithDisplayName: share.share_with_displayname}; + }); + $tr.attr('data-share-recipient-data', JSON.stringify(recipientData)); } else { $tr.removeAttr('data-share-recipients'); + $tr.removeAttr('data-share-recipient-data'); } }, diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index c7b872772b3..fb4909e764e 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -355,7 +355,10 @@ // only store the first ones, they will be the only ones // displayed data.recipients[recipient] = true; - data.recipientData[recipientId] = recipient; + data.recipientData[data.recipientsCount] = { + 'shareWith': recipientId, + 'shareWithDisplayName': recipient + }; } data.recipientsCount++; } diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js index 5b0a78c9c64..1b628f5bb0f 100644 --- a/apps/files_sharing/tests/js/shareSpec.js +++ b/apps/files_sharing/tests/js/shareSpec.js @@ -140,6 +140,7 @@ describe('OCA.Sharing.Util tests', function() { size: 12, permissions: OC.PERMISSION_ALL, shareOwner: 'User One', + shareOwnerId: 'User One', etag: 'abc', shareTypes: [] }]); @@ -161,6 +162,16 @@ describe('OCA.Sharing.Util tests', function() { size: 12, permissions: OC.PERMISSION_ALL, recipientsDisplayName: 'User One, User Two', + recipientData: { + 0: { + shareWith: 'User One', + shareWithDisplayName: 'User One' + }, + 1: { + shareWith: 'User Two', + shareWithDisplayName: 'User Two' + } + }, etag: 'abc', shareTypes: [OC.Share.SHARE_TYPE_USER] }]); @@ -264,10 +275,10 @@ describe('OCA.Sharing.Util tests', function() { // simulate updating shares shareTab._dialog.model.set({ shares: [ - {share_with_displayname: 'User One'}, - {share_with_displayname: 'User Two'}, - {share_with_displayname: 'Group One'}, - {share_with_displayname: 'Group Two'} + {share_with_displayname: 'User One', share_with: 'User One'}, + {share_with_displayname: 'User Two', share_with: 'User Two'}, + {share_with_displayname: 'Group One', share_with: 'Group One'}, + {share_with_displayname: 'Group Two', share_with: 'Group Two'} ] }); @@ -298,9 +309,9 @@ describe('OCA.Sharing.Util tests', function() { // simulate updating shares shareTab._dialog.model.set({ shares: [ - {share_with_displayname: 'User One'}, - {share_with_displayname: 'User Two'}, - {share_with_displayname: 'User Three'} + {share_with_displayname: 'User One', share_with: 'User One'}, + {share_with_displayname: 'User Two', share_with: 'User Two'}, + {share_with_displayname: 'User Three', share_with: 'User Three'} ] }); @@ -348,7 +359,8 @@ describe('OCA.Sharing.Util tests', function() { size: 12, permissions: OC.PERMISSION_ALL, etag: 'abc', - shareOwner: 'User One' + shareOwner: 'User One', + shareOwnerId: 'User One' }]); $action = fileList.$el.find('tbody tr:first .action-share'); $tr = fileList.$el.find('tr:first'); @@ -379,7 +391,9 @@ describe('OCA.Sharing.Util tests', function() { permissions: OC.PERMISSION_ALL, etag: 'abc', shareOwner: 'User One', - recipients: 'User Two' + shareOwnerId: 'User One', + recipients: 'User Two', + recipientData: {'User Two': 'User Two'} }]); $action = fileList.$el.find('tbody tr:first .action-share'); $tr = fileList.$el.find('tr:first'); diff --git a/core/js/share.js b/core/js/share.js index 86954b2ced2..381c42c5de2 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -240,13 +240,14 @@ OC.Share = _.extend(OC.Share || {}, { * Loop over all recipients in the list and format them using * all kind of fancy magic. * - * @param {String[]} recipients array of all the recipients + * @param {Object} recipients array of all the recipients * @return {String[]} modified list of recipients */ _formatShareList: function(recipients) { var _parent = this; - return $.map(recipients, function(shareWithDisplayName, shareWith) { - return _parent._formatRemoteShare(shareWith, shareWithDisplayName, t('core', 'Shared with')); + recipients = _.sortBy(_.toArray(recipients), 'shareWithDisplayName'); + return $.map(recipients, function(recipient) { + return _parent._formatRemoteShare(recipient.shareWith, recipient.shareWithDisplayName, t('core', 'Shared with')); }); }, /** diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js index 19e9a92ca9e..8688705b8e8 100644 --- a/core/js/tests/specs/shareSpec.js +++ b/core/js/tests/specs/shareSpec.js @@ -181,64 +181,133 @@ describe('OC.Share tests', function() { } it('displays the local share owner as is', function() { - checkRecipients({'User One': 'User One'}, 'Shared with User One', null); + var input = { + 0: { + shareWith: 'User One', + shareWithDisplayName: 'User One' + } + }; + checkRecipients(input, 'Shared with User One', null); }); it('displays the user name part of a remote recipient', function() { + var input = { + 0: { + shareWith: 'User One@someserver.com', + shareWithDisplayName: 'User One@someserver.com' + } + }; checkRecipients( - {'User One@someserver.com': 'User One@someserver.com'}, + input, 'User One@…', 'Shared with User One@someserver.com' ); + + input = { + 0: { + shareWith: 'User One@someserver.com/', + shareWithDisplayName: 'User One@someserver.com/' + } + }; checkRecipients( - '{User One@someserver.com/: User One@someserver.com/}', + input, 'User One@…', 'Shared with User One@someserver.com' ); + + input = { + 0: { + shareWith: 'User One@someserver.com/root/of/nextcloud', + shareWithDisplayName: 'User One@someserver.com/root/of/nextcloud' + } + }; checkRecipients( - {'User One@someserver.com/root/of/owncloud': 'User One@someserver.com/root/of/owncloud'}, + input, 'User One@…', 'Shared with User One@someserver.com' ); }); it('displays the user name part with domain of a remote share owner', function() { + var input = { + 0: { + shareWith: 'User One@example.com@someserver.com', + shareWithDisplayName: 'User One@example.com@someserver.com' + } + }; checkRecipients( - {'User One@example.com@someserver.com': 'User One@example.com@someserver.com'}, + input, 'User One@example.com', 'Shared with User One@example.com@someserver.com' ); + + input = { + 0: { + shareWith: 'User One@example.com@someserver.com/', + shareWithDisplayName: 'User One@example.com@someserver.com/' + } + }; checkRecipients( - {'User One@example.com@someserver.com/': 'User One@example.com@someserver.com/'}, + input, 'User One@example.com', 'Shared with User One@example.com@someserver.com' ); + + input = { + 0: { + shareWith: 'User One@example.com@someserver.com/root/of/nextcloud', + shareWithDisplayName: 'User One@example.com@someserver.com/root/of/nextcloud' + } + }; checkRecipients( - {'User One@example.com@someserver.com/root/of/nextcloud': 'User One@example.com@someserver.com/root/of/nextcloud'}, + input, 'User One@example.com', 'Shared with User One@example.com@someserver.com' ); }); it('display multiple remote recipients', function() { - checkRecipients( - { - 'One@someserver.com': 'One@someserver.com', - 'two@otherserver.com': 'two@otherserver.com' + var input = { + 0: { + shareWith: 'One@someserver.com', + shareWithDisplayName: 'One@someserver.com' }, + 1: { + shareWith: 'two@someserver.com', + shareWithDisplayName: 'two@someserver.com' + } + }; + checkRecipients( + input, 'One@… two@…', ['Shared with One@someserver.com', 'Shared with two@otherserver.com'] ); - checkRecipients( - { - 'One@someserver.com/': 'One@someserver.com/', - 'two@otherserver.com': 'two@otherserver.com' + + input = { + 0: { + shareWith: 'One@someserver.com/', + shareWithDisplayName: 'One@someserver.com/' }, + 1: { + shareWith: 'two@someserver.com', + shareWithDisplayName: 'two@someserver.com' + } + }; + checkRecipients( + input, 'One@… two@…', ['Shared with One@someserver.com', 'Shared with two@otherserver.com'] ); - checkRecipients( - { - 'One@someserver.com/root/of/owncloud': 'One@someserver.com/root/of/owncloud', - 'two@otherserver.com': 'two@otherserver.com' + + input = { + 0: { + shareWith: 'One@someserver.com/root/of/nextcloud', + shareWithDisplayName: 'One@someserver.com/root/of/nextcloud' }, + 1: { + shareWith: 'two@someserver.com', + shareWithDisplayName: 'two@someserver.com' + } + }; + checkRecipients( + input, 'One@… two@…', ['Shared with One@someserver.com', 'Shared with two@otherserver.com'] ); @@ -246,8 +315,14 @@ describe('OC.Share tests', function() { it('display mixed recipients', function() { checkRecipients( { - 'One': 'One', - 'two@otherserver.com': 'two@otherserver.com' + 0: { + shareWith: 'One', + shareWithDisplayName: 'One' + }, + 1: { + shareWith: 'two@someserver.com', + shareWithDisplayName: 'two@someserver.com' + } }, 'Shared with One two@…', ['Shared with two@otherserver.com'] @@ -256,9 +331,18 @@ describe('OC.Share tests', function() { it('display multiple with divergent displaynames', function() { checkRecipients( { - 'One': 'Yoko Ono', - 'two@otherserver.com': 'two@otherserver.com', - 'Three': 'Green, Mina' + 0: { + shareWith: 'One', + shareWithDisplayName: 'Yoko Ono' + }, + 1: { + shareWith: 'two@someserver.com', + shareWithDisplayName: 'two@someserver.com' + }, + 2: { + shareWith: 'Three', + shareWithDisplayName: 'Green, Mina' + } }, 'Shared with Yoko Ono two@… Shared with Green, Mina', ['Shared with two@otherserver.com'] -- cgit v1.2.3 From 077381c7b3aa220253a18ce3bbb946e4a46d8213 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 21 Nov 2017 12:45:57 +0100 Subject: rip out obsolete recipientsDisplayName also needs tests adjustements, and this also brings in natural sorting Signed-off-by: Arthur Schiwon --- apps/files_sharing/js/share.js | 40 ++-------------- apps/files_sharing/js/sharedfilelist.js | 17 ++++--- apps/files_sharing/tests/js/shareSpec.js | 57 +---------------------- apps/files_sharing/tests/js/sharedfilelistSpec.js | 4 +- core/js/share.js | 5 +- core/js/tests/specs/shareSpec.js | 16 +++---- 6 files changed, 31 insertions(+), 108 deletions(-) (limited to 'core/js') diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js index 9b794ca2d13..aa0803c491b 100644 --- a/apps/files_sharing/js/share.js +++ b/apps/files_sharing/js/share.js @@ -52,8 +52,7 @@ tr.attr('data-permissions', fileData.permissions | OC.PERMISSION_UPDATE); } } - if (fileData.recipientsDisplayName) { - tr.attr('data-share-recipients', fileData.recipientsDisplayName); + if (fileData.recipientData && !_.isEmpty(fileData.recipientData)) { tr.attr('data-share-recipient-data', JSON.stringify(fileData.recipientData)); } if (fileData.shareTypes) { @@ -78,8 +77,6 @@ fileInfo.shares.push({expiration: expirationTimestamp}); } - fileInfo.recipientsDisplayName = $el.attr('data-share-recipients') || undefined; - return fileInfo; }; @@ -219,14 +216,12 @@ var recipients = _.pluck(shareModel.get('shares'), 'share_with_displayname'); // note: we only update the data attribute because updateIcon() if (recipients.length) { - $tr.attr('data-share-recipients', OCA.Sharing.Util.formatRecipients(recipients)); var recipientData = _.mapObject(shareModel.get('shares'), function (share) { return {shareWith: share.share_with, shareWithDisplayName: share.share_with_displayname}; }); $tr.attr('data-share-recipient-data', JSON.stringify(recipientData)); } else { - $tr.removeAttr('data-share-recipients'); $tr.removeAttr('data-share-recipient-data'); } }, @@ -235,46 +230,21 @@ * Update the file action share icon for the given file * * @param $tr file element of the file to update - * @param {bool} hasUserShares true if a user share exists - * @param {bool} hasLinkShare true if a link share exists + * @param {boolean} hasUserShares true if a user share exists + * @param {boolean} hasLinkShare true if a link share exists * - * @return {bool} true if the icon was set, false otherwise + * @return {boolean} true if the icon was set, false otherwise */ _updateFileActionIcon: function($tr, hasUserShares, hasLinkShare) { // if the statuses are loaded already, use them for the icon // (needed when scrolling to the next page) - if (hasUserShares || hasLinkShare || $tr.attr('data-share-recipients') || $tr.attr('data-share-owner')) { + if (hasUserShares || hasLinkShare || $tr.attr('data-share-recipient-data') || $tr.attr('data-share-owner')) { OC.Share.markFileAsShared($tr, true, hasLinkShare); return true; } return false; }, - /** - * Formats a recipients array to be displayed. - * The first four recipients will be shown and the - * other ones will be shown as "+x" where "x" is the number of - * remaining recipients. - * - * @param {Array.} recipients recipients array - * @param {int} count optional total recipients count (in case the array was shortened) - * @return {String} formatted recipients display text - */ - formatRecipients: function(recipients, count) { - var maxRecipients = 4; - var text; - if (!_.isNumber(count)) { - count = recipients.length; - } - // TODO: use natural sort - recipients = _.first(recipients, maxRecipients).sort(); - text = recipients.join(', '); - if (count > maxRecipients) { - text += ', +' + (count - maxRecipients); - } - return text; - }, - /** * @param {Array} fileData * @returns {String} diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index fb4909e764e..ad6f70a6f1c 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -375,11 +375,6 @@ // convert the recipients map to a flat // array of sorted names data.mountType = 'shared'; - data.recipients = _.keys(data.recipients); - data.recipientsDisplayName = OCA.Sharing.Util.formatRecipients( - data.recipients, - data.recipientsCount - ); delete data.recipientsCount; if (self._sharedWithUser) { // only for outgoing shres @@ -413,9 +408,18 @@ * @property {int} stime share timestamp in milliseconds * @property {String} [targetDisplayName] display name of the recipient * (only when shared with others) + * @property {String} [targetShareWithId] id of the recipient * */ + /** + * Recipient attributes + * + * @typedef {Object} OCA.Sharing.RecipientInfo + * @property {String} shareWith the id of the recipient + * @property {String} shareWithDisplayName the display name of the recipient + */ + /** * Shared file info attributes. * @@ -427,7 +431,8 @@ * @property {String} shareOwner name of the share owner * @property {Array.} recipients name of the first 4 recipients * (this is mostly for display purposes) - * @property {String} recipientsDisplayName display name + * @property {Object.} recipientData (as object for easier + * passing to HTML data attributes with jQuery) */ OCA.Sharing.FileList = FileList; diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js index 1b628f5bb0f..893525f7566 100644 --- a/apps/files_sharing/tests/js/shareSpec.js +++ b/apps/files_sharing/tests/js/shareSpec.js @@ -282,8 +282,6 @@ describe('OCA.Sharing.Util tests', function() { ] }); - expect($tr.attr('data-share-recipients')).toEqual('Group One, Group Two, User One, User Two'); - expect($action.text().trim()).toEqual('Shared with Group One Shared with Group Two Shared with User One Shared with User Two'); expect($action.find('.icon').hasClass('icon-shared')).toEqual(true); expect($action.find('.icon').hasClass('icon-public')).toEqual(false); @@ -315,8 +313,6 @@ describe('OCA.Sharing.Util tests', function() { ] }); - expect($tr.attr('data-share-recipients')).toEqual('User One, User Three, User Two'); - expect($action.text().trim()).toEqual('Shared with User One Shared with User Three Shared with User Two'); expect($action.find('.icon').hasClass('icon-shared')).toEqual(true); expect($action.find('.icon').hasClass('icon-public')).toEqual(false); @@ -345,7 +341,7 @@ describe('OCA.Sharing.Util tests', function() { shares: [] }); - expect($tr.attr('data-share-recipients')).not.toBeDefined(); + expect($tr.attr('data-share-recipient-data')).not.toBeDefined(); }); it('keep share text after updating reshare', function() { var $action, $tr; @@ -372,8 +368,6 @@ describe('OCA.Sharing.Util tests', function() { shares: [{share_with_displayname: 'User Two'}] }); - expect($tr.attr('data-share-recipients')).toEqual('User Two'); - expect($action.find('>span').text().trim()).toEqual('Shared by User One'); expect($action.find('.icon').hasClass('icon-shared')).toEqual(true); expect($action.find('.icon').hasClass('icon-public')).toEqual(false); @@ -405,60 +399,13 @@ describe('OCA.Sharing.Util tests', function() { shares: [] }); - expect($tr.attr('data-share-recipients')).not.toBeDefined(); + expect($tr.attr('data-share-recipient-data')).not.toBeDefined(); expect($action.find('>span').text().trim()).toEqual('Shared by User One'); expect($action.find('.icon').hasClass('icon-shared')).toEqual(true); expect($action.find('.icon').hasClass('icon-public')).toEqual(false); }); }); - describe('formatRecipients', function() { - it('returns a single recipient when one passed', function() { - expect(OCA.Sharing.Util.formatRecipients(['User one'])) - .toEqual('User one'); - }); - it('returns two recipients when two passed', function() { - expect(OCA.Sharing.Util.formatRecipients(['User one', 'User two'])) - .toEqual('User one, User two'); - }); - it('returns four recipients with plus when five passed', function() { - var recipients = [ - 'User one', - 'User two', - 'User three', - 'User four', - 'User five' - ]; - expect(OCA.Sharing.Util.formatRecipients(recipients)) - .toEqual('User four, User one, User three, User two, +1'); - }); - it('returns four recipients with plus when ten passed', function() { - var recipients = [ - 'User one', - 'User two', - 'User three', - 'User four', - 'User five', - 'User six', - 'User seven', - 'User eight', - 'User nine', - 'User ten' - ]; - expect(OCA.Sharing.Util.formatRecipients(recipients)) - .toEqual('User four, User one, User three, User two, +6'); - }); - it('returns four recipients with plus when four passed with counter', function() { - var recipients = [ - 'User one', - 'User two', - 'User three', - 'User four' - ]; - expect(OCA.Sharing.Util.formatRecipients(recipients, 10)) - .toEqual('User four, User one, User three, User two, +6'); - }); - }); describe('Excluded lists', function() { function createListThenAttach(listId) { var fileActions = new OCA.Files.FileActions(); diff --git a/apps/files_sharing/tests/js/sharedfilelistSpec.js b/apps/files_sharing/tests/js/sharedfilelistSpec.js index 3efbb8fcea3..903234947bd 100644 --- a/apps/files_sharing/tests/js/sharedfilelistSpec.js +++ b/apps/files_sharing/tests/js/sharedfilelistSpec.js @@ -628,7 +628,7 @@ describe('OCA.Sharing.FileList tests', function() { expect($tr.attr('data-permissions')).toEqual('31'); // read and delete expect($tr.attr('data-mime')).toEqual('text/plain'); expect($tr.attr('data-mtime')).toEqual('11111000'); - expect($tr.attr('data-share-recipients')).not.toBeDefined(); + expect($tr.attr('data-share-recipient-data')).not.toBeDefined(); expect($tr.attr('data-share-owner')).not.toBeDefined(); expect($tr.attr('data-share-id')).toEqual('7'); expect($tr.attr('data-favorite')).toEqual('true'); @@ -681,7 +681,7 @@ describe('OCA.Sharing.FileList tests', function() { expect($tr.attr('data-permissions')).toEqual('31'); // read and delete expect($tr.attr('data-mime')).toEqual('text/plain'); expect($tr.attr('data-mtime')).toEqual('11111000'); - expect($tr.attr('data-share-recipients')).not.toBeDefined(); + expect($tr.attr('data-share-recipient-data')).not.toBeDefined(); expect($tr.attr('data-share-owner')).not.toBeDefined(); expect($tr.attr('data-share-id')).toEqual('7'); expect($tr.attr('data-favorite')).toEqual('true'); diff --git a/core/js/share.js b/core/js/share.js index 381c42c5de2..a2a3635e145 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -245,7 +245,10 @@ OC.Share = _.extend(OC.Share || {}, { */ _formatShareList: function(recipients) { var _parent = this; - recipients = _.sortBy(_.toArray(recipients), 'shareWithDisplayName'); + recipients = _.toArray(recipients); + recipients.sort(function(a, b) { + return a.shareWithDisplayName.localeCompare(b.shareWithDisplayName); + }); return $.map(recipients, function(recipient) { return _parent._formatRemoteShare(recipient.shareWith, recipient.shareWithDisplayName, t('core', 'Shared with')); }); diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js index 8688705b8e8..399911c3d54 100644 --- a/core/js/tests/specs/shareSpec.js +++ b/core/js/tests/specs/shareSpec.js @@ -157,8 +157,6 @@ describe('OC.Share tests', function() { function checkRecipients(input, output, title) { var $action; - var concatenated = _.values(input).join(', '); - $file.attr('data-share-recipients', concatenated); $file.attr('data-share-recipient-data', JSON.stringify(input)); OC.Share.markFileAsShared($file, true); @@ -270,8 +268,8 @@ describe('OC.Share tests', function() { shareWithDisplayName: 'One@someserver.com' }, 1: { - shareWith: 'two@someserver.com', - shareWithDisplayName: 'two@someserver.com' + shareWith: 'two@otherserver.com', + shareWithDisplayName: 'two@otherserver.com' } }; checkRecipients( @@ -320,8 +318,8 @@ describe('OC.Share tests', function() { shareWithDisplayName: 'One' }, 1: { - shareWith: 'two@someserver.com', - shareWithDisplayName: 'two@someserver.com' + shareWith: 'two@otherserver.com', + shareWithDisplayName: 'two@otherserver.com' } }, 'Shared with One two@…', @@ -336,15 +334,15 @@ describe('OC.Share tests', function() { shareWithDisplayName: 'Yoko Ono' }, 1: { - shareWith: 'two@someserver.com', - shareWithDisplayName: 'two@someserver.com' + shareWith: 'two@otherserver.com', + shareWithDisplayName: 'two@othererver.com' }, 2: { shareWith: 'Three', shareWithDisplayName: 'Green, Mina' } }, - 'Shared with Yoko Ono two@… Shared with Green, Mina', + 'Shared with Green, Mina two@… Shared with Yoko Ono', ['Shared with two@otherserver.com'] ); }); -- cgit v1.2.3 From f8b9ddc0eae6a4222b00b5ff4570315e977659f4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 21 Nov 2017 12:52:49 +0100 Subject: apply tooltip to all avatars Signed-off-by: Arthur Schiwon --- core/js/share.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'core/js') diff --git a/core/js/share.js b/core/js/share.js index a2a3635e145..85a5b0740dc 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -314,9 +314,8 @@ OC.Share = _.extend(OC.Share || {}, { var avatarElement = action.find('.avatar'); avatarElement.each(function () { $(this).avatar($(this).data('username'), 32); + $(this).tooltip({placement: 'top'}); }); - - action.find('.icon-shared + span').tooltip({placement: 'top'}); } } else { action.html('' + t('core', 'Shared') + '').prepend(icon); -- cgit v1.2.3 From b3b34c08d173a2bb4518552ca92d81915d00ca53 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 22 Nov 2017 00:28:53 +0100 Subject: fix tooltips for good Signed-off-by: Arthur Schiwon --- core/js/share.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/js') diff --git a/core/js/share.js b/core/js/share.js index 85a5b0740dc..4260c0e55bd 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -314,8 +314,8 @@ OC.Share = _.extend(OC.Share || {}, { var avatarElement = action.find('.avatar'); avatarElement.each(function () { $(this).avatar($(this).data('username'), 32); - $(this).tooltip({placement: 'top'}); }); + action.find('span[title]').tooltip({placement: 'top'}); } } else { action.html('' + t('core', 'Shared') + '').prepend(icon); -- cgit v1.2.3 From 5a9c99e6ac53ec021bcd124c2b8664fe269a5168 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 23 Nov 2017 17:51:56 +0100 Subject: recycle SharedFileInfo values in fileInfo fileInfo is composed of data from sharing, however additional data is pulled when sidebar opens, e.g. the size. Then, existing data is overwritten by data from the other source (files). The data points that would be lost are not dirty however and still used, so we keep them. Signed-off-by: Arthur Schiwon --- apps/files/js/filelist.js | 5 +---- apps/files_sharing/js/sharedfilelist.js | 22 +++++++++++++++++++++- core/js/share.js | 1 - 3 files changed, 22 insertions(+), 6 deletions(-) (limited to 'core/js') diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js index 6996e423776..10efa54496a 100644 --- a/apps/files/js/filelist.js +++ b/apps/files/js/filelist.js @@ -553,9 +553,6 @@ actionsWidth += $(action).outerWidth(); }); - // subtract app navigation toggle when visible - containerWidth -= $('#app-navigation-toggle').width(); - this.breadcrumb._resize(); this.$table.find('>thead').width($('#app-content').width() - OC.Util.getScrollBarWidth()); @@ -1369,7 +1366,7 @@ * @return new tr element (not appended to the table) */ add: function(fileData, options) { - var index = -1; + var index; var $tr; var $rows; var $insertionPoint; diff --git a/apps/files_sharing/js/sharedfilelist.js b/apps/files_sharing/js/sharedfilelist.js index ad6f70a6f1c..ad818d91413 100644 --- a/apps/files_sharing/js/sharedfilelist.js +++ b/apps/files_sharing/js/sharedfilelist.js @@ -153,6 +153,27 @@ // storage info like free space / used space }, + updateRow: function($tr, fileInfo, options) { + if(!fileInfo instanceof OCA.Sharing.SharedFileInfo) { + // recycle SharedFileInfo values if something tries to overwrite it + var oldModel = this.getModelForFile($tr); + + if(_.isUndefined(fileInfo.recipientData) && oldModel.recipientData) { + fileInfo.recipientData = oldModel.recipientData; + } + if(_.isUndefined(fileInfo.recipients) && oldModel.recipientData) { + fileInfo.recipientData = oldModel.recipientData; + } + if(_.isUndefined(fileInfo.shares) && oldModel.shares) { + fileInfo.shares = oldModel.shares; + } + if(_.isUndefined(fileInfo.shareOwner) && oldModel.shareOwner) { + fileInfo.shareOwner = oldModel.shareOwner; + } + } + OCA.Files.FileList.prototype._createRow.updateRow(this, arguments); + }, + reload: function() { this.showMask(); if (this._reloadCall) { @@ -225,7 +246,6 @@ }, _makeFilesFromRemoteShares: function(data) { - var self = this; var files = data; files = _.chain(files) diff --git a/core/js/share.js b/core/js/share.js index 4260c0e55bd..7662d6cffb9 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -161,7 +161,6 @@ OC.Share = _.extend(OC.Share || {}, { updateIcon:function(itemType, itemSource) { var shares = false; var link = false; - var image = OC.imagePath('core', 'actions/share'); var iconClass = ''; $.each(OC.Share.itemShares, function(index) { if (OC.Share.itemShares[index]) { -- cgit v1.2.3 From 134192d76cf27bee9549710a1eac40db3ef1edf1 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 24 Nov 2017 15:33:26 +0100 Subject: fix sorting test on phantomjs Signed-off-by: Arthur Schiwon --- core/js/tests/specs/shareSpec.js | 44 ++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 15 deletions(-) (limited to 'core/js') diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js index 399911c3d54..05057692e98 100644 --- a/core/js/tests/specs/shareSpec.js +++ b/core/js/tests/specs/shareSpec.js @@ -327,22 +327,36 @@ describe('OC.Share tests', function() { ); }); it('display multiple with divergent displaynames', function() { - checkRecipients( - { - 0: { - shareWith: 'One', - shareWithDisplayName: 'Yoko Ono' - }, - 1: { - shareWith: 'two@otherserver.com', - shareWithDisplayName: 'two@othererver.com' - }, - 2: { - shareWith: 'Three', - shareWithDisplayName: 'Green, Mina' - } + var recipients = { + 0: { + shareWith: 'One', + shareWithDisplayName: 'Yoko Ono', + _output: 'Shared with Yoko Ono' }, - 'Shared with Green, Mina two@… Shared with Yoko Ono', + 1: { + shareWith: 'two@otherserver.com', + shareWithDisplayName: 'two@othererver.com', + _output: 'two@…' + }, + 2: { + shareWith: 'Three', + shareWithDisplayName: 'Green, Mina', + _output: 'Shared with Green, Mina' + } + }; + + // we cannot assume the locale, also because PhantomJS has a bug. + var sortArray = _.toArray(recipients) + .sort(function(a, b) { + return a.shareWithDisplayName.localeCompare(b.shareWithDisplayName); + }); + var sortedOutput = _.map(sortArray, function(recipient) { + return recipient._output; + }).join(' '); + + checkRecipients( + recipients, + sortedOutput, ['Shared with two@otherserver.com'] ); }); -- cgit v1.2.3