Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Maps] Implement searchSessionId in MapEmbeddable #89342

Merged
merged 11 commits into from
Jan 29, 2021
Merged

[Maps] Implement searchSessionId in MapEmbeddable #89342

merged 11 commits into from
Jan 29, 2021

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jan 26, 2021

Fixes #85449

This PR updates MapEmbeddable to pass searchSessionId to setQuery redux action. The PR then updates all calls to searchSource.fetch to pass sessionId into call. With these updates, MapEmbeddable panels will work with searchSessionIds.

To test, add xpack.data_enhanced.search.sessions.enabled: true to kibana.dev.yml. Then add a MapEmbeddable to dashboard. Open the inspector for the panel and notice that there is a sessionId for each _search request.

@nreese nreese added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Jan 26, 2021
@elasticmachine
Copy link
Contributor

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

Dosant
Dosant approved these changes Jan 27, 2021
View reviewed changes
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

I tested the following scenarios using [Flights] Global Flight Dashboard and all LGTM:

  1. searchSessionId appears in the inspector. It properly changes on dashboard state changes
  2. Restoring a session with maps works
  3. When using an incorrect searchSessionId maps requests fail, which is correct.

@Dosant
Copy link
Contributor

Dosant commented Jan 27, 2021

We have some functional test coverage for search session in a dashboard.
It would be awesome to extend that for maps,

For example, if we could add an assertion here that maps embeddable is loaded
https://github.com/elastic/kibana/blob/master/x-pack/test/send_search_to_background_integration/tests/apps/dashboard/async_search/send_to_background_relative_time.ts#L99
That would check if maps embeddable properly restored a search session

thomasneirynck
thomasneirynck approved these changes Jan 27, 2021
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

thx for adding, really cool

@nreese
Copy link
Contributor Author

nreese commented Jan 28, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

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

@nreese
Copy link
Contributor Author

nreese commented Jan 28, 2021

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Jan 28, 2021

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

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.6MB 2.6MB +1.5KB

History

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

@nreese nreese merged commit ad8a2fb into elastic:master Jan 29, 2021
nreese added a commit to nreese/kibana that referenced this pull request Jan 29, 2021
@nreese @kibanamachine
[Maps] Implement searchSessionId in MapEmbeddable (elastic#89342)
9e4390e 
* [Maps] Implement searchSessionId in MapEmbeddable

* clean up

* update method name

* fix _unsubscribeFromStore subscription

* fix unit test

* add maps assertion to send_to_background_relative_time functional test

* fix functional assertion

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@nreese nreese mentioned this pull request Jan 29, 2021
Merged
nreese added a commit that referenced this pull request Jan 29, 2021
@nreese @kibanamachine
[Maps] Implement searchSessionId in MapEmbeddable (#89342) (#89758)
1eea386 
* [Maps] Implement searchSessionId in MapEmbeddable

* clean up

* update method name

* fix _unsubscribeFromStore subscription

* fix unit test

* add maps assertion to send_to_background_relative_time functional test

* fix functional assertion

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@lizozom lizozom mentioned this pull request Feb 9, 2021
Closed
@nreese nreese mentioned this pull request Jun 21, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasneirynck thomasneirynck thomasneirynck approved these changes

@Dosant Dosant Dosant approved these changes

@lizozom lizozom Awaiting requested review from lizozom

Assignees
No one assigned
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Maps][Search Sessions] Pass down sessionId from embeddable to search calls
5 participants
@nreese @elasticmachine @Dosant @kibanamachine @thomasneirynck