From d0593ee6aedf5c91da332ee33006446f9e52129a Mon Sep 17 00:00:00 2001
From: Vitali Tatarintev <vtatarintev@gitlab.com>
Date: Fri, 3 Jul 2020 12:32:57 +0000
Subject: [PATCH] Change alert severity sort order
Change `AlertManagement::Alert` severity sort order:
* Ascending sort order sorts severity from less critical to critical.
* Descending sort order sorts severity from critical to less critical.
---
.../types/alert_management/alert_sort_enum.rb | 8 +-
app/models/alert_management/alert.rb | 12 ++-
...e-alert-severity-and-status-sort-order.yml | 5 ++
.../graphql/reference/gitlab_schema.graphql | 8 +-
doc/api/graphql/reference/gitlab_schema.json | 8 +-
spec/factories/alert_management/alerts.rb | 24 +++++-
.../alert_management/alerts_finder_spec.rb | 78 +++++++------------
.../project/alert_management/alerts_spec.rb | 4 +-
8 files changed, 77 insertions(+), 70 deletions(-)
create mode 100644 changelogs/unreleased/221242-change-alert-severity-and-status-sort-order.yml
diff --git a/app/graphql/types/alert_management/alert_sort_enum.rb b/app/graphql/types/alert_management/alert_sort_enum.rb
index 3faac9ce53c4f..51e7bef0a7f2c 100644
--- a/app/graphql/types/alert_management/alert_sort_enum.rb
+++ b/app/graphql/types/alert_management/alert_sort_enum.rb
@@ -16,10 +16,10 @@ class AlertSortEnum < SortEnum
value 'UPDATED_TIME_DESC', 'Created time by descending order', value: :updated_at_desc
value 'EVENT_COUNT_ASC', 'Events count by ascending order', value: :event_count_asc
value 'EVENT_COUNT_DESC', 'Events count by descending order', value: :event_count_desc
- value 'SEVERITY_ASC', 'Severity by ascending order', value: :severity_asc
- value 'SEVERITY_DESC', 'Severity by descending order', value: :severity_desc
- value 'STATUS_ASC', 'Status by ascending order', value: :status_asc
- value 'STATUS_DESC', 'Status by descending order', value: :status_desc
+ value 'SEVERITY_ASC', 'Severity from less critical to more critical', value: :severity_asc
+ value 'SEVERITY_DESC', 'Severity from more critical to less critical', value: :severity_desc
+ value 'STATUS_ASC', 'Status by order: Ignored > Resolved > Acknowledged > Triggered', value: :status_asc
+ value 'STATUS_DESC', 'Status by order: Triggered > Acknowledged > Resolved > Ignored', value: :status_desc
end
end
end
diff --git a/app/models/alert_management/alert.rb b/app/models/alert_management/alert.rb
index 905245cda6047..2bc073d12be00 100644
--- a/app/models/alert_management/alert.rb
+++ b/app/models/alert_management/alert.rb
@@ -118,8 +118,16 @@ class Alert < ApplicationRecord
scope :order_start_time, -> (sort_order) { order(started_at: sort_order) }
scope :order_end_time, -> (sort_order) { order(ended_at: sort_order) }
scope :order_event_count, -> (sort_order) { order(events: sort_order) }
- scope :order_severity, -> (sort_order) { order(severity: sort_order) }
- scope :order_status, -> (sort_order) { order(status: sort_order) }
+
+ # Ascending sort order sorts severity from less critical to more critical.
+ # Descending sort order sorts severity from more critical to less critical.
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/221242#what-is-the-expected-correct-behavior
+ scope :order_severity, -> (sort_order) { order(severity: sort_order == :asc ? :desc : :asc) }
+
+ # Ascending sort order sorts statuses: Ignored > Resolved > Acknowledged > Triggered
+ # Descending sort order sorts statuses: Triggered > Acknowledged > Resolved > Ignored
+ # https://gitlab.com/gitlab-org/gitlab/-/issues/221242#what-is-the-expected-correct-behavior
+ scope :order_status, -> (sort_order) { order(status: sort_order == :asc ? :desc : :asc) }
scope :counts_by_status, -> { group(:status).count }
diff --git a/changelogs/unreleased/221242-change-alert-severity-and-status-sort-order.yml b/changelogs/unreleased/221242-change-alert-severity-and-status-sort-order.yml
new file mode 100644
index 0000000000000..5278ee537982b
--- /dev/null
+++ b/changelogs/unreleased/221242-change-alert-severity-and-status-sort-order.yml
@@ -0,0 +1,5 @@
+---
+title: Change the sort order for alert severity and status.
+merge_request: 35774
+author:
+type: fixed
diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql
index 70aab7a8c4c63..04b84c6eb65e6 100644
--- a/doc/api/graphql/reference/gitlab_schema.graphql
+++ b/doc/api/graphql/reference/gitlab_schema.graphql
@@ -395,12 +395,12 @@ enum AlertManagementAlertSort {
EVENT_COUNT_DESC
"""
- Severity by ascending order
+ Severity from less critical to more critical
"""
SEVERITY_ASC
"""
- Severity by descending order
+ Severity from more critical to less critical
"""
SEVERITY_DESC
@@ -415,12 +415,12 @@ enum AlertManagementAlertSort {
STARTED_AT_DESC
"""
- Status by ascending order
+ Status by order: Ignored > Resolved > Acknowledged > Triggered
"""
STATUS_ASC
"""
- Status by descending order
+ Status by order: Triggered > Acknowledged > Resolved > Ignored
"""
STATUS_DESC
diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json
index cc75fbcf91d33..ef35dd5588314 100644
--- a/doc/api/graphql/reference/gitlab_schema.json
+++ b/doc/api/graphql/reference/gitlab_schema.json
@@ -1103,25 +1103,25 @@
},
{
"name": "SEVERITY_ASC",
- "description": "Severity by ascending order",
+ "description": "Severity from less critical to more critical",
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "SEVERITY_DESC",
- "description": "Severity by descending order",
+ "description": "Severity from more critical to less critical",
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "STATUS_ASC",
- "description": "Status by ascending order",
+ "description": "Status by order: Ignored > Resolved > Acknowledged > Triggered",
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "STATUS_DESC",
- "description": "Status by descending order",
+ "description": "Status by order: Triggered > Acknowledged > Resolved > Ignored",
"isDeprecated": false,
"deprecationReason": null
}
diff --git a/spec/factories/alert_management/alerts.rb b/spec/factories/alert_management/alerts.rb
index 99ee133f7e4d7..ef511aa54b8cd 100644
--- a/spec/factories/alert_management/alerts.rb
+++ b/spec/factories/alert_management/alerts.rb
@@ -75,10 +75,30 @@
without_ended_at
end
- trait :low_severity do
+ trait :critical do
+ severity { 'critical' }
+ end
+
+ trait :high do
+ severity { 'high' }
+ end
+
+ trait :medium do
+ severity { 'medium' }
+ end
+
+ trait :low do
severity { 'low' }
end
+ trait :info do
+ severity { 'info' }
+ end
+
+ trait :unknown do
+ severity { 'unknown' }
+ end
+
trait :prometheus do
monitoring_tool { Gitlab::AlertManagement::AlertParams::MONITORING_TOOLS[:prometheus] }
end
@@ -91,7 +111,7 @@
with_monitoring_tool
with_host
with_description
- low_severity
+ low
end
end
end
diff --git a/spec/finders/alert_management/alerts_finder_spec.rb b/spec/finders/alert_management/alerts_finder_spec.rb
index 5920d579ba6d7..7bf9047704bf7 100644
--- a/spec/finders/alert_management/alerts_finder_spec.rb
+++ b/spec/finders/alert_management/alerts_finder_spec.rb
@@ -11,7 +11,7 @@
let(:params) { {} }
describe '#execute' do
- subject { described_class.new(current_user, project, params).execute }
+ subject(:execute) { described_class.new(current_user, project, params).execute }
context 'user is not a developer or above' do
it { is_expected.to be_empty }
@@ -144,81 +144,55 @@
end
context 'when sorting by severity' do
- let_it_be(:alert_critical) { create(:alert_management_alert, project: project, severity: :critical) }
- let_it_be(:alert_high) { create(:alert_management_alert, project: project, severity: :high) }
- let_it_be(:alert_medium) { create(:alert_management_alert, project: project, severity: :medium) }
- let_it_be(:alert_low) { create(:alert_management_alert, project: project, severity: :low) }
- let_it_be(:alert_info) { create(:alert_management_alert, project: project, severity: :info) }
- let_it_be(:alert_unknown) { create(:alert_management_alert, project: project, severity: :unknown) }
-
- context 'sorts alerts ascending' do
+ let_it_be(:alert_critical) { create(:alert_management_alert, :critical, project: project) }
+ let_it_be(:alert_high) { create(:alert_management_alert, :high, project: project) }
+ let_it_be(:alert_medium) { create(:alert_management_alert, :medium, project: project) }
+ let_it_be(:alert_low) { create(:alert_management_alert, :low, project: project) }
+ let_it_be(:alert_info) { create(:alert_management_alert, :info, project: project) }
+ let_it_be(:alert_unknown) { create(:alert_management_alert, :unknown, project: project) }
+
+ context 'with ascending sort order' do
let(:params) { { sort: 'severity_asc' } }
- it do
- is_expected.to eq [
- alert_2,
- alert_critical,
- alert_1,
- alert_high,
- alert_medium,
- alert_low,
- alert_info,
- alert_unknown
- ]
+ it 'sorts alerts by severity from less critical to more critical' do
+ expect(execute.pluck(:severity).uniq).to eq(%w(unknown info low medium high critical))
end
end
- context 'sorts alerts descending' do
+ context 'with descending sort order' do
let(:params) { { sort: 'severity_desc' } }
- it do
- is_expected.to eq [
- alert_unknown,
- alert_info,
- alert_low,
- alert_medium,
- alert_1,
- alert_high,
- alert_critical,
- alert_2
- ]
+ it 'sorts alerts by severity from more critical to less critical' do
+ expect(execute.pluck(:severity).uniq).to eq(%w(critical high medium low info unknown))
end
end
end
context 'when sorting by status' do
+ let(:statuses) { AlertManagement::Alert::STATUSES }
+ let(:triggered) { statuses[:triggered] }
+ let(:acknowledged) { statuses[:acknowledged] }
+ let(:resolved) { statuses[:resolved] }
+ let(:ignored) { statuses[:ignored] }
+
let_it_be(:alert_triggered) { create(:alert_management_alert, project: project) }
let_it_be(:alert_acknowledged) { create(:alert_management_alert, :acknowledged, project: project) }
let_it_be(:alert_resolved) { create(:alert_management_alert, :resolved, project: project) }
let_it_be(:alert_ignored) { create(:alert_management_alert, :ignored, project: project) }
- context 'sorts alerts ascending' do
+ context 'with ascending sort order' do
let(:params) { { sort: 'status_asc' } }
- it do
- is_expected.to eq [
- alert_triggered,
- alert_acknowledged,
- alert_1,
- alert_resolved,
- alert_2,
- alert_ignored
- ]
+ it 'sorts by status: Ignored > Resolved > Acknowledged > Triggered' do
+ expect(execute.map(&:status).uniq).to eq([ignored, resolved, acknowledged, triggered])
end
end
- context 'sorts alerts descending' do
+ context 'with descending sort order' do
let(:params) { { sort: 'status_desc' } }
- it do
- is_expected.to eq [
- alert_2,
- alert_ignored,
- alert_1,
- alert_resolved,
- alert_acknowledged,
- alert_triggered
- ]
+ it 'sorts by status: Triggered > Acknowledged > Resolved > Ignored' do
+ expect(execute.map(&:status).uniq).to eq([triggered, acknowledged, resolved, ignored])
end
end
end
diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb
index 9896b0b0cf58b..7734ba51014e2 100644
--- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb
+++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb
@@ -110,14 +110,14 @@
it_behaves_like 'a working graphql query'
it 'sorts in the correct order' do
- expect(iids).to eq [resolved_alert.iid.to_s, triggered_alert.iid.to_s]
+ expect(iids).to eq [triggered_alert.iid.to_s, resolved_alert.iid.to_s]
end
context 'ascending order' do
let(:params) { 'sort: SEVERITY_ASC' }
it 'sorts in the correct order' do
- expect(iids).to eq [triggered_alert.iid.to_s, resolved_alert.iid.to_s]
+ expect(iids).to eq [resolved_alert.iid.to_s, triggered_alert.iid.to_s]
end
end
end
--
GitLab