[SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page by XavierM · Pull Request #44671 · elastic/kibana

XavierM · 2019-09-03T15:18:23Z

Summary

Refactor code to only use react-router to figure out where we are in the app
Fix inspect functionality on hosts and host detail page https://github.com/elastic/siem-team/issues/450
Fix to keep the search query string in sync when switching tab or sub-tab
Fix url to access details page like that http://localhost:5601/app/siem#/hosts/MatBook-Pro.local
Need to update link of the breadcrumb to keep track of the search query string from our redux state
update unit test
Need to addunit and cypress test

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
~~[ ] 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
~~[ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

~~[ ] This was checked for breaking API changes and was labeled appropriately~~
~~[ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

elasticmachine · 2019-09-03T15:18:25Z

Pinging @elastic/siem

elasticmachine · 2019-09-03T16:51:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_network.tsx

x-pack/legacy/plugins/siem/public/pages/hosts/hosts_navigations.tsx

x-pack/legacy/plugins/siem/public/components/url_state/helpers.ts

…se-url-state Fix inspect in hosts/host details page Fix some url state issue too

elasticmachine · 2019-09-04T04:59:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-04T21:04:08Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

stephmilovic

Checked out, tested locally, ran cypress tests, and performed code review. There are a few comments I'd like addressed, but nothing blocking. Great work and nice 🐛 squashing! LGTM 🚀

x-pack/legacy/plugins/siem/public/utils/route/spy_routes.tsx

x-pack/legacy/plugins/siem/cypress/integration/smoke_tests/url_state/url_state.spec.ts

stephmilovic · 2019-09-04T20:10:16Z

x-pack/legacy/plugins/siem/public/components/navigation/index.tsx

We could refactor this to use hooks

I am not sure about that because we still need to be connected to the redux connect. I think that it can be converted to a hook when will update our redux to use hook

x-pack/legacy/plugins/siem/public/components/navigation/index.tsx

x-pack/legacy/plugins/siem/public/components/navigation/tab_navigation/index.tsx

x-pack/legacy/plugins/siem/cypress/integration/lib/util/helpers.ts

stephmilovic · 2019-09-04T20:35:14Z

x-pack/legacy/plugins/siem/public/pages/hosts/details/utils.ts

we're going to have to look out for conflicts between this and my escape query PR: #43030

stephmilovic · 2019-09-04T20:40:40Z

x-pack/legacy/plugins/siem/public/pages/hosts/index.tsx

is there a way we can simplify this? it's a lot of repeated code. perhaps put all this route data in an object and map over that object, returning the <Route>s you need

x-pack/legacy/plugins/siem/public/utils/route/spy_routes.tsx

…se-url-state Fix inspect in hosts/host details page Fix some url state issue too

elasticmachine · 2019-09-05T02:59:58Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

[SIEM] Fix anomaly table

elasticmachine · 2019-09-05T04:48:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-05T08:59:07Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

angorayc · 2019-09-05T09:58:11Z

Nice clean up and thanks for fixing the bug! Sorry that I did a fix for cyclic deps without asking for your permission, please feel free to fix in other solution. I just found it might be upset to see the red light in the morning...

elasticmachine · 2019-09-05T11:16:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

XavierM · 2019-09-05T12:07:07Z

Nice clean up and thanks for fixing the bug! Sorry that I did a fix for cyclic deps without asking for your permission, please feel free to fix in other solution. I just found it might be upset to see the red light in the morning...

@angorayc Please do not be sorry I appreciate it the help.

FrankHassanabad

I worked off this branch personally and have nothing but the nicest things to say about it. Appreciate all the effort from all the people that have gone into this and the Cypress tests as well.

Using my best Hermione voice, I approve using this latin phrase

Haec est magna viverra enim omnis petitionem habere!

elasticmachine · 2019-09-05T14:54:31Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-05T15:48:00Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…ge (elastic#44671) * refactor code to use react-router params instead of regex inside of use-url-state Fix inspect in hosts/host details page Fix some url state issue too * add search in breadcrumb + fix existing unit testing * refactor code to use react-router params instead of regex inside of use-url-state Fix inspect in hosts/host details page Fix some url state issue too * add search in breadcrumb + fix existing unit testing * fix bug by doing cypress test * clean up + unit testing * review I * [SIEM] Fixes the Anomalies table paging to work again * Updated to look more like the PR in flight to have a smaller diff * fix cyclic deps * remove redundant comment * review II * review II + fix circular dependency * fix host url

…ge (#44671) (#44911) * refactor code to use react-router params instead of regex inside of use-url-state Fix inspect in hosts/host details page Fix some url state issue too * add search in breadcrumb + fix existing unit testing * refactor code to use react-router params instead of regex inside of use-url-state Fix inspect in hosts/host details page Fix some url state issue too * add search in breadcrumb + fix existing unit testing * fix bug by doing cypress test * clean up + unit testing * review I * [SIEM] Fixes the Anomalies table paging to work again * Updated to look more like the PR in flight to have a smaller diff * fix cyclic deps * remove redundant comment * review II * review II + fix circular dependency * fix host url

…ge (#44671) (#44912) * refactor code to use react-router params instead of regex inside of use-url-state Fix inspect in hosts/host details page Fix some url state issue too * add search in breadcrumb + fix existing unit testing * refactor code to use react-router params instead of regex inside of use-url-state Fix inspect in hosts/host details page Fix some url state issue too * add search in breadcrumb + fix existing unit testing * fix bug by doing cypress test * clean up + unit testing * review I * [SIEM] Fixes the Anomalies table paging to work again * Updated to look more like the PR in flight to have a smaller diff * fix cyclic deps * remove redundant comment * review II * review II + fix circular dependency * fix host url

…ete-for-distance_feature * 'master' of github.com:elastic/kibana: (89 commits) Replace TSVB timeseries charts with elastic-charts (elastic#33558) [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581) [alerting] Adds Action Type configuration support and whitelisting (elastic#44483) FTR: fix WebDriver Actions calls (elastic#44605) [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677) [skip-ci][Maps] Improve Maps intro page (elastic#44721) [Maps] Update titles and descriptions for data sources (elastic#44833) Types + Extract Integration Util (elastic#44433) Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933) [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359) Update Jest script to output coverage (elastic#44447) [ftr] support --kibana-install-dir flag (elastic#44552) [WATCHER] Allow user to set a threshold value of 0 (elastic#44810) Remove injectI18n in dashboard plugin. (elastic#44580) [Graph] Save modal (elastic#44261) Use external script for the OIDC Implicit flow handler page. (elastic#44866) disable router prefixing with pluginId (elastic#44855) [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671) [ML] File data viz limiting uploaded doc chunk size (elastic#44768) [code] Append go env variable 'GOCACHE' to go lsp spawn command. (elastic#44864) ...

…plate * 'master' of github.com:elastic/kibana: (91 commits) [APM] Make number of x ticks responsive to the plot width (elastic#44870) [ML] Single metric viewer: Fix top nav refresh behaviour. (elastic#44860) Replace TSVB timeseries charts with elastic-charts (elastic#33558) [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581) [alerting] Adds Action Type configuration support and whitelisting (elastic#44483) FTR: fix WebDriver Actions calls (elastic#44605) [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677) [skip-ci][Maps] Improve Maps intro page (elastic#44721) [Maps] Update titles and descriptions for data sources (elastic#44833) Types + Extract Integration Util (elastic#44433) Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933) [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359) Update Jest script to output coverage (elastic#44447) [ftr] support --kibana-install-dir flag (elastic#44552) [WATCHER] Allow user to set a threshold value of 0 (elastic#44810) Remove injectI18n in dashboard plugin. (elastic#44580) [Graph] Save modal (elastic#44261) Use external script for the OIDC Implicit flow handler page. (elastic#44866) disable router prefixing with pluginId (elastic#44855) [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671) ...

XavierM added bug Fixes for quality problems that affect the customer experience Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.4.0 labels Sep 3, 2019

XavierM self-assigned this Sep 3, 2019

XavierM added the WIP Work in progress label Sep 3, 2019

angorayc reviewed Sep 3, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/link_to/redirect_to_network.tsx Outdated Show resolved Hide resolved

angorayc reviewed Sep 3, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/pages/hosts/hosts_navigations.tsx Outdated Show resolved Hide resolved

angorayc reviewed Sep 3, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/url_state/helpers.ts Outdated Show resolved Hide resolved

angorayc reviewed Sep 3, 2019

View reviewed changes

x-pack/legacy/plugins/siem/public/components/url_state/helpers.ts Outdated Show resolved Hide resolved

spong added the v7.5.0 label Sep 4, 2019

XavierM added 2 commits September 3, 2019 23:35

refactor code to use react-router params instead of regex inside of u…

9bbebed

…se-url-state Fix inspect in hosts/host details page Fix some url state issue too

add search in breadcrumb + fix existing unit testing

ecfe07a

XavierM force-pushed the siem-fix-url-sub-tab branch from 90a4559 to ecfe07a Compare September 4, 2019 03:36

XavierM force-pushed the siem-fix-url-sub-tab branch from ecfe07a to 7bb962a Compare September 4, 2019 19:51

stephmilovic approved these changes Sep 4, 2019

View reviewed changes

XavierM added 4 commits September 4, 2019 21:17

refactor code to use react-router params instead of regex inside of u…

bf3f476

…se-url-state Fix inspect in hosts/host details page Fix some url state issue too

add search in breadcrumb + fix existing unit testing

24904dc

fix bug by doing cypress test

f7f1480

clean up + unit testing

0ee232a

XavierM force-pushed the siem-fix-url-sub-tab branch from 7bb962a to 0ee232a Compare September 5, 2019 01:17

review I

e7956fa

XavierM added review and removed WIP Work in progress labels Sep 5, 2019

[SIEM] Fixes the Anomalies table paging to work again

18179d5

Merge branch 'pr/44671' into fix-anomaly-table

728e938

FrankHassanabad and others added 2 commits September 4, 2019 21:11

Updated to look more like the PR in flight to have a smaller diff

1d0f417

Merge pull request #5 from FrankHassanabad/fix-anomaly-table

dbc5b56

[SIEM] Fix anomaly table

fix cyclic deps

3678bfa

remove redundant comment

c24f51e

angorayc approved these changes Sep 5, 2019

View reviewed changes

XavierM added 2 commits September 5, 2019 08:10

review II

4d128a8

review II + fix circular dependency

0eb0046

FrankHassanabad approved these changes Sep 5, 2019

View reviewed changes

fix host url

e334a98

XavierM merged commit a712751 into elastic:master Sep 5, 2019

XavierM mentioned this pull request Sep 5, 2019

[7.x] [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (#44671) #44911

Merged

XavierM mentioned this pull request Sep 5, 2019

[7.4] [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (#44671) #44912

Merged

XavierM deleted the siem-fix-url-sub-tab branch June 4, 2020 16:06

Conversation

XavierM commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Sep 3, 2019

Uh oh!

elasticmachine commented Sep 3, 2019

💔 Build Failed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Sep 4, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 4, 2019

💚 Build Succeeded

Uh oh!

stephmilovic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stephmilovic Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

XavierM Sep 5, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stephmilovic Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

stephmilovic Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticmachine commented Sep 5, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 5, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 5, 2019

💚 Build Succeeded

Uh oh!

angorayc commented Sep 5, 2019

Uh oh!

elasticmachine commented Sep 5, 2019

💚 Build Succeeded

Uh oh!

XavierM commented Sep 5, 2019

Uh oh!

FrankHassanabad left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 5, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 5, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

XavierM commented Sep 3, 2019 •

edited

Loading