Merge branch 'tr-sort-sla-column' into 'master'

Allow sorting of the Incident SLA column

See merge request gitlab-org/gitlab!45344

app/assets/javascripts/incidents/components/incidents_list.vue

@@ -69,9 +69,12 @@ export default {
     {
       key: 'incidentSla',
       label: s__('IncidentManagement|Time to SLA'),
-      thClass: `gl-pointer-events-none gl-text-right gl-w-eighth`,
+      thClass: `gl-text-right gl-w-eighth`,
       tdClass: `${tdClass} gl-text-right`,
       thAttr: TH_INCIDENT_SLA_TEST_ID,
+      sortKey: 'SLA_DUE_AT',
+      sortable: true,
+      sortDirection: 'asc',
     },
     {
       key: 'assignees',
@@ -253,13 +256,22 @@ export default {
       this.redirecting = true;
     },
     fetchSortedData({ sortBy, sortDesc }) {
+      let sortKey;
+      // In bootstrap-vue v2.17.0, sortKey becomes natively supported and we can eliminate this function
+      const field = this.availableFields.find(({ key }) => key === sortBy);
       const sortingDirection = sortDesc ? 'DESC' : 'ASC';
-      const sortingColumn = convertToSnakeCase(sortBy)
-        .replace(/_.*/, '')
-        .toUpperCase();
+
+      // Use `sortKey` if provided, otherwise fall back to existing algorithm
+      if (field?.sortKey) {
+        sortKey = field.sortKey;
+      } else {
+        sortKey = convertToSnakeCase(sortBy)
+          .replace(/_.*/, '')
+          .toUpperCase();
+      }
 
       this.pagination = initialPaginationState;
-      this.sort = `${sortingColumn}_${sortingDirection}`;
+      this.sort = `${sortKey}_${sortingDirection}`;
     },
     getSeverity(severity) {
       return INCIDENT_SEVERITY[severity];

app/models/concerns/issue_available_features.rb

@@ -6,18 +6,25 @@
 module IssueAvailableFeatures
   extend ActiveSupport::Concern
 
-  # EE only features are listed on EE::IssueAvailableFeatures
-  def available_features_for_issue_types
-    {}.with_indifferent_access
+  class_methods do
+    # EE only features are listed on EE::IssueAvailableFeatures
+    def available_features_for_issue_types
+      {}.with_indifferent_access
+    end
+  end
+
+  included do
+    scope :with_feature, ->(feature) { where(issue_type: available_features_for_issue_types[feature]) }
   end
 
   def issue_type_supports?(feature)
-    unless available_features_for_issue_types.has_key?(feature)
+    unless self.class.available_features_for_issue_types.has_key?(feature)
       raise ArgumentError, 'invalid feature'
     end
 
-    available_features_for_issue_types[feature].include?(issue_type)
+    self.class.available_features_for_issue_types[feature].include?(issue_type)
   end
 end
 
 IssueAvailableFeatures.prepend_if_ee('EE::IssueAvailableFeatures')
+IssueAvailableFeatures::ClassMethods.prepend_if_ee('EE::IssueAvailableFeatures::ClassMethods')

doc/api/graphql/reference/gitlab_schema.graphql

@@ -10216,6 +10216,16 @@ enum IssueSort {
   """
   SEVERITY_DESC
 
+  """
+  Issues with earliest SLA due time shown first
+  """
+  SLA_DUE_AT_ASC
+
+  """
+  Issues with latest SLA due time shown first
+  """
+  SLA_DUE_AT_DESC
+
   """
   Updated at ascending order
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -27896,6 +27896,18 @@
               "description": "Published issues shown first",
               "isDeprecated": false,
               "deprecationReason": null
+            },
+            {
+              "name": "SLA_DUE_AT_ASC",
+              "description": "Issues with earliest SLA due time shown first",
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
+            {
+              "name": "SLA_DUE_AT_DESC",
+              "description": "Issues with latest SLA due time shown first",
+              "isDeprecated": false,
+              "deprecationReason": null
             }
           ],
           "possibleTypes": null

doc/api/graphql/reference/index.md

@@ -3489,6 +3489,8 @@ Values for sorting issues.
 | `RELATIVE_POSITION_ASC` | Relative position by ascending order |
 | `SEVERITY_ASC` | Severity from less critical to more critical |
 | `SEVERITY_DESC` | Severity from more critical to less critical |
+| `SLA_DUE_AT_ASC` | Issues with earliest SLA due time shown first |
+| `SLA_DUE_AT_DESC` | Issues with latest SLA due time shown first |
 | `UPDATED_ASC` | Updated at ascending order |
 | `UPDATED_DESC` | Updated at descending order |
 | `WEIGHT_ASC` | Weight by ascending order |

ee/app/graphql/ee/types/issue_sort_enum.rb

@@ -10,6 +10,8 @@ module EE
         value 'WEIGHT_DESC', 'Weight by descending order', value: 'weight_desc'
         value 'PUBLISHED_ASC', 'Published issues shown last', value: :published_asc
         value 'PUBLISHED_DESC', 'Published issues shown first', value: :published_desc
+        value 'SLA_DUE_AT_ASC', 'Issues with earliest SLA due time shown first', value: :sla_due_at_asc
+        value 'SLA_DUE_AT_DESC', 'Issues with latest SLA due time shown first', value: :sla_due_at_desc
       end
     end
   end

ee/app/models/concerns/ee/issue_available_features.rb

@@ -2,13 +2,15 @@
 
 module EE
   module IssueAvailableFeatures
-    include ::Gitlab::Utils::StrongMemoize
-    extend ::Gitlab::Utils::Override
+    extend ActiveSupport::Concern
 
-    override :available_features_for_issue_types
-    def available_features_for_issue_types
-      strong_memoize(:available_features_for_issue_types) do
-        super.merge(epics: %w(issue))
+    class_methods do
+      include ::Gitlab::Utils::StrongMemoize
+
+      def available_features_for_issue_types
+        strong_memoize(:available_features_for_issue_types) do
+          super.merge(epics: %w(issue), sla: %w(incident))
+        end
       end
     end
   end

ee/app/models/ee/issue.rb

@@ -23,6 +23,8 @@ module EE
       scope :order_weight_asc, -> { reorder ::Gitlab::Database.nulls_last_order('weight') }
       scope :order_status_page_published_first, -> { includes(:status_page_published_incident).order('status_page_published_incidents.id ASC NULLS LAST') }
       scope :order_status_page_published_last, -> { includes(:status_page_published_incident).order('status_page_published_incidents.id ASC NULLS FIRST') }
+      scope :order_sla_due_at_asc, -> { includes(:issuable_sla).order('issuable_slas.due_at ASC NULLS LAST') }
+      scope :order_sla_due_at_desc, -> { includes(:issuable_sla).order('issuable_slas.due_at DESC NULLS LAST') }
       scope :no_epic, -> { left_outer_joins(:epic_issue).where(epic_issues: { epic_id: nil }) }
       scope :any_epic, -> { joins(:epic_issue) }
       scope :in_epics, ->(epics) { joins(:epic_issue).where(epic_issues: { epic_id: epics }) }
@@ -190,6 +192,8 @@ module EE
         when 'weight_desc'          then order_weight_desc.with_order_id_desc
         when 'published_asc'        then order_status_page_published_last.with_order_id_desc
         when 'published_desc'       then order_status_page_published_first.with_order_id_desc
+        when 'sla_due_at_asc'       then with_feature(:sla).order_sla_due_at_asc.with_order_id_desc
+        when 'sla_due_at_desc'      then with_feature(:sla).order_sla_due_at_desc.with_order_id_desc
         else
           super
         end

ee/changelogs/unreleased/tr-sort-sla-column.yml

+---
+title: Allow sorting of the Incident SLA column
+merge_request: 45344
+author:
+type: added

ee/spec/graphql/ee/resolvers/issues_resolver_spec.rb

@@ -43,6 +43,24 @@ RSpec.describe Resolvers::IssuesResolver do
             expect(resolve_issues(sort: :published_desc)).to eq [published, not_published]
           end
         end
+
+        context 'when sorting by sla due at' do
+          let_it_be(:sla_due_first) { create(:incident, project: project) }
+          let_it_be(:sla_due_last)  { create(:incident, project: project) }
+
+          before_all do
+            create(:issuable_sla, :exceeded, issue: sla_due_first)
+            create(:issuable_sla, issue: sla_due_last)
+          end
+
+          it 'sorts issues ascending' do
+            expect(resolve_issues(sort: :sla_due_at_asc)).to eq [sla_due_first, sla_due_last]
+          end
+
+          it 'sorts issues descending' do
+            expect(resolve_issues(sort: :sla_due_at_desc)).to eq [sla_due_last, sla_due_first]
+          end
+        end
       end
 
       describe 'filtering by iteration' do

ee/spec/graphql/ee/types/issue_sort_enum_spec.rb

@@ -8,6 +8,6 @@ RSpec.describe GitlabSchema.types['IssueSort'] do
   it_behaves_like 'common sort values'
 
   it 'exposes all the existing EE issue sort values' do
-    expect(described_class.values.keys).to include(*%w[WEIGHT_ASC WEIGHT_DESC PUBLISHED_ASC PUBLISHED_DESC])
+    expect(described_class.values.keys).to include(*%w[WEIGHT_ASC WEIGHT_DESC PUBLISHED_ASC PUBLISHED_DESC SLA_DUE_AT_ASC SLA_DUE_AT_DESC])
   end
 end

ee/spec/models/issue_spec.rb

@@ -100,6 +100,26 @@ RSpec.describe Issue do
       end
     end
 
+    describe '.with_feature' do
+      let_it_be(:project) { create(:project) }
+      let_it_be(:issue) { create(:issue, project: project) }
+      let_it_be(:incident) { create(:incident, project: project) }
+      let_it_be(:test_case) { create(:quality_test_case, project: project) }
+
+      it 'gives issues that support the given feature', :aggregate_failures do
+        expect(described_class.with_feature('epics'))
+          .to contain_exactly(issue)
+
+        expect(described_class.with_feature('sla'))
+          .to contain_exactly(incident)
+      end
+
+      it 'returns an empty collection when given an unknown feature' do
+        expect(described_class.with_feature('something-unknown'))
+          .to be_empty
+      end
+    end
+
     context 'epics' do
       let_it_be(:epic1) { create(:epic) }
       let_it_be(:epic2) { create(:epic) }
@@ -207,6 +227,30 @@ RSpec.describe Issue do
         it { is_expected.to eq([not_published, published]) }
       end
     end
+
+    context 'sla due at' do
+      let_it_be(:project) { create(:project) }
+      let_it_be(:sla_due_first) { create(:issue, project: project) }
+      let_it_be(:sla_due_last)  { create(:issue, project: project) }
+      let_it_be(:no_sla) { create(:issue, project: project) }
+
+      before_all do
+        create(:issuable_sla, :exceeded, issue: sla_due_first)
+        create(:issuable_sla, issue: sla_due_last)
+      end
+
+      describe '.order_sla_due_at_asc' do
+        subject { described_class.order_sla_due_at_asc }
+
+        it { is_expected.to eq([sla_due_first, sla_due_last, no_sla]) }
+      end
+
+      describe '.order_sla_due_at_desc' do
+        subject { described_class.order_sla_due_at_desc }
+
+        it { is_expected.to eq([sla_due_last, sla_due_first, no_sla]) }
+      end
+    end
   end
 
   describe 'validations' do

spec/frontend/incidents/components/incidents_list_spec.js

@@ -10,6 +10,7 @@ import {
   TH_CREATED_AT_TEST_ID,
   TH_SEVERITY_TEST_ID,
   TH_PUBLISHED_TEST_ID,
+  TH_INCIDENT_SLA_TEST_ID,
   trackIncidentCreateNewOptions,
   trackIncidentListViewsOptions,
 } from '~/incidents/constants';
@@ -277,10 +278,11 @@ describe('Incidents List', () => {
     const noneSort = 'none';
 
     it.each`
-      selector                 | initialSort | firstSort   | nextSort
-      ${TH_CREATED_AT_TEST_ID} | ${descSort} | ${ascSort}  | ${descSort}
-      ${TH_SEVERITY_TEST_ID}   | ${noneSort} | ${descSort} | ${ascSort}
-      ${TH_PUBLISHED_TEST_ID}  | ${noneSort} | ${descSort} | ${ascSort}
+      selector                   | initialSort | firstSort   | nextSort
+      ${TH_CREATED_AT_TEST_ID}   | ${descSort} | ${ascSort}  | ${descSort}
+      ${TH_SEVERITY_TEST_ID}     | ${noneSort} | ${descSort} | ${ascSort}
+      ${TH_PUBLISHED_TEST_ID}    | ${noneSort} | ${descSort} | ${ascSort}
+      ${TH_INCIDENT_SLA_TEST_ID} | ${noneSort} | ${ascSort}  | ${descSort}
     `('updates sort with new direction', async ({ selector, initialSort, firstSort, nextSort }) => {
       const [[attr, value]] = Object.entries(selector);
       const columnHeader = () => wrapper.find(`[${attr}="${value}"]`);