engine.removeRule() return value reflects whether rule was removed

Author: Cache Hamm

CHANGELOG.md

@@ -1,3 +1,6 @@
+#### 7.0.0 / 2021-06-03
+  * Rules now receive a default `id` property, when none is passed to the constructor.
+
 #### 6.0.1 / 2021-03-09
   * Updates Typescript types to include `failureEvents` in EngineResult.
 

src/engine.js

@@ -76,9 +76,11 @@ class Engine extends EventEmitter {
    */
   removeRule (rule) {
     if (!(rule instanceof Rule)) {
-      this.rules = this.rules.filter(ruleInEngine => ruleInEngine.id !== rule)
+      const filteredRules = this.rules.filter(ruleInEngine => ruleInEngine.id !== rule)
+      const ruleRemoved = filteredRules.length !== this.rules.length
+      this.rules = filteredRules
       this.prioritizedRules = null
-      return Boolean(this.rules.length)
+      return ruleRemoved
     } else {
       if (!rule) return false
       const index = this.rules.indexOf(rule)

test/engine.test.js

@@ -109,38 +109,60 @@ describe('Engine', () => {
   })
 
   describe('removeRule()', () => {
-    describe('rule instance', () => {
+    context('rule instance', () => {
       it('removes the rule', () => {
         const rule = new Rule(factories.rule())
         engine.addRule(rule)
         expect(engine.rules.length).to.equal(1)
-        engine.removeRule(rule)
+
+        const isRemoved = engine.removeRule(rule)
+
+        expect(isRemoved).to.be.true()
         expect(engine.rules.length).to.equal(0)
         expect(engine.prioritizedRules).to.equal(null)
       })
-    })
 
-    it('can only remove added rules', () => {
-      expect(engine.rules.length).to.equal(0)
-      const rule = new Rule(factories.rule())
-      const isRemoved = engine.removeRule(rule)
-      expect(isRemoved).to.equal(false)
-    })
+      it('can only remove added rules', () => {
+        expect(engine.rules.length).to.equal(0)
+        const rule = new Rule(factories.rule())
 
-    it('clears the "prioritizedRules" cache', () => {
-      const rule = new Rule(factories.rule())
-      engine.addRule(rule)
-      engine.prioritizeRules()
-      engine.removeRule(rule)
-      expect(engine.prioritizedRules).to.equal(null)
+        const isRemoved = engine.removeRule(rule)
+
+        expect(isRemoved).to.equal(false)
+      })
+
+      it('clears the "prioritizedRules" cache', () => {
+        const rule = new Rule(factories.rule())
+        engine.addRule(rule)
+        engine.prioritizeRules()
+        engine.removeRule(rule)
+        expect(engine.prioritizedRules).to.equal(null)
+      })
     })
-    it('removes rule based on ruleKey', () => {
-      const rule = new Rule(factories.rule())
-      engine.addRule(rule)
-      expect(engine.rules.length).to.equal(1)
-      engine.removeRule(rule.id)
-      expect(engine.rules.length).to.equal(0)
-      expect(engine.prioritizedRules).to.equal(null)
+
+    context('rule id', () => {
+      it('removes rule based on rule id', () => {
+        const rule = new Rule(factories.rule())
+        engine.addRule(rule)
+        expect(engine.rules.length).to.equal(1)
+
+        const isRemoved = engine.removeRule(rule.id)
+
+        expect(isRemoved).to.be.true()
+        expect(engine.rules.length).to.equal(0)
+        expect(engine.prioritizedRules).to.equal(null)
+      })
+
+      it('returns false when rule cannot be found', () => {
+        const rule = new Rule(factories.rule())
+        engine.addRule(rule)
+        expect(engine.rules.length).to.equal(1)
+
+        const isRemoved = engine.removeRule('not-found-id')
+
+        expect(isRemoved).to.be.false()
+        expect(engine.rules.length).to.equal(1)
+      })
     })
   })