Support infinite ranges on filters #4948
Conversation
When only one side of the range is infinite, we never add a condition to the script filter for that half of the range, so elasticsearch treats it as infinite. When both sides of the range are infinite, we use a match_all filter instead of a conditional script to define the range.
Test directories should live alongside the files they are testing.
Test directories should live alongside the files they are testing.
|
Looking at the tests it seems like there is an inconsistency on how you use @w33ble just opened #4949 to start a discussion about updating the style guide. I think it would be good to come up with a set of guidelines for when to use |
|
I definitely don't want to block this PR on a styleguide discussion, so I'll change them to describe. |
|
|
||
| beforeEach(ngMock.module('kibana')); | ||
| beforeEach(ngMock.inject(function (Private, $rootScope) { | ||
| resolvePromises = () => $rootScope.$apply(); |
There was a problem hiding this comment.
I get what you were doing here, but I don't think the overhead is worth it. Seeing $scope.$apply() in the code, it's clear what that command is doing since we're already in Angular land. Seeing resolvePromises means you have to go look up what that code is doing.
Calling $apply is also already pretty concise.
There was a problem hiding this comment.
Yeah, I was just trying to keep the tests as expressive as possible, but I'm not married to this approach. I'll swap it out with direct access to $rootScope.
|
@epixa this can't be backported as-is, since we don't have ES6 support pre-4.2. Can you open a new issue (same as 4846, but for 4.1.3), remove the 4.1.3 label from this PR and submit a new PR for backporting? Otherwise, this LGTM. Passing on for last looks. |
|
Yep, I can do that. How do we identify issues that should be backported? The original issue itself was not labeled for 4.1.3. |
|
Usually bugfixes go back 1 version. In this case, #4613 fixed 4.2, and #4846 backported it to 4.1. Since both the original and backport featured this issue with infinite ranges, it needs to be fixed in both places. This is kind of an edge case though, since 4613 was really a feature add, but it fixed some stuff that needed to be fixed in 4.1 too, which is why it was backported as a whole. More generally, the original issue will usually indicate if the fix needs to be backported as well. |
|
Makes sense to me. |
|
LGTM 👍 |
Support infinite ranges on filters
When only one side of the range is infinite, we never add a condition to
the script filter for that half of the range, so elasticsearch treats it
as infinite.
When both sides of the range are infinite, we use a match_all filter
instead of a conditional script to define the range.
The core of the change is in epixa@6761e01. The other commits are just moving existing test files into the correct places.
Fixes #4862