From 197b469fe7bb04638d10c585687b2e4e43a3aebf Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Wed, 31 Jul 2019 17:45:05 +0300 Subject: [PATCH 1/5] Fix issue with Rollup --- .../default/components/default_editor_agg.tsx | 6 +-- .../components/default_editor_agg_params.tsx | 52 ++++++++++--------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx index 7d6bc34ac06c9..f3dc003da857a 100644 --- a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx @@ -63,6 +63,8 @@ function DefaultEditorAgg({ setValidity, }: DefaultEditorAggProps) { const [isEditorOpen, setIsEditorOpen] = useState(agg.brandNew); + // A description of the aggregation, for displaying in the collapsed agg header + const [aggDescription, setAggDescription] = useState(''); const [validState, setValidState] = useState(true); const showDescription = !isEditorOpen && validState; const showError = !isEditorOpen && !validState; @@ -98,12 +100,10 @@ function DefaultEditorAgg({ } }, [lastParentPipelineAggTitle, isLastBucket, agg.type]); - // A description of the aggregation, for displaying in the collapsed agg header - const aggDescription = agg.type && agg.type.makeLabel ? agg.type.makeLabel(agg) : ''; - const onToggle = (isOpen: boolean) => { setIsEditorOpen(isOpen); if (!isOpen) { + setAggDescription(agg.type && agg.type.makeLabel ? agg.type.makeLabel(agg) : ''); setTouched(true); } }; diff --git a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx index ecd6c82f26d6b..ac7622261e1a9 100644 --- a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx @@ -119,35 +119,39 @@ function DefaultEditorAggParams({ // reset validity before component destroyed useUnmount(() => setValidity(true)); + const hasConfig = !!Object.keys(editorConfig).length; + useEffect(() => { - Object.entries(editorConfig).forEach(([param, paramConfig]) => { - const paramOptions = agg.type.params.find( - (paramOption: AggParam) => paramOption.name === param - ); + if (hasConfig) { + Object.entries(editorConfig).forEach(([param, paramConfig]) => { + const paramOptions = agg.type.params.find( + (paramOption: AggParam) => paramOption.name === param + ); - const hasFixedValue = paramConfig.hasOwnProperty(FIXED_VALUE_PROP); - const hasDefault = paramConfig.hasOwnProperty(DEFAULT_PROP); - // If the parameter has a fixed value in the config, set this value. - // Also for all supported configs we should freeze the editor for this param. - if (hasFixedValue || hasDefault) { - let newValue; - let property = FIXED_VALUE_PROP; - let typedParamConfig: EditorParamConfigType = paramConfig as FixedParam; + const hasFixedValue = paramConfig.hasOwnProperty(FIXED_VALUE_PROP); + const hasDefault = paramConfig.hasOwnProperty(DEFAULT_PROP); + // If the parameter has a fixed value in the config, set this value. + // Also for all supported configs we should freeze the editor for this param. + if (hasFixedValue || hasDefault) { + let newValue; + let property = FIXED_VALUE_PROP; + let typedParamConfig: EditorParamConfigType = paramConfig as FixedParam; - if (hasDefault) { - property = DEFAULT_PROP; - typedParamConfig = paramConfig as TimeIntervalParam; - } + if (hasDefault) { + property = DEFAULT_PROP; + typedParamConfig = paramConfig as TimeIntervalParam; + } - if (paramOptions && paramOptions.deserialize) { - newValue = paramOptions.deserialize(typedParamConfig[property]); - } else { - newValue = typedParamConfig[property]; + if (paramOptions && paramOptions.deserialize) { + newValue = paramOptions.deserialize(typedParamConfig[property]); + } else { + newValue = typedParamConfig[property]; + } + onAggParamsChange(agg.params, param, newValue); } - onAggParamsChange(agg.params, param, newValue); - } - }); - }, [agg.type]); + }); + } + }, [agg.type, hasConfig]); useEffect(() => { setTouched(false); From bbc8fc40663c8fd564fda567e60ea668a09fb5ed Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Mon, 5 Aug 2019 10:13:55 +0300 Subject: [PATCH 2/5] Use useMemo for editorConfig --- .../components/default_editor_agg_params.tsx | 65 +++++++++---------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx index ac7622261e1a9..da146043510f2 100644 --- a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg_params.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useReducer, useEffect } from 'react'; +import React, { useReducer, useEffect, useMemo } from 'react'; import { EuiForm, EuiAccordion, EuiSpacer, EuiFormRow } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -94,10 +94,9 @@ function DefaultEditorAggParams({ const groupedAggTypeOptions = getAggTypeOptions(agg, indexPattern, groupName); const errors = getError(agg, aggIsTooLow); - const editorConfig = editorConfigProviders.getConfigForAgg( - aggTypes.byType[groupName], - indexPattern, - agg + const editorConfig = useMemo( + () => editorConfigProviders.getConfigForAgg(aggTypes.byType[groupName], indexPattern, agg), + [groupName, agg] ); const params = getAggParamsToRender({ agg, editorConfig, metricAggs, state }); const allParams = [...params.basic, ...params.advanced]; @@ -119,39 +118,35 @@ function DefaultEditorAggParams({ // reset validity before component destroyed useUnmount(() => setValidity(true)); - const hasConfig = !!Object.keys(editorConfig).length; - useEffect(() => { - if (hasConfig) { - Object.entries(editorConfig).forEach(([param, paramConfig]) => { - const paramOptions = agg.type.params.find( - (paramOption: AggParam) => paramOption.name === param - ); - - const hasFixedValue = paramConfig.hasOwnProperty(FIXED_VALUE_PROP); - const hasDefault = paramConfig.hasOwnProperty(DEFAULT_PROP); - // If the parameter has a fixed value in the config, set this value. - // Also for all supported configs we should freeze the editor for this param. - if (hasFixedValue || hasDefault) { - let newValue; - let property = FIXED_VALUE_PROP; - let typedParamConfig: EditorParamConfigType = paramConfig as FixedParam; - - if (hasDefault) { - property = DEFAULT_PROP; - typedParamConfig = paramConfig as TimeIntervalParam; - } + Object.entries(editorConfig).forEach(([param, paramConfig]) => { + const paramOptions = agg.type.params.find( + (paramOption: AggParam) => paramOption.name === param + ); + + const hasFixedValue = paramConfig.hasOwnProperty(FIXED_VALUE_PROP); + const hasDefault = paramConfig.hasOwnProperty(DEFAULT_PROP); + // If the parameter has a fixed value in the config, set this value. + // Also for all supported configs we should freeze the editor for this param. + if (hasFixedValue || hasDefault) { + let newValue; + let property = FIXED_VALUE_PROP; + let typedParamConfig: EditorParamConfigType = paramConfig as FixedParam; + + if (hasDefault) { + property = DEFAULT_PROP; + typedParamConfig = paramConfig as TimeIntervalParam; + } - if (paramOptions && paramOptions.deserialize) { - newValue = paramOptions.deserialize(typedParamConfig[property]); - } else { - newValue = typedParamConfig[property]; - } - onAggParamsChange(agg.params, param, newValue); + if (paramOptions && paramOptions.deserialize) { + newValue = paramOptions.deserialize(typedParamConfig[property]); + } else { + newValue = typedParamConfig[property]; } - }); - } - }, [agg.type, hasConfig]); + onAggParamsChange(agg.params, param, newValue); + } + }); + }, [agg.type, editorConfig]); useEffect(() => { setTouched(false); From 47f84504f5b0ab64099b84619498d157c98ab2fc Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Mon, 5 Aug 2019 10:14:25 +0300 Subject: [PATCH 3/5] Wrap makeLabel with try catch --- .../default/components/default_editor_agg.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx index 0c08933d447e8..9e3fca4a93fa5 100644 --- a/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/default_editor_agg.tsx @@ -63,8 +63,6 @@ function DefaultEditorAgg({ setValidity, }: DefaultEditorAggProps) { const [isEditorOpen, setIsEditorOpen] = useState(agg.brandNew); - // A description of the aggregation, for displaying in the collapsed agg header - const [aggDescription, setAggDescription] = useState(''); const [validState, setValidState] = useState(true); const showDescription = !isEditorOpen && validState; const showError = !isEditorOpen && !validState; @@ -88,6 +86,19 @@ function DefaultEditorAgg({ } } + // A description of the aggregation, for displaying in the collapsed agg header + let aggDescription = ''; + + if (agg.type && agg.type.makeLabel) { + try { + aggDescription = agg.type.makeLabel(agg); + } catch (e) { + // Date Histogram's `makeLabel` implementation invokes 'write' method for each param, including FieldParamType's 'write', + // which throws an error when field isn't defined. + aggDescription = ''; + } + } + useEffect(() => { if (isLastBucketAgg && ['date_histogram', 'histogram'].includes(agg.type.name)) { onAggParamsChange( @@ -103,7 +114,6 @@ function DefaultEditorAgg({ const onToggle = (isOpen: boolean) => { setIsEditorOpen(isOpen); if (!isOpen) { - setAggDescription(agg.type && agg.type.makeLabel ? agg.type.makeLabel(agg) : ''); setTouched(true); } }; From e8491634821bbbb242dd982d43bfdbd97d9dee3e Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Fri, 9 Aug 2019 14:13:41 +0300 Subject: [PATCH 4/5] Revert fetch to fetchForWildcard --- .../public/controls/point_series/series.js | 4 +++- .../public/controls/point_series/value_axes.js | 4 +++- .../components/step_time_field/step_time_field.js | 2 +- src/legacy/ui/public/vis/editors/default/components/agg.tsx | 4 ++-- .../ui/public/vis/editors/default/components/agg_params.tsx | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/series.js b/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/series.js index 5a98e63d2c79f..3e52df4a4d5c1 100644 --- a/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/series.js +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/series.js @@ -20,6 +20,8 @@ import _ from 'lodash'; import { uiModules } from 'ui/modules'; import vislibSeriesTemplate from './series.html'; +import { safeMakeLabel } from 'ui/agg_types/agg_utils'; + const module = uiModules.get('kibana'); module.directive('vislibSeries', function () { @@ -49,7 +51,7 @@ module.directive('vislibSeries', function () { $scope.series = $scope.editorState.params.seriesParams; $scope.$watch(() => { return $scope.editorState.aggs.map(agg => { - return agg.makeLabel(); + return safeMakeLabel(agg); }).join(); }, () => { const schemaTitle = $scope.vis.type.schemas.metrics[0].title; diff --git a/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/value_axes.js b/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/value_axes.js index e9f839da4ad37..252e9b8a6fa50 100644 --- a/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/value_axes.js +++ b/src/legacy/core_plugins/kbn_vislib_vis_types/public/controls/point_series/value_axes.js @@ -20,6 +20,8 @@ import _ from 'lodash'; import { uiModules } from 'ui/modules'; import vislibValueAxesTemplate from './value_axes.html'; +import { safeMakeLabel } from 'ui/agg_types/agg_utils'; + const module = uiModules.get('kibana'); module.directive('vislibValueAxes', function () { @@ -193,7 +195,7 @@ module.directive('vislibValueAxes', function () { $scope.$watch(() => { return $scope.editorState.aggs.map(agg => { - return agg.makeLabel(); + return safeMakeLabel(agg); }).join(); }, () => { $scope.updateAxisTitle(); diff --git a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/create_index_pattern_wizard/components/step_time_field/step_time_field.js b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/create_index_pattern_wizard/components/step_time_field/step_time_field.js index 4476ad868cbcf..37591d484d201 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/create_index_pattern_wizard/components/step_time_field/step_time_field.js +++ b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/create_index_pattern_wizard/components/step_time_field/step_time_field.js @@ -89,7 +89,7 @@ export class StepTimeFieldComponent extends Component { this.setState({ isFetchingTimeFields: true }); const fields = await ensureMinimumTime( - indexPattern.fieldsFetcher.fetch(getFetchForWildcardOptions()) + indexPattern.fieldsFetcher.fetchForWildcard(pattern, getFetchForWildcardOptions()) ); const timeFields = extractTimeFields(fields); diff --git a/src/legacy/ui/public/vis/editors/default/components/agg.tsx b/src/legacy/ui/public/vis/editors/default/components/agg.tsx index 910e7e6f1225d..d29e48e269e59 100644 --- a/src/legacy/ui/public/vis/editors/default/components/agg.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/agg.tsx @@ -93,8 +93,8 @@ function DefaultEditorAgg({ try { aggDescription = agg.type.makeLabel(agg); } catch (e) { - // Date Histogram's `makeLabel` implementation invokes 'write' method for each param, including FieldParamType's 'write', - // which throws an error when field isn't defined. + // Date Histogram's `makeLabel` implementation invokes 'write' method for each param, including interval's 'write', + // which throws an error when interval is undefined. aggDescription = ''; } } diff --git a/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx b/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx index bd951db5bf24f..daca8ba28b424 100644 --- a/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx @@ -96,7 +96,7 @@ function DefaultEditorAggParams({ const editorConfig = useMemo( () => editorConfigProviders.getConfigForAgg(aggTypes.byType[groupName], indexPattern, agg), - [groupName, agg] + [groupName, agg.type] ); const params = getAggParamsToRender({ agg, editorConfig, metricAggs, state }); const allParams = [...params.basic, ...params.advanced]; From 3780e68ec256beb78802b8a420791550fb354566 Mon Sep 17 00:00:00 2001 From: maryia-lapata Date: Mon, 12 Aug 2019 10:00:55 +0300 Subject: [PATCH 5/5] Update useEffect dependencies --- .../ui/public/vis/editors/default/components/agg_params.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx b/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx index daca8ba28b424..2a8e7f9002a22 100644 --- a/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx +++ b/src/legacy/ui/public/vis/editors/default/components/agg_params.tsx @@ -146,7 +146,7 @@ function DefaultEditorAggParams({ onAggParamsChange(agg.params, param, newValue); } }); - }, [agg.type, editorConfig]); + }, [editorConfig]); useEffect(() => { setTouched(false);