From da17b2c82b219da46c518dfec80a8f6b8dcd417c Mon Sep 17 00:00:00 2001
From: Hannah Wolfe <erisds@gmail.com>
Date: Mon, 7 Jan 2019 13:35:51 +0000
Subject: [PATCH] Settings API should return null instead of ""

refs #10345

- We are standardising on returning null from the Content API for any empty values
---
 core/server/services/settings/cache.js               | 10 +++++-----
 core/test/functional/api/v2/content/settings_spec.js |  5 ++++-
 core/test/unit/services/settings/cache_spec.js       |  2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/core/server/services/settings/cache.js b/core/server/services/settings/cache.js
index c2980cdbe8..62b8903522 100644
--- a/core/server/services/settings/cache.js
+++ b/core/server/services/settings/cache.js
@@ -29,19 +29,19 @@ const doGet = (key, options) => {
 
     // Don't try to resolve to the value of the setting
     if (options && options.resolve === false) {
-        return settingsCache[key];
+        return settingsCache[key] || null;
     }
 
     // Default behaviour is to try to resolve the value and return that
     try {
         // 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;
+            return settingsCache[key].value || null;
         }
 
-        return JSON.parse(settingsCache[key].value);
+        return JSON.parse(settingsCache[key].value) || null;
     } catch (err) {
-        return settingsCache[key].value;
+        return settingsCache[key].value || null;
     }
 };
 
@@ -100,7 +100,7 @@ module.exports = {
         let settings = {};
 
         _.each(publicSettings, (newKey, key) => {
-            settings[newKey] = doGet(key) || '';
+            settings[newKey] = doGet(key) || null;
         });
 
         return settings;
diff --git a/core/test/functional/api/v2/content/settings_spec.js b/core/test/functional/api/v2/content/settings_spec.js
index 88ba1f239d..7a8ad54264 100644
--- a/core/test/functional/api/v2/content/settings_spec.js
+++ b/core/test/functional/api/v2/content/settings_spec.js
@@ -49,11 +49,14 @@ describe('Settings', function () {
                     let defaultKey = _.findKey(publicSettings, (v) => v === key);
                     let defaultValue = _.find(defaultSettings, (setting) => setting.key === defaultKey).defaultValue;
 
+                    // Convert empty strings to null
+                    defaultValue = defaultValue || null;
+
                     if (defaultKey === 'navigation') {
                         defaultValue = JSON.parse(defaultValue);
                     }
 
-                    value.should.eql(defaultValue);
+                    should(value).eql(defaultValue);
                 });
             });
     });
diff --git a/core/test/unit/services/settings/cache_spec.js b/core/test/unit/services/settings/cache_spec.js
index 30def94f08..10d38df7d0 100644
--- a/core/test/unit/services/settings/cache_spec.js
+++ b/core/test/unit/services/settings/cache_spec.js
@@ -47,7 +47,7 @@ describe('UNIT: settings cache', function () {
             active_timezone: {value: 'PST'}
         });
 
-        let values = _.zipObject(_.values(publicSettings), _.fill(Array(_.size(publicSettings)), ''));
+        let values = _.zipObject(_.values(publicSettings), _.fill(Array(_.size(publicSettings)), null));
         values.title = 'hello world';
         values.timezone = 'PST';