Skip to content

[Search][Discover] Restore searchSessionId from URL#81633

Merged
Dosant merged 7 commits intoelastic:masterfrom
Dosant:dev/search/4
Nov 11, 2020
Merged

[Search][Discover] Restore searchSessionId from URL#81633
Dosant merged 7 commits intoelastic:masterfrom
Dosant:dev/search/4

Conversation

@Dosant
Copy link
Copy Markdown
Contributor

@Dosant Dosant commented Oct 26, 2020 •
edited by lizozom
Loading

Release note

Adds the Search Sessions support to Discover in Kibana

Summary

Build on top of #81297
Part of #61738
Similar merged pr for a dashboard app: #81489

Summary

  • Restore searchSessionId from URL
  • Add searchSessionId support to discover's URL generator
  • Pass searchSessionId into inspector

Additional info

How to test

You can build a discover URL with fake session id:

${discover}&searchSessionId=__fake__

Open it.
And you should see __fake__ as session id in inspector.

^^ this is what a functional test does

Checklist

Delete any items that are not applicable to this PR.

@Dosant Dosant added Feature:Search Querying infrastructure in Kibana Feature:Discover Discover Application v8.0.0 Team:AppArch release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Oct 26, 2020
This was referenced Oct 26, 2020
Closed
Merged
Merged
@Dosant Dosant changed the title [Search] Session service in discover improvements [Search][Discover] Restore searchSessionId from URL Nov 2, 2020
@Dosant Dosant marked this pull request as ready for review November 2, 2020 15:21
@Dosant Dosant requested a review from a team November 2, 2020 15:21
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Nov 2, 2020

Pinging @elastic/kibana-app-arch (Team:AppArch)

@Dosant Dosant requested review from lizozom and lukasolson November 2, 2020 15:21
lukasolson
lukasolson reviewed Nov 2, 2020
View reviewed changes
@Dosant Dosant requested a review from a team as a code owner November 3, 2020 12:21
@Dosant Dosant mentioned this pull request Nov 3, 2020
Closed
@Dosant Dosant requested a review from lukasolson November 4, 2020 10:10
@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Nov 9, 2020

@elasticmachine merge upstream

lukasolson
lukasolson approved these changes Nov 10, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM!

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Nov 10, 2020

@kertal could you take a look 🙏

@Dosant Dosant requested a review from kertal November 10, 2020 12:34
@kertal
Copy link
Copy Markdown
Member

kertal commented Nov 10, 2020

@kertal could you take a look 🙏

sure! will have a look tomorrow, but it might be in the afternoon due to giving a Kibana analyst training

@kertal
Copy link
Copy Markdown
Member

kertal commented Nov 11, 2020

On initial load searchSessionId is just removed from URL.

is it? so when I load Discover with the searchSessionId, it should be removed from the URL?

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Nov 11, 2020

@kertal, sorry for confusion, that was outdated info.
It is actually removed on subsequent changes.
(discussion: #81633 (comment))

kertal
kertal approved these changes Nov 11, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM 👍 , tested locally in Chrome, Firefox, Safari, works!

@Dosant Dosant merged commit 7fdd0a1 into elastic:master Nov 11, 2020
Dosant added a commit to Dosant/kibana that referenced this pull request Nov 11, 2020
@Dosant
[Search][Discover] Restore searchSessionId from URL (elastic#81633)
13a1801
@Dosant Dosant mentioned this pull request Nov 11, 2020
Merged
Dosant added a commit that referenced this pull request Nov 11, 2020
@Dosant
[Search][Discover] Restore searchSessionId from URL (#81633) (#83162)
b16bb24
@kertal kertal mentioned this pull request Nov 16, 2020
Merged
6 tasks
@Dosant Dosant mentioned this pull request Nov 18, 2020
Closed
38 tasks
@lizozom lizozom added release_note:feature Makes this part of the condensed release notes and removed release_note:skip Skip the PR/issue when compiling release notes labels Feb 22, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 22, 2021 •
edited
Loading

💔 Build Failed

Failed CI Steps

Test Failures

Jest Integration Tests.src/core/server/ui_settings/integration_tests.uiSettings/routes doc missing get route creates doc, returns a 200 with settings

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 4 times on tracked branches: https://github.com/elastic/kibana/issues/51576

Stack Trace

Error: ES exited with code 1
    at createCliError (/dev/shm/workspace/parallel/10/kibana/packages/kbn-es/target/errors.js:22:17)
    at _outcome.exitCode.then.code (/dev/shm/workspace/parallel/10/kibana/packages/kbn-es/target/cluster.js:396:15)
    at process._tickCallback (internal/process/next_tick.js:68:7)
Jest Integration Tests.src/core/server/ui_settings/integration_tests.uiSettings/routes doc missing set route creates doc, returns a 200 with value set

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 4 times on tracked branches: https://github.com/elastic/kibana/issues/51577

Stack Trace

Error: ES exited with code 1
    at createCliError (/dev/shm/workspace/parallel/10/kibana/packages/kbn-es/target/errors.js:22:17)
    at _outcome.exitCode.then.code (/dev/shm/workspace/parallel/10/kibana/packages/kbn-es/target/cluster.js:396:15)
    at process._tickCallback (internal/process/next_tick.js:68:7)
Jest Integration Tests.src/core/server/ui_settings/integration_tests.uiSettings/routes doc missing setMany route creates doc, returns 200 with updated values

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 4 times on tracked branches: https://github.com/elastic/kibana/issues/51578

Stack Trace

Error: ES exited with code 1
    at createCliError (/dev/shm/workspace/parallel/10/kibana/packages/kbn-es/target/errors.js:22:17)
    at _outcome.exitCode.then.code (/dev/shm/workspace/parallel/10/kibana/packages/kbn-es/target/cluster.js:396:15)
    at process._tickCallback (internal/process/next_tick.js:68:7)

and 14 more failures, only showing the first 3.

Metrics [docs]

‼️ ERROR: no builds found for mergeBase sha [7d3e198]

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kertal kertal kertal approved these changes

@lukasolson lukasolson lukasolson approved these changes

@lizozom lizozom Awaiting requested review from lizozom

Assignees

No one assigned

Labels

Feature:Discover Discover Application Feature:Search Querying infrastructure in Kibana release_note:feature Makes this part of the condensed release notes v7.11.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Dosant @elasticmachine @kertal @kibanamachine @lukasolson @lizozom