From 182f0f66b628d882028a018ebb2643a8bf110de5 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 10 Dec 2020 10:25:21 +0100 Subject: [PATCH] Refactor FeatureToggle to toggle depending on user This enables showing features to individual users only, which enables us to deploy features that are not yet released to gather feedback from product and testing, while no users have access to it. --- app/helpers/application_helper.rb | 4 +- lib/open_food_network/feature_toggle.rb | 70 +++++++++++++++++-- .../open_food_network/feature_toggle_spec.rb | 56 ++++++++++----- 3 files changed, 106 insertions(+), 24 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 2e03e68bc0..c23fc9fab2 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,6 +1,6 @@ module ApplicationHelper - def feature?(feature) - OpenFoodNetwork::FeatureToggle.enabled? feature + def feature?(feature, user = nil) + OpenFoodNetwork::FeatureToggle.enabled?(feature, user) end def ng_form_for(name, *args, &block) diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index e979bbf361..d40a89b0db 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -1,21 +1,81 @@ module OpenFoodNetwork + # This feature toggles implementation provides two mechanisms to conditionally enable features. + # + # You can provide an ENV var with the prefix `OFN_FEATURE_` and query it using the + # `ApplicationHelper#feature?` helper method. For instance, providing the ENV var + # `OFN_FEATURE_NEW_SHINNY_FEATURE` you could then query it from view as follows: + # + # - if feature? :new_shiny_feature + # = render "new_shiny_feature" + # + # Alternatively, you can choose which users have the feature toggled on. To do that you need to + # register the feature and its users from an initializer like: + # + # require 'open_food_network/feature_toggle' + # OpenFoodNetwork::FeatureToggle.enable(:new_shiny_feature, ['ofn@example.com']) + # + # Note, however, that it'd be better to read the user emails from an ENV var provisioned with + # ofn-install: + # + # require 'open_food_network/feature_toggle' + # OpenFoodNetwork::FeatureToggle.enable(:new_shiny_feature, ENV['PRODUCT_TEAM']) + # + # You can then check it from a view like: + # + # - if feature? :new_shiny_feature, spree_current_user + # = render "new_shiny_feature" + # class FeatureToggle - def self.enabled?(feature_name) - true?(env_variable_value(feature_name)) + def self.enabled?(feature_name, user = nil) + new.enabled?(feature_name, user) + end + + def self.enable(feature_name, user_emails) + Thread.current[:features] ||= {} + Thread.current[:features][feature_name] = Feature.new(user_emails) + end + + def initialize + @features = Thread.current[:features] + end + + def enabled?(feature_name, user) + if user.present? + feature = features[feature_name] + feature.enabled?(user) + else + true?(env_variable_value(feature_name)) + end end private - def self.env_variable_value(feature_name) + attr_reader :features + + def env_variable_value(feature_name) ENV.fetch(env_variable_name(feature_name), nil) end - def self.env_variable_name(feature_name) + def env_variable_name(feature_name) "OFN_FEATURE_#{feature_name.to_s.upcase}" end - def self.true?(value) + def true?(value) value.to_s.casecmp("true").zero? end end + + class Feature + def initialize(users = []) + @users = users + end + + def enabled?(user) + users.include?(user.email) + end + + private + + attr_reader :users + end end diff --git a/spec/lib/open_food_network/feature_toggle_spec.rb b/spec/lib/open_food_network/feature_toggle_spec.rb index 84956f556c..73407d671d 100644 --- a/spec/lib/open_food_network/feature_toggle_spec.rb +++ b/spec/lib/open_food_network/feature_toggle_spec.rb @@ -1,30 +1,52 @@ # frozen_string_literal: true -require 'open_food_network/feature_toggle' +require 'spec_helper' module OpenFoodNetwork describe FeatureToggle do - it "returns true when feature is on" do - stub_foo("true") - expect(FeatureToggle.enabled?(:foo)).to be true + context 'when users are not specified' do + it "returns true when feature is on" do + stub_foo("true") + expect(FeatureToggle.enabled?(:foo)).to be true + end + + it "returns false when feature is off" do + stub_foo("false") + expect(FeatureToggle.enabled?(:foo)).to be false + end + + it "returns false when feature is unspecified" do + stub_foo("maybe") + expect(FeatureToggle.enabled?(:foo)).to be false + end + + it "returns false when feature is undefined" do + expect(FeatureToggle.enabled?(:foo)).to be false + end + + def stub_foo(value) + allow(ENV).to receive(:fetch).with("OFN_FEATURE_FOO", nil).and_return(value) + end end - it "returns false when feature is off" do - stub_foo("false") - expect(FeatureToggle.enabled?(:foo)).to be false - end + context 'when specifying users' do + let(:user) { build(:user) } - it "returns false when feature is unspecified" do - stub_foo("maybe") - expect(FeatureToggle.enabled?(:foo)).to be false - end + context 'and the feature is enabled for them' do + before { FeatureToggle.enable(:foo, [user.email]) } - it "returns false when feature is undefined" do - expect(FeatureToggle.enabled?(:foo)).to be false - end + it 'returns true' do + expect(FeatureToggle.enabled?(:foo, user)).to eq(true) + end + end - def stub_foo(value) - allow(ENV).to receive(:fetch).with("OFN_FEATURE_FOO", nil).and_return(value) + context 'and the feature is disabled for them' do + before { FeatureToggle.enable(:foo, []) } + + it 'returns false' do + expect(FeatureToggle.enabled?(:foo, user)).to eq(false) + end + end end end end