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] Add multiword type handling in global search #196087

Merged

Conversation

kowalczyk-krzysztof
Copy link
Member

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

Summary

This PR improves the UX of global search by allowing users to search for types that consist of multiple words without having to turn them into phrases (wrapping them in quotes).

For example:

The following query:

hello type:canvas workpad type:enterprise search world tag:new

Will get mapped to:

hello type:"canvas workpad" type:"enterprise search" world tag:new

Which will result in following Query object:

{
  "term": "hello world",
  "filters": {
    "tags": ["new"]
    "types": ["canvas workpad", "enterprise search"],
   },
}

Fixes: #176877

Checklist

@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 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 14, 2024
@kowalczyk-krzysztof kowalczyk-krzysztof requested a review from a team as a code owner October 14, 2024 10:22
@elasticmachine
Copy link
Contributor

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

@kowalczyk-krzysztof kowalczyk-krzysztof self-assigned this Oct 14, 2024
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.

Tested locally and works as expected, great job! 👍

While testing I realized that we don't allow spaces between the type: and the type.
So this does not work type: canvas workpad.
Do you think it would be possible to first strip any space after type: and the fist letter before doing the conversion to string?

}

const typesPattern = multiWordTypes.join('|');
const canvasWorkpadRegex = new RegExp(`(type:|types:)(\\s*[^"']*?)\\b(${typesPattern})\\b`, 'gi');
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we use a generic name, e.g. termReplaceRegex?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, thanks for noticing this. This variable name is something I used initially, before I made this solution more generic. I'll change that.

it('converts multiword searchable types to phrases so they get picked up as types', () => {
const mockSearchableMultiwordTypes = ['canvas-workpad', 'enterprise search'];
const searchParams = parseSearchParams(
'type:canvas workpad types:canvas-workpad hello type:enterprise search type:not multiword',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add type:"canvas workpad" to the list so we make sure it handles types with existing quotes?

Also by doing so, I realized that we pass twice the same types in the array, could we avoid dedupes when returning the array? (I know it's not part of fixing the issue but we could use the momentum 😊). Thanks 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I didn't think about checking for already existing quotes. It should work but I'll cover it in the test.

As for deduping, I wasn't really sure if this is what we want (although it's makes logical sense) but sure, I'll add that.

@kowalczyk-krzysztof
Copy link
Member Author

kowalczyk-krzysztof commented Oct 15, 2024

Do you think it would be possible to first strip any space after type: and the fist letter before doing the conversion to string?

Sure, I'll add that

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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.5KB 29.9KB +464.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! Great work 🎉 I tested locally and works as expected.
Thanks for adding the tests 👍

@kowalczyk-krzysztof kowalczyk-krzysztof merged commit 3d28d17 into elastic:main Oct 16, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 16, 2024
…#196087)

## Summary

This PR improves the UX of global search by allowing users to search for
types that consist of multiple words without having to turn them into
phrases (wrapping them in quotes).

For example:

The following query:
```
hello type:canvas workpad type:enterprise search world tag:new
```
Will get mapped to:
```
hello type:"canvas workpad" type:"enterprise search" world tag:new
```
Which will result in following `Query` object:
```json
{
  "term": "hello world",
  "filters": {
    "tags": ["new"]
    "types": ["canvas workpad", "enterprise search"],
   },
}
```

Fixes: elastic#176877

### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 3d28d17)
@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 16, 2024
…196087) (#196545)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Global Search] Add multiword type handling in global search
(#196087)](#196087)

<!--- 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-16T12:29:03Z","message":"[Global
Search] Add multiword type handling in global search (#196087)\n\n##
Summary\r\n\r\nThis PR improves the UX of global search by allowing
users to search for\r\ntypes that consist of multiple words without
having to turn them into\r\nphrases (wrapping them in
quotes).\r\n\r\nFor example: \r\n\r\nThe following
query:\r\n```\r\nhello type:canvas workpad type:enterprise search world
tag:new\r\n```\r\nWill get mapped to:\r\n```\r\nhello type:\"canvas
workpad\" type:\"enterprise search\" world tag:new\r\n```\r\nWhich will
result in following `Query` object:\r\n```json\r\n{\r\n \"term\":
\"hello world\",\r\n \"filters\": {\r\n \"tags\": [\"new\"]\r\n
\"types\": [\"canvas workpad\", \"enterprise search\"],\r\n
},\r\n}\r\n```\r\n\r\nFixes: #176877\r\n\r\n### Checklist\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3d28d173a94dc9856fe43cbff8d88ac4e2d42a17","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","Team:SharedUX","backport:prev-minor"],"title":"[Global
Search] Add multiword type handling in global
search","number":196087,"url":"https://github.com/elastic/kibana/pull/196087","mergeCommit":{"message":"[Global
Search] Add multiword type handling in global search (#196087)\n\n##
Summary\r\n\r\nThis PR improves the UX of global search by allowing
users to search for\r\ntypes that consist of multiple words without
having to turn them into\r\nphrases (wrapping them in
quotes).\r\n\r\nFor example: \r\n\r\nThe following
query:\r\n```\r\nhello type:canvas workpad type:enterprise search world
tag:new\r\n```\r\nWill get mapped to:\r\n```\r\nhello type:\"canvas
workpad\" type:\"enterprise search\" world tag:new\r\n```\r\nWhich will
result in following `Query` object:\r\n```json\r\n{\r\n \"term\":
\"hello world\",\r\n \"filters\": {\r\n \"tags\": [\"new\"]\r\n
\"types\": [\"canvas workpad\", \"enterprise search\"],\r\n
},\r\n}\r\n```\r\n\r\nFixes: #176877\r\n\r\n### Checklist\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3d28d173a94dc9856fe43cbff8d88ac4e2d42a17"}},"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/196087","number":196087,"mergeCommit":{"message":"[Global
Search] Add multiword type handling in global search (#196087)\n\n##
Summary\r\n\r\nThis PR improves the UX of global search by allowing
users to search for\r\ntypes that consist of multiple words without
having to turn them into\r\nphrases (wrapping them in
quotes).\r\n\r\nFor example: \r\n\r\nThe following
query:\r\n```\r\nhello type:canvas workpad type:enterprise search world
tag:new\r\n```\r\nWill get mapped to:\r\n```\r\nhello type:\"canvas
workpad\" type:\"enterprise search\" world tag:new\r\n```\r\nWhich will
result in following `Query` object:\r\n```json\r\n{\r\n \"term\":
\"hello world\",\r\n \"filters\": {\r\n \"tags\": [\"new\"]\r\n
\"types\": [\"canvas workpad\", \"enterprise search\"],\r\n
},\r\n}\r\n```\r\n\r\nFixes: #176877\r\n\r\n### Checklist\r\n- [x] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"3d28d173a94dc9856fe43cbff8d88ac4e2d42a17"}}]}]
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.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Global Search] Using search to find Canvas workpads requires quotations
4 participants