You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2022/09/20 19:49:47 UTC
[superset] branch master updated: fix(explore): fix chart save when dashboard deleted (#21497)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 6644a84f79 fix(explore): fix chart save when dashboard deleted (#21497)
6644a84f79 is described below
commit 6644a84f79385ab11fdf1314293ef1fb284431ec
Author: Cody Leff <co...@preset.io>
AuthorDate: Tue Sep 20 15:49:31 2022 -0400
fix(explore): fix chart save when dashboard deleted (#21497)
---
.../src/explore/actions/hydrateExplore.ts | 1 -
.../src/explore/actions/saveModalActions.js | 29 ++++++++--
.../src/explore/actions/saveModalActions.test.js | 61 ++++++++++++++++++----
.../components/ExploreViewContainer/index.jsx | 1 -
.../src/explore/components/SaveModal.tsx | 22 ++++++--
5 files changed, 93 insertions(+), 21 deletions(-)
diff --git a/superset-frontend/src/explore/actions/hydrateExplore.ts b/superset-frontend/src/explore/actions/hydrateExplore.ts
index 24e7382087..929dd2a7f8 100644
--- a/superset-frontend/src/explore/actions/hydrateExplore.ts
+++ b/superset-frontend/src/explore/actions/hydrateExplore.ts
@@ -123,7 +123,6 @@ export const hydrateExplore =
controlsTransferred: explore.controlsTransferred,
standalone: getUrlParam(URL_PARAMS.standalone),
force: getUrlParam(URL_PARAMS.force),
- sliceDashboards: initialFormData.dashboards,
};
// apply initial mapStateToProps for all controls, must execute AFTER
diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js
index 929ef56e54..b5bb2c6f97 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.js
+++ b/superset-frontend/src/explore/actions/saveModalActions.js
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+import rison from 'rison';
import { SupersetClient, t } from '@superset-ui/core';
import { addSuccessToast } from 'src/components/MessageToasts/actions';
import { buildV1ChartDataPayload } from '../exploreUtils';
@@ -66,6 +67,7 @@ export function removeSaveModalAlert() {
export const getSlicePayload = (
sliceName,
formDataWithNativeFilters,
+ dashboards,
owners,
) => {
const adhocFilters = Object.entries(formDataWithNativeFilters).reduce(
@@ -78,6 +80,7 @@ export const getSlicePayload = (
const formData = {
...formDataWithNativeFilters,
...adhocFilters,
+ dashboards,
};
const [datasourceId, datasourceType] = formData.datasource.split('__');
@@ -87,7 +90,7 @@ export const getSlicePayload = (
viz_type: formData.viz_type,
datasource_id: parseInt(datasourceId, 10),
datasource_type: datasourceType,
- dashboards: formData.dashboards,
+ dashboards,
owners,
query_context: JSON.stringify(
buildV1ChartDataPayload({
@@ -142,7 +145,7 @@ const addToasts = (isNewSlice, sliceName, addedToDashboard) => {
// Update existing slice
export const updateSlice =
- ({ slice_id: sliceId, owners }, sliceName, addedToDashboard) =>
+ ({ slice_id: sliceId, owners }, sliceName, dashboards, addedToDashboard) =>
async (dispatch, getState) => {
const {
explore: {
@@ -152,7 +155,7 @@ export const updateSlice =
try {
const response = await SupersetClient.put({
endpoint: `/api/v1/chart/${sliceId}`,
- jsonPayload: getSlicePayload(sliceName, formData, owners),
+ jsonPayload: getSlicePayload(sliceName, formData, dashboards, owners),
});
dispatch(saveSliceSuccess());
@@ -166,7 +169,7 @@ export const updateSlice =
// Create new slice
export const createSlice =
- (sliceName, addedToDashboard) => async (dispatch, getState) => {
+ (sliceName, dashboards, addedToDashboard) => async (dispatch, getState) => {
const {
explore: {
form_data: { url_params: _, ...formData },
@@ -175,7 +178,7 @@ export const createSlice =
try {
const response = await SupersetClient.post({
endpoint: `/api/v1/chart/`,
- jsonPayload: getSlicePayload(sliceName, formData),
+ jsonPayload: getSlicePayload(sliceName, formData, dashboards),
});
dispatch(saveSliceSuccess());
@@ -215,3 +218,19 @@ export const getDashboard = dashboardId => async dispatch => {
throw error;
}
};
+
+// Get dashboards the slice is added to
+export const getSliceDashboards = slice => async dispatch => {
+ try {
+ const response = await SupersetClient.get({
+ endpoint: `/api/v1/chart/${slice.slice_id}?q=${rison.encode({
+ columns: ['dashboards.id'],
+ })}`,
+ });
+
+ return response.json.result.dashboards.map(({ id }) => id);
+ } catch (error) {
+ dispatch(saveSliceFailed());
+ throw error;
+ }
+};
diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js
index 9dfb385b47..8e4a4e5abd 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.test.js
+++ b/superset-frontend/src/explore/actions/saveModalActions.test.js
@@ -27,6 +27,7 @@ import {
FETCH_DASHBOARDS_FAILED,
FETCH_DASHBOARDS_SUCCEEDED,
getDashboard,
+ getSliceDashboards,
SAVE_SLICE_FAILED,
SAVE_SLICE_SUCCESS,
updateSlice,
@@ -97,10 +98,11 @@ test('updateSlice handles success', async () => {
fetchMock.put(updateSliceEndpoint, sliceResponsePayload);
const dispatch = sinon.spy();
const getState = sinon.spy(() => mockExploreState);
- const slice = await updateSlice({ slice_id: sliceId }, sliceName)(
- dispatch,
- getState,
- );
+ const slice = await updateSlice(
+ { slice_id: sliceId },
+ sliceName,
+ [],
+ )(dispatch, getState);
expect(fetchMock.calls(updateSliceEndpoint)).toHaveLength(1);
expect(dispatch.callCount).toBe(2);
@@ -121,7 +123,7 @@ test('updateSlice handles failure', async () => {
const getState = sinon.spy(() => mockExploreState);
let caughtError;
try {
- await updateSlice({ slice_id: sliceId }, sliceName)(dispatch, getState);
+ await updateSlice({ slice_id: sliceId }, sliceName, [])(dispatch, getState);
} catch (error) {
caughtError = error;
}
@@ -142,7 +144,7 @@ test('createSlice handles success', async () => {
fetchMock.post(createSliceEndpoint, sliceResponsePayload);
const dispatch = sinon.spy();
const getState = sinon.spy(() => mockExploreState);
- const slice = await createSlice(sliceName)(dispatch, getState);
+ const slice = await createSlice(sliceName, [])(dispatch, getState);
expect(fetchMock.calls(createSliceEndpoint)).toHaveLength(1);
expect(dispatch.callCount).toBe(2);
expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_SUCCESS);
@@ -162,7 +164,7 @@ test('createSlice handles failure', async () => {
const getState = sinon.spy(() => mockExploreState);
let caughtError;
try {
- await createSlice(sliceName)(dispatch, getState);
+ await createSlice(sliceName, [])(dispatch, getState);
} catch (error) {
caughtError = error;
}
@@ -248,7 +250,7 @@ test('updateSlice with add to new dashboard handles success', async () => {
fetchMock.put(updateSliceEndpoint, sliceResponsePayload);
const dispatch = sinon.spy();
const getState = sinon.spy(() => mockExploreState);
- const slice = await updateSlice({ slice_id: sliceId }, sliceName, {
+ const slice = await updateSlice({ slice_id: sliceId }, sliceName, [], {
new: true,
title: dashboardName,
})(dispatch, getState);
@@ -275,7 +277,7 @@ test('updateSlice with add to existing dashboard handles success', async () => {
fetchMock.put(updateSliceEndpoint, sliceResponsePayload);
const dispatch = sinon.spy();
const getState = sinon.spy(() => mockExploreState);
- const slice = await updateSlice({ slice_id: sliceId }, sliceName, {
+ const slice = await updateSlice({ slice_id: sliceId }, sliceName, [], {
new: false,
title: dashboardName,
})(dispatch, getState);
@@ -296,3 +298,44 @@ test('updateSlice with add to existing dashboard handles success', async () => {
expect(slice).toEqual(sliceResponsePayload);
});
+
+const slice = { slice_id: 10 };
+const dashboardSlicesResponsePayload = {
+ result: {
+ dashboards: [{ id: 21 }, { id: 22 }, { id: 23 }],
+ },
+};
+
+const getDashboardSlicesReturnValue = [21, 22, 23];
+
+/**
+ * Tests getSliceDashboards action
+ */
+
+const getSliceDashboardsEndpoint = `glob:*/api/v1/chart/${sliceId}?q=(columns:!(dashboards.id))`;
+test('getSliceDashboards with slice handles success', async () => {
+ fetchMock.reset();
+ fetchMock.get(getSliceDashboardsEndpoint, dashboardSlicesResponsePayload);
+ const dispatch = sinon.spy();
+ const sliceDashboards = await getSliceDashboards(slice)(dispatch);
+ expect(fetchMock.calls(getSliceDashboardsEndpoint)).toHaveLength(1);
+ expect(dispatch.callCount).toBe(0);
+ expect(sliceDashboards).toEqual(getDashboardSlicesReturnValue);
+});
+
+test('getSliceDashboards with slice handles failure', async () => {
+ fetchMock.reset();
+ fetchMock.get(getSliceDashboardsEndpoint, { throws: sampleError });
+ const dispatch = sinon.spy();
+ let caughtError;
+ try {
+ await getSliceDashboards(slice)(dispatch);
+ } catch (error) {
+ caughtError = error;
+ }
+
+ expect(caughtError).toEqual(sampleError);
+ expect(fetchMock.calls(getSliceDashboardsEndpoint)).toHaveLength(4);
+ expect(dispatch.callCount).toBe(1);
+ expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED);
+});
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
index 7f6a870039..37acbf457a 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx
@@ -599,7 +599,6 @@ function ExploreViewContainer(props) {
form_data={props.form_data}
sliceName={props.sliceName}
dashboardId={props.dashboardId}
- sliceDashboards={props.exploreState.sliceDashboards ?? []}
/>
)}
<Resizable
diff --git a/superset-frontend/src/explore/components/SaveModal.tsx b/superset-frontend/src/explore/components/SaveModal.tsx
index 04c7fd6b8c..a4649d8926 100644
--- a/superset-frontend/src/explore/components/SaveModal.tsx
+++ b/superset-frontend/src/explore/components/SaveModal.tsx
@@ -49,7 +49,6 @@ interface SaveModalProps extends RouteComponentProps {
slice?: Record<string, any>;
datasource?: Record<string, any>;
dashboardId: '' | number | null;
- sliceDashboards: number[];
}
type ActionType = 'overwrite' | 'saveas';
@@ -183,6 +182,16 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
}
}
+ // Get chart dashboards
+ let sliceDashboards: number[] = [];
+ if (this.props.slice && this.state.action === 'overwrite') {
+ promise = promise
+ .then(() => this.props.actions.getSliceDashboards(this.props.slice))
+ .then(dashboards => {
+ sliceDashboards = dashboards;
+ });
+ }
+
let dashboard: DashboardGetResponse | null = null;
if (this.state.newDashboardName || this.state.saveToDashboardId) {
let saveToDashboardId = this.state.saveToDashboardId || null;
@@ -200,12 +209,13 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
.then(() => this.props.actions.getDashboard(saveToDashboardId))
.then((response: { result: DashboardGetResponse }) => {
dashboard = response.result;
- const dashboards = new Set<number>(this.props.sliceDashboards);
- dashboards.add(dashboard.id);
+ sliceDashboards = sliceDashboards.includes(dashboard.id)
+ ? sliceDashboards
+ : [...sliceDashboards, dashboard.id];
const { url_params, ...formData } = this.props.form_data || {};
this.props.actions.setFormData({
...formData,
- dashboards: Array.from(dashboards),
+ dashboards: sliceDashboards,
});
});
}
@@ -216,6 +226,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
this.props.actions.updateSlice(
this.props.slice,
this.state.newSliceName,
+ sliceDashboards,
dashboard
? {
title: dashboard.dashboard_title,
@@ -228,6 +239,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
promise = promise.then(() =>
this.props.actions.createSlice(
this.state.newSliceName,
+ sliceDashboards,
dashboard
? {
title: dashboard.dashboard_title,
@@ -276,7 +288,7 @@ class SaveModal extends React.Component<SaveModalProps, SaveModalState> {
type="warning"
message={
<>
- {this.state.alert ? this.state.alert : this.props.alert}
+ {this.state.alert || this.props.alert}
<i
role="button"
aria-label="Remove alert"