fix: Edit physical dataset from the Edit Dataset modal (#15770)

* Remove unnecessary onChange

* Remove confliciting onChange

* Revert unnecessary change

* Enhance and fix tests

(cherry picked from commit a7cbcde9bf)

superset-frontend/src/components/DatabaseSelector/DatabaseSelector.test.tsx

@@ -38,7 +38,7 @@ const createProps = () => ({
   onDbChange: jest.fn(),
   onSchemaChange: jest.fn(),
   onSchemasLoad: jest.fn(),
-  onChange: jest.fn(),
+  onUpdate: jest.fn(),
 });
 
 beforeEach(() => {
@@ -161,7 +161,7 @@ test('Refresh should work', async () => {
     expect(props.onDbChange).toBeCalledTimes(0);
     expect(props.onSchemaChange).toBeCalledTimes(0);
     expect(props.onSchemasLoad).toBeCalledTimes(1);
-    expect(props.onChange).toBeCalledTimes(0);
+    expect(props.onUpdate).toBeCalledTimes(0);
   });
 
   userEvent.click(screen.getByRole('button'));
@@ -174,7 +174,7 @@ test('Refresh should work', async () => {
     expect(props.onDbChange).toBeCalledTimes(1);
     expect(props.onSchemaChange).toBeCalledTimes(1);
     expect(props.onSchemasLoad).toBeCalledTimes(2);
-    expect(props.onChange).toBeCalledTimes(1);
+    expect(props.onUpdate).toBeCalledTimes(1);
   });
 });
 

superset-frontend/src/components/DatabaseSelector/index.tsx

@@ -72,7 +72,7 @@ interface DatabaseSelectorProps {
   readOnly?: boolean;
   schema?: string;
   sqlLabMode?: boolean;
-  onChange?: ({
+  onUpdate?: ({
     dbId,
     schema,
   }: {
@@ -89,7 +89,7 @@ export default function DatabaseSelector({
   getTableList,
   handleError,
   isDatabaseSelectEnabled = true,
-  onChange,
+  onUpdate,
   onDbChange,
   onSchemaChange,
   onSchemasLoad,
@@ -143,8 +143,8 @@ export default function DatabaseSelector({
   function onSelectChange({ dbId, schema }: { dbId: number; schema?: string }) {
     setCurrentDbId(dbId);
     setCurrentSchema(schema);
-    if (onChange) {
-      onChange({ dbId, schema, tableName: undefined });
+    if (onUpdate) {
+      onUpdate({ dbId, schema, tableName: undefined });
     }
   }
 

superset-frontend/src/components/TableSelector/index.tsx

@@ -90,7 +90,7 @@ interface TableSelectorProps {
   getDbList?: (arg0: any) => {};
   handleError: (msg: string) => void;
   isDatabaseSelectEnabled?: boolean;
-  onChange?: ({
+  onUpdate?: ({
     dbId,
     schema,
   }: {
@@ -117,7 +117,7 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
   getDbList,
   handleError,
   isDatabaseSelectEnabled = true,
-  onChange,
+  onUpdate,
   onDbChange,
   onSchemaChange,
   onSchemasLoad,
@@ -198,8 +198,8 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
   }) {
     setCurrentTableName(tableName);
     setCurrentSchema(schema);
-    if (onChange) {
-      onChange({ dbId, schema, tableName });
+    if (onUpdate) {
+      onUpdate({ dbId, schema, tableName });
     }
   }
 
@@ -299,7 +299,7 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({
         getDbList={getDbList}
         getTableList={fetchTables}
         handleError={handleError}
-        onChange={onSelectionChange}
+        onUpdate={onSelectionChange}
         onDbChange={readOnly ? undefined : onDbChange}
         onSchemaChange={readOnly ? undefined : onSchemaChange}
         onSchemasLoad={onSchemasLoad}

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/SliceHeader/SliceHeader.test.tsx

@@ -57,7 +57,7 @@ jest.mock('src/dashboard/components/SliceHeaderControls', () => ({
       <button
         type="button"
         data-test="exploreChart"
-        onClick={props.exploreChart}
+        onClick={props.logExploreChart}
       >
         exploreChart
       </button>
@@ -155,7 +155,7 @@ const createProps = () => ({
   updateSliceName: jest.fn(),
   toggleExpandSlice: jest.fn(),
   forceRefresh: jest.fn(),
-  exploreChart: jest.fn(),
+  logExploreChart: jest.fn(),
   exportCSV: jest.fn(),
   formData: {},
 });
@@ -176,7 +176,7 @@ test('Should render - default props', () => {
   // @ts-ignore
   delete props.toggleExpandSlice;
   // @ts-ignore
-  delete props.exploreChart;
+  delete props.logExploreChart;
   // @ts-ignore
   delete props.exportCSV;
   // @ts-ignore
@@ -218,7 +218,7 @@ test('Should render default props and "call" actions', () => {
   // @ts-ignore
   delete props.toggleExpandSlice;
   // @ts-ignore
-  delete props.exploreChart;
+  delete props.logExploreChart;
   // @ts-ignore
   delete props.exportCSV;
   // @ts-ignore
@@ -378,9 +378,9 @@ test('Correct actions to "SliceHeaderControls"', () => {
   userEvent.click(screen.getByTestId('forceRefresh'));
   expect(props.forceRefresh).toBeCalledTimes(1);
 
-  expect(props.exploreChart).toBeCalledTimes(0);
+  expect(props.logExploreChart).toBeCalledTimes(0);
   userEvent.click(screen.getByTestId('exploreChart'));
-  expect(props.exploreChart).toBeCalledTimes(1);
+  expect(props.logExploreChart).toBeCalledTimes(1);
 
   expect(props.exportCSV).toBeCalledTimes(0);
   userEvent.click(screen.getByTestId('exportCSV'));

superset-frontend/src/dashboard/components/SliceHeader/index.tsx

@@ -21,48 +21,24 @@ import { styled, t } from '@superset-ui/core';
 import { Tooltip } from 'src/components/Tooltip';
 import { useDispatch, useSelector } from 'react-redux';
 import EditableTitle from 'src/components/EditableTitle';
-import SliceHeaderControls from 'src/dashboard/components/SliceHeaderControls';
+import SliceHeaderControls, {
+  SliceHeaderControlsProps,
+} from 'src/dashboard/components/SliceHeaderControls';
 import FiltersBadge from 'src/dashboard/components/FiltersBadge';
 import Icon from 'src/components/Icon';
 import { RootState } from 'src/dashboard/types';
 import FilterIndicator from 'src/dashboard/components/FiltersBadge/FilterIndicator';
 import { clearDataMask } from 'src/dataMask/actions';
 
-type SliceHeaderProps = {
+type SliceHeaderProps = SliceHeaderControlsProps & {
   innerRef?: string;
-  slice: {
-    description: string;
-    viz_type: string;
-    slice_name: string;
-    slice_id: number;
-    slice_description: string;
-  };
-  isExpanded?: boolean;
-  isCached?: boolean[];
-  cachedDttm?: string[];
-  updatedDttm?: number;
   updateSliceName?: (arg0: string) => void;
-  toggleExpandSlice?: () => void;
-  forceRefresh?: () => void;
-  exploreChart?: () => void;
-  exportCSV?: () => void;
-  exportFullCSV?: () => void;
   editMode?: boolean;
-  isFullSize?: boolean;
   annotationQuery?: object;
   annotationError?: object;
   sliceName?: string;
-  supersetCanExplore?: boolean;
-  supersetCanShare?: boolean;
-  supersetCanCSV?: boolean;
-  sliceCanEdit?: boolean;
-  componentId: string;
-  dashboardId: number;
   filters: object;
-  addSuccessToast: () => void;
-  addDangerToast: () => void;
   handleToggleFullSize: () => void;
-  chartStatus: string;
   formData: object;
 };
 
@@ -84,7 +60,8 @@ const SliceHeader: FC<SliceHeaderProps> = ({
   forceRefresh = () => ({}),
   updateSliceName = () => ({}),
   toggleExpandSlice = () => ({}),
-  exploreChart = () => ({}),
+  logExploreChart = () => ({}),
+  exploreUrl = '#',
   exportCSV = () => ({}),
   editMode = false,
   annotationQuery = {},
@@ -188,7 +165,8 @@ const SliceHeader: FC<SliceHeaderProps> = ({
               updatedDttm={updatedDttm}
               toggleExpandSlice={toggleExpandSlice}
               forceRefresh={forceRefresh}
-              exploreChart={exploreChart}
+              logExploreChart={logExploreChart}
+              exploreUrl={exploreUrl}
               exportCSV={exportCSV}
               exportFullCSV={exportFullCSV}
               supersetCanExplore={supersetCanExplore}

superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx

@@ -193,18 +193,6 @@ test('Should not show export full CSV if report is not table', () => {
   );
 });
 
-test('Should "View chart in Explore"', () => {
-  const props = createProps();
-  render(<SliceHeaderControls {...props} />, { useRedux: true });
-
-  expect(props.exploreChart).toBeCalledTimes(0);
-  userEvent.click(
-    screen.getByRole('menuitem', { name: 'View chart in Explore' }),
-  );
-  expect(props.exploreChart).toBeCalledTimes(1);
-  expect(props.exploreChart).toBeCalledWith(371);
-});
-
 test('Should "Toggle chart description"', () => {
   const props = createProps();
   render(<SliceHeaderControls {...props} />, { useRedux: true });

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -80,7 +80,8 @@ const VerticalDotsTrigger = () => (
     <span className="dot" />
   </VerticalDotsContainer>
 );
-interface Props {
+
+export interface SliceHeaderControlsProps {
   slice: {
     description: string;
     viz_type: string;
@@ -88,35 +89,43 @@ interface Props {
     slice_id: number;
     slice_description: string;
   };
+
   componentId: string;
-  chartStatus: string;
   dashboardId: number;
-  addDangerToast: () => void;
+  chartStatus: string;
   isCached: boolean[];
   cachedDttm: string[] | null;
   isExpanded?: boolean;
   updatedDttm: number | null;
-  supersetCanExplore: boolean;
-  supersetCanShare: boolean;
-  supersetCanCSV: boolean;
-  sliceCanEdit: boolean;
   isFullSize?: boolean;
   formData: object;
-  toggleExpandSlice?: (sliceId: number) => void;
+  exploreUrl?: string;
+
   forceRefresh: (sliceId: number, dashboardId: number) => void;
-  exploreChart?: (sliceId: number) => void;
+  logExploreChart?: (sliceId: number) => void;
+  toggleExpandSlice?: (sliceId: number) => void;
   exportCSV?: (sliceId: number) => void;
   exportFullCSV?: (sliceId: number) => void;
-  addSuccessToast: (message: string) => void;
   handleToggleFullSize: () => void;
+
+  addDangerToast: (message: string) => void;
+  addSuccessToast: (message: string) => void;
+
+  supersetCanExplore?: boolean;
+  supersetCanShare?: boolean;
+  supersetCanCSV?: boolean;
+  sliceCanEdit?: boolean;
 }
 interface State {
   showControls: boolean;
   showCrossFilterScopingModal: boolean;
 }
 
-class SliceHeaderControls extends React.PureComponent<Props, State> {
-  constructor(props: Props) {
+class SliceHeaderControls extends React.PureComponent<
+  SliceHeaderControlsProps,
+  State
+> {
+  constructor(props: SliceHeaderControlsProps) {
     super(props);
     this.toggleControls = this.toggleControls.bind(this);
     this.refreshChart = this.refreshChart.bind(this);
@@ -164,8 +173,8 @@ class SliceHeaderControls extends React.PureComponent<Props, State> {
         break;
       case MENU_KEYS.EXPLORE_CHART:
         // eslint-disable-next-line no-unused-expressions
-        this.props.exploreChart &&
-          this.props.exploreChart(this.props.slice.slice_id);
+        this.props.logExploreChart &&
+          this.props.logExploreChart(this.props.slice.slice_id);
         break;
       case MENU_KEYS.EXPORT_CSV:
         // eslint-disable-next-line no-unused-expressions
@@ -272,7 +281,9 @@ class SliceHeaderControls extends React.PureComponent<Props, State> {
 
         {this.props.supersetCanExplore && (
           <Menu.Item key={MENU_KEYS.EXPLORE_CHART}>
-            {t('View chart in Explore')}
+            <a href={this.props.exploreUrl} rel="noopener noreferrer">
+              {t('View chart in Explore')}
+            </a>
           </Menu.Item>
         )}
 

superset-frontend/src/dashboard/components/gridComponents/Chart.jsx

@@ -21,7 +21,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { styled } from '@superset-ui/core';
 
-import { exploreChart, exportChart } from 'src/explore/exploreUtils';
+import { exportChart, getExploreLongUrl } from 'src/explore/exploreUtils';
 import ChartContainer from 'src/chart/ChartContainer';
 import {
   LOG_ACTIONS_CHANGE_DASHBOARD_FILTER,
@@ -110,7 +110,6 @@ export default class Chart extends React.Component {
     this.changeFilter = this.changeFilter.bind(this);
     this.handleFilterMenuOpen = this.handleFilterMenuOpen.bind(this);
     this.handleFilterMenuClose = this.handleFilterMenuClose.bind(this);
-    this.exploreChart = this.exploreChart.bind(this);
     this.exportCSV = this.exportCSV.bind(this);
     this.exportFullCSV = this.exportFullCSV.bind(this);
     this.forceRefresh = this.forceRefresh.bind(this);
@@ -221,13 +220,14 @@ export default class Chart extends React.Component {
     this.props.unsetFocusedFilterField(chartId, column);
   }
 
-  exploreChart() {
+  logExploreChart = () => {
     this.props.logEvent(LOG_ACTIONS_EXPLORE_DASHBOARD_CHART, {
       slice_id: this.props.slice.slice_id,
       is_cached: this.props.isCached,
     });
-    exploreChart(this.props.formData);
-  }
+  };
+
+  getChartUrl = () => getExploreLongUrl(this.props.formData);
 
   exportCSV(isFullCSV = false) {
     this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, {
@@ -327,7 +327,8 @@ export default class Chart extends React.Component {
           forceRefresh={this.forceRefresh}
           editMode={editMode}
           annotationQuery={chart.annotationQuery}
-          exploreChart={this.exploreChart}
+          logExploreChart={this.logExploreChart}
+          exploreUrl={this.getChartUrl()}
           exportCSV={this.exportCSV}
           exportFullCSV={this.exportFullCSV}
           updateSliceName={updateSliceName}

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/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/explore/exploreUtils/index.js

@@ -89,6 +89,10 @@ export function getURIDirectory(endpointType = 'base') {
   return '/superset/explore/';
 }
 
+/**
+ * This gets the url of the explore page, with all the form data included explicitly.
+ * This includes any form data overrides from the dashboard.
+ */
 export function getExploreLongUrl(
   formData,
   endpointType,
@@ -138,6 +142,11 @@ export function getChartDataUri({ path, qs, allowDomainSharding = false }) {
   return uri;
 }
 
+/**
+ * This gets the minimal url for the given form data.
+ * If there are dashboard overrides present in the form data,
+ * they will not be included in the url.
+ */
 export function getExploreUrl({
   formData,
   endpointType = 'base',

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};
+`;

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

@@ -257,9 +257,8 @@ function dbReducer(
           ...JSON.parse(action.payload.extra || ''),
           metadata_params: JSON.stringify(extra_json?.metadata_params),
           engine_params: JSON.stringify(extra_json?.engine_params),
-          schemas_allowed_for_csv_upload: JSON.stringify(
+          schemas_allowed_for_csv_upload:
             extra_json?.schemas_allowed_for_csv_upload,
-          ),
         };
       }
 
@@ -316,6 +315,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 +363,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     fetchResource,
     createResource,
     updateResource,
+    clearError,
   } = useSingleViewResource<DatabaseObject>(
     'database',
     t('database'),
@@ -387,7 +396,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 +408,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
     setDB({ type: ActionType.reset });
     setHasConnectedDb(false);
     setValidationErrors(null); // reset validation errors on close
+    clearError();
     setEditNewDb(false);
     onHide();
   };
@@ -458,18 +468,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
 
     if (dbToUpdate?.extra_json) {
       // convert extra_json to back to string
-      dbToUpdate.extra = JSON.stringify({
-        ...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) ||
-          '[]',
-      });
+      dbToUpdate.extra = serializeExtra(dbToUpdate?.extra_json);
     }
 
     if (db?.id) {
@@ -530,16 +529,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 +558,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 +617,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 +880,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 +903,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 +933,31 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
             />
           )}
           {!isEditMode && (
-            <Alert
-              closable={false}
-              css={(theme: SupersetTheme) => antDAlertStyles(theme)}
-              message="Additional fields may be required"
-              showIcon
-              description={
-                <>
-                  Select databases require additional fields to be completed in
-                  the Advanced tab to successfully connect the database. Learn
-                  what requirements your databases has{' '}
-                  <a
-                    href={DOCUMENTATION_LINK}
-                    target="_blank"
-                    rel="noopener noreferrer"
-                    className="additional-fields-alert-description"
-                  >
-                    here
-                  </a>
-                  .
-                </>
-              }
-              type="info"
-            />
+            <StyledAlertMargin>
+              <Alert
+                closable={false}
+                css={(theme: SupersetTheme) => antDAlertStyles(theme)}
+                message="Additional fields may be required"
+                showIcon
+                description={
+                  <>
+                    Select databases require additional fields to be completed
+                    in the Advanced tab to successfully connect the database.
+                    Learn what requirements your databases has{' '}
+                    <a
+                      href={DOCUMENTATION_LINK}
+                      target="_blank"
+                      rel="noopener noreferrer"
+                      className="additional-fields-alert-description"
+                    >
+                      here
+                    </a>
+                    .
+                  </>
+                }
+                type="info"
+              />
+            </StyledAlertMargin>
           )}
         </Tabs.TabPane>
         <Tabs.TabPane tab={<span>{t('Advanced')}</span>} key="2">
@@ -1116,7 +1117,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/data/dataset/AddDatasetModal.tsx

@@ -108,7 +108,7 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({
           dbId={datasourceId}
           formMode
           handleError={addDangerToast}
-          onChange={onChange}
+          onUpdate={onChange}
           schema={currentSchema}
           tableName={currentTableName}
         />

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/connectors/sqla/models.py

@@ -60,14 +60,7 @@ from sqlalchemy import (
 )
 from sqlalchemy.orm import backref, Query, relationship, RelationshipProperty, Session
 from sqlalchemy.schema import UniqueConstraint
-from sqlalchemy.sql import (
-    column,
-    ColumnElement,
-    literal_column,
-    quoted_name,
-    table,
-    text,
-)
+from sqlalchemy.sql import column, ColumnElement, literal_column, table, text
 from sqlalchemy.sql.elements import ColumnClause
 from sqlalchemy.sql.expression import Label, Select, TextAsFrom, TextClause
 from sqlalchemy.sql.selectable import Alias, TableClause
@@ -887,7 +880,7 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
 
     def adhoc_metric_to_sqla(
         self, metric: AdhocMetric, columns_by_name: Dict[str, TableColumn]
-    ) -> Column:
+    ) -> ColumnElement:
         """
         Turn an adhoc metric into a sqlalchemy column.
 
@@ -917,28 +910,19 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
         return self.make_sqla_column_compatible(sqla_metric, label)
 
     def make_sqla_column_compatible(
-        self, sqla_col: Column, label: Optional[str] = None
-    ) -> Column:
+        self, sqla_col: ColumnElement, label: Optional[str] = None
+    ) -> ColumnElement:
         """Takes a sqlalchemy column object and adds label info if supported by engine.
-        also adds quotes to the column if engine is configured for quotes.
         :param sqla_col: sqlalchemy column instance
         :param label: alias/label that column is expected to have
         :return: either a sql alchemy column or label instance if supported by engine
         """
         label_expected = label or sqla_col.name
         db_engine_spec = self.db_engine_spec
-
-        # add quotes to column
-        if db_engine_spec.force_column_alias_quotes:
-            sqla_col = column(
-                quoted_name(sqla_col.name, True), sqla_col.type, sqla_col.is_literal
-            )
-
         # add quotes to tables
         if db_engine_spec.allows_alias_in_select:
             label = db_engine_spec.make_label_compatible(label_expected)
             sqla_col = sqla_col.label(label)
-
         sqla_col.key = label_expected
         return sqla_col
 
@@ -1086,7 +1070,8 @@ class SqlaTable(  # pylint: disable=too-many-public-methods,too-many-instance-at
 
         # To ensure correct handling of the ORDER BY labeling we need to reference the
         # metric instance if defined in the SELECT clause.
-        metrics_exprs_by_label = {m.name: m for m in metrics_exprs}
+        # use the key of the ColumnClause for the expected label
+        metrics_exprs_by_label = {m.key: m for m in metrics_exprs}
         metrics_exprs_by_expr = {str(m): m for m in metrics_exprs}
 
         # Since orderby may use adhoc metrics, too; we need to process them first

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

superset/models/core.py

@@ -20,6 +20,7 @@ import enum
 import json
 import logging
 import textwrap
+from ast import literal_eval
 from contextlib import closing
 from copy import deepcopy
 from datetime import datetime
@@ -667,6 +668,10 @@ class Database(
         self,
     ) -> List[str]:
         allowed_databases = self.get_extra().get("schemas_allowed_for_csv_upload", [])
+
+        if isinstance(allowed_databases, str):
+            allowed_databases = literal_eval(allowed_databases)
+
         if hasattr(g, "user"):
             extra_allowed_databases = config["ALLOWED_USER_CSV_SCHEMA_FUNC"](
                 self, g.user

tests/integration_tests/core_tests.py

@@ -25,9 +25,6 @@ import json
 import logging
 from typing import Dict, List
 from urllib.parse import quote
-
-from sqlalchemy.sql import column, quoted_name, literal_column
-from sqlalchemy import select
 from tests.integration_tests.fixtures.birth_names_dashboard import (
     load_birth_names_dashboard_with_slices,
 )
@@ -43,7 +40,7 @@ import pandas as pd
 import sqlalchemy as sqla
 from sqlalchemy.exc import SQLAlchemyError
 from superset.models.cache import CacheKey
-from superset.utils.core import get_example_database, get_or_create_db
+from superset.utils.core import get_example_database
 from tests.integration_tests.conftest import with_feature_flags
 from tests.integration_tests.fixtures.energy_dashboard import (
     load_energy_table_with_slice,
@@ -901,58 +898,6 @@ class TestCore(SupersetTestCase):
         rendered_query = str(table.get_from_clause())
         self.assertEqual(clean_query, rendered_query)
 
-    def test_make_column_compatible(self):
-        """
-        DB Eng Specs: Make column compatible
-        """
-
-        # with force_column_alias_quotes enabled
-        snowflake_database = get_or_create_db("snowflake", "snowflake://")
-
-        table = SqlaTable(
-            table_name="test_columns_with_alias_quotes", database=snowflake_database,
-        )
-
-        col = table.make_sqla_column_compatible(column("foo"))
-        s = select([col])
-        self.assertEqual(str(s), 'SELECT "foo" AS "foo"')
-
-        # with literal_column
-        table = SqlaTable(
-            table_name="test_columns_with_alias_quotes_on_literal_column",
-            database=snowflake_database,
-        )
-
-        col = table.make_sqla_column_compatible(literal_column("foo"))
-        s = select([col])
-        self.assertEqual(str(s), 'SELECT foo AS "foo"')
-
-        # with force_column_alias_quotes NOT enabled
-        postgres_database = get_or_create_db("postgresql", "postgresql://")
-
-        table = SqlaTable(
-            table_name="test_columns_with_no_quotes", database=postgres_database,
-        )
-
-        col = table.make_sqla_column_compatible(column("foo"))
-        s = select([col])
-        self.assertEqual(str(s), "SELECT foo AS foo")
-
-        # with literal_column
-        table = SqlaTable(
-            table_name="test_columns_with_no_quotes_on_literal_column",
-            database=postgres_database,
-        )
-
-        col = table.make_sqla_column_compatible(literal_column("foo"))
-        s = select([col])
-        self.assertEqual(str(s), "SELECT foo AS foo")
-
-        # cleanup
-        db.session.delete(snowflake_database)
-        db.session.delete(postgres_database)
-        db.session.commit()
-
     def test_slice_payload_no_datasource(self):
         self.login(username="admin")
         data = self.get_json_resp("/superset/explore_json/", raise_on_error=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)

tests/integration_tests/db_engine_specs/snowflake_tests.py

@@ -16,8 +16,6 @@
 # under the License.
 import json
 
-from sqlalchemy import column
-
 from superset.db_engine_specs.snowflake import SnowflakeEngineSpec
 from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
 from superset.models.core import Database
@@ -25,20 +23,6 @@ from tests.integration_tests.db_engine_specs.base_tests import TestDbEngineSpec
 
 
 class TestSnowflakeDbEngineSpec(TestDbEngineSpec):
-    def test_snowflake_sqla_column_label(self):
-        """
-        DB Eng Specs (snowflake): Test column label
-        """
-        test_cases = {
-            "Col": "Col",
-            "SUM(x)": "SUM(x)",
-            "SUM[x]": "SUM[x]",
-            "12345_col": "12345_col",
-        }
-        for original, expected in test_cases.items():
-            actual = SnowflakeEngineSpec.make_label_compatible(column(original).name)
-            self.assertEqual(actual, expected)
-
     def test_convert_dttm(self):
         dttm = self.get_dttm()