🐛 Fixed comped subscription duration drop-down sometimes not being visible (#15764)

closes https://github.com/TryGhost/Team/issues/2110

- added failing test showing feature service `@feature` properties
weren't autotracking correctly if accessed before
authentication+settings fetch occurs
- shows labs and feature properties on the feature service are not
reacting to changes in the settings service
- removing the `@computed` on the `feature.labs` getter stops it being
cached but it then fails on the `feature.testFlag` computed property
- updated `settings` service to behave as expected with our current
version of Ember
- inspected the store schema for `Setting` to define the "proxied"
properties up-front rather than only after fetching
- updated the property definition to use `computed` so we're opting in
to the old style reactivity (required adding the `@classic` decorator to
pass linting)

ghost/admin/app/services/settings.js

@@ -1,13 +1,16 @@
 import Service, {inject as service} from '@ember/service';
+import Setting from '../models/setting';
 import ValidationEngine from 'ghost-admin/mixins/validation-engine';
+import classic from 'ember-classic-decorator';
+import {computed, defineProperty, get} from '@ember/object';
 import {tracked} from '@glimmer/tracking';
-
+@classic
 export default class SettingsService extends Service.extend(ValidationEngine) {
     @service store;
 
     // will be set to the single Settings model, it's a reference so any later
     // changes to the settings object in the store will be reflected
-    settingsModel = null;
+    @tracked settingsModel = null;
 
     validationType = 'setting';
     _loadingPromise = null;
@@ -16,6 +19,28 @@ export default class SettingsService extends Service.extend(ValidationEngine) {
     // back from the API rather than local updates
     @tracked settledIcon = '';
 
+    init() {
+        super.init(...arguments);
+
+        // TODO: update to use .getSchemaDefinitionService().attributesDefinitionFor({type: 'settings'});
+        // in later Ember versions
+        const attributes = get(Setting, 'attributes');
+
+        for (const [name] of attributes) {
+            if (!Object.prototype.hasOwnProperty.call(this, name)) {
+                // a standard defineProperty here was not autotracking correctly - retry in a later Ember version
+                defineProperty(this, name, computed(`settingsModel.${name}`, {
+                    get() {
+                        return this.settingsModel?.[name];
+                    },
+                    set(keyName, value) {
+                        return this.settingsModel[name] = value;
+                    }
+                }));
+            }
+        }
+    }
+
     get hasDirtyAttributes() {
         return this.settingsModel?.hasDirtyAttributes || false;
     }
@@ -54,19 +79,6 @@ export default class SettingsService extends Service.extend(ValidationEngine) {
         this.settingsModel = settingsModel;
         this.settledIcon = settingsModel.icon;
 
-        settingsModel.eachAttribute((name) => {
-            if (!Object.prototype.hasOwnProperty.call(this, name)) {
-                Object.defineProperty(this, name, {
-                    get() {
-                        return this.settingsModel[name];
-                    },
-                    set(newValue) {
-                        this.settingsModel[name] = newValue;
-                    }
-                });
-            }
-        });
-
         return this;
     }
 

ghost/admin/tests/integration/services/feature-test.js

@@ -9,7 +9,9 @@ import {settled} from '@ember/test-helpers';
 import {setupTest} from 'ember-mocha';
 
 function stubSettings(server, labs, validSave = true) {
-    let settings = [
+    const site = [];
+
+    const settings = [
         {
             id: '1',
             type: 'labs',
@@ -18,6 +20,10 @@ function stubSettings(server, labs, validSave = true) {
         }
     ];
 
+    server.get(`${ghostPaths().apiRoot}/site/`, function () {
+        return [200, {'Content-Type': 'application/json'}, JSON.stringify({site})];
+    });
+
     server.get(`${ghostPaths().apiRoot}/settings/`, function () {
         return [200, {'Content-Type': 'application/json'}, JSON.stringify({settings})];
     });
@@ -227,7 +233,7 @@ describe('Integration: Service: feature', function () {
             });
 
             return settled().then(() => {
-                expect(server.handlers[1].numberOfCalls).to.equal(1);
+                expect(server.handlers[2].numberOfCalls).to.equal(1);
                 expect(service.get('testFlag')).to.be.true;
             });
         });
@@ -252,7 +258,7 @@ describe('Integration: Service: feature', function () {
             });
 
             return settled().then(() => {
-                expect(server.handlers[3].numberOfCalls).to.equal(1);
+                expect(server.handlers[4].numberOfCalls).to.equal(1);
                 expect(service.get('testUserFlag')).to.be.true;
             });
         });
@@ -279,7 +285,7 @@ describe('Integration: Service: feature', function () {
 
             return settled().then(() => {
                 expect(
-                    server.handlers[1].numberOfCalls,
+                    server.handlers[2].numberOfCalls,
                     'PUT call is made'
                 ).to.equal(1);
 
@@ -340,7 +346,7 @@ describe('Integration: Service: feature', function () {
         service.get('config').set('testFlag', false);
 
         return service.fetch().then(() => {
-            expect(service.get('testFlag')).to.be.false;
+            expect(service.get('testFlag'), 'testFlag before set').to.be.false;
 
             run(() => {
                 expect(() => {
@@ -350,9 +356,27 @@ describe('Integration: Service: feature', function () {
 
             return settled().then(() => {
                 // ensure validation is happening before the API is hit
-                expect(server.handlers[1].numberOfCalls).to.equal(0);
+                expect(server.handlers[2].numberOfCalls).to.equal(0);
                 expect(service.get('testFlag')).to.be.false;
             });
         });
     });
+
+    it('has correct labs flags when accessed before and after settings load', async function () {
+        stubSettings(server, {testFlag: true});
+        stubUser(server, {});
+
+        addTestFlag();
+
+        const settingsService = this.owner.lookup('service:settings');
+        const featureService = this.owner.lookup('service:feature');
+
+        expect(featureService.testFlag, 'testFlag before settings fetch').to.be.false;
+
+        await settingsService.fetch();
+
+        expect(featureService.settings.labs, 'feature.settings.labs after settings fetch').to.equal('{"testFlag":true}');
+        expect(featureService.labs, 'feature.labs after settings fetch').to.deep.equal({testFlag: true});
+        expect(featureService.testFlag, 'feature.testFlag after settings fetch').to.be.true;
+    });
 });