fix(dashboard): FilterBox JS error when datasets API is slow (#15993)

.pylintrc

@@ -373,7 +373,7 @@ max-locals=15
 max-returns=10
 
 # Maximum number of branch for function / method body
-max-branches=12
+max-branches=15
 
 # Maximum number of statements in function / method body
 max-statements=50

superset-frontend/src/chart/Chart.jsx

@@ -22,8 +22,9 @@ import Alert from 'src/components/Alert';
 import { styled, logging, t } from '@superset-ui/core';
 
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
+import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
 import Button from 'src/components/Button';
-import Loading from '../components/Loading';
+import Loading from 'src/components/Loading';
 import ErrorBoundary from '../components/ErrorBoundary';
 import ChartRenderer from './ChartRenderer';
 import { ChartErrorMessage } from './ChartErrorMessage';
@@ -164,10 +165,32 @@ class Chart extends React.PureComponent {
   }
 
   renderErrorMessage(queryResponse) {
-    const { chartId, chartAlert, chartStackTrace, dashboardId } = this.props;
+    const {
+      chartId,
+      chartAlert,
+      chartStackTrace,
+      datasource,
+      dashboardId,
+      height,
+    } = this.props;
 
     const error = queryResponse?.errors?.[0];
     const message = chartAlert || queryResponse?.message;
+
+    // if datasource is still loading, don't render JS errors
+    if (chartAlert && datasource === PLACEHOLDER_DATASOURCE) {
+      return (
+        <Styles
+          data-ui-anchor="chart"
+          className="chart-container"
+          data-test="chart-container"
+          height={height}
+        >
+          <Loading />
+        </Styles>
+      );
+    }
+
     return (
       <ChartErrorMessage
         chartId={chartId}
@@ -198,6 +221,7 @@ class Chart extends React.PureComponent {
     if (chartStatus === 'failed') {
       return queriesResponse.map(item => this.renderErrorMessage(item));
     }
+
     if (errorMessage) {
       return (
         <Alert

superset-frontend/src/chart/ChartRenderer.jsx

@@ -16,11 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { snakeCase } from 'lodash';
+import { snakeCase, isEqual } from 'lodash';
 import PropTypes from 'prop-types';
 import React from 'react';
 import { SuperChart, logging, Behavior } from '@superset-ui/core';
-import { Logger, LOG_ACTIONS_RENDER_CHART } from '../logger/LogUtils';
+import { Logger, LOG_ACTIONS_RENDER_CHART } from 'src/logger/LogUtils';
 
 const propTypes = {
   annotationData: PropTypes.object,
@@ -93,7 +93,7 @@ class ChartRenderer extends React.Component {
         nextProps.queriesResponse !== this.props.queriesResponse;
       return (
         this.hasQueryResponseChange ||
-        nextProps.datasource !== this.props.datasource ||
+        !isEqual(nextProps.datasource, this.props.datasource) ||
         nextProps.annotationData !== this.props.annotationData ||
         nextProps.ownState !== this.props.ownState ||
         nextProps.filterState !== this.props.filterState ||
@@ -183,8 +183,8 @@ class ChartRenderer extends React.Component {
     const {
       width,
       height,
-      annotationData,
       datasource,
+      annotationData,
       initialValues,
       ownState,
       filterState,
@@ -224,7 +224,7 @@ class ChartRenderer extends React.Component {
         width={width}
         height={height}
         annotationData={annotationData}
-        datasource={datasource || {}}
+        datasource={datasource}
         initialValues={initialValues}
         formData={formData}
         ownState={ownState}

superset-frontend/src/chart/chartReducer.ts

@@ -19,6 +19,7 @@
 /* eslint camelcase: 0 */
 import { t } from '@superset-ui/core';
 import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate';
+import { DatasourcesAction } from 'src/dashboard/actions/datasources';
 import { ChartState } from 'src/explore/types';
 import { getFormDataFromControls } from 'src/explore/controlUtils';
 import { now } from 'src/modules/dates';
@@ -196,6 +197,24 @@ export default function chartReducer(
   if (action.type === HYDRATE_DASHBOARD) {
     return { ...action.data.charts };
   }
+  if (action.type === DatasourcesAction.SET_DATASOURCES) {
+    return Object.fromEntries(
+      Object.entries(charts).map(([chartId, chart]) => [
+        chartId,
+        // if render has failed, clear error message,
+        // which will trigger a re-render
+        chart.chartStatus === 'failed'
+          ? {
+              ...chart,
+              chartStatus: '',
+              chartStackTrace: null,
+              chartAlert: null,
+            }
+          : chart,
+      ]),
+    );
+  }
+
   if (action.type in actionHandlers) {
     return {
       ...charts,

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

@@ -20,6 +20,7 @@ import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
 import { styled } from '@superset-ui/core';
+import { isEqual } from 'lodash';
 
 import { exportChart, getExploreLongUrl } from 'src/explore/exploreUtils';
 import ChartContainer from 'src/chart/ChartContainer';
@@ -126,7 +127,7 @@ export default class Chart extends React.Component {
       nextState.width !== this.state.width ||
       nextState.height !== this.state.height ||
       nextState.descriptionHeight !== this.state.descriptionHeight ||
-      nextProps.datasource !== this.props.datasource
+      !isEqual(nextProps.datasource, this.props.datasource)
     ) {
       return true;
     }

superset-frontend/src/dashboard/constants.ts

@@ -0,0 +1,35 @@
+/* eslint-disable import/prefer-default-export */
+import { DatasourceType } from '@superset-ui/core';
+import { Datasource } from 'src/dashboard/types';
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+export const PLACEHOLDER_DATASOURCE: Datasource = {
+  id: 0,
+  type: DatasourceType.Table,
+  uid: '_placeholder_',
+  datasource_name: '',
+  table_name: '',
+  columns: [],
+  column_types: [],
+  metrics: [],
+  column_format: {},
+  verbose_map: {},
+  main_dttm_col: '',
+  description: '',
+};

superset-frontend/src/dashboard/containers/Chart.jsx

@@ -35,6 +35,7 @@ import {
 } from '../util/activeDashboardFilters';
 import getFormDataWithExtraFilters from '../util/charts/getFormDataWithExtraFilters';
 import Chart from '../components/gridComponents/Chart';
+import { PLACEHOLDER_DATASOURCE } from '../constants';
 
 const EMPTY_FILTERS = {};
 
@@ -55,7 +56,8 @@ function mapStateToProps(
   const { id } = ownProps;
   const chart = chartQueries[id] || {};
   const datasource =
-    (chart && chart.form_data && datasources[chart.form_data.datasource]) || {};
+    (chart && chart.form_data && datasources[chart.form_data.datasource]) ||
+    PLACEHOLDER_DATASOURCE;
   const { colorScheme, colorNamespace } = dashboardState;
 
   // note: this method caches filters if possible to prevent render cascades

superset-frontend/src/visualizations/FilterBox/transformProps.ts

@@ -16,16 +16,18 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+import { FilterBoxChartProps } from './types';
+
 const NOOP = () => {};
 
-export default function transformProps(chartProps) {
+export default function transformProps(chartProps: FilterBoxChartProps) {
   const {
     datasource,
     formData,
     hooks,
     initialValues,
     queriesData,
-    rawDatasource,
+    rawDatasource = {},
     rawFormData,
     width,
     height,
@@ -44,7 +46,7 @@ export default function transformProps(chartProps) {
     showSqlaTimeColumn,
     showSqlaTimeGranularity,
   } = formData;
-  const { verboseMap } = datasource;
+  const { verboseMap = {} } = datasource;
   const filterConfigs = formData.filterConfigs || [];
 
   const filtersFields = filterConfigs.map(flt => ({

superset-frontend/src/visualizations/FilterBox/types.ts

@@ -0,0 +1,29 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, Datasource } from '@superset-ui/core';
+
+export interface FilterConfig {
+  column: string;
+  label: string;
+}
+
+export type FilterBoxChartProps = ChartProps & {
+  datasource?: Datasource;
+  formData: ChartProps['formData'] & { filterConfigs: FilterConfig[] };
+};

superset/connectors/base/models.py

@@ -287,20 +287,30 @@ class BaseDatasource(
             for param in METRIC_FORM_DATA_PARAMS:
                 for metric in utils.get_iterable(form_data.get(param) or []):
                     metric_names.add(utils.get_metric_name(metric))
-
                     if utils.is_adhoc_metric(metric):
                         column_names.add(
                             (metric.get("column") or {}).get("column_name")
                         )
 
-            # pull out all required columns from the form_data
-            for filter_ in form_data.get("adhoc_filters") or []:
-                if filter_["clause"] == "WHERE" and filter_.get("subject"):
-                    column_names.add(filter_.get("subject"))
+            # Columns used in query filters
+            column_names.update(
+                filter_["subject"]
+                for filter_ in form_data.get("adhoc_filters") or []
+                if filter_.get("clause") == "WHERE" and filter_.get("subject")
+            )
 
-            for param in COLUMN_FORM_DATA_PARAMS:
-                for column in utils.get_iterable(form_data.get(param) or []):
-                    column_names.add(column)
+            # columns used by Filter Box
+            column_names.update(
+                filter_config["column"]
+                for filter_config in form_data.get("filter_configs") or []
+                if "column" in filter_config
+            )
+
+            column_names.update(
+                column
+                for column in utils.get_iterable(form_data.get(param) or [])
+                for param in COLUMN_FORM_DATA_PARAMS
+            )
 
         filtered_metrics = [
             metric