Skip to content

Commit

Permalink
fix: save dataset and repopulate state (apache#20965)
Browse files Browse the repository at this point in the history
* save dataset and repopulate state

* disable dashboard button if dataset is missing

* fix error message

* fix tests
  • Loading branch information
eschutho authored Aug 5, 2022
1 parent 95fdc08 commit 463406f
Show file tree
Hide file tree
Showing 11 changed files with 283 additions and 59 deletions.
31 changes: 31 additions & 0 deletions superset-frontend/packages/superset-ui-core/src/api/types/core.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* 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.
*/

// /superset/sqllab_viz
interface SqlLabPostRequest {
data: {
schema: string;
sql: string;
dbId: number;
templateParams?: string | undefined;
datasourceName: string;
metrics?: string[];
columns?: string[];
};
}
54 changes: 54 additions & 0 deletions superset-frontend/src/explore/actions/datasourcesActions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,14 @@
* under the License.
*/
import { DatasourceType } from '@superset-ui/core';
import fetchMock from 'fetch-mock';
import {
setDatasource,
changeDatasource,
saveDataset,
} from 'src/explore/actions/datasourcesActions';
import sinon from 'sinon';
import * as ClientError from 'src/utils/getClientErrorObject';
import datasourcesReducer from '../reducers/datasourcesReducer';
import { updateFormDataByDatasource } from './exploreActions';

Expand Down Expand Up @@ -51,10 +55,21 @@ const NEW_DATASOURCE = {
description: null,
};

const SAVE_DATASET_POST_ARGS = {
schema: 'foo',
sql: 'select * from bar',
database: { id: 1 },
templateParams: undefined,
datasourceName: 'new dataset',
columns: [],
};

const defaultDatasourcesReducerState = {
[CURRENT_DATASOURCE.uid]: CURRENT_DATASOURCE,
};

const saveDatasetEndpoint = `glob:*/superset/sqllab_viz/`;

test('sets new datasource', () => {
const newState = datasourcesReducer(
defaultDatasourcesReducerState,
Expand Down Expand Up @@ -83,3 +98,42 @@ test('change datasource action', () => {
updateFormDataByDatasource(CURRENT_DATASOURCE, NEW_DATASOURCE),
);
});

test('saveDataset handles success', async () => {
const datasource = { id: 1 };
const saveDatasetResponse = {
data: datasource,
};
fetchMock.reset();
fetchMock.post(saveDatasetEndpoint, saveDatasetResponse);
const dispatch = sinon.spy();
const getState = sinon.spy(() => ({ explore: { datasource } }));
const dataset = await saveDataset(SAVE_DATASET_POST_ARGS)(dispatch);

expect(fetchMock.calls(saveDatasetEndpoint)).toHaveLength(1);
expect(dispatch.callCount).toBe(1);
const thunk = dispatch.getCall(0).args[0];
thunk(dispatch, getState);
expect(dispatch.getCall(1).args[0].type).toEqual('SET_DATASOURCE');

expect(dataset).toEqual(datasource);
});

test('updateSlice with add to existing dashboard handles failure', async () => {
fetchMock.reset();
const sampleError = new Error('sampleError');
fetchMock.post(saveDatasetEndpoint, { throws: sampleError });
const dispatch = sinon.spy();
const errorSpy = jest.spyOn(ClientError, 'getClientErrorObject');

let caughtError;
try {
await saveDataset(SAVE_DATASET_POST_ARGS)(dispatch);
} catch (error) {
caughtError = error;
}

expect(caughtError).toEqual(sampleError);
expect(fetchMock.calls(saveDatasetEndpoint)).toHaveLength(4);
expect(errorSpy).toHaveBeenCalledWith(sampleError);
});
46 changes: 45 additions & 1 deletion superset-frontend/src/explore/actions/datasourcesActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
* under the License.
*/

import { Dispatch } from 'redux';
import { Dispatch, AnyAction } from 'redux';
import { ThunkDispatch } from 'redux-thunk';
import { Dataset } from '@superset-ui/chart-controls';
import { SupersetClient } from '@superset-ui/core';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { updateFormDataByDatasource } from './exploreActions';
import { ExplorePageState } from '../types';

Expand All @@ -31,6 +35,45 @@ export function setDatasource(datasource: Dataset) {
return { type: SET_DATASOURCE, datasource };
}

export function saveDataset({
schema,
sql,
database,
templateParams,
datasourceName,
columns,
}: Omit<SqlLabPostRequest['data'], 'dbId'> & { database: { id: number } }) {
return async function (dispatch: ThunkDispatch<any, undefined, AnyAction>) {
// Create a dataset object
try {
const {
json: { data },
} = await SupersetClient.post({
endpoint: '/superset/sqllab_viz/',
postPayload: {
data: {
schema,
sql,
dbId: database?.id,
templateParams,
datasourceName,
metrics: [],
columns,
},
},
});
// Update form_data to point to new dataset
dispatch(changeDatasource(data));
return data;
} catch (error) {
getClientErrorObject(error).then(e => {
dispatch(addDangerToast(e.error));
});
throw error;
}
};
}

export function changeDatasource(newDatasource: Dataset) {
return function (dispatch: Dispatch, getState: () => ExplorePageState) {
const {
Expand All @@ -44,6 +87,7 @@ export function changeDatasource(newDatasource: Dataset) {
export const datasourcesActions = {
setDatasource,
changeDatasource,
saveDataset,
};

export type AnyDatasourcesAction = SetDatasource;
5 changes: 5 additions & 0 deletions superset-frontend/src/explore/actions/exploreActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ export function setExploreControls(formData: QueryFormData) {
return { type: SET_EXPLORE_CONTROLS, formData };
}

export const SET_FORM_DATA = 'UPDATE_FORM_DATA';
export function setFormData(formData: QueryFormData) {
return { type: SET_FORM_DATA, formData };
}

export const UPDATE_CHART_TITLE = 'UPDATE_CHART_TITLE';
export function updateChartTitle(sliceName: string) {
return { type: UPDATE_CHART_TITLE, sliceName };
Expand Down
72 changes: 71 additions & 1 deletion superset-frontend/src/explore/actions/hydrateExplore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,77 @@ test('creates hydrate action from initial data', () => {
controls: expect.any(Object),
form_data: exploreInitialData.form_data,
slice: exploreInitialData.slice,
controlsTransferred: [],
standalone: null,
force: null,
},
},
}),
);
});

test('creates hydrate action with existing state', () => {
const dispatch = jest.fn();
const getState = jest.fn(() => ({
user: {},
charts: {},
datasources: {},
common: {},
explore: { controlsTransferred: ['all_columns'] },
}));
// ignore type check - we dont need exact explore state for this test
// @ts-ignore
hydrateExplore(exploreInitialData)(dispatch, getState);
expect(dispatch).toHaveBeenCalledWith(
expect.objectContaining({
type: HYDRATE_EXPLORE,
data: {
charts: {
371: {
id: 371,
chartAlert: null,
chartStatus: null,
chartStackTrace: null,
chartUpdateEndTime: null,
chartUpdateStartTime: 0,
latestQueryFormData: {
cache_timeout: undefined,
datasource: '8__table',
slice_id: 371,
url_params: undefined,
viz_type: 'table',
},
sliceFormData: {
cache_timeout: undefined,
datasource: '8__table',
slice_id: 371,
url_params: undefined,
viz_type: 'table',
},
queryController: null,
queriesResponse: null,
triggerQuery: false,
lastRendered: 0,
},
},
datasources: {
'8__table': exploreInitialData.dataset,
},
saveModal: {
dashboards: [],
saveModalAlert: null,
},
explore: {
can_add: false,
can_download: false,
can_overwrite: false,
isDatasourceMetaLoading: false,
isStarred: false,
triggerRender: false,
datasource: exploreInitialData.dataset,
controls: expect.any(Object),
controlsTransferred: ['all_columns'],
form_data: exploreInitialData.form_data,
slice: exploreInitialData.slice,
standalone: null,
force: null,
},
Expand Down
5 changes: 3 additions & 2 deletions superset-frontend/src/explore/actions/hydrateExplore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export const HYDRATE_EXPLORE = 'HYDRATE_EXPLORE';
export const hydrateExplore =
({ form_data, slice, dataset }: ExplorePageInitialData) =>
(dispatch: Dispatch, getState: () => ExplorePageState) => {
const { user, datasources, charts, sliceEntities, common } = getState();
const { user, datasources, charts, sliceEntities, common, explore } =
getState();

const sliceId = getUrlParam(URL_PARAMS.sliceId);
const dashboardId = getUrlParam(URL_PARAMS.dashboardId);
Expand Down Expand Up @@ -119,7 +120,7 @@ export const hydrateExplore =
controls: initialControls,
form_data: initialFormData,
slice: initialSlice,
controlsTransferred: [],
controlsTransferred: explore.controlsTransferred,
standalone: getUrlParam(URL_PARAMS.standalone),
force: getUrlParam(URL_PARAMS.force),
sliceDashboards: initialFormData.dashboards,
Expand Down
16 changes: 13 additions & 3 deletions superset-frontend/src/explore/actions/saveModalActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,13 @@ const addToasts = (isNewSlice, sliceName, addedToDashboard) => {

// Update existing slice
export const updateSlice =
({ slice_id: sliceId, owners }, sliceName, formData, addedToDashboard) =>
async dispatch => {
({ slice_id: sliceId, owners }, sliceName, addedToDashboard) =>
async (dispatch, getState) => {
const {
explore: {
form_data: { url_params: _, ...formData },
},
} = getState();
try {
const response = await SupersetClient.put({
endpoint: `/api/v1/chart/${sliceId}`,
Expand All @@ -154,7 +159,12 @@ export const updateSlice =

// Create new slice
export const createSlice =
(sliceName, formData, addedToDashboard) => async dispatch => {
(sliceName, addedToDashboard) => async (dispatch, getState) => {
const {
explore: {
form_data: { url_params: _, ...formData },
},
} = getState();
try {
const response = await SupersetClient.post({
endpoint: `/api/v1/chart/`,
Expand Down
Loading

0 comments on commit 463406f

Please sign in to comment.