From f85570d506e29f86bb635ee648dc4190901be948 Mon Sep 17 00:00:00 2001 From: Spencer Date: Thu, 18 May 2017 18:31:35 -0700 Subject: [PATCH 1/4] [fieldFormat] make formatted output non-bindable (#11911) * [fieldFormat] make formatted output non-bindable * [aggTable] totals are consumed as text, produce text * [tests] fix existing tests * [fieldFormat/contentType] add tests for non-bindable wrapper (cherry picked from commit 1d2341fc2b0cce7a156dbd206395eb7fa642c041) --- src/ui/public/agg_table/agg_table.js | 4 +- .../index_patterns/__tests__/_field_format.js | 12 ++- .../_field_format/__tests__/content_types.js | 99 +++++++++++++++++++ .../_field_format/content_types.js | 6 +- src/ui/public/stringify/__tests__/_color.js | 38 ++++--- src/ui/public/stringify/__tests__/_source.js | 6 +- src/ui/public/stringify/__tests__/_url.js | 13 ++- 7 files changed, 151 insertions(+), 27 deletions(-) create mode 100644 src/ui/public/index_patterns/_field_format/__tests__/content_types.js diff --git a/src/ui/public/agg_table/agg_table.js b/src/ui/public/agg_table/agg_table.js index dfd38d17206b8..3b1a5dd7f1251 100644 --- a/src/ui/public/agg_table/agg_table.js +++ b/src/ui/public/agg_table/agg_table.js @@ -10,7 +10,7 @@ uiModules .get('kibana') .directive('kbnAggTable', function ($filter, config, Private, compileRecursiveDirective) { const fieldFormats = Private(RegistryFieldFormatsProvider); - const numberFormatter = fieldFormats.getDefaultInstance('number').getConverterFor('html'); + const numberFormatter = fieldFormats.getDefaultInstance('number').getConverterFor('text'); return { restrict: 'E', @@ -130,7 +130,7 @@ uiModules return prev + curr[i].value; }, 0); } - const formatter = agg.fieldFormatter('html'); + const formatter = agg.fieldFormatter('text'); switch ($scope.totalFunc) { case 'sum': diff --git a/src/ui/public/index_patterns/__tests__/_field_format.js b/src/ui/public/index_patterns/__tests__/_field_format.js index ca8fb8d90fcdf..69443d68e3516 100644 --- a/src/ui/public/index_patterns/__tests__/_field_format.js +++ b/src/ui/public/index_patterns/__tests__/_field_format.js @@ -2,6 +2,7 @@ import _ from 'lodash'; import expect from 'expect.js'; import ngMock from 'ng_mock'; import IndexPatternsFieldFormatFieldFormatProvider from 'ui/index_patterns/_field_format/field_format'; + describe('FieldFormat class', function () { let FieldFormat; @@ -86,7 +87,7 @@ describe('FieldFormat class', function () { const html = f.getConverterFor('html'); expect(text).to.not.be(html); expect(text('formatted')).to.be('formatted'); - expect(html('formatted')).to.be('formatted'); + expect(html('formatted')).to.be('formatted'); }); it('can be an object, with seperate text and html converter', function () { @@ -100,7 +101,7 @@ describe('FieldFormat class', function () { const html = f.getConverterFor('html'); expect(text).to.not.be(html); expect(text('formatted text')).to.be('formatted text'); - expect(html('formatted html')).to.be('formatted html'); + expect(html('formatted html')).to.be('formatted html'); }); it('does not escape the output of the text converter', function () { @@ -112,7 +113,8 @@ describe('FieldFormat class', function () { it('does escape the output of the text converter if used in an html context', function () { TestFormat.prototype._convert = _.constant(''); const f = new TestFormat(); - expect(f.convert('', 'html')).to.not.contain('<'); + expect(_.trimRight(_.trimLeft(f.convert('', 'html'), ''), '')) + .to.not.contain('<'); }); it('does not escape the output of an html specific converter', function () { @@ -123,7 +125,7 @@ describe('FieldFormat class', function () { const f = new TestFormat(); expect(f.convert('', 'text')).to.be(''); - expect(f.convert('', 'html')).to.be(''); + expect(f.convert('', 'html')).to.be(''); }); }); @@ -145,7 +147,7 @@ describe('FieldFormat class', function () { }; const f = new TestFormat(); - expect(f.convert('val', 'html')).to.be('html'); + expect(f.convert('val', 'html')).to.be('html'); }); it('formats a value as " - " when no value is specified', function () { diff --git a/src/ui/public/index_patterns/_field_format/__tests__/content_types.js b/src/ui/public/index_patterns/_field_format/__tests__/content_types.js new file mode 100644 index 0000000000000..8fdb5acfdd485 --- /dev/null +++ b/src/ui/public/index_patterns/_field_format/__tests__/content_types.js @@ -0,0 +1,99 @@ +import angular from 'angular'; +import $ from 'jquery'; +import ngMock from 'ng_mock'; +import sinon from 'sinon'; +import expect from 'expect.js'; +import { escape } from 'lodash'; + +import { IndexPatternsFieldFormatContentTypesProvider } from '../content_types'; + +describe('index_patterns/_field_format/content_types', () => { + + let render; + const callMe = sinon.stub(); + afterEach(() => callMe.reset()); + + function getAllContents(node) { + return [...node.childNodes].reduce((acc, child) => { + return acc.concat(child, getAllContents(child)); + }, []); + } + + angular.module('testApp', []) + .directive('testDirective', () => ({ + restrict: 'EACM', + link: callMe + })); + + beforeEach(ngMock.module('testApp')); + beforeEach(ngMock.inject(($injector) => { + const contentTypes = new IndexPatternsFieldFormatContentTypesProvider(); + const $rootScope = $injector.get('$rootScope'); + const $compile = $injector.get('$compile'); + + $rootScope.callMe = callMe; + + render = (convert) => { + const $el = $('
'); + const { html } = contentTypes.setup({ _convert: { html: convert } }); + $compile($el.html(html(` + +
+ {{callMe()}} + + + `)))($rootScope); + return $el; + }; + })); + + it('no element directive', () => { + const $el = render(value => ` + ${escape(value)} + `); + + expect($el.find('test-directive')).to.have.length(1); + sinon.assert.notCalled(callMe); + }); + + it('no attribute directive', () => { + const $el = render(value => ` +
${escape(value)}
+ `); + + expect($el.find('[test-directive]')).to.have.length(1); + sinon.assert.notCalled(callMe); + }); + + it('no comment directive', () => { + const $el = render(value => ` + +
${escape(value)}
+ `); + + const comments = getAllContents($el.get(0)) + .filter(node => node.nodeType === 8); + + expect(comments).to.have.length(1); + expect(comments[0].textContent).to.contain('test-directive'); + sinon.assert.notCalled(callMe); + }); + + it('no class directive', () => { + const $el = render(value => ` +
${escape(value)}
+ `); + + expect($el.find('.test-directive')).to.have.length(1); + sinon.assert.notCalled(callMe); + }); + + it('no interpolation', () => { + const $el = render(value => ` +
${escape(value)}
+ `); + + expect($el.find('.foo')).to.have.length(1); + sinon.assert.notCalled(callMe); + }); +}); diff --git a/src/ui/public/index_patterns/_field_format/content_types.js b/src/ui/public/index_patterns/_field_format/content_types.js index 4da552e024a1a..55203de053d3e 100644 --- a/src/ui/public/index_patterns/_field_format/content_types.js +++ b/src/ui/public/index_patterns/_field_format/content_types.js @@ -5,7 +5,7 @@ export default function contentTypesProvider() { const types = { html: function (format, convert) { - return function recurse(value, field, hit) { + function recurse(value, field, hit) { if (value == null) { return _.asPrettyString(value); } @@ -22,6 +22,10 @@ export default function contentTypesProvider() { }); return subVals.join(',' + (useMultiLine ? '\n' : ' ')); + } + + return function (...args) { + return `${recurse(...args)}`; }; }, diff --git a/src/ui/public/stringify/__tests__/_color.js b/src/ui/public/stringify/__tests__/_color.js index c918155b3164c..d82f3c1e59a9d 100644 --- a/src/ui/public/stringify/__tests__/_color.js +++ b/src/ui/public/stringify/__tests__/_color.js @@ -21,10 +21,14 @@ describe('Color Format', function () { background: 'yellow' }] }); - expect(colorer.convert(99, 'html')).to.eql('99'); - expect(colorer.convert(100, 'html')).to.eql('100'); - expect(colorer.convert(150, 'html')).to.eql('150'); - expect(colorer.convert(151, 'html')).to.eql('151'); + expect(colorer.convert(99, 'html')).to.eql('99'); + expect(colorer.convert(100, 'html')).to.eql( + '100' + ); + expect(colorer.convert(150, 'html')).to.eql( + '150' + ); + expect(colorer.convert(151, 'html')).to.eql('151'); }); it('should not convert invalid ranges', function () { @@ -36,7 +40,7 @@ describe('Color Format', function () { background: 'yellow' }] }); - expect(colorer.convert(99, 'html')).to.eql('99'); + expect(colorer.convert(99, 'html')).to.eql('99'); }); }); @@ -52,15 +56,23 @@ describe('Color Format', function () { }); const converter = colorer.getConverterFor('html'); - expect(converter('B', 'html')).to.eql('B'); - expect(converter('AAA', 'html')).to.eql('AAA'); - expect(converter('AB', 'html')).to.eql('AB'); - expect(converter('a', 'html')).to.eql('a'); + expect(converter('B', 'html')).to.eql('B'); + expect(converter('AAA', 'html')).to.eql( + 'AAA' + ); + expect(converter('AB', 'html')).to.eql( + 'AB' + ); + expect(converter('a', 'html')).to.eql('a'); - expect(converter('B', 'html')).to.eql('B'); - expect(converter('AAA', 'html')).to.eql('AAA'); - expect(converter('AB', 'html')).to.eql('AB'); - expect(converter('a', 'html')).to.eql('a'); + expect(converter('B', 'html')).to.eql('B'); + expect(converter('AAA', 'html')).to.eql( + 'AAA' + ); + expect(converter('AB', 'html')).to.eql( + 'AB' + ); + expect(converter('a', 'html')).to.eql('a'); }); }); }); diff --git a/src/ui/public/stringify/__tests__/_source.js b/src/ui/public/stringify/__tests__/_source.js index 3f734eabbe7ba..0efa0c3352409 100644 --- a/src/ui/public/stringify/__tests__/_source.js +++ b/src/ui/public/stringify/__tests__/_source.js @@ -29,12 +29,14 @@ describe('_source formatting', function () { it('should use the text content type if a field is not passed', function () { const hit = _.first(hits); - expect(convertHtml(hit._source)).to.be(JSON.stringify(hit._source)); + expect(convertHtml(hit._source)).to.be(`${JSON.stringify(hit._source)}`); }); it('uses the _source, field, and hit to create a
', function () { const hit = _.first(hits); - const $dl = $(convertHtml(hit._source, indexPattern.fields.byName._source, hit)); + const $nonBindable = $(convertHtml(hit._source, indexPattern.fields.byName._source, hit)); + expect($nonBindable.is('span[ng-non-bindable]')).to.be.ok(); + const $dl = $nonBindable.children(); expect($dl.is('dl')).to.be.ok(); expect($dl.find('dt')).to.have.length(_.keys(indexPattern.flattenHit(hit)).length); }); diff --git a/src/ui/public/stringify/__tests__/_url.js b/src/ui/public/stringify/__tests__/_url.js index d7dafd717fa88..d3a32773dfa80 100644 --- a/src/ui/public/stringify/__tests__/_url.js +++ b/src/ui/public/stringify/__tests__/_url.js @@ -11,6 +11,11 @@ describe('Url Format', function () { fieldFormats = Private(RegistryFieldFormatsProvider); })); + const unwrap = $el => { + expect($el.is('span[ng-non-bindable]')).to.be.ok(); + return $el.children(); + }; + describe('Url Format', function () { let Url; @@ -21,7 +26,7 @@ describe('Url Format', function () { it('ouputs a simple tab by default', function () { const url = new Url(); - const $a = $(url.convert('http://elastic.co', 'html')); + const $a = unwrap($(url.convert('http://elastic.co', 'html'))); expect($a.is('a')).to.be(true); expect($a.size()).to.be(1); expect($a.attr('href')).to.be('http://elastic.co'); @@ -32,7 +37,7 @@ describe('Url Format', function () { it('outputs an if type === "img"', function () { const url = new Url({ type: 'img' }); - const $img = $(url.convert('http://elastic.co', 'html')); + const $img = unwrap($(url.convert('http://elastic.co', 'html'))); expect($img.is('img')).to.be(true); expect($img.attr('src')).to.be('http://elastic.co'); }); @@ -40,7 +45,7 @@ describe('Url Format', function () { describe('url template', function () { it('accepts a template', function () { const url = new Url({ urlTemplate: 'url: {{ value }}' }); - const $a = $(url.convert('url', 'html')); + const $a = unwrap($(url.convert('url', 'html'))); expect($a.is('a')).to.be(true); expect($a.size()).to.be(1); expect($a.attr('href')).to.be('url: url'); @@ -57,7 +62,7 @@ describe('Url Format', function () { describe('label template', function () { it('accepts a template', function () { const url = new Url({ labelTemplate: 'extension: {{ value }}' }); - const $a = $(url.convert('php', 'html')); + const $a = unwrap($(url.convert('php', 'html'))); expect($a.is('a')).to.be(true); expect($a.size()).to.be(1); expect($a.attr('href')).to.be('php'); From 1cceeae9ce7e572cc580ee96e966ac5ca1aa89e4 Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 18 May 2017 19:20:09 -0700 Subject: [PATCH 2/4] fix import statement --- .../index_patterns/_field_format/__tests__/content_types.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/index_patterns/_field_format/__tests__/content_types.js b/src/ui/public/index_patterns/_field_format/__tests__/content_types.js index 8fdb5acfdd485..609d22abfd4ff 100644 --- a/src/ui/public/index_patterns/_field_format/__tests__/content_types.js +++ b/src/ui/public/index_patterns/_field_format/__tests__/content_types.js @@ -5,7 +5,7 @@ import sinon from 'sinon'; import expect from 'expect.js'; import { escape } from 'lodash'; -import { IndexPatternsFieldFormatContentTypesProvider } from '../content_types'; +import IndexPatternsFieldFormatContentTypesProvider from '../content_types'; describe('index_patterns/_field_format/content_types', () => { From eea76221bae80def20282f8a0d63f32b31151b1d Mon Sep 17 00:00:00 2001 From: Spencer Date: Fri, 12 May 2017 01:46:20 -0700 Subject: [PATCH 3/4] [metricVis] Fix html support (#11008) * [metricVis] Fix html support * [metric_vis] add test to verify 42d11b021 * [ui/vis] fix import (cherry picked from commit 34c33f389553569ef8d7889cfd584e41277e9381) --- .../metric_vis/public/__tests__/metric_vis.js | 60 +++++++++++++++++++ .../metric_vis/public/metric_vis.html | 2 +- src/test_utils/stub_index_pattern.js | 24 ++++---- .../agg_types/__tests__/buckets/_range.js | 9 +-- .../field_editor/__tests__/field_editor.js | 8 +-- 5 files changed, 76 insertions(+), 27 deletions(-) create mode 100644 src/core_plugins/metric_vis/public/__tests__/metric_vis.js diff --git a/src/core_plugins/metric_vis/public/__tests__/metric_vis.js b/src/core_plugins/metric_vis/public/__tests__/metric_vis.js new file mode 100644 index 0000000000000..ce24438777efa --- /dev/null +++ b/src/core_plugins/metric_vis/public/__tests__/metric_vis.js @@ -0,0 +1,60 @@ +import $ from 'jquery'; +import ngMock from 'ng_mock'; +import expect from 'expect.js'; + +import { VisProvider } from 'ui/vis'; +import LogstashIndexPatternStubProvider from 'fixtures/stubbed_logstash_index_pattern'; +import MetricVisProvider from '../metric_vis'; + +describe('metric_vis', () => { + let setup = null; + + beforeEach(ngMock.module('kibana')); + beforeEach(ngMock.inject((Private, $rootScope) => { + setup = () => { + const Vis = Private(VisProvider); + const metricVisType = Private(MetricVisProvider); + const indexPattern = Private(LogstashIndexPatternStubProvider); + + indexPattern.stubSetFieldFormat('ip', 'url', { + urlTemplate: 'http://ip.info?address={{value}}', + labelTemplate: 'ip[{{value}}]' + }); + + const vis = new Vis(indexPattern, { + type: 'metric', + aggs: [{ id: '1', type: 'top_hits', schema: 'metric', params: { field: 'ip' } }], + }); + + const $el = $('
'); + const renderbot = metricVisType.createRenderbot(vis, $el); + const render = (esResponse) => { + renderbot.render(esResponse); + $rootScope.$digest(); + }; + + return { $el, render }; + }; + })); + + it('renders html value from field formatter', () => { + const { $el, render } = setup(); + + const ip = '235.195.237.208'; + render({ + hits: { total: 0, hits: [] }, + aggregations: { + '1': { + hits: { total: 1, hits: [{ _source: { ip } }] } + } + } + }); + + const $link = $el + .find('a[href]') + .filter(function () { return this.href.includes('ip.info'); }); + + expect($link).to.have.length(1); + expect($link.text()).to.be(`ip[${ip}]`); + }); +}); diff --git a/src/core_plugins/metric_vis/public/metric_vis.html b/src/core_plugins/metric_vis/public/metric_vis.html index c1d9150b679a2..42bc974a73ec5 100644 --- a/src/core_plugins/metric_vis/public/metric_vis.html +++ b/src/core_plugins/metric_vis/public/metric_vis.html @@ -1,6 +1,6 @@
-
{{metric.value}}
+
{{metric.label}}
diff --git a/src/test_utils/stub_index_pattern.js b/src/test_utils/stub_index_pattern.js index c14a08a30c50a..d8c1ca91e3f9e 100644 --- a/src/test_utils/stub_index_pattern.js +++ b/src/test_utils/stub_index_pattern.js @@ -1,19 +1,17 @@ import _ from 'lodash'; import sinon from 'sinon'; import Promise from 'bluebird'; -import IndexedArray from 'ui/indexed_array'; import IndexPattern from 'ui/index_patterns/_index_pattern'; import formatHit from 'ui/index_patterns/_format_hit'; import getComputedFields from 'ui/index_patterns/_get_computed_fields'; import RegistryFieldFormatsProvider from 'ui/registry/field_formats'; import IndexPatternsFlattenHitProvider from 'ui/index_patterns/_flatten_hit'; -import IndexPatternsFieldProvider from 'ui/index_patterns/_field'; +import IndexPatternsFieldListProvider from 'ui/index_patterns/_field_list'; export default function (Private) { const fieldFormats = Private(RegistryFieldFormatsProvider); const flattenHit = Private(IndexPatternsFlattenHitProvider); - - const Field = Private(IndexPatternsFieldProvider); + const FieldList = Private(IndexPatternsFieldListProvider); function StubIndexPattern(pattern, timeField, fields) { this.id = pattern; @@ -39,17 +37,17 @@ export default function (Private) { this.formatHit = formatHit(this, fieldFormats.getDefaultInstance('string')); this.formatField = this.formatHit.formatField; - this._indexFields = function () { - this.fields = new IndexedArray({ - index: ['name'], - group: ['type'], - initialSet: fields.map(function (field) { - return new Field(this, field); - }, this) - }); + this._reindexFields = function () { + this.fields = new FieldList(this, this.fields || fields); + }; + + this.stubSetFieldFormat = function (fieldName, id, params) { + const FieldFormat = fieldFormats.byId[id]; + this.fieldFormatMap[fieldName] = new FieldFormat(params); + this._reindexFields(); }; - this._indexFields(); + this._reindexFields(); } return StubIndexPattern; diff --git a/src/ui/public/agg_types/__tests__/buckets/_range.js b/src/ui/public/agg_types/__tests__/buckets/_range.js index 1a9de3c62a20b..8ada0f283f123 100644 --- a/src/ui/public/agg_types/__tests__/buckets/_range.js +++ b/src/ui/public/agg_types/__tests__/buckets/_range.js @@ -4,7 +4,7 @@ import expect from 'expect.js'; import resp from 'fixtures/agg_resp/range'; import VisProvider from 'ui/vis'; import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern'; -import RegistryFieldFormatsProvider from 'ui/registry/field_formats'; + describe('Range Agg', function () { const buckets = values(resp.aggregations[1].buckets); @@ -15,14 +15,9 @@ describe('Range Agg', function () { beforeEach(ngMock.inject(function (Private) { Vis = Private(VisProvider); indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); - - const BytesFormat = Private(RegistryFieldFormatsProvider).byId.bytes; - - indexPattern.fieldFormatMap.bytes = new BytesFormat({ + indexPattern.stubSetFieldFormat('bytes', 'bytes', { pattern: '0,0.[000] b' }); - - indexPattern._indexFields(); })); describe('formating', function () { diff --git a/src/ui/public/field_editor/__tests__/field_editor.js b/src/ui/public/field_editor/__tests__/field_editor.js index 328245e4564ac..7b0ddc319e0ee 100644 --- a/src/ui/public/field_editor/__tests__/field_editor.js +++ b/src/ui/public/field_editor/__tests__/field_editor.js @@ -2,14 +2,13 @@ import $ from 'jquery'; import ngMock from 'ng_mock'; import expect from 'expect.js'; import IndexPatternsFieldProvider from 'ui/index_patterns/_field'; -import RegistryFieldFormatsProvider from 'ui/registry/field_formats'; + import FixturesStubbedLogstashIndexPatternProvider from 'fixtures/stubbed_logstash_index_pattern'; import _ from 'lodash'; describe('FieldEditor directive', function () { let Field; - let StringFormat; let $rootScope; let compile; @@ -27,12 +26,9 @@ describe('FieldEditor directive', function () { $rootScope = $injector.get('$rootScope'); Field = Private(IndexPatternsFieldProvider); - StringFormat = Private(RegistryFieldFormatsProvider).getType('string'); $rootScope.indexPattern = Private(FixturesStubbedLogstashIndexPatternProvider); - // set the field format for this field - $rootScope.indexPattern.fieldFormatMap.time = new StringFormat({ foo: 1, bar: 2 }); - $rootScope.indexPattern._indexFields(); + $rootScope.indexPattern.stubSetFieldFormat('time', 'string', { foo: 1, bar: 2 }); $rootScope.field = $rootScope.indexPattern.fields.byName.time; compile = function () { From 07d9a3b0bc50155b07d03d8365ba69e62819f72f Mon Sep 17 00:00:00 2001 From: spalger Date: Thu, 18 May 2017 21:41:46 -0700 Subject: [PATCH 4/4] fix import statement --- src/core_plugins/metric_vis/public/__tests__/metric_vis.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core_plugins/metric_vis/public/__tests__/metric_vis.js b/src/core_plugins/metric_vis/public/__tests__/metric_vis.js index ce24438777efa..c3a3f77be37c9 100644 --- a/src/core_plugins/metric_vis/public/__tests__/metric_vis.js +++ b/src/core_plugins/metric_vis/public/__tests__/metric_vis.js @@ -2,7 +2,7 @@ import $ from 'jquery'; import ngMock from 'ng_mock'; import expect from 'expect.js'; -import { VisProvider } from 'ui/vis'; +import VisProvider from 'ui/vis'; import LogstashIndexPatternStubProvider from 'fixtures/stubbed_logstash_index_pattern'; import MetricVisProvider from '../metric_vis';