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

kgabryje · web-flow · commit 268c1dcdad60 · 2023-11-06T19:52:27.000+01:00
diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js
@@ -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)
@@ -71,7 +71,7 @@ export const getSlicePayload = (
   owners,
   formDataFromSlice = {},
 ) => {
-  const adhocFilters = extractAddHocFiltersFromFormData(
+  const adhocFilters = extractAdhocFiltersFromFormData(
     formDataWithNativeFilters,
   );
 
@@ -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',
+            });
+          }
+        });
       }
     });
   }
diff --git a/superset-frontend/src/explore/actions/saveModalActions.test.js b/superset-frontend/src/explore/actions/saveModalActions.test.js
@@ -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,
+    );
+  });
 });
