Skip to content

[Security Solution][Resolver] - Maintain active node#86682

Merged
michaelolo24 merged 11 commits intoelastic:masterfrom
michaelolo24:maintain-active-node
Jan 5, 2021
Merged

[Security Solution][Resolver] - Maintain active node#86682
michaelolo24 merged 11 commits intoelastic:masterfrom
michaelolo24:maintain-active-node

Conversation

@michaelolo24
Copy link
Copy Markdown
Contributor

@michaelolo24 michaelolo24 commented Dec 21, 2020 •
edited
Loading

Summary

This PR removes the deprecated userBroughtNodeIntoView, and adds a useSyncSelectedNode to sync the active node to any url changes that take place via interaction with the graph as well as via the browser back and forward buttons. This also resolves #81189

GIF of the forward and back actions below

syncSelected.gif.zip

Checklist

Delete any items that are not applicable to this PR.

@michaelolo24 michaelolo24 added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 Team:Threat Hunting Security Solution Threat Hunting Team labels Dec 21, 2020
@michaelolo24
Copy link
Copy Markdown
Contributor Author

michaelolo24 commented Dec 21, 2020

@elasticmachine merge upstream

1 similar comment
@michaelolo24
Copy link
Copy Markdown
Contributor Author

michaelolo24 commented Dec 22, 2020

@elasticmachine merge upstream

@michaelolo24 michaelolo24 marked this pull request as ready for review December 22, 2020 21:15
@michaelolo24 michaelolo24 requested review from a team as code owners December 22, 2020 21:15
@michaelolo24 michaelolo24 added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Dec 22, 2020
@michaelolo24
Copy link
Copy Markdown
Contributor Author

michaelolo24 commented Jan 4, 2021

@elasticmachine merge upstream

jonathan-buttner
jonathan-buttner reviewed Jan 4, 2021
View reviewed changes
x-pack/plugins/security_solution/public/resolver/view/process_event_dot.tsx Outdated
Copy link
Copy Markdown
Contributor

@jonathan-buttner jonathan-buttner Jan 4, 2021

Choose a reason for hiding this comment

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

Do we need the time field? I probably missed something but doesn't look like we use it in the reducer right?

Copy link
Copy Markdown
Contributor

@kqualters-elastic kqualters-elastic Jan 4, 2021

Choose a reason for hiding this comment

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

it's used by the animatePanning helper in the reducer https://github.com/elastic/kibana/pull/86682/files#diff-f9c46528634fc1f01e36cc14d7ebcfad609a0ce59c2fcc081520f8e65c0bbc18R69

Copy link
Copy Markdown
Contributor Author

@michaelolo24 michaelolo24 Jan 4, 2021

Choose a reason for hiding this comment

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

Yea, this line specifically:

kibana/x-pack/plugins/security_solution/public/resolver/store/reducer.ts

Line 77 in 4c3c2ac

camera: animatePanning(nextState.camera, action.payload.time, position, animationDuration),

x-pack/plugins/security_solution/public/resolver/view/use_sync_selected_node.test.tsx Outdated
Copy link
Copy Markdown
Contributor

@jonathan-buttner jonathan-buttner Jan 4, 2021

Choose a reason for hiding this comment

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

I can't remember if it's possible but should we import the duration amount from the resolver code?

Copy link
Copy Markdown
Contributor Author

@michaelolo24 michaelolo24 Jan 4, 2021

Choose a reason for hiding this comment

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

We never made this a constant anywhere, but just did so and updated it in any relevant tests

jonathan-buttner
jonathan-buttner approved these changes Jan 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Looks good, just wasn't sure if the cypress changes should be removed.

@michaelolo24
Copy link
Copy Markdown
Contributor Author

michaelolo24 commented Jan 5, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Jan 5, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 2160 2161 +1

Async chunks

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

id before after diff
securitySolution 8.5MB 8.5MB +897.0B

Distributable file count

id before after diff
default 47252 48012 +760

History

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

@michaelolo24
Copy link
Copy Markdown
Contributor Author

michaelolo24 commented Jan 5, 2021

Looks good, just wasn't sure if the cypress changes should be removed.

Spoke with @patrykkopycinski and it should remain in there. Reverted my changes to the cypress.json file. Thanks

@michaelolo24 michaelolo24 merged commit bb3659c into elastic:master Jan 5, 2021
@michaelolo24 michaelolo24 deleted the maintain-active-node branch January 5, 2021 18:27
@michaelolo24 michaelolo24 mentioned this pull request Jan 5, 2021
Merged
michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Jan 5, 2021
@michaelolo24 @kibanamachine
[Security Solution][Resolver] - Maintain active node (elastic#86682)
3fa2843 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
michaelolo24 added a commit that referenced this pull request Jan 6, 2021
@michaelolo24 @kibanamachine
[7.x] [Security Solution][Resolver] - Maintain active node (#86682) (#…
00bf236 
…87371)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@oatkiller oatkiller added the Feature:Resolver Security Solution Resolver feature label Jan 11, 2021
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jan 11, 2021

Pinging @elastic/endpoint-app-team (Feature:Resolver)

@oatkiller oatkiller mentioned this pull request Feb 11, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jonathan-buttner jonathan-buttner jonathan-buttner approved these changes

@kqualters-elastic kqualters-elastic kqualters-elastic approved these changes

Assignees

No one assigned

Labels

Feature:Resolver Security Solution Resolver feature release_note:enhancement Team:Threat Hunting Security Solution Threat Hunting Team v7.12.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Security Solution] [Resolver] Using the browser 'back' button to select a previously selected node does not move the camera to it.

6 participants

@michaelolo24 @kibanamachine @elasticmachine @jonathan-buttner @kqualters-elastic @oatkiller