Repository navigation
Guard against empty and undefined index pattern arrays passed to QueryBar - #24607
Conversation
💚 Build Succeeded |
|
Hmm, so I've been thinking about this and I'm thinking we're going to need to eventually build a service anyway for getting the default index pattern, aren't we? Otherwise how will we choose which index pattern loads up by default in certain places? I'm open to discussion about whether or not we want to have a default altogether. I'm also wondering if this will also affect the rewrite of the filter bar. If you remember we had quite a few issues come up because we weren't using a default index pattern when one wasn't provided to the directive. I'm afraid these same issues might come up if we move forward this way. |
|
Closed by accident. |
💚 Build Succeeded |
|
@lukasolson offering suggestions for the default just doesn't make much sense to me, it seems worse than offering no suggestions at all. Let's say you're on a dashboard that only has TSVB visualizations targeting a non-default index pattern. You're going to get a bunch of non-sensical and confusing suggestions. I didn't get a chance to review #16235, otherwise I would have offered the same feedback there.
Couldn't we take the same approach there that I've taken here? If there is no index pattern, provide no suggestions. That would be my preference for the same reasons as above. |
The problem here is that TSVB/Timelion do support filters, and users had complained in the past when the filter bar wouldn't show up when just a TSVB or Timelion vis was on a dashboard and the filter bar didn't show up. |
What I'm saying is the filter bar would show up, as would the add filter button, but we just wouldn't show any suggestions in the add filter form. |
timroes
left a comment
There was a problem hiding this comment.
Tested in Chrome (Linux) fixes the issue. Code LGTM
💚 Build Succeeded |
29c229d to
e3cb7e9
Compare
|
Rebased this for another fresh CI run since it's been awhile. @lukasolson how are you feeling about this PR? |
💚 Build Succeeded |
lukasolson
left a comment
There was a problem hiding this comment.
Changes LGTM and checked it out and works fine. We'll have to make sure to update the filter bar to have the same behavior, because it seems odd that they work differently as of this PR.
|
This isn't making it into v6.5.2 (unless it's a really serious blocker). I bumped it to v6.5.3, but I'm not sure if really should be backported to 6.5 branch at all. |
|
Unless @timroes disagrees I don't think it needs to go into 6.5.3 either. It only has that label because it was originally slated for 6.5.0 and people kept bumping the label one patch version after each release. |
…yBar (elastic#24607) * guard against empty and undefined index pattern arrays * fix merge issues
|
Removed 6.5.3 label per discussion above |
|
@Bargs as we are adding KQL support to TSVB on 6.7 can you backport this PR? |
…yBar (elastic#24607) * guard against empty and undefined index pattern arrays * fix merge issues
Fixes #24572
The angular implementation of the QueryBar would use the default index pattern if the consumer of the component didn't pass any in. AFAIK we don't have an existing non-angular service for grabbing the default index pattern. I looked into creating one briefly, but as I was thinking about it I decided this probably wasn't the appropriate solution anyway. An index pattern is required for autocomplete suggestions (other than recent searches). If no pattern is passed in, we simply don't provide those suggestions. This seems like the simplest and most logical solution to me.