Merge pull request #15 from achambers/refactor-all-the-things

Refactoring services to be more sane

.travis.yml

@@ -1,7 +1,7 @@
 ---
 language: node_js
 node_js:
-  - "4"
+  - "6.11.4"
 
 sudo: false
 
@@ -11,13 +11,14 @@ cache:
 
 env:
   # we recommend testing LTS's and latest stable release (bonus points to beta/canary)
-  - EMBER_TRY_SCENARIO=ember-lts-2.4
-  - EMBER_TRY_SCENARIO=ember-lts-2.8
   - EMBER_TRY_SCENARIO=ember-release
   - EMBER_TRY_SCENARIO=ember-beta
   - EMBER_TRY_SCENARIO=ember-canary
   - EMBER_TRY_SCENARIO=ember-default
 
+script:
+  - node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO test --skip-cleanup
+
 matrix:
   fast_finish: true
   allow_failures:
@@ -29,9 +30,15 @@ before_install:
   - phantomjs --version
 
 install:
-  - npm install
+  - yarn install
 
-script:
-  # Usually, it's ok to finish the test scenario without reverting
-  #  to the addon's original dependency state, skipping "cleanup".
-  - node_modules/.bin/ember try:one $EMBER_TRY_SCENARIO test --skip-cleanup
+#stages:
+  #- compile
+  #- test-node
+  #- test
+
+jobs:
+  include:
+    - stage: test-node
+      env:
+      script: yarn run test-node

README.md

@@ -215,8 +215,8 @@ import service from 'ember-service/inject';
 export default Component.extend({
   launchDarkly: service(),
 
-  price: computed('launchDarkly.newPricePlan', function() {
-    if (this.get('launchDarkly.newPricePlan')) {
+  price: computed('launchDarkly.new-price-plan', function() {
+    if (this.get('launchDarkly.new-price-plan')) {
       return 99.00;
     }
 
@@ -244,7 +244,7 @@ let ENV = {
   launchDarkly: {
     local: true,
     localFeatureFlags: {
-      'apply-discount': true.
+      'apply-discount': true,
       'new-pricing-plan': 'plan-a'
     }
   }
@@ -254,10 +254,9 @@ let ENV = {
 When `local: true`, the Launch Darkly feature service is available in the browser console via `window.ld`. The service provides the following helper methods to manipulate feature flags:
 
 ```js
-> ld.variation('apply-discount') // return the current value of the feature flag
+> ld.variation('new-pricing-plan', 'plan-a') // return the current value of the feature flag providing a default if it doesn't exist
 
-> ld.variation('apply-discount', true) // set the return value of the feature flag
-> ld.variation('new-pricing-plan', 'plan-a') // set the return value of the feature flag
+> ld.setVariation('new-pricing-plan', 'plan-x') // set the variation value
 
 > ld.enable('apply-discount') // helper to set the return value to `true`
 > ld.disable('apply-discount') // helper to set the return value to `false`
@@ -271,60 +270,81 @@ When `local: true`, the Launch Darkly feature service is available in the browse
 
 ### Acceptance Tests
 
+Stub the Launch Darkly client in acceptance tests using the provided test client which will default all feature flag values to false, instead of using what's defined in the `localFeatureFlags` config. This allows your tests to start off in a known default state.
+
+```js
+import StubClient from 'ember-launch-darkly/test-support/helpers/launch-darkly-client-test';
+
+moduleForAcceptance('Acceptance | Homepage', {
+  beforeEach() {
+    this.application.__container__.registry.register('service:launch-darkly-client', StubClient)
+  }
+});
+
+test( "links go to the new homepage", function () {
+  visit('/');
+  click('a.pricing');
+  andThen(function(){
+    equal(currentRoute(), 'pricing', 'Should be on the old pricing page');
+  });
+});
+```
+
 ember-launch-darkly provides a test helper, `withVariation`, to make it easy to turn feature flags on and off in acceptance tests. Simply import the test helper in your test, or `tests/test-helper.js` file.
 
 ```js
 import 'ember-launch-darkly/test-support/helpers/with-variation';
+import StubClient from 'ember-launch-darkly/test-support/helpers/launch-darkly-client-test';
+
+moduleForAcceptance('Acceptance | Homepage', {
+  beforeEach() {
+    this.application.__container__.registry.register('service:launch-darkly-client', StubClient)
+  }
+});
 
 test( "links go to the new homepage", function () {
-  withVariation('apply-discount', true);
   withVariation('new-pricing-plan', 'plan-a');
 
   visit('/');
   click('a.pricing');
   andThen(function(){
-    equal(currentRoute(), 'new.pricing', 'Should be on the new pricing page');
+    equal(currentRoute(), 'new-pricing', 'Should be on the new pricing page');
   });
 });
 ```
 
 ### Integration Tests
 
-Simply stub the Launch Darkly service in integration tests to control the feature flags
+Use the test client to stub the Launch Darkly client in integration tests to control the feature flags.
 
 ```js
-import getOwner from 'ember-owner/get';
-import Service from 'ember-service';
-
-let stubService = Service.extend({
-  variation(key) {
-    if (key === 'new-pricing-page') {
-      return 'plan-a';
-    }
-
-    return false;
-  }
-});
+import StubClient from 'ember-launch-darkly/test-support/helpers/launch-darkly-client-test';
 
 moduleForComponent('my-component', 'Integration | Component | my component', {
   integration: true,
   beforeEach() {
     // register the stub service
-    this.register('service:launch-darkly', stubService);
+    this.register('service:launch-darkly-client', StubClient);
 
     // inject here if you want to be able to inspect/manipulate the service in tests
-    this.inject.service('launch-darkly', { as: 'launchDarklyService' });
+    this.inject.service('launch-darkly-client', { as: 'launchDarklyClient' });
   }
 });
-```
 
-## Caveats
+test('new pricing', function(assert) {
+  this.render(hbs`
+    {{#if (variation "new-pricing-page")}}
+      <h1 class="price">£ 99</h1>
+    {{else}}
+      <h1 class="price">£ 199</h1>
+    {{/if}}
+  `);
 
-### Default variation state
+  this.get('launchDarklyClient').enable('new-pricing-page');
 
-By default a call to `variation` will return `false` if, for some reason, it can't get the true value of the feature flag. Therefore, it's important that variations are always used in the positive, ie, if the flag is enabled, perform new logic and if it's disabled, revert to the existing logic.
-
-The underlying Launch Darkly client provides the capability to specify what that default value is but we believe it's much easier to reason about feature flags if the default is always the same and you create your flags in such a way that enabling a flag is enabling the new state of the application. Therefore, this addon sets the default to `false` by design.
+  assert.equal(this.$('.price').text().trim(), '£ 99', 'New pricing displayed');
+});
+```
 
 ## TODO
 

addon-test-support/helpers/launch-darkly-client-test.js

@@ -0,0 +1,11 @@
+import LocalClient from 'ember-launch-darkly/services/launch-darkly-client-local';
+
+export default LocalClient.extend({
+  variation(key) {
+    return this._super(key, false);
+  },
+
+  _config() {
+    return {};
+  }
+});

addon-test-support/helpers/with-variation.js

@@ -1,8 +1,8 @@
 import Ember from 'ember';
 
 export function withVariation(app, key, value = true) {
-  let launchDarkly = app.__container__.lookup('service:launch-darkly');
-  launchDarkly.variation(key, value);
+  let client = app.__container__.lookup('service:launch-darkly-client');
+  client.setVariation(key, value);
 }
 
 Ember.Test.registerHelper('withVariation', withVariation);

addon/helpers/variation.js

@@ -7,13 +7,11 @@ export default Helper.extend({
   compute([key]) {
     let service = this.get('launchDarkly');
 
-    if (this._key) {
-      service.removeObserver(this._key, this, 'recompute');
+    if (!this._key) {
+      this._key = key;
+      service.addObserver(key, this, 'recompute');
     }
 
-    this._key = key;
-    service.addObserver(key, this, 'recompute');
-
     return service.get(key);
   },
 

addon/initializers/launch-darkly.js

@@ -1,5 +1,5 @@
-import RemoteService from 'ember-launch-darkly/services/launch-darkly-remote';
-import LocalService from 'ember-launch-darkly/services/launch-darkly-local';
+import RemoteClient from 'ember-launch-darkly/services/launch-darkly-client-remote';
+import LocalClient from 'ember-launch-darkly/services/launch-darkly-client-local';
 import { assign } from 'ember-platform';
 
 export function initialize(application) {
@@ -12,9 +12,9 @@ export function initialize(application) {
   let config = appConfig.launchDarkly || {};
   config = assign({}, defaults, config);
 
-  let Factory = config.local ? LocalService : RemoteService;
+  let Factory = config.local ? LocalClient : RemoteClient;
 
-  application.register('service:launch-darkly', Factory);
+  application.register('service:launch-darkly-client', Factory);
 }
 
 export default {

addon/instance-initializers/expose-local-launch-darkly.js

@@ -11,7 +11,7 @@ export function initialize(appInstance) {
   config = assign({}, defaults, config);
 
   if (config.local) {
-    let client = appInstance.lookup('service:launch-darkly');
+    let client = appInstance.lookup('service:launch-darkly-client');
     window.ld = client;
   }
 }

addon/lib/null-client.js

@@ -6,7 +6,11 @@ export default {
   allFlags() {
     return {};
   },
-  variation() {
+  variation(_, defaultValue) {
+    if (defaultValue !== undefined) {
+      return defaultValue
+    }
+
     return false;
   }
 }

addon/services/launch-darkly-client-local.js

@@ -0,0 +1,64 @@
+import Service from 'ember-service';
+import getOwner from 'ember-owner/get';
+import RSVP from 'rsvp';
+import Evented from 'ember-evented';
+import { assign } from 'ember-platform';
+
+export default Service.extend(Evented, {
+  _allFlags: null,
+  _user: null,
+
+  init() {
+    this._super(...arguments);
+
+    let { localFeatureFlags } = this._config();
+    localFeatureFlags = localFeatureFlags || {}
+
+    this._allFlags = localFeatureFlags;
+  },
+
+  initialize(user) {
+    return this.identify(user);
+  },
+
+  identify(user) {
+    this._user = user;
+    return RSVP.resolve();
+  },
+
+  allFlags() {
+    return assign({}, this._allFlags);
+  },
+
+  variation(key, defaultValue = false) {
+    if (this._allFlags.hasOwnProperty(key)) {
+      return this._allFlags[key];
+    }
+
+    return defaultValue;
+  },
+
+  enable(key) {
+    this.setVariation(key, true);
+  },
+
+  disable(key) {
+    this.setVariation(key, false);
+  },
+
+  setVariation(key, value) {
+    this._allFlags[key] = value;
+    this.trigger(key);
+  },
+
+  user() {
+    return this._user;
+  },
+
+  _config() {
+    let appConfig = getOwner(this).resolveRegistration('config:environment');
+    let config = appConfig.launchDarkly || {};
+
+    return assign({}, config);
+  }
+});