diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js index 14bbe09995..1c3c3b765f 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.js +++ b/superset-frontend/src/explore/actions/saveModalActions.js @@ -19,6 +19,7 @@ import rison from 'rison'; import { SupersetClient, t } from '@superset-ui/core'; import { addSuccessToast } from 'src/components/MessageToasts/actions'; +import { isEmpty } from 'lodash'; import { buildV1ChartDataPayload } from '../exploreUtils'; const ADHOC_FILTER_REGEX = /^adhoc_filters/; @@ -76,19 +77,35 @@ export function removeSaveModalAlert() { return { type: REMOVE_SAVE_MODAL_ALERT }; } -export const getSlicePayload = ( - sliceName, - formDataWithNativeFilters, - dashboards, - owners, -) => { - const adhocFilters = Object.entries(formDataWithNativeFilters).reduce( +const extractAddHocFiltersFromFormData = formDataToHandle => + Object.entries(formDataToHandle).reduce( (acc, [key, value]) => ADHOC_FILTER_REGEX.test(key) ? { ...acc, [key]: value?.filter(f => !f.isExtra) } : acc, {}, ); + +export const getSlicePayload = ( + sliceName, + formDataWithNativeFilters, + dashboards, + owners, + formDataFromSlice = {}, +) => { + let adhocFilters = extractAddHocFiltersFromFormData( + formDataWithNativeFilters, + ); + + // Retain adhoc_filters from the slice if no adhoc_filters are present + // after overwriting a chart. This ensures the dashboard can continue + // to filter the chart. Before, any time range filter applied in the dashboard + // would end up as an extra filter and when overwriting the chart the original + // time range adhoc_filter was lost + if (isEmpty(adhocFilters?.adhoc_filters) && !isEmpty(formDataFromSlice)) { + adhocFilters = extractAddHocFiltersFromFormData(formDataFromSlice); + } + const formData = { ...formDataWithNativeFilters, ...adhocFilters, @@ -157,8 +174,9 @@ const addToasts = (isNewSlice, sliceName, addedToDashboard) => { // Update existing slice export const updateSlice = - ({ slice_id: sliceId, owners }, sliceName, dashboards, addedToDashboard) => + (slice, sliceName, dashboards, addedToDashboard) => async (dispatch, getState) => { + const { slice_id: sliceId, owners, form_data: formDataFromSlice } = slice; const { explore: { form_data: { url_params: _, ...formData }, @@ -167,7 +185,13 @@ export const updateSlice = try { const response = await SupersetClient.put({ endpoint: `/api/v1/chart/${sliceId}`, - jsonPayload: getSlicePayload(sliceName, formData, dashboards, owners), + jsonPayload: getSlicePayload( + sliceName, + formData, + dashboards, + owners, + formDataFromSlice, + ), }); dispatch(saveSliceSuccess()); diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js index 8e4a4e5abd..f89729f5ff 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.test.js +++ b/superset-frontend/src/explore/actions/saveModalActions.test.js @@ -31,6 +31,7 @@ import { SAVE_SLICE_FAILED, SAVE_SLICE_SUCCESS, updateSlice, + getSlicePayload, } from './saveModalActions'; /** @@ -339,3 +340,124 @@ test('getSliceDashboards with slice handles failure', async () => { expect(dispatch.callCount).toBe(1); expect(dispatch.getCall(0).args[0].type).toBe(SAVE_SLICE_FAILED); }); + +describe('getSlicePayload', () => { + const sliceName = 'Test Slice'; + const formDataWithNativeFilters = { + datasource: '22__table', + viz_type: 'pie', + adhoc_filters: [], + }; + const dashboards = [5]; + const owners = [1]; + const formDataFromSlice = { + datasource: '22__table', + viz_type: 'pie', + adhoc_filters: [ + { + clause: 'WHERE', + subject: 'year', + operator: 'TEMPORAL_RANGE', + comparator: 'No filter', + expressionType: 'SIMPLE', + }, + ], + }; + + test('should return the correct payload when no adhoc_filters are present in formDataWithNativeFilters', () => { + const result = getSlicePayload( + sliceName, + formDataWithNativeFilters, + dashboards, + owners, + formDataFromSlice, + ); + expect(result).toHaveProperty('params'); + expect(result).toHaveProperty('slice_name', sliceName); + expect(result).toHaveProperty( + 'viz_type', + formDataWithNativeFilters.viz_type, + ); + expect(result).toHaveProperty('datasource_id', 22); + expect(result).toHaveProperty('datasource_type', 'table'); + expect(result).toHaveProperty('dashboards', dashboards); + expect(result).toHaveProperty('owners', owners); + expect(result).toHaveProperty('query_context'); + expect(JSON.parse(result.params).adhoc_filters).toEqual( + formDataFromSlice.adhoc_filters, + ); + }); + + test('should return the correct payload when adhoc_filters are present in formDataWithNativeFilters', () => { + const formDataWithAdhocFilters = { + ...formDataWithNativeFilters, + adhoc_filters: [ + { + clause: 'WHERE', + subject: 'year', + operator: 'TEMPORAL_RANGE', + comparator: 'No filter', + expressionType: 'SIMPLE', + }, + ], + }; + const result = getSlicePayload( + sliceName, + formDataWithAdhocFilters, + dashboards, + owners, + formDataFromSlice, + ); + expect(result).toHaveProperty('params'); + expect(result).toHaveProperty('slice_name', sliceName); + expect(result).toHaveProperty( + 'viz_type', + formDataWithAdhocFilters.viz_type, + ); + expect(result).toHaveProperty('datasource_id', 22); + expect(result).toHaveProperty('datasource_type', 'table'); + expect(result).toHaveProperty('dashboards', dashboards); + expect(result).toHaveProperty('owners', owners); + expect(result).toHaveProperty('query_context'); + expect(JSON.parse(result.params).adhoc_filters).toEqual( + formDataWithAdhocFilters.adhoc_filters, + ); + }); + + test('should return the correct payload when formDataWithNativeFilters has a filter with isExtra set to true', () => { + const formDataWithAdhocFiltersWithExtra = { + ...formDataWithNativeFilters, + adhoc_filters: [ + { + clause: 'WHERE', + subject: 'year', + operator: 'TEMPORAL_RANGE', + comparator: 'No filter', + expressionType: 'SIMPLE', + isExtra: true, + }, + ], + }; + const result = getSlicePayload( + sliceName, + formDataWithAdhocFiltersWithExtra, + dashboards, + owners, + formDataFromSlice, + ); + expect(result).toHaveProperty('params'); + expect(result).toHaveProperty('slice_name', sliceName); + expect(result).toHaveProperty( + 'viz_type', + formDataWithAdhocFiltersWithExtra.viz_type, + ); + expect(result).toHaveProperty('datasource_id', 22); + expect(result).toHaveProperty('datasource_type', 'table'); + expect(result).toHaveProperty('dashboards', dashboards); + expect(result).toHaveProperty('owners', owners); + expect(result).toHaveProperty('query_context'); + expect(JSON.parse(result.params).adhoc_filters).toEqual( + formDataFromSlice.adhoc_filters, + ); + }); +});