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.

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

[Discover] Revisit how filters should be carried over after navigating from Dashboard to Discover #155852

Open
Tracked by #194790
jughosta opened this issue Apr 26, 2023 · 7 comments
Labels
discuss Feature:Dashboard Dashboard related features Feature:Discover Discover Application Feature:Embeddables Relating to the Embeddable system impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews)

Comments

@jughosta
Copy link
Contributor

jughosta commented Apr 26, 2023

Currently, we have 2 ways of opening a saved search data from Dashboard:

  • "Explore underlying data" which includes filters but it is visible only if user has xpack.discoverEnhanced.actions.exploreDataInContextMenu.enabled=true setting. I guess most of users don't have it.
  • "Open in Discover" which is available by default but it does not include filters

saved-search

Maybe we could combine their functionality and provide a single default option in the context menu for a saved search panel: open a saved search plus include all current dashboards filters (query, filters, custom time range). Or we keep both options but they should be both available by default then.

Related tickets:

@jughosta jughosta added discuss Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews) labels Apr 26, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@jughosta
Copy link
Contributor Author

cc @ninoslavmiskovic @kertal

@ninoslavmiskovic
Copy link
Contributor

@jughosta we probably also need to check with the ESQL saved search table and perhaps unify those also . Cc @stratoula

@lukasolson lukasolson added the loe:needs-research This issue requires some research before it can be worked on or estimated label May 25, 2023
@lukasolson
Copy link
Member

Related: #72637

@lukasolson lukasolson added the impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. label May 25, 2023
@kertal
Copy link
Member

kertal commented May 31, 2023

In our discussion we decided that it makes sense to keep the filters that are applied to the saved search on a dashboard additionally. So we aim to show the same documents like on the dashboard. However, the user should be indicated that in Discover the resulting state / the additional filters is not the persisted version. So when we implement this, the following issue should already be resolved: #135887

@davismcphee
Copy link
Contributor

@jughosta @kertal I looked into this and put together a quick PR with a working implementation: #159361.

But after discussing with @ThomThomson, I think we should reconsider whether this is the right approach. There are two main issues identified:

  1. When clicking "Open in Discover", what are we actually trying to open? Is it the results as you currently see them, or is it the saved search itself? If it's supposed to be the saved search (which would be reasonable since it's a saved search embeddable), it probably makes sense to open it exactly as it was saved rather than appending additional filters.
  2. The other more concrete issue with this approach, which @ThomThomson brought up, is that if you bring the Dashboard filters over to Discover, the user may accidentally save them into the saved search without even realizing it, which is probably not something we want since it can have impact elsewhere. Resolving [Discover] Show "unsaved changes" label when in unsaved state of saved search #135887 would help the issue since there'd be an unsaved changes label, but a user could still miss this and make unintended changes to their search.

It feels like we might be mixing two different actions together here that do slightly different things:

  • An "Open saved search" action that would open the saved search exactly as it was saved. Not appending additional filters would eliminate the risk of unintended changes to the saved search.
  • An "Explore data in Discover" action that would function similar to the Lens embeddable one and let you explore the current results in Discover, carrying over all search state but not the saved search itself (extracting search state from the saved search). Since this action wouldn't carry the saved search over, there also wouldn't be a risk of unintended changes.

@davismcphee davismcphee added Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame Feature:Discover Discover Application labels Jun 9, 2023
@davismcphee davismcphee added loe:small Small Level of Effort and removed loe:needs-research This issue requires some research before it can be worked on or estimated labels Jun 9, 2023
@kertal
Copy link
Member

kertal commented Jun 12, 2023

It feels like we might be mixing two different actions together here that do slightly different things:

  • An "Open saved search" action that would open the saved search exactly as it was saved. Not appending additional filters would eliminate the risk of unintended changes to the saved search.
  • An "Explore data in Discover" action that would function similar to the Lens embeddable one and let you explore the current results in Discover, carrying over all search state but not the saved search itself (extracting search state from the saved search). Since this action wouldn't carry the saved search over, there also wouldn't be a risk of unintended changes.

Yes, I do agree that there are 2 different kind of actions of the user, and therefore it sounds reasonable to have to different UI elements for these actions with a clear naming. So we shouldn't add a quick fix here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:Dashboard Dashboard related features Feature:Discover Discover Application Feature:Embeddables Relating to the Embeddable system impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Data, DataViews)
Projects
None yet
Development

No branches or pull requests

7 participants