refactor: Sync Scoping tree with Forms data

superset-frontend/spec/javascripts/dashboard/components/nativeFilters/ScopingTree_spec.tsx

@@ -21,20 +21,40 @@ import { Provider } from 'react-redux';
 import ScopingTree from 'src/dashboard/components/nativeFilters/ScopingTree';
 import { styledMount as mount } from 'spec/helpers/theming';
 import { mockStore } from 'spec/fixtures/mockStore';
+import { FormInstance } from 'src/common/components';
+import { NativeFiltersForm } from 'src/dashboard/components/nativeFilters/types';
+import { getDefaultScopeValue } from 'src/dashboard/components/nativeFilters/FilterScope';
 
 describe('ScopingTree', () => {
-  const mock = jest.fn();
+  const filterId = '1';
+  const form = {
+    getFieldValue: () => ({
+      [filterId]: {
+        scope: getDefaultScopeValue(),
+      },
+    }),
+  };
   const wrapper = mount(
     <Provider store={mockStore}>
-      <ScopingTree setFilterScope={mock} />
+      <ScopingTree
+        filterId={filterId}
+        initialScope={getDefaultScopeValue()}
+        form={(form as unknown) as FormInstance<NativeFiltersForm>}
+      />
     </Provider>,
   );
   it('is valid', () => {
-    const mock = () => null;
-    expect(React.isValidElement(<ScopingTree setFilterScope={mock} />)).toBe(
-      true,
-    );
+    expect(
+      React.isValidElement(
+        <ScopingTree
+          filterId={filterId}
+          initialScope={getDefaultScopeValue()}
+          form={(form as unknown) as FormInstance<NativeFiltersForm>}
+        />,
+      ),
+    ).toBe(true);
   });
+
   it('renders a tree', () => {
     expect(wrapper.find('TreeNode')).toExist();
   });

superset-frontend/src/common/components/index.tsx

@@ -47,6 +47,7 @@ export {
   Slider,
   Radio,
   Row,
+  Space,
   Select,
   Skeleton,
   Switch,

superset-frontend/src/dashboard/components/nativeFilters/FilterConfigForm.tsx

@@ -24,7 +24,6 @@ import {
   Checkbox,
   Form,
   Input,
-  Radio,
   Typography,
 } from 'src/common/components';
 import { Select } from 'src/components/Select/SupersetStyledSelect';
@@ -34,8 +33,8 @@ import SupersetResourceSelect, {
 import { addDangerToast } from 'src/messageToasts/actions';
 import { ClientErrorObject } from 'src/utils/getClientErrorObject';
 import { ColumnSelect } from './ColumnSelect';
-import ScopingTree from './ScopingTree';
-import { Filter, NativeFiltersForm, Scoping } from './types';
+import { Filter, NativeFiltersForm } from './types';
+import FilterScope from './FilterScope';
 
 type DatasetSelectValue = {
   value: number;
@@ -47,10 +46,6 @@ const datasetToSelectOption = (item: any): DatasetSelectValue => ({
   label: item.table_name,
 });
 
-const ScopingTreeNote = styled.div`
-  margin-bottom: ${({ theme }) => theme.gridUnit * 2}px;
-`;
-
 const RemovedContent = styled.div`
   display: flex;
   flex-direction: column;
@@ -103,9 +98,6 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
   form,
   parentFilters,
 }) => {
-  const [advancedScopingOpen, setAdvancedScopingOpen] = useState<Scoping>(
-    Scoping.all,
-  );
   const [dataset, setDataset] = useState<Value<number> | undefined>();
 
   const onDatasetSelectError = useCallback(
@@ -119,13 +111,6 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
     [],
   );
 
-  const setFilterScope = useCallback(
-    value => {
-      form.setFields([{ name: ['filters', filterId, 'scope'], value }]);
-    },
-    [form, filterId],
-  );
-
   if (removed) {
     return (
       <RemovedContent>
@@ -190,14 +175,6 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
           datasetId={dataset?.value}
         />
       </StyledFormItem>
-
-      <StyledFormItem
-        name={['filters', filterId, 'defaultValue']}
-        label={<StyledLabel>{t('Default Value')}</StyledLabel>}
-        initialValue={filterToEdit?.defaultValue}
-      >
-        <Input />
-      </StyledFormItem>
       <StyledFormItem
         name={['filters', filterId, 'parentFilter']}
         label={<StyledLabel>{t('Parent Filter')}</StyledLabel>}
@@ -244,36 +221,11 @@ export const FilterConfigForm: React.FC<FilterConfigFormProps> = ({
       >
         <Checkbox>{t('Required')}</Checkbox>
       </StyledCheckboxFormItem>
-      <Typography.Title level={5}>{t('Scoping')}</Typography.Title>
-      <StyledCheckboxFormItem
-        name={['filters', filterId, 'scoping']}
-        initialValue={advancedScopingOpen}
-      >
-        <Radio.Group
-          onChange={({ target: { value } }) => {
-            setAdvancedScopingOpen(value as Scoping);
-          }}
-        >
-          <Radio value={Scoping.all}>{t('Apply to all panels')}</Radio>
-          <Radio value={Scoping.specific}>
-            {t('Apply to specific panels')}
-          </Radio>
-        </Radio.Group>
-      </StyledCheckboxFormItem>
-      <>
-        <ScopingTreeNote>
-          <Typography.Text type="secondary">
-            {advancedScopingOpen === Scoping.specific
-              ? t('Only selected panels will be affected by this filter')
-              : t(
-                  'All panels with this column will be affected by this filter',
-                )}
-          </Typography.Text>
-        </ScopingTreeNote>
-        {advancedScopingOpen === Scoping.specific && (
-          <ScopingTree setFilterScope={setFilterScope} />
-        )}
-      </>
+      <FilterScope
+        filterId={filterId}
+        filterToEdit={filterToEdit}
+        form={form}
+      />
     </>
   );
 };

superset-frontend/src/dashboard/components/nativeFilters/FilterConfigModal.tsx

@@ -25,7 +25,6 @@ import { Form } from 'src/common/components';
 import { StyledModal } from 'src/common/components/Modal';
 import Button from 'src/components/Button';
 import { LineEditableTabs } from 'src/common/components/Tabs';
-import { DASHBOARD_ROOT_ID } from 'src/dashboard/util/constants';
 import { usePrevious } from 'src/common/hooks/usePrevious';
 import ErrorBoundary from 'src/components/ErrorBoundary';
 import { useFilterConfigMap, useFilterConfiguration } from './state';
@@ -396,10 +395,7 @@ export function FilterConfigModal({
           cascadeParentIds: formInputs.parentFilter
             ? [formInputs.parentFilter.value]
             : [],
-          scope: {
-            rootPath: [DASHBOARD_ROOT_ID],
-            excluded: [],
-          },
+          scope: formInputs.scope,
           inverseSelection: !!formInputs.inverseSelection,
           isInstant: !!formInputs.isInstant,
           allowsMultipleValues: !!formInputs.allowsMultipleValues,

superset-frontend/src/dashboard/components/nativeFilters/FilterScope.tsx

@@ -0,0 +1,105 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { FC } from 'react';
+import { t, styled } from '@superset-ui/core';
+import {
+  Form,
+  Radio,
+  Typography,
+  Space,
+  FormInstance,
+} from '../../../common/components';
+import { Filter, NativeFiltersForm, Scoping } from './types';
+import ScopingTree from './ScopingTree';
+import { DASHBOARD_ROOT_ID } from '../../util/constants';
+import { isScopingAll, setFilterFieldValues, useForceUpdate } from './utils';
+
+type FilterScopeProps = {
+  filterId: string;
+  filterToEdit?: Filter;
+  form: FormInstance<NativeFiltersForm>;
+};
+
+export const getDefaultScopeValue = () => ({
+  rootPath: [DASHBOARD_ROOT_ID],
+  excluded: [],
+});
+
+const CleanFormItem = styled(Form.Item)`
+  margin-bottom: 0;
+`;
+
+const FilterScope: FC<FilterScopeProps> = ({
+  filterId,
+  filterToEdit,
+  form,
+}) => {
+  const formFilter = form.getFieldValue('filters')?.[filterId];
+  const initialScope = filterToEdit?.scope || getDefaultScopeValue();
+
+  const scoping = isScopingAll(initialScope) ? Scoping.all : Scoping.specific;
+
+  const forceUpdate = useForceUpdate();
+
+  return (
+    <Space direction="vertical">
+      <CleanFormItem
+        name={['filters', filterId, 'scope']}
+        hidden
+        initialValue={initialScope}
+      />
+      <Typography.Title level={5}>{t('Scoping')}</Typography.Title>
+      <CleanFormItem
+        name={['filters', filterId, 'scoping']}
+        initialValue={scoping}
+      >
+        <Radio.Group
+          onChange={({ target: { value } }) => {
+            if (value === Scoping.all) {
+              setFilterFieldValues(form, filterId, {
+                scope: getDefaultScopeValue(),
+              });
+            }
+            forceUpdate();
+          }}
+        >
+          <Radio value={Scoping.all}>{t('Apply to all panels')}</Radio>
+          <Radio value={Scoping.specific}>
+            {t('Apply to specific panels')}
+          </Radio>
+        </Radio.Group>
+      </CleanFormItem>
+      <Typography.Text type="secondary">
+        {formFilter?.scoping === Scoping.specific
+          ? t('Only selected panels will be affected by this filter')
+          : t('All panels with this column will be affected by this filter')}
+      </Typography.Text>
+      {formFilter?.scoping === Scoping.specific && (
+        <ScopingTree
+          initialScope={initialScope}
+          form={form}
+          filterId={filterId}
+        />
+      )}
+    </Space>
+  );
+};
+
+export default FilterScope;

superset-frontend/src/dashboard/components/nativeFilters/ScopingTree.tsx

@@ -17,44 +17,64 @@
  * under the License.
  */
 
-import React, { FC, useState } from 'react';
-import { Tree } from 'src/common/components';
+import React, { FC, useMemo, useState } from 'react';
+import { FormInstance, Tree } from 'src/common/components';
 import { useFilterScopeTree } from './state';
 import { DASHBOARD_ROOT_ID } from '../../util/constants';
-import { findFilterScope } from './utils';
+import {
+  findFilterScope,
+  getTreeCheckedItems,
+  setFilterFieldValues,
+  useForceUpdate,
+} from './utils';
+import { NativeFiltersForm, Scope } from './types';
 
 type ScopingTreeProps = {
-  setFilterScope: Function;
+  form: FormInstance<NativeFiltersForm>;
+  filterId: string;
+  initialScope: Scope;
 };
 
-const ScopingTree: FC<ScopingTreeProps> = ({ setFilterScope }) => {
+const ScopingTree: FC<ScopingTreeProps> = ({
+  form,
+  filterId,
+  initialScope,
+}) => {
   const [expandedKeys, setExpandedKeys] = useState<string[]>([
     DASHBOARD_ROOT_ID,
   ]);
 
-  const { treeData, layout } = useFilterScopeTree();
+  const formFilter = form.getFieldValue('filters')[filterId];
 
+  const { treeData, layout } = useFilterScopeTree();
   const [autoExpandParent, setAutoExpandParent] = useState<boolean>(true);
-  const [checkedKeys, setCheckedKeys] = useState<string[]>([]);
 
-  const onExpand = (expandedKeys: string[]) => {
+  const handleExpand = (expandedKeys: string[]) => {
     setExpandedKeys(expandedKeys);
     setAutoExpandParent(false);
   };
-
-  const onCheck = (checkedKeys: string[]) => {
-    setCheckedKeys(checkedKeys);
-    setFilterScope(findFilterScope(checkedKeys, layout));
+  const forceUpdate = useForceUpdate();
+  const handleCheck = (checkedKeys: string[]) => {
+    forceUpdate();
+    setFilterFieldValues(form, filterId, {
+      scope: findFilterScope(checkedKeys, layout),
+    });
   };
 
+  const checkedKeys = useMemo(
+    () =>
+      getTreeCheckedItems({ ...(formFilter.scope || initialScope) }, layout),
+    [formFilter.scope, initialScope, layout],
+  );
+
   return (
     <Tree
       checkable
       selectable={false}
-      onExpand={onExpand}
+      onExpand={handleExpand}
       expandedKeys={expandedKeys}
       autoExpandParent={autoExpandParent}
-      onCheck={onCheck}
+      onCheck={handleCheck}
       checkedKeys={checkedKeys}
       treeData={treeData}
     />