Skip to content

Commit

Permalink
feat(datasets): Allow swap dataset after deletion (apache#30364)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Antonio-RiveroMartnez
Antonio-RiveroMartnez authored Sep 25, 2024
1 parent 39f1b71 commit 18c2376
Show file tree
Hide file tree
Showing 3 changed files with 214 additions and 16 deletions.
172 changes: 172 additions & 0 deletions superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;',
});
});
39 changes: 25 additions & 14 deletions superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,31 +27,40 @@ import {
JsonValue,
SimpleAdhocFilter,
} from '@superset-ui/core';
import { isEmpty } from 'lodash';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';

const isControlValueCompatibleWithDatasource = (
datasource: Dataset,
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,
Expand All @@ -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) =>
Expand Down
19 changes: 17 additions & 2 deletions superset-frontend/src/pages/Chart/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down

0 comments on commit 18c2376

Please sign in to comment.