perf(dashboard): Send chart requests before native filter requests (#…

…19077) (cherry picked from commit b8091e3)
apache · Apr 3, 2022 · 088f6f7 · 088f6f7
1 parent d0c9df4
commit 088f6f7
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 17 deletions.
diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js
@@ -422,6 +422,16 @@ const refreshCharts = (chartList, force, interval, dashboardId, dispatch) =>
     resolve();
   });
 
+export const ON_FILTERS_REFRESH = 'ON_FILTERS_REFRESH';
+export function onFiltersRefresh() {
+  return { type: ON_FILTERS_REFRESH };
+}
+
+export const ON_FILTERS_REFRESH_SUCCESS = 'ON_FILTERS_REFRESH_SUCCESS';
+export function onFiltersRefreshSuccess() {
+  return { type: ON_FILTERS_REFRESH_SUCCESS };
+}
+
 export const ON_REFRESH_SUCCESS = 'ON_REFRESH_SUCCESS';
 export function onRefreshSuccess() {
   return { type: ON_REFRESH_SUCCESS };
@@ -436,8 +446,11 @@ export function onRefresh(
 ) {
   return dispatch => {
     dispatch({ type: ON_REFRESH });
-    refreshCharts(chartList, force, interval, dashboardId, dispatch).then(() =>
-      dispatch({ type: ON_REFRESH_SUCCESS }),
+    refreshCharts(chartList, force, interval, dashboardId, dispatch).then(
+      () => {
+        dispatch(onRefreshSuccess());
+        dispatch(onFiltersRefresh());
+      },
     );
   };
 }

diff --git a/superset-frontend/src/dashboard/actions/hydrate.js b/superset-frontend/src/dashboard/actions/hydrate.js
@@ -406,6 +406,7 @@ export const hydrateDashboard =
           maxUndoHistoryExceeded: false,
           lastModifiedTime: dashboardData.changed_on,
           isRefreshing: false,
+          isFiltersRefreshing: false,
           activeTabs: dashboardState?.activeTabs || [],
           filterboxMigrationState,
         },

diff --git a/...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/...-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx
@@ -43,6 +43,7 @@ import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import { waitForAsyncData } from 'src/middleware/asyncEvent';
 import { ClientErrorObject } from 'src/utils/getClientErrorObject';
 import { RootState } from 'src/dashboard/types';
+import { onFiltersRefreshSuccess } from 'src/dashboard/actions/dashboardState';
 import { dispatchFocusAction } from './utils';
 import { FilterProps } from './types';
 import { getFormData } from '../../utils';
@@ -62,6 +63,18 @@ const StyledDiv = styled.div`
 const queriesDataPlaceholder = [{ data: [{}] }];
 const behaviors = [Behavior.NATIVE_FILTER];
 
+const useShouldFilterRefresh = () => {
+  const isDashboardRefreshing = useSelector<RootState, boolean>(
+    state => state.dashboardState.isRefreshing,
+  );
+  const isFilterRefreshing = useSelector<RootState, boolean>(
+    state => state.dashboardState.isFiltersRefreshing,
+  );
+
+  // trigger filter requests only after charts requests were triggered
+  return !isDashboardRefreshing && isFilterRefreshing;
+};
+
 const FilterValue: React.FC<FilterProps> = ({
   dataMaskSelected,
   filter,
@@ -75,9 +88,7 @@ const FilterValue: React.FC<FilterProps> = ({
   const { id, targets, filterType, adhoc_filters, time_range } = filter;
   const metadata = getChartMetadataRegistry().get(filterType);
   const dependencies = useFilterDependencies(id, dataMaskSelected);
-  const isDashboardRefreshing = useSelector<RootState, boolean>(
-    state => state.dashboardState.isRefreshing,
-  );
+  const shouldRefresh = useShouldFilterRefresh();
   const [state, setState] = useState<ChartDataResponseResult[]>([]);
   const [error, setError] = useState<string>('');
   const [formData, setFormData] = useState<Partial<QueryFormData>>({
@@ -97,6 +108,14 @@ const FilterValue: React.FC<FilterProps> = ({
   const [isRefreshing, setIsRefreshing] = useState(false);
   const dispatch = useDispatch();
 
+  const handleFilterLoadFinish = useCallback(() => {
+    setIsRefreshing(false);
+    setIsLoading(false);
+    if (shouldRefresh) {
+      dispatch(onFiltersRefreshSuccess());
+    }
+  }, [dispatch, shouldRefresh]);
+
   useEffect(() => {
     if (!inViewFirstTime && inView) {
       setInViewFirstTime(true);
@@ -127,7 +146,7 @@ const FilterValue: React.FC<FilterProps> = ({
       !isRefreshing &&
       (!isEqualWith(formData, newFormData, customizer) ||
         !isEqual(ownState, filterOwnState) ||
-        isDashboardRefreshing)
+        shouldRefresh)
     ) {
       setFormData(newFormData);
       setOwnState(filterOwnState);
@@ -147,22 +166,19 @@ const FilterValue: React.FC<FilterProps> = ({
             const result = 'result' in json ? json.result[0] : json;
 
             if (response.status === 200) {
-              setIsRefreshing(false);
-              setIsLoading(false);
               setState([result]);
+              handleFilterLoadFinish();
             } else if (response.status === 202) {
               waitForAsyncData(result)
                 .then((asyncResult: ChartDataResponseResult[]) => {
-                  setIsRefreshing(false);
-                  setIsLoading(false);
                   setState(asyncResult);
+                  handleFilterLoadFinish();
                 })
                 .catch((error: ClientErrorObject) => {
                   setError(
                     error.message || error.error || t('Check configuration'),
                   );
-                  setIsRefreshing(false);
-                  setIsLoading(false);
+                  handleFilterLoadFinish();
                 });
             } else {
               throw new Error(
@@ -172,25 +188,24 @@ const FilterValue: React.FC<FilterProps> = ({
           } else {
             setState(json.result);
             setError('');
-            setIsRefreshing(false);
-            setIsLoading(false);
+            handleFilterLoadFinish();
           }
         })
         .catch((error: Response) => {
           setError(error.statusText);
-          setIsRefreshing(false);
-          setIsLoading(false);
+          handleFilterLoadFinish();
         });
     }
   }, [
     inViewFirstTime,
     dependencies,
     datasetId,
     groupby,
+    handleFilterLoadFinish,
     JSON.stringify(filter),
     hasDataSource,
     isRefreshing,
-    isDashboardRefreshing,
+    shouldRefresh,
   ]);
 
   useEffect(() => {

diff --git a/superset-frontend/src/dashboard/reducers/dashboardState.js b/superset-frontend/src/dashboard/reducers/dashboardState.js
@@ -39,6 +39,8 @@ import {
   UNSET_FOCUSED_FILTER_FIELD,
   SET_ACTIVE_TABS,
   SET_FULL_SIZE_CHART_ID,
+  ON_FILTERS_REFRESH,
+  ON_FILTERS_REFRESH_SUCCESS,
 } from '../actions/dashboardState';
 import { HYDRATE_DASHBOARD } from '../actions/hydrate';
 
@@ -136,6 +138,18 @@ export default function dashboardStateReducer(state = {}, action) {
         isRefreshing: true,
       };
     },
+    [ON_FILTERS_REFRESH]() {
+      return {
+        ...state,
+        isFiltersRefreshing: true,
+      };
+    },
+    [ON_FILTERS_REFRESH_SUCCESS]() {
+      return {
+        ...state,
+        isFiltersRefreshing: false,
+      };
+    },
     [ON_REFRESH_SUCCESS]() {
       return {
         ...state,

diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts
@@ -63,6 +63,7 @@ export type DashboardState = {
   activeTabs: ActiveTabs;
   fullSizeChartId: number | null;
   isRefreshing: boolean;
+  isFiltersRefreshing: boolean;
   hasUnsavedChanges: boolean;
 };
 export type DashboardInfo = {