fix(native-filters): Fix required filters (#15572)

* fix:fix get permission function

* fix: filters required state

* fix: fix CR notes

* fix: removre required message

* fix: fix validation state

superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx

@@ -204,7 +204,7 @@ const FilterValue: React.FC<FilterProps> = ({
   );
   const filterState = {
     ...filter.dataMask?.filterState,
-    validateMessage: isMissingRequiredValue && t('Value is required'),
+    validateStatus: isMissingRequiredValue && 'error',
   };
   if (filterState.value === undefined && preselection) {
     filterState.value = preselection;

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DefaultValue.tsx

@@ -30,6 +30,7 @@ import { NativeFiltersForm } from '../types';
 import { getFormData } from '../../utils';
 
 type DefaultValueProps = {
+  hasDefaultValue: boolean;
   filterId: string;
   setDataMask: SetDataMaskHook;
   hasDataset: boolean;
@@ -39,6 +40,7 @@ type DefaultValueProps = {
 };
 
 const DefaultValue: FC<DefaultValueProps> = ({
+  hasDefaultValue,
   filterId,
   hasDataset,
   form,
@@ -59,8 +61,7 @@ const DefaultValue: FC<DefaultValueProps> = ({
   }, [hasDataset, queriesData]);
   const value = formFilter.defaultDataMask?.filterState.value;
   const isMissingRequiredValue =
-    (value === null || value === undefined) &&
-    formFilter?.controlValues?.enableEmptyFilter;
+    hasDefaultValue && (value === null || value === undefined);
   return loading ? (
     <Loading position="inline-centered" />
   ) : (
@@ -80,6 +81,7 @@ const DefaultValue: FC<DefaultValueProps> = ({
       filterState={{
         ...formFilter.defaultDataMask?.filterState,
         validateMessage: isMissingRequiredValue && t('Value is required'),
+        validateStatus: isMissingRequiredValue && 'error',
       }}
     />
   );

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx

@@ -756,64 +756,69 @@ const FiltersConfigForm = (
               checked={hasDefaultValue}
               onChange={value => setHasDefaultValue(value)}
             >
-              <StyledRowSubFormItem
-                name={['filters', filterId, 'defaultDataMask']}
-                initialValue={
-                  formFilter.filterType === filterToEdit?.filterType
-                    ? filterToEdit?.defaultDataMask
-                    : null
-                }
-                data-test="default-input"
-                label={<StyledLabel>{t('Default Value')}</StyledLabel>}
-                required={hasDefaultValue}
-                rules={[
-                  {
-                    validator: (rule, value) => {
-                      const hasValue = !!value?.filterState?.value;
-                      if (hasValue) {
-                        return Promise.resolve();
-                      }
-                      return Promise.reject(
-                        new Error(t('Default value is required')),
-                      );
+              {formFilter.filterType && (
+                <StyledRowSubFormItem
+                  name={['filters', filterId, 'defaultDataMask']}
+                  initialValue={
+                    formFilter.filterType === filterToEdit?.filterType
+                      ? filterToEdit?.defaultDataMask
+                      : null
+                  }
+                  data-test="default-input"
+                  label={<StyledLabel>{t('Default Value')}</StyledLabel>}
+                  required={hasDefaultValue}
+                  rules={[
+                    {
+                      validator: (rule, value) => {
+                        const hasValue =
+                          value?.filterState?.value !== null &&
+                          value?.filterState?.value !== undefined;
+                        if (hasValue) {
+                          return Promise.resolve();
+                        }
+                        return Promise.reject(
+                          new Error(t('Default value is required')),
+                        );
+                      },
                     },
-                  },
-                ]}
-              >
-                {error ? (
-                  <BasicErrorAlert
-                    title={t('Cannot load filter')}
-                    body={error}
-                    level="error"
-                  />
-                ) : showDefaultValue ? (
-                  <DefaultValueContainer>
-                    <DefaultValue
-                      setDataMask={dataMask => {
-                        setNativeFilterFieldValues(form, filterId, {
-                          defaultDataMask: dataMask,
-                        });
-                        form.validateFields([
-                          ['filters', filterId, 'defaultDataMask'],
-                        ]);
-                        forceUpdate();
-                      }}
-                      filterId={filterId}
-                      hasDataset={hasDataset}
-                      form={form}
-                      formData={newFormData}
-                      enableNoResults={enableNoResults}
+                  ]}
+                >
+                  {error ? (
+                    <BasicErrorAlert
+                      title={t('Cannot load filter')}
+                      body={error}
+                      level="error"
                     />
-                    {hasDataset && datasetId && (
-                      <Tooltip title={t('Refresh the default values')}>
-                        <RefreshIcon onClick={() => refreshHandler(true)} />
-                      </Tooltip>
-                    )}
-                  </DefaultValueContainer>
-                ) : (
-                  t('Fill all required fields to enable "Default Value"')
-                )}
-              </StyledRowSubFormItem>
+                  ) : showDefaultValue ? (
+                    <DefaultValueContainer>
+                      <DefaultValue
+                        setDataMask={dataMask => {
+                          setNativeFilterFieldValues(form, filterId, {
+                            defaultDataMask: dataMask,
+                          });
+                          form.validateFields([
+                            ['filters', filterId, 'defaultDataMask'],
+                          ]);
+                          forceUpdate();
+                        }}
+                        hasDefaultValue={hasDefaultValue}
+                        filterId={filterId}
+                        hasDataset={hasDataset}
+                        form={form}
+                        formData={newFormData}
+                        enableNoResults={enableNoResults}
+                      />
+                      {hasDataset && datasetId && (
+                        <Tooltip title={t('Refresh the default values')}>
+                          <RefreshIcon onClick={() => refreshHandler(true)} />
+                        </Tooltip>
+                      )}
+                    </DefaultValueContainer>
+                  ) : (
+                    t('Fill all required fields to enable "Default Value"')
+                  )}
+                </StyledRowSubFormItem>
+              )}
             </CollapsibleControl>
             {Object.keys(controlItems)
               .filter(key => BASIC_CONTROL_ITEMS.includes(key))

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts

@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { useCallback, useEffect, useState } from 'react';
+import { useEffect, useState } from 'react';
 import { FormInstance } from 'antd/lib/form';
 import { t } from '@superset-ui/core';
 import { NativeFiltersForm, NativeFiltersFormItem } from '../types';
@@ -52,27 +52,19 @@ export const useBackendFormUpdate = (
 export const useDefaultValue = (
   formFilter?: NativeFiltersFormItem,
   filterToEdit?: Filter,
-) => {
-  const [hasDefaultValue, setHasPartialDefaultValue] = useState(
-    !!filterToEdit?.defaultDataMask?.filterState?.value,
-  );
-  const [isRequired, setisRequired] = useState(
-    formFilter?.controlValues?.enableEmptyFilter,
-  );
+): [boolean, boolean, string, Function] => {
+  const enableEmptyFilter = !!formFilter?.controlValues?.enableEmptyFilter;
+  const defaultToFirstItem = !!formFilter?.controlValues?.defaultToFirstItem;
 
+  const [hasDefaultValue, setHasPartialDefaultValue] = useState(false);
+  const [isRequired, setIsRequired] = useState(enableEmptyFilter);
   const [defaultValueTooltip, setDefaultValueTooltip] = useState('');
 
-  const defaultToFirstItem = formFilter?.controlValues?.defaultToFirstItem;
-
-  const setHasDefaultValue = useCallback(
-    (value?) => {
-      const required =
-        !!formFilter?.controlValues?.enableEmptyFilter && !defaultToFirstItem;
-      setisRequired(required);
-      setHasPartialDefaultValue(required ? true : value);
-    },
-    [formFilter?.controlValues?.enableEmptyFilter, defaultToFirstItem],
-  );
+  const setHasDefaultValue = (value = false) => {
+    const required = enableEmptyFilter && !defaultToFirstItem;
+    setIsRequired(required);
+    setHasPartialDefaultValue(required ? true : value);
+  };
 
   useEffect(() => {
     setHasDefaultValue(
@@ -80,7 +72,16 @@ export const useDefaultValue = (
         ? false
         : !!formFilter?.defaultDataMask?.filterState?.value,
     );
-  }, [setHasDefaultValue, defaultToFirstItem]);
+    // TODO: this logic should be unhardcoded
+  }, [defaultToFirstItem, enableEmptyFilter]);
+
+  useEffect(() => {
+    setHasDefaultValue(
+      defaultToFirstItem
+        ? false
+        : !!filterToEdit?.defaultDataMask?.filterState?.value,
+    );
+  }, []);
 
   useEffect(() => {
     let tooltip = '';

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

@@ -164,16 +164,6 @@ export function FiltersConfigModal({
     addFilter,
   );
 
-  // After this, it should be as if the modal was just opened fresh.
-  // Called when the modal is closed.
-  const resetForm = () => {
-    form.resetFields();
-    setNewFilterIds([]);
-    setCurrentFilterId(initialCurrentFilterId);
-    setRemovedFilters({});
-    setSaveAlertVisible(false);
-  };
-
   const getFilterTitle = (id: string) =>
     formValues.filters[id]?.name ??
     filterConfigMap[id]?.name ??
@@ -209,7 +199,6 @@ export function FiltersConfigModal({
         filterConfigMap,
         filterIds,
         removedFilters,
-        resetForm,
         onSave,
         values,
       )();
@@ -219,7 +208,6 @@ export function FiltersConfigModal({
   };
 
   const handleConfirmCancel = () => {
-    resetForm();
     onCancel();
   };
 

superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts

@@ -104,7 +104,6 @@ export const createHandleSave = (
   filterConfigMap: Record<string, Filter>,
   filterIds: string[],
   removedFilters: Record<string, FilterRemoval>,
-  resetForm: Function,
   saveForm: Function,
   values: NativeFiltersForm,
 ) => async () => {
@@ -145,7 +144,6 @@ export const createHandleSave = (
     });
 
   await saveForm(newFilterConfig);
-  resetForm();
 };
 
 export const createHandleTabEdit = (

superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx

@@ -16,18 +16,15 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { ensureIsArray, ExtraFormData, styled, t, tn } from '@superset-ui/core';
+import { ensureIsArray, ExtraFormData, t, tn } from '@superset-ui/core';
 import React, { useEffect, useState } from 'react';
 import { Select } from 'src/common/components';
-import { Styles, StyledSelect, StyledFormItem } from '../common';
+import { FormItemProps } from 'antd/lib/form';
+import { Styles, StyledSelect, StyledFormItem, StatusMessage } from '../common';
 import { PluginFilterGroupByProps } from './types';
 
 const { Option } = Select;
 
-const Error = styled.div`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
 export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) {
   const {
     data,
@@ -84,11 +81,20 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) {
     columns.length === 0
       ? t('No columns')
       : tn('%s option', '%s options', columns.length, columns.length);
+
+  const formItemData: FormItemProps = {};
+  if (filterState.validateMessage) {
+    formItemData.extra = (
+      <StatusMessage status={filterState.validateStatus}>
+        {filterState.validateMessage}
+      </StatusMessage>
+    );
+  }
   return (
     <Styles height={height} width={width}>
       <StyledFormItem
-        validateStatus={filterState.validateMessage && 'error'}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        validateStatus={filterState.validateStatus}
+        {...formItemData}
       >
         <StyledSelect
           allowClear

superset-frontend/src/filters/components/Range/RangeFilterPlugin.tsx

@@ -25,47 +25,44 @@ import {
 import React, { useEffect, useState } from 'react';
 import { Slider } from 'src/common/components';
 import { rgba } from 'emotion-rgba';
+import { FormItemProps } from 'antd/lib/form';
 import { PluginFilterRangeProps } from './types';
-import { StyledFormItem, Styles } from '../common';
+import { StatusMessage, StyledFormItem, Styles } from '../common';
 import { getRangeExtraFormData } from '../../utils';
 
-const Error = styled.div`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
-const Wrapper = styled.div<{ validateStatus?: string }>`
+const Wrapper = styled.div<{ validateStatus?: 'error' | 'warning' | 'info' }>`
   border: 1px solid transparent;
   &:focus {
     border: 1px solid
       ${({ theme, validateStatus }) =>
-        theme.colors[validateStatus ? 'error' : 'primary'].base};
+        theme.colors[validateStatus || 'primary']?.base};
     outline: 0;
     box-shadow: 0 0 0 3px
       ${({ theme, validateStatus }) =>
-        rgba(theme.colors[validateStatus ? 'error' : 'primary'].base, 0.2)};
+        rgba(theme.colors[validateStatus || 'primary']?.base, 0.2)};
   }
   & .ant-slider {
     & .ant-slider-track {
       background-color: ${({ theme, validateStatus }) =>
-        validateStatus && theme.colors.error.light1};
+        validateStatus && theme.colors[validateStatus]?.light1};
     }
     & .ant-slider-handle {
       border: ${({ theme, validateStatus }) =>
-        validateStatus && `2px solid ${theme.colors.error.light1}`};
+        validateStatus && `2px solid ${theme.colors[validateStatus]?.light1}`};
       &:focus {
         box-shadow: 0 0 0 3px
           ${({ theme, validateStatus }) =>
-            rgba(theme.colors[validateStatus ? 'error' : 'primary'].base, 0.2)};
+            rgba(theme.colors[validateStatus || 'primary']?.base, 0.2)};
       }
     }
     &:hover {
       & .ant-slider-track {
         background-color: ${({ theme, validateStatus }) =>
-          validateStatus && theme.colors.error.base};
+          validateStatus && theme.colors[validateStatus]?.base};
       }
       & .ant-slider-handle {
         border: ${({ theme, validateStatus }) =>
-          validateStatus && `2px solid ${theme.colors.error.base}`};
+          validateStatus && `2px solid ${theme.colors[validateStatus]?.base}`};
       }
     }
   }
@@ -150,22 +147,31 @@ export default function RangeFilterPlugin(props: PluginFilterRangeProps) {
   };
 
   useEffect(() => {
+    // when switch filter type and queriesData still not updated we need ignore this case (in FilterBar)
+    if (row?.min === undefined && row?.max === undefined) {
+      return;
+    }
     handleAfterChange(filterState.value ?? [min, max]);
-  }, [JSON.stringify(filterState.value)]);
+  }, [JSON.stringify(filterState.value), JSON.stringify(data)]);
 
+  const formItemData: FormItemProps = {};
+  if (filterState.validateMessage) {
+    formItemData.extra = (
+      <StatusMessage status={filterState.validateStatus}>
+        {filterState.validateMessage}
+      </StatusMessage>
+    );
+  }
   return (
     <Styles height={height} width={width}>
       {Number.isNaN(Number(min)) || Number.isNaN(Number(max)) ? (
         <h4>{t('Chosen non-numeric column')}</h4>
       ) : (
-        <StyledFormItem
-          validateStatus={filterState.validateMessage && 'error'}
-          extra={<Error>{filterState.validateMessage}</Error>}
-        >
+        <StyledFormItem {...formItemData}>
           <Wrapper
             tabIndex={-1}
             ref={inputRef}
-            validateStatus={filterState.validateMessage}
+            validateStatus={filterState.validateStatus}
             onFocus={setFocusedFilter}
             onBlur={unsetFocusedFilter}
             onMouseEnter={setFocusedFilter}

superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx

@@ -26,7 +26,6 @@ import {
   GenericDataType,
   JsonObject,
   smartDateDetailedFormatter,
-  styled,
   t,
   tn,
 } from '@superset-ui/core';
@@ -44,16 +43,13 @@ import { SLOW_DEBOUNCE } from 'src/constants';
 import { useImmerReducer } from 'use-immer';
 import Icons from 'src/components/Icons';
 import { usePrevious } from 'src/common/hooks/usePrevious';
+import { FormItemProps } from 'antd/lib/form';
 import { PluginFilterSelectProps, SelectValue } from './types';
-import { StyledFormItem, StyledSelect, Styles } from '../common';
+import { StyledFormItem, StyledSelect, Styles, StatusMessage } 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 }
   | {
@@ -152,6 +148,7 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
           inverseSelection,
         ),
         filterState: {
+          ...filterState,
           label: values?.length
             ? `${(values || []).join(', ')}${suffix}`
             : undefined,
@@ -276,11 +273,20 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
       : tn('%s option', '%s options', data.length, data.length);
   const Icon = inverseSelection ? Icons.StopOutlined : Icons.CheckOutlined;
 
+  const formItemData: FormItemProps = {};
+  if (filterState.validateMessage) {
+    formItemData.extra = (
+      <StatusMessage status={filterState.validateStatus}>
+        {filterState.validateMessage}
+      </StatusMessage>
+    );
+  }
+
   return (
     <Styles height={height} width={width}>
       <StyledFormItem
-        validateStatus={filterState.validateMessage && 'error'}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        validateStatus={filterState.validateStatus}
+        {...formItemData}
       >
         <StyledSelect
           allowClear

superset-frontend/src/filters/components/Time/TimeFilterPlugin.tsx

@@ -27,19 +27,23 @@ const TimeFilterStyles = styled(Styles)`
   overflow-x: auto;
 `;
 
-const ControlContainer = styled.div<{ validateStatus?: string }>`
+const ControlContainer = styled.div<{
+  validateStatus?: 'error' | 'warning' | 'info';
+}>`
   padding: 2px;
   & > span {
     border: 2px solid transparent;
     display: inline-block;
     border: ${({ theme, validateStatus }) =>
-      validateStatus && `2px solid ${theme.colors.error.base}`};
+      validateStatus && `2px solid ${theme.colors[validateStatus]?.base}`};
   }
   &:focus {
     & > span {
       border: 2px solid
         ${({ theme, validateStatus }) =>
-          validateStatus ? theme.colors.error.base : theme.colors.primary.base};
+          validateStatus
+            ? theme.colors[validateStatus]?.base
+            : theme.colors.primary.base};
       outline: 0;
       box-shadow: 0 0 0 2px
         ${({ validateStatus }) =>
@@ -85,7 +89,7 @@ export default function TimeFilterPlugin(props: PluginFilterTimeProps) {
       <ControlContainer
         tabIndex={-1}
         ref={inputRef}
-        validateStatus={filterState.validateMessage}
+        validateStatus={filterState.validateStatus}
         onFocus={setFocusedFilter}
         onBlur={unsetFocusedFilter}
         onMouseEnter={setFocusedFilter}

superset-frontend/src/filters/components/TimeColumn/TimeColumnFilterPlugin.tsx

@@ -20,21 +20,17 @@ import {
   ensureIsArray,
   ExtraFormData,
   GenericDataType,
-  styled,
   t,
   tn,
 } from '@superset-ui/core';
 import React, { useEffect, useState } from 'react';
 import { Select } from 'src/common/components';
-import { Styles, StyledSelect, StyledFormItem } from '../common';
+import { FormItemProps } from 'antd/lib/form';
+import { Styles, StyledSelect, StyledFormItem, StatusMessage } from '../common';
 import { PluginFilterTimeColumnProps } from './types';
 
 const { Option } = Select;
 
-const Error = styled.div`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
 export default function PluginFilterTimeColumn(
   props: PluginFilterTimeColumnProps,
 ) {
@@ -86,11 +82,20 @@ export default function PluginFilterTimeColumn(
     timeColumns.length === 0
       ? t('No time columns')
       : tn('%s option', '%s options', timeColumns.length, timeColumns.length);
+
+  const formItemData: FormItemProps = {};
+  if (filterState.validateMessage) {
+    formItemData.extra = (
+      <StatusMessage status={filterState.validateStatus}>
+        {filterState.validateMessage}
+      </StatusMessage>
+    );
+  }
   return (
     <Styles height={height} width={width}>
       <StyledFormItem
-        validateStatus={filterState.validateMessage && 'error'}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        validateStatus={filterState.validateStatus}
+        {...formItemData}
       >
         <StyledSelect
           allowClear

superset-frontend/src/filters/components/TimeGrain/TimeGrainFilterPlugin.tsx

@@ -19,22 +19,18 @@
 import {
   ensureIsArray,
   ExtraFormData,
-  styled,
   t,
   TimeGranularity,
   tn,
 } from '@superset-ui/core';
 import React, { useEffect, useMemo, useState } from 'react';
 import { Select } from 'src/common/components';
-import { Styles, StyledSelect, StyledFormItem } from '../common';
+import { FormItemProps } from 'antd/lib/form';
+import { Styles, StyledSelect, StyledFormItem, StatusMessage } from '../common';
 import { PluginFilterTimeGrainProps } from './types';
 
 const { Option } = Select;
 
-const Error = styled.div`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
 export default function PluginFilterTimegrain(
   props: PluginFilterTimeGrainProps,
 ) {
@@ -96,11 +92,20 @@ export default function PluginFilterTimegrain(
     (data || []).length === 0
       ? t('No data')
       : tn('%s option', '%s options', data.length, data.length);
+
+  const formItemData: FormItemProps = {};
+  if (filterState.validateMessage) {
+    formItemData.extra = (
+      <StatusMessage status={filterState.validateStatus}>
+        {filterState.validateMessage}
+      </StatusMessage>
+    );
+  }
   return (
     <Styles height={height} width={width}>
       <StyledFormItem
-        validateStatus={filterState.validateMessage && 'error'}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        validateStatus={filterState.validateStatus}
+        {...formItemData}
       >
         <StyledSelect
           allowClear

superset-frontend/src/filters/components/common.ts

@@ -35,3 +35,9 @@ export const StyledFormItem = styled(FormItem)`
     margin: 0;
   }
 `;
+
+export const StatusMessage = styled.div<{
+  status?: 'error' | 'warning' | 'info';
+}>`
+  color: ${({ theme, status = 'error' }) => theme.colors[status]?.base};
+`;