From 3aea946e7f590319ff7b85dbbd4eedfc20569deb Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 4 Sep 2018 13:34:37 -0500 Subject: [PATCH] add 'default_enabled' to feature flags This allows you to default a feature flag to 'on' when checking whether it's enabled/disabled. --- doc/development/feature_flags.md | 11 +++++++++-- lib/feature.rb | 12 ++++++++---- spec/lib/feature_spec.rb | 8 ++++++++ spec/support/helpers/stub_feature_flags.rb | 4 ++-- 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 702caacc74f..6f757f1ce7b 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -58,13 +58,20 @@ Features that are developed and are intended to be merged behind a feature flag should not include a changelog entry. The entry should be added in the merge request removing the feature flags. +In the rare case that you need the feature flag to be on automatically, use +`default_enabled: true` when checking: + +```ruby +Feature.enabled?(:feature_flag, project, default_enabled: true) +``` + ### Specs In the test environment `Feature.enabled?` is stubbed to always respond to `true`, so we make sure behavior under feature flag doesn't go untested in some non-specific contexts. - -If you need to test the feature flag in a different state, you need to stub it with: + +If you need to test the feature flag in a different state, you need to stub it with: ```ruby stub_feature_flags(my_feature_flag: false) diff --git a/lib/feature.rb b/lib/feature.rb index 24dbcb32fc0..15ce487ad4e 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -42,13 +42,17 @@ class Feature persisted_names.include?(feature.name.to_s) end - def enabled?(key, thing = nil) - get(key).enabled?(thing) + def enabled?(key, thing = nil, default_enabled: false) + feature = Feature.get(key) + return feature.enabled?(thing) unless default_enabled + + # If the feature has been set, always evaluate + Feature.persisted?(feature) ? feature.enabled?(thing) : true end - def disabled?(key, thing = nil) + def disabled?(key, thing = nil, default_enabled: false) # we need to make different method calls to make it easy to mock / define expectations in test mode - thing.nil? ? !enabled?(key) : !enabled?(key, thing) + thing.nil? ? !enabled?(key, default_enabled: default_enabled) : !enabled?(key, thing, default_enabled: default_enabled) end def enable(key, thing = true) diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 8bb5a843484..48c0ba8a653 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -119,6 +119,10 @@ describe Feature do expect(described_class.enabled?(:some_random_feature_flag)).to be_falsey end + it 'returns true for undefined feature with default_enabled' do + expect(described_class.enabled?(:some_random_feature_flag, default_enabled: true)).to be_truthy + end + it 'returns false for existing disabled feature in the database' do described_class.disable(:disabled_feature_flag) @@ -160,6 +164,10 @@ describe Feature do expect(described_class.disabled?(:some_random_feature_flag)).to be_truthy end + it 'returns false for undefined feature with default_enabled' do + expect(described_class.disabled?(:some_random_feature_flag, default_enabled: true)).to be_falsey + end + it 'returns true for existing disabled feature in the database' do described_class.disable(:disabled_feature_flag) diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb index c54a871b157..4061a8d1bc9 100644 --- a/spec/support/helpers/stub_feature_flags.rb +++ b/spec/support/helpers/stub_feature_flags.rb @@ -4,8 +4,8 @@ module StubFeatureFlags # @param [Hash] features where key is feature name and value is boolean whether enabled or not def stub_feature_flags(features) features.each do |feature_name, enabled| - allow(Feature).to receive(:enabled?).with(feature_name) { enabled } - allow(Feature).to receive(:enabled?).with(feature_name.to_s) { enabled } + allow(Feature).to receive(:enabled?).with(feature_name, any_args) { enabled } + allow(Feature).to receive(:enabled?).with(feature_name.to_s, any_args) { enabled } end end end