diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts index f0eb399267b11..df1afd69a0a0c 100644 --- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts +++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts @@ -286,3 +286,175 @@ test('SQL ad-hoc filter values', () => { sqlExpression: 'select * from sample_column_1;', }); }); + +test('no controlState value but valid column in datasource', () => { + const controlState = { + ...sharedControls.columns, + options: [], // no options in the control state + }; + + expect( + getValues({ + ...controlState, + value: 'sample_column_1', // column only available in datasource + }), + ).toEqual('sample_column_1'); + + expect( + getValues({ + ...controlState, + value: 'non_existing_column', + }), + ).toEqual(controlState.default); +}); + +test('no controlState value but valid saved metric in datasource', () => { + const controlState = { + ...sharedControls.metrics, + savedMetrics: [], // no saved metrics in the control state + }; + + expect( + getValues({ + ...controlState, + value: 'saved_metric_2', // metric only available in datasource + }), + ).toEqual('saved_metric_2'); + + expect( + getValues({ + ...controlState, + value: 'non_existing_metric', + }), + ).toEqual(controlState.default); +}); + +test('no controlState value but valid adhoc metric in datasource', () => { + const controlState = { + ...sharedControls.metrics, + columns: [], // no columns in control state + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + column: { column_name: 'sample_column_1' }, // only in datasource + }, + }), + ).toEqual({ + expressionType: 'SIMPLE', + column: { column_name: 'sample_column_1' }, + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + column: { column_name: 'non_existing_column' }, + }, + }), + ).toEqual(controlState.default); +}); + +test('no controlState value but valid adhoc filter in datasource', () => { + const controlState = { + ...sharedControls.adhoc_filters, + columns: [], // no columns in control state + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + subject: 'sample_column_1', // column available in datasource + }, + }), + ).toEqual({ + expressionType: 'SIMPLE', + subject: 'sample_column_1', + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + subject: 'non_existing_column', + }, + }), + ).toEqual(controlState.default); +}); + +test('SQL ad-hoc metric values without controlState columns', () => { + const controlState = { + ...sharedControls.metrics, + columns: [], // No columns in controlState + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT COUNT(*) FROM sample_table;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT COUNT(*) FROM sample_table;', + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT column FROM non_existing_table;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT column FROM non_existing_table;', + }); +}); + +test('SQL ad-hoc filter values without controlState columns', () => { + const controlState = { + ...sharedControls.adhoc_filters, + columns: [], // No columns in controlState + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM sample_table WHERE column = 1;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM sample_table WHERE column = 1;', + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM non_existing_table;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM non_existing_table;', + }); +}); diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts index 7013b59764cb0..bfc53dc6d8257 100644 --- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts +++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts @@ -27,6 +27,7 @@ import { JsonValue, SimpleAdhocFilter, } from '@superset-ui/core'; +import { isEmpty } from 'lodash'; import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; const isControlValueCompatibleWithDatasource = ( @@ -34,24 +35,32 @@ const isControlValueCompatibleWithDatasource = ( controlState: ControlState, value: any, ) => { + // A datasource might have been deleted, in which case we can't validate + // only using the control state since it might have been hydrated with + // the wrong options or columns (empty arrays). if (controlState.options && typeof value === 'string') { if ( - controlState.options.some( - (option: [string | number, string] | { column_name: string }) => - Array.isArray(option) - ? option[0] === value - : option.column_name === value, - ) + (!isEmpty(controlState.options) && + controlState.options.some( + (option: [string | number, string] | { column_name: string }) => + Array.isArray(option) + ? option[0] === value + : option.column_name === value, + )) || + !isEmpty(datasource?.columns) ) { - return datasource.columns.some(column => column.column_name === value); + return datasource.columns.some( + (column: Column) => column.column_name === value, + ); } } if ( controlState.savedMetrics && isSavedMetric(value) && - controlState.savedMetrics.some( + (controlState.savedMetrics.some( (savedMetric: Metric) => savedMetric.metric_name === value, - ) + ) || + !isEmpty(datasource?.metrics)) ) { return datasource.metrics.some( (metric: Metric) => metric.metric_name === value, @@ -60,11 +69,13 @@ const isControlValueCompatibleWithDatasource = ( if ( controlState.columns && (isAdhocMetricSimple(value) || isSimpleAdhocFilter(value)) && - controlState.columns.some( - (column: Column) => - column.column_name === (value as AdhocMetric).column?.column_name || - column.column_name === (value as SimpleAdhocFilter).subject, - ) + ((!isEmpty(controlState.columns) && + controlState.columns.some( + (column: Column) => + column.column_name === (value as AdhocMetric).column?.column_name || + column.column_name === (value as SimpleAdhocFilter).subject, + )) || + !isEmpty(datasource?.columns)) ) { return datasource.columns.some( (column: Column) => diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index e8e9ae127fe5a..af9386279f027 100644 --- a/superset-frontend/src/pages/Chart/index.tsx +++ b/superset-frontend/src/pages/Chart/index.tsx @@ -43,7 +43,10 @@ import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; import { getFormDataWithDashboardContext } from 'src/explore/controlUtils/getFormDataWithDashboardContext'; const isValidResult = (rv: JsonObject): boolean => - rv?.result?.form_data && isDefined(rv?.result?.dataset?.id); + rv?.result?.form_data && rv?.result?.dataset; + +const hasDatasetId = (rv: JsonObject): boolean => + isDefined(rv?.result?.dataset?.id); const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { try { @@ -52,7 +55,19 @@ const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { endpoint: 'api/v1/explore/', })(exploreUrlParams); if (isValidResult(rv)) { - return rv; + if (hasDatasetId(rv)) { + return rv; + } + // Since there's no dataset id but the API responded with a valid payload, + // we assume the dataset was deleted, so we preserve some values from previous + // state so if the user decide to swap the datasource, the chart config remains + fallbackExploreInitialData.form_data = { + ...rv.result.form_data, + ...fallbackExploreInitialData.form_data, + }; + if (rv.result?.slice) { + fallbackExploreInitialData.slice = rv.result.slice; + } } let message = t('Failed to load chart data'); const responseError = rv?.result?.message;