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

[Visualize] Fixes the case with existing saved search and absent dataview #147327

Merged
merged 3 commits into from
Dec 12, 2022

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Dec 12, 2022

Summary

Closes #147285

It fixes the bug that is described in the issue. If you create a saved search with an index pattern and then delete the index pattern. now it won't break the entire kibana but will navigate to to the following screen that indicates that there is no index.

image

This is an edge case on my point of view, ideally if there are saved searches that correspond to a non-existent dataview should not be on the list but the current architecture doesn't allow to quickly solve this. I think it is fine for now.

@stratoula stratoula changed the title Fix case with existing saved search and absent dataview [Visualize] Fixes the case with existing saved search and absent dataview Dec 12, 2022
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

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
visDefaultEditor 142.3KB 142.5KB +176.0B
visTypeXy 40.5KB 40.7KB +236.0B
total +412.0B
Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

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

@stratoula stratoula added release_note:fix release_note:skip Skip the PR/issue when compiling release notes v8.7.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Vis Editor Visualization editor issues labels Dec 12, 2022
@stratoula stratoula marked this pull request as ready for review December 12, 2022 10:27
@stratoula stratoula requested a review from a team as a code owner December 12, 2022 10:27
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

Copy link
Contributor

@dej611 dej611 left a comment

Choose a reason for hiding this comment

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

Tested on Safari, but I have few questions.

I can see the new error only when creating a new visualization with a saved search referencing a missing dataView.

Also, why do I see two toasts?
Screenshot 2022-12-12 at 12 54 11

When opening a visualization referencing a saved search, which points to a missing dataView things get a little bit more confusing:

  • on dashboard it looks like the saved search object is missing, but it's not

Screenshot 2022-12-12 at 14 40 59

  • also opening the editor (via dashboard link, or via Visualize library) it seems like the saved search object cannot be found:

Screenshot 2022-12-12 at 14 41 08

@stratoula
Copy link
Contributor Author

@dej611 this Pr fixes the bug as described in the issue: Remove an index pattern from an existing saved search for a new visualization.

For existing visualizations, this is how it worked. I agree that is not the best UX but this is how it works on main. We can create an enhancement request to improve the behavior.

About the toasts I also think that it should be investigated on another PR as this happens in general. (multiple toasts creation) My goal here is to not break kibana.

@stratoula stratoula merged commit 3063950 into elastic:main Dec 12, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Vis Editor Visualization editor issues release_note:fix release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.7.0
Projects
None yet
5 participants