From debec1eb27f5e680e4f8f5eade13b5eef463fec2 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 22 Sep 2020 10:22:14 -0400 Subject: [PATCH] feedback --- .../es_geo_grid_source/es_geo_grid_source.js | 36 +++++++++---------- .../resolution_editor.test.tsx | 4 +-- .../update_source_editor.js | 7 ++-- .../classes/styles/heatmap/heatmap_style.tsx | 3 +- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.js b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.js index 2152b2dd16c4e..16694d0d4fc3a 100644 --- a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.js +++ b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/es_geo_grid_source.js @@ -111,10 +111,6 @@ export class ESGeoGridSource extends AbstractESAggSource { ]; } - getMetricFields() { - return super.getMetricFields(); - } - getFieldNames() { return this.getMetricFields().map((esAggMetricField) => esAggMetricField.getName()); } @@ -139,11 +135,12 @@ export class ESGeoGridSource extends AbstractESAggSource { } getGeoGridPrecision(zoom) { - const delta = this._getGeoGridPrecisionResolutionDelta(); - if (delta === null) { + if (this._descriptor.resolution === GRID_RESOLUTION.SUPER_FINE) { + // The target-precision needs to be determined server side. return NaN; } - const targetGeotileLevel = Math.ceil(zoom) + delta; + + const targetGeotileLevel = Math.ceil(zoom) + this._getGeoGridPrecisionResolutionDelta(); return Math.min(targetGeotileLevel, MAX_GEOTILE_LEVEL); } @@ -160,11 +157,6 @@ export class ESGeoGridSource extends AbstractESAggSource { return 4; } - if (this._descriptor.resolution === GRID_RESOLUTION.SUPER_FINE) { - // The target-precision needs to be determined server side. - return null; - } - throw new Error( i18n.translate('xpack.maps.source.esGrid.resolutionParamErrorMessage', { defaultMessage: `Grid resolution param not recognized: {resolution}`, @@ -267,7 +259,9 @@ export class ESGeoGridSource extends AbstractESAggSource { _addNonCompositeAggregationsToSearchSource( searchSource, - { indexPattern, precision, bufferedExtent } + indexPattern, + precision, + bufferedExtent ) { searchSource.setField('aggs', { [GEOTILE_GRID_AGG_NAME]: { @@ -300,11 +294,12 @@ export class ESGeoGridSource extends AbstractESAggSource { registerCancelCallback, bufferedExtent, }) { - this._addNonCompositeAggregationsToSearchSource(searchSource, { + this._addNonCompositeAggregationsToSearchSource( + searchSource, indexPattern, precision, - bufferedExtent, - }); + bufferedExtent + ); const esResponse = await this._runEsQuery({ requestId: this.getId(), @@ -368,11 +363,12 @@ export class ESGeoGridSource extends AbstractESAggSource { const indexPattern = await this.getIndexPattern(); const searchSource = await this.makeSearchSource(searchFilters, 0); - this._addNonCompositeAggregationsToSearchSource(searchSource, { + this._addNonCompositeAggregationsToSearchSource( + searchSource, indexPattern, - precision: -1, // This needs to be set server-side - bufferedExtent: null, //this needs to be stripped server-side - }); + null, // needs to be set server-side + null // needs to be stripped server-side + ); const dsl = await searchSource.getSearchRequestBody(); diff --git a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/resolution_editor.test.tsx b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/resolution_editor.test.tsx index fe4b215912955..369203dbe16c0 100644 --- a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/resolution_editor.test.tsx +++ b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/resolution_editor.test.tsx @@ -18,11 +18,11 @@ const defaultProps = { }; describe('resolution editor', () => { - test('should omit super-fine option', async () => { + test('should omit super-fine option', () => { const component = shallow(); expect(component).toMatchSnapshot(); }); - test('should add super-fine option', async () => { + test('should add super-fine option', () => { const component = shallow(); expect(component).toMatchSnapshot(); }); diff --git a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/update_source_editor.js b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/update_source_editor.js index 9a3b747906c5b..daf19d2a6f0bd 100644 --- a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/update_source_editor.js +++ b/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/update_source_editor.js @@ -68,11 +68,8 @@ export class UpdateSourceEditor extends Component { this.props.currentLayerType === LAYER_TYPE.VECTOR || this.props.currentLayerType === LAYER_TYPE.TILED_VECTOR ) { - if (resolution === GRID_RESOLUTION.SUPER_FINE) { - newLayerType = LAYER_TYPE.TILED_VECTOR; - } else { - newLayerType = LAYER_TYPE.VECTOR; - } + newLayerType = + resolution === GRID_RESOLUTION.SUPER_FINE ? LAYER_TYPE.TILED_VECTOR : LAYER_TYPE.VECTOR; } else if (this.props.currentLayerType === LAYER_TYPE.HEATMAP) { if (resolution === GRID_RESOLUTION.SUPER_FINE) { throw new Error('Heatmap does not support SUPER_FINE resolution'); diff --git a/x-pack/plugins/maps/public/classes/styles/heatmap/heatmap_style.tsx b/x-pack/plugins/maps/public/classes/styles/heatmap/heatmap_style.tsx index 915e9a8a9b57d..c75698805225f 100644 --- a/x-pack/plugins/maps/public/classes/styles/heatmap/heatmap_style.tsx +++ b/x-pack/plugins/maps/public/classes/styles/heatmap/heatmap_style.tsx @@ -85,9 +85,8 @@ export class HeatmapStyle implements IStyle { radius = 64; } else if (resolution === GRID_RESOLUTION.MOST_FINE) { radius = 32; - } else if (resolution === GRID_RESOLUTION.SUPER_FINE) { - radius = 8; } else { + // SUPER_FINE or any other is not supported. const errorMessage = i18n.translate('xpack.maps.style.heatmap.resolutionStyleErrorMessage', { defaultMessage: `Resolution param not recognized: {resolution}`, values: { resolution },