Merge pull request #16380 from rullzer/fix_16281

Properly format recipients

2 files changed, 117 insertions, 15 deletions

diff --git a/core/js/share.js b/core/js/share.js
index 415fe41ef5f..c0bc2c8ab80 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 = '<span class="remoteOwner" title="' + escapeHTML(tooltip) + '">';
+		var html = '<span class="remoteAddress" title="' + escapeHTML(tooltip) + '">';
 		html += '<span class="username">' + escapeHTML(userName) + '</span>';
 		if (userDomain) {
 			html += '<span class="userDomain">@' + escapeHTML(userDomain) + '</span>';
@@ -218,6 +219,20 @@ OC.Share={
 		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(' <span>' + message + '</span>').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 33090bbda74..4e12f3bb0cf 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']
+				);
+			});
+		});
 	});
 });