Merge pull request #6558 from kevinansfield/refactor-feature-service

Refactor the feature service

core/client/app/services/feature.js

@@ -1,13 +1,15 @@
 import Ember from 'ember';
 
 const {
+    RSVP,
     Service,
     computed,
     inject: {service},
-    RSVP: {Promise},
     set
 } = Ember;
 
+const {Promise} = RSVP;
+
 const EmberError = Ember.Error;
 
 export function feature(name) {
@@ -36,10 +38,20 @@ export default Service.extend({
     config: service(),
     notifications: service(),
 
-    _settings: null,
-
     publicAPI: feature('publicAPI'),
 
+    labs: computed('_settings', function () {
+        return this.get('_settings').then((settings) => {
+            return this._parseLabs(settings);
+        });
+    }),
+
+    _settings: computed(function () {
+        let store = this.get('store');
+
+        return store.queryRecord('setting', {type: 'blog'});
+    }),
+
     _parseLabs(settings) {
         let labs = settings.get('labs');
 
@@ -50,41 +62,37 @@ export default Service.extend({
         }
     },
 
-    labs: computed('_settings', function () {
-        return new Promise((resolve, reject) => {
-            if (this.get('_settings')) { // So we don't query the backend every single time
-                resolve(this._parseLabs(this.get('_settings')));
-            }
-            let store = this.get('store');
-
-            store.query('setting', {type: 'blog'}).then((settings) => {
-                let setting = settings.get('firstObject');
-
-                this.set('_settings', setting);
-                resolve(this._parseLabs(setting));
-            }).catch(reject);
-        });
-    }),
-
     update(key, value) {
         return new Promise((resolve, reject) => {
-            this.get('labs').then((labs) => {
-                let settings = this.get('_settings');
+            let promises = {
+                settings: this.get('_settings'),
+                labs: this.get('labs')
+            };
 
+            RSVP.hash(promises).then(({labs, settings}) => {
+                // set the new labs key value
                 set(labs, key, value);
-
+                // update the 'labs' key of the settings model
                 settings.set('labs', JSON.stringify(labs));
+
                 settings.save().then((savedSettings) => {
-                    this.set('_settings', savedSettings);
+                    // replace the cached _settings promise
+                    this.set('_settings', RSVP.resolve(savedSettings));
+
+                    // return the labs key value that we get from the server
                     resolve(this._parseLabs(savedSettings).get(key));
+
                 }).catch((errors) => {
-                    if (errors) { // model.save errors, show notifications
-                        this.get('notifications').showErrors(errors);
-                        settings.rollbackAttributes();
-                    } else {
-                        settings.rollbackAttributes();
+                    settings.rollbackAttributes();
+
+                    // we'll always have an errors object unless we hit a
+                    // validation error
+                    if (!errors) {
                         throw new EmberError(`Validation of the feature service settings model failed when updating labs.`);
                     }
+
+                    this.get('notifications').showErrors(errors);
+
                     resolve(this._parseLabs(settings)[key]);
                 });
             }).catch(reject);

core/client/tests/integration/services/feature-test.js

@@ -8,7 +8,7 @@ import FeatureService, {feature} from 'ghost/services/feature';
 import Ember from 'ember';
 import { errorOverride, errorReset } from 'ghost/tests/helpers/adapter-error';
 
-const {merge, run} = Ember;
+const {RSVP, merge, run} = Ember;
 const EmberError = Ember.Error;
 
 function stubSettings(server, labs, validSave = true, validSettings = true) {
@@ -80,6 +80,24 @@ describeModule(
             });
         });
 
+        it('caches the labs promise', function (done) {
+            stubSettings(server, {testFlag: true});
+
+            let service = this.subject();
+            let calls = [
+                service.get('labs'),
+                service.get('labs'),
+                service.get('labs')
+            ];
+
+            RSVP.all(calls).then(() => {
+                expect(server.handledRequests.length, 'requests after 3 calls')
+                    .to.equal(1);
+
+                done();
+            });
+        });
+
         it('returns false for set flag with config false and labs false', function (done) {
             stubSettings(server, {testFlag: false});
             addTestFlag();