[Cloud Security] Fix for issue where date is not retained when opening new tab #205128

animehart · 2024-12-24T07:03:59Z

Summary

This PR addresses the issue where when user opens new tab by clicking on popout icon on Alerts Data grid on Alerts preview Contextual flyout, the date from original tab is not retained, as such there are scenario where user don't see any alerts on new tab because its defaulted to today when the alerts only exist from view days ago

Screen.Recording.2024-12-23.at.10.59.08.PM.mov

animehart · 2024-12-24T07:04:37Z

/ci

elasticmachine · 2024-12-24T16:58:35Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

Omolola-Akinleye

LGTM!

JordanSh · 2024-12-31T07:24:04Z

...ty/plugins/security_solution/public/common/hooks/use_navigate_to_alerts_page_with_filters.ts

+  return (
+    filterItems: FilterControlConfig | FilterControlConfig[],
+    openInNewTab = false,
+    timerange?: string
+  ) => {
    const urlFilterParams = encode(
      formatPageFilterSearchParam(Array.isArray(filterItems) ? filterItems : [filterItems])
    );
-
+    const timerangePath = timerange ? `&timerange=${timerange}` : '';
    navigateTo({
      deepLinkId: SecurityPageName.alerts,
-      path: `?${URL_PARAM_KEY.pageFilter}=${urlFilterParams}`,
+      path: `?${URL_PARAM_KEY.pageFilter}=${urlFilterParams}${timerangePath}`,


Im wondering if we can't use the existing urlFilterParams and add timerange to them directly.
The problem the might occur with the current fix is that what happens if both urlFilterParams and timerangePath include a timerange filter?

the table file seems to include the needed functions to create this query. and the encoding is happening here on this hook file. so if we can insert the timerange filter from the table it would be ideal.

im approving, but please verify if that double timerange filter case can actually happen. if so, we probably need to address it before merging. i might also be missing the reason why this suggestion wont work so ill be glad if you could explain if you already tried it

hmmmm so before this actually I Tried adding timerange on the Alerts page filter to no avail
but your question is a good one, I'm not 100% sure if its not doable on adding timerange directly on the pageFilter, Im going to ask Security Team about this

Yes this could be a scenario that happens, but I think it is very unlikely, as developers would have to specifically pass a timerange AND a timestamp filter to this hook...

Also in the UI when using this hook, chances are that it would show the following message, letting the user know the filters have changed and would encourage them to look at them

And finally, I'm not expecting many people to use this new timerange prop many often, as the only usage at the moment is for the alerts_findings_details_table.tsx above, all the other instances are not opening in a new tab

PhilippeOberti

I tested and everything looks to be working as expected.

I left a small comment if you're OK adding some code documentation on the properties of the function that the hook returns.
Also, could you add some new unit test cases to cover the new prop?

Thanks!

PhilippeOberti · 2025-01-03T16:09:31Z

...ty/plugins/security_solution/public/common/hooks/use_navigate_to_alerts_page_with_filters.ts

+    openInNewTab = false,
+    timerange?: string


Currently, no-one (except the usage in the file alerts_findings_details_table.tsx above) is using the openInNewTab property. It seems to me that the new timerange property is fully related to the fact that we want to open in a new tab. If openInNewTabis false, the user is just navigating to a different page in Kibana within the same browser tab, and the KQL bar and its date time picker remain the same (meaning the date and time are persisted).

I've been trying to find a good way to combine openInNewTab and timerange somehow together?, but I'm not liking any of the solutions I can come up with.

I'm ok keeping things as you have right now, but if you have an idea let me know!

would you be ok adding these prop documentations?

/** * Pass one or more filter control configurations to be applied to the alerts page filters */ filterItems: FilterControlConfig | FilterControlConfig[], /** * If true, the alerts page will be opened in a new tab */ openInNewTab = false, /** * Allows to customize the timerange url parameter. Should only be used in combination with the openInNewTab=true parameter */ timerange?: string

PhilippeOberti · 2025-01-03T16:12:58Z

...ty/plugins/security_solution/public/common/hooks/use_navigate_to_alerts_page_with_filters.ts

+  return (
+    filterItems: FilterControlConfig | FilterControlConfig[],
+    openInNewTab = false,
+    timerange?: string
+  ) => {
    const urlFilterParams = encode(
      formatPageFilterSearchParam(Array.isArray(filterItems) ? filterItems : [filterItems])
    );
-
+    const timerangePath = timerange ? `&timerange=${timerange}` : '';
    navigateTo({
      deepLinkId: SecurityPageName.alerts,
-      path: `?${URL_PARAM_KEY.pageFilter}=${urlFilterParams}`,
+      path: `?${URL_PARAM_KEY.pageFilter}=${urlFilterParams}${timerangePath}`,


Yes this could be a scenario that happens, but I think it is very unlikely, as developers would have to specifically pass a timerange AND a timestamp filter to this hook...

Also in the UI when using this hook, chances are that it would show the following message, letting the user know the filters have changed and would encourage them to look at them

And finally, I'm not expecting many people to use this new timerange prop many often, as the only usage at the moment is for the alerts_findings_details_table.tsx above, all the other instances are not opening in a new tab

PhilippeOberti

thanks for adding the documentation and the tests!

elasticmachine · 2025-01-03T19:16:47Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 0e8bec2

Failed CI Steps

Test Failures

[job] [logs] Jest Tests #8 / ConnectorFields renders Validation validates correctly if the provider config url is empty
[job] [logs] FTR Configs #33 / InfraOps App Metrics UI Node Details #Asset Type: host Osquery Tab should show a date picker

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`	22.2MB	22.2MB	+615.0B

History

💛 Build #263425 was flaky 3b1c835
💚 Build #263080 succeeded de67b4c
💚 Build #262964 succeeded ffceae9
💛 Build #262904 was flaky f8bceab

kibanamachine · 2025-01-03T19:44:28Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12603042024

…g new tab (elastic#205128) ## Summary This PR addresses the issue where when user opens new tab by clicking on popout icon on Alerts Data grid on Alerts preview Contextual flyout, the date from original tab is not retained, as such there are scenario where user don't see any alerts on new tab because its defaulted to today when the alerts only exist from view days ago https://github.com/user-attachments/assets/58a30d7d-18b3-47a8-ab4b-2ce143583368 (cherry picked from commit 32dbd14)

kibanamachine · 2025-01-03T19:49:29Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…opening new tab (#205128) (#205538) # Backport This will backport the following commits from `main` to `8.x`: - [[Cloud Security] Fix for issue where date is not retained when opening new tab (#205128)](#205128)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Rickyanto Ang <[email protected]>

initial push for fix

f8bceab

animehart linked an issue Dec 24, 2024 that may be closed by this pull request

Date filter is not retained when navigating from Alerts Contextual Flyout to Alerts page #200792

Closed

animehart added v8.18.0 v9.0.0 Team:Cloud Security Cloud Security team related backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes labels Dec 24, 2024

animehart marked this pull request as ready for review December 24, 2024 16:58

animehart requested review from a team as code owners December 24, 2024 16:58

animehart added 2 commits December 24, 2024 09:14

Merge branch 'main' into fix-alert-newtab-timerange

ffceae9

Merge branch 'main' into fix-alert-newtab-timerange

de67b4c

Omolola-Akinleye self-requested a review December 30, 2024 17:27

Omolola-Akinleye approved these changes Dec 30, 2024

View reviewed changes

Merge branch 'main' into fix-alert-newtab-timerange

3b1c835

JordanSh approved these changes Dec 31, 2024

View reviewed changes

Merge branch 'main' into fix-alert-newtab-timerange

27fbeda

PhilippeOberti requested changes Jan 3, 2025

View reviewed changes

pr comments

0e8bec2

PhilippeOberti approved these changes Jan 3, 2025

View reviewed changes

animehart merged commit 32dbd14 into elastic:main Jan 3, 2025
8 checks passed

kibanamachine mentioned this pull request Jan 3, 2025

[8.x] [Cloud Security] Fix for issue where date is not retained when opening new tab (#205128) #205538

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cloud Security] Fix for issue where date is not retained when opening new tab #205128

[Cloud Security] Fix for issue where date is not retained when opening new tab #205128

animehart commented Dec 24, 2024 •

edited by kibanamachine

Loading

animehart commented Dec 24, 2024

elasticmachine commented Dec 24, 2024

Omolola-Akinleye left a comment

JordanSh Dec 31, 2024 •

edited

Loading

animehart Dec 31, 2024

PhilippeOberti Jan 3, 2025

PhilippeOberti left a comment

PhilippeOberti Jan 3, 2025

PhilippeOberti Jan 3, 2025

PhilippeOberti Jan 3, 2025

PhilippeOberti left a comment

elasticmachine commented Jan 3, 2025

kibanamachine commented Jan 3, 2025

kibanamachine commented Jan 3, 2025

[Cloud Security] Fix for issue where date is not retained when opening new tab #205128

[Cloud Security] Fix for issue where date is not retained when opening new tab #205128

Conversation

animehart commented Dec 24, 2024 • edited by kibanamachine Loading

Summary

animehart commented Dec 24, 2024

elasticmachine commented Dec 24, 2024

Omolola-Akinleye left a comment

Choose a reason for hiding this comment

JordanSh Dec 31, 2024 • edited Loading

Choose a reason for hiding this comment

animehart Dec 31, 2024

Choose a reason for hiding this comment

PhilippeOberti Jan 3, 2025

Choose a reason for hiding this comment

PhilippeOberti left a comment

Choose a reason for hiding this comment

PhilippeOberti Jan 3, 2025

Choose a reason for hiding this comment

PhilippeOberti Jan 3, 2025

Choose a reason for hiding this comment

PhilippeOberti Jan 3, 2025

Choose a reason for hiding this comment

PhilippeOberti left a comment

Choose a reason for hiding this comment

elasticmachine commented Jan 3, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Async chunks

History

kibanamachine commented Jan 3, 2025

kibanamachine commented Jan 3, 2025

💚 All backports created successfully

Questions ?

animehart commented Dec 24, 2024 •

edited by kibanamachine

Loading

JordanSh Dec 31, 2024 •

edited

Loading