Merge pull request #9254 from owncloud/fileactions-deferred

Sync file list with file actions

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

@@ -167,12 +167,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);
 		},
 
 		/**
@@ -502,6 +512,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

@@ -1628,6 +1628,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();