Fixed logic bugs in match helper (#13315)

closes: CORE-33

Two bugs:
- lodash isEmpty and handlebars util isEmpty are not the same
- I literally had the truthy and falsy cases the wrong way around 🙈

Notes:
- I have, for now, copied the isEmpty util from handlebars. It's so small it doesn't seem worth trying to require the util right now, although in future it'd be nice if that was easier to do
- Adding the management for the conditional being a SafeString allows the match helper to be a subexpression of itself, I can see this pattern being useful later in combo with the any and all helpers

core/frontend/helpers/match.js

@@ -2,20 +2,35 @@ const {logging, i18n, SafeString, labs} = require('../services/proxy');
 const _ = require('lodash');
 
 /**
- * This is identical to the built-in if helper
+ * This is identical to the built-in if helper, except inverse/fn calls are replaced with false/true
+ * https://github.com/handlebars-lang/handlebars.js/blob/19bdace85a8d0bc5ed3a4dec4071cb08c8d003f2/lib/handlebars/helpers/if.js#L9-L20
  */
+function isEmptyValue(value) {
+    if (!value && value !== 0) {
+        return true;
+    } else if (Array.isArray(value) && value.length === 0) {
+        return true;
+    } else {
+        return false;
+    }
+}
+
 const handleConditional = (conditional, options) => {
     if (_.isFunction(conditional)) {
         conditional = conditional.call(this);
     }
 
+    if (conditional instanceof SafeString) {
+        conditional = conditional.string;
+    }
+
     // Default behavior is to render the positive path if the value is truthy and not empty.
     // The `includeZero` option may be set to treat the condtional as purely not empty based on the
     // behavior of isEmpty. Effectively this determines if 0 is handled by the positive path or negative.
-    if ((!options.hash.includeZero && !conditional) || _.isEmpty(conditional)) {
-        return true;
-    } else {
+    if ((!options.hash.includeZero && !conditional) || isEmptyValue(conditional)) {
         return false;
+    } else {
+        return true;
     }
 };
 

test/unit/helpers/match.test.js

@@ -60,27 +60,27 @@ describe('Match helper', function () {
         };
 
         // @TODO: Fix this!
-        // describe('Basic values', function () {
-        // runTests({
-        // '{{match truthy_bool}}': 'true',
-        // '{{match falsy_bool}}': 'false',
-        // '{{match one}}': 'true',
-        // '{{match zero}}': 'false',
-        // '{{match string}}': 'true',
-        // '{{match empty}}': 'false',
-        // '{{match null}}': 'false',
-        // '{{match undefined}}': 'false',
-        // '{{match unknown}}': 'false',
-        // '{{match object}}': 'true',
+        describe('Basic values', function () {
+            runTests({
+                '{{match truthy_bool}}': 'true',
+                '{{match falsy_bool}}': 'false',
+                '{{match one}}': 'true',
+                '{{match zero}}': 'false',
+                '{{match string}}': 'true',
+                '{{match empty}}': 'false',
+                '{{match null}}': 'false',
+                '{{match undefined}}': 'false',
+                '{{match unknown}}': 'false',
+                '{{match object}}': 'true',
 
-        // // Zero works if includeZero is set
-        // '{{match zero includeZero=true}}': 'true',
+                // Zero works if includeZero is set
+                '{{match zero includeZero=true}}': 'true',
 
-        // // Nesting the helper should still resolve correctly
-        // '{{match (match truthy_bool)}}': 'true',
-        // '{{match (match falsy_bool)}}': 'false'
-        // }, hash);
-        // });
+                // Nesting the helper should still resolve correctly
+                '{{match (match truthy_bool)}}': 'true',
+                '{{match (match falsy_bool)}}': 'false'
+            }, hash);
+        });
 
         // @TODO: Implement Implicit Equals
         // describe('Implicit Equals', function () {
@@ -148,4 +148,41 @@ describe('Match helper', function () {
             shouldCompileToExpected(templateString, {title}, expected);
         });
     });
+
+    // By using match as a block helper, instead of returning true or false, the matching template is executed
+    // We've already tested all the logic of the matches, for the block helpers we only need to test that the correct template is executed
+    describe('{{#match}} (block)', function () {
+        it('Executes the first block when match is true', function () {
+            const templateString = '{{#match title "=" "Hello World"}}case a{{else match title "=" "Hello World!"}}case b{{else}}case c{{/match}}';
+            const hash = {
+                title: 'Hello World'
+            };
+
+            const expected = 'case a';
+
+            shouldCompileToExpected(templateString, hash, expected);
+        });
+
+        it('Executes secondary blocks correctly', function () {
+            const templateString = '{{#match title "=" "Hello World"}}case a{{else match title "=" "Hello World!"}}case b{{else}}case c{{/match}}';
+            const hash = {
+                title: 'Hello World!'
+            };
+
+            const expected = 'case b';
+
+            shouldCompileToExpected(templateString, hash, expected);
+        });
+
+        it('Executes the else block when match is false', function () {
+            const templateString = '{{#match title "=" "Hello World"}}case a{{else match title "=" "Hello World!"}}case b{{else}}case c{{/match}}';
+            const hash = {
+                title: 'Hello'
+            };
+
+            const expected = 'case c';
+
+            shouldCompileToExpected(templateString, hash, expected);
+        });
+    });
 });