diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/index.ts index 66ef14917ab..6d22c2a7ce4 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/index.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/index.ts @@ -37,4 +37,4 @@ export { legacySortBy } from './shared-controls/legacySortBy'; export * from './shared-controls/emitFilterControl'; export * from './shared-controls/components'; export * from './types'; -export { xAxisMixin, temporalColumnMixin } from './shared-controls/constants'; +export { xAxisMixin, temporalColumnMixin } from './shared-controls/mixins'; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx index 691dbca7c0c..35a7f1979e9 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx @@ -40,7 +40,7 @@ import { FilterOption, temporalColumnMixin, } from '..'; -import { xAxisMixin } from './constants'; +import { xAxisMixin } from './mixins'; type Control = { savedMetrics?: Metric[] | null; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/constants.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/mixins.tsx similarity index 73% rename from superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/constants.tsx rename to superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/mixins.tsx index 8de12bfcf6a..4963be01226 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/constants.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/mixins.tsx @@ -20,16 +20,11 @@ import { FeatureFlag, isFeatureEnabled, QueryFormData, - QueryResponse, t, validateNonEmpty, } from '@superset-ui/core'; -import { - BaseControlConfig, - ControlPanelState, - ControlState, - Dataset, -} from '../types'; +import { BaseControlConfig, ControlPanelState, ControlState } from '../types'; +import { getTemporalColumns } from '../utils'; const getAxisLabel = ( formData: QueryFormData, @@ -60,24 +55,11 @@ export const xAxisMixin = { export const temporalColumnMixin: Pick = { mapStateToProps: ({ datasource }) => { - if (datasource?.columns[0]?.hasOwnProperty('column_name')) { - const temporalColumns = - (datasource as Dataset)?.columns?.filter(c => c.is_dttm) ?? []; - return { - options: temporalColumns, - default: - (datasource as Dataset)?.main_dttm_col || - temporalColumns[0]?.column_name || - null, - isTemporal: true, - }; - } - const sortedQueryColumns = (datasource as QueryResponse)?.columns?.sort( - query => (query?.is_dttm ? -1 : 1), - ); + const payload = getTemporalColumns(datasource); + return { - options: sortedQueryColumns, - default: sortedQueryColumns[0]?.name || null, + options: payload.temporalColumns, + default: payload.defaultTemporalColumn, isTemporal: true, }; }, diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts index 99ea6a5325e..8c9ce71874e 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/types.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/types.ts @@ -24,6 +24,7 @@ import type { DatasourceType, JsonValue, Metric, + QueryColumn, QueryFormColumn, QueryFormData, QueryFormMetric, @@ -449,9 +450,9 @@ export type ColorFormatters = { export default {}; export function isColumnMeta( - column: AdhocColumn | ColumnMeta, + column: AdhocColumn | ColumnMeta | QueryColumn, ): column is ColumnMeta { - return 'column_name' in column; + return !!column && 'column_name' in column; } export function isSavedExpression( @@ -477,9 +478,5 @@ export function isDataset( export function isQueryResponse( datasource: Dataset | QueryResponse | null | undefined, ): datasource is QueryResponse { - return ( - !!datasource && - ('results' in datasource || - datasource?.type === ('query' as DatasourceType.Query)) - ); + return !!datasource && 'results' in datasource && 'sql' in datasource; } diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/utils/getTemporalColumns.ts b/superset-frontend/packages/superset-ui-chart-controls/src/utils/getTemporalColumns.ts new file mode 100644 index 00000000000..caae8dfd52d --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/src/utils/getTemporalColumns.ts @@ -0,0 +1,64 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + ensureIsArray, + isDefined, + QueryColumn, + ValueOf, +} from '@superset-ui/core'; +import { + ColumnMeta, + ControlPanelState, + isDataset, + isQueryResponse, +} from '@superset-ui/chart-controls'; + +export const getTemporalColumns = ( + datasource: ValueOf>, +) => { + const rv: { + temporalColumns: ColumnMeta[] | QueryColumn[]; + defaultTemporalColumn: string | null | undefined; + } = { + temporalColumns: [], + defaultTemporalColumn: undefined, + }; + + if (isDataset(datasource)) { + rv.temporalColumns = ensureIsArray(datasource.columns).filter( + c => c.is_dttm, + ); + } + if (isQueryResponse(datasource)) { + rv.temporalColumns = ensureIsArray(datasource.columns).filter( + c => c.is_dttm, + ); + } + + if (isDataset(datasource)) { + rv.defaultTemporalColumn = datasource.main_dttm_col; + } + if (!isDefined(rv.defaultTemporalColumn)) { + rv.defaultTemporalColumn = + (rv.temporalColumns[0] as ColumnMeta)?.column_name ?? + (rv.temporalColumns[0] as QueryColumn)?.name; + } + + return rv; +}; diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts b/superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts index bd5fae8030f..ff0b8db6381 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts +++ b/superset-frontend/packages/superset-ui-chart-controls/src/utils/index.ts @@ -24,3 +24,4 @@ export { default as mainMetric } from './mainMetric'; export { default as columnChoices } from './columnChoices'; export * from './defineSavedMetrics'; export * from './getStandardizedControls'; +export { getTemporalColumns } from './getTemporalColumns'; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/fixtures.ts b/superset-frontend/packages/superset-ui-chart-controls/test/fixtures.ts new file mode 100644 index 00000000000..d694bde883b --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/test/fixtures.ts @@ -0,0 +1,149 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Dataset } from '@superset-ui/chart-controls'; +import { DatasourceType } from '@superset-ui/core'; + +export const TestDataset: Dataset = { + column_format: {}, + columns: [ + { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'num', + description: null, + expression: '', + filterable: true, + groupby: true, + id: 332, + is_certified: false, + is_dttm: false, + python_date_format: null, + type: 'BIGINT', + type_generic: 0, + verbose_name: null, + warning_markdown: null, + }, + { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'gender', + description: null, + expression: '', + filterable: true, + groupby: true, + id: 330, + is_certified: false, + is_dttm: false, + python_date_format: null, + type: 'VARCHAR(16)', + type_generic: 1, + verbose_name: '', + warning_markdown: null, + }, + { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'state', + description: null, + expression: '', + filterable: true, + groupby: true, + id: 333, + is_certified: false, + is_dttm: false, + python_date_format: null, + type: 'VARCHAR(10)', + type_generic: 1, + verbose_name: null, + warning_markdown: null, + }, + { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'ds', + description: null, + expression: '', + filterable: true, + groupby: true, + id: 329, + is_certified: false, + is_dttm: true, + python_date_format: null, + type: 'TIMESTAMP WITHOUT TIME ZONE', + type_generic: 2, + verbose_name: null, + warning_markdown: null, + }, + { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'name', + description: null, + expression: '', + filterable: true, + groupby: true, + id: 331, + is_certified: false, + is_dttm: false, + python_date_format: null, + type: 'VARCHAR(255)', + type_generic: 1, + verbose_name: null, + warning_markdown: null, + }, + ], + datasource_name: 'birth_names', + description: null, + granularity_sqla: 'ds', + id: 2, + main_dttm_col: 'ds', + metrics: [ + { + certification_details: null, + certified_by: null, + d3format: null, + description: null, + expression: 'COUNT(*)', + id: 7, + is_certified: false, + metric_name: 'count', + verbose_name: 'COUNT(*)', + warning_markdown: '', + warning_text: null, + }, + ], + name: 'public.birth_names', + order_by_choices: [], + owners: [ + { + first_name: 'admin', + id: 1, + last_name: 'user', + username: 'admin', + }, + ], + type: DatasourceType.Dataset, + uid: '2__table', + verbose_map: {}, +}; diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx index 3224bbcc26d..59f4796a44d 100644 --- a/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx +++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/columnChoices.test.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { DatasourceType, QueryResponse, testQuery } from '@superset-ui/core'; +import { DatasourceType, testQueryResponse } from '@superset-ui/core'; import { columnChoices } from '../../src'; describe('columnChoices()', () => { @@ -58,7 +58,7 @@ describe('columnChoices()', () => { }); it('should convert columns to choices when source is a Query', () => { - expect(columnChoices(testQuery as QueryResponse)).toEqual([ + expect(columnChoices(testQueryResponse)).toEqual([ ['Column 1', 'Column 1'], ['Column 2', 'Column 2'], ['Column 3', 'Column 3'], diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/utils/getTemporalColumns.test.ts b/superset-frontend/packages/superset-ui-chart-controls/test/utils/getTemporalColumns.test.ts new file mode 100644 index 00000000000..79dee957d15 --- /dev/null +++ b/superset-frontend/packages/superset-ui-chart-controls/test/utils/getTemporalColumns.test.ts @@ -0,0 +1,95 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { testQueryResponse, testQueryResults } from '@superset-ui/core'; +import { Dataset, getTemporalColumns } from '../../src'; +import { TestDataset } from '../fixtures'; + +test('get temporal columns from a Dataset', () => { + expect(getTemporalColumns(TestDataset)).toEqual({ + temporalColumns: [ + { + advanced_data_type: null, + certification_details: null, + certified_by: null, + column_name: 'ds', + description: null, + expression: '', + filterable: true, + groupby: true, + id: 329, + is_certified: false, + is_dttm: true, + python_date_format: null, + type: 'TIMESTAMP WITHOUT TIME ZONE', + type_generic: 2, + verbose_name: null, + warning_markdown: null, + }, + ], + defaultTemporalColumn: 'ds', + }); +}); + +test('get temporal columns from a QueryResponse', () => { + expect(getTemporalColumns(testQueryResponse)).toEqual({ + temporalColumns: [ + { + name: 'Column 2', + type: 'TIMESTAMP', + is_dttm: true, + }, + ], + defaultTemporalColumn: 'Column 2', + }); +}); + +test('get temporal columns from null', () => { + expect(getTemporalColumns(null)).toEqual({ + temporalColumns: [], + defaultTemporalColumn: undefined, + }); +}); + +test('should accept empty Dataset or queryResponse', () => { + expect( + getTemporalColumns({ + ...TestDataset, + ...{ + columns: [], + main_dttm_col: undefined, + }, + } as any as Dataset), + ).toEqual({ + temporalColumns: [], + defaultTemporalColumn: undefined, + }); + + expect( + getTemporalColumns({ + ...testQueryResponse, + ...{ + columns: [], + results: { ...testQueryResults.results, ...{ columns: [] } }, + }, + }), + ).toEqual({ + temporalColumns: [], + defaultTemporalColumn: undefined, + }); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts index 6c86b397fd3..a2d6df39b89 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Query.ts @@ -350,6 +350,7 @@ export type QueryResults = { export type QueryResponse = Query & QueryResults; +// todo: move out from typing export const testQuery: Query = { id: 'clientId2353', dbId: 1, @@ -388,22 +389,69 @@ export const testQuery: Query = { columns: [ { name: 'Column 1', - type: DatasourceType.Query, + type: 'STRING', is_dttm: false, }, { name: 'Column 3', - type: DatasourceType.Query, + type: 'STRING', is_dttm: false, }, { name: 'Column 2', - type: DatasourceType.Query, + type: 'TIMESTAMP', is_dttm: true, }, ], }; +export const testQueryResults = { + results: { + displayLimitReached: false, + columns: [ + { + name: 'Column 1', + type: 'STRING', + is_dttm: false, + }, + { + name: 'Column 3', + type: 'STRING', + is_dttm: false, + }, + { + name: 'Column 2', + type: 'TIMESTAMP', + is_dttm: true, + }, + ], + data: [ + { 'Column 1': 'a', 'Column 2': 'b', 'Column 3': '2014-11-11T00:00:00' }, + ], + expanded_columns: [], + selected_columns: [ + { + name: 'Column 1', + type: 'STRING', + is_dttm: false, + }, + { + name: 'Column 3', + type: 'STRING', + is_dttm: false, + }, + { + name: 'Column 2', + type: 'TIMESTAMP', + is_dttm: true, + }, + ], + query: { limit: 6 }, + }, +}; + +export const testQueryResponse = { ...testQuery, ...testQueryResults }; + export enum ContributionType { Row = 'row', Column = 'column', diff --git a/superset-frontend/packages/superset-ui-core/src/types/index.ts b/superset-frontend/packages/superset-ui-core/src/types/index.ts index eaab5c0b49d..7c75ad42cc8 100644 --- a/superset-frontend/packages/superset-ui-core/src/types/index.ts +++ b/superset-frontend/packages/superset-ui-core/src/types/index.ts @@ -19,3 +19,5 @@ export * from '../query/types'; export type Maybe = T | null; + +export type ValueOf = T[keyof T]; diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts index 14ce144d61e..a897eea156a 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/buildQuery.ts @@ -16,26 +16,44 @@ * specific language governing permissions and limitations * under the License. */ -import { buildQueryContext } from '@superset-ui/core'; +import { + AdhocColumn, + buildQueryContext, + ensureIsArray, + isPhysicalColumn, +} from '@superset-ui/core'; import { boxplotOperator } from '@superset-ui/chart-controls'; import { BoxPlotQueryFormData } from './types'; export default function buildQuery(formData: BoxPlotQueryFormData) { - const { columns = [], granularity_sqla, groupby = [] } = formData; - return buildQueryContext(formData, baseQueryObject => { - const distributionColumns: string[] = []; - // For now default to using the temporal column as distribution column. - // In the future this control should be made mandatory. - if (!columns.length && granularity_sqla) { - distributionColumns.push(granularity_sqla); - } - return [ - { - ...baseQueryObject, - columns: [...distributionColumns, ...columns, ...groupby], - series_columns: groupby, - post_processing: [boxplotOperator(formData, baseQueryObject)], - }, - ]; - }); + return buildQueryContext(formData, baseQueryObject => [ + { + ...baseQueryObject, + columns: [ + ...(ensureIsArray(formData.columns).length === 0 && + formData.granularity_sqla + ? [formData.granularity_sqla] // for backwards compatible: if columns control is empty and granularity_sqla was set, the time columns is default distributed column. + : ensureIsArray(formData.columns) + ).map(col => { + if ( + isPhysicalColumn(col) && + formData.time_grain_sqla && + formData?.datetime_columns_lookup?.[col] + ) { + return { + timeGrain: formData.time_grain_sqla, + columnType: 'BASE_AXIS', + sqlExpression: col, + label: col, + expressionType: 'SQL', + } as AdhocColumn; + } + return col; + }), + ...ensureIsArray(formData.groupby), + ], + series_columns: formData.groupby, + post_processing: [boxplotOperator(formData, baseQueryObject)], + }, + ]); } diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts index 8a85c42bfa5..b439a9888a3 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/src/BoxPlot/controlPanel.ts @@ -16,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { ensureIsArray, t } from '@superset-ui/core'; +import { + ensureIsArray, + isAdhocColumn, + isPhysicalColumn, + t, + validateNonEmpty, +} from '@superset-ui/core'; import { D3_FORMAT_DOCS, D3_FORMAT_OPTIONS, @@ -26,20 +32,53 @@ import { emitFilterControl, ControlPanelConfig, getStandardizedControls, + ControlState, + ControlPanelState, + getTemporalColumns, + sharedControls, } from '@superset-ui/chart-controls'; const config: ControlPanelConfig = { controlPanelSections: [ - sections.legacyTimeseriesTime, + sections.legacyRegularTime, { label: t('Query'), expanded: true, controlSetRows: [ + ['columns'], + [ + { + name: 'time_grain_sqla', + config: { + ...sharedControls.time_grain_sqla, + visibility: ({ controls }) => { + const dttmLookup = Object.fromEntries( + ensureIsArray(controls?.columns?.options).map(option => [ + option.column_name, + option.is_dttm, + ]), + ); + + return ensureIsArray(controls?.columns.value) + .map(selection => { + if (isAdhocColumn(selection)) { + return true; + } + if (isPhysicalColumn(selection)) { + return !!dttmLookup[selection]; + } + return false; + }) + .some(Boolean); + }, + }, + }, + 'datetime_columns_lookup', + ], + ['groupby'], ['metrics'], ['adhoc_filters'], emitFilterControl, - ['groupby'], - ['columns'], // TODO: this should be migrated to `series_columns` ['series_limit'], ['series_limit_metric'], [ @@ -132,9 +171,17 @@ const config: ControlPanelConfig = { columns: { label: t('Distribute across'), multi: true, - description: t( - 'Columns to calculate distribution across. Defaults to temporal column if left empty.', - ), + description: t('Columns to calculate distribution across.'), + initialValue: (control: ControlState, state: ControlPanelState) => { + if ( + (state && !control?.value) || + (Array.isArray(control?.value) && control.value.length === 0) + ) { + return [getTemporalColumns(state.datasource).defaultTemporalColumn]; + } + return control.value; + }, + validators: [validateNonEmpty], }, }, formDataOverrides: formData => {