fix: inability to remove chart filter when dashboard time filter is a…

…pplied (#25217)
apache · Sep 12, 2023 · a9512c1 · a9512c1
1 parent 242921b
commit a9512c1
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 20 deletions.
diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js
@@ -59,14 +59,19 @@ const extractAddHocFiltersFromFormData = formDataToHandle =>
     {},
   );
 
+const hasTemporalRangeFilter = formData =>
+  (formData?.adhoc_filters || []).some(
+    filter => filter.operator === Operators.TEMPORAL_RANGE,
+  );
+
 export const getSlicePayload = (
   sliceName,
   formDataWithNativeFilters,
   dashboards,
   owners,
   formDataFromSlice = {},
 ) => {
-  let adhocFilters = extractAddHocFiltersFromFormData(
+  const adhocFilters = extractAddHocFiltersFromFormData(
     formDataWithNativeFilters,
   );
 
@@ -76,18 +81,24 @@ export const getSlicePayload = (
   // 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);
+    formDataFromSlice?.adhoc_filters?.forEach(filter => {
+      if (filter.operator === Operators.TEMPORAL_RANGE && !filter.isExtra) {
+        adhocFilters.adhoc_filters.push({ ...filter, comparator: 'No filter' });
+      }
+    });
   }
 
   // This loop iterates through the adhoc_filters array in formDataWithNativeFilters.
   // If a filter is of type TEMPORAL_RANGE and isExtra, it sets its comparator to
   // 'No filter' and adds the modified filter to the adhocFilters array. This ensures that all
   // TEMPORAL_RANGE filters are converted to 'No filter' when saving a chart.
-  formDataWithNativeFilters?.adhoc_filters?.forEach(filter => {
-    if (filter.operator === Operators.TEMPORAL_RANGE && filter.isExtra) {
-      adhocFilters.adhoc_filters.push({ ...filter, comparator: 'No filter' });
-    }
-  });
+  if (!hasTemporalRangeFilter(adhocFilters)) {
+    formDataWithNativeFilters?.adhoc_filters?.forEach(filter => {
+      if (filter.operator === Operators.TEMPORAL_RANGE && filter.isExtra) {
+        adhocFilters.adhoc_filters.push({ ...filter, comparator: 'No filter' });
+      }
+    });
+  }
 
   const formData = {
     ...formDataWithNativeFilters,

diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js
@@ -302,19 +302,7 @@ describe('getSlicePayload', () => {
       formDataWithNativeFilters,
       dashboards,
       owners,
-      {
-        datasource: '22__table',
-        viz_type: 'pie',
-        adhoc_filters: [
-          {
-            clause: 'WHERE',
-            subject: 'year',
-            operator: 'TEMPORAL_RANGE',
-            comparator: 'No filter',
-            expressionType: 'SIMPLE',
-          },
-        ],
-      },
+      formDataFromSlice,
     );
     expect(result).toHaveProperty('params');
     expect(result).toHaveProperty('slice_name', sliceName);