[fix] Add Auto Refresh Dashboard user event into dashboard logging (#…

…9087)

1. add logging
2. refactor general periodical render function

superset/assets/spec/javascripts/dashboard/components/Header_spec.jsx

@@ -51,7 +51,6 @@ describe('Header', () => {
     saveFaveStar: () => {},
     savePublished: () => {},
     isPublished: () => {},
-    startPeriodicRender: () => {},
     updateDashboardTitle: () => {},
     editMode: false,
     setEditMode: () => {},

superset/assets/src/chart/chartAction.js

@@ -355,8 +355,12 @@ export function redirectSQLLab(formData) {
   };
 }
 
-export function refreshChart(chart, force, timeout) {
-  return dispatch => {
+export function refreshChart(chartKey, force) {
+  return (dispatch, getState) => {
+    const chart = (getState().charts || {})[chartKey];
+    const timeout = getState().dashboardInfo.common.conf
+      .SUPERSET_WEBSERVER_TIMEOUT;
+
     if (
       !chart.latestQueryFormData ||
       Object.keys(chart.latestQueryFormData).length === 0

superset/assets/src/dashboard/actions/dashboardState.js

@@ -220,10 +220,8 @@ export function saveDashboardRequest(data, id, saveType) {
 
 export function fetchCharts(chartList = [], force = false, interval = 0) {
   return (dispatch, getState) => {
-    const timeout = getState().dashboardInfo.common.conf
-      .SUPERSET_WEBSERVER_TIMEOUT;
     if (!interval) {
-      chartList.forEach(chart => dispatch(refreshChart(chart, force, timeout)));
+      chartList.forEach(chartKey => dispatch(refreshChart(chartKey, force)));
       return;
     }
 
@@ -238,46 +236,12 @@ export function fetchCharts(chartList = [], force = false, interval = 0) {
     const delay = meta.stagger_refresh
       ? refreshTime / (chartList.length - 1)
       : 0;
-    chartList.forEach((chart, i) => {
-      setTimeout(
-        () => dispatch(refreshChart(chart, force, timeout)),
-        delay * i,
-      );
+    chartList.forEach((chartKey, i) => {
+      setTimeout(() => dispatch(refreshChart(chartKey, force)), delay * i);
     });
   };
 }
 
-let refreshTimer = null;
-export function startPeriodicRender(interval) {
-  const stopPeriodicRender = () => {
-    if (refreshTimer) {
-      clearTimeout(refreshTimer);
-      refreshTimer = null;
-    }
-  };
-
-  return (dispatch, getState) => {
-    stopPeriodicRender();
-
-    const { metadata } = getState().dashboardInfo;
-    const immune = metadata.timed_refresh_immune_slices || [];
-    const refreshAll = () => {
-      const affected = Object.values(getState().charts).filter(
-        chart => immune.indexOf(chart.id) === -1,
-      );
-      return dispatch(fetchCharts(affected, true, interval * 0.2));
-    };
-    const fetchAndRender = () => {
-      refreshAll();
-      if (interval > 0) {
-        refreshTimer = setTimeout(fetchAndRender, interval);
-      }
-    };
-
-    fetchAndRender();
-  };
-}
-
 export const SHOW_BUILDER_PANE = 'SHOW_BUILDER_PANE';
 export function showBuilderPane(builderPaneType) {
   return { type: SHOW_BUILDER_PANE, builderPaneType };

superset/assets/src/dashboard/components/Header.jsx

@@ -45,6 +45,7 @@ import {
   LOG_ACTIONS_TOGGLE_EDIT_DASHBOARD,
 } from '../../logger/LogUtils';
 import PropertiesModal from './PropertiesModal';
+import setPeriodicRunner from '../util/setPeriodicRunner';
 
 const propTypes = {
   addSuccessToast: PropTypes.func.isRequired,
@@ -67,7 +68,6 @@ const propTypes = {
   fetchCharts: PropTypes.func.isRequired,
   saveFaveStar: PropTypes.func.isRequired,
   savePublished: PropTypes.func.isRequired,
-  startPeriodicRender: PropTypes.func.isRequired,
   updateDashboardTitle: PropTypes.func.isRequired,
   editMode: PropTypes.bool.isRequired,
   setEditMode: PropTypes.func.isRequired,
@@ -126,7 +126,7 @@ class Header extends React.PureComponent {
 
   componentDidMount() {
     const refreshFrequency = this.props.refreshFrequency;
-    this.props.startPeriodicRender(refreshFrequency * 1000);
+    this.startPeriodicRender(refreshFrequency * 1000);
   }
 
   UNSAFE_componentWillReceiveProps(nextProps) {
@@ -188,7 +188,7 @@ class Header extends React.PureComponent {
 
   forceRefresh() {
     if (!this.props.isLoading) {
-      const chartList = Object.values(this.props.charts);
+      const chartList = Object.keys(this.props.charts);
       this.props.logEvent(LOG_ACTIONS_FORCE_REFRESH_DASHBOARD, {
         force: true,
         interval: 0,
@@ -200,11 +200,26 @@ class Header extends React.PureComponent {
   }
 
   startPeriodicRender(interval) {
-    this.props.logEvent(LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, {
-      force: true,
+    const periodicRender = () => {
+      const { fetchCharts, logEvent, charts, dashboardInfo } = this.props;
+      const { metadata } = dashboardInfo;
+      const immune = metadata.timed_refresh_immune_slices || [];
+      const affectedCharts = Object.values(charts)
+        .filter(chart => immune.indexOf(chart.id) === -1)
+        .map(chart => chart.id);
+
+      logEvent(LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, {
+        interval,
+        chartCount: affectedCharts.length,
+      });
+      return fetchCharts(affectedCharts, true, interval * 0.2);
+    };
+
+    this.refreshTimer = setPeriodicRunner({
       interval,
+      periodicRender,
+      refreshTimer: this.refreshTimer,
     });
-    return this.props.startPeriodicRender(interval);
   }
 
   toggleEditMode() {

superset/assets/src/dashboard/components/gridComponents/Chart.jsx

@@ -205,7 +205,7 @@ class Chart extends React.Component {
       slice_id: this.props.slice.slice_id,
       is_cached: this.props.isCached,
     });
-    return this.props.refreshChart(this.props.chart, true, this.props.timeout);
+    return this.props.refreshChart(this.props.chart.id, true);
   }
 
   render() {

superset/assets/src/dashboard/containers/DashboardHeader.jsx

@@ -31,7 +31,6 @@ import {
   saveFaveStar,
   savePublished,
   fetchCharts,
-  startPeriodicRender,
   updateCss,
   onChange,
   saveDashboardRequest,
@@ -101,7 +100,6 @@ function mapDispatchToProps(dispatch) {
       saveFaveStar,
       savePublished,
       fetchCharts,
-      startPeriodicRender,
       updateDashboardTitle,
       updateCss,
       onChange,

superset/assets/src/dashboard/util/setPeriodicRunner.js

@@ -0,0 +1,36 @@
+/**
+ * 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.
+ */
+const stopPeriodicRender = refreshTimer => {
+  if (refreshTimer) {
+    clearInterval(refreshTimer);
+  }
+};
+
+export default function setPeriodicRunner({
+  interval = 0,
+  periodicRender,
+  refreshTimer,
+}) {
+  stopPeriodicRender(refreshTimer);
+
+  if (interval > 0) {
+    return setInterval(periodicRender, interval);
+  }
+  return 0;
+}

superset/assets/src/logger/LogUtils.js

@@ -52,6 +52,7 @@ export const LOG_EVENT_TYPE_USER = new Set([
   LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS,
   LOG_ACTIONS_TOGGLE_EDIT_DASHBOARD,
   LOG_ACTIONS_FORCE_REFRESH_DASHBOARD,
+  LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD,
   LOG_ACTIONS_OMNIBAR_TRIGGERED,
   LOG_ACTIONS_MOUNT_EXPLORER,
   LOG_ACTIONS_RENDER_CHART_CONTAINER,