From 14cee0e45d15e711d877db9954ebfd317cb630ad Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 3 Feb 2021 11:41:23 +0100 Subject: [PATCH 1/4] Add new Feature class to toggle based on a closure This enables toggling features as best fits us in each case. With this new approach we can then toggle :customer_balance to an entire instance, which is what we want in France. --- config/initializers/feature_toggles.rb | 10 +++- lib/open_food_network/feature_toggle.rb | 25 ++++++++-- spec/initializers/feature_toggles_spec.rb | 47 +++++++++++++++++++ .../open_food_network/feature_toggle_spec.rb | 26 ++++++++++ 4 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 spec/initializers/feature_toggles_spec.rb diff --git a/config/initializers/feature_toggles.rb b/config/initializers/feature_toggles.rb index 13d083979c..9c31940462 100644 --- a/config/initializers/feature_toggles.rb +++ b/config/initializers/feature_toggles.rb @@ -1,5 +1,11 @@ require 'open_food_network/feature_toggle' -beta_testers = ENV['BETA_TESTERS']&.split(/[\s,]+/) +beta_testers = ENV['BETA_TESTERS']&.split(/[\s,]+/) || [] -OpenFoodNetwork::FeatureToggle.enable(:customer_balance, beta_testers) +OpenFoodNetwork::FeatureToggle.enable(:customer_balance) do |user| + if beta_testers == ['all'] + true + else + beta_testers.include?(user.email) + end +end diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index 5ee51a82f9..fb62be5553 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -30,11 +30,14 @@ module OpenFoodNetwork new.enabled?(feature_name, user) end - def self.enable(feature_name, user_emails) - return unless user_emails.present? - + def self.enable(feature_name, user_emails = nil, &block) Thread.current[:features] ||= {} - Thread.current[:features][feature_name] = Feature.new(user_emails) + + if user_emails.nil? + Thread.current[:features][feature_name] = BlockFeature.new(block) + else + Thread.current[:features][feature_name] = Feature.new(user_emails) + end end def initialize @@ -86,4 +89,18 @@ module OpenFoodNetwork false end end + + class BlockFeature + def initialize(block) + @block = block + end + + def enabled?(user) + block.call(user) + end + + private + + attr_reader :block + end end diff --git a/spec/initializers/feature_toggles_spec.rb b/spec/initializers/feature_toggles_spec.rb new file mode 100644 index 0000000000..daea8a2f73 --- /dev/null +++ b/spec/initializers/feature_toggles_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe 'config/initializers/feature_toggles.rb' do + let(:user) { build(:user) } + + around do |example| + original = ENV['BETA_TESTERS'] + example.run + ENV['BETA_TESTERS'] = original + end + + context 'when beta_testers is ["all"]' do + before { ENV['BETA_TESTERS'] = 'all' } + + it 'returns true' do + require './config/initializers/feature_toggles' # execute the initializer's code block + + enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) + expect(enabled).to eq(true) + end + end + + context 'when beta_testers is a list of emails' do + context 'and the user is in the list' do + let(:other_user) { build(:user) } + before { ENV['BETA_TESTERS'] = "#{user.email}, #{other_user.email}" } + + it 'enables the feature' do + require './config/initializers/feature_toggles' # execute the initializer's code block + + enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) + expect(enabled).to eq(true) + end + end + + context 'and the user is not in the list' do + before { ENV['BETA_TESTERS'] = '' } + + it 'disables the feature' do + require './config/initializers/feature_toggles' # execute the initializer's code block + + enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) + expect(enabled).to eq(false) + end + end + 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 73407d671d..f8074410c1 100644 --- a/spec/lib/open_food_network/feature_toggle_spec.rb +++ b/spec/lib/open_food_network/feature_toggle_spec.rb @@ -48,5 +48,31 @@ module OpenFoodNetwork end end end + + context 'when passing in a block' do + let(:user) { build(:user) } + + context 'and the block does not specify arguments' do + before do + FeatureToggle.enable(:foo) { 'return value' } + end + + it "returns the block's return value" do + expect(FeatureToggle.enabled?(:foo, user)).to eq('return value') + end + end + + context 'and the block specifies arguments' do + let(:users) { [user.email] } + + before do + FeatureToggle.enable(:foo) { |user| users.include?(user.email) } + end + + it "returns the block's return value" do + expect(FeatureToggle.enabled?(:foo, user)).to eq(true) + end + end + end end end From d6350c3d0b96f2e5fdafa1a8bfdd6b3c4969d990 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 3 Feb 2021 11:47:27 +0100 Subject: [PATCH 2/4] Remove deprecated Feature class implementation This became dead code now. --- lib/open_food_network/feature_toggle.rb | 35 +++++-------------- .../open_food_network/feature_toggle_spec.rb | 20 ----------- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index fb62be5553..f57e740bc4 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -30,14 +30,9 @@ module OpenFoodNetwork new.enabled?(feature_name, user) end - def self.enable(feature_name, user_emails = nil, &block) + def self.enable(feature_name, &block) Thread.current[:features] ||= {} - - if user_emails.nil? - Thread.current[:features][feature_name] = BlockFeature.new(block) - else - Thread.current[:features][feature_name] = Feature.new(user_emails) - end + Thread.current[:features][feature_name] = Feature.new(block) end def initialize @@ -71,26 +66,6 @@ module OpenFoodNetwork end class Feature - def initialize(users = []) - @users = users - end - - def enabled?(user) - users.include?(user.email) - end - - private - - attr_reader :users - end - - class NullFeature - def enabled?(_user) - false - end - end - - class BlockFeature def initialize(block) @block = block end @@ -103,4 +78,10 @@ module OpenFoodNetwork attr_reader :block end + + class NullFeature + def enabled?(_user) + false + end + 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 f8074410c1..d9904b0fd6 100644 --- a/spec/lib/open_food_network/feature_toggle_spec.rb +++ b/spec/lib/open_food_network/feature_toggle_spec.rb @@ -32,26 +32,6 @@ module OpenFoodNetwork context 'when specifying users' do let(:user) { build(:user) } - context 'and the feature is enabled for them' do - before { FeatureToggle.enable(:foo, [user.email]) } - - it 'returns true' do - expect(FeatureToggle.enabled?(:foo, user)).to eq(true) - end - end - - 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 - - context 'when passing in a block' do - let(:user) { build(:user) } - context 'and the block does not specify arguments' do before do FeatureToggle.enable(:foo) { 'return value' } From 88e22a78c2a9c213c6dce03f351ac39bf792c1fb Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 4 Feb 2021 13:16:03 +0100 Subject: [PATCH 3/4] Execute Ruby file in each test example Kernel#require prevented this from happening thus, the file was only loaded once so tests were passing so far by pure luck. --- spec/initializers/feature_toggles_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/initializers/feature_toggles_spec.rb b/spec/initializers/feature_toggles_spec.rb index daea8a2f73..0051c084a4 100644 --- a/spec/initializers/feature_toggles_spec.rb +++ b/spec/initializers/feature_toggles_spec.rb @@ -1,6 +1,12 @@ require 'spec_helper' describe 'config/initializers/feature_toggles.rb' do + # Executes the initializer's code block by reading the Ruby file. Note that `Kernel#require` would + # prevent this from happening twice. + subject(:execute_initializer) do + load Rails.root.join('config/initializers/feature_toggles.rb') + end + let(:user) { build(:user) } around do |example| @@ -13,7 +19,7 @@ describe 'config/initializers/feature_toggles.rb' do before { ENV['BETA_TESTERS'] = 'all' } it 'returns true' do - require './config/initializers/feature_toggles' # execute the initializer's code block + execute_initializer enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) expect(enabled).to eq(true) @@ -26,7 +32,7 @@ describe 'config/initializers/feature_toggles.rb' do before { ENV['BETA_TESTERS'] = "#{user.email}, #{other_user.email}" } it 'enables the feature' do - require './config/initializers/feature_toggles' # execute the initializer's code block + execute_initializer enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) expect(enabled).to eq(true) @@ -37,7 +43,7 @@ describe 'config/initializers/feature_toggles.rb' do before { ENV['BETA_TESTERS'] = '' } it 'disables the feature' do - require './config/initializers/feature_toggles' # execute the initializer's code block + execute_initializer enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) expect(enabled).to eq(false) From 655fe887f9a04864d700472f84227f49896d2ae1 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Thu, 4 Feb 2021 13:19:04 +0100 Subject: [PATCH 4/4] Distinguish user not present and list empty This is a bit more thorough. --- spec/initializers/feature_toggles_spec.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/initializers/feature_toggles_spec.rb b/spec/initializers/feature_toggles_spec.rb index 0051c084a4..0433d4508b 100644 --- a/spec/initializers/feature_toggles_spec.rb +++ b/spec/initializers/feature_toggles_spec.rb @@ -27,8 +27,9 @@ describe 'config/initializers/feature_toggles.rb' do end context 'when beta_testers is a list of emails' do + let(:other_user) { build(:user) } + context 'and the user is in the list' do - let(:other_user) { build(:user) } before { ENV['BETA_TESTERS'] = "#{user.email}, #{other_user.email}" } it 'enables the feature' do @@ -40,6 +41,17 @@ describe 'config/initializers/feature_toggles.rb' do end context 'and the user is not in the list' do + before { ENV['BETA_TESTERS'] = "#{other_user.email}" } + + it 'disables the feature' do + execute_initializer + + enabled = OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, user) + expect(enabled).to eq(false) + end + end + + context 'and the list is empty' do before { ENV['BETA_TESTERS'] = '' } it 'disables the feature' do