diff --git a/Gemfile b/Gemfile index d5f6eb6455..34f489c5c9 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index d2d7bfb92b..631b86dde3 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,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) 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/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 new file mode 100644 index 0000000000..96e1ce03eb --- /dev/null +++ b/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? } 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 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 diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index f57e740bc4..2863ef4cee 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/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 diff --git a/spec/lib/open_food_network/feature_toggle_spec.rb b/spec/lib/open_food_network/feature_toggle_spec.rb index d9904b0fd6..4d60fd060d 100644 --- a/spec/lib/open_food_network/feature_toggle_spec.rb +++ b/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 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