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

This allows to enable/disable a feature flag for a given user, or a
given Flipper group (must be declared statically in the `flipper.rb`
initializer beforehand).

Signed-off-by: Rémy Coutable <remy@rymai.me>

8 files changed, 233 insertions, 33 deletions

diff --git a/app/models/concerns/flippable.rb b/app/models/concerns/flippable.rb
new file mode 100644
index 00000000000..341501e8250
--- /dev/null
+++ b/app/models/concerns/flippable.rb
@@ -0,0 +1,7 @@
+module Flippable
+  def flipper_id
+    return nil if new_record?
+
+    "#{self.class.name}:#{id}"
+  end
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6dd1b1415d6..bcce260ab08 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -11,6 +11,7 @@ class User < ActiveRecord::Base
   include CaseSensitivity
   include TokenAuthenticatable
   include IgnorableColumn
+  include Flippable
 
   DEFAULT_NOTIFICATION_LEVEL = :participating
 
diff --git a/changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml b/changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml
new file mode 100644
index 00000000000..8ead1b404f3
--- /dev/null
+++ b/changelogs/unreleased/34078-allow-to-enable-feature-flags-with-more-granularity.yml
@@ -0,0 +1,4 @@
+---
+title: Allow the feature flags to be enabled/disabled with more granularity
+merge_request:
+author:
diff --git a/doc/api/features.md b/doc/api/features.md
index 89b8d3ac948..0ca2e637614 100644
--- a/doc/api/features.md
+++ b/doc/api/features.md
@@ -58,6 +58,8 @@ POST /features/:name
 | --------- | ---- | -------- | ----------- |
 | `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 |
+| `flipper_group` | string | no | A Flipper group name |
+| `user` | string | no | A GitLab username |
 
 ```bash
 curl --data "value=30" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/new_library
diff --git a/lib/api/features.rb b/lib/api/features.rb
index cff0ba2ddff..4e10e36fce9 100644
--- a/lib/api/features.rb
+++ b/lib/api/features.rb
@@ -2,6 +2,29 @@ module API
   class Features < Grape::API
     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[:flipper_group]
+          Feature.group(params[:flipper_group])
+        elsif params[:user]
+          User.find_by_username(params[:user])
+        else
+          gate_value(params)
+        end
+      end
+    end
+
     resource :features do
       desc 'Get a list of all features' do
         success Entities::Feature
@@ -17,16 +40,21 @@ module API
       end
       params do
         requires :value, type: String, desc: '`true` or `false` to enable/disable, an integer for percentage of time'
+        optional :flipper_group, type: String, desc: 'A Flipper group name'
+        optional :user, type: String, desc: 'A GitLab username'
       end
       post ':name' do
         feature = Feature.get(params[:name])
+        target = gate_target(params)
+        value = gate_value(params)
 
-        if %w(0 false).include?(params[:value])
-          feature.disable
-        elsif params[:value] == 'true'
-          feature.enable
+        case value
+        when true
+          feature.enable(target)
+        when false
+          feature.disable(target)
         else
-          feature.enable_percentage_of_time(params[:value].to_i)
+          feature.enable_percentage_of_time(value)
         end
 
         present feature, with: Entities::Feature, current_user: current_user
diff --git a/lib/feature.rb b/lib/feature.rb
index d3d972564af..363f66ba60e 100644
--- a/lib/feature.rb
+++ b/lib/feature.rb
@@ -12,6 +12,8 @@ class Feature
   end
 
   class << self
+    delegate :group, to: :flipper
+
     def all
       flipper.features.to_a
     end
@@ -27,16 +29,24 @@ class Feature
       all.map(&:name).include?(feature.name)
     end
 
-    def enabled?(key)
-      get(key).enabled?
+    def enabled?(key, thing = nil)
+      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
 
-    def enable(key)
-      get(key).enable
+    def enable_group(key, group)
+      get(key).enable_group(group)
     end
 
-    def disable(key)
-      get(key).disable
+    def disable_group(key, group)
+      get(key).disable_group(group)
     end
 
     def flipper
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 8e895ec6634..05ba887c51f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -430,6 +430,20 @@ describe User, models: true do
     end
   end
 
+  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
+
   describe '#generate_password' do
     it "does not generate password by default" do
       user = create(:user, password: 'abcdefghe')
diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb
index f169e6661d1..0ee0749c7a1 100644
--- a/spec/requests/api/features_spec.rb
+++ b/spec/requests/api/features_spec.rb
@@ -4,6 +4,13 @@ describe API::Features do
   let(:user)  { create(:user) }
   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
     let(:expected_features) do
       [
@@ -16,6 +23,14 @@ describe API::Features do
           'name' => 'feature_2',
           'state' => 'off',
           'gates' => [{ 'key' => 'boolean', 'value' => false }]
+        },
+        {
+          'name' => 'feature_3',
+          'state' => 'conditional',
+          'gates' => [
+            { 'key' => 'boolean', 'value' => false },
+            { 'key' => 'groups', 'value' => ['perf_team'] }
+          ]
         }
       ]
     end
@@ -23,6 +38,7 @@ describe API::Features do
     before do
       Feature.get('feature_1').enable
       Feature.get('feature_2').disable
+      Feature.get('feature_3').enable Feature.group(:perf_team)
     end
 
     it 'returns a 401 for anonymous users' do
@@ -47,30 +63,70 @@ describe API::Features do
 
   describe 'POST /feature' do
     let(:feature_name) { 'my_feature' }
-    it 'returns a 401 for anonymous users' do
-      post api("/features/#{feature_name}")
 
-      expect(response).to have_http_status(401)
-    end
+    context 'when the feature does not exist' do
+      it 'returns a 401 for anonymous users' do
+        post api("/features/#{feature_name}")
 
-    it 'returns a 403 for users' do
-      post api("/features/#{feature_name}", user)
+        expect(response).to have_http_status(401)
+      end
 
-      expect(response).to have_http_status(403)
-    end
+      it 'returns a 403 for users' do
+        post api("/features/#{feature_name}", user)
 
-    it 'creates an enabled feature if passed true' do
-      post api("/features/#{feature_name}", admin), value: 'true'
+        expect(response).to have_http_status(403)
+      end
 
-      expect(response).to have_http_status(201)
-      expect(Feature.get(feature_name)).to be_enabled
-    end
+      context 'when passed value=true' do
+        it 'creates an enabled feature' do
+          post api("/features/#{feature_name}", admin), value: 'true'
 
-    it 'creates a feature with the given percentage if passed an integer' do
-      post api("/features/#{feature_name}", admin), value: '50'
+          expect(response).to have_http_status(201)
+          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 flipper_group=perf_team' do
+          post api("/features/#{feature_name}", admin), value: 'true', flipper_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(Feature.get(feature_name).percentage_of_time_value).to be(50)
+          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
+
+      it 'creates a feature with the given percentage if passed an integer' do
+        post api("/features/#{feature_name}", admin), value: '50'
+
+        expect(response).to have_http_status(201)
+        expect(json_response).to eq(
+          'name' => 'my_feature',
+          'state' => 'conditional',
+          'gates' => [
+            { 'key' => 'boolean', 'value' => false },
+            { 'key' => 'percentage_of_time', 'value' => 50 }
+          ])
+      end
     end
 
     context 'when the feature exists' do
@@ -80,11 +136,83 @@ describe API::Features do
         feature.disable # This also persists the feature on the DB
       end
 
-      it 'enables the feature if passed true' do
-        post api("/features/#{feature_name}", admin), value: 'true'
+      context 'when passed value=true' do
+        it 'enables the feature' do
+          post api("/features/#{feature_name}", admin), value: 'true'
 
-        expect(response).to have_http_status(201)
-        expect(feature).to be_enabled
+          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 flipper_group=perf_team' do
+          post api("/features/#{feature_name}", admin), value: 'true', flipper_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
+
+          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 flipper_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', flipper_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
 
       context 'with a pre-existing percentage value' do
@@ -96,7 +224,13 @@ describe API::Features do
           post api("/features/#{feature_name}", admin), value: '30'
 
           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