From 2d227378230991f3e47a0c4b82225ad3b6a2a408 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 2 Mar 2024 16:33:25 +0100 Subject: [PATCH 1/6] chore(flipper): enable strict mode and add missing features --- config/initializers/flipper.rb | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index 636d7379c..6b04b20b6 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -2,11 +2,12 @@ # we can add new features from the Web UI. However when the new DB is created # this will immediately migrate the default features to be controlled. def setup_features(features) - features.each do |feature| - if !Flipper.exist?(feature) - # Disable feature by default - Flipper.disable(feature) - end + existing = Flipper.preload(features).map { _1.name.to_sym } + missing = features - existing + + missing.each do |feature| + # Feature is disabled by default + Flipper.add(feature.to_s) end end @@ -14,13 +15,17 @@ end features = [ :administrateur_web_hook, :api_particulier, - :dossier_pdf_vide, - :hide_instructeur_email, - :procedure_routage_api, - :groupe_instructeur_api_hack, + :attestation_v2, + :blocking_pending_correction, :cojo_type_de_champ, + :dossier_pdf_vide, + :engagement_juridique_type_de_champ, + :export_order_by_revision, + :expression_reguliere_type_de_champ, + :groupe_instructeur_api_hack, + :hide_instructeur_email, :sva, - :blocking_pending_correction + :switch_domain ] def database_exists? @@ -35,3 +40,7 @@ ActiveSupport.on_load(:active_record) do setup_features(features) end end + +Rails.application.configure do + config.flipper.strict = Rails.env.development? +end From 3519e5eef1022704989baaaf92da16671af77b92 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 2 Mar 2024 16:37:03 +0100 Subject: [PATCH 2/6] chore(flipper): enable caching in memory --- Gemfile | 1 + Gemfile.lock | 4 ++++ config/initializers/flipper.rb | 14 ++++++++++++++ 3 files changed, 19 insertions(+) diff --git a/Gemfile b/Gemfile index 9838d6719..18a70463f 100644 --- a/Gemfile +++ b/Gemfile @@ -33,6 +33,7 @@ gem 'dotenv-rails', require: 'dotenv/rails-now' # dotenv should always be loaded gem 'dry-monads' gem 'flipper' gem 'flipper-active_record' +gem 'flipper-active_support_cache_store' gem 'flipper-ui' gem 'fugit' gem 'geocoder' diff --git a/Gemfile.lock b/Gemfile.lock index c5e7ab276..088b30b0f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -237,6 +237,9 @@ GEM flipper-active_record (1.2.2) activerecord (>= 4.2, < 8) flipper (~> 1.2.2) + flipper-active_support_cache_store (1.2.2) + activesupport (>= 4.2, < 8) + flipper (~> 1.2.2) flipper-ui (1.2.2) erubi (>= 1.0.0, < 2.0.0) flipper (~> 1.2.2) @@ -833,6 +836,7 @@ DEPENDENCIES factory_bot flipper flipper-active_record + flipper-active_support_cache_store flipper-ui fugit geo_coord diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index 6b04b20b6..4cbd0d1a4 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -1,6 +1,10 @@ # This setup is primarily for first deployment, because consequently # we can add new features from the Web UI. However when the new DB is created # this will immediately migrate the default features to be controlled. +# +require 'flipper/adapters/active_record' +require 'flipper/adapters/active_support_cache_store' + def setup_features(features) existing = Flipper.preload(features).map { _1.name.to_sym } missing = features - existing @@ -41,6 +45,16 @@ ActiveSupport.on_load(:active_record) do end end +Flipper.configure do |config| + config.adapter do + Flipper::Adapters::ActiveSupportCacheStore.new( + Flipper::Adapters::ActiveRecord.new, + ActiveSupport::Cache::MemoryStore.new, + expires_in: 10.seconds + ) + end +end + Rails.application.configure do config.flipper.strict = Rails.env.development? end From dbf04c63a4cf36900fd63965fded8b43d3e11b26 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 2 Mar 2024 16:38:00 +0100 Subject: [PATCH 3/6] fix(flipper): don't clone features globally enabled --- app/models/procedure.rb | 7 ++++--- spec/models/procedure_spec.rb | 20 ++++++++++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/app/models/procedure.rb b/app/models/procedure.rb index f8059a9d7..7f95fbbab 100644 --- a/app/models/procedure.rb +++ b/app/models/procedure.rb @@ -561,9 +561,10 @@ class Procedure < ApplicationRecord procedure.update!(defaut_groupe_instructeur: new_defaut_groupe) Flipper.features.each do |feature| - if feature_enabled?(feature.key) - Flipper.enable(feature.key, procedure) - end + next if feature.enabled? # don't clone features globally enabled + next unless feature_enabled?(feature.key) + + Flipper.enable(feature.key, procedure) end procedure diff --git a/spec/models/procedure_spec.rb b/spec/models/procedure_spec.rb index e0fcc9984..c1aedbd90 100644 --- a/spec/models/procedure_spec.rb +++ b/spec/models/procedure_spec.rb @@ -871,21 +871,33 @@ describe Procedure do describe 'feature flag' do context 'with a feature flag enabled' do before do - Flipper.enable(:procedure_routage_api, procedure) + Flipper.enable(:dossier_pdf_vide, procedure) end it 'should enable feature' do - expect(subject.feature_enabled?(:procedure_routage_api)).to be true + expect(subject.feature_enabled?(:dossier_pdf_vide)).to be true + expect(Flipper.feature(:dossier_pdf_vide).enabled_gate_names).to include(:actor) + end + end + + context 'with feature flag is fully enabled' do + before do + Flipper.enable(:dossier_pdf_vide) + end + + it 'should not clone feature for actor' do + expect(subject.feature_enabled?(:dossier_pdf_vide)).to be true + expect(Flipper.feature(:dossier_pdf_vide).enabled_gate_names).not_to include(:actor) end end context 'with a feature flag disabled' do before do - Flipper.disable(:procedure_routage_api, procedure) + Flipper.disable(:dossier_pdf_vide, procedure) end it 'should not enable feature' do - expect(subject.feature_enabled?(:procedure_routage_api)).to be false + expect(subject.feature_enabled?(:dossier_pdf_vide)).to be false end end end From c18999b8fb6c960c4ec9b542b6c735c701e80f4d Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sat, 2 Mar 2024 16:50:33 +0100 Subject: [PATCH 4/6] chore(flipper): delete obsolete features --- .../20240301223646_clean_old_gates2024.rake | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 lib/tasks/deployment/20240301223646_clean_old_gates2024.rake diff --git a/lib/tasks/deployment/20240301223646_clean_old_gates2024.rake b/lib/tasks/deployment/20240301223646_clean_old_gates2024.rake new file mode 100644 index 000000000..7ff668b04 --- /dev/null +++ b/lib/tasks/deployment/20240301223646_clean_old_gates2024.rake @@ -0,0 +1,29 @@ +namespace :after_party do + desc 'Deployment task: clean_old_gates2024' + task clean_old_gates2024: :environment do + puts "Running deploy task 'clean_old_gates2024'" + + keys = [ + 'admin_affect_experts_to_avis', + 'chorus', + 'disable_label_optional_champ_2023_06_29', + 'expert_not_allowed_to_invite', + 'instructeur_bypass_email_login_token', + 'multi_line_routing', + 'opendata', + 'procedure_conditional', + 'procedure_routage_api', + 'rerouting', + 'routing_rules', + 'zonage' + ] + + Flipper::Adapters::ActiveRecord::Gate.where(feature_key: keys).delete_all + Flipper::Adapters::ActiveRecord::Feature.where(key: keys).delete_all + + # Update task as completed. If you remove the line below, the task will + # run with every deploy (or every time you call after_party:run). + AfterParty::TaskRecord + .create version: AfterParty::TaskRecorder.new(__FILE__).timestamp + end +end From f342bcb58b34050cd087cef9827e85796384f666 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sun, 3 Mar 2024 00:10:01 +0100 Subject: [PATCH 5/6] test(flipper): instance reset is now out-of-box --- spec/rails_helper.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 8ccfede35..6ba94e8a8 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -121,10 +121,6 @@ RSpec.configure do |config| end end - config.before(:each, type: Proc.new { |type| type != :system }) do - Flipper.instance = Flipper.new(Flipper::Adapters::Memory.new) - end - config.include ActiveSupport::Testing::TimeHelpers config.include Shoulda::Matchers::ActiveRecord, type: :model config.include Shoulda::Matchers::ActiveModel, type: :model From ca17b356fc6abe05a9568ac26438a9600ca0d921 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Sun, 3 Mar 2024 00:13:55 +0100 Subject: [PATCH 6/6] chore(flipper): don't preload on /assets/* or /ping --- config/initializers/flipper.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index 4cbd0d1a4..3d3851c9e 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -56,5 +56,7 @@ Flipper.configure do |config| end Rails.application.configure do + # don't preload features for /assets/* but do for everything else + config.flipper.preload = -> (request) { !request.path.start_with?('/assets/', '/ping') } config.flipper.strict = Rails.env.development? end