-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Saved searches aren't loading when first added to a dashboard #14674
Comments
yes, I'm pretty sure that commit is the issue. I checked out master for 12 days ago and the issue showed up. Then I reverted that commit and the issue went away. For some reason, even though that commit was backported to 6.0, the issue doesn't exist in 6.0. Have not checked 6.x. |
In order to repro you have to have a vis on the dashboard. It appears like dashboard isn't catching the So to repro:
It's been on my todo list to find a better way to handle requests than these global broadcasts. I think there is another issue where dashboard isn't updating Still not sure why it's working in 6.0, but qa has confirmed it is. |
actually, pendingVisCount looks to be updating correctly. It's just very fragile that if it ever gets out of sync (e.g. some vis fails to publish it) it will never get back in sync, even if that panel is removed from the dashboard. This code is going to have to change to fully support the concept of embeddables anyway. |
in the PR referenced above we changed the way visualize sends ready:vis ... instead of broadcasting on rootscope it will emit on its own scope .... should still allow the parent to catch that event correctly, but does not pollute the rootscope. emitting on rootscope was generating issues with embedded tooltips, where dashboard would receive ready:vis every time a tooltip was shown. |
i agree @stacey-gammon, however we shouldn't revert the whole PR but just that specific part. I'll put a PR up tomorrow to do that |
btw, pushed a PR with a test to catch this situation: #14775 |
@stacey-gammon the reason this event doesn't propagate correctly is that here we create a new child scope on $rootScope. Instead we should create a new child scope to the dashboard's scope (if we would want that event to propagate correctly) here is a PR that reverts the original change: #14784 |
Thanks for digging in to why this doesn't work @ppisljar. I think the problem is that I don't want the dashboard app to be tied to angular, so the embeddables shouldn't rely on any angular specifics from dashboard. Clearly they still are, because of that bug, but long term, that's the direction I'd like to go in. I filed #14789 to keep track of this work. |
Fixed in #14784 |
Add a saved search to a dashboard and notice it's blank.
I've walked back the commit history and I think this was introduced with #14404.
Will continue to investigate. Should make sure it's not on 6.0.
cc @ppisljar
The text was updated successfully, but these errors were encountered: