From 7cf96d1484070f3254937303c0871c50bb1946e0 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Bellet <jb.bellet@gmail.com>
Date: Fri, 2 Apr 2021 09:38:34 +0200
Subject: [PATCH 1/9] Add flipper to manage our feature toggle

 - Also add flipper-ui to have a small UI that can be used by each instance admin to manage feature toggle
- Mount on `/admin/feature-toggle` with same restriction as `/admin`
---
 Gemfile                | 3 +++
 Gemfile.lock           | 9 +++++++++
 config/routes/admin.rb | 1 +
 3 files changed, 13 insertions(+)

diff --git a/Gemfile b/Gemfile
index 6b4b57ec5d..0069ffd3f8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -113,6 +113,9 @@ gem 'ofn-qz', github: 'openfoodfoundation/ofn-qz', branch: 'ofn-rails-4'
 
 gem 'good_migrations'
 
+gem 'flipper'
+gem 'flipper-ui'
+
 group :production, :staging do
   gem 'ddtrace'
   gem 'unicorn-worker-killer'
diff --git a/Gemfile.lock b/Gemfile.lock
index 53ee985274..6fba30ba60 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -214,6 +214,7 @@ GEM
       devise (>= 4.0.0, < 5.0.0)
     diff-lcs (1.4.4)
     docile (1.3.5)
+    erubi (1.10.0)
     erubis (2.7.0)
     eventmachine (1.2.7)
     excon (0.79.0)
@@ -232,6 +233,12 @@ GEM
     ffi (1.15.0)
     figaro (1.2.0)
       thor (>= 0.14.0, < 2)
+    flipper (0.20.4)
+    flipper-ui (0.20.4)
+      erubi (>= 1.0.0, < 2.0.0)
+      flipper (~> 0.20.4)
+      rack (>= 1.4, < 3)
+      rack-protection (>= 1.5.3, < 2.2.0)
     fog-aws (2.0.1)
       fog-core (~> 1.38)
       fog-json (~> 1.0)
@@ -641,6 +648,8 @@ DEPENDENCIES
   factory_bot_rails (= 6.1.0)
   ffaker
   figaro
+  flipper
+  flipper-ui
   fog-aws (>= 0.6.0)
   foundation-icons-sass-rails
   foundation-rails (= 5.5.2.1)
diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index 5b4a5b3479..88a577df18 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -3,6 +3,7 @@ Openfoodnetwork::Application.routes.draw do
 
     authenticated :spree_user, -> user { user.admin? } do
       mount DelayedJobWeb, at: '/delayed_job'
+      mount Flipper::UI.app(Flipper) => '/feature-toggle'
     end
 
     resources :bulk_line_items

From ad71f925be8f86b442839aa83d3bbd02ab5702da Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Bellet <jb.bellet@gmail.com>
Date: Thu, 1 Apr 2021 11:18:20 +0200
Subject: [PATCH 2/9] Add gem flipper-active_record to store flipper data

 - Creates the migration for flipper: `flipper_features` and `flipper_gates`
---
 Gemfile                                       |  1 +
 Gemfile.lock                                  |  4 +++
 .../20210326094519_create_flipper_tables.rb   | 22 ++++++++++++++
 db/schema.rb                                  | 30 ++++++++++++++-----
 4 files changed, 50 insertions(+), 7 deletions(-)
 create mode 100644 db/migrate/20210326094519_create_flipper_tables.rb

diff --git a/Gemfile b/Gemfile
index 0069ffd3f8..6b512cebd3 100644
--- a/Gemfile
+++ b/Gemfile
@@ -114,6 +114,7 @@ gem 'ofn-qz', github: 'openfoodfoundation/ofn-qz', branch: 'ofn-rails-4'
 gem 'good_migrations'
 
 gem 'flipper'
+gem 'flipper-active_record'
 gem 'flipper-ui'
 
 group :production, :staging do
diff --git a/Gemfile.lock b/Gemfile.lock
index 6fba30ba60..cd0ad8a269 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -234,6 +234,9 @@ GEM
     figaro (1.2.0)
       thor (>= 0.14.0, < 2)
     flipper (0.20.4)
+    flipper-active_record (0.20.4)
+      activerecord (>= 5.0, < 7)
+      flipper (~> 0.20.4)
     flipper-ui (0.20.4)
       erubi (>= 1.0.0, < 2.0.0)
       flipper (~> 0.20.4)
@@ -649,6 +652,7 @@ DEPENDENCIES
   ffaker
   figaro
   flipper
+  flipper-active_record
   flipper-ui
   fog-aws (>= 0.6.0)
   foundation-icons-sass-rails
diff --git a/db/migrate/20210326094519_create_flipper_tables.rb b/db/migrate/20210326094519_create_flipper_tables.rb
new file mode 100644
index 0000000000..e7b94de750
--- /dev/null
+++ b/db/migrate/20210326094519_create_flipper_tables.rb
@@ -0,0 +1,22 @@
+class CreateFlipperTables < ActiveRecord::Migration[5.0]
+  def self.up
+    create_table :flipper_features do |t|
+      t.string :key, null: false
+      t.timestamps null: false
+    end
+    add_index :flipper_features, :key, unique: true
+
+    create_table :flipper_gates do |t|
+      t.string :feature_key, null: false
+      t.string :key, null: false
+      t.string :value
+      t.timestamps null: false
+    end
+    add_index :flipper_gates, [:feature_key, :key, :value], unique: true
+  end
+
+  def self.down
+    drop_table :flipper_gates
+    drop_table :flipper_features
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 92c4a23937..310cc0420f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20210407170804) do
+ActiveRecord::Schema.define(version: 20210326094519) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -250,6 +250,22 @@ ActiveRecord::Schema.define(version: 20210407170804) do
     t.index ["sender_id"], name: "index_exchanges_on_sender_id", using: :btree
   end
 
+  create_table "flipper_features", force: :cascade do |t|
+    t.string   "key",        null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["key"], name: "index_flipper_features_on_key", unique: true, using: :btree
+  end
+
+  create_table "flipper_gates", force: :cascade do |t|
+    t.string   "feature_key", null: false
+    t.string   "key",         null: false
+    t.string   "value"
+    t.datetime "created_at",  null: false
+    t.datetime "updated_at",  null: false
+    t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true, using: :btree
+  end
+
   create_table "inventory_items", force: :cascade do |t|
     t.integer  "enterprise_id",                null: false
     t.integer  "variant_id",                   null: false
@@ -769,15 +785,15 @@ ActiveRecord::Schema.define(version: 20210407170804) do
   end
 
   create_table "spree_shipments", force: :cascade do |t|
-    t.string   "tracking",             limit: 255
-    t.string   "number",               limit: 255
-    t.decimal  "cost",                             precision: 10, scale: 2, default: "0.0", null: false
+    t.string   "tracking",          limit: 255
+    t.string   "number",            limit: 255
+    t.decimal  "cost",                          precision: 10, scale: 2, default: "0.0", null: false
     t.datetime "shipped_at"
     t.integer  "order_id"
     t.integer  "address_id"
-    t.datetime "created_at",                                                                null: false
-    t.datetime "updated_at",                                                                null: false
-    t.string   "state",                limit: 255
+    t.datetime "created_at",                                                             null: false
+    t.datetime "updated_at",                                                             null: false
+    t.string   "state",             limit: 255
     t.integer  "stock_location_id"
     t.decimal  "included_tax_total",               precision: 10, scale: 2, default: "0.0", null: false
     t.decimal  "additional_tax_total",             precision: 10, scale: 2, default: "0.0", null: false

From a4b53d6ac4d9faec9b57ff3b70c17b05efe11dea Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Bellet <jb.bellet@gmail.com>
Date: Tue, 30 Mar 2021 12:23:02 +0200
Subject: [PATCH 3/9] Initialize flipper with default value

 - Use the ActiveRecord adapter
 - Use a middle to cache data through memoization (see https://github.com/jnunemaker/flipper/blob/master/docs/Optimization.md)
 - Create the group `admins`: only user which are admins
 - Create `unit_price` feature attached to `admins` group
 - Add method `flipper_id` on User
---
 app/models/spree/user.rb       |  4 ++++
 config/initializers/flipper.rb | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 config/initializers/flipper.rb

diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb
index ab1ef6ddfc..a2982181e7 100644
--- a/app/models/spree/user.rb
+++ b/app/models/spree/user.rb
@@ -135,6 +135,10 @@ module Spree
       spree_orders.incomplete.where(created_by_id: id).order('created_at DESC').first
     end
 
+    def flipper_id
+      "#{self.class.name};#{id}"
+    end
+
     protected
 
     def password_required?
diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb
new file mode 100644
index 0000000000..f5351b71ab
--- /dev/null
+++ b/config/initializers/flipper.rb
@@ -0,0 +1,16 @@
+require "flipper"
+require "flipper/adapters/active_record"
+
+Flipper.configure do |config|
+  config.default do
+    Flipper.new(Flipper::Adapters::ActiveRecord.new)
+  end
+end
+Rails.configuration.middleware.use Flipper::Middleware::Memoizer, preload_all: true
+
+Flipper.register(:admins) { |actor| actor.respond_to?(:admin?) && actor.admin? }
+
+if !Flipper[:unit_price].exist?
+  # Unit price default setup, could be overided by admin in the flipper-ui interface
+  Flipper.enable_group :unit_price, :admins
+end

From b045a59685fa7f2a970f76f2313995ff4fec2c01 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 6 Apr 2021 14:48:07 +1000
Subject: [PATCH 4/9] Consider feature toggles without user as well

This is running the same feature toggle logic when a user is given or no
user is given. This allows features to define what to do with guests.
---
 lib/open_food_network/feature_toggle.rb       | 44 ++++++++++---------
 .../open_food_network/feature_toggle_spec.rb  |  4 +-
 2 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb
index f57e740bc4..b3c5b5a2ae 100644
--- a/lib/open_food_network/feature_toggle.rb
+++ b/lib/open_food_network/feature_toggle.rb
@@ -40,29 +40,13 @@ module OpenFoodNetwork
     end
 
     def enabled?(feature_name, user)
-      if user.present?
-        feature = features.fetch(feature_name, NullFeature.new)
-        feature.enabled?(user)
-      else
-        true?(env_variable_value(feature_name))
-      end
+      feature = features.fetch(feature_name, DefaultFeature.new(feature_name))
+      feature.enabled?(user)
     end
 
     private
 
     attr_reader :features
-
-    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?
-    end
   end
 
   class Feature
@@ -79,9 +63,29 @@ module OpenFoodNetwork
     attr_reader :block
   end
 
-  class NullFeature
+  class DefaultFeature
+    attr_reader :feature_name
+
+    def initialize(feature_name)
+      @feature_name = feature_name
+    end
+
     def enabled?(_user)
-      false
+      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?
     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 d9904b0fd6..3e5e3cfa4f 100644
--- a/spec/lib/open_food_network/feature_toggle_spec.rb
+++ b/spec/lib/open_food_network/feature_toggle_spec.rb
@@ -46,11 +46,13 @@ module OpenFoodNetwork
         let(:users) { [user.email] }
 
         before do
-          FeatureToggle.enable(:foo) { |user| users.include?(user.email) }
+          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)
+          expect(FeatureToggle.enabled?(:foo, OpenStruct.new(email: "different"))).to eq(false)
+          expect(FeatureToggle.enabled?(:foo, nil)).to eq(false)
         end
       end
     end

From 7a0912d5a44214403f54395374b1ff734cbc9732 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 6 Apr 2021 15:59:15 +1000
Subject: [PATCH 5/9] Use Flipper within the OFN FeatureToggle interface

This ensures that we keep the same interface as before and can migrate
to Flipper in a backwards-compatible way.
---
 lib/open_food_network/feature_toggle.rb           |  8 ++++++--
 spec/lib/open_food_network/feature_toggle_spec.rb | 11 +++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb
index b3c5b5a2ae..50fb761b6e 100644
--- a/lib/open_food_network/feature_toggle.rb
+++ b/lib/open_food_network/feature_toggle.rb
@@ -40,8 +40,12 @@ module OpenFoodNetwork
     end
 
     def enabled?(feature_name, user)
-      feature = features.fetch(feature_name, DefaultFeature.new(feature_name))
-      feature.enabled?(user)
+      if Flipper[feature_name].exist?
+        Flipper.enabled?(feature_name, user)
+      else
+        feature = features.fetch(feature_name, DefaultFeature.new(feature_name))
+        feature.enabled?(user)
+      end
     end
 
     private
diff --git a/spec/lib/open_food_network/feature_toggle_spec.rb b/spec/lib/open_food_network/feature_toggle_spec.rb
index 3e5e3cfa4f..60f23933cf 100644
--- a/spec/lib/open_food_network/feature_toggle_spec.rb
+++ b/spec/lib/open_food_network/feature_toggle_spec.rb
@@ -24,6 +24,17 @@ module OpenFoodNetwork
         expect(FeatureToggle.enabled?(:foo)).to be false
       end
 
+      it "uses Flipper configuration" do
+        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

From 796068439dee9576daefdc7a0ce0e8571d1b060f Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Bellet <jb.bellet@gmail.com>
Date: Thu, 8 Apr 2021 14:57:36 +0200
Subject: [PATCH 6/9] Enable running Flipper migration

The initializer ran code the needs the database in the migrated state.
The decision is to not initialize anything relative to feature. This has to be done within the FlipperUI by the instance managers.
---
 config/initializers/feature_toggles.rb | 4 ----
 config/initializers/flipper.rb         | 5 -----
 2 files changed, 9 deletions(-)

diff --git a/config/initializers/feature_toggles.rb b/config/initializers/feature_toggles.rb
index ed3ee27d23..0aa6aa07d4 100644
--- a/config/initializers/feature_toggles.rb
+++ b/config/initializers/feature_toggles.rb
@@ -3,7 +3,3 @@ require 'open_food_network/feature_toggle'
 OpenFoodNetwork::FeatureToggle.enable(:customer_balance) do |user|
   true
 end
-
-OpenFoodNetwork::FeatureToggle.enable(:unit_price) do
-  Rails.env.development?
-end
diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb
index f5351b71ab..96e1ce03eb 100644
--- a/config/initializers/flipper.rb
+++ b/config/initializers/flipper.rb
@@ -9,8 +9,3 @@ end
 Rails.configuration.middleware.use Flipper::Middleware::Memoizer, preload_all: true
 
 Flipper.register(:admins) { |actor| actor.respond_to?(:admin?) && actor.admin? }
-
-if !Flipper[:unit_price].exist?
-  # Unit price default setup, could be overided by admin in the flipper-ui interface
-  Flipper.enable_group :unit_price, :admins
-end

From 5775051e354eac491f74361b00130c3ca839686b Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 6 Apr 2021 16:18:50 +1000
Subject: [PATCH 7/9] Simplify FeatureToggle module

This saves ten lines of code and makes the simplicity of the
FeatureToggle interface clearer.
---
 lib/open_food_network/feature_toggle.rb | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb
index 50fb761b6e..2863ef4cee 100644
--- a/lib/open_food_network/feature_toggle.rb
+++ b/lib/open_food_network/feature_toggle.rb
@@ -25,21 +25,10 @@ module OpenFoodNetwork
   #   - if feature? :new_shiny_feature, spree_current_user
   #     = render "new_shiny_feature"
   #
-  class FeatureToggle
+  module FeatureToggle
     def self.enabled?(feature_name, user = nil)
-      new.enabled?(feature_name, user)
-    end
+      features = Thread.current[:features] || {}
 
-    def self.enable(feature_name, &block)
-      Thread.current[:features] ||= {}
-      Thread.current[:features][feature_name] = Feature.new(block)
-    end
-
-    def initialize
-      @features = Thread.current[:features] || {}
-    end
-
-    def enabled?(feature_name, user)
       if Flipper[feature_name].exist?
         Flipper.enabled?(feature_name, user)
       else
@@ -48,9 +37,10 @@ module OpenFoodNetwork
       end
     end
 
-    private
-
-    attr_reader :features
+    def self.enable(feature_name, &block)
+      Thread.current[:features] ||= {}
+      Thread.current[:features][feature_name] = Feature.new(block)
+    end
   end
 
   class Feature

From f242dc8a194fd73107675a966b985a8d3afa950d Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Wed, 7 Apr 2021 14:58:32 +1000
Subject: [PATCH 8/9] Spec new #flipper_id method

---
 spec/models/spree/user_spec.rb | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb
index 3a6975a8c8..406f41fdca 100644
--- a/spec/models/spree/user_spec.rb
+++ b/spec/models/spree/user_spec.rb
@@ -189,4 +189,11 @@ describe Spree::User do
       expect { user.destroy }.to raise_exception(Spree::User::DestroyWithOrdersError)
     end
   end
+
+  describe "#flipper_id" do
+    it "provides a unique id" do
+      user = Spree::User.new(id: 42)
+      expect(user.flipper_id).to eq "Spree::User;42"
+    end
+  end
 end

From 8a062b05eb3248755571d202def61d28712f411d Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 9 Apr 2021 16:54:35 +1000
Subject: [PATCH 9/9] Clarify test users in feature toggle spec

---
 spec/lib/open_food_network/feature_toggle_spec.rb | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/spec/lib/open_food_network/feature_toggle_spec.rb b/spec/lib/open_food_network/feature_toggle_spec.rb
index 60f23933cf..4d60fd060d 100644
--- a/spec/lib/open_food_network/feature_toggle_spec.rb
+++ b/spec/lib/open_food_network/feature_toggle_spec.rb
@@ -41,7 +41,8 @@ module OpenFoodNetwork
     end
 
     context 'when specifying users' do
-      let(:user) { build(:user) }
+      let(:insider) { build(:user) }
+      let(:outsider) { build(:user, email: "different") }
 
       context 'and the block does not specify arguments' do
         before do
@@ -49,20 +50,20 @@ module OpenFoodNetwork
         end
 
         it "returns the block's return value" do
-          expect(FeatureToggle.enabled?(:foo, user)).to eq('return value')
+          expect(FeatureToggle.enabled?(:foo, insider)).to eq('return value')
         end
       end
 
       context 'and the block specifies arguments' do
-        let(:users) { [user.email] }
+        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, user)).to eq(true)
-          expect(FeatureToggle.enabled?(:foo, OpenStruct.new(email: "different"))).to eq(false)
+          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