[Embeddable] Always send value input from edit panel action

[ThomThomson]

Information for the Presentation team reviewer

During the Time to Visualize project we began to pass input state around between the dashboard and editors with an embeddable state transfer service.

• For by reference panels we passed along only the id, with the understanding that on save and return to the dashboard, the by reference panel would be re-fetched from the saved object in order to reflect the updates that were made in the editor.
• For by value panels we passed along the entire input, and on save and return we combine the dashboard's old state for that panel with whatever new input was returned from the editor, the input from the editor taking precedence.

This worked great because panels never changed type. Any state that the editor didn't return would be repopulated from the dashboard's state.

When the visualize team started the project to upgrade TSVB / Visualize panels to Lens, we could no longer guarantee that the panel had not changed type, so we added a new statement to only spread the dashboard's previous panel state if the types were the same. This caused problems where some state would get left behind, like Drilldowns and custom time ranges.

@stratoula started work on #155113 to ensure that this state was not lost by taking it from the dashboard and propagating it back on save and return. But this did not work for by reference visualizations, because of the decision we made to pass only the id to by reference visualizations.

This PR fixes that by ensuring that the by value input is passed into the editors regardless of whether the panel is by value or by reference.

[ThomThomson]

@stratoula, I went through and did some testing with this on and everything seems to work properly. Here's what I tested:

• By ref map stays by reference on save and return
• By val map stays by value on save and return
• By ref visualize stays by reference on save and return
• By val visualize stays by value on save and return
• By ref lens stays by reference on save and return
• By val lens stays by value on save and return
• By ref tsvb stays by reference on save and return
• By val tsvb stays by value on save and return
• Upgrade by ref tsvb to lens is transformed to By Value Lens
• Upgrade by val tsvb to lens is transformed to By Value Lens
• Upgrade by ref visualize to lens is transformed to By Value Lens
• Upgrade by val visualize to lens is transformed to By Value Lens

Hopefully this helps solve the weirdness you were encountering in #147646

[elasticmachine]

Pinging @elastic/kibana-presentation (Team:Presentation)

[stratoula]

Thanx Devon! I also did some tests and everything works fine! Also it unblocks me from carrying the custom timerange to the converted panel 🎉 Much appreciated!

[nreese]

This change requires some cleanup in Maps render_app. That was written assuming either savedObjectId would be used for valueInput, not both

let mapEmbeddableInput;
    if (routeProps.match.params.savedMapId) {
      mapEmbeddableInput = {
        savedObjectId: routeProps.match.params.savedMapId,
      } as MapByReferenceInput;
    }
    if (valueInput) {
      mapEmbeddableInput = valueInput as MapByValueInput;
    }

[ThomThomson]

Looking at that logic, it's very strange that on save and return, maps by reference stayed by reference correctly. I wonder if the value input also includes the savedObjectId.

Correct me if I'm wrong, but it seems to me that we just need to use an else if there instead of another if. If so, I can make that change.

[nreese]

Looking at that logic, it's very strange that on save and return, maps by reference stayed by reference correctly. I wonder if the value input also includes the savedObjectId.

value input does contain savedObjectId. I think its more then just an if/else. The first case should may also need to include value input if provided but that could be done later.

[ThomThomson]

It's only really valuable to hold onto the provided valueInput if you're expecting to change types. On save and return if it's the same type, the previous input is spread back on top.

[ThomThomson]

@nreese, I updated the maps render_app with the else if and checked that it was always getting the expected result, either by reference editing or by value editing, and nothing in between.

I also verified that the other editors determine by value vs by reference editing state based on the presence of a savedObjectID in the URL rather than the presence of the valueInput in the incoming editor state. This means that this change is not necessary for any other app.

[nreese]

LGTM
code review and tested in chrome

[ThomThomson]

@elasticmachine merge upstream

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: a7f8ff9

Metrics [docs]

Async chunks

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

id	before	after	diff
maps	2.8MB	2.8MB	-3.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
embeddable	75.7KB	75.7KB	-39.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	16	18	+2
securitySolution	394	397	+3
total			+5

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	17	19	+2
securitySolution	474	477	+3
total			+5

History

• 💔 Build #122124 failed c9ec9b3
• 💚 Build #121269 succeeded dcd47b9
• 💔 Build #121263 failed e89cbf8
• 💔 Build #121167 failed 8b554e1

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