From 2cb13e695e25c2fc07e6f58e8a8eab8e38a9defd Mon Sep 17 00:00:00 2001
From: simcha90 <56388545+simcha90@users.noreply.github.com>
Date: Sun, 4 Jul 2021 11:26:59 +0300
Subject: [PATCH] fix(native-filters): Fix native filters config modal (#15506)

* fix:fix get permission function

* fix: native filters

* fix: remove unneccesary space fo filters / fix some crashes
---
 .../components/gridComponents/ChartHolder.jsx |   2 +-
 .../FilterBar/FilterControls/FilterValue.tsx  |  10 +-
 .../FiltersConfigForm/DefaultValue.tsx        |   6 ++
 .../FiltersConfigForm/FiltersConfigForm.tsx   |  56 +++++-----
 .../GroupBy/GroupByFilterPlugin.tsx           |   7 +-
 .../components/Range/RangeFilterPlugin.tsx    |   7 +-
 .../components/Select/SelectFilterPlugin.tsx  | 102 ++++++++++--------
 .../TimeColumn/TimeColumnFilterPlugin.tsx     |   7 +-
 .../TimeGrain/TimeGrainFilterPlugin.tsx       |   7 +-
 .../src/filters/components/common.ts          |   7 ++
 10 files changed, 120 insertions(+), 91 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx
index 23365b70ff..23f46f292d 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.jsx
@@ -132,7 +132,7 @@ const FilterFocusHighlight = React.forwardRef(
 
     if (focusedNativeFilterId) {
       if (
-        nativeFilters.filters[focusedNativeFilterId].chartsInScope?.includes(
+        nativeFilters.filters[focusedNativeFilterId]?.chartsInScope?.includes(
           chartId,
         )
       ) {
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
index 1bd8cd4d48..f833e9e737 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@@ -44,6 +44,7 @@ import { FilterProps } from './types';
 import { getFormData } from '../../utils';
 import { useCascadingFilters } from './state';
 import { usePreselectNativeFilter } from '../../state';
+import { checkIsMissingRequiredValue } from '../utils';
 
 const HEIGHT = 32;
 
@@ -197,7 +198,14 @@ const FilterValue: React.FC<FilterProps> = ({
       />
     );
   }
-  const filterState = { ...filter.dataMask?.filterState };
+  const isMissingRequiredValue = checkIsMissingRequiredValue(
+    filter,
+    filter.dataMask?.filterState,
+  );
+  const filterState = {
+    ...filter.dataMask?.filterState,
+    validateMessage: isMissingRequiredValue && t('Value is required'),
+  };
   if (filterState.value === undefined && preselection) {
     filterState.value = preselection;
   }
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
index 0b22dc704b..85d55807b2 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx
@@ -22,6 +22,7 @@ import {
   SetDataMaskHook,
   SuperChart,
   AppSection,
+  t,
 } from '@superset-ui/core';
 import { FormInstance } from 'antd/lib/form';
 import Loading from 'src/components/Loading';
@@ -56,6 +57,10 @@ const DefaultValue: FC<DefaultValueProps> = ({
       setLoading(true);
     }
   }, [hasDataset, queriesData]);
+  const value = formFilter.defaultDataMask?.filterState.value;
+  const isMissingRequiredValue =
+    (value === null || value === undefined) &&
+    formFilter?.controlValues?.enableEmptyFilter;
   return loading ? (
     <Loading position="inline-centered" />
   ) : (
@@ -74,6 +79,7 @@ const DefaultValue: FC<DefaultValueProps> = ({
       enableNoResults={enableNoResults}
       filterState={{
         ...formFilter.defaultDataMask?.filterState,
+        validateMessage: isMissingRequiredValue && t('Value is required'),
       }}
     />
   );
diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
index 5e95d1689a..306f06bb32 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -345,13 +345,27 @@ const FiltersConfigForm = (
   const hasDataset = !!nativeFilterItems[formFilter?.filterType]?.value
     ?.datasourceCount;
 
+  const { controlItems = {}, mainControlItems = {} } = formFilter
+    ? getControlItemsMap({
+        disabled: false,
+        forceUpdate,
+        form,
+        filterId,
+        filterType: formFilter.filterType,
+        filterToEdit,
+        formFilter,
+        removed,
+      })
+    : {};
+  const hasColumn = !!mainControlItems.groupby;
+
   const nativeFilterItem = nativeFilterItems[formFilter?.filterType] ?? {};
   // @ts-ignore
   const enableNoResults = !!nativeFilterItem.value?.enableNoResults;
   const datasetId = formFilter?.dataset?.value;
 
   useEffect(() => {
-    if (datasetId && hasDataset) {
+    if (datasetId && hasColumn) {
       cachedSupersetGet({
         endpoint: `/api/v1/dataset/${datasetId}`,
       })
@@ -367,7 +381,7 @@ const FiltersConfigForm = (
           addDangerToast(response.message);
         });
     }
-  }, [datasetId, hasDataset]);
+  }, [datasetId, hasColumn]);
 
   useImperativeHandle(ref, () => ({
     changeTab(tab: 'configuration' | 'scoping') {
@@ -375,10 +389,10 @@ const FiltersConfigForm = (
     },
   }));
 
-  const hasMetrics = hasDataset && !!metrics.length;
+  const hasMetrics = hasColumn && !!metrics.length;
 
   const hasFilledDataset =
-    !hasDataset || (datasetId && (formFilter?.column || !hasDataset));
+    !hasDataset || (datasetId && (formFilter?.column || !hasColumn));
 
   const hasAdditionalFilters = FILTERS_WITH_ADHOC_FILTERS.includes(
     formFilter?.filterType,
@@ -477,7 +491,7 @@ const FiltersConfigForm = (
       : undefined);
   const newFormData = getFormData({
     datasetId,
-    groupby: hasDataset ? formFilter?.column : undefined,
+    groupby: hasColumn ? formFilter?.column : undefined,
     ...formFilter,
   });
 
@@ -534,20 +548,10 @@ const FiltersConfigForm = (
   const hasSorting =
     typeof filterToEdit?.controlValues?.sortAscending === 'boolean';
 
-  const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset);
-
-  const { controlItems = {}, mainControlItems = {} } = formFilter
-    ? getControlItemsMap({
-        disabled: false,
-        forceUpdate,
-        form,
-        filterId,
-        filterType: formFilter.filterType,
-        filterToEdit,
-        formFilter,
-        removed,
-      })
-    : {};
+  const showDefaultValue =
+    !hasDataset ||
+    (!isDataDirty && hasFilledDataset) ||
+    !mainControlItems.groupby;
 
   const onSortChanged = (value: boolean | undefined) => {
     const previous = form.getFieldValue('filters')?.[filterId].controlValues;
@@ -683,6 +687,7 @@ const FiltersConfigForm = (
                 setNativeFilterFieldValues(form, filterId, {
                   filterType: value,
                   defaultDataMask: null,
+                  column: null,
                 });
                 forceUpdate();
               }}
@@ -738,13 +743,6 @@ const FiltersConfigForm = (
             header={FilterPanels.basic.name}
             key={FilterPanels.basic.key}
           >
-            {hasFilledDataset && (
-              <CleanFormItem
-                name={['filters', filterId, 'defaultValueFormData']}
-                hidden
-                initialValue={newFormData}
-              />
-            )}
             <CleanFormItem
               name={['filters', filterId, 'defaultValueQueriesData']}
               hidden
@@ -760,7 +758,11 @@ const FiltersConfigForm = (
             >
               <StyledRowSubFormItem
                 name={['filters', filterId, 'defaultDataMask']}
-                initialValue={filterToEdit?.defaultDataMask}
+                initialValue={
+                  formFilter.filterType === filterToEdit?.filterType
+                    ? filterToEdit?.defaultDataMask
+                    : null
+                }
                 data-test="default-input"
                 label={<StyledLabel>{t('Default Value')}</StyledLabel>}
                 required={hasDefaultValue}
diff --git a/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx b/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx
index 11739c8a12..981a9280e4 100644
--- a/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx
@@ -19,9 +19,8 @@
 import { ensureIsArray, ExtraFormData, styled, t, tn } from '@superset-ui/core';
 import React, { useEffect, useState } from 'react';
 import { Select } from 'src/common/components';
-import { Styles, StyledSelect } from '../common';
+import { Styles, StyledSelect, StyledFormItem } from '../common';
 import { PluginFilterGroupByProps } from './types';
-import FormItem from '../../../components/Form/FormItem';
 
 const { Option } = Select;
 
@@ -87,7 +86,7 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) {
       : tn('%s option', '%s options', columns.length, columns.length);
   return (
     <Styles height={height} width={width}>
-      <FormItem
+      <StyledFormItem
         validateStatus={filterState.validateMessage && 'error'}
         extra={<Error>{filterState.validateMessage}</Error>}
       >
@@ -116,7 +115,7 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) {
             },
           )}
         </StyledSelect>
-      </FormItem>
+      </StyledFormItem>
     </Styles>
   );
 }
diff --git a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx
index f510a6275f..f742b971dd 100644
--- a/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx
@@ -26,9 +26,8 @@ import React, { useEffect, useState } from 'react';
 import { Slider } from 'src/common/components';
 import { rgba } from 'emotion-rgba';
 import { PluginFilterRangeProps } from './types';
-import { Styles } from '../common';
+import { StyledFormItem, Styles } from '../common';
 import { getRangeExtraFormData } from '../../utils';
-import FormItem from '../../../components/Form/FormItem';
 
 const Error = styled.div`
   color: ${({ theme }) => theme.colors.error.base};
@@ -159,7 +158,7 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
       {Number.isNaN(Number(min)) || Number.isNaN(Number(max)) ? (
         <h4>{t('Chosen non-numeric column')}</h4>
       ) : (
-        <FormItem
+        <StyledFormItem
           validateStatus={filterState.validateMessage && 'error'}
           extra={<Error>{filterState.validateMessage}</Error>}
         >
@@ -183,7 +182,7 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
               marks={marks}
             />
           </Wrapper>
-        </FormItem>
+        </StyledFormItem>
       )}
     </Styles>
   );
diff --git a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
index 7e4ffaaef9..9957e5ace7 100644
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@@ -26,6 +26,7 @@ import {
   GenericDataType,
   JsonObject,
   smartDateDetailedFormatter,
+  styled,
   t,
   tn,
 } from '@superset-ui/core';
@@ -44,11 +45,15 @@ import { useImmerReducer } from 'use-immer';
 import Icons from 'src/components/Icons';
 import { usePrevious } from 'src/common/hooks/usePrevious';
 import { PluginFilterSelectProps, SelectValue } from './types';
-import { StyledSelect, Styles } from '../common';
+import { StyledFormItem, StyledSelect, Styles } from '../common';
 import { getDataRecordFormatter, getSelectExtraFormData } from '../../utils';
 
 const { Option } = Select;
 
+const Error = styled.div`
+  color: ${({ theme }) => theme.colors.error.base};
+`;
+
 type DataMaskAction =
   | { type: 'ownState'; ownState: JsonObject }
   | {
@@ -273,52 +278,57 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
 
   return (
     <Styles height={height} width={width}>
-      <StyledSelect
-        allowClear
-        // @ts-ignore
-        value={filterState.value || []}
-        disabled={isDisabled}
-        showSearch={showSearch}
-        mode={multiSelect ? 'multiple' : undefined}
-        placeholder={placeholderText}
-        onSearch={searchWrapper}
-        onSelect={clearSuggestionSearch}
-        onBlur={handleBlur}
-        onDropdownVisibleChange={setIsDropdownVisible}
-        dropdownRender={(
-          originNode: ReactElement & { ref?: RefObject<HTMLElement> },
-        ) => {
-          if (isDropdownVisible && !wasDropdownVisible) {
-            originNode.ref?.current?.scrollTo({ top: 0 });
-          }
-          return originNode;
-        }}
-        onFocus={setFocusedFilter}
-        // @ts-ignore
-        onChange={handleChange}
-        ref={inputRef}
-        loading={isRefreshing}
-        maxTagCount={5}
-        menuItemSelectedIcon={<Icon iconSize="m" />}
+      <StyledFormItem
+        validateStatus={filterState.validateMessage && 'error'}
+        extra={<Error>{filterState.validateMessage}</Error>}
       >
-        {sortedData.map(row => {
-          const [value] = groupby.map(col => row[col]);
-          return (
-            // @ts-ignore
-            <Option key={`${value}`} value={value}>
-              {labelFormatter(value, datatype)}
-            </Option>
-          );
-        })}
-        {currentSuggestionSearch &&
-          !ensureIsArray(filterState.value).some(
-            suggestion => suggestion === currentSuggestionSearch,
-          ) && (
-            <Option value={currentSuggestionSearch}>
-              {`${t('Create "%s"', currentSuggestionSearch)}`}
-            </Option>
-          )}
-      </StyledSelect>
+        <StyledSelect
+          allowClear
+          // @ts-ignore
+          value={filterState.value || []}
+          disabled={isDisabled}
+          showSearch={showSearch}
+          mode={multiSelect ? 'multiple' : undefined}
+          placeholder={placeholderText}
+          onSearch={searchWrapper}
+          onSelect={clearSuggestionSearch}
+          onBlur={handleBlur}
+          onDropdownVisibleChange={setIsDropdownVisible}
+          dropdownRender={(
+            originNode: ReactElement & { ref?: RefObject<HTMLElement> },
+          ) => {
+            if (isDropdownVisible && !wasDropdownVisible) {
+              originNode.ref?.current?.scrollTo({ top: 0 });
+            }
+            return originNode;
+          }}
+          onFocus={setFocusedFilter}
+          // @ts-ignore
+          onChange={handleChange}
+          ref={inputRef}
+          loading={isRefreshing}
+          maxTagCount={5}
+          menuItemSelectedIcon={<Icon iconSize="m" />}
+        >
+          {sortedData.map(row => {
+            const [value] = groupby.map(col => row[col]);
+            return (
+              // @ts-ignore
+              <Option key={`${value}`} value={value}>
+                {labelFormatter(value, datatype)}
+              </Option>
+            );
+          })}
+          {currentSuggestionSearch &&
+            !ensureIsArray(filterState.value).some(
+              suggestion => suggestion === currentSuggestionSearch,
+            ) && (
+              <Option value={currentSuggestionSearch}>
+                {`${t('Create "%s"', currentSuggestionSearch)}`}
+              </Option>
+            )}
+        </StyledSelect>
+      </StyledFormItem>
     </Styles>
   );
 }
diff --git a/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx
index 7ea97edeb0..48629da0ab 100644
--- a/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx
@@ -26,9 +26,8 @@ import {
 } from '@superset-ui/core';
 import React, { useEffect, useState } from 'react';
 import { Select } from 'src/common/components';
-import { Styles, StyledSelect } from '../common';
+import { Styles, StyledSelect, StyledFormItem } from '../common';
 import { PluginFilterTimeColumnProps } from './types';
-import FormItem from '../../../components/Form/FormItem';
 
 const { Option } = Select;
 
@@ -89,7 +88,7 @@ export default function PluginFilterTimeColumn(
       : tn('%s option', '%s options', timeColumns.length, timeColumns.length);
   return (
     <Styles height={height} width={width}>
-      <FormItem
+      <StyledFormItem
         validateStatus={filterState.validateMessage && 'error'}
         extra={<Error>{filterState.validateMessage}</Error>}
       >
@@ -117,7 +116,7 @@ export default function PluginFilterTimeColumn(
             },
           )}
         </StyledSelect>
-      </FormItem>
+      </StyledFormItem>
     </Styles>
   );
 }
diff --git a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx b/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx
index a2c4a185ad..d02e7acc9c 100644
--- a/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx
@@ -26,9 +26,8 @@ import {
 } from '@superset-ui/core';
 import React, { useEffect, useMemo, useState } from 'react';
 import { Select } from 'src/common/components';
-import { Styles, StyledSelect } from '../common';
+import { Styles, StyledSelect, StyledFormItem } from '../common';
 import { PluginFilterTimeGrainProps } from './types';
-import FormItem from '../../../components/Form/FormItem';
 
 const { Option } = Select;
 
@@ -99,7 +98,7 @@ export default function PluginFilterTimegrain(
       : tn('%s option', '%s options', data.length, data.length);
   return (
     <Styles height={height} width={width}>
-      <FormItem
+      <StyledFormItem
         validateStatus={filterState.validateMessage && 'error'}
         extra={<Error>{filterState.validateMessage}</Error>}
       >
@@ -122,7 +121,7 @@ export default function PluginFilterTimegrain(
             );
           })}
         </StyledSelect>
-      </FormItem>
+      </StyledFormItem>
     </Styles>
   );
 }
diff --git a/superset-frontend/src/filters/components/common.ts b/superset-frontend/src/filters/components/common.ts
index 5d0046e7ad..1efafacc57 100644
--- a/superset-frontend/src/filters/components/common.ts
+++ b/superset-frontend/src/filters/components/common.ts
@@ -19,6 +19,7 @@
 import { styled } from '@superset-ui/core';
 import { Select } from 'src/common/components';
 import { PluginFilterStylesProps } from './types';
+import FormItem from '../../components/Form/FormItem';
 
 export const Styles = styled.div<PluginFilterStylesProps>`
   min-height: ${({ height }) => height}px;
@@ -28,3 +29,9 @@ export const Styles = styled.div<PluginFilterStylesProps>`
 export const StyledSelect = styled(Select)`
   width: 100%;
 `;
+
+export const StyledFormItem = styled(FormItem)`
+  &.ant-row.ant-form-item {
+    margin: 0;
+  }
+`;