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

[Global Search] Fix convertTagNameToId to use lowercase values #196819

Merged

Conversation

kowalczyk-krzysztof
Copy link
Member

@kowalczyk-krzysztof kowalczyk-krzysztof commented Oct 18, 2024

Summary

This PR fixes a bug which caused mixed case tags to not be found in global search.
Fixes: #196168

The allTags argument contains tags with names in the original case they were created but the tagName argument passed in search_bar.tsx:180 is lowercase. Since you can't have tags with the same name but different casing, converting them to lowercase is safe.

I've also added lowercase conversion to tagName argument in case this function gets called somewhere else and the input is not lowercase.

@kowalczyk-krzysztof kowalczyk-krzysztof added bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes Feature:Navigational Search Global search bar Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Oct 18, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof self-assigned this Oct 18, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof requested a review from a team as a code owner October 18, 2024 09:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/appex-sharedux (Team:SharedUX)

@kowalczyk-krzysztof kowalczyk-krzysztof changed the title [Global Search] Fix converTagNameToId to use lowercase values [Global Search] Fix convertTagNameToId to use lowercase values Oct 18, 2024
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #50 / discover/group1 "after all" hook: afterTestSuite.trigger in "discover/group1"
  • [job] [logs] FTR Configs #50 / discover/group1 discover test "after all" hook: afterTestSuite.trigger in "discover test"
  • [job] [logs] FTR Configs #50 / discover/group1 discover test "before all" hook in "discover test"
  • [job] [logs] Jest Tests #9 / When on the package policy create page And Route state is provided via Fleet HashRouter and the cancel Link or Button is clicked should use custom "cancel" URL
  • [job] [logs] Jest Tests #9 / When on the package policy create page Submit page On save navigate should navigate to agent policy if no route state is set
  • [job] [logs] Jest Tests #9 / When on the package policy create page Submit page On save navigate should navigate to save navigate app if set
  • [job] [logs] Jest Tests #9 / When on the package policy create page Submit page On save navigate should navigate to save navigate path if set
  • [job] [logs] Jest Tests #9 / When on the package policy create page Submit page On save navigate should navigate to save navigate path with query param if set
  • [job] [logs] Jest Tests #9 / When on the package policy create page Submit page Without query param should create agent policy before creating package policy on submit when new hosts is selected
  • [job] [logs] Jest Tests #9 / When on the package policy create page Submit page Without query param should show modal if agent policy has agents

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
globalSearchBar 29.9KB 29.9KB -14.0B
savedObjectsTagging 22.6KB 22.6KB +28.0B
total +14.0B

History

cc @kowalczyk-krzysztof

Copy link
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

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

LGTM! Tested locally and works as expected 👍

@kowalczyk-krzysztof kowalczyk-krzysztof merged commit b495c37 into elastic:main Oct 22, 2024
26 checks passed
@kowalczyk-krzysztof kowalczyk-krzysztof deleted the fix/mixedcase-tags branch October 22, 2024 09:40
@kowalczyk-krzysztof kowalczyk-krzysztof restored the fix/mixedcase-tags branch October 22, 2024 09:40
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 22, 2024
…ic#196819)

## Summary

This PR fixes a bug which caused mixed case tags to not be found in
global search.
Fixes: elastic#196168

The `allTags` argument contains tags with names in the original case
they were created but the `tagName` argument passed in
[search_bar.tsx:180](https://github.com/elastic/kibana/blob/main/x-pack/plugins/global_search_bar/public/components/search_bar.tsx#L180)
is lowercase. Since you can't have tags with the same name but different
casing, converting them to lowercase is safe.

I've also added lowercase conversion to `tagName` argument in case this
function gets called somewhere else and the input is not lowercase.

(cherry picked from commit b495c37)
@kibanamachine
Copy link
Contributor

💚 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

kibanamachine added a commit that referenced this pull request Oct 22, 2024
…196819) (#197193)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Global Search] Fix convertTagNameToId to use lowercase values
(#196819)](#196819)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Krzysztof
Kowalczyk","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-22T09:40:42Z","message":"[Global
Search] Fix convertTagNameToId to use lowercase values (#196819)\n\n##
Summary\r\n\r\nThis PR fixes a bug which caused mixed case tags to not
be found in\r\nglobal search.\r\nFixes: #196168\r\n\r\nThe `allTags`
argument contains tags with names in the original case\r\nthey were
created but the `tagName` argument passed
in\r\n[search_bar.tsx:180](https://github.com/elastic/kibana/blob/main/x-pack/plugins/global_search_bar/public/components/search_bar.tsx#L180)\r\nis
lowercase. Since you can't have tags with the same name but
different\r\ncasing, converting them to lowercase is safe.\r\n\r\nI've
also added lowercase conversion to `tagName` argument in case
this\r\nfunction gets called somewhere else and the input is not
lowercase.","sha":"b495c371fd946f39341a557599033647f81cdbf3","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","Feature:Navigational
Search","Team:SharedUX","backport:prev-minor"],"title":"[Global Search]
Fix convertTagNameToId to use lowercase
values","number":196819,"url":"https://github.com/elastic/kibana/pull/196819","mergeCommit":{"message":"[Global
Search] Fix convertTagNameToId to use lowercase values (#196819)\n\n##
Summary\r\n\r\nThis PR fixes a bug which caused mixed case tags to not
be found in\r\nglobal search.\r\nFixes: #196168\r\n\r\nThe `allTags`
argument contains tags with names in the original case\r\nthey were
created but the `tagName` argument passed
in\r\n[search_bar.tsx:180](https://github.com/elastic/kibana/blob/main/x-pack/plugins/global_search_bar/public/components/search_bar.tsx#L180)\r\nis
lowercase. Since you can't have tags with the same name but
different\r\ncasing, converting them to lowercase is safe.\r\n\r\nI've
also added lowercase conversion to `tagName` argument in case
this\r\nfunction gets called somewhere else and the input is not
lowercase.","sha":"b495c371fd946f39341a557599033647f81cdbf3"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196819","number":196819,"mergeCommit":{"message":"[Global
Search] Fix convertTagNameToId to use lowercase values (#196819)\n\n##
Summary\r\n\r\nThis PR fixes a bug which caused mixed case tags to not
be found in\r\nglobal search.\r\nFixes: #196168\r\n\r\nThe `allTags`
argument contains tags with names in the original case\r\nthey were
created but the `tagName` argument passed
in\r\n[search_bar.tsx:180](https://github.com/elastic/kibana/blob/main/x-pack/plugins/global_search_bar/public/components/search_bar.tsx#L180)\r\nis
lowercase. Since you can't have tags with the same name but
different\r\ncasing, converting them to lowercase is safe.\r\n\r\nI've
also added lowercase conversion to `tagName` argument in case
this\r\nfunction gets called somewhere else and the input is not
lowercase.","sha":"b495c371fd946f39341a557599033647f81cdbf3"}}]}]
BACKPORT-->

Co-authored-by: Krzysztof Kowalczyk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) bug Fixes for quality problems that affect the customer experience Feature:Navigational Search Global search bar release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Global Search] - mixed case tags not working as expected
4 participants