[Maps][Dashboard] MapBuffer Causing Dashboard Unsaved Changes

[ThomThomson]

Issue: The maps embeddable introduced a new key MapBuffer into its input in order to store the map bounds used for a query, so that the session service could build the exact same query when restoring, in order to avoid cache misses. Since this key is a part of the embeddable input, it is treated as such, causing the following problems:

• A :MapBuffer is saved to the dashboard saved object. This means that if the dashboard was last saved on a small screen, and loaded on a larger screen, the map will not show data for the entire bounds.
• B: MapBuffer is taken into consideration when diffing the dashboard state, which means that any time a user clicks edit on a dashboard with a map, the unsaved changes badge immediately appears.

There are a number of ways to mitigate the problem, but all of the easiest solutions result in a worsening of either problem A, B, or result in cache misses which is unacceptable.

All complete solutions to this problem thought of so far involve giving dashboard a way of discriminate between real state, and temporary state and are blocked by #87304

Possible Soluitions:

• Name an arbitrary key in any embeddable's input that dashboard with ignore when saving and diffing state. temp for instance.

• Have each embeddable in charge of what pieces of its input it considers persistable. The dashboard would loop through its children, and request this persistable state. Only this state would be saved or diffed.

Both of these solutions would require the entire panel state (including any temporary state) to be persisted by the session management API. Currently panels are only persisted by the session management if the dashboard is not saved. The new URL service should make this easier to implement.

[elasticmachine]

Pinging @elastic/kibana-gis (Team:Geo)

[elasticmachine]

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

[ppisljar]

adding this information on embeddable input seems like a hack in the first place. i think we need to remove that and find a different solution.

how do we really want to handle this ?

• how do we restore a map on smaller screen (possibly a lot smaller) ? we could just restore the data we have and ignore the fact that we have too much
• how do we restore a map on bigger screen ? it seems we can't really, at least not if the screen is notably bigger

possible solution: define size groups: size 1, 2, 3, 4, 5 ... don't fetch exact bounds of the map but according to size group. restoring a session on a similarly sized screen will work ok. restoring on a screen with way bigger size should anyway reload the data.

possible solution 2: either use embeddable output or provide another state on embeddable to which maps can write this buffer. this state would need to implement PersistableState interface and should be stored by dashboard and provided back to embeddables at loading. to handle cases when we are restoring on larger screens we could maybe adjust for that by zooming in on the map ? anyway, we shouldn't be using embeddable input for this as that one should not be mutated by embeddable.

[ThomThomson]

There is a little bit more background on this problem that I most likely didn't represent properly above. I will do my best to sum it up in this comment. Additionally, this is also a place where I will try to make sense of the problem in my own head - so if I over-explain or mis-represent anything please correct me.

Overall, this is a problem caused by a lack of a direct communication channel between embeddables on a dashboard and the state that a saved search session restores.

How do maps determine bounds normally?

It's important to note that before search sessions, the bounds of the query on a map was a solved problem. @nreese, correct me if I'm wrong, but before search sessions only two pieces of information were stored with the embeddable input:
zoom level, and map center. Using these bits of information, the map embeddable calculates the bounds based on any screen size / panel size on the fly. Ideally, we wouldn't have to change this behaviour at all, except when restoring a session.

Where did mapBuffer come from?

Because session restoration uses a cache in the format query -> result it's important that each query for a session restore is rebuilt exactly. This means that only in the case of a session restoration, the exact bounds need to be stored and used. MapBuffer is the current place for that storage. MapBuffer is an example of a new type of temporary state that we may run into again. This type of state has the following properties:

• It should never be persisted into a saved object for normal usage.
• It should always be persisted into the session restoration state.

What to do?

We need to be able to distinguish between state that should be persisted normally, and state that is temporary. I'm game for any way at all we can think of making this distinction.

As for embeddables updating their own input, I'm not sure if I understand the suggestion that they shouldn't. Updating their own input currently seems to be the only way for embeddables to make changes (inline) that get persisted in the dashboard. Even before search sessions, when the 'center' or 'zoom level' in the map redux state changed, that information was pushed to the embeddable input.

At a later date, does this need to be refactored? This would require some major changes from the dashboard side as well.

[ppisljar]

thanks for the clarification. I am still wondering how do we plan to restore maps on smaller screens ?

i understand now how url service plays a role here. When the session is persisted the session service collects the current state from application, currently in the form of url. When we introduce the new url service this will actually be full state that will be passed to application. Still we need to have a way for dashboard to collect this information from embeddables and provide it at a later point in time. Currently the only way for dashboard to pass information down to embeddables is thru the embeddableInput, but this on is also persisted with saved dashboard.

from the search session side i think the interface is quite clear and should suffice. When session is stored app needs to provide a url/state so it can be restored in that exact state. Session service does not care about apps internal implementation (like the fact its using embeddables), its just going to provide that full state back to the app when being restored and has associated a bunch of requests with that state.

on embeddables i don't think we currently have a good/complete way to do it. there is embeddableOutput that can be provided to embeddable constructor, can't be changed later from outside (but can be from inside) and can be read from outside. Sounds great except for the part that most actual embeddable implementation are not handling it and also there is no way to provide it to embeddablefactory.create method. one possible way would be to fix this, add optional parameter to embeddablefactory.create and make all embeddables implement this.

[bhavyarm]

I just ran across a confusing behaviour on layer actions in maps. In maps app - any user change to layer action -can be saved. And if you navigated away from the maps after doing something on layer actions - then Kibana warns the user. But if you do the same thing in dashboard - then Kibana wont tell the user to save. IMHO - this should behave like the maps app. Thanks!