From a712f254e0424f7756f73f49103e161802e21cd7 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 18 Jun 2021 10:13:41 +1000 Subject: [PATCH 1/3] Remove Australian custom homepage content This was activated with a feature toggle. But when we re-implemented the feature toggles, we accidentally broke this one and it hasn't been active for two months. Nobody complained and it doesn't seem to be needed while developers are keen to remove instance-specific customisations anyway. --- app/views/home/index.html.haml | 12 ++---------- config/locales/en.yml | 8 -------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/app/views/home/index.html.haml b/app/views/home/index.html.haml index 485fa84cff..49da911e20 100644 --- a/app/views/home/index.html.haml +++ b/app/views/home/index.html.haml @@ -10,16 +10,8 @@ #panes = render "home/brandstory" - - - if feature? :connect_learn_homepage - = render "home/learn" - = render "home/connect" - = render "home/system" - - else - = render "home/system" - = render "home/cta" - + = render "home/system" + = render "home/cta" = render "home/stats" - = render "shared/footer" diff --git a/config/locales/en.yml b/config/locales/en.yml index 85ef5a75a5..323d10b26e 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1476,8 +1476,6 @@ en: label_producers: "Producers" label_groups: "Groups" label_about: "About" - label_connect: "Connect" - label_learn: "Learn" label_blog: "Blog" label_support: "Support" label_shopping: "Shopping" @@ -1571,12 +1569,6 @@ en: brandstory_part5_strong: "We call it Open Food Network." brandstory_part6: "We all love food. Now we can love our food system too." - learn_body: "Explore models, stories and resources to support you to develop your fair food business or organisation. Find training, events and other opportunities to learn from peers." - learn_cta: "Get Inspired" - - connect_body: "Search our full directories of producers, hubs and groups to find fair food traders near you. List your business or organisation on the OFN so buyers can find you. Join the community to get advice and solve problems together." - connect_cta: "Go Exploring" - system_headline: "Shopping - here's how it works." system_step1: "1. Search" system_step1_text: "Search our diverse, independent shops for seasonal local food. Search by neighbourhood and food category, or whether you prefer delivery or pickup." From bba85e5e7947a468ef025e083b3a7439688fec7f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 18 Jun 2021 10:27:06 +1000 Subject: [PATCH 2/3] Remove feature toggling via ENV vars We don't use ENV vars for feature toggles any more. Flipper is much easier to manage. --- lib/open_food_network/feature_toggle.rb | 32 +++---------------- .../open_food_network/feature_toggle_spec.rb | 25 --------------- 2 files changed, 5 insertions(+), 52 deletions(-) diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index 2863ef4cee..ee87886914 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -1,12 +1,10 @@ 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: + # You can configure features via the Flipper config and web interface. See: # - # - if feature? :new_shiny_feature - # = render "new_shiny_feature" + # - config/initializers/flipper.rb + # - http://localhost:3000/admin/feature-toggle/features # # 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: @@ -32,7 +30,7 @@ module OpenFoodNetwork if Flipper[feature_name].exist? Flipper.enabled?(feature_name, user) else - feature = features.fetch(feature_name, DefaultFeature.new(feature_name)) + feature = features.fetch(feature_name, DefaultFeature.new) feature.enabled?(user) end end @@ -58,28 +56,8 @@ module OpenFoodNetwork end class DefaultFeature - attr_reader :feature_name - - def initialize(feature_name) - @feature_name = feature_name - end - def enabled?(_user) - true?(env_variable_value(feature_name)) - end - - private - - def env_variable_value(feature_name) - ENV.fetch(env_variable_name(feature_name), nil) - end - - def env_variable_name(feature_name) - "OFN_FEATURE_#{feature_name.to_s.upcase}" - end - - def true?(value) - value.to_s.casecmp("true").zero? + 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 4d60fd060d..a91434a8a0 100644 --- a/spec/lib/open_food_network/feature_toggle_spec.rb +++ b/spec/lib/open_food_network/feature_toggle_spec.rb @@ -5,21 +5,6 @@ require 'spec_helper' module OpenFoodNetwork describe FeatureToggle do 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 @@ -28,16 +13,6 @@ module OpenFoodNetwork Flipper.enable(:foo) expect(FeatureToggle.enabled?(:foo)).to be true end - - it "uses Flipper over static config" do - Flipper.enable(:foo, false) - stub_foo("true") - 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 context 'when specifying users' do From a8eecf963dcd7aee50014884469da3d7b11428c4 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 18 Jun 2021 10:44:46 +1000 Subject: [PATCH 3/3] Remove unused custom FeatureToggle implementation Flipper is now our only source of truth regarding feature toggles. --- lib/open_food_network/feature_toggle.rb | 56 ++----------------- .../open_food_network/feature_toggle_spec.rb | 29 ---------- 2 files changed, 4 insertions(+), 81 deletions(-) diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index ee87886914..c93e12d069 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -1,63 +1,15 @@ module OpenFoodNetwork - # This feature toggles implementation provides two mechanisms to conditionally enable features. + # Feature toggles are configured via Flipper. # - # You can configure features via the Flipper config and web interface. See: + # We define features in the initializer and then it can be customised via the + # web interface on each server. # # - config/initializers/flipper.rb # - http://localhost:3000/admin/feature-toggle/features # - # 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" - # module FeatureToggle def self.enabled?(feature_name, user = nil) - features = Thread.current[:features] || {} - - if Flipper[feature_name].exist? - Flipper.enabled?(feature_name, user) - else - feature = features.fetch(feature_name, DefaultFeature.new) - feature.enabled?(user) - end - end - - def self.enable(feature_name, &block) - Thread.current[:features] ||= {} - Thread.current[:features][feature_name] = Feature.new(block) - end - end - - class Feature - def initialize(block) - @block = block - end - - def enabled?(user) - block.call(user) - end - - private - - attr_reader :block - end - - class DefaultFeature - def enabled?(_user) - false + Flipper.enabled?(feature_name, user) 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 a91434a8a0..97e41108a6 100644 --- a/spec/lib/open_food_network/feature_toggle_spec.rb +++ b/spec/lib/open_food_network/feature_toggle_spec.rb @@ -14,34 +14,5 @@ module OpenFoodNetwork expect(FeatureToggle.enabled?(:foo)).to be true end end - - context 'when specifying users' do - let(:insider) { build(:user) } - let(:outsider) { build(:user, email: "different") } - - 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, insider)).to eq('return value') - end - end - - context 'and the block specifies arguments' do - let(:users) { [insider.email] } - - before do - FeatureToggle.enable(:foo) { |user| users.include?(user&.email) } - end - - it "returns the block's return value" do - expect(FeatureToggle.enabled?(:foo, insider)).to eq(true) - expect(FeatureToggle.enabled?(:foo, outsider)).to eq(false) - expect(FeatureToggle.enabled?(:foo, nil)).to eq(false) - end - end - end end end