Addressing PR feedback

x-pack/plugins/cases/public/components/user_actions/comment/alert.test.tsx

@@ -10,22 +10,11 @@ import { SnakeToCamelCase } from '../../../../common/types';
 import { getRuleId, getRuleName } from './alert';
 import { Ecs } from '../../../containers/types';
 
-describe('alert', () => {
+describe('rule getters', () => {
   describe.each([
     ['getRuleId', getRuleId],
     ['getRuleName', getRuleName],
-  ])('%s', (name, funcToExec) => {
-    it('returns the first entry in the comment field', () => {
-      const comment = {
-        rule: {
-          id: ['1', '2'],
-          name: ['1', '2'],
-        },
-      } as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
-
-      expect(funcToExec(comment)).toEqual('1');
-    });
-
+  ])('%s null checks', (name, funcToExec) => {
     it('returns null if the comment field is an empty string', () => {
       const comment = {
         rule: {
@@ -54,78 +43,96 @@ describe('alert', () => {
       expect(funcToExec(comment)).toBeNull();
     });
 
+    it('returns null if the signals and alert field is an empty string', () => {
+      const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
+      const alert = {
+        signal: { rule: { id: '', name: '' } },
+        kibana: { alert: { rule: { uuid: '', name: '' } } },
+      } as unknown as Ecs;
+
+      expect(funcToExec(comment, alert)).toBeNull();
+    });
+  });
+
+  describe.each([
+    ['getRuleId', getRuleId, '1'],
+    ['getRuleName', getRuleName, 'Rule name1'],
+  ])('%s', (name, funcToExec, expectedResult) => {
+    it('returns the first entry in the comment field', () => {
+      const comment = {
+        rule: {
+          id: ['1', '2'],
+          name: ['Rule name1', 'Rule name2'],
+        },
+      } as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
+
+      expect(funcToExec(comment)).toEqual(expectedResult);
+    });
+
     it('returns signal field', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
-      const alert = { signal: { rule: { id: '1', name: '1' } } } as unknown as Ecs;
+      const alert = { signal: { rule: { id: '1', name: 'Rule name1' } } } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('1');
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
 
     it('returns kibana alert field', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
-      const alert = { kibana: { alert: { rule: { uuid: '1', name: '1' } } } } as unknown as Ecs;
+      const alert = {
+        kibana: { alert: { rule: { uuid: '1', name: 'Rule name1' } } },
+      } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('1');
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
 
     it('returns signal field even when kibana alert field is defined', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
       const alert = {
-        signal: { rule: { id: 'signal', name: 'signal' } },
-        kibana: { alert: { rule: { uuid: '1', name: '1' } } },
+        signal: { rule: { id: '1', name: 'Rule name1' } },
+        kibana: { alert: { rule: { uuid: 'rule id1', name: 'other rule name1' } } },
       } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('signal');
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
 
     it('returns the first entry in the signals field', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
       const alert = {
-        signal: { rule: { id: ['signal1', 'signal2'], name: ['signal1', 'signal2'] } },
-        kibana: { alert: { rule: { uuid: '1', name: '1' } } },
+        signal: { rule: { id: '1', name: 'Rule name1' } },
+        kibana: { alert: { rule: { uuid: 'rule id1', name: 'other rule name1' } } },
       } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('signal1');
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
 
     it('returns the alert field if the signals field is an empty string', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
       const alert = {
         signal: { rule: { id: '', name: '' } },
-        kibana: { alert: { rule: { uuid: '1', name: '1' } } },
+        kibana: { alert: { rule: { uuid: '1', name: 'Rule name1' } } },
       } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('1');
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
 
     it('returns the alert field if the signals field is an empty string in an array', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
       const alert = {
         signal: { rule: { id: [''], name: [''] } },
-        kibana: { alert: { rule: { uuid: '1', name: '1' } } },
+        kibana: { alert: { rule: { uuid: '1', name: 'Rule name1' } } },
       } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('1');
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
 
     it('returns the alert field first item if the signals field is an empty string in an array', () => {
       const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
       const alert = {
         signal: { rule: { id: [''], name: [''] } },
-        kibana: { alert: { rule: { uuid: ['1', '2'], name: ['1', '2'] } } },
+        kibana: { alert: { rule: { uuid: ['1', '2'], name: ['Rule name1', 'Rule name2'] } } },
       } as unknown as Ecs;
 
-      expect(funcToExec(comment, alert)).toEqual('1');
-    });
-
-    it('returns null if the signals and alert field is an empty string', () => {
-      const comment = {} as unknown as SnakeToCamelCase<CommentResponseAlertsType>;
-      const alert = {
-        signal: { rule: { id: '', name: '' } },
-        kibana: { alert: { rule: { uuid: '', name: '' } } },
-      } as unknown as Ecs;
-
-      expect(funcToExec(comment, alert)).toBeNull();
+      expect(funcToExec(comment, alert)).toEqual(expectedResult);
     });
   });
 });

x-pack/plugins/cases/public/components/user_actions/comment/alert.tsx

@@ -96,11 +96,7 @@ export const createAlertAttachmentUserActionBuilder = ({
 });
 
 const getFirstItem = (items?: string | string[] | null): string | null => {
-  if (items == null) {
-    return null;
-  }
-
-  return Array.isArray(items) ? (items.length > 0 ? items[0] : null) : items;
+  return Array.isArray(items) ? items[0] : items ?? null;
 };
 
 export const getRuleId = (comment: BuilderArgs['comment'], alertData?: Ecs): string | null =>

x-pack/plugins/cases/server/saved_object_types/migrations/user_actions/alerts.test.ts

@@ -0,0 +1,181 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import {
+  SavedObject,
+  SavedObjectMigrationContext,
+  SavedObjectsMigrationLogger,
+} from 'kibana/server';
+import { cloneDeep, omit } from 'lodash';
+import { migrationMocks } from 'src/core/server/mocks';
+import { removeRuleInformation } from './alerts';
+
+describe('alert user actions', () => {
+  describe('removeRuleInformation', () => {
+    let context: jest.Mocked<SavedObjectMigrationContext>;
+
+    beforeEach(() => {
+      context = migrationMocks.createContext();
+    });
+
+    it('does not modify non-alert user action', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['description'],
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      expect(removeRuleInformation(doc, context)).toEqual(doc);
+    });
+
+    it('does not modify the document when it fails to decode the new_value field', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value: '{"type":"alert",',
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      expect(removeRuleInformation(doc, context)).toEqual(doc);
+
+      const log = context.log as jest.Mocked<SavedObjectsMigrationLogger>;
+      expect(log.error.mock.calls[0]).toMatchInlineSnapshot(`
+          Array [
+            "Failed to migrate user action alerts with doc id: 123 version: 8.0.0 error: Unexpected end of JSON input",
+            Object {
+              "migrations": Object {
+                "userAction": Object {
+                  "id": "123",
+                },
+              },
+            },
+          ]
+        `);
+    });
+
+    it('does not modify the document when new_value is null', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value: null,
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      expect(removeRuleInformation(doc, context)).toEqual(doc);
+    });
+
+    it('does not modify the document when new_value is undefined', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      expect(removeRuleInformation(doc, context)).toEqual(doc);
+    });
+
+    it('does not modify the document when the comment type is not an alert', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value:
+            '{"type":"not_an_alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      expect(removeRuleInformation(doc, context)).toEqual(doc);
+    });
+
+    it('sets the rule fields to null', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value:
+            '{"type":"alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      const newDoc = removeRuleInformation(doc, context) as SavedObject<{ new_value: string }>;
+      ensureRuleFieldsAreNull(newDoc);
+
+      expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
+    });
+
+    it('sets the rule fields to null for a generated alert', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value:
+            '{"type":"generated_alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
+        },
+        type: 'abc',
+        references: [],
+      };
+
+      const newDoc = removeRuleInformation(doc, context) as SavedObject<{ new_value: string }>;
+      ensureRuleFieldsAreNull(newDoc);
+
+      expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
+    });
+
+    it('preserves the references field', () => {
+      const doc = {
+        id: '123',
+        attributes: {
+          action: 'create',
+          action_field: ['comment'],
+          new_value:
+            '{"type":"generated_alert","alertId":"4eb4cd05b85bc65c7b9f22b776e0136f970f7538eb0d1b2e6e8c7d35b2e875cb","index":".internal.alerts-security.alerts-default-000001","rule":{"id":"43104810-7875-11ec-abc6-6f72e72f6004","name":"A rule"},"owner":"securitySolution"}',
+        },
+        type: 'abc',
+        references: [{ id: '123', name: 'hi', type: 'awesome' }],
+      };
+
+      const newDoc = removeRuleInformation(doc, context) as SavedObject<{ new_value: string }>;
+      ensureRuleFieldsAreNull(newDoc);
+
+      expect(docWithoutNewValue(newDoc)).toEqual(docWithoutNewValue(doc));
+    });
+  });
+});
+
+const docWithoutNewValue = (doc: {}) => {
+  const copyOfDoc = cloneDeep(doc);
+  return omit(copyOfDoc, 'attributes.new_value');
+};
+
+const ensureRuleFieldsAreNull = (doc: SavedObject<{ new_value: string }>) => {
+  const decodedNewValue = JSON.parse(doc.attributes.new_value);
+
+  expect(decodedNewValue.rule).toEqual({ id: null, name: null });
+};