diff --git a/app/graphql/types/alert_management/alert_sort_enum.rb b/app/graphql/types/alert_management/alert_sort_enum.rb index 3faac9ce53c4f94150d7d6be93969e7d8f96228a..51e7bef0a7f2cd8032793a67e33d3afbb06498fd 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 905245cda6047897e4dac05ebae0c27aa9c7c2d6..2bc073d12be00fca7edd8495f964f22e042cbe47 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 0000000000000000000000000000000000000000..5278ee537982b0797c55fabeb5d27740f5582b1c --- /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 70aab7a8c4c633f275b565dabe438fe0eea052fb..04b84c6eb65e6612671569f40589729b66613b91 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 cc75fbcf91d3310789e3ca1b37e4587c49f39906..ef35dd5588314cfd8e055485f88906cd1dba3b45 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 99ee133f7e4d79143f0570180079074cae546280..ef511aa54b8cdeb949ac8599ccdc8d3da1c93f98 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 5920d579ba6d773063ee22cb5f481f5bd93018a7..7bf9047704bf73df6b7a28e36faf1a21b52da6b5 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 9896b0b0cf58b994019adf6b29e0dc1fa9e74d69..7734ba51014e298d9364d7a0584fef49a447cef2 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