Antonio-RiveroMartnez commented on code in PR #31029:
URL: https://github.com/apache/superset/pull/31029#discussion_r1853832007
##########
superset-frontend/src/dashboard/components/Header/index.jsx:
##########
@@ -383,58 +403,79 @@ class Header extends PureComponent {
certification_details: dashboardInfo.certification_details,
css: customCss,
dashboard_title: dashboardTitle,
- last_modified_time: lastModifiedTime,
+ last_modified_time: actualLastModifiedTime,
owners: dashboardInfo.owners,
roles: dashboardInfo.roles,
slug,
metadata: {
...dashboardInfo?.metadata,
color_namespace: currentColorNamespace,
color_scheme: currentColorScheme,
- positions,
- refresh_frequency: refreshFrequency,
+ positions: layout,
+ refresh_frequency: shouldPersistRefreshFrequency
+ ? refreshFrequency
+ : dashboardInfo.metadata?.refresh_frequency,
},
};
// make sure positions data less than DB storage limitation:
- const positionJSONLength = safeStringify(positions).length;
+ const positionJSONLength = safeStringify(layout).length;
const limit =
- dashboardInfo.common.conf.SUPERSET_DASHBOARD_POSITION_DATA_LIMIT ||
+ dashboardInfo.common?.conf?.SUPERSET_DASHBOARD_POSITION_DATA_LIMIT ||
DASHBOARD_POSITION_DATA_LIMIT;
if (positionJSONLength >= limit) {
- this.props.addDangerToast(
+ boundActionCreators.addDangerToast(
t(
'Your dashboard is too large. Please reduce its size before saving
it.',
),
);
} else {
if (positionJSONLength >= limit * 0.9) {
- this.props.addWarningToast('Your dashboard is near the size limit.');
+ boundActionCreators.addWarningToast(
+ 'Your dashboard is near the size limit.',
Review Comment:
Should we translate this?
##########
superset-frontend/src/dashboard/components/Header/index.jsx:
##########
@@ -170,209 +146,253 @@ const discardBtnStyle = theme => css`
height: ${theme.gridUnit * 8}px;
`;
-class Header extends PureComponent {
- static discardChanges() {
- const url = new URL(window.location.href);
+const discardChanges = () => {
+ const url = new URL(window.location.href);
- url.searchParams.delete('edit');
- window.location.assign(url);
- }
+ url.searchParams.delete('edit');
+ window.location.assign(url);
+};
- constructor(props) {
- super(props);
- this.state = {
- didNotifyMaxUndoHistoryToast: false,
- emphasizeUndo: false,
- emphasizeRedo: false,
- showingPropertiesModal: false,
- isDropdownVisible: false,
- };
+const Header = () => {
+ const dispatch = useDispatch();
+ const [didNotifyMaxUndoHistoryToast, setDidNotifyMaxUndoHistoryToast] =
+ useState(false);
+ const [emphasizeUndo, setEmphasizeUndo] = useState(false);
+ const [emphasizeRedo, setEmphasizeRedo] = useState(false);
+ const [showingPropertiesModal, setShowingPropertiesModal] = useState(false);
+ const [isDropdownVisible, setIsDropdownVisible] = useState(false);
+ const [showingEmbedModal, setShowingEmbedModal] = useState(false);
+ const dashboardInfo = useSelector(state => state.dashboardInfo);
+ const layout = useSelector(state => state.dashboardLayout.present);
+ const undoLength = useSelector(state => state.dashboardLayout.past.length);
+ const redoLength = useSelector(state => state.dashboardLayout.future.length);
+ const dataMask = useSelector(state => state.dataMask);
+ const user = useSelector(state => state.user);
+ const chartIds = useChartIds();
+
+ const {
+ expandedSlices,
+ refreshFrequency,
+ shouldPersistRefreshFrequency,
+ customCss,
+ colorNamespace,
+ colorScheme,
+ isStarred,
+ isPublished,
+ hasUnsavedChanges,
+ maxUndoHistoryExceeded,
+ editMode,
+ lastModifiedTime,
+ } = useSelector(
+ state => ({
+ expandedSlices: state.dashboardState.expandedSlices,
+ refreshFrequency: state.dashboardState.refreshFrequency,
+ shouldPersistRefreshFrequency:
+ !!state.dashboardState.shouldPersistRefreshFrequency,
+ customCss: state.dashboardState.css,
+ colorNamespace: state.dashboardState.colorNamespace,
+ colorScheme: state.dashboardState.colorScheme,
+ isStarred: !!state.dashboardState.isStarred,
+ isPublished: !!state.dashboardState.isPublished,
+ hasUnsavedChanges: !!state.dashboardState.hasUnsavedChanges,
+ maxUndoHistoryExceeded: !!state.dashboardState.maxUndoHistoryExceeded,
+ editMode: !!state.dashboardState.editMode,
+ lastModifiedTime: state.lastModifiedTime,
+ }),
+ shallowEqual,
+ );
+ const isLoading = useSelector(state => isDashboardLoading(state.charts));
+
+ const refreshTimer = useRef(0);
+ const ctrlYTimeout = useRef(0);
+ const ctrlZTimeout = useRef(0);
+
+ const dashboardTitle = layout[DASHBOARD_HEADER_ID]?.meta?.text;
+ const { slug } = dashboardInfo;
+ const actualLastModifiedTime = Math.max(
+ lastModifiedTime,
+ dashboardInfo.last_modified_time,
+ );
+ const boundActionCreators = useMemo(
+ () =>
+ bindActionCreators(
+ {
+ addSuccessToast,
+ addDangerToast,
+ addWarningToast,
+ onUndo: undoLayoutAction,
+ onRedo: redoLayoutAction,
+ setEditMode,
+ setUnsavedChanges,
+ fetchFaveStar,
+ saveFaveStar,
+ savePublished,
+ fetchCharts,
+ updateDashboardTitle,
+ updateCss,
+ onChange,
+ onSave: saveDashboardRequest,
+ setMaxUndoHistoryExceeded,
+ maxUndoHistoryToast,
+ logEvent,
+ setRefreshFrequency,
+ onRefresh,
+ dashboardInfoChanged,
+ dashboardTitleChanged,
+ },
+ dispatch,
+ ),
+ [dispatch],
+ );
+
+ const startPeriodicRender = useCallback(
+ interval => {
+ let intervalMessage;
+
+ if (interval) {
+ const periodicRefreshOptions =
+ dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS;
+ const predefinedValue = periodicRefreshOptions.find(
+ option => Number(option[0]) === interval / 1000,
+ );
- this.handleChangeText = this.handleChangeText.bind(this);
- this.handleCtrlZ = this.handleCtrlZ.bind(this);
- this.handleCtrlY = this.handleCtrlY.bind(this);
- this.toggleEditMode = this.toggleEditMode.bind(this);
- this.forceRefresh = this.forceRefresh.bind(this);
- this.startPeriodicRender = this.startPeriodicRender.bind(this);
- this.overwriteDashboard = this.overwriteDashboard.bind(this);
- this.showPropertiesModal = this.showPropertiesModal.bind(this);
- this.hidePropertiesModal = this.hidePropertiesModal.bind(this);
- this.setIsDropdownVisible = this.setIsDropdownVisible.bind(this);
- }
+ if (predefinedValue) {
+ intervalMessage = t(predefinedValue[1]);
+ } else {
+ intervalMessage = moment.duration(interval,
'millisecond').humanize();
+ }
+ }
- componentDidMount() {
- const { refreshFrequency } = this.props;
- this.startPeriodicRender(refreshFrequency * 1000);
- }
+ const periodicRender = () => {
+ const { metadata } = dashboardInfo;
+ const immune = metadata.timed_refresh_immune_slices || [];
+ const affectedCharts = chartIds.filter(
+ chartId => immune.indexOf(chartId) === -1,
+ );
- componentDidUpdate(prevProps) {
- if (this.props.refreshFrequency !== prevProps.refreshFrequency) {
- const { refreshFrequency } = this.props;
- this.startPeriodicRender(refreshFrequency * 1000);
- }
- }
+ boundActionCreators.logEvent(LOG_ACTIONS_PERIODIC_RENDER_DASHBOARD, {
+ interval,
+ chartCount: affectedCharts.length,
+ });
+ boundActionCreators.addWarningToast(
+ t(
+ `This dashboard is currently auto refreshing; the next auto
refresh will be in %s.`,
+ intervalMessage,
+ ),
+ );
+ if (
+ dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_MODE === 'fetch'
+ ) {
+ // force-refresh while auto-refresh in dashboard
+ return boundActionCreators.fetchCharts(
Review Comment:
nit: I know it was like this before this PR, but we could DRY it with a
helper function so you can pass the boolean as arg and return the `fetchCharts`
call with it here and the one below
##########
superset-frontend/src/dashboard/components/Header/index.jsx:
##########
@@ -170,209 +146,253 @@ const discardBtnStyle = theme => css`
height: ${theme.gridUnit * 8}px;
`;
-class Header extends PureComponent {
- static discardChanges() {
- const url = new URL(window.location.href);
+const discardChanges = () => {
+ const url = new URL(window.location.href);
- url.searchParams.delete('edit');
- window.location.assign(url);
- }
+ url.searchParams.delete('edit');
+ window.location.assign(url);
+};
- constructor(props) {
- super(props);
- this.state = {
- didNotifyMaxUndoHistoryToast: false,
- emphasizeUndo: false,
- emphasizeRedo: false,
- showingPropertiesModal: false,
- isDropdownVisible: false,
- };
+const Header = () => {
+ const dispatch = useDispatch();
+ const [didNotifyMaxUndoHistoryToast, setDidNotifyMaxUndoHistoryToast] =
+ useState(false);
+ const [emphasizeUndo, setEmphasizeUndo] = useState(false);
+ const [emphasizeRedo, setEmphasizeRedo] = useState(false);
+ const [showingPropertiesModal, setShowingPropertiesModal] = useState(false);
+ const [isDropdownVisible, setIsDropdownVisible] = useState(false);
+ const [showingEmbedModal, setShowingEmbedModal] = useState(false);
+ const dashboardInfo = useSelector(state => state.dashboardInfo);
+ const layout = useSelector(state => state.dashboardLayout.present);
+ const undoLength = useSelector(state => state.dashboardLayout.past.length);
+ const redoLength = useSelector(state => state.dashboardLayout.future.length);
+ const dataMask = useSelector(state => state.dataMask);
+ const user = useSelector(state => state.user);
+ const chartIds = useChartIds();
+
+ const {
+ expandedSlices,
+ refreshFrequency,
+ shouldPersistRefreshFrequency,
+ customCss,
+ colorNamespace,
+ colorScheme,
+ isStarred,
+ isPublished,
+ hasUnsavedChanges,
+ maxUndoHistoryExceeded,
+ editMode,
+ lastModifiedTime,
+ } = useSelector(
+ state => ({
+ expandedSlices: state.dashboardState.expandedSlices,
+ refreshFrequency: state.dashboardState.refreshFrequency,
+ shouldPersistRefreshFrequency:
+ !!state.dashboardState.shouldPersistRefreshFrequency,
+ customCss: state.dashboardState.css,
+ colorNamespace: state.dashboardState.colorNamespace,
+ colorScheme: state.dashboardState.colorScheme,
+ isStarred: !!state.dashboardState.isStarred,
+ isPublished: !!state.dashboardState.isPublished,
+ hasUnsavedChanges: !!state.dashboardState.hasUnsavedChanges,
+ maxUndoHistoryExceeded: !!state.dashboardState.maxUndoHistoryExceeded,
+ editMode: !!state.dashboardState.editMode,
+ lastModifiedTime: state.lastModifiedTime,
+ }),
+ shallowEqual,
+ );
+ const isLoading = useSelector(state => isDashboardLoading(state.charts));
+
+ const refreshTimer = useRef(0);
+ const ctrlYTimeout = useRef(0);
+ const ctrlZTimeout = useRef(0);
+
+ const dashboardTitle = layout[DASHBOARD_HEADER_ID]?.meta?.text;
+ const { slug } = dashboardInfo;
+ const actualLastModifiedTime = Math.max(
+ lastModifiedTime,
+ dashboardInfo.last_modified_time,
+ );
+ const boundActionCreators = useMemo(
+ () =>
+ bindActionCreators(
+ {
+ addSuccessToast,
+ addDangerToast,
+ addWarningToast,
+ onUndo: undoLayoutAction,
+ onRedo: redoLayoutAction,
+ setEditMode,
+ setUnsavedChanges,
+ fetchFaveStar,
+ saveFaveStar,
+ savePublished,
+ fetchCharts,
+ updateDashboardTitle,
+ updateCss,
+ onChange,
+ onSave: saveDashboardRequest,
+ setMaxUndoHistoryExceeded,
+ maxUndoHistoryToast,
+ logEvent,
+ setRefreshFrequency,
+ onRefresh,
+ dashboardInfoChanged,
+ dashboardTitleChanged,
+ },
+ dispatch,
+ ),
+ [dispatch],
+ );
+
+ const startPeriodicRender = useCallback(
+ interval => {
+ let intervalMessage;
+
+ if (interval) {
+ const periodicRefreshOptions =
+ dashboardInfo.common?.conf?.DASHBOARD_AUTO_REFRESH_INTERVALS;
Review Comment:
nit: Is this value dynamically changing somewhere or just a static config
loaded once? If static, we could move this out the callback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
