From ddb718f0bb1fa7c5489c38799327b0a9dc43f612 Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <github.erisds@gmail.com>
Date: Tue, 10 May 2022 20:05:56 +0100
Subject: [PATCH] Fixed settingsCache returning falsy as null

refs: https://github.com/TryGhost/Ghost/commit/e68cb8b3142b628406d9b7add3ddaf28719204a9

- a couple of months ago when improving the test coverage here I found some weird behaviour with falsey values
- turned out it didn't matter at the time because we didn't have any settings that are false
- with the introduction of calculated settings we will have: https://github.com/TryGhost/Ghost/pull/14766
- whilst building that, I found settings that should be returned as false were being returned as null
- fixing it in a separate commit to keep the work clean
---
 core/shared/settings-cache/cache.js     |  9 +++++++--
 test/unit/shared/settings-cache.test.js | 11 +++++++----
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/core/shared/settings-cache/cache.js b/core/shared/settings-cache/cache.js
index abb0052d9a..0e973694a5 100644
--- a/core/shared/settings-cache/cache.js
+++ b/core/shared/settings-cache/cache.js
@@ -33,6 +33,11 @@ const doGet = (key, options) => {
 
     // Default behaviour is to try to resolve the value and return that
     try {
+        // CASE: handle literal false
+        if (settingsCache[key].value === false || settingsCache[key].value === 'false') {
+            return false;
+        }
+
         // CASE: if a string contains a number e.g. "1", JSON.parse will auto convert into integer
         if (!isNaN(Number(settingsCache[key].value))) {
             return settingsCache[key].value || null;
@@ -99,7 +104,7 @@ module.exports = {
         let settings = {};
 
         _.each(publicSettings, (key, newKey) => {
-            settings[newKey] = doGet(key) || null;
+            settings[newKey] = doGet(key) ?? null;
         });
 
         return settings;
@@ -110,7 +115,7 @@ module.exports = {
      * Optionally takes a collection of settings & can populate the cache with these.
      *
      * @param {EventEmitter} events
-     * @param {Bookshelf.Collection<Settings>} [settingsCollection]
+     * @param {Bookshelf.Collection<Settings>} settingsCollection
      * @return {object}
      */
     init(events, settingsCollection) {
diff --git a/test/unit/shared/settings-cache.test.js b/test/unit/shared/settings-cache.test.js
index fab8bb46dc..234fd20379 100644
--- a/test/unit/shared/settings-cache.test.js
+++ b/test/unit/shared/settings-cache.test.js
@@ -11,7 +11,7 @@ should.equal(true, true);
 
 describe('UNIT: settings cache', function () {
     beforeEach(function () {
-        cache.init(events);
+        cache.init(events, {});
     });
 
     afterEach(function () {
@@ -64,7 +64,7 @@ describe('UNIT: settings cache', function () {
         should(cache.get('nan')).eql(null);
 
         should(cache.get('true')).eql(true);
-        should(cache.get('false')).eql(null); // EEEK! this is falsy but should be false
+        should(cache.get('false')).eql(false);
         should(cache.get('object')).eql({});
         should(cache.get('array')).eql([]);
 
@@ -81,7 +81,7 @@ describe('UNIT: settings cache', function () {
         should(cache.get('stringnull')).eql(null);
         should(cache.get('stringnan')).eql('NaN');
         should(cache.get('stringtrue')).eql(true);
-        should(cache.get('stringfalse')).eql(null); // EEEK! this is falsy but should be false
+        should(cache.get('stringfalse')).eql(false);
         should(cache.get('stringobj')).eql({});
         should(cache.get('stringarr')).eql([]);
     });
@@ -96,16 +96,19 @@ describe('UNIT: settings cache', function () {
         cache.set('key1', {value: 'something'});
         cache.set('title', {value: 'hello world'});
         cache.set('timezone', {value: 'PST'});
+        cache.set('secondary_navigation', {value: false});
 
         cache.getAll().should.eql({
             key1: {value: 'something'},
             title: {value: 'hello world'},
-            timezone: {value: 'PST'}
+            timezone: {value: 'PST'},
+            secondary_navigation: {value: false}
         });
 
         let values = _.zipObject(_.keys(publicSettings), _.fill(Array(_.size(publicSettings)), null));
         values.title = 'hello world';
         values.timezone = 'PST';
+        values.secondary_navigation = false;
 
         cache.getPublic().should.eql(values);
     });