Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
594f3695
Commit
594f3695
authored
3 years ago
by
Sarah Yasonik
Committed by
Michael Kozono
3 years ago
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Allow incident severity updates to trigger issue webhooks
parent
669eb4d7
Changes
12
Show whitespace changes
Inline
Side-by-side
Showing
12 changed files
with
170 additions
and
169 deletions
+170
-169
app/models/concerns/issuable.rb
app/models/concerns/issuable.rb
+5
-0
app/models/issue.rb
app/models/issue.rb
+1
-0
app/services/incident_management/incidents/create_service.rb
app/services/incident_management/incidents/create_service.rb
+2
-7
app/services/incident_management/incidents/update_severity_service.rb
.../incident_management/incidents/update_severity_service.rb
+0
-38
app/services/issuable_base_service.rb
app/services/issuable_base_service.rb
+12
-7
app/services/issues/update_service.rb
app/services/issues/update_service.rb
+9
-0
lib/gitlab/hook_data/issue_builder.rb
lib/gitlab/hook_data/issue_builder.rb
+3
-1
spec/graphql/mutations/issues/set_severity_spec.rb
spec/graphql/mutations/issues/set_severity_spec.rb
+1
-1
spec/lib/gitlab/hook_data/issue_builder_spec.rb
spec/lib/gitlab/hook_data/issue_builder_spec.rb
+1
-0
spec/models/concerns/issuable_spec.rb
spec/models/concerns/issuable_spec.rb
+20
-0
spec/services/incident_management/incidents/update_severity_service_spec.rb
...dent_management/incidents/update_severity_service_spec.rb
+0
-86
spec/services/issues/update_service_spec.rb
spec/services/issues/update_service_spec.rb
+116
-29
No files found.
app/models/concerns/issuable.rb
View file @
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
)
...
...
This diff is collapsed.
Click to expand it.
app/models/issue.rb
View file @
594f3695
...
...
@@ -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
...
...
This diff is collapsed.
Click to expand it.
app/services/incident_management/incidents/create_service.rb
View file @
594f3695
...
...
@@ -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
This diff is collapsed.
Click to expand it.
app/services/incident_management/incidents/update_severity_service.rb
deleted
100644 → 0
View file @
669eb4d7
# 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
This diff is collapsed.
Click to expand it.
app/services/issuable_base_service.rb
View file @
594f3695
...
...
@@ -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
...
...
This diff is collapsed.
Click to expand it.
app/services/issues/update_service.rb
View file @
594f3695
...
...
@@ -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
...
...
This diff is collapsed.
Click to expand it.
lib/gitlab/hook_data/issue_builder.rb
View file @
594f3695
...
...
@@ -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
)
...
...
This diff is collapsed.
Click to expand it.
spec/graphql/mutations/issues/set_severity_spec.rb
View file @
594f3695
...
...
@@ -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
)
}
...
...
This diff is collapsed.
Click to expand it.
spec/lib/gitlab/hook_data/issue_builder_spec.rb
View file @
594f3695
...
...
@@ -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
...
...
This diff is collapsed.
Click to expand it.
spec/models/concerns/issuable_spec.rb
View file @
594f3695
...
...
@@ -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
...
...
This diff is collapsed.
Click to expand it.
spec/services/incident_management/incidents/update_severity_service_spec.rb
deleted
100644 → 0
View file @
669eb4d7
# 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
This diff is collapsed.
Click to expand it.
spec/services/issues/update_service_spec.rb
View file @
594f3695
...
...
@@ -83,16 +83,16 @@ RSpec.describe Issues::UpdateService, :mailer do
end
context
'when issue type is not incident'
do
it
'returns default severity'
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
)
}
...
...
This diff is collapsed.
Click to expand it.
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment