[Controls] Remove availableOptions from ES|QL values from query saved object#231690
Merged
Zacqary merged 12 commits intoelastic:mainfrom Aug 25, 2025
Merged
[Controls] Remove availableOptions from ES|QL values from query saved object#231690Zacqary merged 12 commits intoelastic:mainfrom
Zacqary merged 12 commits intoelastic:mainfrom
Conversation
Zacqary
added
release_note:fix
Team:Presentation
Contributor
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
Contributor
|
Pinging @elastic/kibana-esql (Team:ESQL) |
stratoula
approved these changes
Aug 14, 2025
Contributor
stratoula
left a comment
stratoula
left a comment
There was a problem hiding this comment.
This looks good! Thanx for tackling this Zac
| width?: ControlWidthOptions; | ||
| title: string; | ||
| availableOptions: string[]; | ||
| availableOptions?: string[]; |
Contributor
There was a problem hiding this comment.
Can you add a comment here mentioning the case that this is optional?
Contributor
Author
There was a problem hiding this comment.
Sure thing. At first I was hoping to make this more self-documenting by splitting this into type ESQLControlState = StaticESQLControlState | ValuesESQLControlState but then I would've had to put new complicated typeguards in a ton of places. Maybe some good tech debt to tackle later but in the interest of keeping this PR small and quick I didn't want to go through all that right now.
…com/Zacqary/kibana into 231668-esql-remove-availableoptions
nreese
reviewed
Aug 14, 2025
| ...selections.getLatestState(), | ||
| }; | ||
|
|
||
| const rawState = |
Contributor
There was a problem hiding this comment.
Can this logic be moved into selections.getLatestState?
Contributor
Author
There was a problem hiding this comment.
No, the availableOptions state is still needed internally in order to display the list of available options. We just want to omit it before serializing, so that it's not saved.
The test logic in get_esql_control_factory.test.tsx documents how this is expected to work.
Contributor
There was a problem hiding this comment.
No, the availableOptions state is still needed internally in order to display the list of available options
availabeOptions is published with availableOptions$. Selections manager should expose availableOptions$ if availabeOptions are needed outside of the selection manager. selections.getLatestState() should not include availabeOptions if they are discarded during serialization
Contributor
Author
There was a problem hiding this comment.
You're right, it actually does still work if I do this at the getLatestState level. Doesn't require any refactoring. Pushing a fix
| }); | ||
| }); | ||
|
|
||
| test('should load availableOptions but not serialize them', async () => { |
Contributor
There was a problem hiding this comment.
How about moving this test to src/platform/plugins/shared/controls/public/controls/esql_control/esql_control_selections.test.ts since initializeESQLControlSelections is responsible for managing this subscription.
Contributor
Author
There was a problem hiding this comment.
This is more of an integration test. The component always uses the availableOptions state internally to render the list of available options in the UI regardless of whether it's a Values From Query or Static Values control. We only want to serialize and save it for Static Values control types, and only fetch at runtime for Values From Query.
This does make it a bit confusing, but since this is a high-priority fix and we're planning on cleaning up this logic with the Data/ESQL control factory unification, I opted not to refactor here.
Contributor
There was a problem hiding this comment.
This does make it a bit confusing, but since this is a high-priority fix and we're planning on cleaning up this logic with the Data/ESQL control factory unification, I opted not to refactor here.
Even if its a high priority fix, we should add the test at the right abstraction layer. selections manger getLatestState should drop availableOptions. Then the test should just test selections manager getLatestState.
nreese
reviewed
Aug 14, 2025
|
|
||
| const onCreateControl = useCallback(async () => { | ||
| if (controlState && controlState.availableOptions.length) { | ||
| if (controlState && controlState.availableOptions?.length) { |
Contributor
There was a problem hiding this comment.
how about moving controlState.availableOptions logic into src/platform/plugins/shared/controls/public/controls/esql_control/get_esql_control_factory.tsx onSaveControl so serialize implementation details do not leak into the UI.
nreese
reviewed
Aug 14, 2025
Contributor
There was a problem hiding this comment.
Now that ESQL options are fetched on load, I think we need to implement PublishesDataLoading to show users when ESQL options are getting fetched.
…availableoptions # Conflicts: # src/platform/plugins/shared/controls/public/controls/esql_control/get_esql_control_factory.tsx # src/platform/plugins/shared/esql/public/triggers/esql_controls/control_flyout/index.tsx
nreese
reviewed
Aug 25, 2025
| return { | ||
| selectedOptions: selectedOptions$.getValue() ?? [], | ||
| availableOptions: availableOptions$.getValue() ?? [], | ||
| ...(controlType$.getValue() === EsqlControlType.STATIC_VALUES |
Contributor
There was a problem hiding this comment.
Can you add 2 unit tests for getLatestState, one with static values and one with valuesFromQuery?
Contributor
Author
There was a problem hiding this comment.
I was screaming at this comment like "I DID" but turns out I forgot to git add -A
It is very Monday today
nreese
approved these changes
Aug 25, 2025
Contributor
nreese
left a comment
nreese
left a comment
There was a problem hiding this comment.
kibana-presentation changes LGTM
code review and tested in chrome
Contributor
|
Starting backport for target branches: 8.19, 9.1 |
Contributor
💚 Build Succeeded
Metrics [docs]Async chunks
History
|
Contributor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
Zacqary
added a commit
to Zacqary/kibana
that referenced
this pull request
Aug 26, 2025
… object (elastic#231690) ## Summary Closes elastic#231668 For `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions` property from the control state before saving, from both the control editor and the dashboard. These controls always fetch their `availableOptions` on page load and dashboard refresh, and when the list is extremely long, saving it in a saved object has been causing serious performance issues for some users. `STATIC_VALUES` controls are unaffected, and still save their `availableOptions` in the control state. (cherry picked from commit 56cfa2b) # Conflicts: # src/platform/plugins/shared/controls/public/controls/esql_control/types.ts
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Zacqary
added a commit
to Zacqary/kibana
that referenced
this pull request
Aug 26, 2025
… object (elastic#231690) ## Summary Closes elastic#231668 For `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions` property from the control state before saving, from both the control editor and the dashboard. These controls always fetch their `availableOptions` on page load and dashboard refresh, and when the list is extremely long, saving it in a saved object has been causing serious performance issues for some users. `STATIC_VALUES` controls are unaffected, and still save their `availableOptions` in the control state. (cherry picked from commit 56cfa2b) # Conflicts: # src/platform/plugins/shared/controls/public/controls/esql_control/types.ts
Zacqary
added a commit
that referenced
this pull request
Aug 26, 2025
… saved object (#231690) (#232989) # Backport This will backport the following commits from `main` to `9.1`: - [[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)](#231690) <!--- Backport version: 10.0.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Zac Xeper","email":"Zacqary@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-08-25T20:55:19Z","message":"[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)\n\n## Summary\n\nCloses #231668 \n\nFor `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions`\nproperty from the control state before saving, from both the control\neditor and the dashboard. These controls always fetch their\n`availableOptions` on page load and dashboard refresh, and when the list\nis extremely long, saving it in a saved object has been causing serious\nperformance issues for some users.\n\n`STATIC_VALUES` controls are unaffected, and still save their\n`availableOptions` in the control state.","sha":"56cfa2b8c214cb7d3d01a8487e85754bf45815e5","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","loe:small","impact:critical","Project:Controls","Team:ESQL","backport:version","v9.2.0","v9.1.3","v8.19.3"],"title":"[Controls] Remove availableOptions from ES|QL values from query saved object","number":231690,"url":"https://github.com/elastic/kibana/pull/231690","mergeCommit":{"message":"[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)\n\n## Summary\n\nCloses #231668 \n\nFor `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions`\nproperty from the control state before saving, from both the control\neditor and the dashboard. These controls always fetch their\n`availableOptions` on page load and dashboard refresh, and when the list\nis extremely long, saving it in a saved object has been causing serious\nperformance issues for some users.\n\n`STATIC_VALUES` controls are unaffected, and still save their\n`availableOptions` in the control state.","sha":"56cfa2b8c214cb7d3d01a8487e85754bf45815e5"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/231690","number":231690,"mergeCommit":{"message":"[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)\n\n## Summary\n\nCloses #231668 \n\nFor `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions`\nproperty from the control state before saving, from both the control\neditor and the dashboard. These controls always fetch their\n`availableOptions` on page load and dashboard refresh, and when the list\nis extremely long, saving it in a saved object has been causing serious\nperformance issues for some users.\n\n`STATIC_VALUES` controls are unaffected, and still save their\n`availableOptions` in the control state.","sha":"56cfa2b8c214cb7d3d01a8487e85754bf45815e5"}},{"branch":"9.1","label":"v9.1.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
kibanamachine
added
the
backport missing
Contributor
|
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Zacqary
added a commit
that referenced
this pull request
Aug 28, 2025
…y saved object (#231690) (#232990) # Backport This will backport the following commits from `main` to `8.19`: - [[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)](#231690) <!--- Backport version: 10.0.1 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Zac Xeper","email":"Zacqary@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-08-25T20:55:19Z","message":"[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)\n\n## Summary\n\nCloses #231668 \n\nFor `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions`\nproperty from the control state before saving, from both the control\neditor and the dashboard. These controls always fetch their\n`availableOptions` on page load and dashboard refresh, and when the list\nis extremely long, saving it in a saved object has been causing serious\nperformance issues for some users.\n\n`STATIC_VALUES` controls are unaffected, and still save their\n`availableOptions` in the control state.","sha":"56cfa2b8c214cb7d3d01a8487e85754bf45815e5","branchLabelMapping":{"^v9.2.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","loe:small","impact:critical","Project:Controls","Team:ESQL","backport:version","v9.2.0","v9.1.3","v8.19.3"],"title":"[Controls] Remove availableOptions from ES|QL values from query saved object","number":231690,"url":"https://github.com/elastic/kibana/pull/231690","mergeCommit":{"message":"[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)\n\n## Summary\n\nCloses #231668 \n\nFor `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions`\nproperty from the control state before saving, from both the control\neditor and the dashboard. These controls always fetch their\n`availableOptions` on page load and dashboard refresh, and when the list\nis extremely long, saving it in a saved object has been causing serious\nperformance issues for some users.\n\n`STATIC_VALUES` controls are unaffected, and still save their\n`availableOptions` in the control state.","sha":"56cfa2b8c214cb7d3d01a8487e85754bf45815e5"}},"sourceBranch":"main","suggestedTargetBranches":["9.1","8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.2.0","branchLabelMappingKey":"^v9.2.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/231690","number":231690,"mergeCommit":{"message":"[Controls] Remove availableOptions from ES|QL values from query saved object (#231690)\n\n## Summary\n\nCloses #231668 \n\nFor `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions`\nproperty from the control state before saving, from both the control\neditor and the dashboard. These controls always fetch their\n`availableOptions` on page load and dashboard refresh, and when the list\nis extremely long, saving it in a saved object has been causing serious\nperformance issues for some users.\n\n`STATIC_VALUES` controls are unaffected, and still save their\n`availableOptions` in the control state.","sha":"56cfa2b8c214cb7d3d01a8487e85754bf45815e5"}},{"branch":"9.1","label":"v9.1.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.19","label":"v8.19.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
kibanamachine
removed
the
backport missing
KodeRad
pushed a commit
to KodeRad/kibana
that referenced
this pull request
Aug 28, 2025
… object (elastic#231690) ## Summary Closes elastic#231668 For `VALUES_FROM_QUERY` ES|QL controls, omit the `availableOptions` property from the control state before saving, from both the control editor and the dashboard. These controls always fetch their `availableOptions` on page load and dashboard refresh, and when the list is extremely long, saving it in a saved object has been causing serious performance issues for some users. `STATIC_VALUES` controls are unaffected, and still save their `availableOptions` in the control state.
1 task
stratoula
added a commit
that referenced
this pull request
Oct 16, 2025
…ble control (#239315) ## Summary Regression caused from this PR #231690 When you have an existing control and edit it, the query doesnt run to get the available options. As a result the UX is kinda broken. ### Checklist - [ ] [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
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Oct 16, 2025
…ble control (elastic#239315) ## Summary Regression caused from this PR elastic#231690 When you have an existing control and edit it, the query doesnt run to get the available options. As a result the UX is kinda broken. ### Checklist - [ ] [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 84ad1f3)
kibanamachine
added a commit
that referenced
this pull request
Oct 17, 2025
… variable control (#239315) (#239350) # Backport This will backport the following commits from `main` to `9.2`: - [[ES|QL] Displays the available options when editing an existing variable control (#239315)](#239315) <!--- Backport version: 9.6.6 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Stratou","email":"efstratia.kalafateli@elastic.co"},"sourceCommit":{"committedDate":"2025-10-16T13:19:40Z","message":"[ES|QL] Displays the available options when editing an existing variable control (#239315)\n\n## Summary\n\nRegression caused from this PR\nhttps://github.com//pull/231690\n\nWhen you have an existing control and edit it, the query doesnt run to\nget the available options. As a result the UX is kinda broken.\n\n### Checklist\n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"84ad1f31553c96fd09f9baa8bc74ba6a6a17eea6","branchLabelMapping":{"^v9.3.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["regression","release_note:fix","Feature:ES|QL","Team:ESQL","backport:version","v9.2.0","v9.3.0"],"title":"[ES|QL] Displays the available options when editing an existing variable control","number":239315,"url":"https://github.com/elastic/kibana/pull/239315","mergeCommit":{"message":"[ES|QL] Displays the available options when editing an existing variable control (#239315)\n\n## Summary\n\nRegression caused from this PR\nhttps://github.com//pull/231690\n\nWhen you have an existing control and edit it, the query doesnt run to\nget the available options. As a result the UX is kinda broken.\n\n### Checklist\n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"84ad1f31553c96fd09f9baa8bc74ba6a6a17eea6"}},"sourceBranch":"main","suggestedTargetBranches":["9.2"],"targetPullRequestStates":[{"branch":"9.2","label":"v9.2.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.3.0","branchLabelMappingKey":"^v9.3.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/239315","number":239315,"mergeCommit":{"message":"[ES|QL] Displays the available options when editing an existing variable control (#239315)\n\n## Summary\n\nRegression caused from this PR\nhttps://github.com//pull/231690\n\nWhen you have an existing control and edit it, the query doesnt run to\nget the available options. As a result the UX is kinda broken.\n\n### Checklist\n\n- [ ] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"84ad1f31553c96fd09f9baa8bc74ba6a6a17eea6"}}]}] BACKPORT--> Co-authored-by: Stratou <efstratia.kalafateli@elastic.co>
nickpeihl
pushed a commit
to nickpeihl/kibana
that referenced
this pull request
Oct 23, 2025
…ble control (elastic#239315) ## Summary Regression caused from this PR elastic#231690 When you have an existing control and edit it, the query doesnt run to get the available options. As a result the UX is kinda broken. ### Checklist - [ ] [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
NicholasPeretti
pushed a commit
to NicholasPeretti/kibana
that referenced
this pull request
Oct 27, 2025
…ble control (elastic#239315) ## Summary Regression caused from this PR elastic#231690 When you have an existing control and edit it, the query doesnt run to get the available options. As a result the UX is kinda broken. ### Checklist - [ ] [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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #231668
For
VALUES_FROM_QUERYES|QL controls, omit theavailableOptionsproperty from the control state before saving, from both the control editor and the dashboard. These controls always fetch theiravailableOptionson page load and dashboard refresh, and when the list is extremely long, saving it in a saved object has been causing serious performance issues for some users.STATIC_VALUEScontrols are unaffected, and still save theiravailableOptionsin the control state.