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, Saved Objects Management] Use new parse option to specify recognized fields #190464

Merged
merged 7 commits into from
Sep 19, 2024

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Aug 13, 2024

This PR adds a new option to the SchemaType interface for parsing a query from a search in EuiSearchBar. This new field controls how EuiSearchBar text is parsed into a Query object. It enables better accuracy in how search terms are parsed when they include a : character.

Release note

Fixed an issue when using search bars with a term that includes a colon : character.

Summary

Closes #184496
Depends on elastic/eui#7960

GLOBAL SEARCH: BEFORE
akshfgkalsfh-before

GLOBAL SEARCH: AFTER
akshfgkalsfh-after

SAVED OBJECTS MANAGEMENT: BEFORE
okjoyofjiuh-before

SAVED OBJECTS MANAGEMENT: AFTER
okjoyofjiuh-after

SAVED OBJECTS FINDER: BEFORE
lfdgnhklfd-before

SAVED OBJECTS FINDER: AFTER
lfdgnhklfd-after

Checklist

  • Ensure that filtering using type: and tags: still works

@tsullivan tsullivan force-pushed the search-bar/use-parse-query-update branch 2 times, most recently from ffa8b77 to a5a5be5 Compare August 20, 2024 18:14
@tsullivan tsullivan force-pushed the search-bar/use-parse-query-update branch from a5a5be5 to 7997583 Compare August 27, 2024 21:47
@tsullivan tsullivan changed the title [Global Search, Saved Objects Management] Use ParseOptions['recognizedFields'] [Global Search, Saved Objects Management] Use new parse option to specify recognized fields Aug 27, 2024
@tsullivan
Copy link
Member Author

/ci

1 similar comment
@tsullivan
Copy link
Member Author

/ci

@tsullivan tsullivan force-pushed the search-bar/use-parse-query-update branch from ba37dc5 to 4461eb8 Compare September 5, 2024 21:47
@tsullivan
Copy link
Member Author

/ci

@tsullivan tsullivan force-pushed the search-bar/use-parse-query-update branch from 4461eb8 to fbc5028 Compare September 6, 2024 17:41
@tsullivan tsullivan marked this pull request as ready for review September 6, 2024 17:41
@tsullivan tsullivan requested review from a team as code owners September 6, 2024 17:41
};
}

const searchTerm = getSearchTerm(query);
const filterValues = applyAliases(getFieldValueMap(query), aliasMap);

const unknownFilters = [...filterValues.entries()]
Copy link
Member Author

Choose a reason for hiding this comment

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

In the previous code, nothing was handling the unknowns field from the result. This PR takes the opinion that pulling out unknown field clauses and making them unusable is not the user's intention.

},
});
});

it('handles unknowns field clauses', () => {
Copy link
Member Author

@tsullivan tsullivan Sep 6, 2024

Choose a reason for hiding this comment

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

See my comment below for explanation

@tsullivan tsullivan added the Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) label Sep 6, 2024
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Works as expected and code looks fine.
LGTM

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

For the following use case it works great on Discover page but not on Saved Objects page. Is it expected?

Screenshot 2024-09-11 at 14 22 23

@tsullivan
Copy link
Member Author

@jughosta

For the following use case it works great on Discover page but not on Saved Objects page. Is it expected?

Yes, this is expected because Elasticsearch seems to be using the default analyzer that breaks down the input into individual terms and then searches for documents that contain any of those terms.

See: https://www.elastic.co/guide/en/elasticsearch/reference/current/analysis-standard-analyzer.html

@tsullivan tsullivan requested a review from jughosta September 17, 2024 23:20
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/test/alerting_api_integration/security_and_spaces/group1/config.ts / alerting api integration security and spaces enabled Alerts - Group 1 alerts backfill rule runs ad hoc backfill task should run all execution sets of a scheduled backfill and correctly generate alerts

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
savedObjectsFinder 5.2KB 5.2KB +41.0B
savedObjectsManagement 84.5KB 84.5KB +41.0B
total +82.0B

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.5KB -44.0B

History

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

@tsullivan tsullivan added v8.16.0 backport:version Backport to applied version labels labels Sep 18, 2024
@tsullivan tsullivan merged commit 460ca2a into elastic:main Sep 19, 2024
23 checks passed
@tsullivan tsullivan deleted the search-bar/use-parse-query-update branch September 19, 2024 16:02
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 19, 2024
…cify recognized fields (elastic#190464)

This PR adds a new option to the `SchemaType` interface for parsing a
query from a search in EuiSearchBar. This new field controls how
EuiSearchBar text is parsed into a Query object. It enables better
accuracy in how search terms are parsed when they include a `:`
character.

## Release note
Fixed an issue when using search bars with a term that includes a colon
`:` character.

## Summary

Closes elastic#184496
Depends on elastic/eui#7960

**GLOBAL SEARCH: BEFORE**

![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)

**GLOBAL SEARCH: AFTER**

![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)

**SAVED OBJECTS MANAGEMENT: BEFORE**

![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)

**SAVED OBJECTS MANAGEMENT: AFTER**

![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)

**SAVED OBJECTS FINDER: BEFORE**

![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)

**SAVED OBJECTS FINDER: AFTER**

![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)

## Checklist
 - [x] Ensure that filtering using `type:` and `tags:` still works

(cherry picked from commit 460ca2a)
@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 Sep 19, 2024
…to specify recognized fields (#190464) (#193448)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Global Search, Saved Objects Management] Use new parse option to
specify recognized fields
(#190464)](#190464)

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

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

<!--BACKPORT [{"author":{"name":"Tim
Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-19T16:02:45Z","message":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized fields (#190464)\n\nThis PR adds a new option to the
`SchemaType` interface for parsing a\r\nquery from a search in
EuiSearchBar. This new field controls how\r\nEuiSearchBar text is parsed
into a Query object. It enables better\r\naccuracy in how search terms
are parsed when they include a `:`\r\ncharacter.\r\n\r\n## Release
note\r\nFixed an issue when using search bars with a term that includes
a colon\r\n`:` character.\r\n\r\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/184496\r\nDepends on
https://github.com/elastic/eui/pull/7960\r\n\r\n**GLOBAL SEARCH:
BEFORE**\r\n\r\n![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)\r\n\r\n**GLOBAL
SEARCH:
AFTER**\r\n\r\n![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
BEFORE**\r\n\r\n![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
AFTER**\r\n\r\n![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)\r\n\r\n**SAVED
OBJECTS FINDER:
BEFORE**\r\n\r\n![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)\r\n\r\n**SAVED
OBJECTS FINDER:
AFTER**\r\n\r\n![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)\r\n\r\n##
Checklist\r\n - [x] Ensure that filtering using `type:` and `tags:`
still
works","sha":"460ca2a83f0fd14b9d8c78c6b695e742c3f25930","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:SharedUX","v8.16.0","backport:version"],"title":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized
fields","number":190464,"url":"https://github.com/elastic/kibana/pull/190464","mergeCommit":{"message":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized fields (#190464)\n\nThis PR adds a new option to the
`SchemaType` interface for parsing a\r\nquery from a search in
EuiSearchBar. This new field controls how\r\nEuiSearchBar text is parsed
into a Query object. It enables better\r\naccuracy in how search terms
are parsed when they include a `:`\r\ncharacter.\r\n\r\n## Release
note\r\nFixed an issue when using search bars with a term that includes
a colon\r\n`:` character.\r\n\r\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/184496\r\nDepends on
https://github.com/elastic/eui/pull/7960\r\n\r\n**GLOBAL SEARCH:
BEFORE**\r\n\r\n![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)\r\n\r\n**GLOBAL
SEARCH:
AFTER**\r\n\r\n![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
BEFORE**\r\n\r\n![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
AFTER**\r\n\r\n![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)\r\n\r\n**SAVED
OBJECTS FINDER:
BEFORE**\r\n\r\n![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)\r\n\r\n**SAVED
OBJECTS FINDER:
AFTER**\r\n\r\n![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)\r\n\r\n##
Checklist\r\n - [x] Ensure that filtering using `type:` and `tags:`
still
works","sha":"460ca2a83f0fd14b9d8c78c6b695e742c3f25930"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190464","number":190464,"mergeCommit":{"message":"[Global
Search, Saved Objects Management] Use new parse option to specify
recognized fields (#190464)\n\nThis PR adds a new option to the
`SchemaType` interface for parsing a\r\nquery from a search in
EuiSearchBar. This new field controls how\r\nEuiSearchBar text is parsed
into a Query object. It enables better\r\naccuracy in how search terms
are parsed when they include a `:`\r\ncharacter.\r\n\r\n## Release
note\r\nFixed an issue when using search bars with a term that includes
a colon\r\n`:` character.\r\n\r\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/184496\r\nDepends on
https://github.com/elastic/eui/pull/7960\r\n\r\n**GLOBAL SEARCH:
BEFORE**\r\n\r\n![akshfgkalsfh-before](https://github.com/user-attachments/assets/22377a3e-394e-43fb-83db-ae2477ce22d7)\r\n\r\n**GLOBAL
SEARCH:
AFTER**\r\n\r\n![akshfgkalsfh-after](https://github.com/user-attachments/assets/406d56eb-c946-493b-94f3-abb0380611f3)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
BEFORE**\r\n\r\n![okjoyofjiuh-before](https://github.com/user-attachments/assets/c1c56572-31aa-41df-b0c5-3eef421a06c5)\r\n\r\n**SAVED
OBJECTS MANAGEMENT:
AFTER**\r\n\r\n![okjoyofjiuh-after](https://github.com/user-attachments/assets/9e19bbcf-72e7-43d5-a9e7-3c5805632a38)\r\n\r\n**SAVED
OBJECTS FINDER:
BEFORE**\r\n\r\n![lfdgnhklfd-before](https://github.com/user-attachments/assets/b826987d-8af6-4c20-93b0-0d0bb76d9501)\r\n\r\n**SAVED
OBJECTS FINDER:
AFTER**\r\n\r\n![lfdgnhklfd-after](https://github.com/user-attachments/assets/e8e007b9-91f7-4209-bfd5-ce43a2cbc894)\r\n\r\n##
Checklist\r\n - [x] Ensure that filtering using `type:` and `tags:`
still
works","sha":"460ca2a83f0fd14b9d8c78c6b695e742c3f25930"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Tim Sullivan <tsullivan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:fix 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.

Choose a source dialog box returns unexpected results when searching data views with colon
7 participants