Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Saving Mixed Chart with dashboard filter applied breaks adhoc_filter_b #25877

Merged
merged 2 commits into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions superset-frontend/src/explore/actions/saveModalActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function saveSliceSuccess(data) {
return { type: SAVE_SLICE_SUCCESS, data };
}

const extractAddHocFiltersFromFormData = formDataToHandle =>
const extractAdhocFiltersFromFormData = formDataToHandle =>
Object.entries(formDataToHandle).reduce(
(acc, [key, value]) =>
ADHOC_FILTER_REGEX.test(key)
Expand All @@ -71,7 +71,7 @@ export const getSlicePayload = (
owners,
formDataFromSlice = {},
) => {
const adhocFilters = extractAddHocFiltersFromFormData(
const adhocFilters = extractAdhocFiltersFromFormData(
formDataWithNativeFilters,
);

Expand All @@ -80,10 +80,17 @@ export const getSlicePayload = (
// 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)) {
formDataFromSlice?.adhoc_filters?.forEach(filter => {
if (filter.operator === Operators.TEMPORAL_RANGE && !filter.isExtra) {
adhocFilters.adhoc_filters.push({ ...filter, comparator: 'No filter' });
if (!isEmpty(formDataFromSlice)) {
Object.keys(adhocFilters || {}).forEach(adhocFilterKey => {
if (isEmpty(adhocFilters[adhocFilterKey])) {
formDataFromSlice?.[adhocFilterKey]?.forEach(filter => {
if (filter.operator === Operators.TEMPORAL_RANGE && !filter.isExtra) {
adhocFilters[adhocFilterKey].push({
...filter,
comparator: 'No filter',
});
}
});
}
});
}
Expand Down
53 changes: 53 additions & 0 deletions superset-frontend/src/explore/actions/saveModalActions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -391,4 +391,57 @@ describe('getSlicePayload', () => {
formDataFromSlice.adhoc_filters,
);
});

test('should return the correct payload when formDataWithNativeFilters has a filter with isExtra set to true in mixed chart', () => {
const formDataFromSliceWithAdhocFilterB = {
...formDataFromSlice,
adhoc_filters_b: [
{
clause: 'WHERE',
subject: 'year',
operator: 'TEMPORAL_RANGE',
comparator: 'No filter',
expressionType: 'SIMPLE',
},
],
};
const formDataWithAdhocFiltersWithExtra = {
...formDataWithNativeFilters,
viz_type: 'mixed_timeseries',
adhoc_filters: [
{
clause: 'WHERE',
subject: 'year',
operator: 'TEMPORAL_RANGE',
comparator: 'No filter',
expressionType: 'SIMPLE',
isExtra: true,
},
],
adhoc_filters_b: [
{
clause: 'WHERE',
subject: 'year',
operator: 'TEMPORAL_RANGE',
comparator: 'No filter',
expressionType: 'SIMPLE',
isExtra: true,
},
],
};
const result = getSlicePayload(
sliceName,
formDataWithAdhocFiltersWithExtra,
dashboards,
owners,
formDataFromSliceWithAdhocFilterB,
);

expect(JSON.parse(result.params).adhoc_filters).toEqual(
formDataFromSliceWithAdhocFilterB.adhoc_filters,
);
expect(JSON.parse(result.params).adhoc_filters_b).toEqual(
formDataFromSliceWithAdhocFilterB.adhoc_filters_b,
);
});
});
Loading