Skip to content
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.

Sign up for GitHub

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

Test: element filter expressions #33908

Merged
merged 3 commits into from
Mar 28, 2019
Merged

Test: element filter expressions #33908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d81e733
test: add more filter elements
w33ble Mar 19, 2019
5c7c958
test: add getGlobalFilterExpression test
w33ble Mar 19, 2019
7fb2fcb
chore: only iterate filter elements once
w33ble Mar 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions x-pack/plugins/canvas/public/state/selectors/__tests__/workpad.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,39 @@ describe('workpad selectors', () => {
},
],
},
'element-3': {
type: 'expression',
chain: [
{
type: 'function',
function: 'demodata',
arguments: {},
},
{
type: 'function',
function: 'dropdownControl',
arguments: {
valueColumn: ['project'],
filterColumn: ['project'],
},
},
{
type: 'function',
function: 'render',
arguments: {},
},
],
},
'element-4': {
type: 'expression',
chain: [
{
type: 'function',
function: 'timefilterControl',
arguments: { compact: [true], column: ['@timestamp'] },
},
],
},
};

state = {
Expand Down Expand Up @@ -63,10 +96,23 @@ describe('workpad selectors', () => {
{
id: 'element-0',
expression: 'markdown',
filter: '',
},
{
id: 'element-1',
expression: 'demodata',
filter: '',
},
{
id: 'element-3',
expression:
'demodata | dropdownControl valueColumn=project filterColumn=project | render',
filter: 'exactly value="beats" column="project"',
},
{
id: 'element-4',
expression: 'timefilterControl compact=true column=@timestamp',
filter: 'timefilter column=@timestamp from=now-24h to=now',
},
],
},
Expand Down Expand Up @@ -135,6 +181,7 @@ describe('workpad selectors', () => {

it('returns all elements on the page', () => {
const { elements } = state.persistent.workpad.pages[0];

const expected = elements.map(element => ({
...element,
ast: asts[element.id],
Expand Down Expand Up @@ -198,6 +245,15 @@ describe('workpad selectors', () => {
});
});

describe('getGlobalFilterExpression', () => {
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'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(|)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, but there's also no harm in just including it twice, just a small increase in expression execution time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I imagine the cost of deduping every time would outweigh any gains it would get us in the scenarios where filters are duplicated.

);
});
});

describe('isWriteable', () => {
it('returns boolean for if the workpad is writeable', () => {
expect(selector.isWriteable(state)).to.equal(false);
Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/canvas/public/state/selectors/workpad.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,14 @@ export function getElementStats(state) {

export function getGlobalFilterExpression(state) {
return getAllElements(state)
.map(el => el.filter)
.filter(str => str != null && str.length)
.reduce((acc, el) => {
// check that a filter is defined
if (el.filter != null && el.filter.length) {
return acc.concat(el.filter);
}

return acc;
}, [])
.join(' | ');
}

Expand Down