Use fileActionsReady to re-render sharing icons

Whenever file actions are modified, either by registering new actions or
when appending a new page of entries, the sharing app is now notified so
it can correctly refresh the sharing icon status.

Additionally, the core's loadIcons() method is also used to load the
existing shares and also refresh the sharing icons afterwards.

6 files changed, 106 insertions, 56 deletions

diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
index fc7c9ccacef..e06d2912274 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -181,11 +181,12 @@
 				return;
 			}
 			this.currentFile = parent;
+			var $tr = parent.closest('tr');
 			var self = this;
 			var actions = this.getActions(this.getCurrentMimeType(), this.getCurrentType(), this.getCurrentPermissions());
 			var file = this.getCurrentFile();
 			var nameLinks;
-			if (parent.closest('tr').data('renaming')) {
+			if ($tr.data('renaming')) {
 				return;
 			}
 
@@ -278,7 +279,7 @@
 			}
 
 			if (triggerEvent){
-				fileList.$fileList.trigger(jQuery.Event("fileActionsReady", {fileList: fileList}));
+				fileList.$fileList.trigger(jQuery.Event("fileActionsReady", {fileList: fileList, $files: $tr}));
 			}
 		},
 		getCurrentFile: function () {
diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index 07e4e523e03..c7fccc5dd66 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -473,6 +473,7 @@
 		/**
 		 * Appends the next page of files into the table
 		 * @param animate true to animate the new elements
+		 * @return array of DOM elements of the newly added files
 		 */
 		_nextPage: function(animate) {
 			var index = this.$fileList.children().length,
@@ -483,7 +484,7 @@
 				isAllSelected = this.isAllSelected();
 
 			if (index >= this.files.length) {
-				return;
+				return false;
 			}
 
 			while (count > 0 && index < this.files.length) {
@@ -496,12 +497,17 @@
 				}
 				if (animate) {
 					tr.addClass('appear transparent');
-					newTrs.push(tr);
 				}
+				newTrs.push(tr);
 				index++;
 				count--;
 			}
 
+			// trigger event for newly added rows
+			if (newTrs.length > 0) {
+				this.$fileList.trigger($.Event('fileActionsReady', {fileList: this, $files: newTrs}));
+			}
+
 			if (animate) {
 				// defer, for animation
 				window.setTimeout(function() {
@@ -510,6 +516,7 @@
 					}
 				}, 0);
 			}
+			return newTrs;
 		},
 
 		/**
@@ -518,10 +525,16 @@
 		 */
 		_onFileActionsUpdated: function() {
 			var self = this;
-			this.$fileList.find('tr td.filename').each(function() {
-				self.fileActions.display($(this), false, self);
+			var $files = this.$fileList.find('tr');
+			if (!$files.length) {
+				return;
+			}
+
+			$files.each(function() {
+				self.fileActions.display($(this).find('td.filename'), false, self);
 			});
-			this.$fileList.trigger($.Event('fileActionsReady', {fileList: this}));
+			this.$fileList.trigger($.Event('fileActionsReady', {fileList: this, $files: $files}));
+
 		},
 
 		/**
@@ -533,7 +546,6 @@
 			// detach to make adding multiple rows faster
 			this.files = filesArray;
 
-			this.$fileList.detach();
 			this.$fileList.empty();
 
 			// clear "Select all" checkbox
@@ -542,10 +554,7 @@
 			this.isEmpty = this.files.length === 0;
 			this._nextPage();
 
-			this.$el.find('thead').after(this.$fileList);
-
 			this.updateEmptyContent();
-			this.$fileList.trigger($.Event('fileActionsReady', {fileList: this}));
 
 			this.fileSummary.calculate(filesArray);
 
@@ -1283,16 +1292,16 @@
 								// reinsert row
 								self.files.splice(tr.index(), 1);
 								tr.remove();
-								self.add(fileInfo, {updateSummary: false, silent: true});
-								self.$fileList.trigger($.Event('fileActionsReady', {fileList: self}));
+								tr = self.add(fileInfo, {updateSummary: false, silent: true});
+								self.$fileList.trigger($.Event('fileActionsReady', {fileList: self, $files: $(tr)}));
 							}
 						});
 					} else {
 						// add back the old file info when cancelled
 						self.files.splice(tr.index(), 1);
 						tr.remove();
-						self.add(oldFileInfo, {updateSummary: false, silent: true});
-						self.$fileList.trigger($.Event('fileActionsReady', {fileList: self}));
+						tr = self.add(oldFileInfo, {updateSummary: false, silent: true});
+						self.$fileList.trigger($.Event('fileActionsReady', {fileList: self, $files: $(tr)}));
 					}
 				} catch (error) {
 					input.attr('title', error);
diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js
index 7a6c390f325..713cd5468d8 100644
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@@ -797,13 +797,24 @@ describe('OCA.Files.FileList tests', function() {
 			fileList.$fileList.on('fileActionsReady', handler);
 			fileList.setFiles(testFiles);
 			expect(handler.calledOnce).toEqual(true);
+			expect(handler.getCall(0).args[0].$files.length).toEqual(testFiles.length);
 		});
 		it('triggers "fileActionsReady" event after single add', function() {
 			var handler = sinon.stub();
+			var $tr;
 			fileList.setFiles(testFiles);
 			fileList.$fileList.on('fileActionsReady', handler);
-			fileList.add({name: 'test.txt'});
+			$tr = fileList.add({name: 'test.txt'});
+			expect(handler.calledOnce).toEqual(true);
+			expect(handler.getCall(0).args[0].$files.is($tr)).toEqual(true);
+		});
+		it('triggers "fileActionsReady" event after next page load with the newly appended files', function() {
+			var handler = sinon.stub();
+			fileList.setFiles(generateFiles(0, 64));
+			fileList.$fileList.on('fileActionsReady', handler);
+			fileList._nextPage();
 			expect(handler.calledOnce).toEqual(true);
+			expect(handler.getCall(0).args[0].$files.length).toEqual(fileList.pageSize);
 		});
 		it('does not trigger "fileActionsReady" event after single add with silent argument', function() {
 			var handler = sinon.stub();
diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js
index 2efed5310bc..e46be4ada46 100644
--- a/apps/files_sharing/js/share.js
+++ b/apps/files_sharing/js/share.js
@@ -36,54 +36,34 @@
 					}
 					return tr;
 				};
-
-				var oldRenderRow = OCA.Files.FileList.prototype._renderRow;
-				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.
-							// 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 shareBy = t('files_sharing', 'Shared by {owner}', {owner: escapeHTML(fileData.shareOwner)});
-								var $result = $(shareNotification + '<span> ' + shareBy + '</span></span>');
-								$result.on('click', function() {
-									return false;
-								});
-								return $result;
-							});
-						}
-					}
-					return $tr;
-				};
 			}
 
 			// use delegate to catch the case with multiple file lists
-			$('#content').delegate('#fileList', 'fileActionsReady',function(ev){
+			$('#content').delegate('#fileList', 'fileActionsReady', function(ev){
 				var fileList = ev.fileList;
+				var $files = ev.$files;
+
+				function updateIcons($files) {
+					if (!$files) {
+						// if none specified, update all
+						$files = fileList.$fileList.find('tr');
+					}
+					_.each($files, function(file) {
+						OCA.Sharing.Util.updateFileActionIcon($(file));
+					});
+				}
+
 				if (!OCA.Sharing.sharesLoaded){
-					OC.Share.loadIcons('file', fileList);
+					OC.Share.loadIcons('file', fileList, function() {
+						// since we don't know which files are affected, just refresh them all
+						updateIcons();
+					});
 					// assume that we got all shares, so switching directories
 					// will not invalidate that list
 					OCA.Sharing.sharesLoaded = true;
 				}
 				else{
-					// this will update the icons for all the currently visible elements
-					// additionally added elements when scrolling down will be
-					// updated in the _renderRow override
-					OC.Share.updateIcons('file', fileList);
+					updateIcons($files);
 				}
 			});
 
@@ -141,6 +121,40 @@
 		},
 
 		/**
+		 * Update the file action share icon for the given file
+		 *
+		 * @param $tr file element of the file to update
+		 */
+		updateFileActionIcon: function($tr) {
+			// if the statuses are loaded already, use them for the icon
+			// (needed when scrolling to the next page)
+			var shareStatus = OC.Share.statuses[$tr.data('id')];
+			if (shareStatus || $tr.attr('data-share-recipients') || $tr.attr('data-share-owner')) {
+				var permissions = $tr.data('permissions');
+				var hasLink = !!(shareStatus && shareStatus.link);
+				OC.Share.markFileAsShared($tr, true, hasLink);
+				if ((permissions & OC.PERMISSION_SHARE) === 0) {
+					// 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.
+					// TODO: make this work like/with OC.Share.markFileAsShared()
+					$tr.find('.fileactions .action-share-notification').remove();
+					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 shareBy = t('files_sharing', 'Shared by {owner}', {owner: escapeHTML($tr.attr('data-share-owner'))});
+						var $result = $(shareNotification + '<span> ' + shareBy + '</span></span>');
+						$result.on('click', function() {
+							return false;
+						});
+						return $result;
+					});
+				}
+			}
+		},
+
+		/**
 		 * 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
diff --git a/apps/files_sharing/tests/js/shareSpec.js b/apps/files_sharing/tests/js/shareSpec.js
index 3d4fc754821..600859b4e7d 100644
--- a/apps/files_sharing/tests/js/shareSpec.js
+++ b/apps/files_sharing/tests/js/shareSpec.js
@@ -80,6 +80,14 @@ describe('OCA.Sharing.Util tests', function() {
 		// TODO: test data-permissions, data-share-owner, etc
 	});
 	describe('Share action icon', function() {
+		beforeEach(function() {
+			OC.Share.statuses = {1: {link: false, path: '/subdir'}};
+			OCA.Sharing.sharesLoaded = true;
+		});
+		afterEach(function() {
+			OC.Share.statuses = {};
+			OCA.Sharing.sharesLoaded = false;
+		});
 		it('do not shows share text when not shared', function() {
 			var $action, $tr;
 			OC.Share.statuses = {};
diff --git a/core/js/share.js b/core/js/share.js
index 5763664c5de..62271a43ceb 100644
--- a/core/js/share.js
+++ b/core/js/share.js
@@ -29,10 +29,13 @@ OC.Share={
 	 * files "Share" icon to "Shared" according to their share status and
 	 * share type.
 	 *
+	 * If a callback is specified, the update step is skipped.
+	 *
 	 * @param itemType item type
 	 * @param fileList file list instance, defaults to OCA.Files.App.fileList
+	 * @param callback function to call after the shares were loaded
 	 */
-	loadIcons:function(itemType, fileList) {
+	loadIcons:function(itemType, fileList, callback) {
 		// Load all share icons
 		$.get(
 			OC.filePath('core', 'ajax', 'share.php'),
@@ -45,7 +48,11 @@ OC.Share={
 					$.each(result.data, function(item, data) {
 						OC.Share.statuses[item] = data;
 					});
-					OC.Share.updateIcons(itemType, fileList);
+					if (_.isFunction(callback)) {
+						callback(OC.Share.statuses);
+					} else {
+						OC.Share.updateIcons(itemType, fileList);
+					}
 				}
 			}
 		);