Commit 7b78c0a7 authored by Jason Goodman's avatar Jason Goodman Committed by Shinya Maeda

Do not allow a user list to be deleted if associated with a strategy

Return an error message in the api
parent f0805a8b
......@@ -9,6 +9,8 @@ module Operations
self.table_name = 'operations_user_lists'
belongs_to :project
has_many :strategy_user_lists
has_many :strategies, through: :strategy_user_lists
has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.operations_feature_flags_user_lists&.maximum(:iid) }, presence: true
......@@ -18,6 +20,17 @@ module Operations
uniqueness: { scope: :project_id },
length: 1..255
validates :user_xids, feature_flag_user_xids: true
before_destroy :ensure_no_associated_strategies
private
def ensure_no_associated_strategies
if strategies.present?
errors.add(:base, 'User list is associated with a strategy')
throw :abort # rubocop: disable Cop/BanCatchThrow
end
end
end
end
end
---
title: Forbid deleting a feature flag user list associated with a strategy
merge_request: 35275
author:
type: fixed
......@@ -84,7 +84,9 @@ module API
end
delete do
list = user_project.operations_feature_flags_user_lists.find_by_iid!(params[:iid])
list.destroy
unless list.destroy
render_api_error!(list.errors.full_messages, :conflict)
end
end
end
end
......
......@@ -67,6 +67,31 @@ RSpec.describe Operations::FeatureFlags::UserList do
end
end
describe '#destroy' do
it 'deletes the model if it is not associated with any feature flag strategies' do
project = create(:project)
user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2')
user_list.destroy
expect(described_class.count).to eq(0)
end
it 'does not delete the model if it is associated with a feature flag strategy' do
project = create(:project)
user_list = described_class.create(project: project, name: 'My User List', user_xids: 'user1,user2')
feature_flag = create(:operations_feature_flag, :new_version_flag, project: project)
strategy = create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: user_list)
user_list.destroy
expect(described_class.count).to eq(1)
expect(::Operations::FeatureFlags::StrategyUserList.count).to eq(1)
expect(strategy.reload.user_list).to eq(user_list)
expect(strategy.valid?).to eq(true)
end
end
it_behaves_like 'AtomicInternalId' do
let(:internal_id_attribute) { :iid }
let(:instance) { build(:operations_feature_flag_user_list) }
......
......@@ -343,8 +343,21 @@ RSpec.describe API::FeatureFlagsUserLists do
delete api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to have_gitlab_http_status(:no_content)
expect(response.body).to be_blank
expect(project.operations_feature_flags_user_lists.count).to eq(0)
end
it 'does not delete the list if it is associated with a strategy' do
list = create_list
feature_flag = create(:operations_feature_flag, :new_version_flag, project: project)
create(:operations_strategy, feature_flag: feature_flag, name: 'gitlabUserList', user_list: list)
delete api("/projects/#{project.id}/feature_flags_user_lists/#{list.iid}", developer)
expect(response).to have_gitlab_http_status(:conflict)
expect(json_response).to eq({ 'message' => ['User list is associated with a strategy'] })
expect(list.reload).to be_persisted
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment