[Security Solution]Fix incorrect number of invalid connectors is show…

…n on the toast message (elastic#152313) ## Summary - Addresses elastic#151988 Before the fix, the number of missing connectors was always 1, even if the message says the invalid connectors are more ![image](https://user-images.githubusercontent.com/12671903/221798261-8fb7ba28-3a1a-4269-9e00-a540f7febb1b.png) After, ![image](https://user-images.githubusercontent.com/12671903/221798298-1f3728b2-d7a9-41c9-a0f4-d80892a7c0f1.png) ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <[email protected]>
bmorelli25 · Mar 10, 2023 · b0d10b3 · b0d10b3
1 parent 63c6cea
commit b0d10b3
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 5 deletions.
diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/utils.test.ts b/x-pack/plugins/security_solution/public/common/components/import_data_modal/utils.test.ts
@@ -306,7 +306,102 @@ describe('showToasterMessage', () => {
       expect(addSuccess).toHaveBeenNthCalledWith(2, 'Successfully imported 1 connector.');
       expect(addError).not.toHaveBeenCalled();
     });
+    it('should display 1 error message has 2 invalid connectors in the title even when error array has one message but "id" field', () => {
+      const addError = jest.fn();
+      const addSuccess = jest.fn();
 
+      showToasterMessage({
+        importResponse: {
+          success: false,
+          success_count: 1,
+          rules_count: 2,
+          action_connectors_success: false,
+          errors: [
+            {
+              rule_id: 'rule_id',
+              error: {
+                status_code: 400,
+                message: 'an error message',
+              },
+            },
+          ],
+          action_connectors_errors: [
+            {
+              rule_id: 'rule_id',
+              id: 'connector1,connector2',
+              error: {
+                status_code: 400,
+                message: 'an error message',
+              },
+            },
+          ],
+          exceptions_success: true,
+          exceptions_success_count: 0,
+        },
+        exceptionsIncluded: false,
+        actionConnectorsIncluded: true,
+        successMessage: (msg) => `success: ${msg}`,
+        errorMessage: (msg) => `error: ${msg}`,
+        errorMessageDetailed: (msg) => `errorDetailed: ${msg}`,
+        addError,
+        addSuccess,
+      });
+
+      expect(addError).toHaveBeenCalledTimes(1);
+
+      expect(addError).toHaveBeenCalledWith(new Error('errorDetailed: an error message'), {
+        title: 'Failed to import 2 connectors',
+      });
+      expect(addSuccess).not.toHaveBeenCalled();
+    });
+    it('should display 1 error message has 1 invalid connectors in the title even when error array has one message but "id" field', () => {
+      const addError = jest.fn();
+      const addSuccess = jest.fn();
+
+      showToasterMessage({
+        importResponse: {
+          success: false,
+          success_count: 1,
+          rules_count: 2,
+          action_connectors_success: false,
+          errors: [
+            {
+              rule_id: 'rule_id',
+              error: {
+                status_code: 400,
+                message: 'an error message',
+              },
+            },
+          ],
+          action_connectors_errors: [
+            {
+              rule_id: 'rule_id',
+              id: 'connector1',
+              error: {
+                status_code: 400,
+                message: 'an error message',
+              },
+            },
+          ],
+          exceptions_success: true,
+          exceptions_success_count: 0,
+        },
+        exceptionsIncluded: false,
+        actionConnectorsIncluded: true,
+        successMessage: (msg) => `success: ${msg}`,
+        errorMessage: (msg) => `error: ${msg}`,
+        errorMessageDetailed: (msg) => `errorDetailed: ${msg}`,
+        addError,
+        addSuccess,
+      });
+
+      expect(addError).toHaveBeenCalledTimes(1);
+
+      expect(addError).toHaveBeenCalledWith(new Error('errorDetailed: an error message'), {
+        title: 'Failed to import 1 connector',
+      });
+      expect(addSuccess).not.toHaveBeenCalled();
+    });
     it('should display 1 error message for rules and connectors even when both fail', () => {
       const addError = jest.fn();
       const addSuccess = jest.fn();

diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/utils.ts b/x-pack/plugins/security_solution/public/common/components/import_data_modal/utils.ts
@@ -33,8 +33,14 @@ export const formatError = (
 const mapErrorMessageToUserMessage = (
   actionConnectorsErrors: Array<ImportRulesResponseError | ImportResponseError>
 ) => {
-  return actionConnectorsErrors.map((connectorError) => {
-    const { error } = connectorError;
+  let concatenatedActionIds: string = '';
+  const mappedErrors = actionConnectorsErrors.map((connectorError) => {
+    // Using "as ImportResponseError" because the "id" field belongs only to
+    // "ImportResponseError" and if the connectorError has the id we use it to get the
+    // number of failing connectors by spliting the unique the connectors ids.
+    const { id, error } = connectorError as ImportResponseError;
+    concatenatedActionIds =
+      concatenatedActionIds && concatenatedActionIds !== id ? `${concatenatedActionIds},${id}` : id;
     const { status_code: statusCode, message: originalMessage } = error || {};
     let message;
     switch (statusCode) {
@@ -47,9 +53,12 @@ const mapErrorMessageToUserMessage = (
 
         break;
     }
-
     return { ...connectorError, error: { ...error, message } };
   });
+  const actionIds: Set<string> = new Set(
+    concatenatedActionIds && [...concatenatedActionIds.split(',')]
+  );
+  return { mappedErrors, numberOfActions: actionIds.size };
 };
 
 export const showToasterMessage = ({
@@ -100,13 +109,13 @@ export const showToasterMessage = ({
       importResponse.action_connectors_errors != null &&
       importResponse.action_connectors_errors.length > 0
     ) {
-      const userErrorMessages = mapErrorMessageToUserMessage(
+      const { mappedErrors: userErrorMessages, numberOfActions } = mapErrorMessageToUserMessage(
         importResponse.action_connectors_errors
       );
       const connectorError = formatError(errorMessageDetailed, importResponse, userErrorMessages);
 
       return addError(connectorError, {
-        title: i18n.IMPORT_CONNECTORS_FAILED(userErrorMessages.length),
+        title: i18n.IMPORT_CONNECTORS_FAILED(numberOfActions || userErrorMessages.length),
       });
     }
     const ruleError = formatError(errorMessageDetailed, importResponse, importResponse.errors);

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/utils.ts
@@ -21,7 +21,9 @@ export interface OutputError {
   statusCode: number;
 }
 export interface BulkError {
+  // Id can be single id or stringified ids.
   id?: string;
+  // rule_id can be single rule_id or stringified rules ids.
   rule_id?: string;
   error: {
     status_code: number;

diff --git a/...gine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts b/...gine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts
@@ -174,6 +174,7 @@ describe('importRuleActionConnectors', () => {
               '1 connector is missing. Connector id missing is: cabc78e0-9031-11ed-b076-53cc4d57aaf1',
             status_code: 404,
           },
+          id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1',
           rule_id: 'rule-1',
         },
       ],
@@ -220,6 +221,7 @@ describe('importRuleActionConnectors', () => {
             status_code: 404,
           },
           rule_id: 'rule-1',
+          id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1,cabc78e0-9031-11ed-b076-53cc4d57aaf2',
         },
       ],
       warnings: [],
@@ -270,6 +272,7 @@ describe('importRuleActionConnectors', () => {
             status_code: 404,
           },
           rule_id: 'rule-1,rule-2',
+          id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1,cabc78e0-9031-11ed-b076-53cc4d57aaf2',
         },
       ],
       warnings: [],

diff --git a/...server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts b/...server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts
@@ -43,6 +43,7 @@ export const handleActionsHaveNoConnectors = (
         : 'connector is missing. Connector id missing is:';
     errors.push(
       createBulkErrorObject({
+        id: actionsIds.join(),
         statusCode: 404,
         message: `${actionsIds.length} ${errorMessage} ${actionsIds.join(', ')}`,
         ruleId: ruleIds,

diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts
@@ -866,6 +866,7 @@ export default ({ getService }: FtrProviderContext): void => {
           errors: [
             {
               rule_id: 'rule-1',
+              id: '123',
               error: {
                 status_code: 404,
                 message: '1 connector is missing. Connector id missing is: 123',
@@ -881,6 +882,7 @@ export default ({ getService }: FtrProviderContext): void => {
           action_connectors_errors: [
             {
               rule_id: 'rule-1',
+              id: '123',
               error: {
                 status_code: 404,
                 message: '1 connector is missing. Connector id missing is: 123',
@@ -1153,6 +1155,7 @@ export default ({ getService }: FtrProviderContext): void => {
           errors: [
             {
               rule_id: 'rule-2',
+              id: 'cabc78e0-9031-11ed-b076-53cc4d57aa22',
               error: {
                 status_code: 404,
                 message:
@@ -1173,6 +1176,7 @@ export default ({ getService }: FtrProviderContext): void => {
                   '1 connector is missing. Connector id missing is: cabc78e0-9031-11ed-b076-53cc4d57aa22',
               },
               rule_id: 'rule-2',
+              id: 'cabc78e0-9031-11ed-b076-53cc4d57aa22',
             },
           ],
           action_connectors_warnings: [],