[Entity Analytics][UI] UI changes for Risk Engine to include closed alerts for risk score calculation #201909

abhishekbhatia1710 · 2024-11-27T06:28:08Z

Summary

We are introducing a new feature that allows users to include "closed" alerts in risk score calculations.

Users can toggle a button to include closed alerts in the risk score calculation and specify a date/time range for the calculation. Additionally, they can preview the data before finalising and saving these changes for the next engine run.

Note : This PR is an extension to the following PRs.

[API] : [Entity Analytics][API] New API to allow user to configure risk engine SO #201344
[API] : [Entity Analytics][API] Changes for preview-risk-scores API to accept new params #201397

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

…t_statuses and exclude_alert_tags

elasticmachine · 2024-11-27T06:28:11Z

Pinging @elastic/security-entity-analytics (Team:Entity Analytics)

machadoum · 2024-11-27T08:20:38Z

@abhishekbhatia1710 This PR name mentions "UI" but also has server changes. Is it intentional?

abhishekbhatia1710 · 2024-11-27T08:26:20Z

@abhishekbhatia1710 This PR name mentions "UI" but also has server changes. Is it intentional?

Yes @machadoum, because the UI changes rely on the API updates, so I rebased this branch with the changes from the API PRs (mentioned in the summary of this PR).

CAWilson94 · 2024-11-27T13:10:19Z

Tested locally, working well 🚀

Also, this might be more the EUI component, but I love the options for relative and absolute given here:

…on display

abhishekbhatia1710 · 2024-11-28T09:18:18Z

The log message which shows the configuration with which the Risk Score Engine has run is :

Risk Engine running with configuration

@elastic/security-entity-analytics

x-pack/plugins/security_solution/public/entity_analytics/translations.ts

CAWilson94 · 2024-11-27T13:20:43Z

...ugins/security_solution/public/entity_analytics/components/include_closed_alerts_section.tsx

+  useEffect(() => {
+    const savedIncludeClosedAlerts = localStorage.getItem('includeClosedAlerts');
+    const savedStart = localStorage.getItem('dateStart');
+    const savedEnd = localStorage.getItem('dateEnd');


Would using useLocalStorage have any benefit over the basic localStorage? Is error handling baked into there or do we have something to check an unhappy path? :D

☝️
Please use useLocalStorage

Please use a specific local storage key name; otherwise, it will conflict with other usages in Kibana.

...ugins/security_solution/public/entity_analytics/components/include_closed_alerts_section.tsx

machadoum · 2024-11-28T10:51:47Z

...ugins/security_solution/public/entity_analytics/components/include_closed_alerts_section.tsx

+    const savedStart = localStorage.getItem('dateStart');
+    const savedEnd = localStorage.getItem('dateEnd');
+
+    if (savedIncludeClosedAlerts !== null) {


This file has a lot of logic. Could your unit-test it?

...ugins/security_solution/public/entity_analytics/components/include_closed_alerts_section.tsx

machadoum · 2024-11-28T10:57:42Z

...plugins/security_solution/public/entity_analytics/pages/entity_analytics_management_page.tsx

+    }
+  };
+
+  const handleIncludeClosedAlertsToggle = (value: boolean) => {


You probably need a useCallback here because setIncludeClosedAlerts param is used as a dependency for useEffect in IncludeClosedAlertsSection component.

...plugins/security_solution/public/entity_analytics/pages/entity_analytics_management_page.tsx

machadoum · 2024-11-28T11:54:08Z

...ugins/security_solution/public/entity_analytics/components/include_closed_alerts_section.tsx

+  useEffect(() => {
+    const savedIncludeClosedAlerts = localStorage.getItem('includeClosedAlerts');
+    const savedStart = localStorage.getItem('dateStart');
+    const savedEnd = localStorage.getItem('dateEnd');


Please use a specific local storage key name; otherwise, it will conflict with other usages in Kibana.

...ins/security_solution/public/entity_analytics/components/risk_score_useful_links_section.tsx

machadoum · 2024-11-28T11:55:37Z

...plugins/security_solution/public/entity_analytics/pages/entity_analytics_management_page.tsx

+import { useRiskEngineStatus } from '../api/hooks/use_risk_engine_status';
+import { useScheduleNowRiskEngineMutation } from '../api/hooks/use_schedule_now_risk_engine_mutation';
+import { useAppToasts } from '../../common/hooks/use_app_toasts';
+import * as i18n from '../translations';

 export const EntityAnalyticsManagementPage = () => {


Please add unit tests

...plugins/security_solution/public/entity_analytics/pages/entity_analytics_management_page.tsx

machadoum · 2024-11-28T11:57:37Z

...plugins/security_solution/public/entity_analytics/pages/entity_analytics_management_page.tsx

+                  />
+                )}
+
+                {/* Text: "Next engine run in {} minutes" */}


nit: In this case, creating an isolated component with a clear name is usually better than adding comments.

machadoum

I left some comments.

Here are some interesting resources about kibana UI:
https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/GUIDELINE.md
https://eui.elastic.co/#/theming/typography/utilities
https://eui.elastic.co/#/utilities/i18n

…nenets and emotion for css and px related styles.

elasticmachine · 2024-11-29T09:48:51Z

💔 Build Failed

Buildkite Build
Commit: b00be60

Failed CI Steps

History

cc @abhishekbhatia1710

abhishekbhatia1710 and others added 6 commits November 22, 2024 19:55

API changes for preview-risk-scores to accept new params exclude_aler…

3d8b4ce

…t_statuses and exclude_alert_tags

New API to allow user to configure risk engine so

9a4a494

[CI] Auto-commit changed files from 'yarn openapi:bundle'

cf8aaa5

[CI] Auto-commit changed files from 'make api-docs'

c8d5d02

UI changes

404cf77

Save nav bar and configSO API call

3188237

abhishekbhatia1710 added release_note:enhancement v9.0.0 Team:Entity Analytics Security Entity Analytics Team 8.17 candidate labels Nov 27, 2024

abhishekbhatia1710 self-assigned this Nov 27, 2024

abhishekbhatia1710 requested review from a team as code owners November 27, 2024 06:28

abhishekbhatia1710 requested a review from CAWilson94 November 27, 2024 06:28

abhishekbhatia1710 mentioned this pull request Nov 27, 2024

[Request] Risk Score calculation for "closed" alerts elastic/security-docs#6254

Open

abhishekbhatia1710 added 2 commits November 27, 2024 14:03

Persititng the state of the buttons after the user interaction

e71e589

Changing the logic to get the next engine run time

5109f8e

abhishekbhatia1710 added the backport:skip This commit does not require backporting label Nov 27, 2024

abhishekbhatia1710 mentioned this pull request Nov 27, 2024

[Entity Analytics] [UI] Allow user configure so #197956

Closed

7 tasks

abhishekbhatia1710 added the ci:project-deploy-security Create a Security Serverless Project label Nov 27, 2024

abhishekbhatia1710 added 2 commits November 28, 2024 14:12

Added polling for the next engine run status and logs for configurati…

e3909eb

…on display

Changes for risk engine SO update when it is not installed.

d68f5a0

Merge branch 'main' into ea-task-ui-userConfiguresavedObject

78cfca0

CAWilson94 approved these changes Nov 28, 2024

View reviewed changes

machadoum requested changes Nov 28, 2024

View reviewed changes

machadoum reviewed Nov 28, 2024

View reviewed changes

Addressing review comments related to using i18n for texts, EUI compo…

b00be60

…nenets and emotion for css and px related styles.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Entity Analytics][UI] UI changes for Risk Engine to include closed alerts for risk score calculation #201909

[Entity Analytics][UI] UI changes for Risk Engine to include closed alerts for risk score calculation #201909

abhishekbhatia1710 commented Nov 27, 2024 •

edited by CAWilson94

Loading

elasticmachine commented Nov 27, 2024

machadoum commented Nov 27, 2024 •

edited

Loading

abhishekbhatia1710 commented Nov 27, 2024

CAWilson94 commented Nov 27, 2024 •

edited

Loading

abhishekbhatia1710 commented Nov 28, 2024

CAWilson94 Nov 27, 2024

machadoum Nov 28, 2024

machadoum Nov 28, 2024

machadoum Nov 28, 2024

machadoum Nov 28, 2024

machadoum Nov 28, 2024

machadoum Nov 28, 2024

machadoum Nov 28, 2024

machadoum left a comment

elasticmachine commented Nov 29, 2024 •

edited

Loading

[Entity Analytics][UI] UI changes for Risk Engine to include closed alerts for risk score calculation #201909

Are you sure you want to change the base?

[Entity Analytics][UI] UI changes for Risk Engine to include closed alerts for risk score calculation #201909

Conversation

abhishekbhatia1710 commented Nov 27, 2024 • edited by CAWilson94 Loading

Summary

Note : This PR is an extension to the following PRs.

Checklist

elasticmachine commented Nov 27, 2024

machadoum commented Nov 27, 2024 • edited Loading

abhishekbhatia1710 commented Nov 27, 2024

CAWilson94 commented Nov 27, 2024 • edited Loading

abhishekbhatia1710 commented Nov 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machadoum left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 29, 2024 • edited Loading

💔 Build Failed

Failed CI Steps

History

abhishekbhatia1710 commented Nov 27, 2024 •

edited by CAWilson94

Loading

machadoum commented Nov 27, 2024 •

edited

Loading

CAWilson94 commented Nov 27, 2024 •

edited

Loading

elasticmachine commented Nov 29, 2024 •

edited

Loading