Commit 5a1613f8 authored by Peter Leitzen's avatar Peter Leitzen

Merge branch '229123-remove-race-condition-check-for-incident-label' into 'master'

Resolve "Consider removing the race condition check for labels when creating incident issues"

Closes #229123

See merge request gitlab-org/gitlab!37479
parents a3ee515e da520bc4
......@@ -39,17 +39,12 @@ module AlertManagement
def create_issue
label_result = find_or_create_incident_label
# Create an unlabelled issue if we couldn't create the label
# due to a race condition.
# See https://gitlab.com/gitlab-org/gitlab-foss/issues/65042
extra_params = label_result.success? ? { label_ids: [label_result.payload[:label].id] } : {}
issue = Issues::CreateService.new(
project,
user,
title: alert_presenter.title,
description: alert_presenter.issue_description,
**extra_params
label_ids: [label_result.payload[:label].id]
).execute
return error(object_errors(issue), issue) unless issue.valid?
......
......@@ -14,27 +14,9 @@ module IncidentManagement
def execute
label = Labels::FindOrCreateService
.new(current_user, project, **LABEL_PROPERTIES)
.execute
if label.invalid?
log_invalid_label_info(label)
return ServiceResponse.error(payload: { label: label }, message: full_error_message(label))
end
.execute(skip_authorization: true)
ServiceResponse.success(payload: { label: label })
end
private
def log_invalid_label_info(label)
log_info <<~TEXT.chomp
Cannot create incident label "#{label.title}" \
for "#{label.project.full_name}": #{full_error_message(label)}.
TEXT
end
def full_error_message(label)
label.errors.full_messages.to_sentence
end
end
end
......@@ -23,17 +23,12 @@ module IncidentManagement
def create_issue
label_result = find_or_create_incident_label
# Create an unlabelled issue if we couldn't create the label
# due to a race condition.
# See https://gitlab.com/gitlab-org/gitlab-foss/issues/65042
extra_params = label_result.success? ? { label_ids: [label_result.payload[:label].id] } : {}
Issues::CreateService.new(
project,
current_user,
title: issue_title,
description: issue_description,
**extra_params
label_ids: [label_result.payload[:label].id]
).execute
end
......
......@@ -25,17 +25,12 @@ module IncidentManagement
def create_issue
label_result = find_or_create_incident_label
# Create an unlabelled issue if we couldn't create the label
# due to a race condition.
# See https://gitlab.com/gitlab-org/gitlab-foss/issues/65042
extra_params = label_result.success? ? { label_ids: [label_result.payload[:label].id] } : {}
Issues::CreateService.new(
project,
current_user,
title: issue_title,
description: issue_description,
**extra_params
label_ids: [label_result.payload[:label].id]
).execute
end
......
......@@ -52,7 +52,15 @@ RSpec.describe IncidentManagement::CreateIncidentLabelService do
end
context 'without label' do
it_behaves_like 'new label'
context 'when user has permissions to create labels' do
it_behaves_like 'new label'
end
context 'when user has no permissions to create labels' do
let_it_be(:user) { create(:user) }
it_behaves_like 'new label'
end
end
end
end
......@@ -16,12 +16,4 @@ RSpec.shared_examples 'create alert issue sets issue labels' do
expect(issue.labels).to eq([label])
end
end
context 'when create incident label responds with error' do
let(:label_service_response) { ServiceResponse.error(payload: { label: label }, message: 'label error') }
it 'creates an issue without labels' do
expect(issue.labels).to be_empty
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