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.

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

[6.x] Preserve x-axis ordering in split series vis. #28733

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 53 additions & 20 deletions src/ui/public/agg_response/point_series/__tests__/_add_to_siri.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,49 +21,82 @@ import expect from 'expect.js';
import { addToSiri } from '../_add_to_siri';

describe('addToSiri', function () {
const xKeys = ['', 'foo', 'bar', 'baz'];

it('creates a new series the first time it sees an id', function () {
const series = new Map();
const point = {};
const point = { x: 'foo' };
const id = 'id';
addToSiri(series, point, id, id, { id: id });
addToSiri(xKeys, series, point, id, id, { id: id });

expect(series.has(id)).to.be(true);
expect(series.get(id)).to.be.an('object');
expect(series.get(id).label).to.be(id);
expect(series.get(id).values).to.have.length(1);
expect(series.get(id).values[0]).to.be(point);
});

it('adds points to existing series if id has been seen', function () {
it('instantiates a zero-filled array with ordered x values the first time it sees an id', function () {
const series = new Map();
const point = { x: 'foo' };
const id = 'id';
const expected = {
x: '',
xi: Infinity,
y: 0,
series: id,
};
addToSiri(xKeys, series, point, id, id, { id: id });

const point = {};
addToSiri(series, point, id, id, { id: id });
expect(series.get(id).values).to.have.length(4);
expect(series.get(id).values[0]).to.eql(expected);
});

const point2 = {};
addToSiri(series, point2, id, id, { id: id });
it('writes the correct value to the zero-filled array the first time it sees an id', function () {
const series = new Map();
const point = { x: 'foo' };
const id = 'id';
const zeroFilled = {
x: '',
xi: Infinity,
y: 0,
series: id,
};
addToSiri(xKeys, series, point, id, id, { id: id });

expect(series.get(id).values).to.have.length(4);
expect(series.get(id).values[0]).to.eql(zeroFilled);
expect(series.get(id).values[1]).to.be(point);
});

it('updates points in existing series if it has been seen', function () {
const series = new Map();
const id = 'id';

const point0 = { x: '' };
addToSiri(xKeys, series, point0, id, id, { id: id });

const point1 = { x: 'foo' };
addToSiri(xKeys, series, point1, id, id, { id: id });

const point2 = { x: 'bar' };
addToSiri(xKeys, series, point2, id, id, { id: id });

const point3 = { x: 'baz' };
addToSiri(xKeys, series, point3, id, id, { id: id });

expect(series.has(id)).to.be(true);
expect(series.get(id)).to.be.an('object');
expect(series.get(id).label).to.be(id);
expect(series.get(id).values).to.have.length(2);
expect(series.get(id).values[0]).to.be(point);
expect(series.get(id).values[1]).to.be(point2);
expect(series.get(id).values[0]).to.be(point0);
expect(series.get(id).values[1]).to.be(point1);
expect(series.get(id).values[2]).to.be(point2);
expect(series.get(id).values[3]).to.be(point3);
});

it('allows overriding the series label', function () {
const series = new Map();
const id = 'id';
const label = 'label';
const point = {};
addToSiri(series, point, id, label, { id: id });
const point = { x: 'foo' };
addToSiri(xKeys, series, point, id, label, { id: id });

expect(series.has(id)).to.be(true);
expect(series.get(id)).to.be.an('object');
expect(series.get(id).label).to.be(label);
expect(series.get(id).values).to.have.length(1);
expect(series.get(id).values[0]).to.be(point);
});
});
39 changes: 39 additions & 0 deletions src/ui/public/agg_response/point_series/__tests__/_get_series.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,43 @@ describe('getSeries', function () {
expect(series2[2]).to.have.property('label', '1: 1');
expect(series2[3]).to.have.property('label', '1: 0');
});

it('produces series values with x values in the same order as the provided rows', function () {
const rows = [
[1, 4],
[1, 3],
[1, 4],
[1, 4],
[1, 2],
[1, 1],
[1, 1],
[1, 2],
[1, 3],
].map(wrapRows);

const chart = {
aspects: {
x: { i: 1 },
y: { i: 0, title: 'y', aggConfig: { id: 'id' } },
}
};

const series = getSeries(rows, chart);

expect(series)
.to.be.an('array')
.and.to.have.length(1);

const siri = series[0];
expect(siri)
.to.be.an('object')
.and.have.property('label', chart.aspects.y.title)
.and.have.property('values');

expect(siri.values)
.to.be.an('array')
.and.have.length(9);

expect(siri.values.map(v => v.x)).to.eql([4, 4, 4, 3, 3, 2, 2, 1, 1]);
});
});
37 changes: 21 additions & 16 deletions src/ui/public/agg_response/point_series/__tests__/_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,7 @@ describe('pointSeriesChartDataFromTable', function () {
// one series for each extension, and then one for each metric inside
expect(chartData.series).to.have.length(extensions.length * metricCount);
chartData.series.forEach(function (siri) {
// figure out the metric used to create this series
const metricAgg = siri.values[0].aggConfigResult.aggConfig;
const metric = avg.agg === metricAgg ? avg : max;

expect(siri.values).to.have.length(rowsPerSegment);
expect(siri.values).to.have.length(rowCount);
siri.values.forEach(function (point) {
expect(point).to.have.property('x');
expect(point.x).to.be.a('number');
Expand All @@ -254,18 +250,27 @@ describe('pointSeriesChartDataFromTable', function () {
expect(point.y).to.be.a('number');

expect(point).to.have.property('series');
expect(_.contains(extensions, point.series)).to.be.ok();

expect(point).to.have.property('aggConfigResult');
expect(point.aggConfigResult)
.to.be.a(AggConfigResult)
.and.have.property('aggConfig', metric.agg)
.and.have.property('value', point.y)
.and.to.have.property('$parent');

expect(point.aggConfigResult.$parent)
.to.be.an(AggConfigResult)
.and.have.property('aggConfig', term.agg);
// only perform these assertions on points that are non zero-filled
if (!point.xi) {
// figure out the metric used to create this series
const firstVal = siri.values.find(val => !val.xi);
const metricAgg = firstVal.aggConfigResult.aggConfig;
const metric = avg.agg === metricAgg ? avg : max;

expect(point).to.have.property('aggConfigResult');
expect(_.contains(extensions, point.series)).to.be.ok();

expect(point.aggConfigResult)
.to.be.a(AggConfigResult)
.and.have.property('aggConfig', metric.agg)
.and.have.property('value', point.y)
.and.to.have.property('$parent');

expect(point.aggConfigResult.$parent)
.to.be.an(AggConfigResult)
.and.have.property('aggConfig', term.agg);
}
});
});
});
Expand Down
40 changes: 29 additions & 11 deletions src/ui/public/agg_response/point_series/_add_to_siri.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,38 @@
* specific language governing permissions and limitations
* under the License.
*/
import { findLastIndex } from 'lodash';

export function addToSiri(series, point, id, label, agg) {
function instantiateZeroFilledArray({ xKeys, series }) {
// borrowed from `ui/public/vislib/components/zero_injection/zero_filled_array`
return xKeys.map(x => ({
x,
xi: Infinity,
y: 0,
series,
}));
}

export function addToSiri(xKeys, series, point, id, label, agg) {
id = id == null ? '' : id + '';

if (series.has(id)) {
series.get(id).values.push(point);
return;
if (!series.has(id)) {
series.set(id, {
label: label == null ? id : label,
aggLabel: agg.type ? agg.type.makeLabel(agg) : label,
aggId: agg.parentId ? agg.parentId : agg.id,
count: 0,
values: instantiateZeroFilledArray({ xKeys, series: label }),
});
}

series.set(id, {
label: label == null ? id : label,
aggLabel: agg.type ? agg.type.makeLabel(agg) : label,
aggId: agg.parentId ? agg.parentId : agg.id,
count: 0,
values: [point]
});
const seriesValues = series.get(id).values;
const lastXIndex = findLastIndex(seriesValues, v => v.x === point.x);
if (seriesValues[lastXIndex].xi) {
// update the ordered, zero-filled array with the "real" value
seriesValues[lastXIndex] = point;
} else {
// add the point to the list of values at the correct position
seriesValues.splice(lastXIndex + 1, 0, point);
}
}
24 changes: 21 additions & 3 deletions src/ui/public/agg_response/point_series/_get_series.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,22 @@ export function getSeries(rows, chart) {
const multiY = Array.isArray(aspects.y);
const yScale = chart.yScale;
const partGetPoint = _.partial(getPoint, aspects.x, aspects.series, yScale);
const xKeys = aspects.x.i > -1 ? _.uniq(rows.map(r => r[aspects.x.i].value)) : ['_all'];

let series = _(rows)
.transform(function (series, row) {
if (!multiY) {
const point = partGetPoint(row, aspects.y, aspects.z);
if (point) addToSiri(series, point, point.series, point.series, aspects.y.aggConfig);
if (point) {
addToSiri(
xKeys,
series,
point,
point.series,
point.series,
aspects.y.aggConfig,
);
}
return;
}

Expand All @@ -51,7 +61,14 @@ export function getSeries(rows, chart) {
seriesLabel = prefix + seriesLabel;
}

addToSiri(series, point, seriesId, seriesLabel, y.aggConfig);
addToSiri(
xKeys,
series,
point,
seriesId,
seriesLabel,
y.aggConfig,
);
});

}, new Map())
Expand All @@ -60,7 +77,8 @@ export function getSeries(rows, chart) {

if (multiY) {
series = _.sortBy(series, function (siri) {
const firstVal = siri.values[0];
// find first non zero-filled value
const firstVal = siri.values.find(val => !val.xi);
let y;

if (firstVal) {
Expand Down
4 changes: 2 additions & 2 deletions src/ui/public/vislib/lib/types/point_series.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import _ from 'lodash';

export function VislibTypesPointSeries() {

const createSerieFromParams = (cfg, seri) => {
const createSeriesFromParams = (cfg, seri) => {
const matchingSeriesParams = cfg.seriesParams ? cfg.seriesParams.find(seriConfig => {
return seri.aggId === seriConfig.data.id;
}) : null;
Expand Down Expand Up @@ -55,7 +55,7 @@ export function VislibTypesPointSeries() {
type: 'point_series',
addTimeMarker: cfg.addTimeMarker,
series: _.map(series, (seri) => {
return createSerieFromParams(cfg, seri);
return createSeriesFromParams(cfg, seri);
})
};
};
Expand Down