Multiple fixes to sharing overview

- scrolling to next page now correctly renders share icons
- reshareing/unsharing a reshare will now still keep "Shared by" label

apps/files_sharing/js/share.js

@@ -14,7 +14,6 @@
 	}
 	OCA.Sharing.Util = {
 		initialize: function(fileActions) {
-			// TODO: make a separate class for this or a hook or jQuery event ?
 			if (OCA.Files.FileList) {
 				var oldCreateRow = OCA.Files.FileList.prototype._createRow;
 				OCA.Files.FileList.prototype._createRow = function(fileData) {
@@ -32,28 +31,28 @@
 					}
 					return tr;
 				};
-			}
 
-			// use delegate to catch the case with multiple file lists
+				var oldRenderRow = OCA.Files.FileList.prototype._renderRow;
-			$('#content').delegate('#fileList', 'fileActionsReady',function(ev){
+				OCA.Files.FileList.prototype._renderRow = function(fileData) {
+					var $tr = oldRenderRow.apply(this, arguments);
+					// if the statuses are loaded already, use them for the icon
+					// (needed when scrolling to the next page)
+					var shareStatus = OC.Share.statuses[fileData.id];
+					if (fileData.shareOwner || fileData.recipientsDisplayName || shareStatus) {
+						var permissions = $tr.data('permissions');
+						var hasLink = !!(shareStatus && shareStatus.link);
+						if (permissions & OC.PERMISSION_SHARE) {
+							OC.Share.markFileAsShared($tr, true, hasLink);
+						} else {
 							// if no share action exists because the admin disabled sharing for this user
 							// we create a share notification action to inform the user about files
 							// shared with him otherwise we just update the existing share action.
-				var fileList = ev.fileList;
-				var $fileList = $(this);
-				$fileList.find('[data-share-owner]').each(function() {
-					var $tr = $(this);
-					var permissions = $tr.data('permissions');
-					if(permissions & OC.PERMISSION_SHARE) {
-						OC.Share.markFileAsShared($tr, true);
-					} else {
 							// TODO: make this work like/with OC.Share.markFileAsShared()
 							var shareNotification = '<a class="action action-share-notification permanent"' +
 									' data-action="Share-Notification" href="#" original-title="">' +
 									' <img class="svg" src="' + OC.imagePath('core', 'actions/share') + '"></img>';
 							$tr.find('.fileactions').append(function() {
-							var owner = $(this).closest('tr').attr('data-share-owner');
+								var shareBy = t('files_sharing', 'Shared by {owner}', {owner: fileData.shareOwner});
-							var shareBy = t('files_sharing', 'Shared by {owner}', {owner: owner});
 								var $result = $(shareNotification + '<span> ' + shareBy + '</span></span>');
 								$result.on('click', function() {
 									return false;
@@ -61,8 +60,14 @@
 								return $result;
 							});
 						}
-				});
+					}
+					return $tr;
+				};
+			}
 
+			// use delegate to catch the case with multiple file lists
+			$('#content').delegate('#fileList', 'fileActionsReady',function(ev){
+				var fileList = ev.fileList;
 				if (!OCA.Sharing.sharesLoaded){
 					OC.Share.loadIcons('file', fileList);
 					// assume that we got all shares, so switching directories
@@ -70,6 +75,7 @@
 					OCA.Sharing.sharesLoaded = true;
 				}
 				else{
+					// this will update the icons for all the visible elements
 					OC.Share.updateIcons('file', fileList);
 				}
 			});

apps/files_sharing/js/sharedfilelist.js

@@ -38,6 +38,13 @@
 			}
 		},
 
+		_renderRow: function() {
+			// HACK: needed to call the overridden _renderRow
+			// this is because at the time this class is created
+			// the overriding hasn't been done yet...
+			return OCA.Files.FileList.prototype._renderRow.apply(this, arguments);
+		},
+
 		_createRow: function(fileData) {
 			// TODO: hook earlier and render the whole row here
 			var $tr = OCA.Files.FileList.prototype._createRow.apply(this, arguments);

apps/files_sharing/tests/js/shareSpec.js

@@ -55,7 +55,7 @@ describe('OCA.Sharing.Util tests', function() {
 			path: '/subdir',
 			mimetype: 'text/plain',
 			size: 12,
-			permissions: 31,
+			permissions: OC.PERMISSION_ALL,
 			etag: 'abc',
 			shareOwner: 'User One',
 			isShareMountPoint: false
@@ -87,7 +87,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				etag: 'abc'
 			}]);
 			$tr = fileList.$el.find('tbody tr:first');
@@ -106,7 +106,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				etag: 'abc'
 			}]);
 			$tr = fileList.$el.find('tbody tr:first');
@@ -127,7 +127,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				etag: 'abc'
 			}]);
 			$tr = fileList.$el.find('tbody tr:first');
@@ -147,7 +147,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				shareOwner: 'User One',
 				etag: 'abc'
 			}]);
@@ -167,7 +167,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				recipientsDisplayName: 'User One, User Two',
 				etag: 'abc'
 			}]);
@@ -179,6 +179,28 @@ describe('OCA.Sharing.Util tests', function() {
 			expect(OC.basename(getImageUrl($tr.find('.filename')))).toEqual('folder-shared.svg');
 			expect($action.find('img').length).toEqual(1);
 		});
+		it('shows static share text when file shared with user that has no share permission', function() {
+			var $action, $tr;
+			fileList.setFiles([{
+				id: 1,
+				type: 'dir',
+				name: 'One',
+				path: '/subdir',
+				mimetype: 'text/plain',
+				size: 12,
+				permissions: OC.PERMISSION_CREATE,
+				etag: 'abc',
+				shareOwner: 'User One'
+			}]);
+			$tr = fileList.$el.find('tbody tr:first');
+			expect($tr.find('.action-share').length).toEqual(0);
+			$action = $tr.find('.action-share-notification');
+			expect($action.hasClass('permanent')).toEqual(true);
+			expect($action.find('>span').text().trim()).toEqual('Shared by User One');
+			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+			expect(OC.basename(getImageUrl($tr.find('.filename')))).toEqual('folder-shared.svg');
+			expect($action.find('img').length).toEqual(1);
+		});
 	});
 	describe('Share action', function() {
 		var showDropDownStub;
@@ -201,7 +223,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				etag: 'abc'
 			}]);
 			$action = fileList.$el.find('tbody tr:first .action-share');
@@ -237,7 +259,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				etag: 'abc'
 			}]);
 			$action = fileList.$el.find('tbody tr:first .action-share');
@@ -273,7 +295,7 @@ describe('OCA.Sharing.Util tests', function() {
 				path: '/subdir',
 				mimetype: 'text/plain',
 				size: 12,
-				permissions: 31,
+				permissions: OC.PERMISSION_ALL,
 				etag: 'abc',
 				recipients: 'User One, User Two'
 			}]);
@@ -296,6 +318,80 @@ describe('OCA.Sharing.Util tests', function() {
 			OC.Share.updateIcon('file', 1);
 			expect($action.hasClass('permanent')).toEqual(false);
 		});
+		it('keep share text after updating reshare', function() {
+			var $action, $tr;
+			OC.Share.statuses = {1: {link: false, path: '/subdir'}};
+			fileList.setFiles([{
+				id: 1,
+				type: 'file',
+				name: 'One.txt',
+				path: '/subdir',
+				mimetype: 'text/plain',
+				size: 12,
+				permissions: OC.PERMISSION_ALL,
+				etag: 'abc',
+				shareOwner: 'User One'
+			}]);
+			$action = fileList.$el.find('tbody tr:first .action-share');
+			$tr = fileList.$el.find('tr:first');
+
+			expect($action.hasClass('permanent')).toEqual(true);
+
+			$tr.find('.action-share').click();
+
+			expect(showDropDownStub.calledOnce).toEqual(true);
+
+			// simulate what the dropdown does
+			var itemShares = {};
+			itemShares[OC.Share.SHARE_TYPE_USER] = ['User Two'];
+			OC.Share.itemShares = itemShares;
+			$('#dropdown').trigger(new $.Event('sharesChanged', {itemShares: itemShares}));
+
+			expect($tr.attr('data-share-recipients')).toEqual('User Two');
+
+			OC.Share.updateIcon('file', 1);
+
+			expect($action.hasClass('permanent')).toEqual(true);
+			expect($action.find('>span').text()).toEqual('Shared by User One');
+			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+		});
+		it('keep share text after unsharing reshare', function() {
+			var $action, $tr;
+			OC.Share.statuses = {1: {link: false, path: '/subdir'}};
+			fileList.setFiles([{
+				id: 1,
+				type: 'file',
+				name: 'One.txt',
+				path: '/subdir',
+				mimetype: 'text/plain',
+				size: 12,
+				permissions: OC.PERMISSION_ALL,
+				etag: 'abc',
+				shareOwner: 'User One',
+				recipients: 'User Two'
+			}]);
+			$action = fileList.$el.find('tbody tr:first .action-share');
+			$tr = fileList.$el.find('tr:first');
+
+			expect($action.hasClass('permanent')).toEqual(true);
+
+			$tr.find('.action-share').click();
+
+			expect(showDropDownStub.calledOnce).toEqual(true);
+
+			// simulate what the dropdown does
+			var itemShares = {};
+			OC.Share.itemShares = itemShares;
+			$('#dropdown').trigger(new $.Event('sharesChanged', {itemShares: itemShares}));
+
+			expect($tr.attr('data-share-recipients')).not.toBeDefined();
+
+			OC.Share.updateIcon('file', 1);
+
+			expect($action.hasClass('permanent')).toEqual(true);
+			expect($action.find('>span').text()).toEqual('Shared by User One');
+			expect(OC.basename($action.find('img').attr('src'))).toEqual('share.svg');
+		});
 	});
 	describe('formatRecipients', function() {
 		it('returns a single recipient when one passed', function() {

core/js/share.js

@@ -151,7 +151,7 @@ OC.Share={
 		var img = action.find('img');
 		var message;
 		var recipients;
-		var owner;
+		var owner = $tr.attr('data-share-owner');
 		var shareFolderIcon;
 		var image = OC.imagePath('core', 'actions/share');
 		// update folder icon
@@ -168,16 +168,16 @@ OC.Share={
 			$tr.children('.filename').css('background-image', 'url(' + shareFolderIcon + ')');
 		}
 		// update share action text / icon
-		if (hasShares) {
+		if (hasShares || owner) {
 			recipients = $tr.attr('data-share-recipients');
-			owner = $tr.attr('data-share-owner');
 
 			action.addClass('permanent');
 			message = t('core', 'Shared');
-			if (owner && !recipients) {
+			// even if reshared, only show "Shared by"
+			if (owner) {
 				message = t('files_sharing', 'Shared by {owner}', {owner: owner});
 			}
-			if (recipients) {
+			else if (recipients) {
 				message = t('core', 'Shared with {recipients}', {recipients: recipients});
 			}
 			action.html(' <span>'+ message + '</span>').prepend(img);