[Discover] Implement ad-hoc data views #138283
Conversation
dimaanj
added
release_note:enhancement
Feature:Discover
|
@elasticmachine merge upstream |
|
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
There was a problem hiding this comment.
I'm excited to have this finally in Discover soon. It's one of the things I wanted to have when I first played with it, more than 5 years ago!
Been playing around with it and this are my first comments
- Sharing needs a persisted data view
- Field statistics doesn't need a persisted one, that's a pleasant surprise, however the Lens button in needs it, so for now till Lens supports the ad hoc ones, I'd suggest to disable field statistics, because I think the effort to pass the check for persistence down might be to high
- We should cover the new functionality with a functional
- We need some fine words, and adding @gchaps for this purpose
So when users decides to create a data view without saving
They get prompted if an action requires saved data view. So they need to confirm that one step further saves the current data view automatically. Currently it looks like this:
many thx!
|
Oh, one more thing @dimaanj if I confirm the data view to be persisted, the data view selector doesn't contain the new entry , it stays selected of course |
There was a problem hiding this comment.
Pulled and tested locally, and overall it seems to be working well! I agree with @kertal that some functional tests for the flyout and invalid actions that trigger the modal would be ideal, and maybe Jest tests for use_persisted_data_view.tsx if possible.
I noticed a few things while poking around that we may want to fix:
- 'Manage this data view' somewhat works with an ad hoc data view, but it seems to throw some errors in the console when I get to the page. Maybe this should be prevented?
'Add a field to this data view' opens the flyout and partially works, but everything explodes when you try to save it. This might be something to prevent for ad hoc data views.This behaviour is unrelated to the work in this PR and may even be intended -- the issue stems from runtime fields on data views that match more than one index.- Creating an ad hoc data view shows a 'saved' toast even though we haven't saved anything yet — maybe we should change the message for ad hoc data views?
- Refreshing the page clears the ad hoc data view and shows an error toast with
"{GENERATED_ID}" is not a configured data view ID, then falls back on the default data view — I think we should either make it clear what happened here or consider persisting the ad hoc data view to a temporary state like query string or local storage. - Filters work but throw some PUT request errors in the console.
- Adding fields from the sidebar also works but throws some PUT request errors in the console.
And a couple observations that aren't things to fix, but might be useful to bring up anyway:
- Clicking 'New' keeps the ad hoc data view. I think this might actually make sense, but I'm not sure, so I figured I should bring it up.
- It's not clear to me if there's a way to persist the ad hoc data view without attempting an invalid action to trigger the modal. I feel it would make sense to have a way to persist the current ad hoc data view without having to trigger the modal.
- Not directly related to this PR, but an idea to help make it clear what our state is would be to show a badge similar to the one we intend to implement for 'unsaved changes' that indicates something along the lines of 'unsaved data view'.
src/plugins/discover/public/application/main/components/chart/discover_chart.tsx
Outdated
Show resolved
Hide resolved
src/plugins/discover/public/application/main/utils/resolve_data_view.ts
Outdated
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
There was a problem hiding this comment.
Finishing line in sight, so when giving it another testing round I've encountered that when sharing a adhoc one in a saved search, and deciding to persist to ad hoc one, the name is lost, you can see how the data view name is switched in the data view selector
Kapture.2022-09-13.at.10.11.29.mp4
Secondly, not sure if it was an issue before, but when switching to Lens with an ad hoc one, reloading the page there's an error displayed:
When fixing this and merging main it should be good to merge. Could you add a screen showing where the "Use without saving" is displayed in the description, then it would be clear how to test, in can be easily overlooked. many thx!
…ce-hoc-data-views # Conflicts: # src/plugins/discover/public/__mocks__/services.ts # src/plugins/discover/public/application/main/discover_main_app.test.tsx # src/plugins/discover/public/application/main/hooks/use_discover_state.test.ts # src/plugins/discover/public/application/main/hooks/use_discover_state.ts # src/plugins/discover/public/application/main/hooks/use_saved_search.test.ts
There was a problem hiding this comment.
LGTM, great work @dimaanj ! Tested again and my last issues were resolved! Huge thanks for your effort everything you've addressed! With this PR one of the first UX issues in Discover I've encountered years ago is resolve: The need to persist a thing like index pattern / data view before I can actually see my data of interest, well it's not completely resolved. Once it's doable right in the data view picker, then this is resolved. But that's a big step!
|
@stratoula agreed to dismiss the change request |
src/plugins/discover/public/application/main/hooks/use_adhoc_data_views.ts
Show resolved
Hide resolved
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @dimaanj |
kertal
changed the title
Summary
Closes #137671
This PR enables basic hoc Data Views experience. When action needs persisted data view, the following prompt will be shown.
Actions which need persistence of data view currently:
Discover Alerts, Share, navigation from lens and adding multiple adhoc data views will be implemented in follow up #139984
How to test
k*and select timestamp fieldtimefieldto see documents from flights and logs sample dataUse without savingChecklist