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 <grace....@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",