From bf491183c1e2e3ad1f2cd9d42e09339b5705acd3 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Sat, 16 May 2015 14:22:10 +0200 Subject: [PATCH] Properly format remote recipients * A list of recipients can now be properly formatted with remote shares. Before the shares where simply shown in full in the "Shared with others" section. * Unit tests updated and added --- core/js/share.js | 39 +++++++++----- core/js/tests/specs/shareSpec.js | 93 ++++++++++++++++++++++++++++++-- 2 files changed, 117 insertions(+), 15 deletions(-) diff --git a/core/js/share.js b/core/js/share.js index 415fe41ef5..c0bc2c8ab8 100644 --- a/core/js/share.js +++ b/core/js/share.js @@ -15,7 +15,7 @@ OC.Share={ * "user@example.com/path/to/owncloud" * "user@anotherexample.com@example.com/path/to/owncloud */ - _REMOTE_OWNER_REGEXP: new RegExp("^([^@]*)@(([^@]*)@)?([^/]*)(.*)?$"), + _REMOTE_OWNER_REGEXP: new RegExp("^([^@]*)@(([^@]*)@)?([^/]*)([/](.*)?)?$"), /** * @deprecated use OC.Share.currentShares instead @@ -183,21 +183,22 @@ OC.Share={ } }, /** - * Format remote share owner to make it more readable + * Format a remote address * - * @param {String} owner full remote share owner name - * @return {String} HTML code for the owner display + * @param {String} remoteAddress full remote share + * @return {String} HTML code to display */ - _formatSharedByOwner: function(owner) { - var parts = this._REMOTE_OWNER_REGEXP.exec(owner); + _formatRemoteShare: function(remoteAddress) { + var parts = this._REMOTE_OWNER_REGEXP.exec(remoteAddress); if (!parts) { // display as is, most likely to be a simple owner name - return escapeHTML(owner); + return escapeHTML(remoteAddress); } var userName = parts[1]; var userDomain = parts[3]; var server = parts[4]; + var dir = parts[6]; var tooltip = userName; if (userDomain) { tooltip += '@' + userDomain; @@ -209,7 +210,7 @@ OC.Share={ tooltip += '@' + server; } - var html = ''; + var html = ''; html += '' + escapeHTML(userName) + ''; if (userDomain) { html += '@' + escapeHTML(userDomain) + ''; @@ -217,6 +218,20 @@ OC.Share={ html += ''; return html; }, + /** + * Loop over all recipients in the list and format them using + * all kind of fancy magic. + * + * @param {String[]} recipients array of all the recipients + * @return {String[]} modified list of recipients + */ + _formatShareList: function(recipients) { + var _parent = this; + return $.map(recipients, function(recipient) { + recipient = _parent._formatRemoteShare(recipient); + return recipient; + }); + }, /** * Marks/unmarks a given file as shared by changing its action icon * and folder icon. @@ -255,14 +270,14 @@ OC.Share={ message = t('core', 'Shared'); // even if reshared, only show "Shared by" if (owner) { - message = this._formatSharedByOwner(owner); + message = this._formatRemoteShare(owner); } else if (recipients) { - message = t('core', 'Shared with {recipients}', {recipients: recipients}); + message = t('core', 'Shared with {recipients}', {recipients: this._formatShareList(recipients.split(", ")).join(", ")}, 0, {escape: false}); } action.html(' ' + message + '').prepend(img); - if (owner) { - action.find('.remoteOwner').tipsy({gravity: 's'}); + if (owner || recipients) { + action.find('.remoteAddress').tipsy({gravity: 's'}); } } else { diff --git a/core/js/tests/specs/shareSpec.js b/core/js/tests/specs/shareSpec.js index 33090bbda7..4e12f3bb0c 100644 --- a/core/js/tests/specs/shareSpec.js +++ b/core/js/tests/specs/shareSpec.js @@ -1134,9 +1134,9 @@ describe('OC.Share tests', function() { $action = $file.find('.action-share>span'); expect($action.text()).toEqual(output); if (_.isString(title)) { - expect($action.find('.remoteOwner').attr('title')).toEqual(title); + expect($action.find('.remoteAddress').attr('title')).toEqual(title); } else { - expect($action.find('.remoteOwner').attr('title')).not.toBeDefined(); + expect($action.find('.remoteAddress').attr('title')).not.toBeDefined(); } expect(tipsyStub.calledOnce).toEqual(true); tipsyStub.reset(); @@ -1227,7 +1227,94 @@ describe('OC.Share tests', function() { checkIcon('filetypes/folder-public'); }); }); - // TODO: add unit tests for share recipients + + describe('displaying the recipoients', function() { + function checkRecipients(input, output, title) { + var $action; + + $file.attr('data-share-recipients', input); + OC.Share.markFileAsShared($file, true); + + $action = $file.find('.action-share>span'); + expect($action.text()).toEqual(output); + if (_.isString(title)) { + expect($action.find('.remoteAddress').attr('title')).toEqual(title); + } else if (_.isArray(title)) { + var tooltips = $action.find('.remoteAddress'); + expect(tooltips.length).toEqual(title.length); + + tooltips.each(function(i) { + expect($(this).attr('title')).toEqual(title[i]); + }); + } else { + expect($action.find('.remoteAddress').attr('title')).not.toBeDefined(); + } + expect(tipsyStub.calledOnce).toEqual(true); + tipsyStub.reset(); + } + + it('displays the local share owner as is', function() { + checkRecipients('User One', 'Shared with User One', null); + }); + it('displays the user name part of a remote recipient', function() { + checkRecipients( + 'User One@someserver.com', + 'Shared with User One@…', + 'User One@someserver.com' + ); + checkRecipients( + 'User One@someserver.com/', + 'Shared with User One@…', + 'User One@someserver.com' + ); + checkRecipients( + 'User One@someserver.com/root/of/owncloud', + 'Shared with User One@…', + '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', + 'Shared with User One@example.com', + 'User One@example.com@someserver.com' + ); + checkRecipients( + 'User One@example.com@someserver.com/', + 'Shared with User One@example.com', + 'User One@example.com@someserver.com' + ); + checkRecipients( + 'User One@example.com@someserver.com/root/of/owncloud', + 'Shared with User One@example.com', + 'User One@example.com@someserver.com' + ); + }); + it('display multiple remote recipients', function() { + checkRecipients( + 'One@someserver.com, two@otherserver.com', + 'Shared with One@…, two@…', + ['One@someserver.com', 'two@otherserver.com'] + ); + checkRecipients( + 'One@someserver.com/, two@otherserver.com', + 'Shared with One@…, two@…', + ['One@someserver.com', 'two@otherserver.com'] + ); + checkRecipients( + 'One@someserver.com/root/of/owncloud, two@otherserver.com', + 'Shared with One@…, two@…', + ['One@someserver.com', 'two@otherserver.com'] + ); + }); + it('display mixed recipients', function() { + checkRecipients( + 'One, two@otherserver.com', + 'Shared with One, two@…', + ['two@otherserver.com'] + ); + }); + }); }); });