From c8fd477a713a9c6dd3372901a46d6798c3d55bdf Mon Sep 17 00:00:00 2001 From: ppisljar Date: Fri, 31 Aug 2018 13:53:38 +0200 Subject: [PATCH 1/6] hierarchical response handler rewrite --- .../kbn_vislib_vis_types/public/pie.js | 3 + .../__tests__/array_to_linked_list.js | 46 --- .../__tests__/build_hierarchical_data.js | 304 +++++++----------- .../hierarchical/__tests__/create_raw_data.js | 101 ------ .../hierarchical/__tests__/extract_buckets.js | 82 ----- .../__tests__/transform_aggregation.js | 116 ------- .../hierarchical/_array_to_linked_list.js | 31 -- .../agg_response/hierarchical/_build_split.js | 36 --- .../hierarchical/_collect_keys.js | 29 -- .../hierarchical/_create_raw_data.js | 113 ------- .../hierarchical/_extract_buckets.js | 41 --- .../_hierarchical_tooltip_formatter.js | 2 +- .../hierarchical/_transform_aggregation.js | 60 ---- .../hierarchical/build_hierarchical_data.js | 121 +++---- src/ui/public/vis/response_handlers/legacy.js | 10 +- src/ui/public/vis/response_handlers/tabify.js | 7 +- src/ui/public/vis/response_handlers/vislib.js | 8 - 17 files changed, 177 insertions(+), 933 deletions(-) delete mode 100644 src/ui/public/agg_response/hierarchical/__tests__/array_to_linked_list.js delete mode 100644 src/ui/public/agg_response/hierarchical/__tests__/create_raw_data.js delete mode 100644 src/ui/public/agg_response/hierarchical/__tests__/extract_buckets.js delete mode 100644 src/ui/public/agg_response/hierarchical/__tests__/transform_aggregation.js delete mode 100644 src/ui/public/agg_response/hierarchical/_array_to_linked_list.js delete mode 100644 src/ui/public/agg_response/hierarchical/_build_split.js delete mode 100644 src/ui/public/agg_response/hierarchical/_collect_keys.js delete mode 100644 src/ui/public/agg_response/hierarchical/_create_raw_data.js delete mode 100644 src/ui/public/agg_response/hierarchical/_extract_buckets.js delete mode 100644 src/ui/public/agg_response/hierarchical/_transform_aggregation.js diff --git a/src/core_plugins/kbn_vislib_vis_types/public/pie.js b/src/core_plugins/kbn_vislib_vis_types/public/pie.js index 8b5e5efbbb1e1..7d281cb971ea9 100644 --- a/src/core_plugins/kbn_vislib_vis_types/public/pie.js +++ b/src/core_plugins/kbn_vislib_vis_types/public/pie.js @@ -20,11 +20,13 @@ import { VisFactoryProvider } from 'ui/vis/vis_factory'; import { Schemas } from 'ui/vis/editors/default/schemas'; import { CATEGORY } from 'ui/vis/vis_category'; +import { BuildHierarchicalDataProvider } from 'ui/agg_response/hierarchical/build_hierarchical_data'; import pieTemplate from './editors/pie.html'; import image from './images/icon-donut.svg'; export default function HistogramVisType(Private) { const VisFactory = Private(VisFactoryProvider); + const buildHierarchicalData = Private(BuildHierarchicalDataProvider); return VisFactory.createVislibVisualization({ name: 'pie', @@ -98,5 +100,6 @@ export default function HistogramVisType(Private) { ]) }, hierarchicalData: true, + responseConverter: buildHierarchicalData, }); } diff --git a/src/ui/public/agg_response/hierarchical/__tests__/array_to_linked_list.js b/src/ui/public/agg_response/hierarchical/__tests__/array_to_linked_list.js deleted file mode 100644 index 1af7ca8234ab4..0000000000000 --- a/src/ui/public/agg_response/hierarchical/__tests__/array_to_linked_list.js +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - - -import { arrayToLinkedList } from '../_array_to_linked_list'; -import expect from 'expect.js'; -describe('buildHierarchicalData()', function () { - describe('arrayToLinkedList', function () { - - let results; - beforeEach(function () { - results = arrayToLinkedList([ - { id: 1 }, - { id: 2 }, - { id: 3 } - ]); - }); - - it('should set the next element', function () { - expect(results[0]).to.have.property('_next', results[1]); - expect(results[1]).to.have.property('_next', results[2]); - }); - - it('should set the previous element', function () { - expect(results[1]).to.have.property('_previous', results[0]); - expect(results[2]).to.have.property('_previous', results[1]); - }); - - }); -}); diff --git a/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js b/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js index f0b5f8f2bb533..81e4ef7e9cf2b 100644 --- a/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js +++ b/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js @@ -20,50 +20,48 @@ import _ from 'lodash'; import fixtures from 'fixtures/fake_hierarchical_data'; -import sinon from 'sinon'; import expect from 'expect.js'; import ngMock from 'ng_mock'; -import { toastNotifications } from 'ui/notify'; +import { BuildHierarchicalDataProvider } from '../build_hierarchical_data'; import { VisProvider } from '../../../vis'; import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern'; -import { BuildHierarchicalDataProvider } from '../build_hierarchical_data'; +import { VislibResponseHandlerProvider } from '../../../vis/response_handlers/vislib'; -let Vis; -let Notifier; -let indexPattern; -let buildHierarchicalData; +let hierarchicalConverter; describe('buildHierarchicalData', function () { - const sandbox = sinon.createSandbox(); + let Vis; + let indexPattern; + let responseHandler; beforeEach(ngMock.module('kibana')); - beforeEach(ngMock.inject(function (Private, $injector) { - // stub the error method before requiring vis causes Notifier#error to be bound - Notifier = $injector.get('Notifier'); - sandbox.stub(Notifier.prototype, 'error'); - + beforeEach(ngMock.inject(function (Private) { + hierarchicalConverter = Private(BuildHierarchicalDataProvider); Vis = Private(VisProvider); indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); - buildHierarchicalData = Private(BuildHierarchicalDataProvider); + responseHandler = Private(VislibResponseHandlerProvider).handler; })); - afterEach(function () { - sandbox.restore(); - }); + const buildHierarchicalData = async (aggs, data) => { + const vis = new Vis(indexPattern, { type: 'histogram', aggs: aggs }); + vis.isHierarchical = () => true; + vis.type.responseConverter = hierarchicalConverter; + return await responseHandler(vis, data); + }; describe('metric only', function () { - let vis; let results; - beforeEach(function () { - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'avg', schema: 'metric', params: { field: 'bytes' } }, - ] - }); - vis.aggs[0].id = 'agg_1'; - results = buildHierarchicalData(vis, fixtures.metricOnly); + beforeEach(async function () { + const aggs = [{ + id: 'agg_1', + schema: 'metric', + type: 'avg', + params: { + field: 'bytes', + } + }]; + results = await buildHierarchicalData(aggs, fixtures.metricOnly); }); it('should set the slices with one child to a consistent label', function () { @@ -78,67 +76,90 @@ describe('buildHierarchicalData', function () { expect(results).to.have.property('raw'); expect(results.raw).to.have.property('rows'); expect(results.raw.rows).to.have.length(1); - expect(results.raw.rows).to.eql([[412032]]); }); }); describe('rows and columns', function () { + let results; - it('should set the rows', function () { - let id = 1; - const vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'avg', schema: 'metric', params: { field: 'bytes' } }, - { type: 'terms', schema: 'split', params: { field: 'extension', row: true } }, - { type: 'terms', schema: 'segment', params: { field: 'machine.os' } }, - { type: 'terms', schema: 'segment', params: { field: 'geo.src' } } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - const results = buildHierarchicalData(vis, fixtures.threeTermBuckets); + it('should set the rows', async function () { + const aggs = [{ + id: 'agg_2', + type: 'terms', + schema: 'split', + params: { + field: 'extension', + } + }, { + id: 'agg_3', + type: 'terms', + schema: 'group', + params: { + field: 'geo.src', + } + }]; + results = await buildHierarchicalData(aggs, fixtures.threeTermBuckets); expect(results).to.have.property('rows'); }); - it('should set the columns', function () { - let id = 1; - const vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'avg', schema: 'metric', params: { field: 'bytes' } }, - { type: 'terms', schema: 'split', params: { field: 'extension', row: false } }, - { type: 'terms', schema: 'segment', params: { field: 'machine.os' } }, - { type: 'terms', schema: 'segment', params: { field: 'geo.src' } } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - const results = buildHierarchicalData(vis, fixtures.threeTermBuckets); + it('should set the columns', async function () { + const aggs = [{ + id: 'agg_2', + type: 'terms', + schema: 'split', + params: { + row: false, + field: 'extension', + } + }, { + id: 'agg_3', + type: 'terms', + schema: 'group', + params: { + field: 'geo.src', + } + }]; + results = await buildHierarchicalData(aggs, fixtures.threeTermBuckets); expect(results).to.have.property('columns'); }); }); describe('threeTermBuckets', function () { - let vis; let results; - beforeEach(function () { - let id = 1; - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'avg', schema: 'metric', params: { field: 'bytes' } }, - { type: 'terms', schema: 'split', params: { field: 'extension' } }, - { type: 'terms', schema: 'segment', params: { field: 'machine.os' } }, - { type: 'terms', schema: 'segment', params: { field: 'geo.src' } } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - results = buildHierarchicalData(vis, fixtures.threeTermBuckets); + beforeEach(async function () { + const aggs = [{ + id: 'agg_1', + type: 'avg', + schema: 'metric', + params: { + field: 'bytes', + } + }, { + id: 'agg_2', + type: 'terms', + schema: 'split', + params: { + field: 'extension', + } + }, { + id: 'agg_3', + type: 'terms', + schema: 'group', + params: { + field: 'geo.src', + } + }, { + id: 'agg_4', + type: 'terms', + schema: 'group', + params: { + field: 'machine.os', + } + }]; + results = await buildHierarchicalData(aggs, fixtures.threeTermBuckets); }); it('should set the hits attribute for the results', function () { @@ -148,7 +169,6 @@ describe('buildHierarchicalData', function () { expect(item).to.have.property('slices'); expect(item.slices).to.have.property('children'); }); - expect(results).to.have.property('raw'); }); it('should set the parent of the first item in the split', function () { @@ -158,30 +178,24 @@ describe('buildHierarchicalData', function () { expect(results.rows[0].slices).to.have.property('children'); expect(results.rows[0].slices.children).to.have.length(2); expect(results.rows[0].slices.children[0]).to.have.property('aggConfigResult'); - expect(results.rows[0].slices.children[0].aggConfigResult.$parent).to.have.property('key', 'png'); + expect(results.rows[0].slices.children[0].aggConfigResult.$parent.$parent).to.have.property('key', 'png'); }); }); describe('oneHistogramBucket', function () { - let vis; let results; - beforeEach(function () { - let id = 1; - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { - type: 'count', - schema: 'metric' - }, - { type: 'histogram', schema: 'segment', params: { field: 'bytes', interval: 8192 } } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - results = buildHierarchicalData(vis, fixtures.oneHistogramBucket); + beforeEach(async function () { + const aggs = [{ + id: 'agg_2', + type: 'date_histogram', + schema: 'group', + params: { + field: 'time', + } + }]; + results = await buildHierarchicalData(aggs, fixtures.oneHistogramBucket); }); it('should set the hits attribute for the results', function () { @@ -196,34 +210,18 @@ describe('buildHierarchicalData', function () { }); describe('oneRangeBucket', function () { - let vis; let results; - beforeEach(function () { - let id = 1; - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { - type: 'count', - schema: 'metric' - }, - { - type: 'range', - schema: 'segment', - params: { - field: 'bytes', - ranges: [ - { from: 0, to: 1000 }, - { from: 1000, to: 2000 } - ] - } - } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - results = buildHierarchicalData(vis, fixtures.oneRangeBucket); + beforeEach(async function () { + const aggs = [{ + id: 'agg_2', + type: 'range', + schema: 'group', + params: { + field: 'bytes', + } + }]; + results = await buildHierarchicalData(aggs, fixtures.oneRangeBucket); }); it('should set the hits attribute for the results', function () { @@ -237,30 +235,19 @@ describe('buildHierarchicalData', function () { }); describe('oneFilterBucket', function () { - let vis; let results; - beforeEach(function () { - let id = 1; - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'count', schema: 'metric' }, - { - type: 'filters', - schema: 'segment', - params: { - filters: [ - { input: { query: { query_string: { query: 'type:apache' } } } }, - { input: { query: { query_string: { query: 'type:nginx' } } } } - ] - } - } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - results = buildHierarchicalData(vis, fixtures.oneFilterBucket); + beforeEach(async function () { + const aggs = [{ + id: 'agg_2', + type: 'filters', + schema: 'group', + params: { + field: 'geo.src', + filters: [ { label: 'type:apache' }, { label: 'type:nginx' } ] + } + }]; + results = await buildHierarchicalData(aggs, fixtures.oneFilterBucket); }); it('should set the hits attribute for the results', function () { @@ -272,47 +259,4 @@ describe('buildHierarchicalData', function () { }); - describe('oneFilterBucket that is a split', function () { - let vis; - let results; - - beforeEach(function () { - // Clear existing toasts. - toastNotifications.list.splice(0); - - let id = 1; - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'count', schema: 'metric' }, - { - type: 'filters', - schema: 'split', - params: { - filters: [ - { input: { query: { query_string: { query: 'type:apache' } } } }, - { input: { query: { query_string: { query: 'type:nginx' } } } } - ] - } - } - ] - }); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - results = buildHierarchicalData(vis, fixtures.oneFilterBucket); - }); - - it('should set the hits attribute for the results', function () { - // Ideally, buildHierarchicalData shouldn't be tightly coupled to toastNotifications. Instead, - // it should notify its consumer of this error and the consumer should be responsible for - // notifying the user. This test verifies the side effect of the error until we can remove - // this coupling. - expect(toastNotifications.list).to.have.length(1); - expect(results).to.have.property('slices'); - expect(results).to.have.property('names'); - expect(results.names).to.have.length(2); - expect(results).to.have.property('raw'); - }); - }); - }); diff --git a/src/ui/public/agg_response/hierarchical/__tests__/create_raw_data.js b/src/ui/public/agg_response/hierarchical/__tests__/create_raw_data.js deleted file mode 100644 index 7451ebd86b260..0000000000000 --- a/src/ui/public/agg_response/hierarchical/__tests__/create_raw_data.js +++ /dev/null @@ -1,101 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - - -import _ from 'lodash'; -import fixtures from 'fixtures/fake_hierarchical_data'; -import { createRawData } from '../_create_raw_data'; -import { arrayToLinkedList } from '../_array_to_linked_list'; -import expect from 'expect.js'; -import ngMock from 'ng_mock'; -import { VisProvider } from '../../../vis'; -import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern'; - -let Vis; -let indexPattern; - -describe('buildHierarchicalData()', function () { - describe('createRawData()', function () { - let vis; - let results; - - beforeEach(ngMock.module('kibana')); - - beforeEach(ngMock.inject(function (Private) { - Vis = Private(VisProvider); - indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); - })); - - beforeEach(function () { - let id = 1; - vis = new Vis(indexPattern, { - type: 'pie', - aggs: [ - { type: 'avg', schema: 'metric', params: { field: 'bytes' } }, - { type: 'terms', schema: 'split', params: { field: 'extension' } }, - { type: 'terms', schema: 'segment', params: { field: 'machine.os' } }, - { type: 'terms', schema: 'segment', params: { field: 'geo.src' } } - ] - }); - arrayToLinkedList(vis.aggs.bySchemaGroup.buckets); - // We need to set the aggs to a known value. - _.each(vis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - results = createRawData(vis, fixtures.threeTermBuckets); - }); - - it('should have columns set', function () { - expect(results).to.have.property('columns'); - expect(results.columns).to.have.length(6); - _.each(results.columns, function (column) { - expect(column).to.have.property('aggConfig'); - const agg = column.aggConfig; - expect(column).to.have.property('categoryName', agg.schema.name); - expect(column).to.have.property('id', agg.id); - expect(column).to.have.property('aggType', agg.type); - expect(column).to.have.property('field', agg.params.field); - expect(column).to.have.property('label', agg.type.makeLabel(agg)); - }); - expect(results.columns[0].aggConfig.id).to.be('agg_2'); - expect(results.columns[1].aggConfig.id).to.be('agg_1'); - expect(results.columns[2].aggConfig.id).to.be('agg_3'); - expect(results.columns[3].aggConfig.id).to.be('agg_1'); - expect(results.columns[4].aggConfig.id).to.be('agg_4'); - expect(results.columns[5].aggConfig.id).to.be('agg_1'); - }); - - it('should have rows set', function () { - expect(results).to.have.property('rows'); - expect(results.rows).to.eql([ - ['png', 412032, 'IT', 9299, 'win', 0], - ['png', 412032, 'IT', 9299, 'mac', 9299], - ['png', 412032, 'US', 8293, 'linux', 3992], - ['png', 412032, 'US', 8293, 'mac', 3029], - ['css', 412032, 'MX', 9299, 'win', 4992], - ['css', 412032, 'MX', 9299, 'mac', 5892], - ['css', 412032, 'US', 8293, 'linux', 3992], - ['css', 412032, 'US', 8293, 'mac', 3029], - ['html', 412032, 'CN', 9299, 'win', 4992], - ['html', 412032, 'CN', 9299, 'mac', 5892], - ['html', 412032, 'FR', 8293, 'win', 3992], - ['html', 412032, 'FR', 8293, 'mac', 3029] - ]); - }); - - }); -}); diff --git a/src/ui/public/agg_response/hierarchical/__tests__/extract_buckets.js b/src/ui/public/agg_response/hierarchical/__tests__/extract_buckets.js deleted file mode 100644 index 42109737547be..0000000000000 --- a/src/ui/public/agg_response/hierarchical/__tests__/extract_buckets.js +++ /dev/null @@ -1,82 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - - -import { extractBuckets } from '../_extract_buckets'; -import expect from 'expect.js'; - -describe('buildHierarchicalData()', function () { - describe('extractBuckets()', function () { - - it('should normalize a bucket object into an array', function () { - - const bucket = { - buckets: { - foo: { doc_count: 1 }, - bar: { doc_count: 2 } - } - }; - - const buckets = extractBuckets(bucket); - expect(buckets).to.be.an(Array); - expect(buckets).to.have.length(2); - expect(buckets[0]).to.have.property('key', 'foo'); - expect(buckets[0]).to.have.property('doc_count', 1); - expect(buckets[1]).to.have.property('key', 'bar'); - expect(buckets[1]).to.have.property('doc_count', 2); - }); - - it('should return an empty array for undefined buckets', function () { - const buckets = extractBuckets(); - expect(buckets).to.be.an(Array); - expect(buckets).to.have.length(0); - }); - - it('should return the bucket array', function () { - const bucket = { - buckets: [ - { key: 'foo', doc_count: 1 }, - { key: 'bar', doc_count: 2 } - ] - }; - const buckets = extractBuckets(bucket); - expect(buckets).to.be.an(Array); - expect(buckets).to.eql(bucket.buckets); - }); - - it('should attach keys using agg.getKey for array of buckets', () => { - const bucket = { - buckets: [ - { from: 10, doc_count: 1 }, - { from: 20, doc_count: 2 } - ] - }; - const agg = { - getKey(bucket) { - return bucket.from; - } - }; - const buckets = extractBuckets(bucket, agg); - expect(buckets).to.have.length(2); - expect(buckets[0].key).to.be(10); - expect(buckets[1].key).to.be(20); - }); - - }); -}); diff --git a/src/ui/public/agg_response/hierarchical/__tests__/transform_aggregation.js b/src/ui/public/agg_response/hierarchical/__tests__/transform_aggregation.js deleted file mode 100644 index ba8b72636b647..0000000000000 --- a/src/ui/public/agg_response/hierarchical/__tests__/transform_aggregation.js +++ /dev/null @@ -1,116 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import _ from 'lodash'; -import expect from 'expect.js'; -import ngMock from 'ng_mock'; -import { HierarchicalTransformAggregationProvider } from '../_transform_aggregation'; - -describe('buildHierarchicalData()', function () { - describe('transformAggregation()', function () { - let transform; - let fixture; - - beforeEach(ngMock.module('kibana')); - beforeEach(ngMock.inject(function (Private) { - transform = Private(HierarchicalTransformAggregationProvider); - })); - - beforeEach(function () { - - function fakeAgg(id, name) { - return { - id: id, - name: name, - type: { type: 'buckets' }, - getKey: (bucket) => bucket.key, - fieldFormatter: _.constant(String) - }; - } - - fixture = {}; - fixture.agg = fakeAgg('agg_2', 'test'); - fixture.agg._next = fakeAgg('agg_3', 'example'); - - fixture.metric = fakeAgg('agg_1', 'metric'); - fixture.metric.getValue = function (b) { return _.has(b, this.id) ? b[this.id] : b.doc_count; }; - - fixture.aggData = { - buckets: [ - { key: 'foo', doc_count: 2, agg_3: { buckets: [ { key: 'win', doc_count: 1 }, { key: 'mac', doc_count: 1 }] } }, - { key: 'bar', doc_count: 4, agg_3: { buckets: [ { key: 'win', doc_count: 2 }, { key: 'mac', doc_count: 2 }] } } - ] - }; - - }); - - it('relies on metricAgg#getValue() for the size of the children', function () { - const aggData = { - buckets: [ - { key: 'foo' }, - { key: 'bar' } - ] - }; - - const football = {}; - fixture.metric.getValue = _.constant(football); - - const children = transform(fixture.agg, fixture.metric, aggData); - expect(children).to.be.an(Array); - expect(children).to.have.length(2); - expect(children[0]).to.have.property('size', football); - expect(children[1]).to.have.property('size', football); - }); - - it('should create two levels of metrics', function () { - const children = transform(fixture.agg, fixture.metric, fixture.aggData); - fixture.metric.getValue = function (b) { return b.doc_count; }; - - expect(children).to.be.an(Array); - expect(children).to.have.length(2); - expect(children[0]).to.have.property('children'); - expect(children[1]).to.have.property('children'); - expect(children[0]).to.have.property('aggConfig', fixture.agg); - expect(children[1]).to.have.property('aggConfig', fixture.agg); - expect(children[0].children).to.have.length(2); - expect(children[1].children).to.have.length(2); - expect(children[0]).to.have.property('name', 'foo'); - expect(children[0]).to.have.property('size', 2); - expect(children[1]).to.have.property('name', 'bar'); - expect(children[1]).to.have.property('size', 4); - expect(children[0].children[0]).to.have.property('size', 1); - expect(children[0].children[0]).to.have.property('aggConfig', fixture.agg.agg_3); - expect(children[0].children[0]).to.have.property('name', 'win'); - expect(children[0].children[1]).to.have.property('size', 1); - expect(children[0].children[1]).to.have.property('parent', children[0]); - expect(children[0].children[1]).to.have.property('aggConfig', fixture.agg.agg_3); - expect(children[0].children[1]).to.have.property('name', 'mac'); - expect(children[1].children[0]).to.have.property('size', 2); - expect(children[0].children[1]).to.have.property('parent', children[0]); - expect(children[1].children[0]).to.have.property('aggConfig', fixture.agg.agg_3); - expect(children[1].children[0]).to.have.property('name', 'win'); - expect(children[1].children[1]).to.have.property('size', 2); - expect(children[1].children[1]).to.have.property('parent', children[1]); - expect(children[1].children[1]).to.have.property('aggConfig', fixture.agg.agg_3); - expect(children[1].children[1]).to.have.property('name', 'mac'); - expect(children[1].children[1]).to.have.property('parent', children[1]); - }); - - }); -}); diff --git a/src/ui/public/agg_response/hierarchical/_array_to_linked_list.js b/src/ui/public/agg_response/hierarchical/_array_to_linked_list.js deleted file mode 100644 index 3db07d22e3266..0000000000000 --- a/src/ui/public/agg_response/hierarchical/_array_to_linked_list.js +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import _ from 'lodash'; -export function arrayToLinkedList(buckets) { - let previous; - _.each(buckets, function (bucket) { - if (previous) { - bucket._previous = previous; - previous._next = bucket; - } - previous = bucket; - }); - return buckets; -} diff --git a/src/ui/public/agg_response/hierarchical/_build_split.js b/src/ui/public/agg_response/hierarchical/_build_split.js deleted file mode 100644 index b8d5f8107abe5..0000000000000 --- a/src/ui/public/agg_response/hierarchical/_build_split.js +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { collectKeys } from './_collect_keys'; -import { HierarchicalTransformAggregationProvider } from './_transform_aggregation'; - -export function AggResponseHierarchicalBuildSplitProvider(Private) { - const transformer = Private(HierarchicalTransformAggregationProvider); - return function (agg, metric, aggData) { - // Ceate the split structure - const split = { label: '', slices: { children: [] } }; - - // Transform the aggData into splits - split.slices.children = transformer(agg, metric, aggData); - - // Collect all the keys - split.names = collectKeys(split.slices.children); - return split; - }; -} diff --git a/src/ui/public/agg_response/hierarchical/_collect_keys.js b/src/ui/public/agg_response/hierarchical/_collect_keys.js deleted file mode 100644 index f5f9b910521dc..0000000000000 --- a/src/ui/public/agg_response/hierarchical/_collect_keys.js +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import _ from 'lodash'; -export function collectKeys(children) { - const nextChildren = _.pluck(children, 'children'); - const keys = _.pluck(children, 'name'); - return _(nextChildren) - .map(collectKeys) - .flattenDeep() - .union(keys) - .value(); -} diff --git a/src/ui/public/agg_response/hierarchical/_create_raw_data.js b/src/ui/public/agg_response/hierarchical/_create_raw_data.js deleted file mode 100644 index 50c8580042c05..0000000000000 --- a/src/ui/public/agg_response/hierarchical/_create_raw_data.js +++ /dev/null @@ -1,113 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import _ from 'lodash'; -import { extractBuckets } from './_extract_buckets'; - -export function createRawData(vis, resp) { - - // Create the initial results structure - const results = { rows: [] }; - - // Create a reference to the buckets and metrics - const metrics = vis.getAggConfig().bySchemaGroup.metrics; - const buckets = vis.getAggConfig().bySchemaGroup.buckets; - const aggs = []; - - if (buckets) { - _.each(buckets, function (bucket) { - aggs.push(bucket); - aggs.push(metrics); - }); - } else { - aggs.push(metrics); - } - - // Create the columns - results.columns = _(aggs) - .flattenDeep() - .map(function (agg) { - return { - categoryName: agg.schema.name, - id: agg.id, - aggConfig: agg, - aggType: agg.type, - field: agg.params.field, - label: agg.makeLabel() - }; - }) - .value(); - - - // if there are no buckets then we need to just set the value and return - if (!buckets) { - const value = resp.aggregations - && resp.aggregations[metrics[0].id] - && resp.aggregations[metrics[0].id].value - || resp.hits.total; - results.rows.push([value]); - return results; - } - - /** - * Walk the buckets and create records for each leaf - * @param {aggConfig} agg The aggConfig for the current level - * @param {object} data The aggregation object - * @param {array} [record] The record that will eventually get pushed to the rows - * @returns {void} - */ - function walkBuckets(agg, data, record) { - if (!data) return; - if (!Array.isArray(record)) { - record = []; - } - - // iterate through all the buckets - _.each(extractBuckets(data[agg.id], agg), function (bucket) { - - const _record = _.flattenDeep([record, bucket.key]); - _.each(metrics, function (metric) { - let value = bucket.doc_count; - if (bucket[metric.id] && !_.isUndefined(bucket[metric.id].value)) { - value = bucket[metric.id].value; - } - _record.push(value); - }); - - // If there is another agg to call we need to check to see if it has - // buckets. If it does then we need to keep on walking the tree. - // This is where the recursion happens. - if (agg._next) { - const nextBucket = bucket[agg._next.id]; - if (nextBucket && nextBucket.buckets) { - walkBuckets(agg._next, bucket, _record); - } - } - // if there are no more aggs to walk then push the record to the rows. - else { - results.rows.push(_record); - } - }); - } - - // Start walking the buckets at the beginning of the aggregations object. - walkBuckets(buckets[0], resp.aggregations); - - return results; -} diff --git a/src/ui/public/agg_response/hierarchical/_extract_buckets.js b/src/ui/public/agg_response/hierarchical/_extract_buckets.js deleted file mode 100644 index a0d92cf015f8a..0000000000000 --- a/src/ui/public/agg_response/hierarchical/_extract_buckets.js +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import _ from 'lodash'; - -function decorateWithKey(agg, bucket, key) { - const decorated = _.cloneDeep(bucket); - decorated.key = agg ? agg.getKey(bucket, key) : key; - return decorated; -} - -export function extractBuckets(bucket, agg) { - if (bucket) { - if (_.isPlainObject(bucket.buckets)) { - return _.map(bucket.buckets, function (value, key) { - return decorateWithKey(agg, value, key); - }); - } else if(_.isArray(bucket.buckets)) { - return bucket.buckets.map(value => { - return decorateWithKey(agg, value, value.key); - }); - } - } - return bucket && bucket.buckets || []; -} diff --git a/src/ui/public/agg_response/hierarchical/_hierarchical_tooltip_formatter.js b/src/ui/public/agg_response/hierarchical/_hierarchical_tooltip_formatter.js index 6e2d7e024d616..6bb00a31eaa45 100644 --- a/src/ui/public/agg_response/hierarchical/_hierarchical_tooltip_formatter.js +++ b/src/ui/public/agg_response/hierarchical/_hierarchical_tooltip_formatter.js @@ -35,7 +35,7 @@ export function HierarchicalTooltipFormatterProvider($rootScope, $compile, $sce) // Collect the current leaf and parents into an array of values $tooltipScope.rows = collectBranch(datum); - const metricCol = $tooltipScope.metricCol = _.find(columns, { categoryName: 'metric' }); + const metricCol = $tooltipScope.metricCol = columns.find(column => column.aggConfig.type.type === 'metrics'); // Map those values to what the tooltipSource.rows format. _.forEachRight($tooltipScope.rows, function (row) { diff --git a/src/ui/public/agg_response/hierarchical/_transform_aggregation.js b/src/ui/public/agg_response/hierarchical/_transform_aggregation.js deleted file mode 100644 index ac94864fe1286..0000000000000 --- a/src/ui/public/agg_response/hierarchical/_transform_aggregation.js +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import _ from 'lodash'; -import { extractBuckets } from './_extract_buckets'; -import AggConfigResult from '../../vis/agg_config_result'; - -export function HierarchicalTransformAggregationProvider() { - return function transformAggregation(agg, metric, aggData, parent) { - return _.map(extractBuckets(aggData, agg), function (bucket) { - const aggConfigResult = new AggConfigResult( - agg, - parent && parent.aggConfigResult, - metric.getValue(bucket), - agg.getKey(bucket), - bucket.filters - ); - - const branch = { - name: agg.fieldFormatter()(bucket.key), - size: aggConfigResult.value, - aggConfig: agg, - aggConfigResult: aggConfigResult - }; - - // if the parent is defined then we need to set the parent of the branch - // this will be used later for filters for waking up the parent path. - if (parent) { - branch.parent = parent; - } - - // If the next bucket exists and it has children the we need to - // transform it as well. This is where the recursion happens. - if (agg._next) { - const nextBucket = bucket[agg._next.id]; - if (nextBucket && nextBucket.buckets) { - branch.children = transformAggregation(agg._next, metric, nextBucket, branch); - } - } - - return branch; - }); - }; -} diff --git a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js index fc7734a13d852..4466d4f524b2d 100644 --- a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js +++ b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js @@ -17,99 +17,52 @@ * under the License. */ -import _ from 'lodash'; -import { toastNotifications } from 'ui/notify'; -import { extractBuckets } from './_extract_buckets'; -import { createRawData } from './_create_raw_data'; -import { arrayToLinkedList } from './_array_to_linked_list'; -import AggConfigResult from '../../vis/agg_config_result'; -import { AggResponseHierarchicalBuildSplitProvider } from './_build_split'; +import { toArray } from 'lodash'; import { HierarchicalTooltipFormatterProvider } from './_hierarchical_tooltip_formatter'; export function BuildHierarchicalDataProvider(Private) { - const buildSplit = Private(AggResponseHierarchicalBuildSplitProvider); const tooltipFormatter = Private(HierarchicalTooltipFormatterProvider); - return function (vis, resp) { - // Create a reference to the buckets - let buckets = vis.getAggConfig().bySchemaGroup.buckets; - - // Find the metric so it's easier to reference. - // TODO: Change this to support multiple metrics. - const metric = vis.getAggConfig().bySchemaGroup.metrics[0]; - - // Link each agg to the next agg. This will be - // to identify the next bucket aggregation - buckets = arrayToLinkedList(buckets); - - // Create the raw data to be used in the spy panel - const raw = createRawData(vis, resp); + return function (table) { + let slices; + const names = {}; + if (table.columns.length === 1) { + slices = [{ name: table.columns[0].title, size: table.rows[0][0].value }]; + names[table.columns[0].title] = table.columns[0].title; + } else { + let parent; + slices = []; + table.rows.forEach(row => { + let dataLevel = slices; + // we always have one bucket column and one metric column (for every level) + for (let columnIndex = 0; columnIndex < table.columns.length; columnIndex += 2) { + const name = row[columnIndex].value; + const size = row[columnIndex + 1].value; + const aggConfig = table.columns[columnIndex].aggConfig; + const aggConfigResult = row[columnIndex + 1]; + names[name] = name; - // If buckets is falsy then we should just return the aggs - if (!buckets) { - const label = raw.columns[0].label; - const value = resp.aggregations - && resp.aggregations[metric.id] - && resp.aggregations[metric.id].value - || resp.hits.total; - return { - hits: resp.hits.total, - raw: raw, - names: [label], - tooltipFormatter: tooltipFormatter(raw.columns), - slices: { - children: [ - { name: label, size: value } - ] + let slice = dataLevel.find(slice => slice.name === name); + if (!slice) { + slice = { name, size, parent, aggConfig, aggConfigResult, children: [] }; + dataLevel.push(slice); + } + parent = slice; + dataLevel = slice.children; } - }; - } - - const firstAgg = buckets[0]; - const aggData = resp.aggregations ? resp.aggregations[firstAgg.id] : null; - - if (!firstAgg._next && firstAgg.schema.name === 'split') { - toastNotifications.addDanger({ - title: 'Splitting charts without splitting slices is not supported', - text: 'Pretending that we are just splitting slices.' - }); - } - - // start with splitting slices - if (!firstAgg._next || firstAgg.schema.name === 'segment') { - const split = buildSplit(firstAgg, metric, aggData); - split.hits = resp.hits.total; - split.raw = raw; - split.tooltipFormatter = tooltipFormatter(raw.columns); - return split; - } - - // map the split aggregations into rows. - const rows = _.map(extractBuckets(aggData, firstAgg), function (bucket) { - const agg = firstAgg._next; - const split = buildSplit(agg, metric, bucket[agg.id]); - // Since splits display labels we need to set it. - split.label = firstAgg.fieldFormatter()(agg.getKey(bucket)); - - const displayName = firstAgg.getFieldDisplayName(); - if (!_.isEmpty(displayName)) split.label += ': ' + displayName; - - split.tooltipFormatter = tooltipFormatter(raw.columns); - const aggConfigResult = new AggConfigResult(firstAgg, null, null, firstAgg.getKey(bucket), bucket.filters); - split.split = { aggConfig: firstAgg, aggConfigResult: aggConfigResult, key: bucket.key }; - _.each(split.slices.children, function (child) { - child.aggConfigResult.$parent = aggConfigResult; }); - return split; - }); - - const result = { hits: resp.hits.total, raw: raw }; - if (firstAgg.params.row) { - result.rows = rows; - } else { - result.columns = rows; } - return result; + return { + hits: table.rows.length, + raw: table, + names: toArray(names), + tooltipFormatter: tooltipFormatter(table.columns), + slices: { + children: [ + ...slices + ] + } + }; }; } diff --git a/src/ui/public/vis/response_handlers/legacy.js b/src/ui/public/vis/response_handlers/legacy.js index c8072efa4b4d2..92ac48143c236 100644 --- a/src/ui/public/vis/response_handlers/legacy.js +++ b/src/ui/public/vis/response_handlers/legacy.js @@ -40,6 +40,10 @@ const LegacyResponseHandlerProvider = function () { const asAggConfigResults = _.get(vis, 'type.responseHandlerConfig.asAggConfigResults', false); const splitColumn = table.columns.find(column => column.aggConfig.schema.name === 'split'); + const numberOfMetrics = table.columns.filter(column => column.aggConfig.type.type === 'metrics').length; + const numberOfBuckets = table.columns.filter(column => column.aggConfig.type.type === 'buckets').length; + const metricsPerBucket = numberOfMetrics / numberOfBuckets; + if (splitColumn) { const splitAgg = splitColumn.aggConfig; const splitMap = {}; @@ -60,7 +64,11 @@ const LegacyResponseHandlerProvider = function () { }; tableGroup.tables.push({ $parent: tableGroup, - columns: table.columns.filter((column, i) => i !== splitColumnIndex).map(column => ({ title: column.name, ...column })), + columns: table.columns.filter((column, i) => { + const isSplitColumn = i === splitColumnIndex; + const isSplitMetric = metricsAtAllLevels && i > splitColumnIndex && i <= splitColumnIndex + metricsPerBucket; + return !isSplitColumn && !isSplitMetric; + }).map(column => ({ title: column.name, ...column })), rows: [] }); diff --git a/src/ui/public/vis/response_handlers/tabify.js b/src/ui/public/vis/response_handlers/tabify.js index ac45995b73c24..a226645e8c1bf 100644 --- a/src/ui/public/vis/response_handlers/tabify.js +++ b/src/ui/public/vis/response_handlers/tabify.js @@ -17,12 +17,11 @@ * under the License. */ -import { AggResponseIndexProvider } from '../../agg_response'; +import { tabifyAggResponse } from '../../agg_response/tabify'; import { VisResponseHandlersRegistryProvider } from '../../registry/vis_response_handlers'; import { getTime } from 'ui/timefilter/get_time'; -const TabifyResponseHandlerProvider = function (Private) { - const aggResponse = Private(AggResponseIndexProvider); +const TabifyResponseHandlerProvider = function () { return { name: 'tabify', @@ -30,7 +29,7 @@ const TabifyResponseHandlerProvider = function (Private) { return new Promise((resolve) => { const time = getTime(vis.indexPattern, vis.filters.timeRange); - const tableGroup = aggResponse.tabify(vis.getAggConfig(), response, { + const tableGroup = tabifyAggResponse(vis.getAggConfig(), response, { metricsAtAllLevels: vis.isHierarchical(), partialRows: vis.params.showPartialRows, timeRange: time ? time.range : undefined, diff --git a/src/ui/public/vis/response_handlers/vislib.js b/src/ui/public/vis/response_handlers/vislib.js index d8359b7bdef6a..bdea0aeb569a2 100644 --- a/src/ui/public/vis/response_handlers/vislib.js +++ b/src/ui/public/vis/response_handlers/vislib.js @@ -17,12 +17,10 @@ * under the License. */ -import { AggResponseIndexProvider } from '../../agg_response'; import { LegacyResponseHandlerProvider } from './legacy'; import { VisResponseHandlersRegistryProvider } from '../../registry/vis_response_handlers'; const VislibResponseHandlerProvider = function (Private) { - const aggResponse = Private(AggResponseIndexProvider); const tableResponseProvider = Private(LegacyResponseHandlerProvider).handler; function convertTableGroup(vis, tableGroup) { @@ -68,12 +66,6 @@ const VislibResponseHandlerProvider = function (Private) { name: 'vislib', handler: function (vis, response) { return new Promise((resolve) => { - if (vis.isHierarchical()) { - // the hierarchical converter is very self-contained (woot!) - // todo: it should be updated to be based on tabified data just as other responseConverters - resolve(aggResponse.hierarchical(vis, response)); - } - return tableResponseProvider(vis, response).then(tableGroup => { let converted = convertTableGroup(vis, tableGroup); if (!converted) { From 9f0d3d05632d5b9ed0e7a9fed5599ebe8d492811 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Tue, 11 Sep 2018 13:28:31 +0200 Subject: [PATCH 2/6] fixing unit tests --- .../__tests__/visualizations/pie_chart.js | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/ui/public/vislib/__tests__/visualizations/pie_chart.js b/src/ui/public/vislib/__tests__/visualizations/pie_chart.js index 5d6fcd8090d81..fb7bcf9eb638f 100644 --- a/src/ui/public/vislib/__tests__/visualizations/pie_chart.js +++ b/src/ui/public/vislib/__tests__/visualizations/pie_chart.js @@ -28,6 +28,7 @@ import { VisProvider } from '../../../vis'; import '../../../persisted_state'; import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern'; import { BuildHierarchicalDataProvider } from '../../../agg_response/hierarchical/build_hierarchical_data'; +import { VislibResponseHandlerProvider } from '../../../vis/response_handlers/vislib'; const rowAgg = [ { type: 'avg', schema: 'metric', params: { field: 'bytes' } }, @@ -89,8 +90,11 @@ describe('No global chart settings', function () { let persistedState; let indexPattern; let buildHierarchicalData; + let responseHandler; let data1; let data2; + let stubVis1; + let stubVis2; beforeEach(ngMock.module('kibana')); beforeEach(ngMock.inject(function (Private, $injector) { @@ -100,18 +104,25 @@ describe('No global chart settings', function () { persistedState = new ($injector.get('PersistedState'))(); indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); buildHierarchicalData = Private(BuildHierarchicalDataProvider); + responseHandler = Private(VislibResponseHandlerProvider).handler; let id1 = 1; let id2 = 1; - const stubVis1 = new Vis(indexPattern, { + stubVis1 = new Vis(indexPattern, { type: 'pie', aggs: rowAgg }); - const stubVis2 = new Vis(indexPattern, { + stubVis2 = new Vis(indexPattern, { type: 'pie', aggs: colAgg }); + stubVis1.isHierarchical = () => true; + stubVis1.type.responseConverter = buildHierarchicalData; + + stubVis2.isHierarchical = () => true; + stubVis2.type.responseConverter = buildHierarchicalData; + // We need to set the aggs to a known value. _.each(stubVis1.aggs, function (agg) { agg.id = 'agg_' + id1++; @@ -119,13 +130,15 @@ describe('No global chart settings', function () { _.each(stubVis2.aggs, function (agg) { agg.id = 'agg_' + id2++; }); + })); - data1 = buildHierarchicalData(stubVis1, fixtures.threeTermBuckets); - data2 = buildHierarchicalData(stubVis2, fixtures.threeTermBuckets); + beforeEach(async () => { + data1 = await responseHandler(stubVis1, fixtures.threeTermBuckets); + data2 = await responseHandler(stubVis2, fixtures.threeTermBuckets); chart1.render(data1, persistedState); chart2.render(data2, persistedState); - })); + }); afterEach(function () { chart1.destroy(); @@ -185,8 +198,9 @@ describe('Vislib PieChart Class Test Suite', function () { let Vis; let persistedState; let indexPattern; - let buildHierarchicalData; let data; + let stubVis; + let responseHandler; beforeEach(ngMock.module('kibana')); beforeEach(ngMock.inject(function (Private, $injector) { @@ -194,10 +208,10 @@ describe('Vislib PieChart Class Test Suite', function () { Vis = Private(VisProvider); persistedState = new ($injector.get('PersistedState'))(); indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); - buildHierarchicalData = Private(BuildHierarchicalDataProvider); + responseHandler = Private(VislibResponseHandlerProvider).handler; let id = 1; - const stubVis = new Vis(indexPattern, { + stubVis = new Vis(indexPattern, { type: 'pie', aggs: dataAgg }); @@ -205,10 +219,12 @@ describe('Vislib PieChart Class Test Suite', function () { // We need to set the aggs to a known value. _.each(stubVis.aggs, function (agg) { agg.id = 'agg_' + id++; }); - data = buildHierarchicalData(stubVis, fixtures.threeTermBuckets); + })); + beforeEach(async () => { + data = await responseHandler(stubVis, fixtures.threeTermBuckets); vis.render(data, persistedState); - })); + }); afterEach(function () { vis.destroy(); From 2ed4518ab1e7df6fc58aa2df0148b1666c93e18d Mon Sep 17 00:00:00 2001 From: ppisljar Date: Tue, 11 Sep 2018 13:43:37 +0200 Subject: [PATCH 3/6] using field formatter on name --- .../agg_response/hierarchical/build_hierarchical_data.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js index 4466d4f524b2d..465eb957f826d 100644 --- a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js +++ b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js @@ -36,7 +36,8 @@ export function BuildHierarchicalDataProvider(Private) { let dataLevel = slices; // we always have one bucket column and one metric column (for every level) for (let columnIndex = 0; columnIndex < table.columns.length; columnIndex += 2) { - const name = row[columnIndex].value; + const fieldFormatter = table.columns[columnIndex].aggConfig.fieldFormatter('text'); + const name = fieldFormatter(row[columnIndex].value); const size = row[columnIndex + 1].value; const aggConfig = table.columns[columnIndex].aggConfig; const aggConfigResult = row[columnIndex + 1]; From 24faf147e8cf9af58f6216ffbd300085c5ae4ca1 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 12 Sep 2018 13:20:39 +0200 Subject: [PATCH 4/6] fixing unit test --- .../hierarchical/__tests__/build_hierarchical_data.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js b/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js index 81e4ef7e9cf2b..5dec0e1615b10 100644 --- a/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js +++ b/src/ui/public/agg_response/hierarchical/__tests__/build_hierarchical_data.js @@ -189,10 +189,11 @@ describe('buildHierarchicalData', function () { beforeEach(async function () { const aggs = [{ id: 'agg_2', - type: 'date_histogram', + type: 'histogram', schema: 'group', params: { - field: 'time', + field: 'bytes', + interval: 8192 } }]; results = await buildHierarchicalData(aggs, fixtures.oneHistogramBucket); From 524621772510dfafd3d1170a9d568f6cedfa37a8 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Thu, 13 Sep 2018 08:40:15 +0200 Subject: [PATCH 5/6] updating based on marcos feedback --- .../hierarchical/build_hierarchical_data.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js index 465eb957f826d..82e00275ee758 100644 --- a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js +++ b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js @@ -36,16 +36,17 @@ export function BuildHierarchicalDataProvider(Private) { let dataLevel = slices; // we always have one bucket column and one metric column (for every level) for (let columnIndex = 0; columnIndex < table.columns.length; columnIndex += 2) { - const fieldFormatter = table.columns[columnIndex].aggConfig.fieldFormatter('text'); - const name = fieldFormatter(row[columnIndex].value); - const size = row[columnIndex + 1].value; - const aggConfig = table.columns[columnIndex].aggConfig; - const aggConfigResult = row[columnIndex + 1]; + const { aggConfig } = table.columns[columnIndex]; + const fieldFormatter = aggConfig.fieldFormatter('text'); + const bucketColumn = row[columnIndex]; + const metricColumn = row[columnIndex + 1]; + const name = fieldFormatter(bucketColumn.value); + const size = metricColumn.value; names[name] = name; let slice = dataLevel.find(slice => slice.name === name); if (!slice) { - slice = { name, size, parent, aggConfig, aggConfigResult, children: [] }; + slice = { name, size, parent, aggConfig, metricColumn, children: [] }; dataLevel.push(slice); } parent = slice; From 142deac054ba2be974fb84e05e1ecd26aa28da75 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Thu, 13 Sep 2018 09:49:52 +0200 Subject: [PATCH 6/6] :| --- .../public/agg_response/hierarchical/build_hierarchical_data.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js index 82e00275ee758..1393bba064a78 100644 --- a/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js +++ b/src/ui/public/agg_response/hierarchical/build_hierarchical_data.js @@ -46,7 +46,7 @@ export function BuildHierarchicalDataProvider(Private) { let slice = dataLevel.find(slice => slice.name === name); if (!slice) { - slice = { name, size, parent, aggConfig, metricColumn, children: [] }; + slice = { name, size, parent, aggConfig, aggConfigResult: metricColumn, children: [] }; dataLevel.push(slice); } parent = slice;