From 1b4fab3968683685dc1e6d32790b8a8c5735bf17 Mon Sep 17 00:00:00 2001
From: Chris Earle <pickypg@users.noreply.github.com>
Date: Mon, 5 Feb 2018 14:39:30 -0500
Subject: [PATCH] [Config] Acknowledge when Advanced Settings respond (#16485)

This adds a return value to the `Promise` returned by `config.set`, which allows callers to `await` it  and verify that their setting was applied without checking the settings (which is a race condition).
---
 src/ui/public/config/__tests__/config.js | 31 +++++++++++++++++++++++-
 src/ui/public/config/config.js           | 14 ++++++-----
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/src/ui/public/config/__tests__/config.js b/src/ui/public/config/__tests__/config.js
index 0c6699490ec32f..e73928b04afc1e 100644
--- a/src/ui/public/config/__tests__/config.js
+++ b/src/ui/public/config/__tests__/config.js
@@ -1,6 +1,8 @@
 import expect from 'expect.js';
 import ngMock from 'ng_mock';
 
+import { Notifier } from 'ui/notify';
+
 describe('config component', function () {
   let config;
   let $scope;
@@ -67,7 +69,7 @@ describe('config component', function () {
       expect($scope).to.have.property('dateFormat', dateFormat);
     });
 
-    it('alows overriding the property name', function () {
+    it('allows overriding the property name', function () {
       const dateFormat = config.get('dateFormat');
       config.bindToScope($scope, 'dateFormat', 'defaultDateFormat');
       expect($scope).to.not.have.property('dateFormat');
@@ -88,4 +90,31 @@ describe('config component', function () {
 
   });
 
+  describe('#_change', () => {
+
+    it('returns true for success', async () => {
+      // immediately resolve to avoid timing issues
+      const delayedUpdate = () => Promise.resolve();
+
+      expect(await config._change('expect_true', 'value', { _delayedUpdate: delayedUpdate })).to.be(true);
+      // setting to the same should set it to true as well
+      expect(await config._change('expect_true', 'value')).to.be(true);
+
+      config.remove('expect_true');
+    });
+
+    it('returns false for failure', async () => {
+      const message = 'TEST - _change - EXPECTED';
+      // immediately resolve to avoid timing issues
+      const delayedUpdate = () => Promise.reject(new Error(message));
+
+      expect(await config._change('expected_false', 'value', { _delayedUpdate: delayedUpdate })).to.be(false);
+
+      // cleanup the notification so that the test harness does not complain
+      const notif = Notifier.prototype._notifs.find(notif => notif.content.indexOf(message) !== -1);
+      notif.clear();
+    });
+
+  });
+
 });
diff --git a/src/ui/public/config/config.js b/src/ui/public/config/config.js
index 45e671984c6a32..d52b3ed42c4862 100644
--- a/src/ui/public/config/config.js
+++ b/src/ui/public/config/config.js
@@ -15,8 +15,8 @@ module.service(`config`, function (Private, $rootScope, chrome, uiSettings) {
 
   config.getAll = () => cloneDeep(settings);
   config.get = (key, defaultValue) => getCurrentValue(key, defaultValue);
-  config.set = (key, val) => change(key, isPlainObject(val) ? angular.toJson(val) : val);
-  config.remove = key => change(key, null);
+  config.set = (key, val) => config._change(key, isPlainObject(val) ? angular.toJson(val) : val);
+  config.remove = key => config._change(key, null);
   config.isDeclared = key => key in settings;
   config.isDefault = key => !config.isDeclared(key) || nullOrEmpty(settings[key].userValue);
   config.isCustom = key => config.isDeclared(key) && !('value' in settings[key]);
@@ -57,26 +57,28 @@ any custom setting configuration watchers for "${key}" may fix this issue.`);
     return scope.$on(`change:config`, update);
   }
 
-  function change(key, value) {
+  config._change = (key, value, { _delayedUpdate = delayedUpdate } = { }) => {
     const declared = config.isDeclared(key);
     const oldVal = declared ? settings[key].userValue : undefined;
     const newVal = key in defaults && defaults[key].defaultValue === value ? null : value;
     const unchanged = oldVal === newVal;
     if (unchanged) {
-      return Promise.resolve();
+      return Promise.resolve(true);
     }
     const initialVal = declared ? config.get(key) : undefined;
     localUpdate(key, newVal, initialVal);
 
-    return delayedUpdate(key, newVal)
+    return _delayedUpdate(key, newVal)
       .then(updatedSettings => {
         settings = mergeSettings(defaults, updatedSettings);
+        return true;
       })
       .catch(reason => {
         localUpdate(key, initialVal, config.get(key));
         notify.error(reason);
+        return false;
       });
-  }
+  };
 
   function localUpdate(key, newVal, oldVal) {
     patch(key, newVal);