From a55c17d73fa7bfa9e2780fce3ea9a7770a11d8d3 Mon Sep 17 00:00:00 2001
From: Simon Brunel <simonbrunel@users.noreply.github.com>
Date: Fri, 10 Jun 2016 22:26:55 +0200
Subject: [PATCH] Enhance plugin notification system

Change the plugin notification behavior: this method now returns false as soon as a plugin *explicitly* returns false, else returns true. Also, plugins are now called in their own scope (so remove the never used `scope` parameter).
---
 src/core/core.plugin.js   | 31 +++++++++++++++++++++----------
 test/core.plugin.tests.js | 39 +++++++++++++++++++++++++++++----------
 2 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/src/core/core.plugin.js b/src/core/core.plugin.js
index 7126dcd43..2b020ac68 100644
--- a/src/core/core.plugin.js
+++ b/src/core/core.plugin.js
@@ -2,8 +2,7 @@
 
 module.exports = function(Chart) {
 
-	var helpers = Chart.helpers;
-	var noop = helpers.noop;
+	var noop = Chart.helpers.noop;
 
 	/**
 	 * The plugin service singleton
@@ -30,21 +29,33 @@ module.exports = function(Chart) {
 		},
 
 		/**
-		 * Calls registered plugins on the specified method, with the given args. This
-		 * method immediately returns as soon as a plugin explicitly returns false.
+		 * Calls registered plugins on the specified extension, with the given args. This
+		 * method immediately returns as soon as a plugin explicitly returns false. The
+		 * returned value can be used, for instance, to interrupt the current action.
+		 * @param {String} extension the name of the plugin method to call (e.g. 'beforeUpdate').
+		 * @param {Array} [args] extra arguments to apply to the extension call.
 		 * @returns {Boolean} false if any of the plugins return false, else returns true.
 		 */
-		notify: function(method, args, scope) {
-			helpers.each(this._plugins, function(plugin) {
-				if (plugin[method] && typeof plugin[method] === 'function') {
-					plugin[method].apply(scope, args);
+		notify: function(extension, args) {
+			var plugins = this._plugins;
+			var ilen = plugins.length;
+			var i, plugin;
+
+			for (i=0; i<ilen; ++i) {
+				plugin = plugins[i];
+				if (typeof plugin[extension] === 'function') {
+					if (plugin[extension].apply(plugin, args || []) === false) {
+						return false;
+					}
 				}
-			}, scope);
+			}
+
+			return true;
 		}
 	};
 
 	Chart.PluginBase = Chart.Element.extend({
-		// Plugin methods. All functions are passed the chart instance
+		// Plugin extensions. All functions are passed the chart instance
 
 		// Called at start of chart init
 		beforeInit: noop,
diff --git a/test/core.plugin.tests.js b/test/core.plugin.tests.js
index 62e5e2188..c3c290128 100644
--- a/test/core.plugin.tests.js
+++ b/test/core.plugin.tests.js
@@ -37,17 +37,36 @@ describe('Test the plugin system', function() {
 		Chart.plugins.remove(myplugin);
 	});
 
-	it ('Should allow notifying plugins', function() {
-		var myplugin = {
-			count: 0,
-			trigger: function(chart) {
-				myplugin.count = chart.count;
-			}
-		};
-		Chart.plugins.register(myplugin);
+	describe('Chart.plugins.notify', function() {
+		it ('should call plugins with arguments', function() {
+			var myplugin = {
+				count: 0,
+				trigger: function(chart) {
+					myplugin.count = chart.count;
+				}
+			};
 
-		Chart.plugins.notify('trigger', [{ count: 10 }]);
+			Chart.plugins.register(myplugin);
+			Chart.plugins.notify('trigger', [{ count: 10 }]);
+			expect(myplugin.count).toBe(10);
+		});
 
-		expect(myplugin.count).toBe(10);
+		it('should return TRUE if no plugin explicitly returns FALSE', function() {
+			Chart.plugins.register({ check: function() {} });
+			Chart.plugins.register({ check: function() { return; } });
+			Chart.plugins.register({ check: function() { return null; } });
+			Chart.plugins.register({ check: function() { return 42 } });
+			var res = Chart.plugins.notify('check');
+			expect(res).toBeTruthy();
+		});
+
+		it('should return FALSE if no plugin explicitly returns FALSE', function() {
+			Chart.plugins.register({ check: function() {} });
+			Chart.plugins.register({ check: function() { return; } });
+			Chart.plugins.register({ check: function() { return false; } });
+			Chart.plugins.register({ check: function() { return 42 } });
+			var res = Chart.plugins.notify('check');
+			expect(res).toBeFalsy();
+		});
 	});
 });