feat(dashboard): Add description to the native filter (#17025)

* Adding description works * Add some tests * Fix tests * Styled look good * Tests successful * Address PR comments * fix a test
apache · Oct 27, 2021 · 65f1644 · 65f1644
1 parent faf7c74
commit 65f1644
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 51 deletions.
diff --git a/superset-frontend/spec/fixtures/mockNativeFilters.ts b/superset-frontend/spec/fixtures/mockNativeFilters.ts
@@ -52,6 +52,7 @@ export const nativeFilters: NativeFiltersState = {
         inverseSelection: false,
       },
       type: NativeFilterType.NATIVE_FILTER,
+      description: '',
     },
     'NATIVE_FILTER-x9QPw0so1': {
       id: 'NATIVE_FILTER-x9QPw0so1',
@@ -81,6 +82,7 @@ export const nativeFilters: NativeFiltersState = {
         inverseSelection: false,
       },
       type: NativeFilterType.NATIVE_FILTER,
+      description: '2 letter code',
     },
   },
 };

diff --git a/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts b/superset-frontend/spec/javascripts/dashboard/fixtures/mockNativeFilters.ts
@@ -68,6 +68,7 @@ export const nativeFiltersInfo: NativeFiltersState = {
         isRequired: false,
       },
       type: NativeFilterType.NATIVE_FILTER,
+      description: 'test description',
     },
   },
 };
diff --git a/...nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/CascadeFilterControl.test.tsx b/...nativeFilters/FilterBar/CascadeFilters/CascadeFilterControl/CascadeFilterControl.test.tsx
@@ -17,27 +17,27 @@
  * under the License.
  */
 import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import { fireEvent, render, screen } from 'spec/helpers/testing-library';
 import { mockStore } from 'spec/fixtures/mockStore';
 import { Provider } from 'react-redux';
 import { nativeFiltersInfo } from 'spec/javascripts/dashboard/fixtures/mockNativeFilters';
 import CascadeFilterControl, { CascadeFilterControlProps } from '.';
 
-const mockedProps = {
+const createProps = (defaultsId = nativeFiltersInfo.filters.DefaultsID) => ({
   filter: {
-    ...nativeFiltersInfo.filters.DefaultsID,
+    ...defaultsId,
     cascadeChildren: [
       {
-        ...nativeFiltersInfo.filters.DefaultsID,
+        ...defaultsId,
         name: 'test child filter 1',
         cascadeChildren: [],
       },
       {
-        ...nativeFiltersInfo.filters.DefaultsID,
+        ...defaultsId,
         name: 'test child filter 2',
         cascadeChildren: [
           {
-            ...nativeFiltersInfo.filters.DefaultsID,
+            ...defaultsId,
             name: 'test child of a child filter',
             cascadeChildren: [],
           },
@@ -46,7 +46,7 @@ const mockedProps = {
     ],
   },
   onFilterSelectionChange: jest.fn(),
-};
+});
 
 const setup = (props: CascadeFilterControlProps) => (
   <Provider store={mockStore}>
@@ -55,22 +55,41 @@ const setup = (props: CascadeFilterControlProps) => (
 );
 
 test('should render', () => {
-  const { container } = render(setup(mockedProps));
+  const { container } = render(setup(createProps()));
   expect(container).toBeInTheDocument();
 });
 
 test('should render the filter name', () => {
-  render(setup(mockedProps));
+  render(setup(createProps()));
   expect(screen.getByText('test')).toBeInTheDocument();
 });
 
 test('should render the children filter names', () => {
-  render(setup(mockedProps));
+  render(setup(createProps()));
   expect(screen.getByText('test child filter 1')).toBeInTheDocument();
   expect(screen.getByText('test child filter 2')).toBeInTheDocument();
 });
 
 test('should render the child of a child filter name', () => {
-  render(setup(mockedProps));
+  render(setup(createProps()));
   expect(screen.getByText('test child of a child filter')).toBeInTheDocument();
 });
+
+test('should render tooltip if description is not empty', async () => {
+  render(setup(createProps()));
+  expect(screen.getByText('test')).toBeInTheDocument();
+  const toolTip = screen.getByText('test')?.parentElement?.querySelector('i');
+  expect(toolTip).not.toBe(null);
+  fireEvent.mouseOver(toolTip as HTMLElement);
+  expect(await screen.findByText('test description')).toBeInTheDocument();
+});
+
+test('should not render tooltip if description is empty', () => {
+  render(
+    setup(
+      createProps({ ...nativeFiltersInfo.filters.DefaultsID, description: '' }),
+    ),
+  );
+  const toolTip = screen.getByText('test')?.parentElement?.querySelector('i');
+  expect(toolTip).toBe(null);
+});
diff --git a/...rontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx b/...rontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl.tsx
@@ -17,11 +17,12 @@
  * under the License.
  */
 import React, { useMemo } from 'react';
-import { styled } from '@superset-ui/core';
-import { Form, FormItem } from 'src/components/Form';
+import { styled, SupersetTheme } from '@superset-ui/core';
+import { FormItem as StyledFormItem, Form } from 'src/components/Form';
+import { Tooltip } from 'src/components/Tooltip';
+import { checkIsMissingRequiredValue } from '../utils';
 import FilterValue from './FilterValue';
 import { FilterProps } from './types';
-import { checkIsMissingRequiredValue } from '../utils';
 
 const StyledIcon = styled.div`
   position: absolute;
@@ -50,8 +51,62 @@ const StyledFilterControlContainer = styled(Form)`
     width: 100%;
     padding-right: ${({ theme }) => theme.gridUnit * 11}px;
   }
+  .ant-form-item-tooltip {
+    margin-bottom: ${({ theme }) => theme.gridUnit}px;
+  }
+`;
+
+const FormItem = styled(StyledFormItem)`
+  .ant-form-item-label {
+    label.ant-form-item-required:not(.ant-form-item-required-mark-optional) {
+      &::after {
+        display: none;
+      }
+    }
+  }
 `;
 
+const ToolTipContainer = styled.div`
+  font-size: ${({ theme }) => theme.typography.sizes.m}px;
+  display: flex;
+`;
+
+const RequiredFieldIndicator = () => (
+  <span
+    css={(theme: SupersetTheme) => ({
+      color: theme.colors.error.base,
+      fontSize: `${theme.typography.sizes.s}px`,
+      paddingLeft: '1px',
+    })}
+  >
+    *
+  </span>
+);
+
+const DescriptionToolTip = ({ description }: { description: string }) => (
+  <ToolTipContainer>
+    <Tooltip
+      title={description}
+      placement="right"
+      overlayInnerStyle={{
+        display: '-webkit-box',
+        overflow: 'hidden',
+        WebkitLineClamp: 20,
+        WebkitBoxOrient: 'vertical',
+        textOverflow: 'ellipsis',
+      }}
+    >
+      <i
+        className="fa fa-info-circle text-muted"
+        css={(theme: SupersetTheme) => ({
+          paddingLeft: `${theme.gridUnit}px`,
+          cursor: 'pointer',
+        })}
+      />
+    </Tooltip>
+  </ToolTipContainer>
+);
+
 const FilterControl: React.FC<FilterProps> = ({
   dataMaskSelected,
   filter,
@@ -68,17 +123,22 @@ const FilterControl: React.FC<FilterProps> = ({
     filter,
     filter.dataMask?.filterState,
   );
+  const isRequired = !!filter.controlValues?.enableEmptyFilter;
 
   const label = useMemo(
     () => (
       <StyledFilterControlTitleBox>
         <StyledFilterControlTitle data-test="filter-control-name">
           {name}
         </StyledFilterControlTitle>
+        {isRequired && <RequiredFieldIndicator />}
+        {filter.description && filter.description.trim() && (
+          <DescriptionToolTip description={filter.description} />
+        )}
         <StyledIcon data-test="filter-icon">{icon}</StyledIcon>
       </StyledFilterControlTitleBox>
     ),
-    [icon, name],
+    [name, isRequired, filter.description, icon],
   );
 
   return (
@@ -101,4 +161,5 @@ const FilterControl: React.FC<FilterProps> = ({
     </StyledFilterControlContainer>
   );
 };
+
 export default React.memo(FilterControl);
diff --git a/...board/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/...board/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
@@ -16,6 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import {
+  ColumnMeta,
+  InfoTooltipWithTrigger,
+  Metric,
+} from '@superset-ui/chart-controls';
 import {
   AdhocFilter,
   Behavior,
@@ -26,15 +31,11 @@ import {
   JsonResponse,
   styled,
   SupersetApiError,
-  t,
   SupersetClient,
+  t,
 } from '@superset-ui/core';
-import {
-  ColumnMeta,
-  InfoTooltipWithTrigger,
-  Metric,
-} from '@superset-ui/chart-controls';
 import { FormInstance } from 'antd/lib/form';
+import { isEmpty, isEqual } from 'lodash';
 import React, {
   forwardRef,
   useCallback,
@@ -44,53 +45,55 @@ import React, {
   useState,
 } from 'react';
 import { useSelector } from 'react-redux';
-import { isEqual, isEmpty } from 'lodash';
-import { FormItem } from 'src/components/Form';
-import { Input } from 'src/common/components';
+import { getChartDataRequest } from 'src/chart/chartAction';
+import { Input, TextArea } from 'src/common/components';
 import { Select } from 'src/components';
-import { cacheWrapper } from 'src/utils/cacheWrapper';
-import AdhocFilterControl from 'src/explore/components/controls/FilterControl/AdhocFilterControl';
-import DateFilterControl from 'src/explore/components/controls/DateFilterControl';
-import { addDangerToast } from 'src/components/MessageToasts/actions';
-import { ClientErrorObject } from 'src/utils/getClientErrorObject';
 import Collapse from 'src/components/Collapse';
-import { getChartDataRequest } from 'src/chart/chartAction';
-import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
-import { waitForAsyncData } from 'src/middleware/asyncEvent';
-import Tabs from 'src/components/Tabs';
+import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert';
+import { FormItem } from 'src/components/Form';
 import Icons from 'src/components/Icons';
-import { Tooltip } from 'src/components/Tooltip';
+import Loading from 'src/components/Loading';
+import { addDangerToast } from 'src/components/MessageToasts/actions';
 import { Radio } from 'src/components/Radio';
-import BasicErrorAlert from 'src/components/ErrorMessage/BasicErrorAlert';
+import Tabs from 'src/components/Tabs';
+import { Tooltip } from 'src/components/Tooltip';
 import {
   Chart,
   ChartsState,
   DatasourcesState,
   RootState,
 } from 'src/dashboard/types';
-import Loading from 'src/components/Loading';
+import DateFilterControl from 'src/explore/components/controls/DateFilterControl';
+import AdhocFilterControl from 'src/explore/components/controls/FilterControl/AdhocFilterControl';
+import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
+import { waitForAsyncData } from 'src/middleware/asyncEvent';
+import { cacheWrapper } from 'src/utils/cacheWrapper';
+import { ClientErrorObject } from 'src/utils/getClientErrorObject';
+import {
+  Filter,
+  NativeFilterType,
+} from 'src/dashboard/components/nativeFilters/types';
+import { getFormData } from 'src/dashboard/components/nativeFilters/utils';
+import {
+  CASCADING_FILTERS,
+  getFiltersConfigModalTestId,
+} from '../FiltersConfigModal';
+import { FilterRemoval, NativeFiltersForm } from '../types';
+import { CollapsibleControl } from './CollapsibleControl';
 import { ColumnSelect } from './ColumnSelect';
-import { NativeFiltersForm, FilterRemoval } from '../types';
+import DatasetSelect from './DatasetSelect';
+import DefaultValue from './DefaultValue';
+import FilterScope from './FilterScope/FilterScope';
+import getControlItemsMap from './getControlItemsMap';
+import RemovedFilter from './RemovedFilter';
+import { useBackendFormUpdate, useDefaultValue } from './state';
 import {
   FILTER_SUPPORTED_TYPES,
   hasTemporalColumns,
+  mostUsedDataset,
   setNativeFilterFieldValues,
   useForceUpdate,
-  mostUsedDataset,
 } from './utils';
-import { useBackendFormUpdate, useDefaultValue } from './state';
-import { getFormData } from '../../utils';
-import { Filter, NativeFilterType } from '../../types';
-import getControlItemsMap from './getControlItemsMap';
-import FilterScope from './FilterScope/FilterScope';
-import RemovedFilter from './RemovedFilter';
-import DefaultValue from './DefaultValue';
-import { CollapsibleControl } from './CollapsibleControl';
-import {
-  CASCADING_FILTERS,
-  getFiltersConfigModalTestId,
-} from '../FiltersConfigModal';
-import DatasetSelect from './DatasetSelect';
 
 const TabPane = styled(Tabs.TabPane)`
   padding: ${({ theme }) => theme.gridUnit * 4}px 0px;
@@ -966,6 +969,13 @@ const FiltersConfigForm = (
             {Object.keys(controlItems)
               .filter(key => BASIC_CONTROL_ITEMS.includes(key))
               .map(key => controlItems[key].element)}
+            <StyledFormItem
+              name={['filters', filterId, 'description']}
+              initialValue={filterToEdit?.description}
+              label={<StyledLabel>{t('Description')}</StyledLabel>}
+            >
+              <TextArea />
+            </StyledFormItem>
           </Collapse.Panel>
           {hasAdvancedSection && (
             <Collapse.Panel

diff --git a/...components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx b/...components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx
@@ -64,6 +64,7 @@ const filterMock: Filter = {
   targets: [{}],
   controlValues: {},
   type: NativeFilterType.NATIVE_FILTER,
+  description: '',
 };
 
 const createProps: () => ControlItemsProps = () => ({

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/types.ts
@@ -45,6 +45,7 @@ export interface NativeFiltersFormItem {
   time_range?: string;
   granularity_sqla?: string;
   type: NativeFilterType;
+  description: string;
   hierarchicalFilter?: boolean;
 }
 

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/utils.ts
@@ -159,6 +159,7 @@ export const createHandleSave = (
         scope: formInputs.scope,
         sortMetric: formInputs.sortMetric,
         type: formInputs.type,
+        description: (formInputs.description || '').trim(),
       };
     });
 

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/types.ts b/superset-frontend/src/dashboard/components/nativeFilters/types.ts
@@ -60,6 +60,7 @@ export interface Filter {
   tabsInScope?: string[];
   chartsInScope?: number[];
   type: NativeFilterType;
+  description: string;
 }
 
 export type FilterConfiguration = Filter[];