feat(native-filters): Disable Apply button if filter required (#15222)

* fix:fix get permission function * fix: fix select first value by clear all * lint: fix lint * feat: disable select on missed value * fix: refactor ff * feat: connect nulls for line chart * lint: fix lint * docs: fix message * fix: fix CR comments * fix: fix Collapsed items * fix: fix Collapsed items
2021-06-17 16:35:33 +03:00 · 2021-06-17 16:35:33 +03:00 · 388eb01f06
parent 1269cc2f88
commit 388eb01f06
7 changed files with 180 additions and 78 deletions
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@ -43,6 +43,7 @@ import { ClientErrorObject } from 'src/utils/getClientErrorObject';
 import { FilterProps } from './types';
 import { getFormData } from '../../utils';
 import { useCascadingFilters } from './state';
+import { checkIsMissingRequiredValue } from '../utils';

 const FilterItem = styled.div`
  min-height: ${({ theme }) => theme.gridUnit * 11}px;
@ -181,6 +182,11 @@ const FilterValue: React.FC<FilterProps> = ({
    );
  }

+  const isMissingRequiredValue = checkIsMissingRequiredValue(
+    filter,
+    filter.dataMask?.filterState,
+  );
+
  return (
    <FilterItem data-test="form-item-value">
      {isLoading ? (
@ -194,7 +200,11 @@ const FilterValue: React.FC<FilterProps> = ({
          queriesData={hasDataSource ? state : [{ data: [{}] }]}
          chartType={filterType}
          behaviors={[Behavior.NATIVE_FILTER]}
-          filterState={filter.dataMask?.filterState}
+          filterState={{
+            ...filter.dataMask?.filterState,
+            validateMessage: isMissingRequiredValue && t('Value is required'),
+            validateStatus: isMissingRequiredValue && 'error',
+          }}
          ownState={filter.dataMask?.ownState}
          enableNoResults={metadata?.enableNoResults}
          isRefreshing={isRefreshing}
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
@ -29,12 +29,11 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import { updateDataMask } from 'src/dataMask/actions';
 import { DataMaskStateWithId, DataMaskWithId } from 'src/dataMask/types';
 import { useImmer } from 'use-immer';
-import { areObjectsEqual } from 'src/reduxUtils';
 import { testWithId } from 'src/utils/testUtils';
 import { Filter } from 'src/dashboard/components/nativeFilters/types';
 import Loading from 'src/components/Loading';
 import { getInitialDataMask } from 'src/dataMask/reducer';
-import { getOnlyExtraFormData, TabIds } from './utils';
+import { checkIsApplyDisabled, TabIds } from './utils';
 import FilterSets from './FilterSets';
 import {
  useNativeFiltersDataMask,
@ -214,16 +213,11 @@ const FilterBar: React.FC<FiltersBarProps> = ({
  };

  useFilterUpdates(dataMaskSelected, setDataMaskSelected);
-
-  const dataSelectedValues = Object.values(dataMaskSelected);
-  const dataAppliedValues = Object.values(dataMaskApplied);
-  const isApplyDisabled =
-    areObjectsEqual(
-      getOnlyExtraFormData(dataMaskSelected),
-      getOnlyExtraFormData(dataMaskApplied),
-      { ignoreUndefined: true },
-    ) || dataSelectedValues.length !== dataAppliedValues.length;
-
+  const isApplyDisabled = checkIsApplyDisabled(
+    dataMaskSelected,
+    dataMaskApplied,
+    filterValues,
+  );
  const isInitialized = useInitialization();

  return (
--- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/utils.ts
@ -18,6 +18,8 @@
 */

 import { DataMaskStateWithId } from 'src/dataMask/types';
+import { areObjectsEqual } from 'src/reduxUtils';
+import { FilterState } from '@superset-ui/core';
 import { Filter } from '../types';

 export enum TabIds {
@ -46,3 +48,39 @@ export const getOnlyExtraFormData = (data: DataMaskStateWithId) =>
    (prev, next) => ({ ...prev, [next.id]: next.extraFormData }),
    {},
  );
+
+export const checkIsMissingRequiredValue = (
+  filter: Filter,
+  filterState?: FilterState,
+) => {
+  const value = filterState?.value;
+  // TODO: this property should be unhardcoded
+  return (
+    filter.controlValues.enableEmptyFilter &&
+    (value === null || value === undefined)
+  );
+};
+
+export const checkIsApplyDisabled = (
+  dataMaskSelected: DataMaskStateWithId,
+  dataMaskApplied: DataMaskStateWithId,
+  filters: Filter[],
+) => {
+  const dataSelectedValues = Object.values(dataMaskSelected);
+  const dataAppliedValues = Object.values(dataMaskApplied);
+
+  return (
+    areObjectsEqual(
+      getOnlyExtraFormData(dataMaskSelected),
+      getOnlyExtraFormData(dataMaskApplied),
+      { ignoreUndefined: true },
+    ) ||
+    dataSelectedValues.length !== dataAppliedValues.length ||
+    filters.some(filter =>
+      checkIsMissingRequiredValue(
+        filter,
+        dataMaskSelected?.[filter?.id]?.filterState,
+      ),
+    )
+  );
+};
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/CollapsibleControl.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/CollapsibleControl.tsx
@ -16,11 +16,12 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import React, { ReactNode, useState } from 'react';
+import React, { ReactNode, useEffect, useState } from 'react';
 import { styled } from '@superset-ui/core';
 import { Checkbox } from 'src/common/components';

 interface CollapsibleControlProps {
+  initialValue?: boolean;
  checked?: boolean;
  title: string;
  children: ReactNode;
@ -45,8 +46,23 @@ const StyledContainer = styled.div<{ checked: boolean }>`
 `;

 const CollapsibleControl = (props: CollapsibleControlProps) => {
-  const { checked = false, title, children, onChange } = props;
-  const [isChecked, setIsChecked] = useState(checked);
+  const {
+    checked,
+    title,
+    children,
+    onChange = () => {},
+    initialValue = false,
+  } = props;
+  const [isChecked, setIsChecked] = useState(initialValue);
+
+  useEffect(() => {
+    // if external `checked` changed to `undefined`, it means that we work now in uncontrolled mode with local state
+    // and we need ignore external value
+    if (checked !== undefined) {
+      setIsChecked(checked);
+    }
+  }, [checked]);
+
  return (
    <StyledContainer checked={isChecked}>
      <Checkbox
@ -54,10 +70,12 @@ const CollapsibleControl = (props: CollapsibleControlProps) => {
        checked={isChecked}
        onChange={e => {
          const value = e.target.checked;
-          setIsChecked(value);
-          if (onChange) {
-            onChange(value);
+          // external `checked` value has more priority then local state
+          if (checked === undefined) {
+            // uncontrolled mode
+            setIsChecked(value);
          }
+          onChange(value);
        }}
      >
        {title}
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@ -69,7 +69,7 @@ import {
  setNativeFilterFieldValues,
  useForceUpdate,
 } from './utils';
-import { useBackendFormUpdate } from './state';
+import { useBackendFormUpdate, useDefaultValue } from './state';
 import { getFormData } from '../../utils';
 import { Filter } from '../../types';
 import getControlItemsMap from './getControlItemsMap';
@ -280,14 +280,13 @@ const FiltersConfigForm = (
  const [activeFilterPanelKey, setActiveFilterPanelKey] = useState<
    string | string[]
  >(FilterPanels.basic.key);
-  const [hasDefaultValue, setHasDefaultValue] = useState(
-    !!filterToEdit?.defaultDataMask?.filterState?.value,
-  );
+
  const forceUpdate = useForceUpdate();
  const [datasetDetails, setDatasetDetails] = useState<Record<string, any>>();
  const defaultFormFilter = useMemo(() => {}, []);
  const formFilter =
    form.getFieldValue('filters')?.[filterId] || defaultFormFilter;
+
  const nativeFilterItems = getChartMetadataRegistry().items;
  const nativeFilterVizTypes = Object.entries(nativeFilterItems)
    // @ts-ignore
@ -431,6 +430,11 @@ const FiltersConfigForm = (
    ...formFilter,
  });

+  const [hasDefaultValue, setHasDefaultValue] = useDefaultValue(
+    formFilter,
+    filterToEdit,
+  );
+
  useEffect(() => {
    if (hasDataset && hasFilledDataset && hasDefaultValue && isDataDirty) {
      refreshHandler();
@ -672,6 +676,7 @@ const FiltersConfigForm = (
            />
            <CollapsibleControl
              title={t('Filter has default value')}
+              initialValue={hasDefaultValue}
              checked={hasDefaultValue}
              onChange={value => setHasDefaultValue(value)}
            >
@ -680,18 +685,17 @@ const FiltersConfigForm = (
                initialValue={filterToEdit?.defaultDataMask}
                data-test="default-input"
                label={<StyledLabel>{t('Default Value')}</StyledLabel>}
-                required
+                required={formFilter?.controlValues?.enableEmptyFilter}
                rules={[
-                  {
-                    required: true,
-                  },
                  {
                    validator: (rule, value) => {
                      const hasValue = !!value?.filterState?.value;
                      if (
                        hasValue ||
                        // TODO: do more generic
-                        formFilter.controlValues?.defaultToFirstItem
+                        formFilter.controlValues?.defaultToFirstItem ||
+                        // Not marked as required
+                        !formFilter.controlValues?.enableEmptyFilter
                      ) {
                        return Promise.resolve();
                      }
@ -760,7 +764,7 @@ const FiltersConfigForm = (
              {isCascadingFilter && (
                <CollapsibleControl
                  title={t('Filter is hierarchical')}
-                  checked={hasParentFilter}
+                  initialValue={hasParentFilter}
                  onChange={checked => {
                    if (checked) {
                      // execute after render
@ -801,7 +805,7 @@ const FiltersConfigForm = (
              {hasDataset && hasAdditionalFilters && (
                <CollapsibleControl
                  title={t('Pre-filter available values')}
-                  checked={hasPreFilter}
+                  initialValue={hasPreFilter}
                  onChange={checked => {
                    if (checked) {
                      validatePreFilter();
@ -902,7 +906,7 @@ const FiltersConfigForm = (
                <CollapsibleControl
                  title={t('Sort filter values')}
                  onChange={checked => onSortChanged(checked || undefined)}
-                  checked={hasSorting}
+                  initialValue={hasSorting}
                >
                  <StyledFormItem
                    name={[
--- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/state.ts
@ -16,10 +16,11 @@
 * specific language governing permissions and limitations
 * under the License.
 */
-import { useEffect } from 'react';
+import { useCallback, useEffect, useState } from 'react';
 import { FormInstance } from 'antd/lib/form';
-import { NativeFiltersForm } from '../types';
+import { NativeFiltersForm, NativeFiltersFormItem } from '../types';
 import { setNativeFilterFieldValues, useForceUpdate } from './utils';
+import { Filter } from '../../types';

 // When some fields in form changed we need re-fetch data for Filter defaultValue
 // eslint-disable-next-line import/prefer-default-export
@ -46,3 +47,29 @@ export const useBackendFormUpdate = (
    filterId,
  ]);
 };
+
+export const useDefaultValue = (
+  formFilter?: NativeFiltersFormItem,
+  filterToEdit?: Filter,
+) => {
+  const [hasDefaultValue, setHasPartialDefaultValue] = useState(
+    !!filterToEdit?.defaultDataMask?.filterState?.value ||
+      formFilter?.controlValues?.enableEmptyFilter,
+  );
+  const setHasDefaultValue = useCallback(
+    (value?) => {
+      setHasPartialDefaultValue(
+        value || formFilter?.controlValues?.enableEmptyFilter
+          ? true
+          : undefined,
+      );
+    },
+    [formFilter?.controlValues?.enableEmptyFilter],
+  );
+
+  useEffect(() => {
+    setHasDefaultValue();
+  }, [setHasDefaultValue]);
+
+  return [hasDefaultValue, setHasDefaultValue];
+};
--- a/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
+++ b/superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
@ -26,9 +26,11 @@ import {
  GenericDataType,
  JsonObject,
  smartDateDetailedFormatter,
+  styled,
  t,
  tn,
 } from '@superset-ui/core';
+import { FormItem } from 'src/components/Form';
 import React, {
  RefObject,
  ReactElement,
@ -80,6 +82,10 @@ function reducer(
  }
 }

+const Error = styled.div`
+  color: ${({ theme }) => theme.colors.error.base};
+`;
+
 export default function PluginFilterSelect(props: PluginFilterSelectProps) {
  const {
    coltypeMap,
@ -273,52 +279,57 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {

  return (
    <Styles height={height} width={width}>
-      <StyledSelect
-        allowClear={!enableEmptyFilter}
-        // @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" />}
+      <FormItem
+        validateStatus={filterState.validateStatus}
+        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={!enableEmptyFilter}
+          // @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>
+      </FormItem>
    </Styles>
  );
 }