Skip to content

[Security Solution] Resolve JS warnings triggered by incorrect state changed#148552

Merged
angorayc merged 16 commits intoelastic:mainfrom
angorayc:lens-js-error
Jan 12, 2023
Merged

[Security Solution] Resolve JS warnings triggered by incorrect state changed#148552
angorayc merged 16 commits intoelastic:mainfrom
angorayc:lens-js-error

Conversation

@angorayc
Copy link
Contributor

@angorayc angorayc commented Jan 9, 2023 •
edited
Loading

Summary

Found these js warning after replacing charts with Lens in #148519:
Screenshot 2023-01-09 at 15 31 17

Wrap search.session.start() with useEffect to avoid incorrect state changed.

Checklist

Delete any items that are not applicable to this PR.

@angorayc angorayc added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore v8.7.0 labels Jan 9, 2023
@angorayc angorayc changed the title Lens js error [Security Solution] Resolve JS warnings triggered by incorrect state changed Jan 9, 2023
@angorayc angorayc mentioned this pull request Jan 9, 2023
Merged
1 task
angorayc added a commit that referenced this pull request Jan 10, 2023
@angorayc @kibanamachine
[SecuritySolution] Replace donuts with Lens (#148519)
09e10b8 
## Summary

This pr is a part of #147261:
Replace donut charts with Lens:
This is behind feature flag `chartEmbeddablesEnabled `

Items to verify:
1. If the queries and filters from the global search box are applied.
2. If the correct alert index is applied by different spaced.
3. Visualization actions working correctly.

Detection and Response dashboard:
<img width="1662" alt="Screenshot 2023-01-09 at 09 21 56"
src="https://user-images.githubusercontent.com/6295984/211275765-8177e9bd-3623-4bb2-b1d9-8a3044d523a0.png">

Host details:
<img width="1666" alt="Screenshot 2023-01-09 at 09 21 38"
src="https://user-images.githubusercontent.com/6295984/211275770-be95353f-4d1b-410a-b7bf-b232692af1ab.png">

User details:
<img width="1662" alt="Screenshot 2023-01-09 at 09 21 19"
src="https://user-images.githubusercontent.com/6295984/211275773-dd0bcaaf-58e6-404b-b010-d1c464cbd101.png">

Network details:
<img width="1663" alt="Screenshot 2023-01-09 at 09 20 47"
src="https://user-images.githubusercontent.com/6295984/211275775-0fd39ac3-e977-44bd-bd40-304463dce613.png">

Known issues:
1. Not showing legend for alerts donut charts at the moment: There is a
logic in Lens that it doesn't show the legend item if its value is zero.
4. JS warnings triggered by incorrect state changed. Fixed by
#148552
5. No label in the donut by default when open in Lens - Lens doesn't
support displaying a label in the middle of the donut chart by default,
so it is currently available in Security Solution.
6. Applying filters or extra action while clicking the donut is not
available atm



### Checklist

Delete any items that are not applicable to this PR.


- [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: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@angorayc angorayc marked this pull request as ready for review January 10, 2023 16:06
@angorayc angorayc requested review from a team as code owners January 10, 2023 16:06
@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 10, 2023

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

@elasticmachine
Copy link
Contributor

elasticmachine commented Jan 10, 2023

Pinging @elastic/security-solution (Team: SecuritySolution)

@angorayc angorayc added the backport:skip This PR does not require backporting label Jan 10, 2023
@stephmilovic stephmilovic mentioned this pull request Jan 10, 2023
Closed
stephmilovic
stephmilovic approved these changes Jan 11, 2023
View reviewed changes
Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

LGTM!! Thanks for the fix @angorayc 🚀

@kibana-ci
Copy link

kibana-ci commented Jan 11, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Security Solution Tests #1 / Detections : Page Filters Alert list is updated when the alerts are updated

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 12.7MB 12.7MB -17.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@angorayc angorayc merged commit a275262 into elastic:main Jan 12, 2023
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
@angorayc @kibanamachine @jennypavlova
[SecuritySolution] Replace donuts with Lens (elastic#148519)
582b75d 
## Summary

This pr is a part of elastic#147261:
Replace donut charts with Lens:
This is behind feature flag `chartEmbeddablesEnabled `

Items to verify:
1. If the queries and filters from the global search box are applied.
2. If the correct alert index is applied by different spaced.
3. Visualization actions working correctly.

Detection and Response dashboard:
<img width="1662" alt="Screenshot 2023-01-09 at 09 21 56"
src="https://user-images.githubusercontent.com/6295984/211275765-8177e9bd-3623-4bb2-b1d9-8a3044d523a0.png">

Host details:
<img width="1666" alt="Screenshot 2023-01-09 at 09 21 38"
src="https://user-images.githubusercontent.com/6295984/211275770-be95353f-4d1b-410a-b7bf-b232692af1ab.png">

User details:
<img width="1662" alt="Screenshot 2023-01-09 at 09 21 19"
src="https://user-images.githubusercontent.com/6295984/211275773-dd0bcaaf-58e6-404b-b010-d1c464cbd101.png">

Network details:
<img width="1663" alt="Screenshot 2023-01-09 at 09 20 47"
src="https://user-images.githubusercontent.com/6295984/211275775-0fd39ac3-e977-44bd-bd40-304463dce613.png">

Known issues:
1. Not showing legend for alerts donut charts at the moment: There is a
logic in Lens that it doesn't show the legend item if its value is zero.
4. JS warnings triggered by incorrect state changed. Fixed by
elastic#148552
5. No label in the donut by default when open in Lens - Lens doesn't
support displaying a label in the middle of the donut chart by default,
so it is currently available in Security Solution.
6. Applying filters or extra action while clicking the donut is not
available atm



### Checklist

Delete any items that are not applicable to this PR.


- [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: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
@angorayc @stephmilovic @jennypavlova
[Security Solution] Resolve JS warnings triggered by incorrect state …
1c7d335 
…changed (elastic#148552)

## Summary

Found these js warning after replacing charts with Lens in
elastic#148519:
<img width="1671" alt="Screenshot 2023-01-09 at 15 31 17"
src="https://user-images.githubusercontent.com/6295984/211345750-8c4e67ee-bf96-49d2-8bb2-0f71e5f9bcd2.png">

Wrap `search.session.start()` with useEffect to avoid incorrect state
changed.



### Checklist

Delete any items that are not applicable to this PR.


- [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: Steph Milovic <stephanie.milovic@elastic.co>
@angorayc angorayc mentioned this pull request Jan 19, 2023
Closed
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stephmilovic stephmilovic stephmilovic approved these changes

Assignees

No one assigned

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v8.7.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@angorayc @elasticmachine @kibana-ci @stephmilovic