fix: safe removal of tab with filters still scoped to a non-existing chart (#15650)

(cherry picked from commit 52ad779a27)

superset-frontend/src/dashboard/components/Header/HeaderActionsDropdown/index.jsx

@@ -159,6 +159,8 @@ class HeaderActionsDropdown extends React.PureComponent {
         downloadAsImage(
           SCREENSHOT_NODE_SELECTOR,
           this.props.dashboardTitle,
+          {},
+          true,
         )(domEvent).then(() => {
           menu.style.visibility = 'visible';
         });

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) &&
+    hasDefaultValue && (value === null || value === undefined);
-    formFilter?.controlValues?.enableEmptyFilter;
   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
+              {formFilter.filterType && (
-                name={['filters', filterId, 'defaultDataMask']}
+                <StyledRowSubFormItem
-                initialValue={
+                  name={['filters', filterId, 'defaultDataMask']}
-                  formFilter.filterType === filterToEdit?.filterType
+                  initialValue={
-                    ? filterToEdit?.defaultDataMask
+                    formFilter.filterType === filterToEdit?.filterType
-                    : null
+                      ? filterToEdit?.defaultDataMask
-                }
+                      : null
-                data-test="default-input"
+                  }
-                label={<StyledLabel>{t('Default Value')}</StyledLabel>}
+                  data-test="default-input"
-                required={hasDefaultValue}
+                  label={<StyledLabel>{t('Default Value')}</StyledLabel>}
-                rules={[
+                  required={hasDefaultValue}
-                  {
+                  rules={[
-                    validator: (rule, value) => {
+                    {
-                      const hasValue = !!value?.filterState?.value;
+                      validator: (rule, value) => {
-                      if (hasValue) {
+                        const hasValue =
-                        return Promise.resolve();
+                          value?.filterState?.value !== null &&
-                      }
+                          value?.filterState?.value !== undefined;
-                      return Promise.reject(
+                        if (hasValue) {
-                        new Error(t('Default value is required')),
+                          return Promise.resolve();
-                      );
+                        }
+                        return Promise.reject(
+                          new Error(t('Default value is required')),
+                        );
+                      },
                     },
-                  },
+                  ]}
-                ]}
+                >
-              >
+                  {error ? (
-                {error ? (
+                    <BasicErrorAlert
-                  <BasicErrorAlert
+                      title={t('Cannot load filter')}
-                    title={t('Cannot load filter')}
+                      body={error}
-                    body={error}
+                      level="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}
                     />
-                    {hasDataset && datasetId && (
+                  ) : showDefaultValue ? (
-                      <Tooltip title={t('Refresh the default values')}>
+                    <DefaultValueContainer>
-                        <RefreshIcon onClick={() => refreshHandler(true)} />
+                      <DefaultValue
-                      </Tooltip>
+                        setDataMask={dataMask => {
-                    )}
+                          setNativeFilterFieldValues(form, filterId, {
-                  </DefaultValueContainer>
+                            defaultDataMask: dataMask,
-                ) : (
+                          });
-                  t('Fill all required fields to enable "Default Value"')
+                          form.validateFields([
-                )}
+                            ['filters', filterId, 'defaultDataMask'],
-              </StyledRowSubFormItem>
+                          ]);
+                          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,
-) => {
+): [boolean, boolean, string, Function] => {
-  const [hasDefaultValue, setHasPartialDefaultValue] = useState(
+  const enableEmptyFilter = !!formFilter?.controlValues?.enableEmptyFilter;
-    !!filterToEdit?.defaultDataMask?.filterState?.value,
+  const defaultToFirstItem = !!formFilter?.controlValues?.defaultToFirstItem;
-  );
-  const [isRequired, setisRequired] = useState(
-    formFilter?.controlValues?.enableEmptyFilter,
-  );
 
+  const [hasDefaultValue, setHasPartialDefaultValue] = useState(false);
+  const [isRequired, setIsRequired] = useState(enableEmptyFilter);
   const [defaultValueTooltip, setDefaultValueTooltip] = useState('');
 
-  const defaultToFirstItem = formFilter?.controlValues?.defaultToFirstItem;
+  const setHasDefaultValue = (value = false) => {
-
+    const required = enableEmptyFilter && !defaultToFirstItem;
-  const setHasDefaultValue = useCallback(
+    setIsRequired(required);
-    (value?) => {
+    setHasPartialDefaultValue(required ? true : value);
-      const required =
+  };
-        !!formFilter?.controlValues?.enableEmptyFilter && !defaultToFirstItem;
-      setisRequired(required);
-      setHasPartialDefaultValue(required ? true : value);
-    },
-    [formFilter?.controlValues?.enableEmptyFilter, defaultToFirstItem],
-  );
 
   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/dashboard/util/activeAllDashboardFilters.ts

@@ -104,7 +104,7 @@ export const getAllActiveFilters = ({
       };
     // Iterate over all roots to find all affected charts
     scope.rootPath.forEach((layoutItemId: string | number) => {
-      layout[layoutItemId].children.forEach((child: string) => {
+      layout[layoutItemId]?.children?.forEach((child: string) => {
         // Need exclude from affected charts, charts that located in scope `excluded`
         findAffectedCharts({
           child,

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'}
+        validateStatus={filterState.validateStatus}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        {...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`
+const Wrapper = styled.div<{ validateStatus?: 'error' | 'warning' | 'info' }>`
-  color: ${({ theme }) => theme.colors.error.base};
-`;
-
-const Wrapper = styled.div<{ validateStatus?: string }>`
   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
+        <StyledFormItem {...formItemData}>
-          validateStatus={filterState.validateMessage && 'error'}
-          extra={<Error>{filterState.validateMessage}</Error>}
-        >
           <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'}
+        validateStatus={filterState.validateStatus}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        {...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'}
+        validateStatus={filterState.validateStatus}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        {...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'}
+        validateStatus={filterState.validateStatus}
-        extra={<Error>{filterState.validateMessage}</Error>}
+        {...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};
+`;

superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx

@@ -360,7 +360,7 @@ const forceSSLField = ({
     <InfoTooltip
       tooltip={t('SSL Mode "require" will be used.')}
       placement="right"
-      viewBox="0 0 24 24"
+      viewBox="0 -5 24 24"
     />
   </div>
 );

superset-frontend/src/views/CRUD/data/database/DatabaseModal/ExtraOptions.tsx

@@ -414,7 +414,7 @@ const ExtraOptions = ({
           <div className="input-container">
             <StyledJsonEditor
               name="metadata_params"
-              value={db?.extra_json?.metadata_params || '{}'}
+              value={db?.extra_json?.metadata_params || ''}
               placeholder={t('Metadata Parameters')}
               onChange={(json: string) =>
                 onExtraEditorChange({ json, name: 'metadata_params' })
@@ -436,7 +436,7 @@ const ExtraOptions = ({
           <div className="input-container">
             <StyledJsonEditor
               name="engine_params"
-              value={db?.extra_json?.engine_params || '{}'}
+              value={db?.extra_json?.engine_params || ''}
               placeholder={t('Engine Parameters')}
               onChange={(json: string) =>
                 onExtraEditorChange({ json, name: 'engine_params' })

superset-frontend/src/views/CRUD/data/database/DatabaseModal/ModalHeader.tsx

@@ -27,11 +27,12 @@ import {
 } from './styles';
 import { DatabaseForm, DatabaseObject } from '../types';
 
-export const DOCUMENTATION_LINK =
-  'https://superset.apache.org/docs/databases/installing-database-drivers';
-
 const supersetTextDocs = getDatabaseDocumentationLinks();
 
+export const DOCUMENTATION_LINK = supersetTextDocs
+  ? supersetTextDocs.support
+  : 'https://superset.apache.org/docs/databases/installing-database-drivers';
+
 const irregularDocumentationLinks = {
   postgresql: 'https://superset.apache.org/docs/databases/postgres',
   mssql: 'https://superset.apache.org/docs/databases/sql-server',

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

@@ -316,6 +316,15 @@ function dbReducer(
 
 const DEFAULT_TAB_KEY = '1';
 
+const serializeExtra = (extraJson: DatabaseObject['extra_json']) =>
+  JSON.stringify({
+    ...extraJson,
+    metadata_params: JSON.parse((extraJson?.metadata_params as string) || '{}'),
+    engine_params: JSON.parse((extraJson?.engine_params as string) || '{}'),
+    schemas_allowed_for_csv_upload:
+      (extraJson?.schemas_allowed_for_csv_upload as string) || '[]',
+  });
+
 const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
   addDangerToast,
   addSuccessToast,
@@ -355,6 +364,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     fetchResource,
     createResource,
     updateResource,
+    clearError,
   } = useSingleViewResource<DatabaseObject>(
     'database',
     t('database'),
@@ -387,7 +397,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
       sqlalchemy_uri: db?.sqlalchemy_uri || '',
       database_name: db?.database_name?.trim() || undefined,
       impersonate_user: db?.impersonate_user || undefined,
-      extra: db?.extra || undefined,
+      extra: serializeExtra(db?.extra_json) || undefined,
       encrypted_extra: db?.encrypted_extra || '',
       server_cert: db?.server_cert || undefined,
     };
@@ -399,6 +409,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     setDB({ type: ActionType.reset });
     setHasConnectedDb(false);
     setValidationErrors(null); // reset validation errors on close
+    clearError();
     setEditNewDb(false);
     onHide();
   };
@@ -458,18 +469,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
 
     if (dbToUpdate?.extra_json) {
       // convert extra_json to back to string
-      dbToUpdate.extra = JSON.stringify({
+      dbToUpdate.extra = serializeExtra(dbToUpdate?.extra_json);
-        ...dbToUpdate.extra_json,
-        metadata_params: JSON.parse(
-          (dbToUpdate?.extra_json?.metadata_params as string) || '{}',
-        ),
-        engine_params: JSON.parse(
-          (dbToUpdate?.extra_json?.engine_params as string) || '{}',
-        ),
-        schemas_allowed_for_csv_upload:
-          (dbToUpdate?.extra_json?.schemas_allowed_for_csv_upload as string) ||
-          '[]',
-      });
     }
 
     if (db?.id) {
@@ -530,16 +530,16 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     }
   };
 
-  const setDatabaseModel = (engine: string) => {
+  const setDatabaseModel = (database_name: string) => {
     const selectedDbModel = availableDbs?.databases.filter(
-      (db: DatabaseObject) => db.engine === engine,
+      (db: DatabaseObject) => db.name === database_name,
     )[0];
-    const { name, parameters } = selectedDbModel;
+    const { engine, parameters } = selectedDbModel;
     const isDynamic = parameters !== undefined;
     setDB({
       type: ActionType.dbSelected,
       payload: {
-        database_name: name,
+        database_name,
         configuration_method: isDynamic
           ? CONFIGURATION_METHOD.DYNAMIC_FORM
           : CONFIGURATION_METHOD.SQLALCHEMY_URI,
@@ -559,12 +559,12 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
         onChange={setDatabaseModel}
         placeholder="Choose a database..."
       >
-        {availableDbs?.databases
+        {[...(availableDbs?.databases || [])]
           ?.sort((a: DatabaseForm, b: DatabaseForm) =>
             a.name.localeCompare(b.name),
           )
           .map((database: DatabaseForm) => (
-            <Select.Option value={database.engine} key={database.engine}>
+            <Select.Option value={database.name} key={database.name}>
               {database.name}
             </Select.Option>
           ))}
@@ -618,7 +618,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
         .map((database: DatabaseForm) => (
           <IconButton
             className="preferred-item"
-            onClick={() => setDatabaseModel(database.engine)}
+            onClick={() => setDatabaseModel(database.name)}
             buttonText={database.name}
             icon={dbImages?.[database.engine]}
           />
@@ -881,7 +881,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                 testConnection={testConnection}
                 isEditMode={isEditMode}
               />
-              {isDynamic(db?.backend || db?.engine) && (
+              {isDynamic(db?.backend || db?.engine) && !isEditMode && (
                 <div css={(theme: SupersetTheme) => infoTooltip(theme)}>
                   <Button
                     buttonStyle="link"
@@ -904,7 +904,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                     tooltip={t(
                       'Click this link to switch to an alternate form that exposes only the required fields needed to connect this database.',
                     )}
-                    viewBox="0 -3 24 24"
+                    viewBox="0 -6 24 24"
                   />
                 </div>
               )}
@@ -934,29 +934,31 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
             />
           )}
           {!isEditMode && (
-            <Alert
+            <StyledAlertMargin>
-              closable={false}
+              <Alert
-              css={(theme: SupersetTheme) => antDAlertStyles(theme)}
+                closable={false}
-              message="Additional fields may be required"
+                css={(theme: SupersetTheme) => antDAlertStyles(theme)}
-              showIcon
+                message="Additional fields may be required"
-              description={
+                showIcon
-                <>
+                description={
-                  Select databases require additional fields to be completed in
+                  <>
-                  the Advanced tab to successfully connect the database. Learn
+                    Select databases require additional fields to be completed
-                  what requirements your databases has{' '}
+                    in the Advanced tab to successfully connect the database.
-                  <a
+                    Learn what requirements your databases has{' '}
-                    href={DOCUMENTATION_LINK}
+                    <a
-                    target="_blank"
+                      href={DOCUMENTATION_LINK}
-                    rel="noopener noreferrer"
+                      target="_blank"
-                    className="additional-fields-alert-description"
+                      rel="noopener noreferrer"
-                  >
+                      className="additional-fields-alert-description"
-                    here
+                    >
-                  </a>
+                      here
-                  .
+                    </a>
-                </>
+                    .
-              }
+                  </>
-              type="info"
+                }
-            />
+                type="info"
+              />
+            </StyledAlertMargin>
           )}
         </Tabs.TabPane>
         <Tabs.TabPane tab={<span>{t('Advanced')}</span>} key="2">
@@ -1116,7 +1118,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
                     tooltip={t(
                       'Click this link to switch to an alternate form that allows you to input the SQLAlchemy URL for this database manually.',
                     )}
-                    viewBox="6 4 24 24"
+                    viewBox="0 -6 24 24"
                   />
                 </div>
                 {/* Step 2 */}

superset-frontend/src/views/CRUD/data/database/DatabaseModal/styles.ts

@@ -124,7 +124,6 @@ export const antDModalNoPaddingStyles = css`
 export const infoTooltip = (theme: SupersetTheme) => css`
   margin-bottom: ${theme.gridUnit * 5}px;
   svg {
-    vertical-align: bottom;
     margin-bottom: ${theme.gridUnit * 0.25}px;
   }
 `;

superset-frontend/src/views/CRUD/data/database/types.ts

@@ -25,6 +25,7 @@ export type DatabaseObject = {
   // Connection + general
   id?: number;
   database_name: string;
+  name: string; // synonym to database_name
   sqlalchemy_uri?: string;
   backend?: string;
   created_by?: null | DatabaseUser;

superset-frontend/src/views/CRUD/hooks.ts

@@ -631,7 +631,7 @@ export function useAvailableDatabases() {
 
   const getAvailable = useCallback(() => {
     SupersetClient.get({
-      endpoint: `/api/v1/database/available`,
+      endpoint: `/api/v1/database/available/`,
     }).then(({ json }) => {
       setAvailableDbs(json);
     });

superset/databases/api.py

@@ -246,6 +246,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
             new_model = CreateDatabaseCommand(g.user, item).run()
             # Return censored version for sqlalchemy URI
             item["sqlalchemy_uri"] = new_model.sqlalchemy_uri
+            item["expose_in_sqllab"] = new_model.expose_in_sqllab
 
             # If parameters are available return them in the payload
             if new_model.parameters:
@@ -919,6 +920,9 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
         preferred_databases: List[str] = app.config.get("PREFERRED_DATABASES", [])
         available_databases = []
         for engine_spec, drivers in get_available_engine_specs().items():
+            if not drivers:
+                continue
+
             payload: Dict[str, Any] = {
                 "name": engine_spec.engine_name,
                 "engine": engine_spec.engine,

superset/db_engine_specs/postgres.py

@@ -186,7 +186,7 @@ class PostgresEngineSpec(PostgresBaseEngineSpec, BasicParametersMixin):
         "postgresql://user:password@host:port/dbname[?key=value&key=value...]"
     )
     # https://www.postgresql.org/docs/9.1/libpq-ssl.html#LIBQ-SSL-CERTIFICATES
-    encryption_parameters = {"sslmode": "verify-ca"}
+    encryption_parameters = {"sslmode": "require"}
 
     max_column_name_length = 63
     try_remove_schema_from_table_name = False

tests/integration_tests/db_engine_specs/postgres_tests.py

@@ -460,7 +460,7 @@ def test_base_parameters_mixin():
     )
     assert sqlalchemy_uri == (
         "postgresql+psycopg2://username:password@localhost:5432/dbname?"
-        "foo=bar&sslmode=verify-ca"
+        "foo=bar&sslmode=require"
     )
 
     parameters_from_uri = PostgresEngineSpec.get_parameters_from_uri(sqlalchemy_uri)