Merge pull request #2221 from spenceralger/split_dat_pie

Prevent failure in Hierarchical converter

src/kibana/components/agg_response/hierarchical/build_hierarchical_data.js

@@ -1,5 +1,5 @@
 define(function (require) {
-  return function buildHierarchicalDataProvider(Private) {
+  return function buildHierarchicalDataProvider(Private, Notifier) {
     var _ = require('lodash');
     var buildSplit = Private(require('components/agg_response/hierarchical/_build_split'));
     var extractBuckets = require('components/agg_response/hierarchical/_extract_buckets');
@@ -9,6 +9,10 @@ define(function (require) {
 
     var AggConfigResult = require('components/vis/_agg_config_result');
 
+    var notify = new Notifier({
+      location: 'Pie chart response converter'
+    });
+
     return function (vis, resp) {
       // Create a refrenece to the buckets
       var buckets = vis.aggs.bySchemaGroup.buckets;
@@ -47,48 +51,42 @@ define(function (require) {
       var firstAgg = buckets[0];
       var aggData = resp.aggregations[firstAgg.id];
 
-      var convertKey = function (key) {
-        if (firstAgg.params.field.format) {
-          return firstAgg.params.field.format.convert(key);
-        }
+      if (!firstAgg._next && firstAgg.schema.name === 'split') {
+        notify.error('Splitting charts without splitting slices is not supported. Pretending that we are just splitting slices.');
+      }
 
-        return key;
-      };
+      // start with splitting slices
+      if (!firstAgg._next || firstAgg.schema.name === 'segment') {
+        var split = buildSplit(firstAgg, metric, aggData);
+        split.hits = resp.hits.total;
+        split.raw = raw;
+        split.tooltipFormatter = tooltipFormatter(raw.columns);
+        return split;
+      }
 
-      // If the firstAgg is a split then we need to map
-      // the split aggregations into rows.
-      if (firstAgg.schema.name === 'split') {
-        var rows = _.map(extractBuckets(aggData), function (bucket) {
-          var agg = firstAgg._next;
-          var split = buildSplit(agg, metric, bucket[agg.id]);
-          // Since splits display labels we need to set it.
-          split.label = convertKey(bucket.key) + ': ' + firstAgg.params.field.displayName;
-          split.tooltipFormatter = tooltipFormatter(raw.columns);
-          var aggConfigResult = new AggConfigResult(firstAgg, null, null, bucket.key);
-          split.split = { aggConfig: firstAgg, aggConfigResult: aggConfigResult, key: bucket.key };
-          _.each(split.slices.children, function (child) {
-            child.aggConfigResult.$parent = aggConfigResult;
-          });
-          return split;
+      // map the split aggregations into rows.
+      var rows = _.map(extractBuckets(aggData), function (bucket) {
+        var agg = firstAgg._next;
+        var split = buildSplit(agg, metric, bucket[agg.id]);
+        // Since splits display labels we need to set it.
+        split.label = firstAgg.fieldFormatter()(bucket.key) + ': ' + firstAgg.field().displayName;
+        split.tooltipFormatter = tooltipFormatter(raw.columns);
+        var aggConfigResult = new AggConfigResult(firstAgg, null, null, bucket.key);
+        split.split = { aggConfig: firstAgg, aggConfigResult: aggConfigResult, key: bucket.key };
+        _.each(split.slices.children, function (child) {
+          child.aggConfigResult.$parent = aggConfigResult;
         });
-        var result = { hits: resp.hits.total, raw: raw };
-        if (firstAgg.params.row) {
-          result.rows = rows;
-        } else {
-          result.columns = rows;
-        }
-        return result;
-        // otherwise we can start at the first bucket.
+        return split;
+      });
+
+      var result = { hits: resp.hits.total, raw: raw };
+      if (firstAgg.params.row) {
+        result.rows = rows;
       } else {
-        return (function () {
-          var split = buildSplit(firstAgg, metric, aggData);
-          split.hits = resp.hits.total;
-          split.raw = raw;
-          split.tooltipFormatter = tooltipFormatter(raw.columns);
-          return split;
-        })();
+        result.columns = rows;
       }
 
+      return result;
     };
   };
 });

test/unit/specs/components/agg_response/hierarchical/build_hierarchical_data.js

@@ -1,16 +1,22 @@
 define(function (require) {
   var _ = require('lodash');
   var fixtures = require('fixtures/fake_hierarchical_data');
+  var sinon = require('test_utils/auto_release_sinon');
 
-  var AggConfigs;
   var Vis;
+  var Notifier;
+  var AggConfigs;
   var indexPattern;
   var buildHierarchicalData;
 
   describe('buildHierarchicalData()', function () {
 
     beforeEach(module('kibana'));
-    beforeEach(inject(function (Private) {
+    beforeEach(inject(function (Private, $injector) {
+      // stub the error method before requiring vis causes Notifier#error to be bound
+      Notifier = $injector.get('Notifier');
+      sinon.stub(Notifier.prototype, 'error');
+
       Vis = Private(require('components/vis/vis'));
       AggConfigs = Private(require('components/vis/_agg_configs'));
       indexPattern = Private(require('fixtures/stubbed_logstash_index_pattern'));
@@ -232,6 +238,41 @@ define(function (require) {
 
     });
 
+    describe('oneFilterBucket that is a split', function () {
+      var vis, results;
+
+      beforeEach(function () {
+        var 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 () {
+        var errCall = Notifier.prototype.error.getCall(0);
+        expect(errCall).to.be.ok();
+        expect(errCall.args[0]).to.contain('not supported');
+
+        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');
+      });
+    });
+
   });
 });