[Security Solution] Remove the advanced sorting switch from the rules…

… management page (#149840)

**Resolves: #138907
**Resolves: #148196

## Summary

- Added missing `outcomeOrder` field to the alerting framework
- Removed the advanced sorting switch from the rules management page
- Both rules management and rules monitoring tables now have all columns
sortable, including indexing time, query time, last gap, and last
response
- Sorting now happens entirely on the server side. No in-memory
structures are needed.
- Removed the version column as intended in
#148196

There's one caveat, though. The running status is represented as a
different rule field. So when sorting by the last status, if there are
any "running" rules, they could appear in any row on the table:

![Screenshot 2023-01-31 at 16 18
29](https://user-images.githubusercontent.com/1938181/216108688-6abe9cb9-3307-4b72-9b6e-27a485a130d5.png)

x-pack/plugins/alerting/common/rule.ts

@@ -35,6 +35,12 @@ export type RuleExecutionStatuses = typeof RuleExecutionStatusValues[number];
 export const RuleLastRunOutcomeValues = ['succeeded', 'warning', 'failed'] as const;
 export type RuleLastRunOutcomes = typeof RuleLastRunOutcomeValues[number];
 
+export const RuleLastRunOutcomeOrderMap: Record<RuleLastRunOutcomes, number> = {
+  succeeded: 0,
+  warning: 10,
+  failed: 20,
+};
+
 export enum RuleExecutionStatusErrorReasons {
   Read = 'read',
   Decrypt = 'decrypt',
@@ -93,6 +99,7 @@ export interface RuleAggregations {
 
 export interface RuleLastRun {
   outcome: RuleLastRunOutcomes;
+  outcomeOrder?: number;
   warning?: RuleExecutionStatusErrorReasons | RuleExecutionStatusWarningReasons | null;
   outcomeMsg?: string[] | null;
   alertsCount: {

x-pack/plugins/alerting/public/lib/common_transformations.test.ts

@@ -81,6 +81,7 @@ describe('common_transformations', () => {
       },
       last_run: {
         outcome: RuleLastRunOutcomeValues[2],
+        outcome_order: 20,
         outcome_msg: ['this is just a test'],
         warning: RuleExecutionStatusErrorReasons.Unknown,
         alerts_count: {
@@ -137,6 +138,7 @@ describe('common_transformations', () => {
           "outcomeMsg": Array [
             "this is just a test",
           ],
+          "outcomeOrder": 20,
           "warning": "unknown",
         },
         "monitoring": Object {
@@ -255,6 +257,7 @@ describe('common_transformations', () => {
       },
       last_run: {
         outcome: 'failed',
+        outcome_order: 20,
         outcome_msg: ['this is just a test'],
         warning: RuleExecutionStatusErrorReasons.Unknown,
         alerts_count: {
@@ -300,6 +303,7 @@ describe('common_transformations', () => {
           "outcomeMsg": Array [
             "this is just a test",
           ],
+          "outcomeOrder": 20,
           "warning": "unknown",
         },
         "monitoring": Object {

x-pack/plugins/alerting/public/lib/common_transformations.ts

@@ -63,10 +63,16 @@ function transformMonitoring(input: RuleMonitoring): RuleMonitoring {
 }
 
 function transformLastRun(input: AsApiContract<RuleLastRun>): RuleLastRun {
-  const { outcome_msg: outcomeMsg, alerts_count: alertsCount, ...rest } = input;
+  const {
+    outcome_msg: outcomeMsg,
+    alerts_count: alertsCount,
+    outcome_order: outcomeOrder,
+    ...rest
+  } = input;
   return {
     outcomeMsg,
     alertsCount,
+    outcomeOrder,
     ...rest,
   };
 }

x-pack/plugins/alerting/server/lib/last_run_status.ts

@@ -8,7 +8,7 @@
 import { RuleTaskStateAndMetrics } from '../task_runner/types';
 import { getReasonFromError } from './error_with_reason';
 import { getEsErrorMessage } from './errors';
-import { ActionsCompletion, RuleLastRunOutcomes } from '../../common';
+import { ActionsCompletion, RuleLastRunOutcomeOrderMap, RuleLastRunOutcomes } from '../../common';
 import {
   RuleLastRunOutcomeValues,
   RuleExecutionStatusWarningReasons,
@@ -65,6 +65,7 @@ export const lastRunFromState = (
   return {
     lastRun: {
       outcome,
+      outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
       outcomeMsg: outcomeMsg.length > 0 ? outcomeMsg : null,
       warning: warning || null,
       alertsCount: {
@@ -80,9 +81,11 @@ export const lastRunFromState = (
 
 export const lastRunFromError = (error: Error): ILastRun => {
   const esErrorMessage = getEsErrorMessage(error);
+  const outcome = RuleLastRunOutcomeValues[2];
   return {
     lastRun: {
-      outcome: RuleLastRunOutcomeValues[2],
+      outcome,
+      outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
       warning: getReasonFromError(error),
       outcomeMsg: esErrorMessage ? [esErrorMessage] : null,
       alertsCount: {},
@@ -104,5 +107,6 @@ export const lastRunToRaw = (lastRun: ILastRun['lastRun']): RawRuleLastRun => {
     },
     warning: warning ?? null,
     outcomeMsg: outcomeMsg && !Array.isArray(outcomeMsg) ? [outcomeMsg] : outcomeMsg,
+    outcomeOrder: RuleLastRunOutcomeOrderMap[lastRun.outcome],
   };
 };

x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts

@@ -9,10 +9,11 @@ import { omit } from 'lodash';
 import { RuleTypeParams, SanitizedRule, RuleLastRun } from '../../types';
 
 export const rewriteRuleLastRun = (lastRun: RuleLastRun) => {
-  const { outcomeMsg, alertsCount, ...rest } = lastRun;
+  const { outcomeMsg, outcomeOrder, alertsCount, ...rest } = lastRun;
   return {
     alerts_count: alertsCount,
     outcome_msg: outcomeMsg,
+    outcome_order: outcomeOrder,
     ...rest,
   };
 };

x-pack/plugins/alerting/server/saved_objects/migrations/8.7/index.ts

@@ -14,7 +14,7 @@ import {
   isLogThresholdRuleType,
   pipeMigrations,
 } from '../utils';
-import { RawRule } from '../../../types';
+import { RawRule, RuleLastRunOutcomeOrderMap } from '../../../types';
 
 function addGroupByToEsQueryRule(
   doc: SavedObjectUnsanitizedDoc<RawRule>
@@ -70,9 +70,29 @@ function addLogViewRefToLogThresholdRule(
   return doc;
 }
 
+function addOutcomeOrder(
+  doc: SavedObjectUnsanitizedDoc<RawRule>
+): SavedObjectUnsanitizedDoc<RawRule> {
+  if (!doc.attributes.lastRun) {
+    return doc;
+  }
+
+  const outcome = doc.attributes.lastRun.outcome;
+  return {
+    ...doc,
+    attributes: {
+      ...doc.attributes,
+      lastRun: {
+        ...doc.attributes.lastRun,
+        outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
+      },
+    },
+  };
+}
+
 export const getMigrations870 = (encryptedSavedObjects: EncryptedSavedObjectsPluginSetup) =>
   createEsoMigration(
     encryptedSavedObjects,
     (doc: SavedObjectUnsanitizedDoc<RawRule>): doc is SavedObjectUnsanitizedDoc<RawRule> => true,
-    pipeMigrations(addGroupByToEsQueryRule, addLogViewRefToLogThresholdRule)
+    pipeMigrations(addGroupByToEsQueryRule, addLogViewRefToLogThresholdRule, addOutcomeOrder)
   );

x-pack/plugins/alerting/server/saved_objects/migrations/index.test.ts

@@ -2606,6 +2606,28 @@ describe('successful migrations', () => {
         expect(migratedAlert870.references).toEqual([]);
       });
     });
+
+    test('migrates last run outcome order', () => {
+      const migration870 = getMigrations(encryptedSavedObjectsSetup, {}, isPreconfigured)['8.7.0'];
+
+      // Failed rule
+      const failedRule = getMockData({ lastRun: { outcome: 'failed' } });
+      const failedRule870 = migration870(failedRule, migrationContext);
+      expect(failedRule870.attributes.lastRun).toEqual({ outcome: 'failed', outcomeOrder: 20 });
+
+      // Rule with warnings
+      const warningRule = getMockData({ lastRun: { outcome: 'warning' } });
+      const warningRule870 = migration870(warningRule, migrationContext);
+      expect(warningRule870.attributes.lastRun).toEqual({ outcome: 'warning', outcomeOrder: 10 });
+
+      // Succeeded rule
+      const succeededRule = getMockData({ lastRun: { outcome: 'succeeded' } });
+      const succeededRule870 = migration870(succeededRule, migrationContext);
+      expect(succeededRule870.attributes.lastRun).toEqual({
+        outcome: 'succeeded',
+        outcomeOrder: 0,
+      });
+    });
   });
 
   describe('Metrics Inventory Threshold rule', () => {

x-pack/plugins/alerting/server/task_runner/fixtures.ts

@@ -6,7 +6,14 @@
  */
 
 import { TaskStatus } from '@kbn/task-manager-plugin/server';
-import { Rule, RuleTypeParams, RecoveredActionGroup, RuleMonitoring } from '../../common';
+import {
+  Rule,
+  RuleTypeParams,
+  RecoveredActionGroup,
+  RuleMonitoring,
+  RuleLastRunOutcomeOrderMap,
+  RuleLastRunOutcomes,
+} from '../../common';
 import { getDefaultMonitoring } from '../lib/monitoring';
 import { UntypedNormalizedRuleType } from '../rule_type_registry';
 import { EVENT_LOG_ACTIONS } from '../plugin';
@@ -74,7 +81,7 @@ export const generateSavedObjectParams = ({
   error?: null | { reason: string; message: string };
   warning?: null | { reason: string; message: string };
   status?: string;
-  outcome?: string;
+  outcome?: RuleLastRunOutcomes;
   nextRun?: string | null;
   successRatio?: number;
   history?: RuleMonitoring['run']['history'];
@@ -110,6 +117,7 @@ export const generateSavedObjectParams = ({
     },
     lastRun: {
       outcome,
+      outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
       outcomeMsg:
         (error?.message && [error?.message]) || (warning?.message && [warning?.message]) || null,
       warning: error?.reason || warning?.reason || null,

x-pack/plugins/alerting/server/task_runner/task_runner.test.ts

@@ -279,7 +279,7 @@ describe('Task Runner', () => {
     );
     expect(logger.debug).nthCalledWith(
       3,
-      'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
+      'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
     );
     expect(logger.debug).nthCalledWith(
       4,
@@ -360,7 +360,7 @@ describe('Task Runner', () => {
       );
       expect(logger.debug).nthCalledWith(
         4,
-        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
+        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
       );
       expect(logger.debug).nthCalledWith(
         5,
@@ -447,7 +447,7 @@ describe('Task Runner', () => {
     );
     expect(logger.debug).nthCalledWith(
       5,
-      'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
+      'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
     );
     expect(logger.debug).nthCalledWith(
       6,
@@ -627,7 +627,7 @@ describe('Task Runner', () => {
       );
       expect(logger.debug).nthCalledWith(
         5,
-        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":2,"new":2,"recovered":0,"ignored":0}}'
+        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":2,"new":2,"recovered":0,"ignored":0}}'
       );
       expect(logger.debug).nthCalledWith(
         6,
@@ -1066,7 +1066,7 @@ describe('Task Runner', () => {
       );
       expect(logger.debug).nthCalledWith(
         5,
-        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
+        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
       );
       expect(logger.debug).nthCalledWith(
         6,
@@ -1192,7 +1192,7 @@ describe('Task Runner', () => {
       );
       expect(logger.debug).nthCalledWith(
         5,
-        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
+        'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
       );
       expect(logger.debug).nthCalledWith(
         6,
@@ -2330,7 +2330,7 @@ describe('Task Runner', () => {
     );
     expect(logger.debug).nthCalledWith(
       3,
-      'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
+      'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
     );
     expect(logger.debug).nthCalledWith(
       4,

x-pack/plugins/alerting/server/task_runner/task_runner.ts

@@ -45,6 +45,7 @@ import {
   RuleTypeState,
   parseDuration,
   RawAlertInstance,
+  RuleLastRunOutcomeOrderMap,
 } from '../../common';
 import { NormalizedRuleType, UntypedNormalizedRuleType } from '../rule_type_registry';
 import { getEsErrorMessage } from '../lib/errors';
@@ -825,10 +826,12 @@ export class TaskRunner<
     this.logger.debug(
       `Updating rule task for ${this.ruleType.id} rule with id ${ruleId} - execution error due to timeout`
     );
+    const outcome = 'failed';
     await this.updateRuleSavedObjectPostRun(ruleId, namespace, {
       executionStatus: ruleExecutionStatusToRaw(executionStatus),
       lastRun: {
-        outcome: 'failed',
+        outcome,
+        outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
         warning: RuleExecutionStatusErrorReasons.Timeout,
         outcomeMsg,
         alertsCount: {},

x-pack/plugins/alerting/server/task_runner/task_runner_cancel.test.ts

@@ -235,6 +235,7 @@ describe('Task Runner Cancel', () => {
           outcomeMsg: [
             'test:1: execution cancelled due to timeout - exceeded rule type timeout of 5m',
           ],
+          outcomeOrder: 20,
           warning: 'timeout',
         },
         monitoring: {

x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/find_rules/request_schema.test.ts

@@ -29,7 +29,7 @@ describe('Find rules request schema', () => {
     const payload: FindRulesRequestQuery = {
       per_page: 5,
       page: 1,
-      sort_field: 'some field',
+      sort_field: 'name',
       fields: ['field 1', 'field 2'],
       filter: 'some filter',
       sort_order: 'asc',
@@ -80,14 +80,14 @@ describe('Find rules request schema', () => {
 
   test('sort_field validates', () => {
     const payload: FindRulesRequestQuery = {
-      sort_field: 'value',
+      sort_field: 'name',
     };
 
     const decoded = FindRulesRequestQuery.decode(payload);
     const checked = exactCheck(payload, decoded);
     const message = pipe(checked, foldLeftRight);
     expect(getPaths(left(message.errors))).toEqual([]);
-    expect((message.schema as FindRulesRequestQuery).sort_field).toEqual('value');
+    expect((message.schema as FindRulesRequestQuery).sort_field).toEqual('name');
   });
 
   test('fields validates with a string', () => {
@@ -173,7 +173,7 @@ describe('Find rules request schema', () => {
   test('sort_order validates with desc and sort_field', () => {
     const payload: FindRulesRequestQuery = {
       sort_order: 'desc',
-      sort_field: 'some field',
+      sort_field: 'name',
     };
 
     const decoded = FindRulesRequestQuery.decode(payload);
@@ -187,7 +187,7 @@ describe('Find rules request schema', () => {
   test('sort_order does not validate with a string other than asc and desc', () => {
     const payload: Omit<FindRulesRequestQuery, 'sort_order'> & { sort_order: string } = {
       sort_order: 'some other string',
-      sort_field: 'some field',
+      sort_field: 'name',
     };
 
     const decoded = FindRulesRequestQuery.decode(payload);

x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/find_rules/request_schema.ts

@@ -8,7 +8,28 @@
 import * as t from 'io-ts';
 import { DefaultPerPage, DefaultPage } from '@kbn/securitysolution-io-ts-alerting-types';
 import type { PerPage, Page } from '../../../../schemas/common';
-import { queryFilter, fields, SortField, SortOrder } from '../../../../schemas/common';
+import { queryFilter, fields, SortOrder } from '../../../../schemas/common';
+
+export type FindRulesSortField = t.TypeOf<typeof FindRulesSortField>;
+export const FindRulesSortField = t.union([
+  t.literal('created_at'),
+  t.literal('createdAt'), // Legacy notation, keeping for backwards compatibility
+  t.literal('enabled'),
+  t.literal('execution_summary.last_execution.date'),
+  t.literal('execution_summary.last_execution.metrics.execution_gap_duration_s'),
+  t.literal('execution_summary.last_execution.metrics.total_indexing_duration_ms'),
+  t.literal('execution_summary.last_execution.metrics.total_search_duration_ms'),
+  t.literal('execution_summary.last_execution.status'),
+  t.literal('name'),
+  t.literal('risk_score'),
+  t.literal('riskScore'), // Legacy notation, keeping for backwards compatibility
+  t.literal('severity'),
+  t.literal('updated_at'),
+  t.literal('updatedAt'), // Legacy notation, keeping for backwards compatibility
+]);
+
+export type FindRulesSortFieldOrUndefined = t.TypeOf<typeof FindRulesSortFieldOrUndefined>;
+export const FindRulesSortFieldOrUndefined = t.union([FindRulesSortField, t.undefined]);
 
 /**
  * Query string parameters of the API route.
@@ -18,7 +39,7 @@ export const FindRulesRequestQuery = t.exact(
   t.partial({
     fields,
     filter: queryFilter,
-    sort_field: SortField,
+    sort_field: FindRulesSortField,
     sort_order: SortOrder,
     page: DefaultPage, // defaults to 1
     per_page: DefaultPerPage, // defaults to 20