Commit 066a6c8b authored by Robert Speicher's avatar Robert Speicher

Merge branch '34078-allow-to-enable-feature-flags-with-more-granularity' into 'master'

Allow the feature flags to be enabled/disabled with more granularity

Closes #34078

See merge request !12357
parents 5a983ac4 289fae78
module FeatureGate
def flipper_id
return nil if new_record?
"#{self.class.name}:#{id}"
end
end
...@@ -11,6 +11,7 @@ class User < ActiveRecord::Base ...@@ -11,6 +11,7 @@ class User < ActiveRecord::Base
include CaseSensitivity include CaseSensitivity
include TokenAuthenticatable include TokenAuthenticatable
include IgnorableColumn include IgnorableColumn
include FeatureGate
DEFAULT_NOTIFICATION_LEVEL = :participating DEFAULT_NOTIFICATION_LEVEL = :participating
......
---
title: Allow the feature flags to be enabled/disabled with more granularity
merge_request: 12357
author:
...@@ -58,6 +58,10 @@ POST /features/:name ...@@ -58,6 +58,10 @@ POST /features/:name
| --------- | ---- | -------- | ----------- | | --------- | ---- | -------- | ----------- |
| `name` | string | yes | Name of the feature to create or update | | `name` | string | yes | Name of the feature to create or update |
| `value` | integer/string | yes | `true` or `false` to enable/disable, or an integer for percentage of time | | `value` | integer/string | yes | `true` or `false` to enable/disable, or an integer for percentage of time |
| `feature_group` | string | no | A Feature group name |
| `user` | string | no | A GitLab username |
Note that `feature_group` and `user` are mutually exclusive.
```bash ```bash
curl --data "value=30" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/new_library curl --data "value=30" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/new_library
......
...@@ -2,6 +2,29 @@ module API ...@@ -2,6 +2,29 @@ module API
class Features < Grape::API class Features < Grape::API
before { authenticated_as_admin! } before { authenticated_as_admin! }
helpers do
def gate_value(params)
case params[:value]
when 'true'
true
when '0', 'false'
false
else
params[:value].to_i
end
end
def gate_target(params)
if params[:feature_group]
Feature.group(params[:feature_group])
elsif params[:user]
User.find_by_username(params[:user])
else
gate_value(params)
end
end
end
resource :features do resource :features do
desc 'Get a list of all features' do desc 'Get a list of all features' do
success Entities::Feature success Entities::Feature
...@@ -17,16 +40,22 @@ module API ...@@ -17,16 +40,22 @@ module API
end end
params do params do
requires :value, type: String, desc: '`true` or `false` to enable/disable, an integer for percentage of time' requires :value, type: String, desc: '`true` or `false` to enable/disable, an integer for percentage of time'
optional :feature_group, type: String, desc: 'A Feature group name'
optional :user, type: String, desc: 'A GitLab username'
mutually_exclusive :feature_group, :user
end end
post ':name' do post ':name' do
feature = Feature.get(params[:name]) feature = Feature.get(params[:name])
target = gate_target(params)
value = gate_value(params)
if %w(0 false).include?(params[:value]) case value
feature.disable when true
elsif params[:value] == 'true' feature.enable(target)
feature.enable when false
feature.disable(target)
else else
feature.enable_percentage_of_time(params[:value].to_i) feature.enable_percentage_of_time(value)
end end
present feature, with: Entities::Feature, current_user: current_user present feature, with: Entities::Feature, current_user: current_user
......
...@@ -12,6 +12,8 @@ class Feature ...@@ -12,6 +12,8 @@ class Feature
end end
class << self class << self
delegate :group, to: :flipper
def all def all
flipper.features.to_a flipper.features.to_a
end end
...@@ -27,16 +29,24 @@ class Feature ...@@ -27,16 +29,24 @@ class Feature
all.map(&:name).include?(feature.name) all.map(&:name).include?(feature.name)
end end
def enabled?(key) def enabled?(key, thing = nil)
get(key).enabled? get(key).enabled?(thing)
end
def enable(key, thing = true)
get(key).enable(thing)
end
def disable(key, thing = false)
get(key).disable(thing)
end end
def enable(key) def enable_group(key, group)
get(key).enable get(key).enable_group(group)
end end
def disable(key) def disable_group(key, group)
get(key).disable get(key).disable_group(group)
end end
def flipper def flipper
......
require 'spec_helper'
describe FeatureGate do
describe 'User' do
describe '#flipper_id' do
context 'when user is not persisted' do
let(:user) { build(:user) }
it { expect(user.flipper_id).to be_nil }
end
context 'when user is persisted' do
let(:user) { create(:user) }
it { expect(user.flipper_id).to eq "User:#{user.id}" }
end
end
end
end
...@@ -4,6 +4,13 @@ describe API::Features do ...@@ -4,6 +4,13 @@ describe API::Features do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:admin) { create(:admin) } let(:admin) { create(:admin) }
before do
Flipper.unregister_groups
Flipper.register(:perf_team) do |actor|
actor.respond_to?(:admin) && actor.admin?
end
end
describe 'GET /features' do describe 'GET /features' do
let(:expected_features) do let(:expected_features) do
[ [
...@@ -16,6 +23,14 @@ describe API::Features do ...@@ -16,6 +23,14 @@ describe API::Features do
'name' => 'feature_2', 'name' => 'feature_2',
'state' => 'off', 'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }] 'gates' => [{ 'key' => 'boolean', 'value' => false }]
},
{
'name' => 'feature_3',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'groups', 'value' => ['perf_team'] }
]
} }
] ]
end end
...@@ -23,6 +38,7 @@ describe API::Features do ...@@ -23,6 +38,7 @@ describe API::Features do
before do before do
Feature.get('feature_1').enable Feature.get('feature_1').enable
Feature.get('feature_2').disable Feature.get('feature_2').disable
Feature.get('feature_3').enable Feature.group(:perf_team)
end end
it 'returns a 401 for anonymous users' do it 'returns a 401 for anonymous users' do
...@@ -47,6 +63,8 @@ describe API::Features do ...@@ -47,6 +63,8 @@ describe API::Features do
describe 'POST /feature' do describe 'POST /feature' do
let(:feature_name) { 'my_feature' } let(:feature_name) { 'my_feature' }
context 'when the feature does not exist' do
it 'returns a 401 for anonymous users' do it 'returns a 401 for anonymous users' do
post api("/features/#{feature_name}") post api("/features/#{feature_name}")
...@@ -59,18 +77,56 @@ describe API::Features do ...@@ -59,18 +77,56 @@ describe API::Features do
expect(response).to have_http_status(403) expect(response).to have_http_status(403)
end end
it 'creates an enabled feature if passed true' do context 'when passed value=true' do
it 'creates an enabled feature' do
post api("/features/#{feature_name}", admin), value: 'true' post api("/features/#{feature_name}", admin), value: 'true'
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(Feature.get(feature_name)).to be_enabled expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'on',
'gates' => [{ 'key' => 'boolean', 'value' => true }])
end
it 'creates an enabled feature for the given Flipper group when passed feature_group=perf_team' do
post api("/features/#{feature_name}", admin), value: 'true', feature_group: 'perf_team'
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'groups', 'value' => ['perf_team'] }
])
end
it 'creates an enabled feature for the given user when passed user=username' do
post api("/features/#{feature_name}", admin), value: 'true', user: user.username
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'actors', 'value' => ["User:#{user.id}"] }
])
end
end end
it 'creates a feature with the given percentage if passed an integer' do it 'creates a feature with the given percentage if passed an integer' do
post api("/features/#{feature_name}", admin), value: '50' post api("/features/#{feature_name}", admin), value: '50'
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(Feature.get(feature_name).percentage_of_time_value).to be(50) expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'percentage_of_time', 'value' => 50 }
])
end
end end
context 'when the feature exists' do context 'when the feature exists' do
...@@ -80,11 +136,83 @@ describe API::Features do ...@@ -80,11 +136,83 @@ describe API::Features do
feature.disable # This also persists the feature on the DB feature.disable # This also persists the feature on the DB
end end
it 'enables the feature if passed true' do context 'when passed value=true' do
it 'enables the feature' do
post api("/features/#{feature_name}", admin), value: 'true' post api("/features/#{feature_name}", admin), value: 'true'
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'on',
'gates' => [{ 'key' => 'boolean', 'value' => true }])
end
it 'enables the feature for the given Flipper group when passed feature_group=perf_team' do
post api("/features/#{feature_name}", admin), value: 'true', feature_group: 'perf_team'
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'groups', 'value' => ['perf_team'] }
])
end
it 'enables the feature for the given user when passed user=username' do
post api("/features/#{feature_name}", admin), value: 'true', user: user.username
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'actors', 'value' => ["User:#{user.id}"] }
])
end
end
context 'when feature is enabled and value=false is passed' do
it 'disables the feature' do
feature.enable
expect(feature).to be_enabled expect(feature).to be_enabled
post api("/features/#{feature_name}", admin), value: 'false'
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }])
end
it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do
feature.enable(Feature.group(:perf_team))
expect(Feature.get(feature_name).enabled?(admin)).to be_truthy
post api("/features/#{feature_name}", admin), value: 'false', feature_group: 'perf_team'
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }])
end
it 'disables the feature for the given user when passed user=username' do
feature.enable(user)
expect(Feature.get(feature_name).enabled?(user)).to be_truthy
post api("/features/#{feature_name}", admin), value: 'false', user: user.username
expect(response).to have_http_status(201)
expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'off',
'gates' => [{ 'key' => 'boolean', 'value' => false }])
end
end end
context 'with a pre-existing percentage value' do context 'with a pre-existing percentage value' do
...@@ -96,7 +224,13 @@ describe API::Features do ...@@ -96,7 +224,13 @@ describe API::Features do
post api("/features/#{feature_name}", admin), value: '30' post api("/features/#{feature_name}", admin), value: '30'
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(Feature.get(feature_name).percentage_of_time_value).to be(30) expect(json_response).to eq(
'name' => 'my_feature',
'state' => 'conditional',
'gates' => [
{ 'key' => 'boolean', 'value' => false },
{ 'key' => 'percentage_of_time', 'value' => 30 }
])
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment