fix(native-filters): show human readable time grain label in indicator (#15411)

* fix(native-filters): show human readable time grain label in indicator

* lint

* simplify

(cherry picked from commit ddcf461749)

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

@@ -18,11 +18,24 @@
  */
 import React from 'react';
 import { styled } from '@superset-ui/core';
+import { Form, FormItem } from 'src/components/Form';
 import FilterValue from './FilterValue';
 import { FilterProps } from './types';
+import { checkIsMissingRequiredValue } from '../utils';
+
+const StyledFormItem = styled(FormItem)`
+  & label {
+    width: 100%;
+    padding-right: ${({ theme }) => theme.gridUnit * 11}px;
+  }
+`;
+
+const StyledIcon = styled.div`
+  position: absolute;
+  right: 0;
+`;
 
 const StyledFilterControlTitle = styled.h4`
-  width: 100%;
   font-size: ${({ theme }) => theme.typography.sizes.s}px;
   color: ${({ theme }) => theme.colors.grayscale.dark1};
   margin: 0;
@@ -37,7 +50,7 @@ const StyledFilterControlTitleBox = styled.div`
   margin-bottom: ${({ theme }) => theme.gridUnit}px;
 `;
 
-const StyledFilterControlContainer = styled.div`
+const StyledFilterControlContainer = styled(Form)`
   width: 100%;
 `;
 
@@ -50,21 +63,34 @@ const FilterControl: React.FC<FilterProps> = ({
   inView,
 }) => {
   const { name = '<undefined>' } = filter;
+
+  const isMissingRequiredValue = checkIsMissingRequiredValue(
+    filter,
+    filter.dataMask?.filterState,
+  );
+
   return (
-    <StyledFilterControlContainer>
+    <StyledFilterControlContainer layout="vertical">
-      <StyledFilterControlTitleBox>
+      <StyledFormItem
-        <StyledFilterControlTitle data-test="filter-control-name">
+        label={
-          {name}
+          <StyledFilterControlTitleBox>
-        </StyledFilterControlTitle>
+            <StyledFilterControlTitle data-test="filter-control-name">
-        <div data-test="filter-icon">{icon}</div>
+              {name}
-      </StyledFilterControlTitleBox>
+            </StyledFilterControlTitle>
-      <FilterValue
+            <StyledIcon data-test="filter-icon">{icon}</StyledIcon>
-        dataMaskSelected={dataMaskSelected}
+          </StyledFilterControlTitleBox>
-        filter={filter}
+        }
-        directPathToChild={directPathToChild}
+        required={filter?.controlValues?.enableEmptyFilter}
-        onFilterSelectionChange={onFilterSelectionChange}
+        validateStatus={isMissingRequiredValue ? 'error' : undefined}
-        inView={inView}
+      >
-      />
+        <FilterValue
+          dataMaskSelected={dataMaskSelected}
+          filter={filter}
+          directPathToChild={directPathToChild}
+          onFilterSelectionChange={onFilterSelectionChange}
+          inView={inView}
+        />
+      </StyledFormItem>
     </StyledFilterControlContainer>
   );
 };

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

@@ -19,10 +19,10 @@
 import React, { useEffect, useRef, useState } from 'react';
 import {
   QueryFormData,
-  styled,
   SuperChart,
   DataMask,
   t,
+  styled,
   Behavior,
   ChartDataResponseResult,
   JsonObject,
@@ -43,13 +43,14 @@ 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`
+const HEIGHT = 32;
-  min-height: ${({ theme }) => theme.gridUnit * 11}px;
+
-  padding-bottom: ${({ theme }) => theme.gridUnit * 3}px;
+// Overrides superset-ui height with min-height
-  & > div > div {
+const StyledDiv = styled.div`
-    height: auto;
+  & > div {
+    height: auto !important;
+    min-height: ${HEIGHT}px;
   }
 `;
 
@@ -184,35 +185,27 @@ const FilterValue: React.FC<FilterProps> = ({
     );
   }
 
-  const isMissingRequiredValue = checkIsMissingRequiredValue(
-    filter,
-    filter.dataMask?.filterState,
-  );
-
   return (
-    <FilterItem data-test="form-item-value">
+    <StyledDiv data-test="form-item-value">
       {isLoading ? (
         <Loading position="inline-centered" />
       ) : (
         <SuperChart
-          height={50}
+          height={HEIGHT}
           width="100%"
           formData={formData}
           // For charts that don't have datasource we need workaround for empty placeholder
           queriesData={hasDataSource ? state : [{ data: [{}] }]}
           chartType={filterType}
           behaviors={[Behavior.NATIVE_FILTER]}
-          filterState={{
+          filterState={{ ...filter.dataMask?.filterState }}
-            ...filter.dataMask?.filterState,
-            validateMessage: isMissingRequiredValue && t('Value is required'),
-          }}
           ownState={filter.dataMask?.ownState}
           enableNoResults={metadata?.enableNoResults}
           isRefreshing={isRefreshing}
           hooks={{ setDataMask, setFocusedFilter, unsetFocusedFilter }}
         />
       )}
-    </FilterItem>
+    </StyledDiv>
   );
 };
 

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

@@ -22,7 +22,6 @@ import {
   SetDataMaskHook,
   SuperChart,
   AppSection,
-  t,
 } from '@superset-ui/core';
 import { FormInstance } from 'antd/lib/form';
 import Loading from 'src/components/Loading';
@@ -57,10 +56,6 @@ 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" />
   ) : (
@@ -79,7 +74,6 @@ const DefaultValue: FC<DefaultValueProps> = ({
       enableNoResults={enableNoResults}
       filterState={{
         ...formFilter.defaultDataMask?.filterState,
-        validateMessage: isMissingRequiredValue && t('Value is required'),
       }}
     />
   );

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

@@ -130,14 +130,24 @@ export const StyledRowFormItem = styled(FormItem)`
 `;
 
 export const StyledRowSubFormItem = styled(FormItem)`
+  min-width: 50%;
+
   & .ant-form-item-label {
     padding-bottom: 0;
   }
 
+  .ant-form-item {
+    margin-bottom: 0;
+  }
+
   .ant-form-item-control-input-content > div > div {
     height: auto;
   }
 
+  .ant-form-item-extra {
+    display: none;
+  }
+
   & .ant-form-item-control-input {
     height: auto;
   }
@@ -749,7 +759,9 @@ const FiltersConfigForm = (
                       if (hasValue) {
                         return Promise.resolve();
                       }
-                      return Promise.reject();
+                      return Promise.reject(
+                        new Error(t('Default value is required')),
+                      );
                     },
                   },
                 ]}
@@ -958,7 +970,7 @@ const FiltersConfigForm = (
                   onChange={checked => onSortChanged(checked || undefined)}
                   initialValue={hasSorting}
                 >
-                  <StyledFormItem
+                  <StyledRowFormItem
                     name={[
                       'filters',
                       filterId,
@@ -986,7 +998,7 @@ const FiltersConfigForm = (
                         onSortChanged(value)
                       }
                     />
-                  </StyledFormItem>
+                  </StyledRowFormItem>
                   {hasMetrics && (
                     <StyledRowSubFormItem
                       name={['filters', filterId, 'sortMetric']}

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

@@ -54,8 +54,7 @@ export const useDefaultValue = (
   filterToEdit?: Filter,
 ) => {
   const [hasDefaultValue, setHasPartialDefaultValue] = useState(
-    !!filterToEdit?.defaultDataMask?.filterState?.value ||
+    !!filterToEdit?.defaultDataMask?.filterState?.value,
-      formFilter?.controlValues?.enableEmptyFilter,
   );
   const [isRequired, setisRequired] = useState(
     formFilter?.controlValues?.enableEmptyFilter,

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

@@ -26,11 +26,9 @@ import {
   GenericDataType,
   JsonObject,
   smartDateDetailedFormatter,
-  styled,
   t,
   tn,
 } from '@superset-ui/core';
-import { FormItem } from 'src/components/Form';
 import React, {
   RefObject,
   ReactElement,
@@ -82,10 +80,6 @@ function reducer(
   }
 }
 
-const Error = styled.div`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
 export default function PluginFilterSelect(props: PluginFilterSelectProps) {
   const {
     coltypeMap,
@@ -279,57 +273,52 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
 
   return (
     <Styles height={height} width={width}>
-      <FormItem
+      <StyledSelect
-        validateStatus={filterState.validateMessage && 'error'}
+        allowClear
-        extra={<Error>{filterState.validateMessage}</Error>}
+        // @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" />}
       >
-        <StyledSelect
+        {sortedData.map(row => {
-          allowClear
+          const [value] = groupby.map(col => row[col]);
-          // @ts-ignore
+          return (
-          value={filterState.value || []}
+            // @ts-ignore
-          disabled={isDisabled}
+            <Option key={`${value}`} value={value}>
-          showSearch={showSearch}
+              {labelFormatter(value, datatype)}
-          mode={multiSelect ? 'multiple' : undefined}
+            </Option>
-          placeholder={placeholderText}
+          );
-          onSearch={searchWrapper}
+        })}
-          onSelect={clearSuggestionSearch}
+        {currentSuggestionSearch &&
-          onBlur={handleBlur}
+          !ensureIsArray(filterState.value).some(
-          onDropdownVisibleChange={setIsDropdownVisible}
+            suggestion => suggestion === currentSuggestionSearch,
-          dropdownRender={(
+          ) && (
-            originNode: ReactElement & { ref?: RefObject<HTMLElement> },
+            <Option value={currentSuggestionSearch}>
-          ) => {
+              {`${t('Create "%s"', currentSuggestionSearch)}`}
-            if (isDropdownVisible && !wasDropdownVisible) {
+            </Option>
-              originNode.ref?.current?.scrollTo({ top: 0 });
+          )}
-            }
+      </StyledSelect>
-            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>
   );
 }

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

@@ -24,7 +24,7 @@ import {
   TimeGranularity,
   tn,
 } from '@superset-ui/core';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { Select } from 'src/common/components';
 import { Styles, StyledSelect } from '../common';
 import { PluginFilterTimeGrainProps } from './types';
@@ -52,10 +52,22 @@ export default function PluginFilterTimegrain(
   const { defaultValue, inputRef } = formData;
 
   const [value, setValue] = useState<string[]>(defaultValue ?? []);
+  const durationMap = useMemo(
+    () =>
+      data.reduce(
+        (agg, { duration, name }: { duration: string; name: string }) => ({
+          ...agg,
+          [duration]: name,
+        }),
+        {} as { [key in string]: string },
+      ),
+    [JSON.stringify(data)],
+  );
 
   const handleChange = (values: string[] | string | undefined | null) => {
     const resultValue: string[] = ensureIsArray<string>(values);
     const [timeGrain] = resultValue;
+    const label = timeGrain ? durationMap[timeGrain] : undefined;
 
     const extraFormData: ExtraFormData = {};
     if (timeGrain) {
@@ -65,6 +77,7 @@ export default function PluginFilterTimegrain(
     setDataMask({
       extraFormData,
       filterState: {
+        label,
         value: resultValue.length ? resultValue : null,
       },
     });

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

@@ -21,7 +21,7 @@ import { Select } from 'src/common/components';
 import { PluginFilterStylesProps } from './types';
 
 export const Styles = styled.div<PluginFilterStylesProps>`
-  height: ${({ height }) => height}px;
+  min-height: ${({ height }) => height}px;
   width: ${({ width }) => width}px;
 `;
 

superset/views/core.py

@@ -2477,14 +2477,34 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
             CtasMethod, query_params.get("ctas_method", CtasMethod.TABLE)
         )
         tmp_table_name: str = cast(str, query_params.get("tmp_table_name"))
-        client_id: str = cast(
+        client_id: str = cast(str, query_params.get("client_id"))
-            str, query_params.get("client_id") or utils.shortid()[:10]
+        client_id_or_short_id: str = cast(str, client_id or utils.shortid()[:10])
-        )
         sql_editor_id: str = cast(str, query_params.get("sql_editor_id"))
         tab_name: str = cast(str, query_params.get("tab"))
         status: str = QueryStatus.PENDING if async_flag else QueryStatus.RUNNING
+        user_id: int = g.user.get_id() if g.user else None
 
         session = db.session()
+
+        # check to see if this query is already running
+        query = (
+            session.query(Query)
+            .filter_by(
+                client_id=client_id, user_id=user_id, sql_editor_id=sql_editor_id
+            )
+            .one_or_none()
+        )
+        if query is not None and query.status in [
+            QueryStatus.RUNNING,
+            QueryStatus.PENDING,
+            QueryStatus.TIMED_OUT,
+        ]:
+            # return the existing query
+            payload = json.dumps(
+                {"query": query.to_dict()}, default=utils.json_int_dttm_ser
+            )
+            return json_success(payload)
+
         mydb = session.query(Database).get(database_id)
         if not mydb:
             return json_error_response("Database with id %i is missing.", database_id)
@@ -2512,8 +2532,8 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
             sql_editor_id=sql_editor_id,
             tmp_table_name=tmp_table_name,
             tmp_schema_name=tmp_schema_name,
-            user_id=g.user.get_id() if g.user else None,
+            user_id=user_id,
-            client_id=client_id,
+            client_id=client_id_or_short_id,
         )
         try:
             session.add(query)

superset/views/sql_lab.py

@@ -228,10 +228,12 @@ class TabStateView(BaseSupersetView):
     @has_access_api
     @expose("<int:tab_state_id>/query/<client_id>", methods=["DELETE"])
     def delete_query(  # pylint: disable=no-self-use
-        self, tab_state_id: str, client_id: str
+        self, tab_state_id: int, client_id: str
     ) -> FlaskResponse:
         db.session.query(Query).filter_by(
-            client_id=client_id, user_id=g.user.get_id(), sql_editor_id=tab_state_id
+            client_id=client_id,
+            user_id=g.user.get_id(),
+            sql_editor_id=str(tab_state_id),
         ).delete(synchronize_session=False)
         db.session.commit()
         return json_success(json.dumps("OK"))

tests/core_tests.py

@@ -1412,7 +1412,7 @@ class TestCore(SupersetTestCase):
             "client_id_1",
             user_name=username,
             raise_on_error=True,
-            sql_editor_id=tab_state_id,
+            sql_editor_id=str(tab_state_id),
         )
         # run an orphan query (no tab)
         self.run_sql(