Saved searches aren't loading when first added to a dashboard

[stacey-gammon]

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

[stacey-gammon]

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.

[stacey-gammon]

In order to repro you have to have a vis on the dashboard. It appears like dashboard isn't catching the ready:vis from the visualization when it's done rendering, so it never calls scope.refresh which apparently only searches need so courier re-fetches their searches.

So to repro:

1. Create new dashboard.
2. Add a pie chart
3. add a saved search.

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 pendingVisCount correctly, but that is something different to the vis not emitting ready:vis.

Still not sure why it's working in 6.0, but qa has confirmed it is.

[stacey-gammon]

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.

[ppisljar]

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.

[stacey-gammon]

@ppisljar I can work on improving or changing the way dashboard catches these signals, but if there is no immediate or obvious solution, I think the PR should be reverted and we can work on implementing a better solution for the tooltip problem. What do you think?

cc @nreese

[ppisljar]

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

[stacey-gammon]

btw, pushed a PR with a test to catch this situation: #14775

[ppisljar]

@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

[stacey-gammon]

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.

[stacey-gammon]

Fixed in #14784