Commit 459799dd authored by Michael Kozono's avatar Michael Kozono

Merge branch 'sy-add-incident-webhooks-for-severity' into 'master'

Allow incident severity updates to trigger issue webhooks

See merge request gitlab-org/gitlab!64748
parents b2298416 594f3695
......@@ -459,6 +459,7 @@ module Issuable
if old_associations
old_labels = old_associations.fetch(:labels, labels)
old_assignees = old_associations.fetch(:assignees, assignees)
old_severity = old_associations.fetch(:severity, severity)
if old_labels != labels
changes[:labels] = [old_labels.map(&:hook_attrs), labels.map(&:hook_attrs)]
......@@ -468,6 +469,10 @@ module Issuable
changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
end
if supports_severity? && old_severity != severity
changes[:severity] = [old_severity, severity]
end
if self.respond_to?(:total_time_spent)
old_total_time_spent = old_associations.fetch(:total_time_spent, total_time_spent)
old_time_change = old_associations.fetch(:time_change, time_change)
......
......@@ -80,6 +80,7 @@ class Issue < ApplicationRecord
has_and_belongs_to_many :prometheus_alert_events, join_table: :issues_prometheus_alert_events # rubocop: disable Rails/HasAndBelongsToMany
has_many :prometheus_alerts, through: :prometheus_alert_events
accepts_nested_attributes_for :issuable_severity, update_only: true
accepts_nested_attributes_for :sentry_issue
validates :project, presence: true
......
......@@ -20,15 +20,14 @@ module IncidentManagement
params: {
title: title,
description: description,
issue_type: ISSUE_TYPE
issue_type: ISSUE_TYPE,
severity: severity
},
spam_params: nil
).execute
return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid?
update_severity_for(issue)
success(issue)
end
......@@ -43,10 +42,6 @@ module IncidentManagement
def error(message, issue = nil)
ServiceResponse.error(payload: { issue: issue }, message: message)
end
def update_severity_for(issue)
::IncidentManagement::Incidents::UpdateSeverityService.new(issue, current_user, severity).execute
end
end
end
end
# frozen_string_literal: true
module IncidentManagement
module Incidents
class UpdateSeverityService < BaseService
def initialize(issuable, current_user, severity)
super(issuable.project, current_user)
@issuable = issuable
@severity = severity.to_s.downcase
@severity = IssuableSeverity::DEFAULT unless IssuableSeverity.severities.key?(@severity)
end
def execute
return unless issuable.supports_severity?
update_severity!
add_system_note
end
private
attr_reader :issuable, :severity
def issuable_severity
issuable.issuable_severity || issuable.build_issuable_severity(issue_id: issuable.id)
end
def update_severity!
issuable_severity.update!(severity: severity)
end
def add_system_note
::IncidentManagement::AddSeveritySystemNoteWorker.perform_async(issuable.id, current_user.id)
end
end
end
end
......@@ -57,6 +57,7 @@ class IssuableBaseService < ::BaseProjectService
filter_assignees(issuable)
filter_milestone
filter_labels
filter_severity(issuable)
end
def filter_assignees(issuable)
......@@ -135,6 +136,16 @@ class IssuableBaseService < ::BaseProjectService
@labels_service ||= ::Labels::AvailableLabelsService.new(current_user, parent, params)
end
def filter_severity(issuable)
severity = params.delete(:severity)
return unless severity && issuable.supports_severity?
severity = IssuableSeverity::DEFAULT unless IssuableSeverity.severities.key?(severity)
return if severity == issuable.severity
params[:issuable_severity_attributes] = { severity: severity }
end
def process_label_ids(attributes, existing_label_ids: nil, extra_label_ids: [])
label_ids = attributes.delete(:label_ids)
add_label_ids = attributes.delete(:add_label_ids)
......@@ -352,7 +363,6 @@ class IssuableBaseService < ::BaseProjectService
def change_additional_attributes(issuable)
change_state(issuable)
change_severity(issuable)
change_subscription(issuable)
change_todo(issuable)
toggle_award(issuable)
......@@ -371,12 +381,6 @@ class IssuableBaseService < ::BaseProjectService
end
end
def change_severity(issuable)
if severity = params.delete(:severity)
::IncidentManagement::Incidents::UpdateSeverityService.new(issuable, current_user, severity).execute
end
end
def change_subscription(issuable)
case params.delete(:subscription_event)
when 'subscribe'
......@@ -443,6 +447,7 @@ class IssuableBaseService < ::BaseProjectService
associations[:time_change] = issuable.time_change if issuable.respond_to?(:time_change)
associations[:description] = issuable.description
associations[:reviewers] = issuable.reviewers.to_a if issuable.allows_reviewers?
associations[:severity] = issuable.severity if issuable.supports_severity?
associations
end
......
......@@ -42,6 +42,7 @@ module Issues
old_labels = old_associations.fetch(:labels, [])
old_mentioned_users = old_associations.fetch(:mentioned_users, [])
old_assignees = old_associations.fetch(:assignees, [])
old_severity = old_associations[:severity]
if has_changes?(issue, old_labels: old_labels, old_assignees: old_assignees)
todo_service.resolve_todos_for_target(issue, current_user)
......@@ -74,6 +75,8 @@ module Issues
if added_mentions.present?
notification_service.async.new_mentions_in_issue(issue, added_mentions, current_user)
end
handle_severity_change(issue, old_severity)
end
def handle_assignee_changes(issue, old_assignees)
......@@ -181,6 +184,12 @@ module Issues
end
end
def handle_severity_change(issue, old_severity)
return unless old_severity && issue.severity != old_severity
::IncidentManagement::AddSeveritySystemNoteWorker.perform_async(issue.id, current_user.id)
end
# rubocop: disable CodeReuse/ActiveRecord
def issuable_for_positioning(id, board_group_id = nil)
return unless id
......
......@@ -8,6 +8,7 @@ module Gitlab
labels
total_time_spent
time_change
severity
].freeze
def self.safe_hook_attributes
......@@ -51,7 +52,8 @@ module Gitlab
assignee_ids: issue.assignee_ids,
assignee_id: issue.assignee_ids.first, # This key is deprecated
labels: issue.labels_hook_attrs,
state: issue.state
state: issue.state,
severity: issue.severity
}
issue.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes)
......
......@@ -11,7 +11,7 @@ RSpec.describe Mutations::Issues::SetSeverity do
specify { expect(described_class).to require_graphql_authorizations(:update_issue) }
describe '#resolve' do
let(:severity) { 'CRITICAL' }
let(:severity) { 'critical' }
let(:mutated_incident) { subject[:issue] }
subject(:resolve) { mutation.resolve(project_path: issue.project.full_path, iid: issue.iid, severity: severity) }
......
......@@ -48,6 +48,7 @@ RSpec.describe Gitlab::HookData::IssueBuilder do
expect(data).to include(:human_time_change)
expect(data).to include(:assignee_ids)
expect(data).to include(:state)
expect(data).to include(:severity)
expect(data).to include('labels' => [label.hook_attrs])
end
......
......@@ -535,6 +535,26 @@ RSpec.describe Issuable do
merge_request.to_hook_data(user, old_associations: { assignees: [user] })
end
end
context 'incident severity is updated' do
let(:issue) { create(:incident) }
before do
issue.update!(issuable_severity_attributes: { severity: 'low' })
expect(Gitlab::HookData::IssuableBuilder)
.to receive(:new).with(issue).and_return(builder)
end
it 'delegates to Gitlab::HookData::IssuableBuilder#build' do
expect(builder).to receive(:build).with(
user: user,
changes: hash_including(
'severity' => %w(unknown low)
))
issue.to_hook_data(user, old_associations: { severity: 'unknown' })
end
end
end
describe '#labels_array' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe IncidentManagement::Incidents::UpdateSeverityService do
let_it_be(:user) { create(:user) }
describe '#execute' do
let(:severity) { 'low' }
let(:system_note_worker) { ::IncidentManagement::AddSeveritySystemNoteWorker }
subject(:update_severity) { described_class.new(issuable, user, severity).execute }
before do
allow(system_note_worker).to receive(:perform_async)
end
shared_examples 'adds a system note' do
it 'calls AddSeveritySystemNoteWorker' do
update_severity
expect(system_note_worker).to have_received(:perform_async).with(issuable.id, user.id)
end
end
context 'when issuable not an incident' do
%i(issue merge_request).each do |issuable_type|
let(:issuable) { build_stubbed(issuable_type) }
it { is_expected.to be_nil }
it 'does not set severity' do
expect { update_severity }.not_to change(IssuableSeverity, :count)
end
it 'does not add a system note' do
update_severity
expect(system_note_worker).not_to have_received(:perform_async)
end
end
end
context 'when issuable is an incident' do
let!(:issuable) { create(:incident) }
context 'when issuable does not have issuable severity yet' do
it 'creates new record' do
expect { update_severity }.to change { IssuableSeverity.where(issue: issuable).count }.to(1)
end
it 'sets severity to specified value' do
expect { update_severity }.to change { issuable.severity }.to('low')
end
it_behaves_like 'adds a system note'
end
context 'when issuable has an issuable severity' do
let!(:issuable_severity) { create(:issuable_severity, issue: issuable, severity: 'medium') }
it 'does not create new record' do
expect { update_severity }.not_to change(IssuableSeverity, :count)
end
it 'updates existing issuable severity' do
expect { update_severity }.to change { issuable_severity.severity }.to(severity)
end
it_behaves_like 'adds a system note'
end
context 'when severity value is unsupported' do
let(:severity) { 'unsupported-severity' }
it 'sets the severity to default value' do
update_severity
expect(issuable.issuable_severity.severity).to eq(IssuableSeverity::DEFAULT)
end
it_behaves_like 'adds a system note'
end
end
end
end
......@@ -83,16 +83,16 @@ RSpec.describe Issues::UpdateService, :mailer do
end
context 'when issue type is not incident' do
it 'returns default severity' do
before do
update_issue(opts)
expect(issue.severity).to eq(IssuableSeverity::DEFAULT)
end
it_behaves_like 'not an incident issue' do
before do
update_issue(opts)
end
it_behaves_like 'not an incident issue'
context 'when confidentiality is changed' do
subject { update_issue(confidential: true) }
it_behaves_like 'does not track incident management event'
end
end
......@@ -105,12 +105,16 @@ RSpec.describe Issues::UpdateService, :mailer do
it_behaves_like 'incident issue'
it 'changes updates the severity' do
expect(issue.severity).to eq('low')
it 'does not add an incident label' do
expect(issue.labels).to match_array [label]
end
it 'does not apply incident labels' do
expect(issue.labels).to match_array [label]
context 'when confidentiality is changed' do
let(:current_user) { user }
subject { update_issue(confidential: true) }
it_behaves_like 'an incident management tracked event', :incident_management_incident_change_confidential
end
end
......@@ -140,24 +144,6 @@ RSpec.describe Issues::UpdateService, :mailer do
expect(issue.confidential).to be_falsey
end
context 'issue in incident type' do
let(:current_user) { user }
before do
opts.merge!(issue_type: 'incident', confidential: true)
end
subject { update_issue(opts) }
it_behaves_like 'an incident management tracked event', :incident_management_incident_change_confidential
it_behaves_like 'incident issue' do
before do
subject
end
end
end
context 'changing issue_type' do
let!(:label_1) { create(:label, project: project, title: 'incident') }
let!(:label_2) { create(:label, project: project, title: 'missed-sla') }
......@@ -167,6 +153,12 @@ RSpec.describe Issues::UpdateService, :mailer do
end
context 'from issue to incident' do
it_behaves_like 'incident issue' do
before do
update_issue(**opts, issue_type: 'incident')
end
end
it 'adds a `incident` label if one does not exist' do
expect { update_issue(issue_type: 'incident') }.to change(issue.labels, :count).by(1)
expect(issue.labels.pluck(:title)).to eq(['incident'])
......@@ -1020,6 +1012,101 @@ RSpec.describe Issues::UpdateService, :mailer do
include_examples 'updating mentions', described_class
end
context 'updating severity' do
let(:opts) { { severity: 'low' } }
shared_examples 'updates the severity' do |expected_severity|
it 'has correct value' do
update_issue(opts)
expect(issue.severity).to eq(expected_severity)
end
it 'creates a system note' do
expect(::IncidentManagement::AddSeveritySystemNoteWorker).to receive(:perform_async).with(issue.id, user.id)
update_issue(opts)
end
it 'triggers webhooks' do
expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks)
expect(project).to receive(:execute_integrations).with(an_instance_of(Hash), :issue_hooks)
update_issue(opts)
end
end
shared_examples 'does not change the severity' do
it 'retains the original value' do
expected_severity = issue.severity
update_issue(opts)
expect(issue.severity).to eq(expected_severity)
end
it 'does not trigger side-effects' do
expect(::IncidentManagement::AddSeveritySystemNoteWorker).not_to receive(:perform_async)
expect(project).not_to receive(:execute_hooks)
expect(project).not_to receive(:execute_integrations)
expect { update_issue(opts) }.not_to change(IssuableSeverity, :count)
end
end
context 'on incidents' do
let(:issue) { create(:incident, project: project) }
context 'when severity has not been set previously' do
it_behaves_like 'updates the severity', 'low'
it 'creates a new record' do
expect { update_issue(opts) }.to change(IssuableSeverity, :count).by(1)
end
context 'with unsupported severity value' do
let(:opts) { { severity: 'unsupported-severity' } }
it_behaves_like 'does not change the severity'
end
context 'with severity value defined but unchanged' do
let(:opts) { { severity: IssuableSeverity::DEFAULT } }
it_behaves_like 'does not change the severity'
end
end
context 'when severity has been set before' do
before do
create(:issuable_severity, issue: issue, severity: 'high')
end
it_behaves_like 'updates the severity', 'low'
it 'does not create a new record' do
expect { update_issue(opts) }.not_to change(IssuableSeverity, :count)
end
context 'with unsupported severity value' do
let(:opts) { { severity: 'unsupported-severity' } }
it_behaves_like 'updates the severity', IssuableSeverity::DEFAULT
end
context 'with severity value defined but unchanged' do
let(:opts) { { severity: issue.severity } }
it_behaves_like 'does not change the severity'
end
end
end
context 'when issue type is not incident' do
it_behaves_like 'does not change the severity'
end
end
context 'duplicate issue' do
let(:canonical_issue) { create(:issue, project: project) }
......
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