[Lens] [ES|QL] Single layer ES|QL conversion#248078
Conversation
0dac017 to
17c6c4d
Compare
🔍 Preview links for changed docs |
204cf56 to
c51bbc2
Compare
|
Pinging @elastic/kibana-visualizations (Team:Visualizations) |
src/platform/packages/shared/kbn-unified-histogram/components/chart/chart_config_panel.tsx
Show resolved
Hide resolved
|
@walterra let me know when it is ready to test again |
stratoula
left a comment
There was a problem hiding this comment.
I confirm that the DSL mode works well now with the setting off. Thanks a lot for creating issues for the bugs I had found! Ping me at the next PRs to test again
(I didn't check the code)
x-pack/platform/plugins/shared/lens/public/datasources/form_based/generate_esql_query.ts
Outdated
Show resolved
Hide resolved
x-pack/platform/plugins/shared/lens/public/datasources/form_based/generate_esql_query.ts
Outdated
Show resolved
Hide resolved
x-pack/platform/plugins/shared/lens/public/datasources/form_based/generate_esql_query.ts
Outdated
Show resolved
Hide resolved
| const currentDatasourceId = getActiveDatasourceIdFromDoc( | ||
| currentAttributes | ||
| ) as SupportedDatasourceId; |
There was a problem hiding this comment.
would you mind refactoring quickly the getActiveDatasourceIdFromDoc to return that SupportedDatasourceId? everone probably would benefit from this
There was a problem hiding this comment.
I looked into this and unfortunately it's a bit of a rabbit hole. If I fix the return type on that function I trigger another round of TS errors in 10+ files unrelated to this PR. Happy to pick this up in a follow up though.
There was a problem hiding this comment.
yea, let's consider this in a follow up
There was a problem hiding this comment.
created an issue here: https://github.com/elastic/kibana-team/issues/2747
.../plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/convert_to_text_based_layer.ts
Outdated
Show resolved
Hide resolved
x-pack/platform/plugins/shared/lens/public/datasources/form_based/to_expression.ts
Show resolved
Hide resolved
| } | ||
| Object.values(layer.columns).forEach((column) => { | ||
| columnLabelMap[column.columnId] = uniqueLabelGenerator(column.fieldName); | ||
| columnLabelMap[column.columnId] = uniqueLabelGenerator(column.label ?? column.fieldName); |
There was a problem hiding this comment.
I added this change to preserve custom labels on chart configurations, otherwise it would always just use the field name.
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
cc @walterra |
...tform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/convert_to_esql_modal.tsx
Show resolved
Hide resolved
.../plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/convert_to_text_based_layer.ts
Show resolved
Hide resolved
davismcphee
left a comment
There was a problem hiding this comment.
Pulled the latest changes and confirmed that the ES|QL editor is still hidden in the vis edit flyout, and I was able to edit the vis and save it with my Discover session. Thanks for the ping!
## Summary Follow up to elastic#246209. Implements elastic/kibana-team#2391. Also implements part of elastic/kibana-team#2179. This applies the work done in PoCs elastic#199632 and elastic#245716 to the new UI flow with the conversion modal added in elastic#246209. This PR wires up the ES|QL conversion modal to actually perform the datasource conversion. Related to that, it also cleans up the inline editing flyout code by removing redundant props. - `handleConvertToEsql` callback to switch flyout from formBased to textBased mode - Updates visualization state to remap column IDs (xAccessor, accessors, splitAccessor) to ES|QL field names - Reduces prop drilling of `datasourceId` and `canEditTextBasedQuery`, adds `hideTextBasedEditor` to Redux state for Discover integration. Note: The full feature is still behind a dev-mode flag, it's not enabled in production. The scope of this PR is to enable the full UI flow of DSL->ES|QL conversion and enabling the conversion based on wether the button to convert is enabled. Out of scope of the PR is fine tuning actual conversions, we'll pick that up in follow-ups. https://github.com/user-attachments/assets/d545c759-417b-4705-a820-bc669ca17ae2 ### To test that the conversion button isn't showing up in production builds: ``` # local kibana build node scripts/build --skip-os-packages --skip-docker-contexts # create ./build/default/kibana-9.4.0-SNAPSHOT-darwin-aarch64/config/kibana.yml elasticsearch.hosts: - https://localhost:9200/ elasticsearch.password: **** elasticsearch.username: kibana_system # run kibana ./build/default/kibana-9.4.0-SNAPSHOT-darwin-aarch64/bin/kibana --elasticsearch.ssl.verificationMode=none ``` ### 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 - [x] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. - [x] Used `cursor-cli` with `opus-4.5` to refactor the PoCs onto the new UI flow. --------- Co-authored-by: Stratou <efstratia.kalafateli@elastic.co>
#249819) ## Summary Close #250383 Close #250384 - Init scout in Lens - Add `switchToEditMode` and `openInLineEditor` to the dashboard app - Create lens app and add the following methods: - `getConvertToEsqlButton` - `getConvertToEsqModal` - `getConvertToEsqModalConfirmButton` - Add `should display ES|QL conversion modal` test For the test, we have used the LOGSTASH ES archive. > [!NOTE] > The complete conversion flow will be tested in a separate PR, once #248078 is merged. > [!IMPORTANT] > The test introduced in this pull request has been skipped until we introduce a feature flag to hide the feature we want to test: elastic/kibana-team#2740 ### How to run the tests Run server: ``` node scripts/scout.js start-server --stateful ``` In a separate terminal, run tests: ``` npx playwright test --project local --grep @ess --config x-pack/platform/plugins/shared/lens/test/scout/ui/ --ui ``` ## Checklist - [ ] [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 - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [x] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
## Summary - Follow up to #248078. - Fixes elastic/kibana-team#2747 Updates in this PR: - Moves ES|QL conversion types into a dedicated file (`esql_conversion_types.ts`) - Return type for `getActiveDatasourceIdFromDoc`, this is what triggers more or less all other updates and test updates - `getEditLensConfiguration` is a factory that returns a React component. The code was structured in a way that didn't let React linting kick in, this missed a "rule of hooks" where we returned early from the component but used other hooks after the return. This PR refactors the factory and breaks out the component to the top level of the file so these linting rules kickin. ### 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
Summary
Follow up to #246209. Implements https://github.com/elastic/kibana-team/issues/2391. Also implements part of https://github.com/elastic/kibana-team/issues/2179.
This applies the work done in PoCs #199632 and #245716 to the new UI flow with the conversion modal added in #246209.
This PR wires up the ES|QL conversion modal to actually perform the datasource conversion. Related to that, it also cleans up the inline editing flyout code by removing redundant props.
handleConvertToEsqlcallback to switch flyout from formBased to textBased modedatasourceIdandcanEditTextBasedQuery, addshideTextBasedEditorto Redux state for Discover integration.Note: The full feature is still behind a dev-mode flag, it's not enabled in production. The scope of this PR is to enable the full UI flow of DSL->ES|QL conversion and enabling the conversion based on wether the button to convert is enabled. Out of scope of the PR is fine tuning actual conversions, we'll pick that up in follow-ups.
lens-ux-esql-conversion-0003.mp4
To test that the conversion button isn't showing up in production builds:
Checklist
release_note:breakinglabel should be applied in these situations.backport:*labels.cursor-cliwithopus-4.5to refactor the PoCs onto the new UI flow.