You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by hu...@apache.org on 2023/04/10 22:40:36 UTC
[superset] 05/07: fix(charts): Time range filters are not being applied to charts that were overwritten (#23589)
This is an automated email from the ASF dual-hosted git repository.
hugh pushed a commit to branch 2023.13.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 83bda092b85066a9d710ad9b0fe340a89b5108f0
Author: Antonio Rivero <38...@users.noreply.github.com>
AuthorDate: Fri Apr 7 15:44:47 2023 -0400
fix(charts): Time range filters are not being applied to charts that were overwritten (#23589)
---
.../src/explore/actions/saveModalActions.js | 40 +++++--
.../src/explore/actions/saveModalActions.test.js | 122 +++++++++++++++++++++
2 files changed, 154 insertions(+), 8 deletions(-)
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 };
}
+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,
@@ -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,
+ );
+ });
+});