Skip to content

[Controls Anywhere] Fix initialization of control group renderer#242049

Merged
Heenawter merged 6 commits intoelastic:controlsAnywherefrom
Heenawter:controlsAnywhere_fix-discover-race-condition_2025-11-05
Nov 7, 2025
Merged

[Controls Anywhere] Fix initialization of control group renderer#242049
Heenawter merged 6 commits intoelastic:controlsAnywherefrom
Heenawter:controlsAnywhere_fix-discover-race-condition_2025-11-05

Conversation

@Heenawter
Copy link
Copy Markdown
Contributor

@Heenawter Heenawter commented Nov 5, 2025

Warning

This work is being merged into a feature branch, not main!
Because of this, we only need a review from @elastic/kibana-presentation for now.

Closes #241680

Summary

This PR ensures that fetch$ only emits once when reloading a Discover session with variables. This prevents the histogram from trying to load before the variables are ready, which caused an error.

Before

Screen.Recording.2025-11-05.at.3.27.47.PM.mov

After

Screen.Recording.2025-11-05.at.3.28.48.PM.mov

@Heenawter Heenawter marked this pull request as ready for review November 5, 2025 22:30
@Heenawter Heenawter requested review from a team as code owners November 5, 2025 22:30
@Heenawter Heenawter requested review from ThomThomson and removed request for a team November 5, 2025 22:30
@Heenawter Heenawter added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:small Small Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Nov 5, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 5, 2025

⏳ Build in-progress, with failures

Failed CI Steps

History

Comment on lines +100 to +104
// ignore meta data when comparing and storing filters, since we do not use it
const variablesWithoutMetaData = newVariables.map((variable) => omit(variable, 'meta'));
if (!isEqual(variablesWithoutMetaData, currentEsqlVariables)) {
// Update the ESQL variables in the internal state
dispatch(setEsqlVariables({ esqlVariables: newVariables }));
dispatch(setEsqlVariables({ esqlVariables: variablesWithoutMetaData }));
Copy link
Copy Markdown
Contributor Author

@Heenawter Heenawter Nov 6, 2025

Choose a reason for hiding this comment

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

Discover initializes the variables from the URL state so that the histogram can load right away; but when the variables actually publish via esqlVariables$, they now have metadata injected. This was causing unsaved changes issues + refetching issues because it thought things were different due to the new metadata. Since Discover doesn't use this info, it's easier to just omit it at the point of publishing

Copy link
Copy Markdown
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Approach here looks good! I like that the changes are pretty well limited to the control-group-renderer.

*/
const ignoreWhileLoading = pipe(
combineLatestWith(parentApi.childrenLoading$),
filter(([, loading]) => !loading),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clever way to do this!

@Heenawter Heenawter merged commit 82bdb21 into elastic:controlsAnywhere Nov 7, 2025
11 of 12 checks passed
@Heenawter Heenawter deleted the controlsAnywhere_fix-discover-race-condition_2025-11-05 branch November 7, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t//

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants