Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Cases] Fix pushing alerts count on every push to external service #105030

Merged
merged 3 commits into from
Jul 12, 2021
Merged

[Cases] Fix pushing alerts count on every push to external service #105030

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6428378
Do not push alerts count if not updated
cnasikas Jul 9, 2021
19e5310
Merge branch 'master' into fix_103071
kibanamachine Jul 12, 2021
d0d3fe7
PR feedback
cnasikas Jul 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 28 additions & 19 deletions x-pack/plugins/cases/server/client/cases/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,7 @@ describe('utils', () => {
},
],
},
// Remove second push
userActions: userActions.filter((item, index) => index !== 4),
userActions,
connector,
mappings: [
...mappings,
Expand All @@ -551,7 +550,7 @@ describe('utils', () => {
]);
});

it('it removes alerts correctly', async () => {
it('it filters out the alerts from the comments correctly', async () => {
const res = await createIncident({
actionsClient: actionsMock,
theCase: {
Expand Down Expand Up @@ -582,6 +581,32 @@ describe('utils', () => {
]);
});

it('does not add the alerts count comment if all alerts have been pushed', async () => {
const res = await createIncident({
actionsClient: actionsMock,
theCase: {
...theCase,
comments: [
{ ...commentObj, id: 'comment-user-1', pushed_at: '2019-11-25T21:55:00.177Z' },
{ ...commentGeneratedAlert, pushed_at: '2019-11-25T21:55:00.177Z' },
],
},
userActions,
connector,
mappings,
alerts: [],
casesConnectors,
});

expect(res.comments).toEqual([
{
comment:
'Wow, good luck catching that bad meanie! (added at 2019-11-25T21:55:00.177Z by elastic)',
commentId: 'comment-user-1',
},
]);
});

it('updates an existing incident', async () => {
const existingIncidentData = {
priority: null,
Expand Down Expand Up @@ -644,22 +669,6 @@ describe('utils', () => {
});
});

it('throws error if connector is not supported', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we deleting this one?

Copy link
Member Author

@cnasikas cnasikas Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that test is not valid anymore. The functionality changed in a previous PR. Although the test passes I get the following warning:

(node:27107) UnhandledPromiseRejectionWarning: Error: expect(received).toEqual(expected) // deep equality

Expected: [Error: Invalid external service]
Received: [Error: Retrieving Incident by id external-id from not-supported failed with exception: Error: exception]
(Use `node --trace-warnings ...` to show where the warning was created)
(node:27107) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 146)
(node:27107) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

expect.assertions(2);
createIncident({
actionsClient: actionsMock,
theCase,
userActions,
connector: { ...connector, actionTypeId: 'not-supported' },
mappings,
alerts: [],
casesConnectors,
}).catch((e) => {
expect(e).not.toBeNull();
expect(e).toEqual(new Error('Invalid external service'));
});
});

describe('getLatestPushInfo', () => {
it('it returns the latest push information correctly', async () => {
const res = getLatestPushInfo('456', userActions);
Expand Down
36 changes: 27 additions & 9 deletions x-pack/plugins/cases/server/client/cases/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,29 @@ const getCommentContent = (comment: CommentResponse): string => {
return '';
};

const countAlerts = (comments: CaseResponse['comments']): number =>
comments?.reduce<number>((total, comment) => {
if (comment.type === CommentType.alert || comment.type === CommentType.generatedAlert) {
return total + (Array.isArray(comment.alertId) ? comment.alertId.length : 1);
}
return total;
}, 0) ?? 0;
interface CountAlertsInfo {
total: number;
pushed: number;
totalAlerts: number;
}

const countAlerts = (comments: CaseResponse['comments']): CountAlertsInfo => {
cnasikas marked this conversation as resolved.
Show resolved Hide resolved
const countingInfo = { total: 0, pushed: 0, totalAlerts: 0 };
return (
comments?.reduce<CountAlertsInfo>(({ total, pushed, totalAlerts }, comment) => {
if (comment.type === CommentType.alert || comment.type === CommentType.generatedAlert) {
return {
// We could use ++total but total + 1 is more readable.
total: total + 1,
// We could use ++pushed but pushed + 1 is more readable.
cnasikas marked this conversation as resolved.
Show resolved Hide resolved
pushed: comment.pushed_at != null ? pushed + 1 : pushed,
totalAlerts: totalAlerts + (Array.isArray(comment.alertId) ? comment.alertId.length : 1),
};
}
return { total, pushed, totalAlerts };
}, countingInfo) ?? countingInfo
);
};

export const createIncident = async ({
actionsClient,
Expand Down Expand Up @@ -164,7 +180,9 @@ export const createIncident = async ({
comment.type === CommentType.user && commentsIdsToBeUpdated.has(comment.id)
);

const totalAlerts = countAlerts(caseComments);
const { total: totalCommentsTypeAlert, pushed: pushedAlerts, totalAlerts } = countAlerts(
caseComments
);

let comments: ExternalServiceComment[] = [];

Expand All @@ -175,7 +193,7 @@ export const createIncident = async ({
}
}

if (totalAlerts > 0) {
if (totalCommentsTypeAlert > pushedAlerts) {
cnasikas marked this conversation as resolved.
Show resolved Hide resolved
comments.push({
comment: `Elastic Alerts attached to the case: ${totalAlerts}`,
commentId: `${theCase.id}-total-alerts`,
Expand Down