feat: Support multiple queries per request (apache#11880)

* refactor: add queriesData fields for multiple queries * feat: support multi queries request * lint: fix lint * lint: fix lint * lint: fix lint * fix: fix CR notes * fix: fix CR notes * fix: fix CR notes * fix: fix error case for multi queries * feat: change queryResponse to queriesResponse * fix: revert webpack * test: fix tests * chore: lint * chore: adjust asyncEvent to multiple results * fix: lint * fix: eslint * fix: another eslint rule Co-authored-by: Amit Miran <[email protected]> Co-authored-by: amitmiran137 <[email protected]>
preset-io · Jan 7, 2021 · eb9c678 · eb9c678
1 parent ce05a3d
commit eb9c678
Show file tree

Hide file tree

Showing 17 changed files with 127 additions and 105 deletions.
diff --git a/superset-frontend/spec/javascripts/chart/chartActions_spec.js b/superset-frontend/spec/javascripts/chart/chartActions_spec.js
@@ -189,7 +189,7 @@ describe('chart actions', () => {
         expect(dispatch.callCount).toBe(5);
         const updateFailedAction = dispatch.args[4][0];
         expect(updateFailedAction.type).toBe(actions.CHART_UPDATE_FAILED);
-        expect(updateFailedAction.queryResponse.error).toBe('misc error');
+        expect(updateFailedAction.queriesResponse[0].error).toBe('misc error');
 
         setupDefaultFetchMock();
       });

diff --git a/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx b/superset-frontend/spec/javascripts/dashboard/components/FiltersBadge_spec.tsx
@@ -51,11 +51,13 @@ describe('FiltersBadge', () => {
     store.dispatch({
       type: CHART_UPDATE_SUCCEEDED,
       key: sliceId,
-      queryResponse: {
-        status: 'success',
-        applied_filters: [],
-        rejected_filters: [],
-      },
+      queriesResponse: [
+        {
+          status: 'success',
+          applied_filters: [],
+          rejected_filters: [],
+        },
+      ],
       dashboardFilters,
     });
     const wrapper = shallow(
@@ -74,11 +76,13 @@ describe('FiltersBadge', () => {
     store.dispatch({
       type: CHART_UPDATE_SUCCEEDED,
       key: sliceId,
-      queryResponse: {
-        status: 'success',
-        applied_filters: [{ column: 'region' }],
-        rejected_filters: [],
-      },
+      queriesResponse: [
+        {
+          status: 'success',
+          applied_filters: [{ column: 'region' }],
+          rejected_filters: [],
+        },
+      ],
       dashboardFilters,
     });
     const wrapper = shallow(
@@ -97,11 +101,13 @@ describe('FiltersBadge', () => {
     store.dispatch({
       type: CHART_UPDATE_SUCCEEDED,
       key: sliceId,
-      queryResponse: {
-        status: 'success',
-        applied_filters: [],
-        rejected_filters: [{ column: 'region', reason: 'not_in_datasource' }],
-      },
+      queriesResponse: [
+        {
+          status: 'success',
+          applied_filters: [],
+          rejected_filters: [{ column: 'region', reason: 'not_in_datasource' }],
+        },
+      ],
       dashboardFilters,
     });
     const wrapper = shallow(

diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx
@@ -56,7 +56,7 @@ const propTypes = {
   chartAlert: PropTypes.string,
   chartStatus: PropTypes.string,
   chartStackTrace: PropTypes.string,
-  queryResponse: PropTypes.object,
+  queriesResponse: PropTypes.arrayOf(PropTypes.object),
   triggerQuery: PropTypes.bool,
   refreshOverlayVisible: PropTypes.bool,
   errorMessage: PropTypes.node,
@@ -150,14 +150,8 @@ class Chart extends React.PureComponent {
     });
   }
 
-  renderErrorMessage() {
-    const {
-      chartAlert,
-      chartStackTrace,
-      dashboardId,
-      owners,
-      queryResponse,
-    } = this.props;
+  renderErrorMessage(queryResponse) {
+    const { chartAlert, chartStackTrace, dashboardId, owners } = this.props;
 
     const error = queryResponse?.errors?.[0];
     if (error) {
@@ -187,14 +181,14 @@ class Chart extends React.PureComponent {
       errorMessage,
       onQuery,
       refreshOverlayVisible,
+      queriesResponse = [],
     } = this.props;
 
     const isLoading = chartStatus === 'loading';
-
     const isFaded = refreshOverlayVisible && !errorMessage;
     this.renderContainerStartTime = Logger.getTimestamp();
     if (chartStatus === 'failed') {
-      return this.renderErrorMessage();
+      return queriesResponse.map(item => this.renderErrorMessage(item));
     }
     if (errorMessage) {
       return (

diff --git a/superset-frontend/src/chart/ChartRenderer.jsx b/superset-frontend/src/chart/ChartRenderer.jsx
@@ -37,7 +37,7 @@ const propTypes = {
   // state
   chartAlert: PropTypes.string,
   chartStatus: PropTypes.string,
-  queryResponse: PropTypes.object,
+  queriesResponse: PropTypes.arrayOf(PropTypes.object),
   triggerQuery: PropTypes.bool,
   refreshOverlayVisible: PropTypes.bool,
   // dashboard callbacks
@@ -78,14 +78,14 @@ class ChartRenderer extends React.Component {
 
   shouldComponentUpdate(nextProps) {
     const resultsReady =
-      nextProps.queryResponse &&
+      nextProps.queriesResponse &&
       ['success', 'rendered'].indexOf(nextProps.chartStatus) > -1 &&
-      !nextProps.queryResponse.error &&
+      !nextProps.queriesResponse?.[0]?.error &&
       !nextProps.refreshOverlayVisible;
 
     if (resultsReady) {
       this.hasQueryResponseChange =
-        nextProps.queryResponse !== this.props.queryResponse;
+        nextProps.queriesResponse !== this.props.queriesResponse;
       return (
         this.hasQueryResponseChange ||
         nextProps.annotationData !== this.props.annotationData ||
@@ -179,7 +179,7 @@ class ChartRenderer extends React.Component {
       datasource,
       initialValues,
       formData,
-      queryResponse,
+      queriesResponse,
     } = this.props;
 
     // It's bad practice to use unprefixed `vizType` as classnames for chart
@@ -218,7 +218,8 @@ class ChartRenderer extends React.Component {
         initialValues={initialValues}
         formData={formData}
         hooks={this.hooks}
-        queryData={queryResponse}
+        queryData={queriesResponse?.[0]} // deprecated
+        queriesData={queriesResponse}
         onRenderSuccess={this.handleRenderSuccess}
         onRenderFailure={this.handleRenderFailure}
       />

diff --git a/superset-frontend/src/chart/chartAction.js b/superset-frontend/src/chart/chartAction.js
@@ -52,8 +52,8 @@ export function chartUpdateStarted(queryController, latestQueryFormData, key) {
 }
 
 export const CHART_UPDATE_SUCCEEDED = 'CHART_UPDATE_SUCCEEDED';
-export function chartUpdateSucceeded(queryResponse, key) {
-  return { type: CHART_UPDATE_SUCCEEDED, queryResponse, key };
+export function chartUpdateSucceeded(queriesResponse, key) {
+  return { type: CHART_UPDATE_SUCCEEDED, queriesResponse, key };
 }
 
 export const CHART_UPDATE_STOPPED = 'CHART_UPDATE_STOPPED';
@@ -62,8 +62,8 @@ export function chartUpdateStopped(key) {
 }
 
 export const CHART_UPDATE_FAILED = 'CHART_UPDATE_FAILED';
-export function chartUpdateFailed(queryResponse, key) {
-  return { type: CHART_UPDATE_FAILED, queryResponse, key };
+export function chartUpdateFailed(queriesResponse, key) {
+  return { type: CHART_UPDATE_FAILED, queriesResponse, key };
 }
 
 export const CHART_UPDATE_QUEUED = 'CHART_UPDATE_QUEUED';
@@ -361,38 +361,35 @@ export function exploreJSON(
 
     const chartDataRequestCaught = chartDataRequest
       .then(response => {
+        const queriesResponse = response.result;
         if (isFeatureEnabled(FeatureFlag.GLOBAL_ASYNC_QUERIES)) {
           // deal with getChartDataRequest transforming the response data
           const result = 'result' in response ? response.result[0] : response;
           return dispatch(chartUpdateQueued(result, key));
         }
 
-        // new API returns an object with an array of restults
-        // problem: response holds a list of results, when before we were just getting one result.
-        // How to make the entire app compatible with multiple results?
-        // For now just use the first result.
-        const result = response.result[0];
-
-        dispatch(
-          logEvent(LOG_ACTIONS_LOAD_CHART, {
-            slice_id: key,
-            applied_filters: result.applied_filters,
-            is_cached: result.is_cached,
-            force_refresh: force,
-            row_count: result.rowcount,
-            datasource: formData.datasource,
-            start_offset: logStart,
-            ts: new Date().getTime(),
-            duration: Logger.getTimestamp() - logStart,
-            has_extra_filters:
-              formData.extra_filters && formData.extra_filters.length > 0,
-            viz_type: formData.viz_type,
-            data_age: result.is_cached
-              ? moment(new Date()).diff(moment.utc(result.cached_dttm))
-              : null,
-          }),
+        queriesResponse.forEach(resultItem =>
+          dispatch(
+            logEvent(LOG_ACTIONS_LOAD_CHART, {
+              slice_id: key,
+              applied_filters: resultItem.applied_filters,
+              is_cached: resultItem.is_cached,
+              force_refresh: force,
+              row_count: resultItem.rowcount,
+              datasource: formData.datasource,
+              start_offset: logStart,
+              ts: new Date().getTime(),
+              duration: Logger.getTimestamp() - logStart,
+              has_extra_filters:
+                formData.extra_filters && formData.extra_filters.length > 0,
+              viz_type: formData.viz_type,
+              data_age: resultItem.is_cached
+                ? moment(new Date()).diff(moment.utc(resultItem.cached_dttm))
+                : null,
+            }),
+          ),
         );
-        return dispatch(chartUpdateSucceeded(result, key));
+        return dispatch(chartUpdateSucceeded(queriesResponse, key));
       })
       .catch(response => {
         const appendErrorLog = (errorDetails, isCached) => {
@@ -419,7 +416,7 @@ export function exploreJSON(
           } else {
             appendErrorLog(parsedResponse.error, parsedResponse.is_cached);
           }
-          return dispatch(chartUpdateFailed(parsedResponse, key));
+          return dispatch(chartUpdateFailed([parsedResponse], key));
         });
       });
 

diff --git a/superset-frontend/src/chart/chartReducer.js b/superset-frontend/src/chart/chartReducer.js
@@ -30,7 +30,7 @@ export const chart = {
   chartUpdateStartTime: 0,
   latestQueryFormData: {},
   queryController: null,
-  queryResponse: null,
+  queriesResponse: null,
   triggerQuery: true,
   lastRendered: 0,
 };
@@ -47,8 +47,8 @@ export default function chartReducer(charts = {}, action) {
       return {
         ...state,
         chartStatus: 'success',
-        queryResponse: action.queryResponse,
         chartAlert: null,
+        queriesResponse: action.queriesResponse,
         chartUpdateEndTime: now(),
       };
     },
@@ -97,13 +97,13 @@ export default function chartReducer(charts = {}, action) {
       return {
         ...state,
         chartStatus: 'failed',
-        chartAlert: action.queryResponse
-          ? action.queryResponse.error
+        chartAlert: action.queriesResponse
+          ? action.queriesResponse?.[0]?.error
           : t('Network error.'),
         chartUpdateEndTime: now(),
-        queryResponse: action.queryResponse,
-        chartStackTrace: action.queryResponse
-          ? action.queryResponse.stacktrace
+        queriesResponse: action.queriesResponse,
+        chartStackTrace: action.queriesResponse
+          ? action.queriesResponse?.[0]?.stacktrace
           : null,
       };
     },

diff --git a/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts b/superset-frontend/src/dashboard/components/FiltersBadge/selectors.ts
@@ -132,12 +132,12 @@ export const selectIndicatorsForChart = (
   // for now we only need to know which columns are compatible/incompatible,
   // so grab the columns from the applied/rejected filters
   const appliedColumns: Set<string> = new Set(
-    (chart?.queryResponse?.applied_filters || []).map(
+    (chart?.queriesResponse?.[0]?.applied_filters || []).map(
       (filter: any) => filter.column,
     ),
   );
   const rejectedColumns: Set<string> = new Set(
-    (chart?.queryResponse?.rejected_filters || []).map(
+    (chart?.queriesResponse?.[0]?.rejected_filters || []).map(
       (filter: any) => filter.column,
     ),
   );

diff --git a/superset-frontend/src/dashboard/components/SliceHeader.jsx b/superset-frontend/src/dashboard/components/SliceHeader.jsx
@@ -29,8 +29,8 @@ const propTypes = {
   innerRef: PropTypes.func,
   slice: PropTypes.object.isRequired,
   isExpanded: PropTypes.bool,
-  isCached: PropTypes.bool,
-  cachedDttm: PropTypes.string,
+  isCached: PropTypes.arrayOf(PropTypes.bool),
+  cachedDttm: PropTypes.arrayOf(PropTypes.string),
   updatedDttm: PropTypes.number,
   updateSliceName: PropTypes.func,
   toggleExpandSlice: PropTypes.func,
@@ -64,8 +64,8 @@ const defaultProps = {
   annotationError: {},
   cachedDttm: null,
   updatedDttm: null,
-  isCached: false,
-  isExpanded: false,
+  isCached: [],
+  isExpanded: [],
   sliceName: '',
   supersetCanExplore: false,
   supersetCanCSV: false,

diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx b/superset-frontend/src/dashboard/components/SliceHeaderControls.jsx
@@ -31,9 +31,9 @@ const propTypes = {
   componentId: PropTypes.string.isRequired,
   dashboardId: PropTypes.number.isRequired,
   addDangerToast: PropTypes.func.isRequired,
-  isCached: PropTypes.bool,
+  isCached: PropTypes.arrayOf(PropTypes.bool),
+  cachedDttm: PropTypes.arrayOf(PropTypes.string),
   isExpanded: PropTypes.bool,
-  cachedDttm: PropTypes.string,
   updatedDttm: PropTypes.number,
   supersetCanExplore: PropTypes.bool,
   supersetCanCSV: PropTypes.bool,
@@ -49,9 +49,9 @@ const defaultProps = {
   toggleExpandSlice: () => ({}),
   exploreChart: () => ({}),
   exportCSV: () => ({}),
-  cachedDttm: null,
+  cachedDttm: [],
   updatedDttm: null,
-  isCached: false,
+  isCached: [],
   isExpanded: false,
   supersetCanExplore: false,
   supersetCanCSV: false,
@@ -82,9 +82,14 @@ const VerticalDotsContainer = styled.div`
 `;
 
 const RefreshTooltip = styled.div`
-  height: ${({ theme }) => theme.gridUnit * 4}px;
+  height: auto;
   margin: ${({ theme }) => theme.gridUnit}px 0;
   color: ${({ theme }) => theme.colors.grayscale.base};
+  line-height: ${({ theme }) => theme.typography.sizes.m * 1.5}px;
+  display: flex;
+  flex-direction: column;
+  align-items: flex-start;
+  justify-content: flex-start;
 `;
 
 const SCREENSHOT_NODE_SELECTOR = '.dashboard-component-chart-holder';
@@ -171,13 +176,26 @@ class SliceHeaderControls extends React.PureComponent {
       addDangerToast,
       isFullSize,
     } = this.props;
-    const cachedWhen = moment.utc(cachedDttm).fromNow();
+    const cachedWhen = cachedDttm.map(itemCachedDttm =>
+      moment.utc(itemCachedDttm).fromNow(),
+    );
     const updatedWhen = updatedDttm ? moment.utc(updatedDttm).fromNow() : '';
-    const refreshTooltip = isCached
-      ? t('Cached %s', cachedWhen)
-      : (updatedWhen && t('Fetched %s', updatedWhen)) || '';
+    const getCachedTitle = itemCached => {
+      return itemCached
+        ? t('Cached %s', cachedWhen)
+        : updatedWhen && t('Fetched %s', updatedWhen);
+    };
+    const refreshTooltipData = isCached.map(getCachedTitle) || '';
+    // If all queries have same cache time we can unit them to one
+    let refreshTooltip = [...new Set(refreshTooltipData)];
+    refreshTooltip = refreshTooltip.map((item, index) => (
+      <div>
+        {refreshTooltip.length > 1
+          ? `${t('Query')} ${index + 1}: ${item}`
+          : item}
+      </div>
+    ));
     const resizeLabel = isFullSize ? t('Minimize Chart') : t('Maximize Chart');
-
     const menu = (
       <Menu
         onClick={this.handleMenuClick}