[Discover][Visualize] Fix visualizing a selected field in discover search

[sulemanof]

Summary

Fixes #61213
Fixes #61103

Data should be deep cloned since the object is freezed on mutations.
The mutation was occurred in AggConfig.ensureIds while creating new AggConfigs object .

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
• This was checked for cross-browser compatibility, including a check against IE11

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)

[timroes]

I would suggest fixing it instead inside the Vis.setState method (the one that earlier also did the cloneDeep, which got removed when refactoring it to TypeScript in #59958):

diff --git a/src/legacy/core_plugins/visualizations/public/np_ready/public/vis.ts b/src/legacy/core_plugins/visualizations/public/np_ready/public/vis.ts
index 0ba936c9f6..91b6a2368f 100644
--- a/src/legacy/core_plugins/visualizations/public/np_ready/public/vis.ts
+++ b/src/legacy/core_plugins/visualizations/public/np_ready/public/vis.ts
@@ -132,8 +132,10 @@ export class Vis {
       this.data.savedSearchId = state.data.savedSearchId;
     }
     if (state.data && state.data.aggs) {
-      let configStates = state.data.aggs;
-      configStates = this.initializeDefaultsFromSchemas(configStates, this.type.schemas.all || []);
+      const configStates = this.initializeDefaultsFromSchemas(
+        cloneDeep(state.data.aggs),
+        this.type.schemas.all || []
+      );
       if (!this.data.indexPattern) {
         if (state.data.aggs.length) {
           throw new Error('trying to initialize aggs without index pattern');

[sulemanof]

I would suggest fixing it instead inside the Vis.setState method (the one that earlier also did the cloneDeep, which got removed when refactoring it to TypeScript):

Agree!
I did not take into account other possible cases

[kertal]

Thx for fixing this so quickly! Could you add a functional test for that, the maps team recently added one for linking geo point fields to maps. Would be nice to catch this kinds of errors in the future.

kibana/x-pack/test/functional/apps/maps/discover.js

Line 13 in 583104b

describe('discover visualize button', () => {

[sulemanof]

functional/apps/maps/discover.js

Good call @kertal !
I've added a functional test for it.
This also helped to reveal one more issue:
if an app filter or query are configured in discover, they are not applied in visualize!
I'm going to create an issue for that with additional details, and an appropriate fix could also be prepared with additional tests in test/functional/apps/discover/_field_visualize.ts.

I'd kindly ask @dmlemeshko to take a look at functional tests changes! 🙂

[sulemanof]

As @timroes suggested, fixing the issue with filters is moved into a separate PR #61392

[kertal]

Code LGTM, tested locally in chrome, thx a lot for the functional test!

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: b7dd1cc

History

• 💚 Build #36230 succeeded de3bd58
• 💚 Build #36191 succeeded 3f396ee
• 💔 Build #36132 failed 873a19f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[dmlemeshko]

LGTM. I ran broken scripted fields and the newly added tests against Istanbul-instrumented Kibana and both are passing. Thank you for the fix!