Merge branch 'ff-userid-only-strat' into 'master'

Enable Target Users Across All Feature Flag Environment Scopes

See merge request gitlab-org/gitlab-ee!15500

ee/app/models/operations/feature_flag_scope.rb

@@ -3,6 +3,7 @@
 module Operations
   class FeatureFlagScope < ApplicationRecord
     prepend HasEnvironmentScope
+    include Gitlab::Utils::StrongMemoize
 
     self.table_name = 'operations_feature_flag_scopes'
 
@@ -25,6 +26,12 @@ module Operations
     scope :enabled, -> { where(active: true) }
     scope :disabled, -> { where(active: false) }
 
+    def userwithid_strategy
+      strong_memoize(:userwithid_strategy) do
+        strategies.select { |s| s['name'] == FeatureFlagStrategiesValidator::STRATEGY_USERWITHID }
+      end
+    end
+
     def self.with_name_and_description
       joins(:feature_flag)
         .select(FeatureFlag.arel_table[:name], FeatureFlag.arel_table[:description])

ee/changelogs/unreleased/ff-userid-only-strat.yml

+---
+title: Enable target users across all feature flag environment scopes
+merge_request: 15500
+author:
+type: fixed

ee/lib/ee/api/entities.rb

@@ -705,8 +705,22 @@ module EE
       class UnleashFeature < Grape::Entity
         expose :name
         expose :description, unless: ->(feature) { feature.description.nil? }
-        expose :active, as: :enabled
-        expose :strategies
+        # The UI has a single field for user ids for whom the feature flag should be enabled across all scopes.
+        # Each scope is given a userWithId strategy with the list of user ids.
+        # However, the user can also directly toggle the active field of a scope.
+        # So if the user has entered user ids, and disabled the scope, we need to send an enabled scope with
+        # the list of user ids.
+        # See: https://gitlab.com/gitlab-org/gitlab-ee/issues/14011
+        expose :active, as: :enabled do |feature|
+          feature.active || feature.userwithid_strategy.present?
+        end
+        expose :strategies do |feature|
+          if !feature.active && feature.userwithid_strategy.present?
+            feature.userwithid_strategy
+          else
+            feature.strategies
+          end
+        end
       end
 
       class GitlabSubscription < Grape::Entity

ee/spec/requests/api/unleash_spec.rb

@@ -150,6 +150,8 @@ describe API::Unleash do
   %w(/feature_flags/unleash/:project_id/features /feature_flags/unleash/:project_id/client/features).each do |features_endpoint|
     describe "GET #{features_endpoint}" do
       let(:features_url) { features_endpoint.sub(':project_id', project_id.to_s) }
+      let(:client) { create(:operations_feature_flags_client, project: project) }
+      let(:feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true) }
 
       subject { get api(features_url), params: params, headers: headers }
 
@@ -157,7 +159,6 @@ describe API::Unleash do
       it_behaves_like 'support multiple environments'
 
       context 'with a list of feature flag' do
-        let(:client) { create(:operations_feature_flags_client, project: project) }
         let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" }}
         let!(:enable_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature1', active: true) }
         let!(:disabled_feature_flag) { create(:operations_feature_flag, project: project, name: 'feature2', active: false) }
@@ -169,6 +170,7 @@ describe API::Unleash do
           expect(json_response['version']).to eq(1)
           expect(json_response['features']).not_to be_empty
           expect(json_response['features'].map { |f| f['name'] }.sort).to eq(%w[feature1 feature2])
+          expect(json_response['features'].sort_by {|f| f['name'] }.map { |f| f['enabled'] }).to eq([true, false])
         end
 
         it 'matches json schema' do
@@ -180,8 +182,6 @@ describe API::Unleash do
       end
 
       it 'returns a feature flag strategy' do
-        client = create(:operations_feature_flags_client, project: project)
-        feature_flag = create(:operations_feature_flag, project: project, name: 'feature1', active: true)
         create(:operations_feature_flag_scope,
                feature_flag: feature_flag,
                environment_scope: 'sandbox',
@@ -193,6 +193,7 @@ describe API::Unleash do
         get api(features_url), headers: headers
 
         expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['features'].first['enabled']).to eq(true)
         strategies = json_response['features'].first['strategies']
         expect(strategies).to eq([{
           "name" => "gradualRolloutUserId",
@@ -204,21 +205,18 @@ describe API::Unleash do
       end
 
       it 'returns a default strategy for a scope' do
-        client = create(:operations_feature_flags_client, project: project)
-        feature_flag = create(:operations_feature_flag, project: project, name: 'feature1', active: true)
         create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'sandbox', active: true)
         headers = { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "sandbox" }
 
         get api(features_url), headers: headers
 
         expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['features'].first['enabled']).to eq(true)
         strategies = json_response['features'].first['strategies']
         expect(strategies).to eq([{ "name" => "default", "parameters" => {} }])
       end
 
       it 'returns multiple strategies for a feature flag' do
-        client = create(:operations_feature_flags_client, project: project)
-        feature_flag = create(:operations_feature_flag, project: project, name: 'feature1', active: true)
         create(:operations_feature_flag_scope,
                feature_flag: feature_flag,
                environment_scope: 'staging',
@@ -231,6 +229,7 @@ describe API::Unleash do
         get api(features_url), headers: headers
 
         expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['features'].first['enabled']).to eq(true)
         strategies = json_response['features'].first['strategies'].sort_by { |s| s['name'] }
         expect(strategies).to eq([{
           "name" => "gradualRolloutUserId",
@@ -245,6 +244,42 @@ describe API::Unleash do
           }
         }])
       end
+
+      context "with an inactive scope" do
+        let!(:scope) { create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production', active: false, strategies: strategies) }
+        let(:headers) { { "UNLEASH-INSTANCEID" => client.token, "UNLEASH-APPNAME" => "production" } }
+
+        context 'with userWithId strategy' do
+          let(:strategies) { [{ name: "default", parameters: {} }, { name: "userWithId", parameters: { userIds: "fred" } }] }
+
+          it 'returns an enabled feature with only the userWithId strategy' do
+            get api(features_url), headers: headers
+
+            expect(response).to have_gitlab_http_status(:ok)
+            feature_json = json_response['features'].first
+            expect(feature_json['enabled']).to eq(true)
+            expect(feature_json['strategies']).to eq([{
+              'name' => 'userWithId',
+              'parameters' => {
+                'userIds' => 'fred'
+              }
+            }])
+          end
+        end
+
+        context 'with default strategy' do
+          let(:strategies) { [{ name: "default", parameters: {} }] }
+
+          it 'returns a disabled feature that does not contain a userWithId strategy' do
+            get api(features_url), headers: headers
+
+            expect(response).to have_gitlab_http_status(:ok)
+            feature_json = json_response['features'].first
+            expect(feature_json['enabled']).to eq(false)
+            expect(feature_json['strategies']).to eq([{ 'name' => 'default', 'parameters' => {} }])
+          end
+        end
+      end
     end
   end