-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Discover] Extend URL state by selected data source #181963
Labels
enhancement
New value added to drive a business result
Project:OneDiscover
Enrich Discover with contextual awareness
Team:DataDiscovery
Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
Comments
kertal
added
Project:OneDiscover
Enrich Discover with contextual awareness
Team:DataDiscovery
Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
labels
Apr 29, 2024
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
This was referenced Apr 30, 2024
davismcphee
added a commit
that referenced
this issue
May 3, 2024
#182063) ## Summary These commits are split out from my working branch for #181963 to make reviewing easier. Our state management code is currently split between a folder called `services` and the `utils` folder. This makes working with the code more difficult, especially where One Discover will soon introduce additional complexity. To make the state management code easier to work with, I moved all of the related files into a single folder called `state_management` with a `utils` subfolder for its utility functions. Similarly, all of our data fetching code was spread between files in the `utils` folder. For the same reason, I moved these files into a new folder called `data_fetching`, since they will also undergo a lot of changes for One Discover. Part of #181963. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
davismcphee
added a commit
that referenced
this issue
May 15, 2024
## Summary This PR replaces the `index` property of Discover's app state with a new `dataSource` property, containing two initial types: `dataView` and `esql`. The new data source abstraction will be used in the One Discover data source context resolution process, as well as preparing Discover to support additional data sources as needed in the future. Additionally, it creates a clearer division in the Discover state between "data view mode" and "ES|QL mode", where previously this was implicit based on whether the `query` property was an ES|QL query vs KQL or Lucene. The goal of this PR is to add initial support for the `dataSource` property without introducing broader changes to the Discover state management. However, these changes open up opportunities for future improvements to simplify the state management, such as checking the data source type to determine which mode Discover is in instead of always checking the query directly. These types of enhancements can be built on this foundation in followup PRs. Part of #181963. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
10 tasks
davismcphee
added a commit
that referenced
this issue
May 18, 2024
## Summary This PR updates Discover to rely on `dataSource` for determining if ES|QL mode is enabled instead of relying on `query` directly. This creates a clearer separation between the modes and moves Discover toward supporting multiple data source types. It also includes a number of cleanups around ES|QL mode that make the intent of the code clearer and removes tech debt / code duplication. Changes included in the PR: - Add a new `useIsEsqlMode` hook that relies on `dataSource` for checking if Discover is in ES|QL mode. - Remove "raw record type" concept in Discover and replace it with ES|QL `dataSource` checks. - Remove references to `isPlainRecord` and replace them with ES|QL `dataSource` checks. - Remove `isTextBasedQuery` utility function and replace it with ES|QL `dataSource` checks or `isOfAggregateQueryType` where casting is necessary. - Replace references to `isOfAggregateQueryType` with ES|QL `dataSource` checks except where casting is necessary. - Replace other references to "text based" with "ES|QL" for clarity and consistency. Closes #181963. ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [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 - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New value added to drive a business result
Project:OneDiscover
Enrich Discover with contextual awareness
Team:DataDiscovery
Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL.
📓 Summary
Today Discover supports two different data sources:
Saved search (contains an ES|QL query or data view reference)Saved search is not a data source itself, but contains a data source (ES|QL query or data view)For One Discover, this list will need to extend to an unknown number of data sources, which the current URL structure and application state are not well suited for. To support this requirement, we will introduce an abstraction called a “data source” which represents some target for data exploration in Discover.
Supported data sources will be modelled as a union type, and passed to Discover on navigation through the URL. This is an example of what this type might look like:
The data source abstraction will later be used as part of the context resolution process, but the first step is to update Discover's URL handling and state management to support this concept.
✔ Acceptance Criteria
The text was updated successfully, but these errors were encountered: