From 1a4e3e8cc62d4d9ce25d68e90ec2cd323e3f0dce Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Tue, 25 May 2021 13:37:52 +0200 Subject: [PATCH] feat(core): remove defaults for time range filter and Metrics (#1114) * feat(core): remove defaults for time range filter and Metrics * Display errors on all 3 controls * Fix for raw mode * Refactor duplicated code --- .../src/shared-controls/dndControls.tsx | 13 +--- .../src/shared-controls/index.tsx | 8 +-- .../plugin-chart-table/src/controlPanel.tsx | 64 +++++++++++++++++-- 3 files changed, 60 insertions(+), 25 deletions(-) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx index 5e49652fb868..11bfacecef4a 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx @@ -17,16 +17,10 @@ * specific language governing permissions and limitations * under the License. */ -import { Metric, t, validateNonEmpty } from '@superset-ui/core'; +import { t, validateNonEmpty } from '@superset-ui/core'; import { ExtraControlProps, SharedControlConfig } from '../types'; -import { mainMetric } from '../utils'; import { TIME_COLUMN_OPTION } from '../constants'; -type Control = { - savedMetrics?: Metric[] | null; - default?: unknown; -}; - export const dndGroupByControl: SharedControlConfig<'DndColumnSelect'> = { type: 'DndColumnSelect', label: t('Group by'), @@ -93,10 +87,6 @@ export const dnd_adhoc_metrics: SharedControlConfig<'DndMetricSelect'> = { multi: true, label: t('Metrics'), validators: [validateNonEmpty], - default: (c: Control) => { - const metric = mainMetric(c.savedMetrics); - return metric ? [metric] : null; - }, mapStateToProps: ({ datasource }) => ({ columns: datasource ? datasource.columns : [], savedMetrics: datasource ? datasource.metrics : [], @@ -110,7 +100,6 @@ export const dnd_adhoc_metric: SharedControlConfig<'DndMetricSelect'> = { multi: false, label: t('Metric'), description: t('Metric'), - default: (c: Control) => mainMetric(c.savedMetrics), }; export const dnd_sort_by: SharedControlConfig<'DndMetricSelect'> = { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx index 1dfd03ce949c..c301600c550b 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-chart-controls/src/shared-controls/index.tsx @@ -46,7 +46,6 @@ import { } from '@superset-ui/core'; import { - mainMetric, formatSelectOptions, D3_FORMAT_OPTIONS, D3_FORMAT_DOCS, @@ -130,10 +129,6 @@ const metrics: SharedControlConfig<'MetricsControl'> = { multi: true, label: t('Metrics'), validators: [validateNonEmpty], - default: (c: Control) => { - const metric = mainMetric(c.savedMetrics); - return metric ? [metric] : null; - }, mapStateToProps: ({ datasource }) => ({ columns: datasource ? datasource.columns : [], savedMetrics: datasource ? datasource.metrics : [], @@ -147,7 +142,6 @@ const metric: SharedControlConfig<'MetricsControl'> = { multi: false, label: t('Metric'), description: t('Metric'), - default: (c: Control) => mainMetric(c.savedMetrics), }; const datasourceControl: SharedControlConfig<'DatasourceControl'> = { @@ -304,7 +298,7 @@ const time_range: SharedControlConfig<'DateFilterControl'> = { type: 'DateFilterControl', freeForm: true, label: TIME_FILTER_LABELS.time_range, - default: t('Last week'), // this value is translated, but the backend wouldn't understand a translated value? + default: t('No filter'), // this value is translated, but the backend wouldn't understand a translated value? description: t( 'The time range for the visualization. All relative times, e.g. "Last month", ' + '"Last 7 days", "now", etc. are evaluated on the server using the server\'s ' + diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/controlPanel.tsx b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/controlPanel.tsx index 1a2b6d09f177..2aa354ba52fd 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-table/src/controlPanel.tsx @@ -36,10 +36,12 @@ import { ControlPanelsContainerProps, ControlStateMapping, D3_TIME_FORMAT_OPTIONS, - ExtraControlProps, QueryModeLabel, sections, sharedControls, + ControlPanelState, + ExtraControlProps, + ControlState, } from '@superset-ui/chart-controls'; import i18n from './i18n'; @@ -68,6 +70,13 @@ function isQueryMode(mode: QueryMode) { const isAggMode = isQueryMode(QueryMode.aggregate); const isRawMode = isQueryMode(QueryMode.raw); +const validateAggControlValues = (controls: ControlStateMapping, values: any[]) => { + const areControlsEmpty = values.every(val => ensureIsArray(val).length === 0); + return areControlsEmpty && isAggMode({ controls }) + ? [t('Group By, Metrics or Percentage Metrics must have a value')] + : []; +}; + const queryMode: ControlConfig<'RadioButtonControl'> = { type: 'RadioButtonControl', label: t('Query mode'), @@ -77,6 +86,7 @@ const queryMode: ControlConfig<'RadioButtonControl'> = { [QueryMode.raw, QueryModeLabel[QueryMode.raw]], ], mapStateToProps: ({ controls }) => ({ value: getQueryMode(controls) }), + rerender: ['all_columns', 'groupby', 'metrics', 'percent_metrics'], }; const all_columns: typeof sharedControls.groupby = { @@ -91,9 +101,13 @@ const all_columns: typeof sharedControls.groupby = { optionRenderer: c => , valueRenderer: c => , valueKey: 'column_name', - mapStateToProps: ({ datasource, controls }) => ({ + mapStateToProps: ({ datasource, controls }, controlState) => ({ options: datasource?.columns || [], queryMode: getQueryMode(controls), + externalValidationErrors: + isRawMode({ controls }) && ensureIsArray(controlState.value).length === 0 + ? [t('must have a value')] + : [], }), visibility: isRawMode, }; @@ -127,12 +141,18 @@ const percent_metrics: typeof sharedControls.metrics = { ), multi: true, visibility: isAggMode, - mapStateToProps: ({ datasource, controls }) => ({ + mapStateToProps: ({ datasource, controls }, controlState) => ({ columns: datasource?.columns || [], savedMetrics: datasource?.metrics || [], datasourceType: datasource?.type, queryMode: getQueryMode(controls), + externalValidationErrors: validateAggControlValues(controls, [ + controls.groupby?.value, + controls.metrics?.value, + controlState.value, + ]), }), + rerender: ['groupby', 'metrics'], default: [], validators: [], }; @@ -160,6 +180,20 @@ const config: ControlPanelConfig = { name: 'groupby', override: { visibility: isAggMode, + mapStateToProps: (state: ControlPanelState, controlState: ControlState) => { + const { controls } = state; + const originalMapStateToProps = sharedControls?.groupby?.mapStateToProps; + // @ts-ignore + const newState = originalMapStateToProps?.(state, controlState) ?? {}; + newState.externalValidationErrors = validateAggControlValues(controls, [ + controls.metrics?.value, + controls.percent_metrics?.value, + controlState.value, + ]); + + return newState; + }, + rerender: ['metrics', 'percent_metrics'], }, }, ], @@ -169,6 +203,22 @@ const config: ControlPanelConfig = { override: { validators: [], visibility: isAggMode, + mapStateToProps: ( + { controls, datasource, form_data }: ControlPanelState, + controlState: ControlState, + ) => ({ + columns: datasource?.columns.filter(c => c.filterable) || [], + savedMetrics: datasource?.metrics || [], + // current active adhoc metrics + selectedMetrics: form_data.metrics || (form_data.metric ? [form_data.metric] : []), + datasource, + externalValidationErrors: validateAggControlValues(controls, [ + controls.groupby?.value, + controls.percent_metrics?.value, + controlState.value, + ]), + }), + rerender: ['groupby', 'percent_metrics'], }, }, { @@ -181,9 +231,11 @@ const config: ControlPanelConfig = { [ { name: 'percent_metrics', - config: isFeatureEnabled(FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP) - ? dnd_percent_metrics - : percent_metrics, + config: { + ...(isFeatureEnabled(FeatureFlag.ENABLE_EXPLORE_DRAG_AND_DROP) + ? dnd_percent_metrics + : percent_metrics), + }, }, ], [