Merge branch 'ff-v2-controller-get-post' into 'master'

Support V2 Feature Flags GET and POST Controller Actions

See merge request gitlab-org/gitlab!27463

ee/app/controllers/projects/feature_flags_controller.rb

@@ -94,14 +94,23 @@ class Projects::FeatureFlagsController < Projects::ApplicationController
   protected
 
   def feature_flag
-    @feature_flag ||= project.operations_feature_flags.find(params[:id])
+    @feature_flag ||= if new_version_feature_flags_enabled?
+                        project.operations_feature_flags.find(params[:id])
+                      else
+                        project.operations_feature_flags.legacy_flag.find(params[:id])
+                      end
+  end
+
+  def new_version_feature_flags_enabled?
+    ::Feature.enabled?(:feature_flags_new_version, project)
   end
 
   def create_params
     params.require(:operations_feature_flag)
-      .permit(:name, :description, :active,
+      .permit(:name, :description, :active, :version,
               scopes_attributes: [:environment_scope, :active,
-                                  strategies: [:name, parameters: [:groupId, :percentage, :userIds]]])
+                                  strategies: [:name, parameters: [:groupId, :percentage, :userIds]]],
+             strategies_attributes: [:name, parameters: [:groupId, :percentage, :userIds], scopes_attributes: [:environment_scope]])
   end
 
   def update_params

ee/app/serializers/feature_flag_entity.rb

@@ -10,6 +10,7 @@ class FeatureFlagEntity < Grape::Entity
   expose :updated_at
   expose :name
   expose :description
+  expose :version
 
   expose :edit_path, if: -> (feature_flag, _) { can_update?(feature_flag) } do |feature_flag|
     edit_project_feature_flag_path(feature_flag.project, feature_flag)
@@ -27,6 +28,8 @@ class FeatureFlagEntity < Grape::Entity
     feature_flag.scopes.sort_by(&:id)
   end
 
+  expose :strategies, with: FeatureFlags::StrategyEntity
+
   private
 
   def can_update?(feature_flag)

ee/app/serializers/feature_flags/scope_entity.rb

+# frozen_string_literal: true
+
+module FeatureFlags
+  class ScopeEntity < Grape::Entity
+    expose :id
+    expose :environment_scope
+  end
+end

ee/app/serializers/feature_flags/strategy_entity.rb

+# frozen_string_literal: true
+
+module FeatureFlags
+  class StrategyEntity < Grape::Entity
+    expose :id
+    expose :name
+    expose :parameters
+    expose :scopes, with: FeatureFlags::ScopeEntity
+  end
+end

ee/app/services/feature_flags/create_service.rb

@@ -4,6 +4,8 @@ module FeatureFlags
   class CreateService < FeatureFlags::BaseService
     def execute
       return error('Access Denied', 403) unless can_create?
+      return error('Version is invalid', :bad_request) unless valid_version?
+      return error('New version feature flags are not enabled for this project', :bad_request) unless flag_version_enabled?
 
       ActiveRecord::Base.transaction do
         feature_flag = project.operations_feature_flags.new(params)
@@ -34,5 +36,17 @@ module FeatureFlags
     def can_create?
       Ability.allowed?(current_user, :create_feature_flag, project)
     end
+
+    def valid_version?
+      !params.key?(:version) || Operations::FeatureFlag.versions.key?(params[:version])
+    end
+
+    def flag_version_enabled?
+      params[:version] != 'new_version_flag' || new_version_feature_flags_enabled?
+    end
+
+    def new_version_feature_flags_enabled?
+      ::Feature.enabled?(:feature_flags_new_version, project)
+    end
   end
 end

ee/lib/api/feature_flags.rb

@@ -40,7 +40,7 @@ module API
         params do
           requires :name, type: String, desc: 'The name of feature flag'
           optional :description, type: String, desc: 'The description of the feature flag'
-          optional :version, type: String, desc: 'The version of the feature flag', values: Operations::FeatureFlag.versions.keys
+          optional :version, type: String, desc: 'The version of the feature flag'
           optional :scopes, type: Array do
             requires :environment_scope, type: String, desc: 'The environment scope of the scope'
             requires :active, type: Boolean, desc: 'Active/inactive of the scope'

ee/spec/controllers/projects/feature_flags_controller_spec.rb

@@ -206,6 +206,27 @@ describe Projects::FeatureFlagsController do
         expect(related_count).to be_within(5).of(2)
       end
     end
+
+    context 'with version 1 and 2 feature flags' do
+      let!(:new_version_feature_flag) do
+        create(:operations_feature_flag, :new_version_flag, project: project)
+      end
+
+      it 'returns all feature flags as json response' do
+        subject
+
+        expect(json_response['feature_flags'].count).to eq(3)
+      end
+
+      it 'returns only version 1 flags when new version flags are disabled' do
+        stub_feature_flags(feature_flags_new_version: false)
+
+        subject
+
+        expected = [feature_flag_active.name, feature_flag_inactive.name].sort
+        expect(json_response['feature_flags'].map { |f| f['name'] }.sort).to eq(expected)
+      end
+    end
   end
 
   describe 'GET new' do
@@ -233,11 +254,12 @@ describe Projects::FeatureFlagsController do
       }
     end
 
-    it 'returns all feature flags as json response' do
+    it 'returns the feature flag as json response' do
       subject
 
       expect(json_response['name']).to eq(feature_flag.name)
       expect(json_response['active']).to eq(feature_flag.active)
+      expect(json_response['version']).to eq('legacy_flag')
     end
 
     it 'matches json schema' do
@@ -307,6 +329,36 @@ describe Projects::FeatureFlagsController do
         end
       end
     end
+
+    context 'with a version 2 feature flag' do
+      let!(:new_version_feature_flag) do
+        create(:operations_feature_flag, :new_version_flag, project: project)
+      end
+
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          id: new_version_feature_flag.id
+        }
+      end
+
+      it 'returns the feature flag' do
+        subject
+
+        expect(json_response['name']).to eq(new_version_feature_flag.name)
+        expect(json_response['active']).to eq(new_version_feature_flag.active)
+        expect(json_response['version']).to eq('new_version_flag')
+      end
+
+      it 'returns a 404 when new version flags are disabled' do
+        stub_feature_flags(feature_flags_new_version: false)
+
+        subject
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
   end
 
   describe 'POST create.json' do
@@ -503,6 +555,169 @@ describe Projects::FeatureFlagsController do
         expect(default_strategies_json).to eq([{ "name" => "default", "parameters" => {} }])
       end
     end
+
+    context 'when creating a version 2 feature flag' do
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          operations_feature_flag: {
+            name: 'my_feature_flag',
+            active: true,
+            version: 'new_version_flag'
+          }
+        }
+      end
+
+      it 'creates a new feature flag' do
+        subject
+
+        expect(json_response['name']).to eq('my_feature_flag')
+        expect(json_response['active']).to be_truthy
+        expect(json_response['version']).to eq('new_version_flag')
+      end
+    end
+
+    context 'when creating a version 2 feature flag with strategies and scopes' do
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          operations_feature_flag: {
+            name: 'my_feature_flag',
+            active: true,
+            version: 'new_version_flag',
+            strategies_attributes: [{
+              name: 'userWithId',
+              parameters: { userIds: 'user1' },
+              scopes_attributes: [{ environment_scope: '*' }]
+            }]
+          }
+        }
+      end
+
+      it 'creates a new feature flag with the strategies and scopes' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['name']).to eq('my_feature_flag')
+        expect(json_response['active']).to eq(true)
+        expect(json_response['strategies'].count).to eq(1)
+
+        strategy_json = json_response['strategies'].first
+        expect(strategy_json).to have_key('id')
+        expect(strategy_json['name']).to eq('userWithId')
+        expect(strategy_json['parameters']).to eq({ 'userIds' => 'user1' })
+        expect(strategy_json['scopes'].count).to eq(1)
+
+        scope_json = strategy_json['scopes'].first
+        expect(scope_json).to have_key('id')
+        expect(scope_json['environment_scope']).to eq('*')
+      end
+    end
+
+    context 'when creating a version 2 feature flag with a gradualRolloutUserId strategy' do
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          operations_feature_flag: {
+            name: 'my_feature_flag',
+            active: true,
+            version: 'new_version_flag',
+            strategies_attributes: [{
+              name: 'gradualRolloutUserId',
+              parameters: { groupId: 'default', percentage: '15' },
+              scopes_attributes: [{ environment_scope: 'production' }]
+            }]
+          }
+        }
+      end
+
+      it 'creates the new strategy' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+
+        strategy_json = json_response['strategies'].first
+        expect(strategy_json['name']).to eq('gradualRolloutUserId')
+        expect(strategy_json['parameters']).to eq({ 'groupId' => 'default', 'percentage' => '15' })
+        expect(strategy_json['scopes'].count).to eq(1)
+
+        scope_json = strategy_json['scopes'].first
+        expect(scope_json['environment_scope']).to eq('production')
+      end
+    end
+
+    context 'when version parameter is invalid' do
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project,
+          operations_feature_flag: {
+            name: 'my_feature_flag',
+            active: true,
+            version: 'bad_version'
+          }
+        }
+      end
+
+      it 'returns a 400' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response).to eq({ 'message' => 'Version is invalid' })
+        expect(Operations::FeatureFlag.count).to eq(0)
+      end
+    end
+
+    context 'when version 2 flags are disabled' do
+      context 'and attempting to create a version 2 flag' do
+        let(:params) do
+          {
+            namespace_id: project.namespace,
+            project_id: project,
+            operations_feature_flag: {
+              name: 'my_feature_flag',
+              active: true,
+              version: 'new_version_flag'
+            }
+          }
+        end
+
+        it 'returns a 400' do
+          stub_feature_flags(feature_flags_new_version: false)
+
+          subject
+
+          expect(response).to have_gitlab_http_status(:bad_request)
+          expect(Operations::FeatureFlag.count).to eq(0)
+        end
+      end
+
+      context 'and attempting to create a version 1 flag' do
+        let(:params) do
+          {
+            namespace_id: project.namespace,
+            project_id: project,
+            operations_feature_flag: {
+              name: 'my_feature_flag',
+              active: true
+            }
+          }
+        end
+
+        it 'creates the flag' do
+          stub_feature_flags(feature_flags_new_version: false)
+
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(Operations::FeatureFlag.count).to eq(1)
+          expect(json_response['version']).to eq('legacy_flag')
+        end
+      end
+    end
   end
 
   describe 'DELETE destroy.json' do

ee/spec/fixtures/api/schemas/feature_flag.json

@@ -7,6 +7,7 @@
   "properties" : {
     "id": { "type": "integer" },
     "iid": { "type": ["integer", "null"] },
+    "version": { "type": "string" },
     "created_at": { "type": "date" },
     "updated_at": { "type": "date" },
     "name": { "type": "string" },
@@ -15,7 +16,8 @@
     "edit_path": { "type": ["string", "null"] },
     "update_path": { "type": ["string", "null"] },
     "destroy_path": { "type": ["string", "null"] },
-    "scopes": { "type": "array", "items": { "$ref": "feature_flag_scope.json" } }
+    "scopes": { "type": "array", "items": { "$ref": "feature_flag_scope.json" } },
+    "strategies": { "type": "array", "items": { "$ref": "feature_flag_strategy.json" } }
   },
   "additionalProperties": false
 }

ee/spec/requests/api/feature_flags_spec.rb

@@ -452,6 +452,7 @@ describe API::FeatureFlags do
         post api("/projects/#{project.id}/feature_flags", user), params: params
 
         expect(response).to have_gitlab_http_status(:bad_request)
+        expect(json_response).to eq({ 'message' => 'Version is invalid' })
       end
     end
   end