-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[Search Sessions] Make search session indicator UI opt-in #88699
[Search Sessions] Make search session indicator UI opt-in #88699
Conversation
…t-in-to-session-ui # Conflicts: # src/plugins/data/public/public.api.md
Pinging @elastic/kibana-app-services (Team:AppServices) |
@elasticmachine merge upstream |
…t-in-to-session-ui
…ibana into dev/search/opt-in-to-session-ui
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great.
Maybe we could add this to the sessions_in_space
tests?
Should be simple enough to create a user that doesn't have this permission, and see that the component is properly disabled.
src/plugins/dashboard/public/application/hooks/use_dashboard_state_manager.ts
Show resolved
Hide resolved
@elasticmachine merge upstream |
@elasticmachine merge upstream |
I added these tests as you suggested. |
Requires a deeper look into Discover, but indicator isn't showing up anymore in Lens, which LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested with and without permissions and seems good!
LGTM
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, search sessions still work in Discover, tested locally in Chrome 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code only review - Dashboard changes LGTM.
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
…er-app capabilities (elastic#88699)
…y-tests * 'master' of github.com:elastic/kibana: (276 commits) [Telemetry] Settings Collector: redact sensitive reported values (elastic#88675) [CI] Combines Jest test jobs (elastic#85850) [Upgrade Assistant] Migrate server to new es-js client (elastic#89207) Migrate maps_legacy, maps_oss, region_map, and tile_map plugions to TS projects (elastic#89351) [Vega Docs] Add experimental flag on the vega maps title (elastic#89402) Increase the time needed to locate the save viz toast (elastic#89301) [Enterprise Search] Add links to doc links service (elastic#89260) Fixed regex bug in Safari (elastic#89399) [Lens] Fix indexpattern checks for missing references (elastic#88840) [Lens] Clean up usage collector (elastic#89109) update apm index pattern (elastic#89395) [APM] Upgrade ES client (elastic#86594) Enable v2 so migrations, disable in FTR tests (elastic#89297) [Search Sessions] Make search session indicator UI opt-in, refactor per-app capabilities (elastic#88699) Cleanup OSS code from visualizations wizard (elastic#89092) [APM] Optimize API test order (elastic#88654) Rename conversion function, extract to module scope and add tests. (elastic#89018) [core.logging] Add ops logs to the KP logging system (elastic#88070) chore(NA): improve ts build refs performance on kbn bootstrap (elastic#89333) skip flaky suite (elastic#89379) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/timeline/timeline.tsx # x-pack/test/accessibility/config.ts
…ana into task-manager/shift-on-trend * 'task-manager/shift-on-trend' of github.com:gmmorris/kibana: (74 commits) [Metrics UI] Fix Host Overview boxes in Host Detail page (elastic#89299) [Telemetry] Settings Collector: redact sensitive reported values (elastic#88675) [CI] Combines Jest test jobs (elastic#85850) [Upgrade Assistant] Migrate server to new es-js client (elastic#89207) Migrate maps_legacy, maps_oss, region_map, and tile_map plugions to TS projects (elastic#89351) [Vega Docs] Add experimental flag on the vega maps title (elastic#89402) Increase the time needed to locate the save viz toast (elastic#89301) [Enterprise Search] Add links to doc links service (elastic#89260) Fixed regex bug in Safari (elastic#89399) [Lens] Fix indexpattern checks for missing references (elastic#88840) [Lens] Clean up usage collector (elastic#89109) update apm index pattern (elastic#89395) [APM] Upgrade ES client (elastic#86594) Enable v2 so migrations, disable in FTR tests (elastic#89297) [Search Sessions] Make search session indicator UI opt-in, refactor per-app capabilities (elastic#88699) Cleanup OSS code from visualizations wizard (elastic#89092) [APM] Optimize API test order (elastic#88654) Rename conversion function, extract to module scope and add tests. (elastic#89018) [core.logging] Add ops logs to the KP logging system (elastic#88070) chore(NA): improve ts build refs performance on kbn bootstrap (elastic#89333) ...
Summary
Closes #88442
Previously the idea was that the search session indicator won't appear if an app doesn't use the search session. But recently Lens started using them without completing full integration #86297. If to enable
xpack.data_enhanced.search.sessions.enabled
in master, then Lens show search session indicator, but sending session to background doesn't properly work.This pr changes the underlying logic of session service to show session indicator only app explicitly asked for that.
Also, I noticed, that we checked for per-app capabilities inside the data plugin component. I moved it outside and so now apps can configure search session indicator UI. In this case by setting it to disabled state.
What to test
The only user-facing difference comparing to master is that the search session indicator shouldn't appear in the Lens app.
Checklist
For maintainers