Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Commit

Permalink
fix(charts): Time range filters are not being applied to charts that …
Browse files Browse the repository at this point in the history 
…were overwritten (apache#23589)
  • Loading branch information
@Antonio-RiveroMartnez
Antonio-RiveroMartnez authored Apr 7, 2023
1 parent a823033 commit 1f3774d
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 8 deletions.
40 changes: 32 additions & 8 deletions superset-frontend/src/explore/actions/saveModalActions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/;
Expand Down Expand Up @@ -76,19 +77,35 @@ export function removeSaveModalAlert() {
return { type: REMOVE_SAVE_MODAL_ALERT };
}

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 = {},
) => {
const adhocFilters = Object.entries(formDataWithNativeFilters).reduce(
(acc, [key, value]) =>
ADHOC_FILTER_REGEX.test(key)
? { ...acc, [key]: value?.filter(f => !f.isExtra) }
: acc,
{},
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,
Expand Down Expand Up @@ -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 },
Expand All @@ -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());
Expand Down
122 changes: 122 additions & 0 deletions superset-frontend/src/explore/actions/saveModalActions.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
SAVE_SLICE_FAILED,
SAVE_SLICE_SUCCESS,
updateSlice,
getSlicePayload,
} from './saveModalActions';

/**
Expand Down Expand Up @@ -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,
);
});
});

0 comments on commit 1f3774d

Please sign in to comment.