Merge pull request #582 from nextcloud/checks-from-js

Generate the checks list in JS

7 files changed, 58 insertions, 77 deletions

diff --git a/apps/workflowengine/appinfo/routes.php b/apps/workflowengine/appinfo/routes.php
index 69478b1715c..b8c9ae1c236 100644
--- a/apps/workflowengine/appinfo/routes.php
+++ b/apps/workflowengine/appinfo/routes.php
@@ -21,7 +21,6 @@
 
 return [
 	'routes' => [
-		['name' => 'flowOperations#getChecks', 'url' => '/checks', 'verb' => 'GET'], // TODO rm and do via js?
 		['name' => 'flowOperations#getOperations', 'url' => '/operations', 'verb' => 'GET'],
 		['name' => 'flowOperations#addOperation', 'url' => '/operations', 'verb' => 'POST'],
 		['name' => 'flowOperations#updateOperation', 'url' => '/operations/{id}', 'verb' => 'PUT'],
diff --git a/apps/workflowengine/js/admin.js b/apps/workflowengine/js/admin.js
index f51352c45e4..48d1592b457 100644
--- a/apps/workflowengine/js/admin.js
+++ b/apps/workflowengine/js/admin.js
@@ -28,11 +28,27 @@
 	});
 
 	Handlebars.registerHelper('getOperators', function(classname) {
-		return OCA.WorkflowEngine.availableChecks
-			.getOperatorsByClassName(classname);
+		var check = OCA.WorkflowEngine.getCheckByClass(classname);
+		if (!_.isUndefined(check)) {
+			return check['operators'];
+		}
+		return [];
 	});
 
-	OCA.WorkflowEngine = OCA.WorkflowEngine || {};
+	OCA.WorkflowEngine = _.extend(OCA.WorkflowEngine || {}, {
+			availablePlugins: [],
+			availableChecks: [],
+
+			getCheckByClass: function(className) {
+				var length = OCA.WorkflowEngine.availableChecks.length;
+				for (var i = 0; i < length; i++) {
+					if (OCA.WorkflowEngine.availableChecks[i]['class'] === className) {
+						return OCA.WorkflowEngine.availableChecks[i];
+					}
+				}
+				return undefined;
+			}
+	});
 
 	/**
 	 * 888b     d888               888          888
@@ -87,22 +103,6 @@
 		});
 
 	/**
-	 * @class OCA.WorkflowEngine.AvailableChecksCollection
-	 *
-	 * collection for all available checks
-	 */
-	OCA.WorkflowEngine.AvailableChecksCollection =
-		OC.Backbone.Collection.extend({
-			model: OCA.WorkflowEngine.AvailableCheck,
-			url: OC.generateUrl('apps/workflowengine/checks'),
-			getOperatorsByClassName: function(classname) {
-				return OCA.WorkflowEngine.availableChecks
-					.findWhere({'class': classname})
-					.get('operators');
-			}
-		});
-
-	/**
 	 * 888     888 d8b
 	 * 888     888 Y8P
 	 * 888     888
@@ -154,7 +154,6 @@
 			message: '',
 			errorMessage: '',
 			saving: false,
-			plugins: [],
 			initialize: function() {
 				// this creates a new copy of the object to definitely have a new reference and being able to reset the model
 				this.originalModel = JSON.parse(JSON.stringify(this.model));
@@ -167,13 +166,6 @@
 				if (this.model.get('id') === undefined) {
 					this.hasChanged = true;
 				}
-
-				this.plugins = OC.Plugins.getPlugins('OCA.WorkflowEngine.CheckPlugins');
-				_.each(this.plugins, function(plugin) {
-					if (_.isFunction(plugin.initialize)) {
-						plugin.initialize();
-					}
-				});
 			},
 			delete: function() {
 				this.model.destroy();
@@ -209,13 +201,12 @@
 			},
 			add: function() {
 				var checks = _.clone(this.model.get('checks')),
-					classname = OCA.WorkflowEngine.availableChecks.at(0).get('class'),
-					operators = OCA.WorkflowEngine.availableChecks
-						.getOperatorsByClassName(classname);
+					classname = OCA.WorkflowEngine.availableChecks[0]['class'],
+					operators = OCA.WorkflowEngine.availableChecks[0]['operators'];
 
 				checks.push({
 					'class': classname,
-					'operator': operators[0],
+					'operator': operators[0]['operator'],
 					'value': ''
 				});
 				this.model.set({'checks': checks});
@@ -249,9 +240,10 @@
 				// if the class is changed most likely also the operators have changed
 				// with this we set the operator to the first possible operator
 				if (key === 'class') {
-					var operators = OCA.WorkflowEngine.availableChecks
-						.getOperatorsByClassName(value);
-					checks[id]['operator'] = operators[0];
+					var check = OCA.WorkflowEngine.getCheckByClass(value);
+					if (!_.isUndefined(check)) {
+						checks[id]['operator'] = check['operators'][0]['operator'];
+					}
 				}
 				// model change will trigger render
 				this.model.set({'checks': checks});
@@ -294,7 +286,7 @@
 			render: function() {
 				this.$el.html(this.template({
 					operation: this.model.toJSON(),
-					classes: OCA.WorkflowEngine.availableChecks.toJSON(),
+					classes: OCA.WorkflowEngine.availableChecks,
 					hasChanged: this.hasChanged,
 					message: this.message,
 					errorMessage: this.errorMessage,
@@ -308,7 +300,7 @@
 						check = checks[id],
 						valueElement = $element.find('.check-value').first();
 
-					_.each(this.plugins, function(plugin) {
+					_.each(OCA.WorkflowEngine.availablePlugins, function(plugin) {
 						if (_.isFunction(plugin.render)) {
 							plugin.render(valueElement, check['class'], check['value']);
 						}
@@ -334,6 +326,8 @@
 	OCA.WorkflowEngine.OperationsView =
 		OCA.WorkflowEngine.TemplateView.extend({
 			templateId: '#operations-template',
+			collection: null,
+			$el: null,
 			events: {
 				'click .button-add-operation': 'add'
 			},
@@ -341,6 +335,13 @@
 				this._initialize('OCA\\WorkflowEngine\\Operation');
 			},
 			_initialize: function(classname) {
+				OCA.WorkflowEngine.availablePlugins = OC.Plugins.getPlugins('OCA.WorkflowEngine.CheckPlugins');
+				_.each(OCA.WorkflowEngine.availablePlugins, function(plugin) {
+					if (_.isFunction(plugin.getCheck)) {
+						OCA.WorkflowEngine.availableChecks.push(plugin.getCheck());
+					}
+				});
+
 				this.collection.fetch({data: {
 					'class': classname
 				}});
diff --git a/apps/workflowengine/js/usergroupmembershipplugin.js b/apps/workflowengine/js/usergroupmembershipplugin.js
index 2a6068cda90..528a7bd3e3d 100644
--- a/apps/workflowengine/js/usergroupmembershipplugin.js
+++ b/apps/workflowengine/js/usergroupmembershipplugin.js
@@ -24,6 +24,16 @@
 	OCA.WorkflowEngine.Plugins = OCA.WorkflowEngine.Plugins || {};
 
 	OCA.WorkflowEngine.Plugins.UserGroupMembershipPlugin = {
+		getCheck: function() {
+			return {
+				'class': 'OCA\\WorkflowEngine\\Check\\UserGroupMembership',
+				'name': t('workflowengine', 'User group membership'),
+				'operators': [
+					{'operator': 'is', 'name': t('workflowengine', 'is member of')},
+					{'operator': '!is', 'name': t('workflowengine', 'is not member of')}
+				]
+			};
+		},
 		render: function(element, classname, value) {
 			if (classname !== 'OCA\\WorkflowEngine\\Check\\UserGroupMembership') {
 				return;
diff --git a/apps/workflowengine/lib/AppInfo/Application.php b/apps/workflowengine/lib/AppInfo/Application.php
index c196ecd955c..84339503047 100644
--- a/apps/workflowengine/lib/AppInfo/Application.php
+++ b/apps/workflowengine/lib/AppInfo/Application.php
@@ -38,18 +38,6 @@ class Application extends \OCP\AppFramework\App {
 	public function registerHooksAndListeners() {
 		$dispatcher = $this->getContainer()->getServer()->getEventDispatcher();
 		$dispatcher->addListener(
-			'OCP\WorkflowEngine\RegisterCheckEvent',
-			function(RegisterCheckEvent $event) {
-				$event->addCheck(
-					'OCA\WorkflowEngine\Check\UserGroupMembership',
-					'User group membership',
-					['is', '!is']
-				);
-			},
-			-100
-		);
-
-		$dispatcher->addListener(
 			'OCP\WorkflowEngine::loadAdditionalSettingScripts',
 			function() {
 				Util::addStyle('workflowengine', 'admin');
diff --git a/apps/workflowengine/lib/Controller/FlowOperations.php b/apps/workflowengine/lib/Controller/FlowOperations.php
index e0836c727a2..94b8b9ddc79 100644
--- a/apps/workflowengine/lib/Controller/FlowOperations.php
+++ b/apps/workflowengine/lib/Controller/FlowOperations.php
@@ -26,38 +26,19 @@ use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\IRequest;
-use OCP\WorkflowEngine\RegisterCheckEvent;
-use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 
 class FlowOperations extends Controller {
 
 	/** @var Manager */
 	protected $manager;
 
-	/** @var EventDispatcherInterface */
-	protected $dispatcher;
-
 	/**
 	 * @param IRequest $request
 	 * @param Manager $manager
-	 * @param EventDispatcherInterface $dispatcher
 	 */
-	public function __construct(IRequest $request, Manager $manager, EventDispatcherInterface $dispatcher) {
+	public function __construct(IRequest $request, Manager $manager) {
 		parent::__construct('workflowengine', $request);
 		$this->manager = $manager;
-		$this->dispatcher = $dispatcher;
-	}
-
-	/**
-	 * @NoCSRFRequired
-	 *
-	 * @return JSONResponse
-	 */
-	public function getChecks() {
-		$event = new RegisterCheckEvent();
-		$this->dispatcher->dispatch('OCP\WorkflowEngine\RegisterCheckEvent', $event);
-
-		return new JSONResponse($event->getChecks());
 	}
 
 	/**
diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php
index 98c34e894cc..f2a04dfb0fa 100644
--- a/apps/workflowengine/lib/Manager.php
+++ b/apps/workflowengine/lib/Manager.php
@@ -110,8 +110,8 @@ class Manager implements IManager {
 			$checkInstance->setFileInfo($this->storage, $this->path);
 			return $checkInstance->executeCheck($check['operator'], $check['value']);
 		} else {
-			// Check is invalid, assume it matches.
-			return true;
+			// Check is invalid
+			throw new \RuntimeException('Check ' . htmlspecialchars($check['class']) . ' is invalid or does not exist');
 		}
 	}
 
@@ -258,10 +258,12 @@ class Manager implements IManager {
 		}
 		$result->closeCursor();
 
-		// TODO What if a check is missing? Should we throw?
-		// As long as we only allow AND-concatenation of checks, a missing check
-		// is like a matching check, so it evaluates to true and therefor blocks
-		// access. So better save than sorry.
+		$checkIds = array_diff($checkIds, array_keys($checks));
+
+		if (!empty($checkIds)) {
+			$missingCheck = array_pop($checkIds);
+			throw new \RuntimeException('Check #' . htmlspecialchars($missingCheck) . ' is invalid or does not exist');
+		}
 
 		return $checks;
 	}
diff --git a/apps/workflowengine/templates/admin.php b/apps/workflowengine/templates/admin.php
index 86ecfe08556..9e01f8359fa 100644
--- a/apps/workflowengine/templates/admin.php
+++ b/apps/workflowengine/templates/admin.php
@@ -22,7 +22,7 @@
 /** @var array $_ */
 /** @var OC_L10N $l */
 ?>
-<div id="workflowengine" class="section workflowengine">
+<div id="<?php p($_['appid']); ?>" class="section workflowengine">
 	<h2 class="inlineblock"><?php p($_['heading']); ?></h2>
 	<script type="text/template" id="operations-template">
 		<div class="operations"></div>
@@ -48,7 +48,7 @@
 					</select>
 					<select class="check-operator">
 						{{#each (getOperators class)}}
-						<option value="{{this}}" {{selectItem this ../operator}}>{{this}}</option>
+						<option value="{{operator}}" {{selectItem operator ../operator}}>{{name}}</option>
 						{{/each}}
 					</select>
 					<input type="text" class="check-value" value="{{value}}">