diff --git a/ee/app/controllers/projects/feature_flags_controller.rb b/ee/app/controllers/projects/feature_flags_controller.rb index 8becfa34d3b1d1dbdecb3ee4c3fd7724dfcb0dd5..e60b3ceda5b54dd37809cb364970f751c24655e7 100644 --- a/ee/app/controllers/projects/feature_flags_controller.rb +++ b/ee/app/controllers/projects/feature_flags_controller.rb @@ -111,7 +111,7 @@ def create_params .permit(:name, :description, :active, :version, scopes_attributes: [:environment_scope, :active, strategies: [:name, parameters: [:groupId, :percentage, :userIds]]], - strategies_attributes: [:name, parameters: [:groupId, :percentage, :userIds], scopes_attributes: [:environment_scope]]) + strategies_attributes: [:name, :user_list_id, parameters: [:groupId, :percentage, :userIds], scopes_attributes: [:environment_scope]]) end def update_params @@ -119,7 +119,8 @@ def update_params .permit(:name, :description, :active, scopes_attributes: [:id, :environment_scope, :active, :_destroy, strategies: [:name, parameters: [:groupId, :percentage, :userIds]]], - strategies_attributes: [:id, :name, :_destroy, parameters: [:groupId, :percentage, :userIds], + strategies_attributes: [:id, :name, :user_list_id, :_destroy, + parameters: [:groupId, :percentage, :userIds], scopes_attributes: [:id, :environment_scope, :_destroy]]) end diff --git a/ee/app/models/operations/feature_flags/strategy.rb b/ee/app/models/operations/feature_flags/strategy.rb index 1c3d7bf6f108a4cc461c394af3779e81f3434891..ff68af9741e251d89c9c4f38d156e864678e64b9 100644 --- a/ee/app/models/operations/feature_flags/strategy.rb +++ b/ee/app/models/operations/feature_flags/strategy.rb @@ -35,6 +35,10 @@ class Strategy < ApplicationRecord accepts_nested_attributes_for :scopes, allow_destroy: true + def user_list_id=(user_list_id) + self.user_list = ::Operations::FeatureFlags::UserList.find(user_list_id) + end + private def same_project_validation diff --git a/ee/app/serializers/feature_flags/strategy_entity.rb b/ee/app/serializers/feature_flags/strategy_entity.rb index 56e526c4028a82edc6500697e99a08cd9821d217..7345047686903171fc9e97062079f460ac389b8c 100644 --- a/ee/app/serializers/feature_flags/strategy_entity.rb +++ b/ee/app/serializers/feature_flags/strategy_entity.rb @@ -6,5 +6,6 @@ class StrategyEntity < Grape::Entity expose :name expose :parameters expose :scopes, with: FeatureFlags::ScopeEntity + expose :user_list, with: FeatureFlags::UserListEntity, expose_nil: false end end diff --git a/ee/app/serializers/feature_flags/user_list_entity.rb b/ee/app/serializers/feature_flags/user_list_entity.rb new file mode 100644 index 0000000000000000000000000000000000000000..d3fddb4fa7adbcaa166f750285a092bb9e77368a --- /dev/null +++ b/ee/app/serializers/feature_flags/user_list_entity.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module FeatureFlags + class UserListEntity < Grape::Entity + expose :id + expose :iid + expose :name + expose :user_xids + end +end diff --git a/ee/spec/controllers/projects/feature_flags_controller_spec.rb b/ee/spec/controllers/projects/feature_flags_controller_spec.rb index ca4488ec2bfbb1deebe697fe16c65d9d6e8ed666..627d7e70c320abd9c1a83e7d3c25d78e8f83126c 100644 --- a/ee/spec/controllers/projects/feature_flags_controller_spec.rb +++ b/ee/spec/controllers/projects/feature_flags_controller_spec.rb @@ -693,6 +693,50 @@ end end + context 'when creating a version 2 feature flag with a gitlabUserList strategy' do + let!(:user_list) do + create(:operations_feature_flag_user_list, project: project, + name: 'My List', user_xids: 'user1,user2') + end + + 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: 'gitlabUserList', + parameters: {}, + user_list_id: user_list.id, + scopes_attributes: [{ environment_scope: 'production' }] + }] + } + } + end + + it 'creates the new strategy' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['strategies']).to match([a_hash_including({ + 'name' => 'gitlabUserList', + 'parameters' => {}, + 'user_list' => { + 'id' => user_list.id, + 'iid' => user_list.iid, + 'name' => 'My List', + 'user_xids' => 'user1,user2' + }, + 'scopes' => [a_hash_including({ + 'environment_scope' => 'production' + })] + })]) + end + end + context 'when version parameter is invalid' do let(:params) do { @@ -1316,6 +1360,114 @@ def request_params(scope, strategies) expect(strategy_json['scopes']).to eq([]) end + it 'creates a gitlabUserList strategy' do + user_list = create(:operations_feature_flag_user_list, project: project, name: 'My List', user_xids: 'user1,user2') + params = { + namespace_id: project.namespace, + project_id: project, + iid: new_version_flag.iid, + operations_feature_flag: { + strategies_attributes: [{ + name: 'gitlabUserList', + parameters: {}, + user_list_id: user_list.id + }] + } + } + + put(:update, params: params, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['strategies']).to match([a_hash_including({ + 'id' => an_instance_of(Integer), + 'name' => 'gitlabUserList', + 'parameters' => {}, + 'user_list' => { + 'id' => user_list.id, + 'iid' => user_list.iid, + 'name' => 'My List', + 'user_xids' => 'user1,user2' + }, + 'scopes' => [] + })]) + end + + it 'supports switching the associated user list for an existing gitlabUserList strategy' do + user_list = create(:operations_feature_flag_user_list, project: project, name: 'My List', user_xids: 'user1,user2') + strategy = create(:operations_strategy, feature_flag: new_version_flag, name: 'gitlabUserList', parameters: {}, user_list: user_list) + other_user_list = create(:operations_feature_flag_user_list, project: project, name: 'Other List', user_xids: 'user3') + params = { + namespace_id: project.namespace, + project_id: project, + iid: new_version_flag.iid, + operations_feature_flag: { + strategies_attributes: [{ + id: strategy.id, + user_list_id: other_user_list.id + }] + } + } + + put(:update, params: params, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['strategies']).to eq([{ + 'id' => strategy.id, + 'name' => 'gitlabUserList', + 'parameters' => {}, + 'user_list' => { + 'id' => other_user_list.id, + 'iid' => other_user_list.iid, + 'name' => 'Other List', + 'user_xids' => 'user3' + }, + 'scopes' => [] + }]) + end + + it 'does not delete a user list when deleting a gitlabUserList strategy' do + user_list = create(:operations_feature_flag_user_list, project: project, name: 'My List', user_xids: 'user1,user2') + strategy = create(:operations_strategy, feature_flag: new_version_flag, name: 'gitlabUserList', parameters: {}, user_list: user_list) + params = { + namespace_id: project.namespace, + project_id: project, + iid: new_version_flag.iid, + operations_feature_flag: { + strategies_attributes: [{ + id: strategy.id, + _destroy: true + }] + } + } + + put(:update, params: params, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['strategies']).to eq([]) + expect(::Operations::FeatureFlags::Strategy.count).to eq(0) + expect(::Operations::FeatureFlags::StrategyUserList.count).to eq(0) + expect(::Operations::FeatureFlags::UserList.first).to eq(user_list) + end + + it 'returns not found when trying to create a gitlabUserList strategy with an invalid user list id' do + params = { + namespace_id: project.namespace, + project_id: project, + iid: new_version_flag.iid, + operations_feature_flag: { + strategies_attributes: [{ + name: 'gitlabUserList', + parameters: {}, + user_list_id: 1 + }] + } + } + + put(:update, params: params, format: :json) + + expect(response).to have_gitlab_http_status(:not_found) + end + it 'updates an existing strategy' do strategy = create(:operations_strategy, feature_flag: new_version_flag, name: 'default', parameters: {}) params = {