Test: element filter expressions #33908
Conversation
w33ble
added
review
Team:Presentation
|
Pinging @elastic/kibana-canvas |
w33ble
force-pushed
the
test/element-filter-expressions
branch
from
aa3b459 to
7fb2fcb
Compare
💚 Build Succeeded |
| it('gets filters from all elements', () => { | ||
| const filters = selector.getGlobalFilterExpression(state); | ||
| expect(filters).to.equal( | ||
| 'exactly value="beats" column="project" | timefilter column=@timestamp from=now-24h to=now' |
There was a problem hiding this comment.
if two elements have the same filters, then that filter string will be duped in the globalFilterExpression. Is there any value in deduping the array before doing the .join(|)?
There was a problem hiding this comment.
Possibly, but there's also no harm in just including it twice, just a small increase in expression execution time.
There was a problem hiding this comment.
@crob611 I should add, if you think it's worth doing, open an issue about it. It wouldn't be hard to add, but since this PR isn't a functional change, I'm not going to do it here.
There was a problem hiding this comment.
Yeah, I imagine the cost of deduping every time would outweigh any gains it would get us in the scenarios where filters are duplicated.
* test: add more filter elements * test: add getGlobalFilterExpression test * chore: only iterate filter elements once
Since I need to change how this function works a bit for filter grouping, I thought it would be helpful to add a test for it.
Also, I refactored it a bit so the element list would not need to be iterated twice, since I now had a test to confirm the functionality did not change.