[ES|QL][Lens] Improves the ES|QL suggestions logic when a query changes by stratoula · Pull Request #231767 · elastic/kibana

stratoula · 2025-08-14T09:57:00Z

Summary

Improves this #220939

How it improves it:

When the user edits an existing chart, we try to keep the chart type the viz has as long as the query returns compatible columns. If the user changes the chart type from the dropdown then this means he deliberately wants to change it and we try to respect the new choice.
When the user creates a new chart we are following the suggestions until the user starts changing something in the visualization parameters panel. Then we are assuming that the user wants this chart type and we try to respect it (according to this comment). If the user changes the chart type from the dropdown then we try to respect this choice.
There is a special treatment for tables as they are not suggested from the suggestions api (they are always hidden)
The above work as long as the query doesnt change significantly. When the query changes significantly there is the possibility that the columns used in the chart are not being returned from the _query api. For example if I have a stats by date_histogram and I change to from index you have now all index columns but not the histogram buckets, this will reset. In the future we should inform the user that this change will reset the visualization.

TL;DR

If the user edits an existing chart then we try to respect the chart type defined there
If the user creates a new one we follow the suggestions as long as the user hasn't changed something in the visualization settings panel
If the user changes the query and the returned columns don't contain the visualization columns we reset. Here we should inform the users in the future

Checklist

Unit or functional tests were updated or added to match the most common scenarios

stratoula · 2025-08-14T10:00:44Z

x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/helpers.ts

  let queryColumns = results.response.columns;
  // if the query columns are empty, we need to use the all_columns property
  // which has all columns regardless if they have data or not
-  if (queryColumns.length === 0 && results.response.all_columns) {


Make sure that all columns are returned if they exist in the payload. This ensures that the textBasedColumns that are passed in the api has all columns with or without data.

stratoula · 2025-08-14T10:02:01Z

x-pack/platform/plugins/shared/lens/public/lens_suggestions_api/helpers.ts

-        layer.columns?.some(
-          (c: { fieldName: string }) =>
-            !context?.textBasedColumns?.find((col) => col.id === c.fieldName)
-        ) || layer.columns?.length !== context?.textBasedColumns?.length


I removed the columns length check, we dont want to revert if the user selected different columns as long as they exist in the response

stratoula · 2025-08-14T13:10:27Z

x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/helpers.test.ts

    });
  });

-  describe('injectESQLQueryIntoLensLayers', () => {


Move this to suggestions api tests

stratoula · 2025-08-14T13:10:46Z

x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/helpers.ts

  }
  return undefined;
 };
-


Moves this into suggestions api logic

stratoula · 2025-08-14T13:11:24Z

x-pack/platform/plugins/shared/lens/public/lens_suggestions_api/helpers.ts

-            !context?.textBasedColumns?.find((col) => col.id === c.fieldName)
-        ) || layer.columns?.length !== context?.textBasedColumns?.length
+  // Check if index patterns match when context has a query
+  if (context && 'query' in context && context.query && 'esql' in context.query) {


Here we will return the suggestion if the user changed datasource

kibanamachine · 2025-08-18T07:28:34Z

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#9164

[✅] x-pack/platform/test/functional/apps/lens/group7/config.ts: 50/50 tests passed.

see run history

…no-cache --fix'

elasticmachine · 2025-08-25T09:46:47Z

Pinging @elastic/kibana-visualizations (Team:Visualizations)

markov00 · 2025-09-08T12:18:22Z

x-pack/platform/plugins/shared/lens/public/settings_storage.tsx

+ * Get the user's preferred chart type from localStorage
+ */
+export const getUserChartTypeFromLocalStorage = (): string | null => {
+  const storage = new Storage(localStorage);


I think it is better to use the sessionStorage instead of the localStorage to avoid conflicts and changes of the same variable across different tabs.
I tested and clicking Close on a tab besically removes the currently set viz type from the other tab and viceversa.

Good point, I will address tomorrow

Marco changed it to use session storage instead 211cf65

x-pack/platform/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/helpers.ts

...platform/plugins/shared/lens/public/react_embeddable/inline_editing/setup_inline_editing.tsx

dej611 · 2025-09-09T09:59:04Z

...m/plugins/shared/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.tsx

Tested few cases here, but the totality of the bugs I've found are tied to the lack of handling of the ESC key down event on the new logic.

Typical bug scenario:

create a ES|QL chart on a dashboard

close the inline initial editing

reopen the inline editor

tweak the query or change the chart type

now press ESC

the chart is saved in the current state

A a user the ESC button is a tricky point: it "sounds" something similar to a Cancel than a Save logically, but I can see how much of a slippery slope that can be.

Maybe pressing ESC should have a confirmation modal before cancelling? cc @gvnmagni

The ESC works the same in the main (I mean it applies the changes). You mean that it doesnt reset the session? I don't think it will be a problem mostly because when you edit another panel it will add to the session storage the new chart type.

Which bug have you found that is related with the ESC button functionality and the changes this PR is introducing?

Synced with Marco offline, I can reset on ESC, there is a keydown handler already

Done here ac2e092

I didn't know we had added this handler, thanx for pointing out Marco!

…dit_on_the_fly/helpers.ts Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

…line_editing/setup_inline_editing.tsx Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

elasticmachine · 2025-09-09T15:12:03Z

💚 Build Succeeded

Buildkite Build
Commit: ac2e092

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`lens`	1414	1415	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`lens`	1.5MB	1.5MB	+904.0B

History

💔 Build #337295 failed f2eb29a
💚 Build #336623 succeeded b367f5d
💛 Build #333926 was flaky 19932bf
💛 Build #332019 was flaky 088b9be
💛 Build #331032 was flaky 2930635
💚 Build #330238 succeeded c7bf8ae

…es (elastic#231767) ## Summary Improves this elastic#220939 ![meow](https://github.com/user-attachments/assets/87f68a1a-1110-4969-9f93-b5fa1c740c89) How it improves it: 1. When the user edits an existing chart, we try to keep the chart type the viz has as long as the query returns compatible columns. If the user changes the chart type from the dropdown then this means he deliberately wants to change it and we try to respect the new choice. 2. When the user creates a new chart we are following the suggestions until the user starts changing something in the visualization parameters panel. Then we are assuming that the user wants this chart type and we try to respect it (according to this [comment](elastic#220939 (comment))). If the user changes the chart type from the dropdown then we try to respect this choice. 4. There is a special treatment for tables as they are not suggested from the suggestions api (they are always hidden) 5. **The above work as long as the query doesnt change significantly**. When the query changes significantly there is the possibility that the columns used in the chart are not being returned from the _query api. For example if I have a stats by date_histogram and I change to `from index` you have now all index columns but not the histogram buckets, this will reset. In the future we should inform the user that this change will reset the visualization. TL;DR - If the user edits an existing chart then we try to respect the chart type defined there - If the user creates a new one we follow the suggestions as long as the user hasn't changed something in the visualization settings panel - If the user changes the query and the returned columns don't contain the visualization columns we reset. Here we should inform the users in the future ### 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 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

…es (#231767) ## Summary Improves this #220939 ![meow](https://github.com/user-attachments/assets/87f68a1a-1110-4969-9f93-b5fa1c740c89) How it improves it: 1. When the user edits an existing chart, we try to keep the chart type the viz has as long as the query returns compatible columns. If the user changes the chart type from the dropdown then this means he deliberately wants to change it and we try to respect the new choice. 2. When the user creates a new chart we are following the suggestions until the user starts changing something in the visualization parameters panel. Then we are assuming that the user wants this chart type and we try to respect it (according to this [comment](#220939 (comment))). If the user changes the chart type from the dropdown then we try to respect this choice. 4. There is a special treatment for tables as they are not suggested from the suggestions api (they are always hidden) 5. **The above work as long as the query doesnt change significantly**. When the query changes significantly there is the possibility that the columns used in the chart are not being returned from the _query api. For example if I have a stats by date_histogram and I change to `from index` you have now all index columns but not the histogram buckets, this will reset. In the future we should inform the user that this change will reset the visualization. TL;DR - If the user edits an existing chart then we try to respect the chart type defined there - If the user creates a new one we follow the suggestions as long as the user hasn't changed something in the visualization settings panel - If the user changes the query and the returned columns don't contain the visualization columns we reset. Here we should inform the users in the future ### 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 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

[ES|QL][Lens] Improves the datatable suggestions

ded2fdf

stratoula commented Aug 14, 2025

View reviewed changes

stratoula added 2 commits August 14, 2025 12:56

Some cleanup

416341a

Fixes

90ba1ef

stratoula commented Aug 14, 2025

View reviewed changes

stratoula added 12 commits August 14, 2025 15:32

Fix

07a5ef2

Change the logic

c1dcc5e

Fix test

04e0c60

Remove console error

070934a

Add to lens settings-storage

15c800d

Small fix

0a71574

Add some FTs

b930957

Further fixes

fa5182d

Simplifications

58fafdf

Remove wrong commit

af83a5d

Merge with main and resolve conflict

5a0a162

Do not reset even if the chart types do not match

c7bf8ae

stratoula added the Feature:ES|QL ES|QL related features in Kibana label Aug 18, 2025

Merge branch 'main' into lens-esql-improve-table-suggestions

2930635

stratoula changed the title ~~[ES|QL][Lens] Improves the datatable suggestions~~ Aug 20, 2025

stratoula and others added 5 commits August 25, 2025 07:29

Merge branch 'main' into lens-esql-improve-table-suggestions

8ea76a6

Save the chart type in local storage when the user gets in a dirty mode

ddc8e04

Cleanup

ec96635

[CI] Auto-commit changed files from 'node scripts/eslint_all_files --…

ad41159

…no-cache --fix'

Cleanup

088b9be

stratoula added the release_note:enhancement label Aug 25, 2025

stratoula added backport:skip This PR does not require backporting v9.2.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// Feature:Lens labels Aug 25, 2025

stratoula marked this pull request as ready for review August 25, 2025 09:46

stratoula requested a review from a team as a code owner August 25, 2025 09:46

stratoula added 5 commits August 27, 2025 08:04

Merge branch 'main' into lens-esql-improve-table-suggestions

3b4d7d9

Merge branch 'main' into lens-esql-improve-table-suggestions

19932bf

Merge branch 'main' into lens-esql-improve-table-suggestions

11a9735

Merge branch 'main' into lens-esql-improve-table-suggestions

5fd60f0

Merge branch 'main' into lens-esql-improve-table-suggestions

b367f5d

markov00 reviewed Sep 8, 2025

View reviewed changes

Use session storage instead

211cf65

dej611 reviewed Sep 9, 2025

View reviewed changes

stratoula and others added 4 commits September 9, 2025 12:59

Update x-pack/platform/plugins/shared/lens/public/app_plugin/shared/e…

cb80dec

…dit_on_the_fly/helpers.ts Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

Update x-pack/platform/plugins/shared/lens/public/react_embeddable/in…

f2eb29a

…line_editing/setup_inline_editing.tsx Co-authored-by: Marco Liberati <dej611@users.noreply.github.com>

Fix linting

707a0ba

Reset on ESC

ac2e092

dej611 approved these changes Sep 10, 2025

View reviewed changes

Merge branch 'main' into lens-esql-improve-table-suggestions

963c366

stratoula enabled auto-merge (squash) September 10, 2025 08:10

stratoula merged commit c1f9728 into elastic:main Sep 10, 2025
12 checks passed

florent-leborgne added the needs_docs label Sep 22, 2025

florent-leborgne mentioned this pull request Sep 22, 2025

[ES|QL visualizations] Improvements to chart configuration preservation upon query update elastic/docs-content#3087

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ES|QL][Lens] Improves the ES|QL suggestions logic when a query changes#231767

[ES|QL][Lens] Improves the ES|QL suggestions logic when a query changes#231767
stratoula merged 32 commits intoelastic:mainfrom
stratoula:lens-esql-improve-table-suggestions

stratoula commented Aug 14, 2025 •

edited

Loading

stratoula Aug 14, 2025

stratoula Aug 14, 2025 •

edited

Loading

stratoula Aug 14, 2025

stratoula Aug 14, 2025

stratoula Aug 14, 2025

kibanamachine commented Aug 18, 2025

elasticmachine commented Aug 25, 2025

markov00 Sep 8, 2025

stratoula Sep 8, 2025

stratoula Sep 9, 2025

Uh oh!

Uh oh!

dej611 Sep 9, 2025

stratoula Sep 9, 2025 •

edited

Loading

stratoula Sep 9, 2025

stratoula Sep 9, 2025

elasticmachine commented Sep 9, 2025

Uh oh!

Labels

6 participants

Conversation

stratoula commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Choose a reason for hiding this comment

stratoula Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Aug 18, 2025

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#9164

elasticmachine commented Aug 25, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

stratoula Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Sep 9, 2025

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

Uh oh!

Labels

6 participants

stratoula commented Aug 14, 2025 •

edited

Loading

stratoula Aug 14, 2025 •

edited

Loading

stratoula Sep 9, 2025 •

edited

Loading