Sync file list with file actions

Whenever file actions are registered later, now the file lists are
automatically notified.

Added FileActions.addUpdateListener() to be able to receive such
notifications.

This removes the need for apps to manually call FileActions.display()
after registering new actions.

This fixes issues with race conditions when file actions are
registered after the file list was already rendered.

apps/files/js/app.js

@@ -32,6 +32,10 @@
 			// regular actions
 			fileActions.merge(OCA.Files.fileActions);
 
+			// in case apps would decide to register file actions later,
+			// replace the global object with this one
+			OCA.Files.fileActions = fileActions;
+
 			this.files = OCA.Files.Files;
 
 			// TODO: ideally these should be in a separate class / app (the embedded "all files" app)

apps/files/js/fileactions.js

@@ -22,9 +22,51 @@
 		defaults: {},
 		icons: {},
 		currentFile: null,
+
+		/**
+		 * List of handlers to be notified whenever a register() or
+		 * setDefault() was called.
+		 */
+		_updateListeners: [],
+
 		initialize: function() {
 			this.clear();
 		},
+
+		/**
+		 * Adds an update listener to be notified whenever register()
+		 * or setDefault() has been called.
+		 *
+		 * @param Function callback
+		 */
+		addUpdateListener: function(callback) {
+			if (!_.isFunction(callback)) {
+				throw 'Argument passed to FileActions.addUpdateListener must be a function';
+			}
+			this._updateListeners.push(callback);
+		},
+
+		/**
+		 * Removes an update listener.
+		 *
+		 * @param Function callback
+		 */
+		removeUpdateListener: function(callback) {
+			if (!_.isFunction(callback)) {
+				throw 'Argument passed to FileActions.removeUpdateListener must be a function';
+			}
+			this._updateListeners = _.without(this._updateListeners, callback);
+		},
+
+		/**
+		 * Notifies the registered update listeners
+		 */
+		_notifyUpdateListeners: function() {
+			for (var i = 0; i < this._updateListeners.length; i++) {
+				this._updateListeners[i](this);
+			}
+		},
+
 		/**
 		 * Merges the actions from the given fileActions into
 		 * this instance.
@@ -59,15 +101,18 @@
 			this.actions[mime][name]['permissions'] = permissions;
 			this.actions[mime][name]['displayName'] = displayName;
 			this.icons[name] = icon;
+			this._notifyUpdateListeners();
 		},
 		clear: function() {
 			this.actions = {};
 			this.defaults = {};
 			this.icons = {};
 			this.currentFile = null;
+			this._updateListeners = [];
 		},
 		setDefault: function (mime, name) {
 			this.defaults[mime] = name;
+			this._notifyUpdateListeners();
 		},
 		get: function (mime, type, permissions) {
 			var actions = this.getActions(mime, type, permissions);
@@ -133,8 +178,7 @@
 		display: function (parent, triggerEvent, fileList) {
 			if (!fileList) {
 				console.warn('FileActions.display() MUST be called with a OCA.Files.FileList instance');
-				// using default list instead, which could be wrong
-				fileList = OCA.Files.App.fileList;
+				return;
 			}
 			this.currentFile = parent;
 			var self = this;
@@ -309,9 +353,10 @@
 				window.FileActions, mime, name, permissions, icon, action, displayName
 		);
 	};
-	window.FileActions.setDefault = function (mime, name) {
-		console.warn('FileActions.setDefault() is deprecated, please use OCA.Files.fileActions.setDefault() instead', mime, name);
-		OCA.Files.FileActions.prototype.setDefault.call(window.FileActions, mime, name);
+	window.FileActions.display = function (parent, triggerEvent, fileList) {
+		fileList = fileList || OCA.Files.App.fileList;
+		console.warn('FileActions.display() is deprecated, please use OCA.Files.fileActions.register() which automatically redisplays actions', mime, name);
+		OCA.Files.FileActions.prototype.display.call(window.FileActions, parent, triggerEvent, fileList);
 	};
 })();
 

apps/files/js/filelist.js

@@ -168,12 +168,22 @@
 			this.$container.on('scroll', _.bind(this._onScroll, this));
 		},
 
+		/**
+		 * Destroy / uninitialize this instance.
+		 */
+		destroy: function() {
+			// TODO: also unregister other event handlers
+			this.fileActions.removeUpdateListener(this._onFileActionsUpdated);
+		},
+
 		_initFileActions: function(fileActions) {
 			this.fileActions = fileActions;
 			if (!this.fileActions) {
 				this.fileActions = new OCA.Files.FileActions();
 				this.fileActions.registerDefaultActions();
 			}
+			this._onFileActionsUpdated = _.debounce(_.bind(this._onFileActionsUpdated, this), 100);
+			this.fileActions.addUpdateListener(this._onFileActionsUpdated);
 		},
 
 		/**
@@ -487,6 +497,18 @@
 			}
 		},
 
+		/**
+		 * Event handler for when file actions were updated.
+		 * This will refresh the file actions on the list.
+		 */
+		_onFileActionsUpdated: function() {
+			console.log('onFileActionsUpdated');
+			var self = this;
+			this.$fileList.find('tr td.filename').each(function() {
+				self.fileActions.display($(this), true, self);
+			});
+		},
+
 		/**
 		 * Sets the files to be displayed in the list.
 		 * This operation will re-render the list and update the summary.

apps/files/tests/js/fileactionsSpec.js

@@ -21,7 +21,7 @@
 
 describe('OCA.Files.FileActions tests', function() {
 	var $filesTable, fileList;
-	var FileActions; 
+	var FileActions;
 
 	beforeEach(function() {
 		// init horrible parameters
@@ -36,6 +36,7 @@ describe('OCA.Files.FileActions tests', function() {
 	});
 	afterEach(function() {
 		FileActions = null;
+		fileList.destroy();
 		fileList = undefined;
 		$('#dir, #permissions, #filestable').remove();
 	});
@@ -192,4 +193,54 @@ describe('OCA.Files.FileActions tests', function() {
 		context = actionStub.getCall(0).args[1];
 		expect(context.dir).toEqual('/somepath');
 	});
+	describe('events', function() {
+		var clock;
+		beforeEach(function() {
+			clock = sinon.useFakeTimers();
+		});
+		afterEach(function() {
+			clock.restore();
+		});
+		it('notifies update event handlers once after multiple changes', function() {
+			var actionStub = sinon.stub();
+			var handler = sinon.stub();
+			FileActions.addUpdateListener(handler);
+			FileActions.register(
+					'all',
+					'Test',
+					OC.PERMISSION_READ,
+					OC.imagePath('core', 'actions/test'),
+					actionStub
+			);
+			FileActions.register(
+					'all',
+					'Test2',
+					OC.PERMISSION_READ,
+					OC.imagePath('core', 'actions/test'),
+					actionStub
+			);
+			expect(handler.calledTwice).toEqual(true);
+		});
+		it('does not notifies update event handlers after unregistering', function() {
+			var actionStub = sinon.stub();
+			var handler = sinon.stub();
+			FileActions.addUpdateListener(handler);
+			FileActions.removeUpdateListener(handler);
+			FileActions.register(
+					'all',
+					'Test',
+					OC.PERMISSION_READ,
+					OC.imagePath('core', 'actions/test'),
+					actionStub
+			);
+			FileActions.register(
+					'all',
+					'Test2',
+					OC.PERMISSION_READ,
+					OC.imagePath('core', 'actions/test'),
+					actionStub
+			);
+			expect(handler.notCalled).toEqual(true);
+		});
+	});
 });

apps/files/tests/js/filelistSpec.js

@@ -1623,6 +1623,38 @@ describe('OCA.Files.FileList tests', function() {
 			expect(context.fileActions).toBeDefined();
 			expect(context.dir).toEqual('/subdir');
 		});
+		it('redisplays actions when new actions have been registered', function() {
+			var actionStub = sinon.stub();
+			var clock = sinon.useFakeTimers();
+			var debounceStub = sinon.stub(_, 'debounce', function(callback) {
+				return function() {
+					// defer instead of debounce, to make it work with clock
+					_.defer(callback);
+				};
+			});
+			// need to reinit the list to make the debounce call
+			fileList.destroy();
+			fileList = new OCA.Files.FileList($('#app-content-files'));
+
+			fileList.setFiles(testFiles);
+			fileList.fileActions.register(
+				'text/plain',
+				'Test',
+				OC.PERMISSION_ALL,
+				function() {
+					// Specify icon for hitory button
+					return OC.imagePath('core','actions/history');
+				},
+				actionStub
+			);
+			var $tr = fileList.findFileEl('One.txt');
+			expect($tr.find('.action-test').length).toEqual(0);
+			// update is delayed
+			clock.tick(100);
+			expect($tr.find('.action-test').length).toEqual(1);
+			clock.restore();
+			debounceStub.restore();
+		});
 	});
 	describe('Sorting files', function() {
 		it('Sorts by name by default', function() {

apps/files_external/tests/appSpec.js

@@ -42,6 +42,7 @@ describe('OCA.External.App tests', function() {
 	});
 	afterEach(function() {
 		App.fileList = null;
+		fileList.destroy();
 		fileList = null;
 	});
 

apps/files_external/tests/js/mountsfilelistSpec.js

@@ -54,6 +54,7 @@ describe('OCA.External.FileList tests', function() {
 	afterEach(function() {
 		OCA.Files.FileList.prototype = oldFileListPrototype;
 		testFiles = undefined;
+		fileList.destroy();
 		fileList = undefined;
 		fileActions = undefined;
 

apps/files_sharing/tests/js/appSpec.js

@@ -47,6 +47,8 @@ describe('OCA.Sharing.App tests', function() {
 	afterEach(function() {
 		App._inFileList = null;
 		App._outFileList = null;
+		fileListIn.destroy();
+		fileListOut.destroy();
 		fileListIn = null;
 		fileListOut = null;
 	});

apps/files_sharing/tests/js/shareSpec.js

@@ -70,6 +70,8 @@ describe('OCA.Sharing.Util tests', function() {
 		OCA.Files.FileList.prototype = oldFileListPrototype;
 		delete OCA.Sharing.sharesLoaded;
 		delete OC.Share.droppedDown;
+		fileList.destroy();
+		fileList = null;
 		OC.Share.statuses = {};
 		OC.Share.currentShares = {};
 	});

apps/files_sharing/tests/js/sharedfilelistSpec.js

@@ -55,6 +55,7 @@ describe('OCA.Sharing.FileList tests', function() {
 	afterEach(function() {
 		OCA.Files.FileList.prototype = oldFileListPrototype;
 		testFiles = undefined;
+		fileList.destroy();
 		fileList = undefined;
 		fileActions = undefined;
 

apps/files_trashbin/tests/js/filelistSpec.js

@@ -96,6 +96,7 @@ describe('OCA.Trashbin.FileList tests', function() {
 	});
 	afterEach(function() {
 		testFiles = undefined;
+		fileList.destroy();
 		fileList = undefined;
 
 		$('#dir').remove();