From 67106e86981852d2d55830a8fca494c7d33eff37 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samy=20Pess=C3=A9?= <samypesse@gmail.com>
Date: Tue, 7 Jun 2016 11:06:53 +0200
Subject: [PATCH] Improve plugins config modifier

---
 .../config/__tests__/removePlugin.js          | 33 +++++++++++++++++++
 lib/modifiers/config/hasPlugin.js             | 15 +++++++++
 lib/modifiers/config/index.js                 | 10 +++---
 lib/modifiers/config/isDefaultPlugin.js       |  5 ++-
 lib/modifiers/config/removePlugin.js          |  2 +-
 lib/modifiers/config/togglePlugin.js          | 14 ++++++--
 testing/setup.js                              | 28 +++++++++++-----
 7 files changed, 87 insertions(+), 20 deletions(-)
 create mode 100644 lib/modifiers/config/__tests__/removePlugin.js
 create mode 100644 lib/modifiers/config/hasPlugin.js

diff --git a/lib/modifiers/config/__tests__/removePlugin.js b/lib/modifiers/config/__tests__/removePlugin.js
new file mode 100644
index 000000000..253cc397f
--- /dev/null
+++ b/lib/modifiers/config/__tests__/removePlugin.js
@@ -0,0 +1,33 @@
+var removePlugin = require('../removePlugin');
+var Config = require('../../../models/config');
+
+describe('removePlugin', function() {
+    var config = Config.createWithValues({
+        plugins: ['hello', 'world', '-disabled']
+    });
+
+    it('should remove the plugin from the list', function() {
+        var newConfig = removePlugin(config, 'hello');
+
+        var testDep = newConfig.getPluginDependency('hello');
+        expect(testDep).toNotBeDefined();
+    });
+
+    it('should remove the disabled plugin from the list', function() {
+        var newConfig = removePlugin(config, 'disabled');
+
+        var testDep = newConfig.getPluginDependency('disabled');
+        expect(testDep).toNotBeDefined();
+    });
+
+    it('should disable default plugin', function() {
+        var newConfig = removePlugin(config, 'search');
+
+        var disabledDep = newConfig.getPluginDependency('search');
+        expect(disabledDep).toBeDefined();
+        expect(disabledDep.getVersion()).toEqual('*');
+        expect(disabledDep.isEnabled()).toBeFalsy();
+    });
+});
+
+
diff --git a/lib/modifiers/config/hasPlugin.js b/lib/modifiers/config/hasPlugin.js
new file mode 100644
index 000000000..9aab4f29d
--- /dev/null
+++ b/lib/modifiers/config/hasPlugin.js
@@ -0,0 +1,15 @@
+
+/**
+ * Test if a plugin is listed
+ * @param { {List<PluginDependency}} deps
+ * @param {String} plugin
+ * @param {String} version
+ * @return {Boolean}
+ */
+function hasPlugin(deps, pluginName, version) {
+    return !!deps.find(function(dep) {
+        return dep.getName() === pluginName && (!version || dep.getVersion() === version);
+    });
+}
+
+module.exports = hasPlugin;
diff --git a/lib/modifiers/config/index.js b/lib/modifiers/config/index.js
index 37f32a0b8..5705dbb6e 100644
--- a/lib/modifiers/config/index.js
+++ b/lib/modifiers/config/index.js
@@ -1,7 +1,9 @@
 
 module.exports = {
-    addPlugin:    require('./addPlugin'),
-    removePlugin: require('./removePlugin'),
-    togglePlugin: require('./togglePlugin'),
-    editPlugin:   require('./editPlugin')
+    addPlugin:       require('./addPlugin'),
+    removePlugin:    require('./removePlugin'),
+    togglePlugin:    require('./togglePlugin'),
+    editPlugin:      require('./editPlugin'),
+    hasPlugin:       require('./hasPlugin'),
+    isDefaultPlugin: require('./isDefaultPlugin')
 };
diff --git a/lib/modifiers/config/isDefaultPlugin.js b/lib/modifiers/config/isDefaultPlugin.js
index e68874a56..63a141dc0 100644
--- a/lib/modifiers/config/isDefaultPlugin.js
+++ b/lib/modifiers/config/isDefaultPlugin.js
@@ -1,4 +1,5 @@
 var DEFAULT_PLUGINS = require('../../constants/defaultPlugins');
+var hasPlugin = require('./hasPlugin');
 
 /**
  * Test if a plugin is a default one
@@ -7,9 +8,7 @@ var DEFAULT_PLUGINS = require('../../constants/defaultPlugins');
  * @return {Boolean}
  */
 function isDefaultPlugin(pluginName, version) {
-    return !!DEFAULT_PLUGINS.find(function(dep) {
-        return dep.getName() === pluginName && (!version || dep.getVersion() === version);
-    });
+    return hasPlugin(DEFAULT_PLUGINS, pluginName, version);
 }
 
 module.exports = isDefaultPlugin;
diff --git a/lib/modifiers/config/removePlugin.js b/lib/modifiers/config/removePlugin.js
index dd7c248ea..ec06d1ec4 100644
--- a/lib/modifiers/config/removePlugin.js
+++ b/lib/modifiers/config/removePlugin.js
@@ -16,7 +16,7 @@ function removePlugin(config, pluginName) {
     }
 
     // Remove the dependency from the list
-    deps = deps.filter(function(dep) {
+    deps = deps.filterNot(function(dep) {
         return dep.getName() === pluginName;
     });
     return config.setPluginDependencies(deps);
diff --git a/lib/modifiers/config/togglePlugin.js b/lib/modifiers/config/togglePlugin.js
index 0cc49326a..a49e3b912 100644
--- a/lib/modifiers/config/togglePlugin.js
+++ b/lib/modifiers/config/togglePlugin.js
@@ -1,16 +1,24 @@
+var PluginDependency = require('../../models/pluginDependency');
+var hasPlugin = require('./hasPlugin');
+var isDefaultPlugin = require('./isDefaultPlugin');
 
 /**
  * Enable/disable a plugin dependency
  * @param {Config} config
- * @param {String} plugin
+ * @param {String} pluginName
  * @param {Boolean} state (optional)
  * @return {Config}
  */
-function togglePlugin(config, plugin, state) {
+function togglePlugin(config, pluginName, state) {
     var deps = config.getPluginDependencies();
 
+    // For default plugin, we should ensure it's listed first
+    if (isDefaultPlugin(pluginName) && !hasPlugin(deps, pluginName)) {
+        deps = deps.push(PluginDependency.create(pluginName));
+    }
+
     deps = deps.map(function(dep) {
-        if (dep.getName() === plugin) {
+        if (dep.getName() === pluginName) {
             return dep.toggle(state);
         }
 
diff --git a/testing/setup.js b/testing/setup.js
index 59fef7741..110500277 100644
--- a/testing/setup.js
+++ b/testing/setup.js
@@ -6,10 +6,9 @@ var cheerio = require('cheerio');
 
 expect.extend({
     /**
-        Check that a file is created in a directory:
-
-        expect('myFolder').toHaveFile('hello.md');
-    */
+     * Check that a file is created in a directory:
+     * expect('myFolder').toHaveFile('hello.md');
+     */
     toHaveFile: function(fileName) {
         var filePath = path.join(this.actual, fileName);
         var exists = fs.existsSync(filePath);
@@ -36,8 +35,8 @@ expect.extend({
     },
 
     /**
-        Check that a value is defined (not null nor undefined)
-    */
+     * Check that a value is defined (not null nor undefined)
+     */
     toBeDefined: function() {
         expect.assert(
             !(is.undefined(this.actual) || is.null(this.actual)),
@@ -47,10 +46,21 @@ expect.extend({
     },
 
     /**
-        Check that a dom element exists in HTML
+     * Check that a value is defined (not null nor undefined)
+     */
+    toNotBeDefined: function() {
+        expect.assert(
+            (is.undefined(this.actual) || is.null(this.actual)),
+            'expected %s to be not defined',
+            this.actual
+        );
+        return this;
+    },
 
-        @param {String} selector
-    */
+    /**
+     * Check that a dom element exists in HTML
+     * @param {String} selector
+     */
     toHaveDOMElement: function(selector) {
         var $ = cheerio.load(this.actual);
         var $el = $(selector);