Merge branch 'introduce-feature-flag-api-enable-disable' into 'master'

Support Enable/Disable operations in Feature Flag API

See merge request gitlab-org/gitlab!18368

changelogs/unreleased/introduce-feature-flag-api-enable-disable.yml

+---
+title: Support Enable/Disable operations in Feature Flag API
+merge_request: 18368
+author:
+type: added

ee/app/services/feature_flags/base_service.rb

@@ -2,6 +2,8 @@
 
 module FeatureFlags
   class BaseService < ::BaseService
+    include Gitlab::Utils::StrongMemoize
+
     AUDITABLE_ATTRIBUTES = %w(name description).freeze
 
     protected
@@ -37,5 +39,17 @@ module FeatureFlags
       "and set it as <strong>#{scope.active ? "active" : "inactive"}</strong> "\
       "with strategies <strong>#{scope.strategies}</strong>."
     end
+
+    def feature_flag_by_name
+      strong_memoize(:feature_flag_by_name) do
+        project.operations_feature_flags.find_by_name(params[:name])
+      end
+    end
+
+    def feature_flag_scope_by_environment_scope
+      strong_memoize(:feature_flag_scope_by_environment_scope) do
+        feature_flag_by_name.scopes.find_by_environment_scope(params[:environment_scope])
+      end
+    end
   end
 end

ee/app/services/feature_flags/disable_service.rb

+# frozen_string_literal: true
+
+module FeatureFlags
+  class DisableService < BaseService
+    def execute
+      return error('Feature Flag not found', 404) unless feature_flag_by_name
+      return error('Feature Flag Scope not found', 404) unless feature_flag_scope_by_environment_scope
+      return error('Strategy not found', 404) unless strategy_exist_in_persisted_data?
+
+      ::FeatureFlags::UpdateService
+        .new(project, current_user, update_params)
+        .execute(feature_flag_by_name)
+    end
+
+    private
+
+    def update_params
+      if remaining_strategies.empty?
+        params_to_destroy_scope
+      else
+        params_to_update_scope
+      end
+    end
+
+    def remaining_strategies
+      strong_memoize(:remaining_strategies) do
+        feature_flag_scope_by_environment_scope.strategies.reject do |strategy|
+          strategy['name'] == params[:strategy]['name'] &&
+            strategy['parameters'] == params[:strategy]['parameters']
+        end
+      end
+    end
+
+    def strategy_exist_in_persisted_data?
+      feature_flag_scope_by_environment_scope.strategies != remaining_strategies
+    end
+
+    def params_to_destroy_scope
+      { scopes_attributes: [{ id: feature_flag_scope_by_environment_scope.id, _destroy: true }] }
+    end
+
+    def params_to_update_scope
+      { scopes_attributes: [{ id: feature_flag_scope_by_environment_scope.id, strategies: remaining_strategies }] }
+    end
+  end
+end

ee/app/services/feature_flags/enable_service.rb

+# frozen_string_literal: true
+
+module FeatureFlags
+  class EnableService < BaseService
+    def execute
+      if feature_flag_by_name
+        update_feature_flag
+      else
+        create_feature_flag
+      end
+    end
+
+    private
+
+    def create_feature_flag
+      ::FeatureFlags::CreateService
+        .new(project, current_user, create_params)
+        .execute
+    end
+
+    def update_feature_flag
+      ::FeatureFlags::UpdateService
+        .new(project, current_user, update_params)
+        .execute(feature_flag_by_name)
+    end
+
+    def create_params
+      if params[:environment_scope] == '*'
+        params_to_create_flag_with_default_scope
+      else
+        params_to_create_flag_with_additional_scope
+      end
+    end
+
+    def update_params
+      if feature_flag_scope_by_environment_scope
+        params_to_update_scope
+      else
+        params_to_create_scope
+      end
+    end
+
+    def params_to_create_flag_with_default_scope
+      {
+        name: params[:name],
+        scopes_attributes: [
+          {
+            active: true,
+            environment_scope: '*',
+            strategies: [params[:strategy]]
+          }
+        ]
+      }
+    end
+
+    def params_to_create_flag_with_additional_scope
+      {
+        name: params[:name],
+        scopes_attributes: [
+          {
+            active: false,
+            environment_scope: '*'
+          },
+          {
+            active: true,
+            environment_scope: params[:environment_scope],
+            strategies: [params[:strategy]]
+          }
+        ]
+      }
+    end
+
+    def params_to_create_scope
+      {
+        scopes_attributes: [{
+          active: true,
+          environment_scope: params[:environment_scope],
+          strategies: [params[:strategy]]
+        }]
+      }
+    end
+
+    def params_to_update_scope
+      {
+        scopes_attributes: [{
+          id: feature_flag_scope_by_environment_scope.id,
+          active: true,
+          strategies: feature_flag_scope_by_environment_scope.strategies | [params[:strategy]]
+        }]
+      }
+    end
+  end
+end

ee/lib/api/feature_flags.rb

@@ -76,6 +76,44 @@ module API
           present feature_flag, with: EE::API::Entities::FeatureFlag
         end
 
+        desc 'Enable a strategy for a feature flag on an environment' do
+          success EE::API::Entities::FeatureFlag
+        end
+        params do
+          requires :environment_scope, type: String, desc: 'The environment scope of the feature flag'
+          requires :strategy,          type: JSON,   desc: 'The strategy to be enabled on the scope'
+        end
+        post :enable do
+          result = ::FeatureFlags::EnableService
+            .new(user_project, current_user, params).execute
+
+          if result[:status] == :success
+            status :ok
+            present result[:feature_flag], with: EE::API::Entities::FeatureFlag
+          else
+            render_api_error!(result[:message], result[:http_status])
+          end
+        end
+
+        desc 'Disable a strategy for a feature flag on an environment' do
+          success EE::API::Entities::FeatureFlag
+        end
+        params do
+          requires :environment_scope, type: String, desc: 'The environment scope of the feature flag'
+          requires :strategy,          type: JSON,   desc: 'The strategy to be disabled on the scope'
+        end
+        post :disable do
+          result = ::FeatureFlags::DisableService
+            .new(user_project, current_user, params).execute
+
+          if result[:status] == :success
+            status :ok
+            present result[:feature_flag], with: EE::API::Entities::FeatureFlag
+          else
+            render_api_error!(result[:message], result[:http_status])
+          end
+        end
+
         desc 'Delete a feature flag' do
           success EE::API::Entities::FeatureFlag
         end

ee/spec/requests/api/feature_flags_spec.rb

@@ -186,6 +186,159 @@ describe API::FeatureFlags do
     end
   end
 
+  describe 'POST /projects/:id/feature_flags/:name/enable' do
+    subject do
+      post api("/projects/#{project.id}/feature_flags/#{params[:name]}/enable", user),
+           params: params
+    end
+
+    let(:params) do
+      {
+        name: 'awesome-feature',
+        environment_scope: 'production',
+        strategy: { name: 'userWithId', parameters: { userIds: 'Project:1' } }.to_json
+      }
+    end
+
+    context 'when feature flag does not exist yet' do
+      it 'creates a new feature flag with the specified scope and strategy' do
+        subject
+
+        feature_flag = project.operations_feature_flags.last
+        scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
+        expect(feature_flag.name).to eq(params[:name])
+        expect(scope.strategies).to eq([JSON.parse(params[:strategy])])
+      end
+
+      it_behaves_like 'check user permission'
+    end
+
+    context 'when feature flag exists already' do
+      let!(:feature_flag) { create_flag(project, params[:name]) }
+
+      context 'when feature flag scope does not exist yet' do
+        it 'creates a new scope with the specified strategy' do
+          subject
+
+          scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(scope.strategies).to eq([JSON.parse(params[:strategy])])
+        end
+
+        it_behaves_like 'check user permission'
+      end
+
+      context 'when feature flag scope exists already' do
+        let(:defined_strategy) { { name: 'userWithId', parameters: { userIds: 'Project:2' } } }
+
+        before do
+          create_scope(feature_flag, params[:environment_scope], true, [defined_strategy])
+        end
+
+        it 'adds an additional strategy to the scope' do
+          subject
+
+          scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(scope.strategies).to eq([defined_strategy.deep_stringify_keys, JSON.parse(params[:strategy])])
+        end
+
+        context 'when the specified strategy exists already' do
+          let(:defined_strategy) { JSON.parse(params[:strategy]) }
+
+          it 'does not add a duplicate strategy' do
+            subject
+
+            scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+            strategy_count = scope.strategies.select { |strategy| strategy['name'] == 'userWithId' }.count
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(strategy_count).to eq(1)
+          end
+        end
+      end
+    end
+  end
+
+  describe 'POST /projects/:id/feature_flags/:name/disable' do
+    subject do
+      post api("/projects/#{project.id}/feature_flags/#{params[:name]}/disable", user),
+           params: params
+    end
+
+    let(:params) do
+      {
+        name: 'awesome-feature',
+        environment_scope: 'production',
+        strategy: { name: 'userWithId', parameters: { userIds: 'Project:1' } }.to_json
+      }
+    end
+
+    context 'when feature flag does not exist yet' do
+      it_behaves_like 'not found'
+    end
+
+    context 'when feature flag exists already' do
+      let!(:feature_flag) { create_flag(project, params[:name]) }
+
+      context 'when feature flag scope does not exist yet' do
+        it_behaves_like 'not found'
+      end
+
+      context 'when feature flag scope exists already and has the specified strategy' do
+        let(:defined_strategies) do
+          [
+            { name: 'userWithId', parameters: { userIds: 'Project:1' } },
+            { name: 'userWithId', parameters: { userIds: 'Project:2' } }
+          ]
+        end
+
+        before do
+          create_scope(feature_flag, params[:environment_scope], true, defined_strategies)
+        end
+
+        it 'removes the strategy from the scope' do
+          subject
+
+          scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to match_response_schema('public_api/v4/feature_flag', dir: 'ee')
+          expect(scope.strategies)
+            .to eq([{ name: 'userWithId', parameters: { userIds: 'Project:2' } }.deep_stringify_keys])
+        end
+
+        it_behaves_like 'check user permission'
+
+        context 'when strategies become empty array after the removal' do
+          let(:defined_strategies) do
+            [{ name: 'userWithId', parameters: { userIds: 'Project:1' } }]
+          end
+
+          it 'destroys the scope' do
+            subject
+
+            scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+            expect(response).to have_gitlab_http_status(:ok)
+            expect(scope).to be_nil
+          end
+
+          it_behaves_like 'check user permission'
+        end
+      end
+
+      context 'when scope exists already but cannot find the corresponding strategy' do
+        let(:defined_strategy) { { name: 'userWithId', parameters: { userIds: 'Project:2' } } }
+
+        before do
+          create_scope(feature_flag, params[:environment_scope], true, [defined_strategy])
+        end
+
+        it_behaves_like 'not found'
+      end
+    end
+  end
+
   describe 'DELETE /projects/:id/feature_flags/:name' do
     subject do
       delete api("/projects/#{project.id}/feature_flags/#{feature_flag.name}", user),

ee/spec/services/feature_flags/disable_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe FeatureFlags::DisableService do
+  include FeatureFlagHelpers
+
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:service) { described_class.new(project, user, params) }
+  let(:params) { {} }
+
+  before do
+    stub_licensed_features(feature_flags: true)
+    project.add_developer(user)
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'with params to disable default strategy on prd scope' do
+      let(:params) do
+        {
+          name: 'awesome',
+          environment_scope: 'prd',
+          strategy: { name: 'userWithId', parameters: { 'userIds': 'User:1' } }.deep_stringify_keys
+        }
+      end
+
+      context 'when there is a persisted feature flag' do
+        let!(:feature_flag) { create_flag(project, params[:name]) }
+
+        context 'when there is a persisted scope' do
+          let!(:scope) do
+            create_scope(feature_flag, params[:environment_scope], true, strategies)
+          end
+
+          context 'when there is a persisted strategy' do
+            let(:strategies) do
+              [
+                { name: 'userWithId', parameters: { 'userIds': 'User:1' } }.deep_stringify_keys,
+                { name: 'userWithId', parameters: { 'userIds': 'User:2' } }.deep_stringify_keys
+              ]
+            end
+
+            it 'deletes the specified strategy' do
+              subject
+
+              scope.reload
+              expect(scope.strategies.count).to eq(1)
+              expect(scope.strategies).not_to include(params[:strategy])
+            end
+
+            context 'when strategies will be empty' do
+              let(:strategies) { [params[:strategy]] }
+
+              it 'deletes the persisted scope' do
+                subject
+
+                expect(feature_flag.scopes.exists?(environment_scope: params[:environment_scope]))
+                  .to eq(false)
+              end
+            end
+          end
+
+          context 'when there is no persisted strategy' do
+            let(:strategies) { [{ name: 'default', parameters: {} }] }
+
+            it 'returns error' do
+              expect(subject[:status]).to eq(:error)
+              expect(subject[:message]).to include('Strategy not found')
+            end
+          end
+        end
+
+        context 'when there is no persisted scope' do
+          it 'returns error' do
+            expect(subject[:status]).to eq(:error)
+            expect(subject[:message]).to include('Feature Flag Scope not found')
+          end
+        end
+      end
+
+      context 'when there is no persisted feature flag' do
+        it 'returns error' do
+          expect(subject[:status]).to eq(:error)
+          expect(subject[:message]).to include('Feature Flag not found')
+        end
+      end
+    end
+  end
+end

ee/spec/services/feature_flags/enable_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe FeatureFlags::EnableService do
+  include FeatureFlagHelpers
+
+  let_it_be(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let(:service) { described_class.new(project, user, params) }
+  let(:params) { {} }
+
+  before do
+    stub_licensed_features(feature_flags: true)
+    project.add_developer(user)
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    context 'with params to enable default strategy on prd scope' do
+      let(:params) do
+        {
+          name: 'awesome',
+          environment_scope: 'prd',
+          strategy: { name: 'default', parameters: {} }.stringify_keys
+        }
+      end
+
+      context 'when there is no persisted feature flag' do
+        it 'creates a new feature flag with scope' do
+          feature_flag = subject[:feature_flag]
+          scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+          expect(subject[:status]).to eq(:success)
+          expect(feature_flag.name).to eq(params[:name])
+          expect(feature_flag.default_scope).not_to be_active
+          expect(scope).to be_active
+          expect(scope.strategies).to include(params[:strategy])
+        end
+
+        context 'when params include default scope' do
+          let(:params) do
+            {
+              name: 'awesome',
+              environment_scope: '*',
+              strategy: { name: 'userWithId', parameters: { 'userIds': 'abc' } }.deep_stringify_keys
+            }
+          end
+
+          it 'create a new feature flag with an active default scope with the specified strategy' do
+            feature_flag = subject[:feature_flag]
+            expect(subject[:status]).to eq(:success)
+            expect(feature_flag.default_scope).to be_active
+            expect(feature_flag.default_scope.strategies).to include(params[:strategy])
+          end
+        end
+      end
+
+      context 'when there is a persisted feature flag' do
+        let!(:feature_flag) { create_flag(project, params[:name]) }
+
+        context 'when there is no persisted scope' do
+          it 'creates a new scope for the persisted feature flag' do
+            feature_flag = subject[:feature_flag]
+            scope = feature_flag.scopes.find_by_environment_scope(params[:environment_scope])
+            expect(subject[:status]).to eq(:success)
+            expect(feature_flag.name).to eq(params[:name])
+            expect(scope).to be_active
+            expect(scope.strategies).to include(params[:strategy])
+          end
+        end
+
+        context 'when there is a persisted scope' do
+          let!(:feature_flag_scope) do
+            create_scope(feature_flag, params[:environment_scope], active, strategies)
+          end
+
+          let(:active) { true }
+
+          context 'when the persisted scope does not have the specified strategy yet' do
+            let(:strategies) { [{ name: 'userWithId', parameters: { 'userIds': 'abc' } }] }
+
+            it 'adds the specified strategy to the scope' do
+              subject
+
+              feature_flag_scope.reload
+              expect(feature_flag_scope.strategies).to include(params[:strategy])
+            end
+
+            context 'when the persisted scope is inactive' do
+              let(:active) { false }
+
+              it 'reactivates the scope' do
+                expect { subject }
+                  .to change { feature_flag_scope.reload.active }.from(false).to(true)
+              end
+            end
+          end
+
+          context 'when the persisted scope has the specified strategy already' do
+            let(:strategies) { [params[:strategy]] }
+
+            it 'does not add a duplicated strategy to the scope' do
+              expect { subject }
+                .not_to change { feature_flag_scope.reload.strategies.count }
+            end
+          end
+        end
+      end
+    end
+
+    context 'when strategy is not specified in params' do
+      let(:params) do
+        {
+          name: 'awesome',
+          environment_scope: 'prd'
+        }
+      end
+
+      it 'returns error' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to include('Scopes strategies must be an array of strategy hashes')
+      end
+    end
+
+    context 'when environment scope is not specified in params' do
+      let(:params) do
+        {
+          name: 'awesome',
+          strategy: { name: 'default', parameters: {} }.stringify_keys
+        }
+      end
+
+      it 'returns error' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to include("Scopes environment scope can't be blank")
+      end
+    end
+
+    context 'when name is not specified in params' do
+      let(:params) do
+        {
+          environment_scope: 'prd',
+          strategy: { name: 'default', parameters: {} }.stringify_keys
+        }
+      end
+
+      it 'returns error' do
+        expect(subject[:status]).to eq(:error)
+        expect(subject[:message]).to include("Name can't be blank")
+      end
+    end
+  end
+end