Merge pull request #7259 from jibees/7193-implements-feature-toggle-with-flipper

Implements feature toggle with flipper

Gemfile

@@ -113,6 +113,10 @@ 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
   gem 'ddtrace'
   gem 'unicorn-worker-killer'

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,15 @@ GEM
     ffi (1.15.0)
     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)
+      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 +651,9 @@ DEPENDENCIES
   factory_bot_rails (= 6.1.0)
   ffaker
   figaro
+  flipper
+  flipper-active_record
+  flipper-ui
   fog-aws (>= 0.6.0)
   foundation-icons-sass-rails
   foundation-rails (= 5.5.2.1)

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?

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

config/initializers/flipper.rb

@@ -0,0 +1,11 @@
+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? }

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

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

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

lib/open_food_network/feature_toggle.rb

@@ -25,44 +25,22 @@ 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)
+      features = Thread.current[:features] || {}
+
+      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
 
     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 user.present?
-        feature = features.fetch(feature_name, NullFeature.new)
-        feature.enabled?(user)
-      else
-        true?(env_variable_value(feature_name))
-      end
-    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 +57,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

spec/lib/open_food_network/feature_toggle_spec.rb

@@ -24,13 +24,25 @@ 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
     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
@@ -38,19 +50,21 @@ 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) }
+          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, insider)).to eq(true)
+          expect(FeatureToggle.enabled?(:foo, outsider)).to eq(false)
+          expect(FeatureToggle.enabled?(:foo, nil)).to eq(false)
         end
       end
     end

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