Merge pull request #2354 from hypothesis/proper-feature-flags

Proper feature flags

h/static/scripts/app-controller.coffee

@@ -5,19 +5,24 @@ module.exports = class AppController
   this.$inject = [
     '$controller', '$document', '$location', '$rootScope', '$route', '$scope',
     '$window',
-    'auth', 'drafts', 'identity',
+    'auth', 'drafts', 'features', 'identity',
     'permissions', 'streamer', 'annotationUI',
     'annotationMapper', 'threading'
   ]
   constructor: (
      $controller,   $document,   $location,   $rootScope,   $route,   $scope,
      $window,
-     auth,   drafts,   identity,
+     auth,   drafts,   features,   identity,
      permissions,   streamer,   annotationUI,
      annotationMapper, threading
   ) ->
     $controller('AnnotationUIController', {$scope})
 
+    # Allow all child scopes to look up feature flags as:
+    #
+    #     if ($scope.feature('foo')) { ... }
+    $scope.feature = features.flagEnabled
+
     $scope.auth = auth
     isFirstRun = $location.search().hasOwnProperty('firstrun')
 

h/static/scripts/app.coffee

@@ -78,6 +78,8 @@ setupStreamer = [
     $http.defaults.headers.common['X-Client-Id'] = clientId
 ]
 
+setupFeatures = ['features', (features) -> features.fetch()]
+
 module.exports = angular.module('h', [
   'angulartics'
   'angulartics.google.analytics'
@@ -128,6 +130,7 @@ module.exports = angular.module('h', [
 .service('bridge', require('./bridge'))
 .service('crossframe', require('./cross-frame'))
 .service('drafts', require('./drafts'))
+.service('features', require('./features'))
 .service('flash', require('./flash'))
 .service('formRespond', require('./form-respond'))
 .service('host', require('./host'))
@@ -155,6 +158,7 @@ module.exports = angular.module('h', [
 .config(configureRoutes)
 .config(configureTemplates)
 
+.run(setupFeatures)
 .run(setupCrossFrame)
 .run(setupStreamer)
 .run(setupHost)

h/static/scripts/features.js

+/**
+ * Feature flag client.
+ *
+ * This is a small utility which will periodically retrieve the application
+ * feature flags from a JSON endpoint in order to expose these to the
+ * client-side application.
+ *
+ * All feature flags implicitly start toggled off. When `flagEnabled` is first
+ * called (or alternatively when `fetch` is called explicitly) an XMLHTTPRequest
+ * will be made to retrieve the current feature flag values from the server.
+ * Once these are retrieved, `flagEnabled` will return current values.
+ *
+ * If `flagEnabled` is called and the cache is more than `CACHE_TTL`
+ * milliseconds old, then it will trigger a new fetch of the feature flag
+ * values. Note that this is again done asynchronously, so it is only later
+ * calls to `flagEnabled` that will return the updated values.
+ *
+ * Users of this service should assume that the value of any given flag can
+ * change at any time and should write code accordingly. Feature flags should
+ * not be cached, and should not be interrogated only at setup time.
+ */
+"use strict";
+
+var CACHE_TTL = 5 * 60 * 1000; // 5 minutes
+
+function features ($document, $http, $log) {
+    var cache = null;
+    var featuresURL = new URL('/app/features', $document.prop('baseURI'));
+
+    function fetch() {
+        $http.get(featuresURL)
+        .success(function(data) {
+            cache = [Date.now(), data];
+        })
+        .error(function() {
+            $log.warn('features service: failed to load features data');
+        });
+    }
+
+    function flagEnabled(name) {
+        // Trigger a fetch if the cache is more than CACHE_TTL milliseconds old.
+        // We don't wait for the fetch to complete, so it's not this call that
+        // will see new data.
+        if (cache === null || (Date.now() - cache[0]) > CACHE_TTL) {
+            fetch();
+        }
+
+        if (cache === null) {
+            return false;
+        }
+        var flags = cache[1];
+        if (!flags.hasOwnProperty(name)) {
+            $log.warn('features service: looked up unknown feature:', name);
+            return false;
+        }
+        return flags[name];
+    }
+
+    return {
+        fetch: fetch,
+        flagEnabled: flagEnabled
+    };
+}
+
+module.exports = ['$document', '$http', '$log', features];

h/static/scripts/test/app-controller-test.coffee

@@ -11,6 +11,7 @@ describe 'AppController', ->
   fakeAnnotationUI = null
   fakeAuth = null
   fakeDrafts = null
+  fakeFeatures = null
   fakeIdentity = null
   fakeLocation = null
   fakeParams = null
@@ -54,6 +55,11 @@ describe 'AppController', ->
       discard: sandbox.spy()
     }
 
+    fakeFeatures = {
+      fetch: sandbox.spy()
+      flagEnabled: sandbox.stub().returns(false)
+    }
+
     fakeIdentity = {
       watch: sandbox.spy()
       request: sandbox.spy()
@@ -97,6 +103,7 @@ describe 'AppController', ->
     $provide.value 'annotationUI', fakeAnnotationUI
     $provide.value 'auth', fakeAuth
     $provide.value 'drafts', fakeDrafts
+    $provide.value 'features', fakeFeatures
     $provide.value 'identity', fakeIdentity
     $provide.value '$location', fakeLocation
     $provide.value '$routeParams', fakeParams

h/static/scripts/test/features-test.js

+"use strict";
+
+var mock = require('angular-mock');
+
+var assert = chai.assert;
+sinon.assert.expose(assert, {prefix: null});
+
+
+describe('h:features', function () {
+    var $httpBackend;
+    var httpHandler;
+    var features;
+    var sandbox;
+
+    before(function () {
+        angular.module('h', [])
+        .service('features', require('../features'));
+    });
+
+    beforeEach(mock.module('h'));
+
+    beforeEach(mock.module(function ($provide) {
+        sandbox = sinon.sandbox.create();
+
+        var fakeDocument = {
+            prop: sandbox.stub()
+        };
+        fakeDocument.prop.withArgs('baseURI').returns('http://foo.com/');
+        $provide.value('$document', fakeDocument);
+    }));
+
+    beforeEach(mock.inject(function ($injector) {
+        $httpBackend = $injector.get('$httpBackend');
+        features = $injector.get('features');
+
+        httpHandler = $httpBackend.when('GET', 'http://foo.com/app/features');
+        httpHandler.respond(200, {foo: true, bar: false});
+    }));
+
+    afterEach(function () {
+        $httpBackend.verifyNoOutstandingExpectation();
+        $httpBackend.verifyNoOutstandingRequest();
+        sandbox.restore();
+    });
+
+    it('fetch should retrieve features data', function () {
+        $httpBackend.expect('GET', 'http://foo.com/app/features');
+        features.fetch();
+        $httpBackend.flush();
+    });
+
+    it('fetch should not explode for errors fetching features data', function () {
+        httpHandler.respond(500, "ASPLODE!");
+        features.fetch();
+        $httpBackend.flush();
+    });
+
+    it('flagEnabled should retrieve features data', function () {
+        $httpBackend.expect('GET', 'http://foo.com/app/features');
+        features.flagEnabled('foo');
+        $httpBackend.flush();
+    });
+
+    it('flagEnabled should return false initially', function () {
+        var result = features.flagEnabled('foo');
+        $httpBackend.flush();
+
+        assert.isFalse(result);
+    });
+
+    it('flagEnabled should return flag values when data is loaded', function () {
+        features.fetch();
+        $httpBackend.flush();
+
+        var foo = features.flagEnabled('foo');
+        assert.isTrue(foo);
+
+        var bar = features.flagEnabled('bar');
+        assert.isFalse(bar);
+    });
+
+    it('flagEnabled should return false for unknown flags', function () {
+        features.fetch();
+        $httpBackend.flush();
+
+        var baz = features.flagEnabled('baz');
+        assert.isFalse(baz);
+    });
+
+    it('flagEnabled should trigger a new fetch after cache expiry', function () {
+        var clock = sandbox.useFakeTimers();
+
+        $httpBackend.expect('GET', 'http://foo.com/app/features');
+        features.flagEnabled('foo');
+        $httpBackend.flush();
+
+        clock.tick(301 * 1000);
+
+        $httpBackend.expect('GET', 'http://foo.com/app/features');
+        features.flagEnabled('foo');
+        $httpBackend.flush();
+    });
+});

h/templates/client/settings/notifications.html

-<div class="tab-pane" title="Notifications">
+<div ng-if="feature('notification')" class="tab-pane" title="Notifications">
   <form class="account-form form" name="notificationsForm">
     <p class="form-description">Receive notification emails when:</p>
     <div class="form-field form-checkbox-list">