Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Url state parameter for external alerts checkbox #142344

Merged
merged 5 commits into from
Oct 4, 2022

Conversation

machadoum
Copy link
Member

@machadoum machadoum commented Sep 30, 2022

issue #137436

Summary

Add Url state parameter for external alerts checkbox only when the user is on the page, when the user navigates to another page the URL param is removed from the query string.

*** The implementation doesn't use global_query_stringbecause external alerts URL param isn't global.

181497810-53082411-7363-4513-a45b-c7e2cf34ef86

Sep-30-2022 15-30-57

Extra:

  • It updates global_query_string by extracting reusable code to a helper file
  • It fixes a bug that made unregistered URL params stay in the URL.

Checklist

Delete any items that are not applicable to this PR.

@machadoum machadoum self-assigned this Sep 30, 2022
@machadoum machadoum added enhancement New value added to drive a business result v8.6.0 Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore release_note:skip Skip the PR/issue when compiling release notes labels Sep 30, 2022
@machadoum machadoum marked this pull request as ready for review September 30, 2022 15:19
@machadoum machadoum requested a review from a team as a code owner September 30, 2022 15:19
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

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

@machadoum
Copy link
Member Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #17 / APM API tests trial no data Updating ML jobs to v3 when there are only v2 jobs creates a new job for each environment that has a v2 job
  • [job] [logs] Security Solution Tests #1 / Detection rules, bulk edit Prerequisites "before each" hook for "Prebuilt and custom rules selected: user proceeds with custom rules editing"

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 6.6MB 6.6MB +1.1KB

History

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

cc @machadoum

const getInitialUrlParamValue = useGetInitialUrlParamValue<boolean>(EXTERNAL_ALERTS_URL_PARAM);

const { decodedParam: showExternalAlertsInitialUrlState } = useMemo(
() => getInitialUrlParamValue(),
Copy link
Contributor

@jamster10 jamster10 Oct 3, 2022

Choose a reason for hiding this comment

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

Made me curious, and not important, but do you know if destructuring allocates memory for the intermediary object the value gets pulled out of?
Would:

const showExternalAlertsInitialUrlState = useMemo(() => getInitialUrlParamState().decodedParam, [•••])

Cause an improvement to memory or memorization? Not that it would make much of a difference here?

@machadoum machadoum merged commit 3c0086b into elastic:main Oct 4, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 4, 2022
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
* Refactor global_query_string to move reusabel code to helper

* Add Url state parameter for external alerts checkbox

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
* Refactor global_query_string to move reusabel code to helper

* Add Url state parameter for external alerts checkbox

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting enhancement New value added to drive a business result 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:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants