From 2178a675ad4adbb95546f26fd67c131d7084a1c6 Mon Sep 17 00:00:00 2001
From: Lily Kuang <lily@preset.io>
Date: Wed, 6 Sep 2023 17:35:03 -0700
Subject: [PATCH 1/2] fix: Inability to Remove Chart Filter When Dashboard Time
 Filter is Applied

---
 .../src/explore/actions/saveModalActions.js   | 25 ++++++++++++++-----
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js
index 8c864eceacd2d..57ca9aa306017 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.js
+++ b/superset-frontend/src/explore/actions/saveModalActions.js
@@ -59,6 +59,13 @@ const extractAddHocFiltersFromFormData = formDataToHandle =>
     {},
   );
 
+const hasTemporalRangeFilter = formData => {
+  const { adhoc_filters = [] } = formData;
+  return adhoc_filters.some(
+    filter => filter.operator === Operators.TEMPORAL_RANGE,
+  );
+};
+
 export const getSlicePayload = (
   sliceName,
   formDataWithNativeFilters,
@@ -76,18 +83,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,

From 56094627c957edfdec740075b34e28b95946a6ba Mon Sep 17 00:00:00 2001
From: Lily Kuang <lily@preset.io>
Date: Wed, 6 Sep 2023 18:43:07 -0700
Subject: [PATCH 2/2] fix lint and test

---
 .../src/explore/actions/saveModalActions.js        | 10 ++++------
 .../src/explore/actions/saveModalActions.test.js   | 14 +-------------
 2 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js
index 57ca9aa306017..0bf7ced475e44 100644
--- a/superset-frontend/src/explore/actions/saveModalActions.js
+++ b/superset-frontend/src/explore/actions/saveModalActions.js
@@ -59,12 +59,10 @@ const extractAddHocFiltersFromFormData = formDataToHandle =>
     {},
   );
 
-const hasTemporalRangeFilter = formData => {
-  const { adhoc_filters = [] } = formData;
-  return adhoc_filters.some(
+const hasTemporalRangeFilter = formData =>
+  (formData?.adhoc_filters || []).some(
     filter => filter.operator === Operators.TEMPORAL_RANGE,
   );
-};
 
 export const getSlicePayload = (
   sliceName,
@@ -73,7 +71,7 @@ export const getSlicePayload = (
   owners,
   formDataFromSlice = {},
 ) => {
-  let adhocFilters = extractAddHocFiltersFromFormData(
+  const adhocFilters = extractAddHocFiltersFromFormData(
     formDataWithNativeFilters,
   );
 
@@ -84,7 +82,7 @@ export const getSlicePayload = (
   // 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) {
+      if (filter.operator === Operators.TEMPORAL_RANGE && !filter.isExtra) {
         adhocFilters.adhoc_filters.push({ ...filter, comparator: 'No filter' });
       }
     });
diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js
index dedddfb4cd725..4c0da5132dd70 100644
--- 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);