-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[Security Solution] Alerts Treemap and Multi-Dimensional Alert Grouping #126896
Changes from all commits
137204e
1df7808
74acd52
562f9e7
f11636d
49d27fe
848f8da
2a43fa0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,17 +9,20 @@ import { | |
ADD_EXCEPTION_BTN, | ||
ALERT_RISK_SCORE_HEADER, | ||
ALERT_CHECKBOX, | ||
CHART_SELECT, | ||
CLOSE_ALERT_BTN, | ||
CLOSE_SELECTED_ALERTS_BTN, | ||
CLOSED_ALERTS_FILTER_BTN, | ||
EXPAND_ALERT_BTN, | ||
GROUP_BY_TOP_INPUT, | ||
ACKNOWLEDGED_ALERTS_FILTER_BTN, | ||
LOADING_ALERTS_PANEL, | ||
MANAGE_ALERT_DETECTION_RULES_BTN, | ||
MARK_ALERT_ACKNOWLEDGED_BTN, | ||
OPEN_ALERT_BTN, | ||
OPENED_ALERTS_FILTER_BTN, | ||
SEND_ALERT_TO_TIMELINE_BTN, | ||
SELECT_TABLE, | ||
TAKE_ACTION_POPOVER_BTN, | ||
TIMELINE_CONTEXT_MENU_BTN, | ||
} from '../screens/alerts'; | ||
|
@@ -65,7 +68,6 @@ export const closeAlerts = () => { | |
|
||
export const expandFirstAlertActions = () => { | ||
cy.get(TIMELINE_CONTEXT_MENU_BTN).should('be.visible'); | ||
cy.get(TIMELINE_CONTEXT_MENU_BTN).find('svg').should('have.attr', 'data-is-loaded'); | ||
cy.get(TIMELINE_CONTEXT_MENU_BTN).first().click({ force: true }); | ||
}; | ||
|
||
|
@@ -125,6 +127,16 @@ export const openAlerts = () => { | |
cy.get(OPEN_ALERT_BTN).click(); | ||
}; | ||
|
||
export const selectCountTable = () => { | ||
cy.get(CHART_SELECT).click({ force: true }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: What does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The In
So it's ~ a 50 / 50 split in the existing code in alerts.ts, but many functions in that file also mix the use Example 1: export const addExceptionFromFirstAlert = () => {
cy.get(TIMELINE_CONTEXT_MENU_BTN).first().click({ force: true });
cy.get(ADD_EXCEPTION_BTN).click();
}; Example 2: export const openFirstAlert = () => {
cy.get(TIMELINE_CONTEXT_MENU_BTN).first().click({ force: true });
cy.get(OPEN_ALERT_BTN).click();
}; Example 3: export const openAlerts = () => {
cy.get(TAKE_ACTION_POPOVER_BTN).click({ force: true });
cy.get(OPEN_ALERT_BTN).click();
}; With the aim of reducing flakey test runs, the new functions follow the established convention above and use |
||
cy.get(SELECT_TABLE).click(); | ||
}; | ||
|
||
export const clearGroupByTopInput = () => { | ||
cy.get(GROUP_BY_TOP_INPUT).focus(); | ||
cy.get(GROUP_BY_TOP_INPUT).type('{backspace}'); | ||
}; | ||
|
||
export const goToAcknowledgedAlerts = () => { | ||
cy.get(ACKNOWLEDGED_ALERTS_FILTER_BTN).click(); | ||
cy.get(REFRESH_BUTTON).should('not.have.attr', 'aria-label', 'Needs updating'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -99,9 +99,9 @@ export const goToExceptionsTab = () => { | |
}; | ||
|
||
export const editException = () => { | ||
cy.get(EXCEPTION_ITEM_ACTIONS_BUTTON).click(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The This
The test passes locally. I'm implementing the fix suggested in the error message above. |
||
cy.get(EXCEPTION_ITEM_ACTIONS_BUTTON).click({ force: true }); | ||
|
||
cy.get(EDIT_EXCEPTION_BTN).click(); | ||
cy.get(EDIT_EXCEPTION_BTN).click({ force: true }); | ||
}; | ||
|
||
export const removeException = () => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
/* | ||
* 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 { render, screen } from '@testing-library/react'; | ||
import React from 'react'; | ||
|
||
import { TestProviders } from '../../mock'; | ||
import { | ||
mockAlertSearchResponse, | ||
mockNoDataAlertSearchResponse, | ||
} from './lib/mocks/mock_alert_search_response'; | ||
import * as i18n from './translations'; | ||
import type { Props } from '.'; | ||
import { AlertsTreemap } from '.'; | ||
|
||
const defaultProps: Props = { | ||
data: mockAlertSearchResponse, | ||
maxBuckets: 1000, | ||
minChartHeight: 370, | ||
stackByField0: 'kibana.alert.rule.name', | ||
stackByField1: 'host.name', | ||
}; | ||
|
||
describe('AlertsTreemap', () => { | ||
describe('when the response has data', () => { | ||
beforeEach(() => { | ||
render( | ||
<TestProviders> | ||
<AlertsTreemap {...defaultProps} /> | ||
</TestProviders> | ||
); | ||
}); | ||
|
||
test('it renders the treemap', () => { | ||
expect(screen.getByTestId('treemap').querySelector('.echChart')).toBeInTheDocument(); | ||
}); | ||
|
||
test('it renders the legend', () => { | ||
expect(screen.getByTestId('draggable-legend')).toBeInTheDocument(); | ||
}); | ||
}); | ||
|
||
describe('when the response does NOT have data', () => { | ||
beforeEach(() => { | ||
render( | ||
<TestProviders> | ||
<AlertsTreemap {...defaultProps} data={mockNoDataAlertSearchResponse} /> | ||
</TestProviders> | ||
); | ||
}); | ||
|
||
test('it does NOT render the treemap', () => { | ||
expect(screen.queryByTestId('treemap')).not.toBeInTheDocument(); | ||
}); | ||
|
||
test('it does NOT render the legend', () => { | ||
expect(screen.queryByTestId('draggable-legend')).not.toBeInTheDocument(); | ||
}); | ||
|
||
test('it renders the "no data" message', () => { | ||
expect(screen.getByText(i18n.NO_DATA_LABEL)).toBeInTheDocument(); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
should allow a user with crud privileges to attach alerts to cases
test inattach_alert_to_case.spec.ts
invokes theexpandFirstAlertActions()
test helper.The test is failing on CI with the following error:
The
expandFirstAlertActions()
test helper incypress/tasks/alerts.ts
was updated ~ 21 hours ago with this change: https://github.com/elastic/kibana/pull/135373/files#diff-9881e669a4305a15a96f3880d5d68906edfc15991e384a71d7ef5498ed04c0c8R68hhBased on desk testing, the
data-is-loaded
attribute:Based on the git history, it looks like the
expandFirstAlertActions()
test helper didn't include this check:in it's original implementation. The check was first added in this PR: 683463e#diff-9881e669a4305a15a96f3880d5d68906edfc15991e384a71d7ef5498ed04c0c8R68
Since this check only determines whether or not the SVG was loaded in the button icon, and
data-is-loaded
does not appear to be reilable, I'm removing thiscy.get
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good 👍
I was the one that actually added that check, so for context, this is the comment that details the change: #130072 (comment)
When I ran into this I had no issue with it passing locally, but it would fail on CI, so you may have to click-loop and/or wait for the menu if there's still an issue waiting on the table to load before the icon is visible to click.