From 88b4c5571d672a3c42d0297d8214928957dd271e Mon Sep 17 00:00:00 2001
From: Fabien O'Carroll <fabien@allou.is>
Date: Thu, 28 May 2020 12:40:47 +0200
Subject: [PATCH] Refactored the settings edit controller

no-issue

There was some unused code here, the variable  was never used, also we
were looping and collecting a list of errors, but only every using the
first one, so switched to the `find` method which stops iteration after
an element has matched.
---
 core/server/api/canary/settings.js | 69 ++++++++++++------------------
 1 file changed, 27 insertions(+), 42 deletions(-)

diff --git a/core/server/api/canary/settings.js b/core/server/api/canary/settings.js
index dd9b67be2b..fcfff5a6cf 100644
--- a/core/server/api/canary/settings.js
+++ b/core/server/api/canary/settings.js
@@ -87,59 +87,44 @@ module.exports = {
             unsafeAttrsObject(frame) {
                 return _.find(frame.data.settings, {key: 'labs'});
             },
-            before(frame) {
-                const errors = [];
+            async before(frame) {
+                if (frame.options.context && frame.options.context.internal) {
+                    return;
+                }
 
-                frame.data.settings.map((setting) => {
-                    if (setting.type === 'core' && !(frame.options.context && frame.options.context.internal)) {
-                        errors.push(new NoPermissionError({
-                            message: i18n.t('errors.api.settings.accessCoreSettingFromExtReq')
-                        }));
-                    }
-                });
-
-                if (errors.length) {
-                    return Promise.reject(errors[0]);
+                const firstCoreSetting = frame.data.settings.find(setting => setting.type === 'core');
+                if (firstCoreSetting) {
+                    throw new NoPermissionError({
+                        message: i18n.t('errors.api.settings.accessCoreSettingFromExtReq')
+                    });
                 }
             }
         },
-        query(frame) {
-            let type = frame.data.settings.find((setting) => {
-                return setting.key === 'type';
-            });
+        async query(frame) {
+            const settings = frame.data.settings;
 
-            if (_.isObject(type)) {
-                type = type.value;
+            const getSetting = setting => settingsCache.get(setting.key, {resolve: false});
+
+            const firstUnknownSetting = settings.find(setting => !getSetting(setting));
+
+            if (firstUnknownSetting) {
+                throw new NotFoundError({
+                    message: i18n.t('errors.api.settings.problemFindingSetting', {
+                        key: firstUnknownSetting.key
+                    })
+                });
             }
 
-            frame.data.settings = _.reject(frame.data.settings, (setting) => {
-                return setting.key === 'type';
-            });
-
-            const errors = [];
-
-            _.each(frame.data.settings, (setting) => {
-                const settingFromCache = settingsCache.get(setting.key, {resolve: false});
-
-                if (!settingFromCache) {
-                    errors.push(new NotFoundError({
-                        message: i18n.t('errors.api.settings.problemFindingSetting', {
-                            key: setting.key
-                        })
-                    }));
-                } else if (settingFromCache.type === 'core' && !(frame.options.context && frame.options.context.internal)) {
-                    // @TODO: handle in settings model permissible fn
-                    errors.push(new NoPermissionError({
+            if (!(frame.options.context && frame.options.context.internal)) {
+                const firstCoreSetting = settings.find(setting => getSetting(setting).type === 'core');
+                if (firstCoreSetting) {
+                    throw new NoPermissionError({
                         message: i18n.t('errors.api.settings.accessCoreSettingFromExtReq')
-                    }));
+                    });
                 }
-            });
-
-            if (errors.length) {
-                return Promise.reject(errors[0]);
             }
 
-            return models.Settings.edit(frame.data.settings, frame.options);
+            return models.Settings.edit(settings, frame.options);
         }
     },