Merge pull request #7591 from nextcloud/trigger-events-before-and-aft…

…er-a-file-action-is-executed Trigger events before and after a file action is executed
nextcloud · Dec 22, 2017 · e37fa60 · e37fa60
2 parents e40d6f6 + c059fbd
commit e37fa60
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 14 deletions.
diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
@@ -47,14 +47,6 @@
 		 */
 		$el: null,
 
-		/**
-		 * List of handlers to be notified whenever a register() or
-		 * setDefault() was called.
-		 *
-		 * @member {Function[]}
-		 */
-		_updateListeners: {},
-
 		_fileActionTriggerTemplate: null,
 
 		/**
@@ -142,7 +134,22 @@
 			var mime = action.mime;
 			var name = action.name;
 			var actionSpec = {
-				action: action.actionHandler,
+				action: function(fileName, context) {
+					// Actions registered in one FileAction may be executed on a
+					// different one (for example, due to the "merge" function),
+					// so the listeners have to be updated on the FileActions
+					// from the context instead of on the one in which it was
+					// originally registered.
+					if (context && context.fileActions) {
+						context.fileActions._notifyUpdateListeners('beforeTriggerAction', {action: actionSpec, fileName: fileName, context: context});
+					}
+
+					action.actionHandler(fileName, context);
+
+					if (context && context.fileActions) {
+						context.fileActions._notifyUpdateListeners('afterTriggerAction', {action: actionSpec, fileName: fileName, context: context});
+					}
+				},
 				name: name,
 				displayName: action.displayName,
 				mime: mime,
@@ -174,7 +181,6 @@
 			this.defaults = {};
 			this.icons = {};
 			this.currentFile = null;
-			this._updateListeners = [];
 		},
 		/**
 		 * Sets the default action for a given mime type.

diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
@@ -676,8 +676,25 @@
 						$(event.target).closest('a').blur();
 					}
 				} else {
-					this._updateDetailsView($tr.attr('data-file'));
+					// Even if there is no Details action the default event
+					// handler is prevented for consistency (although there
+					// should always be a Details action); otherwise the link
+					// would be downloaded by the browser when the user expected
+					// the details to be shown.
 					event.preventDefault();
+					var filename = $tr.attr('data-file');
+					var mime = this.fileActions.getCurrentMimeType();
+					var type = this.fileActions.getCurrentType();
+					var permissions = this.fileActions.getCurrentPermissions();
+					var action = this.fileActions.get(mime, type, permissions)['Details'];
+					if (action) {
+						action(filename, {
+							$file: $tr,
+							fileList: this,
+							fileActions: this.fileActions,
+							dir: $tr.attr('data-path') || this.getCurrentDirectory()
+						});
+					}
 				}
 			}
 		},

diff --git a/apps/files/tests/js/appSpec.js b/apps/files/tests/js/appSpec.js
@@ -112,9 +112,22 @@ describe('OCA.Files.App tests', function() {
 			App.initialize();
 
 			var actions = App.fileList.fileActions.actions;
-			expect(actions.all.OverwriteThis.action).toBe(actionStub);
-			expect(actions.all.LegacyTest.action).toBe(legacyActionStub);
-			expect(actions.all.RegularTest.action).toBe(actionStub);
+			var context = { fileActions: sinon.createStubInstance(OCA.Files.FileActions) };
+			actions.all.OverwriteThis.action('testFileName', context);
+			expect(actionStub.calledOnce).toBe(true);
+			expect(context.fileActions._notifyUpdateListeners.callCount).toBe(2);
+			expect(context.fileActions._notifyUpdateListeners.getCall(0).calledWith('beforeTriggerAction')).toBe(true);
+			expect(context.fileActions._notifyUpdateListeners.getCall(1).calledWith('afterTriggerAction')).toBe(true);
+			actions.all.LegacyTest.action('testFileName', context);
+			expect(legacyActionStub.calledOnce).toBe(true);
+			expect(context.fileActions._notifyUpdateListeners.callCount).toBe(4);
+			expect(context.fileActions._notifyUpdateListeners.getCall(2).calledWith('beforeTriggerAction')).toBe(true);
+			expect(context.fileActions._notifyUpdateListeners.getCall(3).calledWith('afterTriggerAction')).toBe(true);
+			actions.all.RegularTest.action('testFileName', context);
+			expect(actionStub.calledTwice).toBe(true);
+			expect(context.fileActions._notifyUpdateListeners.callCount).toBe(6);
+			expect(context.fileActions._notifyUpdateListeners.getCall(4).calledWith('beforeTriggerAction')).toBe(true);
+			expect(context.fileActions._notifyUpdateListeners.getCall(5).calledWith('afterTriggerAction')).toBe(true);
 			// default one still there
 			expect(actions.dir.Open.action).toBeDefined();
 		});

diff --git a/apps/files/tests/js/fileactionsSpec.js b/apps/files/tests/js/fileactionsSpec.js
@@ -299,6 +299,7 @@ describe('OCA.Files.FileActions tests', function() {
 			clock.restore();
 		});
 		it('passes context to action handler', function() {
+			var notifyUpdateListenersSpy = sinon.spy(fileList.fileActions, '_notifyUpdateListeners');
 			$tr.find('.action-test').click();
 			expect(actionStub.calledOnce).toEqual(true);
 			expect(actionStub.getCall(0).args[0]).toEqual('testName.txt');
@@ -309,6 +310,22 @@ describe('OCA.Files.FileActions tests', function() {
 			expect(context.dir).toEqual('/subdir');
 			expect(context.fileInfoModel.get('name')).toEqual('testName.txt');
 
+			expect(notifyUpdateListenersSpy.calledTwice).toEqual(true);
+			expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true);
+			expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true);
+			expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction');
+			expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({
+				action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'],
+				fileName: 'testName.txt',
+				context: context
+			});
+			expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction');
+			expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({
+				action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'],
+				fileName: 'testName.txt',
+				context: context
+			});
+
 			// when data-path is defined
 			actionStub.reset();
 			$tr.attr('data-path', '/somepath');
@@ -317,6 +334,7 @@ describe('OCA.Files.FileActions tests', function() {
 			expect(context.dir).toEqual('/somepath');
 		});
 		it('also triggers action handler when calling triggerAction()', function() {
+			var notifyUpdateListenersSpy = sinon.spy(fileList.fileActions, '_notifyUpdateListeners');
 			var model = new OCA.Files.FileInfoModel({
 				id: 1,
 				name: 'Test.txt',
@@ -331,7 +349,62 @@ describe('OCA.Files.FileActions tests', function() {
 			expect(actionStub.getCall(0).args[1].fileList).toEqual(fileList);
 			expect(actionStub.getCall(0).args[1].fileActions).toEqual(fileActions);
 			expect(actionStub.getCall(0).args[1].fileInfoModel).toEqual(model);
+
+			expect(notifyUpdateListenersSpy.calledTwice).toEqual(true);
+			expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true);
+			expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true);
+			expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction');
+			expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({
+				action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'],
+				fileName: 'Test.txt',
+				context: {
+					fileActions: fileActions,
+					fileInfoModel: model,
+					dir: '/subdir',
+					fileList: fileList,
+					$file: fileList.findFileEl('Test.txt')
+				}
+			});
+			expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction');
+			expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({
+				action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'],
+				fileName: 'Test.txt',
+				context: {
+					fileActions: fileActions,
+					fileInfoModel: model,
+					dir: '/subdir',
+					fileList: fileList,
+					$file: fileList.findFileEl('Test.txt')
+				}
+			});
 		});
+		it('triggers listener events when invoked directly', function() {
+			var context = {fileActions: new OCA.Files.FileActions()}
+			var notifyUpdateListenersSpy = sinon.spy(context.fileActions, '_notifyUpdateListeners');
+			var testAction = fileActions.get('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'];
+
+			testAction('Test.txt', context);
+
+			expect(actionStub.calledOnce).toEqual(true);
+			expect(actionStub.getCall(0).args[0]).toEqual('Test.txt');
+			expect(actionStub.getCall(0).args[1]).toBe(context);
+
+			expect(notifyUpdateListenersSpy.calledTwice).toEqual(true);
+			expect(notifyUpdateListenersSpy.calledBefore(actionStub)).toEqual(true);
+			expect(notifyUpdateListenersSpy.calledAfter(actionStub)).toEqual(true);
+			expect(notifyUpdateListenersSpy.getCall(0).args[0]).toEqual('beforeTriggerAction');
+			expect(notifyUpdateListenersSpy.getCall(0).args[1]).toEqual({
+				action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'],
+				fileName: 'Test.txt',
+				context: context
+			});
+			expect(notifyUpdateListenersSpy.getCall(1).args[0]).toEqual('afterTriggerAction');
+			expect(notifyUpdateListenersSpy.getCall(1).args[1]).toEqual({
+				action: fileActions.getActions('all', OCA.Files.FileActions.TYPE_INLINE, OC.PERMISSION_READ)['Test'],
+				fileName: 'Test.txt',
+				context: context
+			});
+		}),
 		describe('actions menu', function() {
 			it('shows actions menu inside row when clicking the menu trigger', function() {
 				expect($tr.find('td.filename .fileActionsMenu').length).toEqual(0);