You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by gr...@apache.org on 2020/06/03 17:21:20 UTC
[incubator-superset] branch master updated: feat: [dashboard]
notification and warning for auto force refresh (#9886)
This is an automated email from the ASF dual-hosted git repository.
graceguo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git
The following commit(s) were added to refs/heads/master by this push:
new dcac860 feat: [dashboard] notification and warning for auto force refresh (#9886)
dcac860 is described below
commit dcac860f3e5528ecbc39e58f045c7388adb5c3d0
Author: Grace Guo <gr...@airbnb.com>
AuthorDate: Wed Jun 3 10:20:56 2020 -0700
feat: [dashboard] notification and warning for auto force refresh (#9886)
* feat: [dashboard] notification and warning for auto force refresh
* fix review comments
---
.../dashboard/components/Header_spec.jsx | 8 ++++
.../components/RefreshIntervalModal_spec.jsx | 19 +++++++-
.../src/dashboard/components/Header.jsx | 40 ++++++++++++++++-
.../dashboard/components/HeaderActionsDropdown.jsx | 14 ++++++
.../dashboard/components/RefreshIntervalModal.jsx | 50 +++++++++++++++++++++-
.../src/dashboard/components/SaveModal.jsx | 10 ++++-
.../src/dashboard/containers/DashboardHeader.jsx | 1 +
.../src/dashboard/reducers/dashboardState.js | 1 +
.../src/dashboard/reducers/getInitialState.js | 3 ++
.../src/dashboard/stylesheets/dashboard.less | 4 ++
.../src/datasource/DatasourceEditor.jsx | 2 +-
superset/config.py | 8 ++++
superset/views/base.py | 2 +
13 files changed, 156 insertions(+), 6 deletions(-)
diff --git a/superset-frontend/spec/javascripts/dashboard/components/Header_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/Header_spec.jsx
index 336a100..b2d7bc0 100644
--- a/superset-frontend/spec/javascripts/dashboard/components/Header_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/Header_spec.jsx
@@ -36,6 +36,10 @@ describe('Header', () => {
dash_edit_perm: true,
dash_save_perm: true,
userId: 1,
+ metadata: {},
+ common: {
+ conf: {},
+ },
},
dashboardTitle: 'title',
charts: {},
@@ -79,6 +83,7 @@ describe('Header', () => {
describe('read-only-user', () => {
const overrideProps = {
dashboardInfo: {
+ ...props.dashboardInfo,
id: 1,
dash_edit_perm: false,
dash_save_perm: false,
@@ -121,6 +126,7 @@ describe('Header', () => {
const overrideProps = {
editMode: false,
dashboardInfo: {
+ ...props.dashboardInfo,
id: 1,
dash_edit_perm: true,
dash_save_perm: true,
@@ -163,6 +169,7 @@ describe('Header', () => {
const overrideProps = {
editMode: true,
dashboardInfo: {
+ ...props.dashboardInfo,
id: 1,
dash_edit_perm: true,
dash_save_perm: true,
@@ -204,6 +211,7 @@ describe('Header', () => {
describe('logged-out-user', () => {
const overrideProps = {
dashboardInfo: {
+ ...props.dashboardInfo,
id: 1,
dash_edit_perm: false,
dash_save_perm: false,
diff --git a/superset-frontend/spec/javascripts/dashboard/components/RefreshIntervalModal_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/RefreshIntervalModal_spec.jsx
index 09cb78f..92f34ba 100644
--- a/superset-frontend/spec/javascripts/dashboard/components/RefreshIntervalModal_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/RefreshIntervalModal_spec.jsx
@@ -17,9 +17,11 @@
* under the License.
*/
import React from 'react';
-import { mount } from 'enzyme';
+import { mount, shallow } from 'enzyme';
+import ModalTrigger from 'src/components/ModalTrigger';
import RefreshIntervalModal from 'src/dashboard/components/RefreshIntervalModal';
+import { Modal, Alert } from 'react-bootstrap';
describe('RefreshIntervalModal', () => {
const mockedProps = {
@@ -44,7 +46,22 @@ describe('RefreshIntervalModal', () => {
it('should change refreshFrequency with edit mode', () => {
const wrapper = mount(<RefreshIntervalModal {...mockedProps} />);
wrapper.instance().handleFrequencyChange({ value: 30 });
+ wrapper.instance().onSave();
expect(mockedProps.onChange).toHaveBeenCalled();
expect(mockedProps.onChange).toHaveBeenCalledWith(30, mockedProps.editMode);
});
+ it('should show warning message', () => {
+ const props = {
+ ...mockedProps,
+ refreshLimit: 3600,
+ refreshWarning: 'Show warning',
+ };
+
+ const wrapper = shallow(<RefreshIntervalModal {...props} />);
+ wrapper.instance().handleFrequencyChange({ value: 30 });
+ expect(wrapper.find(ModalTrigger).dive().find(Alert)).toHaveLength(1);
+
+ wrapper.instance().handleFrequencyChange({ value: 3601 });
+ expect(wrapper.find(ModalTrigger).dive().find(Alert)).toHaveLength(0);
+ });
});
diff --git a/superset-frontend/src/dashboard/components/Header.jsx b/superset-frontend/src/dashboard/components/Header.jsx
index 89ba785..45c6727 100644
--- a/superset-frontend/src/dashboard/components/Header.jsx
+++ b/superset-frontend/src/dashboard/components/Header.jsx
@@ -17,6 +17,7 @@
* under the License.
*/
/* eslint-env browser */
+import moment from 'moment';
import React from 'react';
import PropTypes from 'prop-types';
import { CategoricalColorNamespace } from '@superset-ui/color';
@@ -46,6 +47,7 @@ import {
} from '../../logger/LogUtils';
import PropertiesModal from './PropertiesModal';
import setPeriodicRunner from '../util/setPeriodicRunner';
+import { options as PeriodicRefreshOptions } from './RefreshIntervalModal';
const propTypes = {
addSuccessToast: PropTypes.func.isRequired,
@@ -86,6 +88,7 @@ const propTypes = {
setMaxUndoHistoryExceeded: PropTypes.func.isRequired,
maxUndoHistoryToast: PropTypes.func.isRequired,
refreshFrequency: PropTypes.number.isRequired,
+ shouldPersistRefreshFrequency: PropTypes.bool.isRequired,
setRefreshFrequency: PropTypes.func.isRequired,
dashboardInfoChanged: PropTypes.func.isRequired,
dashboardTitleChanged: PropTypes.func.isRequired,
@@ -206,6 +209,18 @@ class Header extends React.PureComponent {
}
startPeriodicRender(interval) {
+ let intervalMessage;
+ if (interval) {
+ const predefinedValue = PeriodicRefreshOptions.find(
+ option => option.value === interval / 1000,
+ );
+ if (predefinedValue) {
+ intervalMessage = predefinedValue.label;
+ } else {
+ intervalMessage = moment.duration(interval, 'millisecond').humanize();
+ }
+ }
+
const periodicRender = () => {
const { fetchCharts, logEvent, charts, dashboardInfo } = this.props;
const { metadata } = dashboardInfo;
@@ -218,6 +233,13 @@ class Header extends React.PureComponent {
interval,
chartCount: affectedCharts.length,
});
+ this.props.addWarningToast(
+ t(
+ `This dashboard is currently force refreshing; the next force refresh will be in %s.`,
+ intervalMessage,
+ ),
+ );
+
return fetchCharts(
affectedCharts,
true,
@@ -249,7 +271,8 @@ class Header extends React.PureComponent {
colorNamespace,
colorScheme,
dashboardInfo,
- refreshFrequency,
+ refreshFrequency: currentRefreshFrequency,
+ shouldPersistRefreshFrequency,
} = this.props;
const scale = CategoricalColorNamespace.getScale(
@@ -257,6 +280,11 @@ class Header extends React.PureComponent {
colorNamespace,
);
const labelColors = colorScheme ? scale.getColorMap() : {};
+ // check refresh frequency is for current session or persist
+ const refreshFrequency = shouldPersistRefreshFrequency
+ ? currentRefreshFrequency
+ : dashboardInfo.metadata.refresh_frequency; // eslint-disable camelcase
+
const data = {
positions,
expanded_slices: expandedSlices,
@@ -318,11 +346,17 @@ class Header extends React.PureComponent {
hasUnsavedChanges,
isLoading,
refreshFrequency,
+ shouldPersistRefreshFrequency,
setRefreshFrequency,
} = this.props;
const userCanEdit = dashboardInfo.dash_edit_perm;
const userCanSaveAs = dashboardInfo.dash_save_perm;
+ const refreshLimit =
+ dashboardInfo.common.conf.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT;
+ const refreshWarning =
+ dashboardInfo.common.conf
+ .SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE;
const popButton = hasUnsavedChanges;
return (
@@ -474,6 +508,7 @@ class Header extends React.PureComponent {
addDangerToast={this.props.addDangerToast}
dashboardId={dashboardInfo.id}
dashboardTitle={dashboardTitle}
+ dashboardInfo={dashboardInfo}
layout={layout}
expandedSlices={expandedSlices}
customCss={customCss}
@@ -484,6 +519,7 @@ class Header extends React.PureComponent {
forceRefreshAllCharts={this.forceRefresh}
startPeriodicRender={this.startPeriodicRender}
refreshFrequency={refreshFrequency}
+ shouldPersistRefreshFrequency={shouldPersistRefreshFrequency}
setRefreshFrequency={setRefreshFrequency}
updateCss={updateCss}
editMode={editMode}
@@ -492,6 +528,8 @@ class Header extends React.PureComponent {
userCanSave={userCanSaveAs}
isLoading={isLoading}
showPropertiesModal={this.showPropertiesModal}
+ refreshLimit={refreshLimit}
+ refreshWarning={refreshWarning}
/>
</div>
</div>
diff --git a/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx b/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx
index 0097c0c..95189a1 100644
--- a/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx
+++ b/superset-frontend/src/dashboard/components/HeaderActionsDropdown.jsx
@@ -35,6 +35,7 @@ import { getActiveFilters } from '../util/activeDashboardFilters';
const propTypes = {
addSuccessToast: PropTypes.func.isRequired,
addDangerToast: PropTypes.func.isRequired,
+ dashboardInfo: PropTypes.object.isRequired,
dashboardId: PropTypes.number.isRequired,
dashboardTitle: PropTypes.string.isRequired,
hasUnsavedChanges: PropTypes.bool.isRequired,
@@ -45,6 +46,7 @@ const propTypes = {
updateCss: PropTypes.func.isRequired,
forceRefreshAllCharts: PropTypes.func.isRequired,
refreshFrequency: PropTypes.number.isRequired,
+ shouldPersistRefreshFrequency: PropTypes.bool.isRequired,
setRefreshFrequency: PropTypes.func.isRequired,
startPeriodicRender: PropTypes.func.isRequired,
editMode: PropTypes.bool.isRequired,
@@ -55,11 +57,15 @@ const propTypes = {
expandedSlices: PropTypes.object.isRequired,
onSave: PropTypes.func.isRequired,
showPropertiesModal: PropTypes.func.isRequired,
+ refreshLimit: PropTypes.number,
+ refreshWarning: PropTypes.string,
};
const defaultProps = {
colorNamespace: undefined,
colorScheme: undefined,
+ refreshLimit: 0,
+ refreshWarning: null,
};
class HeaderActionsDropdown extends React.PureComponent {
@@ -114,8 +120,10 @@ class HeaderActionsDropdown extends React.PureComponent {
const {
dashboardTitle,
dashboardId,
+ dashboardInfo,
forceRefreshAllCharts,
refreshFrequency,
+ shouldPersistRefreshFrequency,
editMode,
customCss,
colorNamespace,
@@ -127,6 +135,8 @@ class HeaderActionsDropdown extends React.PureComponent {
userCanEdit,
userCanSave,
isLoading,
+ refreshLimit,
+ refreshWarning,
} = this.props;
const emailTitle = t('Superset Dashboard');
@@ -147,10 +157,12 @@ class HeaderActionsDropdown extends React.PureComponent {
addDangerToast={this.props.addDangerToast}
dashboardId={dashboardId}
dashboardTitle={dashboardTitle}
+ dashboardInfo={dashboardInfo}
saveType={SAVE_TYPE_NEWDASHBOARD}
layout={layout}
expandedSlices={expandedSlices}
refreshFrequency={refreshFrequency}
+ shouldPersistRefreshFrequency={shouldPersistRefreshFrequency}
customCss={customCss}
colorNamespace={colorNamespace}
colorScheme={colorScheme}
@@ -180,6 +192,8 @@ class HeaderActionsDropdown extends React.PureComponent {
<RefreshIntervalModal
refreshFrequency={refreshFrequency}
+ refreshLimit={refreshLimit}
+ refreshWarning={refreshWarning}
onChange={this.changeRefreshInterval}
editMode={editMode}
triggerNode={
diff --git a/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx b/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx
index d98ef52..592c322 100644
--- a/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx
+++ b/superset-frontend/src/dashboard/components/RefreshIntervalModal.jsx
@@ -20,6 +20,7 @@ import React from 'react';
import PropTypes from 'prop-types';
import Select from 'src/components/Select';
import { t } from '@superset-ui/translation';
+import { Alert, Button } from 'react-bootstrap';
import ModalTrigger from '../../components/ModalTrigger';
@@ -28,9 +29,16 @@ const propTypes = {
refreshFrequency: PropTypes.number.isRequired,
onChange: PropTypes.func.isRequired,
editMode: PropTypes.bool.isRequired,
+ refreshLimit: PropTypes.number,
+ refreshWarning: PropTypes.string,
};
-const options = [
+const defaultProps = {
+ refreshLimit: 0,
+ refreshWarning: null,
+};
+
+export const options = [
[0, t("Don't refresh")],
[10, t('10 seconds')],
[30, t('30 seconds')],
@@ -46,10 +54,25 @@ const options = [
class RefreshIntervalModal extends React.PureComponent {
constructor(props) {
super(props);
+ this.modalRef = React.createRef();
this.state = {
refreshFrequency: props.refreshFrequency,
};
this.handleFrequencyChange = this.handleFrequencyChange.bind(this);
+ this.onSave = this.onSave.bind(this);
+ this.onCancel = this.onCancel.bind(this);
+ }
+
+ onSave() {
+ this.props.onChange(this.state.refreshFrequency, this.props.editMode);
+ this.modalRef.current.close();
+ }
+
+ onCancel() {
+ this.setState({
+ refreshFrequency: this.props.refreshFrequency,
+ });
+ this.modalRef.current.close();
}
handleFrequencyChange(opt) {
@@ -57,12 +80,17 @@ class RefreshIntervalModal extends React.PureComponent {
this.setState({
refreshFrequency: value,
});
- this.props.onChange(value, this.props.editMode);
}
render() {
+ const { refreshLimit = 0, refreshWarning, editMode } = this.props;
+ const { refreshFrequency = 0 } = this.state;
+ const showRefreshWarning =
+ !!refreshFrequency && !!refreshWarning && refreshFrequency < refreshLimit;
+
return (
<ModalTrigger
+ ref={this.modalRef}
triggerNode={this.props.triggerNode}
isMenuItem
modalTitle={t('Refresh Interval')}
@@ -74,12 +102,30 @@ class RefreshIntervalModal extends React.PureComponent {
value={this.state.refreshFrequency}
onChange={this.handleFrequencyChange}
/>
+ {showRefreshWarning && (
+ <div className="refresh-warning-container">
+ <Alert bsStyle="warning">
+ <div>{refreshWarning}</div>
+ <br />
+ <strong>{t('Are you sure you want to proceed?')}</strong>
+ </Alert>
+ </div>
+ )}
</div>
}
+ modalFooter={
+ <>
+ <Button bsStyle="primary" onClick={this.onSave}>
+ {editMode ? t('Save') : t('Save for this session')}
+ </Button>
+ <Button onClick={this.onCancel}>{t('Cancel')}</Button>
+ </>
+ }
/>
);
}
}
RefreshIntervalModal.propTypes = propTypes;
+RefreshIntervalModal.defaultProps = defaultProps;
export default RefreshIntervalModal;
diff --git a/superset-frontend/src/dashboard/components/SaveModal.jsx b/superset-frontend/src/dashboard/components/SaveModal.jsx
index e8dcb91..16a5d1e 100644
--- a/superset-frontend/src/dashboard/components/SaveModal.jsx
+++ b/superset-frontend/src/dashboard/components/SaveModal.jsx
@@ -32,6 +32,7 @@ const propTypes = {
addDangerToast: PropTypes.func.isRequired,
dashboardId: PropTypes.number.isRequired,
dashboardTitle: PropTypes.string.isRequired,
+ dashboardInfo: PropTypes.object.isRequired,
expandedSlices: PropTypes.object.isRequired,
layout: PropTypes.object.isRequired,
saveType: PropTypes.oneOf([SAVE_TYPE_OVERWRITE, SAVE_TYPE_NEWDASHBOARD]),
@@ -94,13 +95,15 @@ class SaveModal extends React.PureComponent {
const { saveType, newDashName } = this.state;
const {
dashboardTitle,
+ dashboardInfo,
layout: positions,
customCss,
colorNamespace,
colorScheme,
expandedSlices,
dashboardId,
- refreshFrequency,
+ refreshFrequency: currentRefreshFrequency,
+ shouldPersistRefreshFrequency,
} = this.props;
const scale = CategoricalColorNamespace.getScale(
@@ -108,6 +111,11 @@ class SaveModal extends React.PureComponent {
colorNamespace,
);
const labelColors = colorScheme ? scale.getColorMap() : {};
+ // check refresh frequency is for current session or persist
+ const refreshFrequency = shouldPersistRefreshFrequency
+ ? currentRefreshFrequency
+ : dashboardInfo.metadata.refresh_frequency; // eslint-disable camelcase
+
const data = {
positions,
css: customCss,
diff --git a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
index 9516f24..962fc9a 100644
--- a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
+++ b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
@@ -71,6 +71,7 @@ function mapStateToProps({
).text,
expandedSlices: dashboardState.expandedSlices,
refreshFrequency: dashboardState.refreshFrequency,
+ shouldPersistRefreshFrequency: !!dashboardState.shouldPersistRefreshFrequency,
customCss: dashboardState.css,
colorNamespace: dashboardState.colorNamespace,
colorScheme: dashboardState.colorScheme,
diff --git a/superset-frontend/src/dashboard/reducers/dashboardState.js b/superset-frontend/src/dashboard/reducers/dashboardState.js
index 61c9a8b..fc7e079 100644
--- a/superset-frontend/src/dashboard/reducers/dashboardState.js
+++ b/superset-frontend/src/dashboard/reducers/dashboardState.js
@@ -120,6 +120,7 @@ export default function dashboardStateReducer(state = {}, action) {
return {
...state,
refreshFrequency: action.refreshFrequency,
+ shouldPersistRefreshFrequency: action.isPersistent,
hasUnsavedChanges: action.isPersistent,
};
},
diff --git a/superset-frontend/src/dashboard/reducers/getInitialState.js b/superset-frontend/src/dashboard/reducers/getInitialState.js
index bd27618..3d4e38a 100644
--- a/superset-frontend/src/dashboard/reducers/getInitialState.js
+++ b/superset-frontend/src/dashboard/reducers/getInitialState.js
@@ -292,6 +292,9 @@ export default function (bootstrapData) {
focusedFilterField: [],
expandedSlices: dashboard.metadata.expanded_slices || {},
refreshFrequency: dashboard.metadata.refresh_frequency || 0,
+ // dashboard viewers can set refresh frequency for the current visit,
+ // only persistent refreshFrequency will be saved to backend
+ shouldPersistRefreshFrequency: false,
css: dashboard.css || '',
colorNamespace: dashboard.metadata.color_namespace,
colorScheme: dashboard.metadata.color_scheme,
diff --git a/superset-frontend/src/dashboard/stylesheets/dashboard.less b/superset-frontend/src/dashboard/stylesheets/dashboard.less
index fb7593f..2d99763 100644
--- a/superset-frontend/src/dashboard/stylesheets/dashboard.less
+++ b/superset-frontend/src/dashboard/stylesheets/dashboard.less
@@ -165,6 +165,10 @@ body {
width: 80%;
}
+ .refresh-warning-container {
+ margin-top: 24px;
+ }
+
.dashboard-modal-actions-container {
margin-top: 24px;
text-align: right;
diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx
index bb56fd2..114a36d 100644
--- a/superset-frontend/src/datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/datasource/DatasourceEditor.jsx
@@ -731,7 +731,7 @@ export class DatasourceEditor extends React.PureComponent {
<div>
<div className="m-t-10">
<Alert bsStyle="warning">
- <span className="bold">{t('Be careful.')} </span>
+ <strong>{t('Be careful.')} </strong>
{t(
'Changing these settings will affect all charts using this datasource, including charts owned by other people.',
)}
diff --git a/superset/config.py b/superset/config.py
index 32ce4ae..738c251 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -126,6 +126,14 @@ SUPERSET_WEBSERVER_PORT = 8088
# (gunicorn, nginx, apache, ...) timeout setting to be <= to this setting
SUPERSET_WEBSERVER_TIMEOUT = 60
+# this 2 settings are used by dashboard period force refresh feature
+# When user choose auto force refresh frequency
+# < SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT
+# they will see warning message in the Refresh Interval Modal.
+# please check PR #9886
+SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT = 0
+SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE = None
+
SUPERSET_DASHBOARD_POSITION_DATA_LIMIT = 65535
CUSTOM_SECURITY_MANAGER = None
SQLALCHEMY_TRACK_MODIFICATIONS = False
diff --git a/superset/views/base.py b/superset/views/base.py
index 6995fd1..5821619 100644
--- a/superset/views/base.py
+++ b/superset/views/base.py
@@ -61,6 +61,8 @@ if TYPE_CHECKING:
FRONTEND_CONF_KEYS = (
"SUPERSET_WEBSERVER_TIMEOUT",
"SUPERSET_DASHBOARD_POSITION_DATA_LIMIT",
+ "SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT",
+ "SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE",
"ENABLE_JAVASCRIPT_CONTROLS",
"DEFAULT_SQLLAB_LIMIT",
"SQL_MAX_ROW",