Commit c2c941f1 authored by Jan Provaznik's avatar Jan Provaznik

Merge branch 'sy-allow-to-unlabel-incidents' into 'master'

Do not auto-reapply incident label to all incidents

See merge request gitlab-org/gitlab!49188
parents b3941ef8 b348db3c
...@@ -8,7 +8,6 @@ module AlertManagement ...@@ -8,7 +8,6 @@ module AlertManagement
MARKDOWN_LINE_BREAK = " \n" MARKDOWN_LINE_BREAK = " \n"
HORIZONTAL_LINE = "\n\n---\n\n" HORIZONTAL_LINE = "\n\n---\n\n"
INCIDENT_LABEL_NAME = ::IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES[:title]
delegate :metrics_dashboard_url, :runbook, to: :parsed_payload delegate :metrics_dashboard_url, :runbook, to: :parsed_payload
...@@ -48,7 +47,7 @@ module AlertManagement ...@@ -48,7 +47,7 @@ module AlertManagement
end end
def incident_issues_link def incident_issues_link
project_issues_url(project, label_name: INCIDENT_LABEL_NAME) project_incidents_url(project)
end end
def performance_dashboard_link def performance_dashboard_link
......
...@@ -73,22 +73,6 @@ module Issues ...@@ -73,22 +73,6 @@ module Issues
Milestones::IssuesCountService.new(milestone).delete_cache Milestones::IssuesCountService.new(milestone).delete_cache
end end
# Applies label "incident" (creates it if missing) to incident issues.
# Please use in "after" hooks only to ensure we are not appyling
# labels prematurely.
def add_incident_label(issue)
return unless issue.incident?
label = ::IncidentManagement::CreateIncidentLabelService
.new(project, current_user)
.execute
.payload[:label]
return if issue.label_ids.include?(label.id)
issue.labels << label
end
end end
end end
......
...@@ -49,6 +49,22 @@ module Issues ...@@ -49,6 +49,22 @@ module Issues
def user_agent_detail_service def user_agent_detail_service
UserAgentDetailService.new(@issue, @request) UserAgentDetailService.new(@issue, @request)
end end
# Applies label "incident" (creates it if missing) to incident issues.
# For use in "after" hooks only to ensure we are not appyling
# labels prematurely.
def add_incident_label(issue)
return unless issue.incident?
label = ::IncidentManagement::CreateIncidentLabelService
.new(project, current_user)
.execute
.payload[:label]
return if issue.label_ids.include?(label.id)
issue.labels << label
end
end end
end end
......
...@@ -34,7 +34,6 @@ module Issues ...@@ -34,7 +34,6 @@ module Issues
end end
def after_update(issue) def after_update(issue)
add_incident_label(issue)
IssuesChannel.broadcast_to(issue, event: 'updated') if Gitlab::ActionCable::Config.in_app? || Feature.enabled?(:broadcast_issue_updates, issue.project) IssuesChannel.broadcast_to(issue, event: 'updated') if Gitlab::ActionCable::Config.in_app? || Feature.enabled?(:broadcast_issue_updates, issue.project)
end end
......
---
title: Do not automatically reapply incident label after user removes it
merge_request: 49188
author:
type: fixed
...@@ -19,11 +19,9 @@ RSpec.describe Emails::Projects do ...@@ -19,11 +19,9 @@ RSpec.describe Emails::Projects do
create(:project_incident_management_setting, project: project, create_issue: true) create(:project_incident_management_setting, project: project, create_issue: true)
end end
let(:incident_issues_url) do let(:incidents_url) { project_incidents_url(project) }
project_issues_url(project, label_name: 'incident')
end
it { is_expected.to have_body_text(incident_issues_url) } it { is_expected.to have_body_text(incidents_url) }
end end
end end
......
...@@ -37,6 +37,8 @@ RSpec.describe IncidentManagement::Incidents::CreateService do ...@@ -37,6 +37,8 @@ RSpec.describe IncidentManagement::Incidents::CreateService do
end end
let(:issue) { new_issue } let(:issue) { new_issue }
include_examples 'has incident label'
end end
context 'with default severity' do context 'with default severity' do
......
...@@ -63,6 +63,7 @@ RSpec.describe Issues::CreateService do ...@@ -63,6 +63,7 @@ RSpec.describe Issues::CreateService do
subject { issue } subject { issue }
it_behaves_like 'incident issue' it_behaves_like 'incident issue'
it_behaves_like 'has incident label'
it_behaves_like 'an incident management tracked event', :incident_management_incident_created it_behaves_like 'an incident management tracked event', :incident_management_incident_created
it 'does create an incident label' do it 'does create an incident label' do
......
...@@ -99,31 +99,18 @@ RSpec.describe Issues::UpdateService, :mailer do ...@@ -99,31 +99,18 @@ RSpec.describe Issues::UpdateService, :mailer do
context 'when issue type is incident' do context 'when issue type is incident' do
let(:issue) { create(:incident, project: project) } let(:issue) { create(:incident, project: project) }
it 'changes updates the severity' do
update_issue(opts)
expect(issue.severity).to eq('low')
end
it_behaves_like 'incident issue' do
before do before do
update_issue(opts) update_issue(opts)
end end
end
context 'with existing incident label' do it_behaves_like 'incident issue'
let_it_be(:incident_label) { create(:label, :incident, project: project) }
before do it 'changes updates the severity' do
opts.delete(:label_ids) # don't override but retain existing labels expect(issue.severity).to eq('low')
issue.labels << incident_label
end end
it_behaves_like 'incident issue' do it 'does not apply incident labels' do
before do expect(issue.labels).to match_array [label]
update_issue(opts)
end
end
end end
end end
...@@ -155,7 +142,6 @@ RSpec.describe Issues::UpdateService, :mailer do ...@@ -155,7 +142,6 @@ RSpec.describe Issues::UpdateService, :mailer do
context 'issue in incident type' do context 'issue in incident type' do
let(:current_user) { user } let(:current_user) { user }
let(:incident_label_attributes) { attributes_for(:label, :incident) }
before do before do
opts.merge!(issue_type: 'incident', confidential: true) opts.merge!(issue_type: 'incident', confidential: true)
...@@ -170,21 +156,6 @@ RSpec.describe Issues::UpdateService, :mailer do ...@@ -170,21 +156,6 @@ RSpec.describe Issues::UpdateService, :mailer do
subject subject
end end
end end
it 'does create an incident label' do
expect { subject }
.to change { Label.where(incident_label_attributes).count }.by(1)
end
context 'when invalid' do
before do
opts.merge!(title: '')
end
it 'does not create an incident label prematurely' do
expect { subject }.not_to change(Label, :count)
end
end
end end
it 'updates open issue counter for assignees when issue is reassigned' do it 'updates open issue counter for assignees when issue is reassigned' do
......
...@@ -11,11 +11,13 @@ ...@@ -11,11 +11,13 @@
# #
# include_examples 'incident issue' # include_examples 'incident issue'
RSpec.shared_examples 'incident issue' do RSpec.shared_examples 'incident issue' do
let(:label_properties) { attributes_for(:label, :incident) }
it 'has incident as issue type' do it 'has incident as issue type' do
expect(issue.issue_type).to eq('incident') expect(issue.issue_type).to eq('incident')
end end
end
RSpec.shared_examples 'has incident label' do
let(:label_properties) { attributes_for(:label, :incident) }
it 'has exactly one incident label' do it 'has exactly one incident label' do
expect(issue.labels).to be_one do |label| expect(issue.labels).to be_one do |label|
......
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