Merge pull request #9538 from owncloud/fileactions-cleanup

Propagate file action changes to the file lists

7 files changed, 137 insertions, 43 deletions

diff --git a/apps/files/js/app.js b/apps/files/js/app.js
index 45b6b6a0e16..6f5206fcdb6 100644
--- a/apps/files/js/app.js
+++ b/apps/files/js/app.js
@@ -32,9 +32,11 @@
 			// 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._onActionsUpdated = _.bind(this._onActionsUpdated, this);
+			OCA.Files.fileActions.on('setDefault.app-files', this._onActionsUpdated);
+			OCA.Files.fileActions.on('registerAction.app-files', this._onActionsUpdated);
+			window.FileActions.on('setDefault.app-files', this._onActionsUpdated);
+			window.FileActions.on('registerAction.app-files', this._onActionsUpdated);
 
 			this.files = OCA.Files.Files;
 
@@ -60,6 +62,32 @@
 		},
 
 		/**
+		 * Destroy the app
+		 */
+		destroy: function() {
+			this.navigation = null;
+			this.fileList.destroy();
+			this.fileList = null;
+			this.files = null;
+			OCA.Files.fileActions.off('setDefault.app-files', this._onActionsUpdated);
+			OCA.Files.fileActions.off('registerAction.app-files', this._onActionsUpdated);
+			window.FileActions.off('setDefault.app-files', this._onActionsUpdated);
+			window.FileActions.off('registerAction.app-files', this._onActionsUpdated);
+		},
+
+		_onActionsUpdated: function(ev, newAction) {
+			// forward new action to the file list
+			if (ev.action) {
+				this.fileList.fileActions.registerAction(ev.action);
+			} else if (ev.defaultAction) {
+				this.fileList.fileActions.setDefault(
+					ev.defaultAction.mime,
+					ev.defaultAction.name
+				);
+			}
+		},
+
+		/**
 		 * Returns the container of the currently visible app.
 		 *
 		 * @return app container
diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
index cbfd047e98f..fd038765ea5 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -24,47 +24,51 @@
 		currentFile: null,
 
 		/**
+		 * Dummy jquery element, for events
+		 */
+		$el: null,
+
+		/**
 		 * List of handlers to be notified whenever a register() or
 		 * setDefault() was called.
 		 */
-		_updateListeners: [],
+		_updateListeners: {},
 
 		initialize: function() {
 			this.clear();
+			// abusing jquery for events until we get a real event lib
+			this.$el = $('<div class="dummy-fileactions hidden"></div>');
+			$('body').append(this.$el);
 		},
 
 		/**
-		 * Adds an update listener to be notified whenever register()
-		 * or setDefault() has been called.
+		 * Adds an event handler
 		 *
+		 * @param {String} eventName event name
 		 * @param Function callback
 		 */
-		addUpdateListener: function(callback) {
-			if (!_.isFunction(callback)) {
-				throw 'Argument passed to FileActions.addUpdateListener must be a function';
-			}
-			this._updateListeners.push(callback);
+		on: function(eventName, callback) {
+			this.$el.on(eventName, callback);
 		},
 
 		/**
-		 * Removes an update listener.
+		 * Removes an event handler
 		 *
+		 * @param {String} eventName event name
 		 * @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);
+		off: function(eventName, callback) {
+			this.$el.off(eventName, callback);
 		},
 
 		/**
-		 * Notifies the registered update listeners
+		 * Notifies the event handlers
+		 *
+		 * @param {String} eventName event name
+		 * @param {Object} data data
 		 */
-		_notifyUpdateListeners: function() {
-			for (var i = 0; i < this._updateListeners.length; i++) {
-				this._updateListeners[i](this);
-			}
+		_notifyUpdateListeners: function(eventName, data) {
+			this.$el.trigger(new $.Event(eventName, data));
 		},
 
 		/**
@@ -87,17 +91,44 @@
 			this.defaults = _.extend(this.defaults, fileActions.defaults);
 			this.icons = _.extend(this.icons, fileActions.icons);
 		},
-		register: function (mime, name, permissions, icon, action, displayName) {
+		/**
+		 * @deprecated use #registerAction() instead
+		 */
+		register: function(mime, name, permissions, icon, action, displayName) {
+			return this.registerAction({
+				name: name,
+				mime: mime,
+				permissions: permissions,
+				icon: icon,
+				actionHandler: action,
+				displayName: displayName
+			});
+		},
+		/**
+		 * Register action
+		 *
+		 * @param {Object} action action object
+		 * @param {String} action.name identifier of the action
+		 * @param {String} action.displayName display name of the action, defaults
+		 * to the name given in action.name
+		 * @param {String} action.mime mime type
+		 * @param {int} action.permissions permissions
+		 * @param {(Function|String)} action.icon icon
+		 * @param {Function} action.actionHandler function that performs the action
+		 */
+		registerAction: function (action) {
+			var mime = action.mime;
+			var name = action.name;
 			if (!this.actions[mime]) {
 				this.actions[mime] = {};
 			}
 			this.actions[mime][name] = {
-				action: action,
-				permissions: permissions,
-				displayName: displayName || t('files', name)
+				action: action.actionHandler,
+				permissions: action.permissions,
+				displayName: action.displayName || t('files', name)
 			};
-			this.icons[name] = icon;
-			this._notifyUpdateListeners();
+			this.icons[name] = action.icon;
+			this._notifyUpdateListeners('registerAction', {action: action});
 		},
 		clear: function() {
 			this.actions = {};
@@ -108,7 +139,7 @@
 		},
 		setDefault: function (mime, name) {
 			this.defaults[mime] = name;
-			this._notifyUpdateListeners();
+			this._notifyUpdateListeners('setDefault', {defaultAction: {mime: mime, name: name}});
 		},
 		get: function (mime, type, permissions) {
 			var actions = this.getActions(mime, type, permissions);
diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index 3e311655c91..96436d33a2c 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -172,7 +172,8 @@
 		 */
 		destroy: function() {
 			// TODO: also unregister other event handlers
-			this.fileActions.removeUpdateListener(this._onFileActionsUpdated);
+			this.fileActions.off('registerAction', this._onFileActionsUpdated);
+			this.fileActions.off('setDefault', this._onFileActionsUpdated);
 		},
 
 		_initFileActions: function(fileActions) {
@@ -182,7 +183,8 @@
 				this.fileActions.registerDefaultActions();
 			}
 			this._onFileActionsUpdated = _.debounce(_.bind(this._onFileActionsUpdated, this), 100);
-			this.fileActions.addUpdateListener(this._onFileActionsUpdated);
+			this.fileActions.on('registerAction', this._onFileActionsUpdated);
+			this.fileActions.on('setDefault', this._onFileActionsUpdated);
 		},
 
 		/**
diff --git a/apps/files/tests/js/appSpec.js b/apps/files/tests/js/appSpec.js
index 99a73d0b01d..77d36d59fa9 100644
--- a/apps/files/tests/js/appSpec.js
+++ b/apps/files/tests/js/appSpec.js
@@ -52,9 +52,7 @@ describe('OCA.Files.App tests', function() {
 		App.initialize();
 	});
 	afterEach(function() {
-		App.navigation = null;
-		App.fileList = null;
-		App.files = null;
+		App.destroy();
 
 		pushStateStub.restore();
 		parseUrlQueryStub.restore();
diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js
index f087239de9d..f5f18a45a75 100644
--- a/apps/files/tests/js/fileactionsSpec.js
+++ b/apps/files/tests/js/fileactionsSpec.js
@@ -354,7 +354,7 @@ describe('OCA.Files.FileActions tests', function() {
 		it('notifies update event handlers once after multiple changes', function() {
 			var actionStub = sinon.stub();
 			var handler = sinon.stub();
-			FileActions.addUpdateListener(handler);
+			FileActions.on('registerAction', handler);
 			FileActions.register(
 					'all',
 					'Test',
@@ -374,8 +374,8 @@ describe('OCA.Files.FileActions tests', function() {
 		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.on('registerAction', handler);
+			FileActions.off('registerAction', handler);
 			FileActions.register(
 					'all',
 					'Test',
diff --git a/apps/files_sharing/js/app.js b/apps/files_sharing/js/app.js
index 1fc13d00382..1a3bfac5b97 100644
--- a/apps/files_sharing/js/app.js
+++ b/apps/files_sharing/js/app.js
@@ -92,6 +92,21 @@ OCA.Sharing.App = {
 		}
 	},
 
+	/**
+	 * Destroy the app
+	 */
+	destroy: function() {
+		OCA.Files.fileActions.off('setDefault.app-sharing', this._onActionsUpdated);
+		OCA.Files.fileActions.off('registerAction.app-sharing', this._onActionsUpdated);
+		this.removeSharingIn();
+		this.removeSharingOut();
+		this.removeSharingLinks();
+		this._inFileList = null;
+		this._outFileList = null;
+		this._linkFileList = null;
+		delete this._globalActionsInitialized;
+	},
+
 	_createFileActions: function() {
 		// inherit file actions from the files app
 		var fileActions = new OCA.Files.FileActions();
@@ -100,6 +115,14 @@ OCA.Sharing.App = {
 		fileActions.registerDefaultActions();
 		fileActions.merge(OCA.Files.fileActions);
 
+		if (!this._globalActionsInitialized) {
+			// in case actions are registered later
+			this._onActionsUpdated = _.bind(this._onActionsUpdated, this);
+			OCA.Files.fileActions.on('setDefault.app-sharing', this._onActionsUpdated);
+			OCA.Files.fileActions.on('registerAction.app-sharing', this._onActionsUpdated);
+			this._globalActionsInitialized = true;
+		}
+
 		// when the user clicks on a folder, redirect to the corresponding
 		// folder in the files app instead of opening it directly
 		fileActions.register('dir', 'Open', OC.PERMISSION_READ, '', function (filename, context) {
@@ -110,6 +133,23 @@ OCA.Sharing.App = {
 		return fileActions;
 	},
 
+	_onActionsUpdated: function(ev) {
+		_.each([this._inFileList, this._outFileList, this._linkFileList], function(list) {
+			if (!list) {
+				return;
+			}
+
+			if (ev.action) {
+				list.fileActions.registerAction(ev.action);
+			} else if (ev.defaultAction) {
+				list.fileActions.setDefault(
+					ev.defaultAction.mime,
+					ev.defaultAction.name
+				);
+			}
+		});
+	},
+
 	_extendFileList: function(fileList) {
 		// remove size column from summary
 		fileList.fileSummary.$el.find('.filesize').remove();
diff --git a/apps/files_sharing/tests/js/appSpec.js b/apps/files_sharing/tests/js/appSpec.js
index 5fb269da75e..3f9cc61da5d 100644
--- a/apps/files_sharing/tests/js/appSpec.js
+++ b/apps/files_sharing/tests/js/appSpec.js
@@ -45,12 +45,7 @@ describe('OCA.Sharing.App tests', function() {
 		fileListOut = App.initSharingOut($('#app-content-sharingout'));
 	});
 	afterEach(function() {
-		App._inFileList = null;
-		App._outFileList = null;
-		fileListIn.destroy();
-		fileListOut.destroy();
-		fileListIn = null;
-		fileListOut = null;
+		App.destroy();
 	});
 
 	describe('initialization', function() {