Added serializer for missing settings endpoints

refs: https://github.com/TryGhost/Toolbox/issues/245

- Upload, updateMembersEmail, validateMembersEmailUpdate & disconnectStripeConnectIntegration were all missing serializers
- Upload is an as-is response, same as download
- updateMembersEmail, validateMembersEmailUpdate & disconnectStripeConnectIntegration are all passthroughs with no response
- With the upcoming refactor we want to have all the serializers defined explicitly
- This will allow us to change the default behaviour
- Updated the file based tests to prove the body doesn't change
- Tests were added to cover updateMembersEmail, validateMembersEmailUpdate & disconnectStripeConnectIntegration in 68c1bc0285

core/server/api/canary/utils/serializers/output/settings.js

@@ -1,65 +1,92 @@
 const _ = require('lodash');
 const utils = require('../../index');
 const mappers = require('./mappers');
-const _private = {};
 
 /**
- * ### Settings Filter
  * Filters an object based on a given filter object
  * @private
  * @param {Object} settings
  * @param {String} filter
  * @returns {*}
  */
-_private.settingsFilter = (settings, filter) => {
-    let filteredGroups = filter ? filter.split(',') : false;
+function settingsFilter(settings, filter) {
+    let filteredGroups = filter ? filter.split(',') : [];
     return _.filter(settings, (setting) => {
-        if (filteredGroups) {
+        if (filteredGroups.length > 0) {
             return _.includes(filteredGroups, setting.group);
         }
 
         return true;
     });
-};
+}
+
+/**
+ * Serialies a settings object into the desired API repsonse format
+ *
+ * @param {Object} models
+ * @param {Object} apiConfig
+ * @param {Object} frame
+ */
+function serializeSettings(models, apiConfig, frame) {
+    let filteredSettings;
+
+    // If this is public, we already have the right data, we just need to add an Array wrapper
+    if (utils.isContentAPI(frame)) {
+        filteredSettings = models;
+    } else {
+        filteredSettings = _.values(settingsFilter(models, frame.options.group));
+    }
+
+    frame.response = {
+        settings: mappers.settings(filteredSettings, frame),
+        meta: {}
+    };
+
+    if (frame.options.type || frame.options.group) {
+        frame.response.meta.filters = {};
+
+        if (frame.options.type) {
+            frame.response.meta.filters.type = frame.options.type;
+        }
+
+        if (frame.options.group) {
+            frame.response.meta.filters.group = frame.options.group;
+        }
+    }
+}
+
+/**
+ * This noop results in there being no response body
+ *
+ * @template Data
+ * @param {Data} data
+ * @returns Data
+ */
+function passthrough(data) {
+    return data;
+}
+
+/**
+ * Returns the data as-is without any further modiications
+ *
+ * @template Data
+ * @param {Data} data
+ * @param {Object} apiConfig
+ * @param {Object} frame
+ */
+function serializeData(data, apiConfig, frame) {
+    frame.response = data;
+}
 
 module.exports = {
-    browse(models, apiConfig, frame) {
-        let filteredSettings;
+    browse: serializeSettings,
+    read: serializeSettings,
+    edit: serializeSettings,
 
-        // If this is public, we already have the right data, we just need to add an Array wrapper
-        if (utils.isContentAPI(frame)) {
-            filteredSettings = models;
-        } else {
-            filteredSettings = _.values(_private.settingsFilter(models, frame.options.group));
-        }
+    download: serializeData,
+    upload: serializeData,
 
-        frame.response = {
-            settings: mappers.settings(filteredSettings, frame),
-            meta: {}
-        };
-
-        if (frame.options.type || frame.options.group) {
-            frame.response.meta.filters = {};
-
-            if (frame.options.type) {
-                frame.response.meta.filters.type = frame.options.type;
-            }
-
-            if (frame.options.group) {
-                frame.response.meta.filters.group = frame.options.group;
-            }
-        }
-    },
-
-    read() {
-        this.browse(...arguments);
-    },
-
-    edit() {
-        this.browse(...arguments);
-    },
-
-    download(bytes, apiConfig, frame) {
-        frame.response = bytes;
-    }
+    updateMembersEmail: passthrough,
+    validateMembersEmailUpdate: passthrough,
+    disconnectStripeConnectIntegration: passthrough
 };

test/e2e-api/admin/settings-files.test.js

@@ -26,7 +26,10 @@ describe('Settings File API', function () {
         const res = await request.get(localUtils.API.getApiQuery('settings/routes/yaml/'))
             .set('Origin', config.get('url'))
             .set('Accept', 'application/yaml')
-            .expect(200);
+            .expect(200)
+            .expect((_res) => {
+                _res.body.should.be.empty();
+            });
 
         res.headers['content-disposition'].should.eql('Attachment; filename="routes.yaml"');
         res.headers['content-type'].should.eql('application/yaml; charset=utf-8');
@@ -42,9 +45,9 @@ describe('Settings File API', function () {
             .set('Origin', config.get('url'))
             .attach('routes', newRoutesYamlPath)
             .expect('Content-Type', /application\/json/)
-            .expect(200);
-
-        res.headers['x-cache-invalidate'].should.eql('/*');
-        await testUtils.stopGhost();
+            .expect(200)
+            .expect((_res) => {
+                _res.body.should.be.empty();
+            });
     });
 });