refactor(native-filters): Move filtersState to dataMask redux root (

#13437)
apache · Mar 7, 2021 · 45972e9 · 45972e9
1 parent d0714a0
commit 45972e9
Show file tree

Hide file tree

Showing 33 changed files with 428 additions and 323 deletions.
diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json
diff --git a/superset-frontend/package.json b/superset-frontend/package.json
@@ -112,6 +112,7 @@
     "geolib": "^2.0.24",
     "global-box": "^1.2.0",
     "html-webpack-plugin": "^4.5.1",
+    "immer": "^8.0.1",
     "immutable": "^4.0.0-rc.12",
     "interweave": "^11.2.0",
     "jquery": "^3.5.1",

diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts
@@ -17,6 +17,7 @@
  * under the License.
  */
 import { NativeFiltersState } from 'src/dashboard/reducers/types';
+import { DataMaskStateWithId } from '../../src/dataMask/types';
 
 export const nativeFilters: NativeFiltersState = {
   filterSets: {},
@@ -72,33 +73,34 @@ export const nativeFilters: NativeFiltersState = {
       isInstant: true,
     },
   },
-  filtersState: {
-    crossFilters: {},
-    ownFilters: {},
-    nativeFilters: {
-      'NATIVE_FILTER-e7Q8zKixx': {
-        id: 'NATIVE_FILTER-e7Q8zKixx',
-        extraFormData: {
-          append_form_data: {
-            filters: [
-              {
-                col: 'region',
-                op: 'IN',
-                val: ['East Asia & Pacific'],
-              },
-            ],
-          },
-        },
-        currentState: {
-          value: ['East Asia & Pacific'],
+};
+
+export const dataMaskWith2Filters: DataMaskStateWithId = {
+  crossFilters: {},
+  ownFilters: {},
+  nativeFilters: {
+    'NATIVE_FILTER-e7Q8zKixx': {
+      id: 'NATIVE_FILTER-e7Q8zKixx',
+      extraFormData: {
+        append_form_data: {
+          filters: [
+            {
+              col: 'region',
+              op: 'IN',
+              val: ['East Asia & Pacific'],
+            },
+          ],
         },
       },
-      'NATIVE_FILTER-x9QPw0so1': {
-        id: 'NATIVE_FILTER-x9QPw0so1',
-        extraFormData: {},
-        currentState: {},
+      currentState: {
+        value: ['East Asia & Pacific'],
       },
     },
+    'NATIVE_FILTER-x9QPw0so1': {
+      id: 'NATIVE_FILTER-x9QPw0so1',
+      extraFormData: {},
+      currentState: {},
+    },
   },
 };
 
@@ -132,14 +134,15 @@ export const singleNativeFiltersState = {
       isRequired: false,
     },
   },
-  filtersState: {
-    nativeFilters: {
-      [NATIVE_FILTER_ID]: {
-        id: NATIVE_FILTER_ID,
-        extraFormData,
-        currentState: {
-          value: ['No, not an ethnic minority'],
-        },
+};
+
+export const dataMaskWith1Filter = {
+  nativeFilters: {
+    [NATIVE_FILTER_ID]: {
+      id: NATIVE_FILTER_ID,
+      extraFormData,
+      currentState: {
+        value: ['No, not an ethnic minority'],
       },
     },
   },

diff --git a/superset-frontend/spec/fixtures/mockState.js b/superset-frontend/spec/fixtures/mockState.js
@@ -18,7 +18,10 @@
  */
 import datasources from 'spec/fixtures/mockDatasource';
 import messageToasts from 'spec/javascripts/messageToasts/mockMessageToasts';
-import { nativeFiltersInfo } from 'spec/javascripts/dashboard/fixtures/mockNativeFilters';
+import {
+  nativeFiltersInfo,
+  mockDataMaskInfo,
+} from 'spec/javascripts/dashboard/fixtures/mockNativeFilters';
 import chartQueries from './mockChartQueries';
 import { dashboardLayout } from './mockDashboardLayout';
 import dashboardInfo from './mockDashboardInfo';
@@ -31,6 +34,7 @@ export default {
   sliceEntities: sliceEntitiesForChart,
   charts: chartQueries,
   nativeFilters: nativeFiltersInfo,
+  dataMask: mockDataMaskInfo,
   dashboardInfo,
   dashboardFilters: emptyFilters,
   dashboardState,

diff --git a/superset-frontend/spec/fixtures/mockStore.js b/superset-frontend/spec/fixtures/mockStore.js
@@ -28,7 +28,7 @@ import {
 } from './mockDashboardLayout';
 import { sliceId } from './mockChartQueries';
 import { dashboardFilters } from './mockDashboardFilters';
-import { nativeFilters } from './mockNativeFilters';
+import { nativeFilters, dataMaskWith2Filters } from './mockNativeFilters';
 
 export const storeWithState = state =>
   createStore(rootReducer, state, compose(applyMiddleware(thunk)));
@@ -77,6 +77,7 @@ export const getMockStoreWithFilters = () =>
   createStore(rootReducer, {
     ...mockState,
     dashboardFilters,
+    dataMask: dataMaskWith2Filters,
     charts: {
       ...mockState.charts,
       [sliceIdWithAppliedFilter]: {
@@ -102,6 +103,7 @@ export const getMockStoreWithNativeFilters = () =>
   createStore(rootReducer, {
     ...mockState,
     nativeFilters,
+    dataMask: dataMaskWith2Filters,
     charts: {
       ...mockState.charts,
       [sliceIdWithAppliedFilter]: {

diff --git a/superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/Dashboard_spec.jsx
@@ -33,6 +33,7 @@ import {
   NATIVE_FILTER_ID,
   layoutForSingleNativeFilter,
   singleNativeFiltersState,
+  dataMaskWith1Filter,
 } from 'spec/fixtures/mockNativeFilters';
 import dashboardInfo from 'spec/fixtures/mockDashboardInfo';
 import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout';
@@ -154,7 +155,8 @@ describe('Dashboard', () => {
         activeFilters: {
           ...OVERRIDE_FILTERS,
           ...getActiveNativeFilters({
-            nativeFilters: singleNativeFiltersState,
+            dataMask: dataMaskWith1Filter,
+            filters: singleNativeFiltersState.filters,
             layout: layoutForSingleNativeFilter,
           }),
         },

diff --git a/...s/dashboard/fixtures/mockNativeFilters.js → ...s/dashboard/fixtures/mockNativeFilters.ts b/...s/dashboard/fixtures/mockNativeFilters.js → ...s/dashboard/fixtures/mockNativeFilters.ts
@@ -16,12 +16,17 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-export const nativeFiltersInfo = {
+import { DataMaskStateWithId, DataMaskType } from 'src/dataMask/types';
+import { NativeFiltersState } from 'src/dashboard/reducers/types';
+
+export const nativeFiltersInfo: NativeFiltersState = {
+  filterSets: {},
   filters: {
     DefaultsID: {
+      cascadeParentIds: [],
       id: 'DefaultsID',
       name: 'test',
-      type: 'filter_select',
+      filterType: 'filter_select',
       targets: [
         {
           datasetId: 0,
@@ -37,17 +42,22 @@ export const nativeFiltersInfo = {
         excluded: [],
       },
       isInstant: true,
-      allowsMultipleValues: true,
-      isRequired: false,
+      controlValues: {
+        allowsMultipleValues: true,
+        isRequired: false,
+      },
     },
   },
-  filtersState: {
-    nativeFilters: {
-      DefaultsID: {
-        id: 'DefaultId',
-        currentState: {
-          value: [],
-        },
+};
+
+export const mockDataMaskInfo: DataMaskStateWithId = {
+  [DataMaskType.CrossFilters]: {},
+  [DataMaskType.OwnFilters]: {},
+  [DataMaskType.NativeFilters]: {
+    DefaultsID: {
+      id: 'DefaultId',
+      currentState: {
+        value: [],
       },
     },
   },

diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts
@@ -61,15 +61,15 @@ describe('getFormDataWithExtraFilters', () => {
           },
         } as unknown) as Filter,
       },
-      filtersState: {
-        crossFilters: {},
-        ownFilters: {},
-        nativeFilters: {
-          [filterId]: {
-            id: filterId,
-            extraFormData: {},
-            currentState: {},
-          },
+    },
+    dataMask: {
+      crossFilters: {},
+      ownFilters: {},
+      nativeFilters: {
+        [filterId]: {
+          id: filterId,
+          extraFormData: {},
+          currentState: {},
         },
       },
     },

diff --git a/superset-frontend/src/chart/ChartContainer.jsx b/superset-frontend/src/chart/ChartContainer.jsx
@@ -22,14 +22,14 @@ import { bindActionCreators } from 'redux';
 import * as actions from './chartAction';
 import { logEvent } from '../logger/actions';
 import Chart from './Chart';
-import { updateExtraFormData } from '../dashboard/actions/nativeFilters';
+import { updateDataMask } from '../dataMask/actions';
 
 function mapDispatchToProps(dispatch) {
   return {
     actions: bindActionCreators(
       {
         ...actions,
-        updateExtraFormData,
+        updateDataMask,
         logEvent,
       },
       dispatch,

diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx
@@ -75,11 +75,8 @@ class ChartRenderer extends React.Component {
       setControlValue: this.handleSetControlValue,
       onFilterMenuOpen: this.props.onFilterMenuOpen,
       onFilterMenuClose: this.props.onFilterMenuClose,
-      setDataMask: filtersState => {
-        this.props.actions?.updateExtraFormData(
-          this.props.chartId,
-          filtersState,
-        );
+      setDataMask: dataMask => {
+        this.props.actions?.updateDataMask(this.props.chartId, dataMask);
       },
     };
   }
@@ -97,6 +94,7 @@ class ChartRenderer extends React.Component {
       return (
         this.hasQueryResponseChange ||
         nextProps.annotationData !== this.props.annotationData ||
+        nextProps.ownCurrentState !== this.props.ownCurrentState ||
         nextProps.height !== this.props.height ||
         nextProps.width !== this.props.width ||
         nextProps.triggerRender ||

diff --git a/superset-frontend/src/chart/chartAction.js b/superset-frontend/src/chart/chartAction.js
@@ -41,7 +41,7 @@ import { logEvent } from '../logger/actions';
 import { Logger, LOG_ACTIONS_LOAD_CHART } from '../logger/LogUtils';
 import { getClientErrorObject } from '../utils/getClientErrorObject';
 import { allowCrossDomain as domainShardingEnabled } from '../utils/hostNamesConfig';
-import { updateExtraFormData } from '../dashboard/actions/nativeFilters';
+import { updateDataMask } from '../dataMask/actions';
 
 export const CHART_UPDATE_STARTED = 'CHART_UPDATE_STARTED';
 export function chartUpdateStarted(queryController, latestQueryFormData, key) {
@@ -366,8 +366,8 @@ export function exploreJSON(
     };
     if (dashboardId) requestParams.dashboard_id = dashboardId;
 
-    const setDataMask = filtersState => {
-      dispatch(updateExtraFormData(formData.slice_id, filtersState));
+    const setDataMask = dataMask => {
+      dispatch(updateDataMask(formData.slice_id, dataMask));
     };
     const chartDataRequest = getChartDataRequest({
       setDataMask,