Skip to content

Commit

Permalink
Fixed pointseries to ignore empty strings as args (elastic#331)
Browse files Browse the repository at this point in the history
  • Loading branch information
cqliu1 authored and Rashid Khan committed Feb 13, 2018
1 parent dbe419a commit dd2a1ea
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 27 deletions.
11 changes: 11 additions & 0 deletions common/lib/__tests__/datatable_to_math_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ describe('datatableToMathContext', () => {
});
it('filters out non-numeric columns and pivots datatable', () => {
expect(datatableToMathContext(testTable)).to.be.eql({
time: [
1517842800950,
1517929200950,
1518015600950,
1517842800950,
1517929200950,
1518015600950,
1517842800950,
1517842800950,
1517842800950,
],
price: [605, 583, 420, 216, 200, 190, 67, 311, 288],
quantity: [100, 200, 300, 350, 256, 231, 240, 447, 384],
});
Expand Down
4 changes: 3 additions & 1 deletion common/lib/datatable_to_math_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { pivotObjectArray } from '../lib/pivot_object_array.js';

// filters columns with type number and passes filtered columns to pivotObjectArray
export function datatableToMathContext(datatable) {
const filteredColumns = datatable.columns.filter(col => col.type === 'number');
const filteredColumns = datatable.columns.filter(
col => col.type === 'number' || col.type === 'date'
);
return pivotObjectArray(datatable.rows, filteredColumns.map(col => col.name));
}
72 changes: 48 additions & 24 deletions server/functions/__tests__/pointseries.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,6 @@ describe('pointseries', () => {
rows: [{ x: 1, y: 2 }],
});
});
it('invalid args', () => {
expect(fn(testTable, { x: 'name', y: 'quantity * 3' })).to.be.eql({
type: 'pointseries',
columns: {
x: {
type: 'string',
role: 'dimension',
expression: 'name',
},
y: {
type: 'number',
role: 'measure',
expression: 'quantity * 3',
},
},
rows: [
{ x: 'product1', y: null },
{ x: 'product2', y: null },
{ x: 'product3', y: null },
{ x: 'product4', y: null },
{ x: 'product5', y: null },
],
});
});
it('args with dimensions only', () => {
expect(fn(testTable, { x: 'name', y: 'price', size: 'quantity' })).to.be.eql({
type: 'pointseries',
Expand Down Expand Up @@ -161,5 +137,53 @@ describe('pointseries', () => {
expect(row.y).to.be.within(0, 1);
});
});
it('empty string arg', () => {
expect(fn(testTable, { x: 'name', y: 'max(time)', size: ' ', text: '' })).to.be.eql({
type: 'pointseries',
columns: {
x: {
type: 'string',
role: 'dimension',
expression: 'name',
},
y: {
type: 'number',
role: 'measure',
expression: 'max(time)',
},
},
rows: [
{ x: 'product1', y: 1518015600950 },
{ x: 'product2', y: 1518015600950 },
{ x: 'product3', y: 1517842800950 },
{ x: 'product4', y: 1517842800950 },
{ x: 'product5', y: 1517842800950 },
],
});
});
it('invalid args', () => {
expect(fn(testTable, { x: 'name', y: 'quantity * 3' })).to.be.eql({
type: 'pointseries',
columns: {
x: {
type: 'string',
role: 'dimension',
expression: 'name',
},
y: {
type: 'number',
role: 'measure',
expression: 'quantity * 3',
},
},
rows: [
{ x: 'product1', y: null },
{ x: 'product2', y: null },
{ x: 'product3', y: null },
{ x: 'product4', y: null },
{ x: 'product5', y: null },
],
});
});
});
});
3 changes: 1 addition & 2 deletions server/functions/pointseries/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ export const pointseries = () => ({
// Separates args into dimensions and measures arrays by checking if arg is a column reference (dimension)
Object.keys(args).forEach(arg => {
const mathExp = args[arg];

if (mathExp != null) {
if (mathExp != null && mathExp.trim() !== '') {
const col = {
type: '',
role: '',
Expand Down

0 comments on commit dd2a1ea

Please sign in to comment.