From c75409b2a45ce8a448fcaa0c4f98386fc025afb9 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Thu, 23 Feb 2023 19:13:08 +0000 Subject: [PATCH 1/7] initial --- .../common/components/import_data_modal/utils.ts | 12 ++++++++---- .../logic/import/action_connectors/utils/index.ts | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) 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 index 92c6f9f666e4b..679b42daeb701 100644 --- 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,10 @@ export const formatError = ( const mapErrorMessageToUserMessage = ( actionConnectorsErrors: Array ) => { - return actionConnectorsErrors.map((connectorError) => { - const { error } = connectorError; + let actionIds: string[] | '' = []; + const mappedErrors = actionConnectorsErrors.map((connectorError) => { + const { id, error } = connectorError as ImportResponseError; + actionIds = id && [...id.split(',')]; const { status_code: statusCode, message: originalMessage } = error || {}; let message; switch (statusCode) { @@ -50,6 +52,8 @@ const mapErrorMessageToUserMessage = ( return { ...connectorError, error: { ...error, message } }; }); + const numberOfAction = Array.isArray(actionIds) ? actionIds.length : actionIds; + return { mappedErrors, numberOfAction }; }; export const showToasterMessage = ({ @@ -100,13 +104,13 @@ export const showToasterMessage = ({ importResponse.action_connectors_errors != null && importResponse.action_connectors_errors.length > 0 ) { - const userErrorMessages = mapErrorMessageToUserMessage( + const { mappedErrors: userErrorMessages, numberOfAction } = 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(numberOfAction || userErrorMessages.length), }); } const ruleError = formatError(errorMessageDetailed, importResponse, importResponse.errors); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts index 6d9b3b9da9e6d..cecee94ffa5ad 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts @@ -40,6 +40,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, From 443447141dca3704a853078166306d8bd6fb6acd Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 28 Feb 2023 08:33:18 +0000 Subject: [PATCH 2/7] fix incorrect number of connectors in the toast title --- .../import_data_modal/utils.test.ts | 95 +++++++++++++++++++ .../components/import_data_modal/utils.ts | 12 ++- 2 files changed, 102 insertions(+), 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 index 6b432edfb5bd1..132563ee4fd8c 100644 --- 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 with the 2 invalid connectors even the error array has one message because the "id" was passed', () => { + 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 with the 1 invalid connector when the "id" was passed with 1 id', () => { + 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 index 679b42daeb701..5761de982d129 100644 --- 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,10 +33,11 @@ export const formatError = ( const mapErrorMessageToUserMessage = ( actionConnectorsErrors: Array ) => { - let actionIds: string[] | '' = []; + let concatenatedActionIds: string = ''; const mappedErrors = actionConnectorsErrors.map((connectorError) => { const { id, error } = connectorError as ImportResponseError; - actionIds = id && [...id.split(',')]; + concatenatedActionIds = + concatenatedActionIds && concatenatedActionIds !== id ? `${concatenatedActionIds},${id}` : id; const { status_code: statusCode, message: originalMessage } = error || {}; let message; switch (statusCode) { @@ -49,11 +50,12 @@ const mapErrorMessageToUserMessage = ( break; } - return { ...connectorError, error: { ...error, message } }; }); - const numberOfAction = Array.isArray(actionIds) ? actionIds.length : actionIds; - return { mappedErrors, numberOfAction }; + const actionIds: Set = new Set( + concatenatedActionIds && [...concatenatedActionIds.split(',')] + ); + return { mappedErrors, numberOfAction: actionIds.size }; }; export const showToasterMessage = ({ From 7ade2cca2ef66501ed59a31e14538f9e4ceff549 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 28 Feb 2023 08:39:45 +0000 Subject: [PATCH 3/7] change variable name --- .../public/common/components/import_data_modal/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index 5761de982d129..d906b6cf7daea 100644 --- 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 @@ -55,7 +55,7 @@ const mapErrorMessageToUserMessage = ( const actionIds: Set = new Set( concatenatedActionIds && [...concatenatedActionIds.split(',')] ); - return { mappedErrors, numberOfAction: actionIds.size }; + return { mappedErrors, numberOfActions: actionIds.size }; }; export const showToasterMessage = ({ @@ -106,13 +106,13 @@ export const showToasterMessage = ({ importResponse.action_connectors_errors != null && importResponse.action_connectors_errors.length > 0 ) { - const { mappedErrors: userErrorMessages, numberOfAction } = mapErrorMessageToUserMessage( + const { mappedErrors: userErrorMessages, numberOfActions } = mapErrorMessageToUserMessage( importResponse.action_connectors_errors ); const connectorError = formatError(errorMessageDetailed, importResponse, userErrorMessages); return addError(connectorError, { - title: i18n.IMPORT_CONNECTORS_FAILED(numberOfAction || userErrorMessages.length), + title: i18n.IMPORT_CONNECTORS_FAILED(numberOfActions || userErrorMessages.length), }); } const ruleError = formatError(errorMessageDetailed, importResponse, importResponse.errors); From 9f710a273a4815a03d711622a7a0bb5af08233dc Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 28 Feb 2023 08:45:40 +0000 Subject: [PATCH 4/7] add des --- .../public/common/components/import_data_modal/utils.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 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 index 132563ee4fd8c..376c7fbf66d39 100644 --- 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,7 @@ describe('showToasterMessage', () => { expect(addSuccess).toHaveBeenNthCalledWith(2, 'Successfully imported 1 connector.'); expect(addError).not.toHaveBeenCalled(); }); - it('should display 1 error message with the 2 invalid connectors even the error array has one message because the "id" was passed', () => { + 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(); @@ -354,7 +354,7 @@ describe('showToasterMessage', () => { }); expect(addSuccess).not.toHaveBeenCalled(); }); - it('should display 1 error message with the 1 invalid connector when the "id" was passed with 1 id', () => { + 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(); From 4d2e9a13ee01f2ee523e6655fa933a887c7229b7 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 28 Feb 2023 13:12:55 +0000 Subject: [PATCH 5/7] tests --- .../action_connectors/import_rule_action_connectors.test.ts | 5 ++++- .../security_and_spaces/group10/import_rules.ts | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts index 455274006297e..6c732d4057313 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts @@ -36,7 +36,7 @@ const actionsClient = actionsClientMock.create(); actionsClient.getAll.mockResolvedValue([]); const core = coreMock.createRequestHandlerContext(); -describe('checkRuleExceptionReferences', () => { +describe('importRuleActionConnectors', () => { beforeEach(() => { jest.clearAllMocks(); }); @@ -178,6 +178,7 @@ describe('checkRuleExceptionReferences', () => { '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', }, ], @@ -224,6 +225,7 @@ describe('checkRuleExceptionReferences', () => { status_code: 404, }, rule_id: 'rule-1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1,cabc78e0-9031-11ed-b076-53cc4d57aaf2', }, ], warnings: [], @@ -274,6 +276,7 @@ describe('checkRuleExceptionReferences', () => { status_code: 404, }, rule_id: 'rule-1,rule-2', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1,cabc78e0-9031-11ed-b076-53cc4d57aaf2', }, ], warnings: [], 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 index 41b49ac35bfc6..dd82fe27e726f 100644 --- 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 @@ -788,6 +788,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', @@ -803,6 +804,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', @@ -1075,6 +1077,7 @@ export default ({ getService }: FtrProviderContext): void => { errors: [ { rule_id: 'rule-2', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aa22', error: { status_code: 404, message: @@ -1095,6 +1098,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: [], From a2b46ef8f528e465a220d96ee1288719a92b2b3a Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Thu, 2 Mar 2023 16:09:21 +0000 Subject: [PATCH 6/7] explain the type casting in mapErrorMessage --- .../public/common/components/import_data_modal/utils.ts | 3 +++ 1 file changed, 3 insertions(+) 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 index d906b6cf7daea..4ab30388168df 100644 --- 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 @@ -35,6 +35,9 @@ const mapErrorMessageToUserMessage = ( ) => { 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; From bc5547c9a5e562b986aec7e6d4e3d7bd4f28b6fb Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Thu, 2 Mar 2023 16:55:59 +0000 Subject: [PATCH 7/7] add description for BulkError id and rule_id --- .../server/lib/detection_engine/routes/utils.ts | 2 ++ 1 file changed, 2 insertions(+) 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 index 7d822733b4da5..a200afdd34199 100644 --- 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;