diff --git a/superset-frontend/src/dashboard/actions/dashboardState.js b/superset-frontend/src/dashboard/actions/dashboardState.js index 18b567e20f51..e19567d8c780 100644 --- a/superset-frontend/src/dashboard/actions/dashboardState.js +++ b/superset-frontend/src/dashboard/actions/dashboardState.js @@ -603,13 +603,21 @@ export function onRefresh( force = false, interval = 0, dashboardId, + isLazyLoad = false, ) { return dispatch => { - dispatch({ type: ON_REFRESH }); + // Only dispatch ON_REFRESH for dashboard-level refreshes + // Skip it for lazy-loaded tabs to prevent infinite loops + if (!isLazyLoad) { + dispatch({ type: ON_REFRESH }); + } + refreshCharts(chartList, force, interval, dashboardId, dispatch).then( () => { dispatch(onRefreshSuccess()); - dispatch(onFiltersRefresh()); + if (!isLazyLoad) { + dispatch(onFiltersRefresh()); + } }, ); }; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx index 8902e84a4675..b56066af05e6 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Fragment, useCallback, memo, useEffect } from 'react'; +import { Fragment, useCallback, memo, useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import { useDispatch, useSelector } from 'react-redux'; @@ -130,6 +130,9 @@ const Tab = props => { ); const dashboardInfo = useSelector(state => state.dashboardInfo); + // Track which refresh we've already handled to prevent duplicates + const handledRefreshRef = useRef(null); + useEffect(() => { if (props.renderType === RENDER_TAB_CONTENT && props.isComponentVisible) { if ( @@ -137,13 +140,20 @@ const Tab = props => { tabActivationTime && lastRefreshTime > tabActivationTime ) { - const chartIds = getChartIdsFromComponent(props.id, dashboardLayout); - if (chartIds.length > 0) { - requestAnimationFrame(() => { + // Create a unique key for this specific refresh + const refreshKey = `${props.id}-${lastRefreshTime}`; + + // Only proceed if we haven't already handled this refresh + if (handledRefreshRef.current !== refreshKey) { + handledRefreshRef.current = refreshKey; + + const chartIds = getChartIdsFromComponent(props.id, dashboardLayout); + if (chartIds.length > 0) { + // Use lazy load flag to avoid updating global refresh time setTimeout(() => { - dispatch(onRefresh(chartIds, true, 0, dashboardInfo.id)); + dispatch(onRefresh(chartIds, true, 0, dashboardInfo.id, true)); }, CHART_MOUNT_DELAY); - }); + } } } } diff --git a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx index a5072293ada2..7f0392fc5f48 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.test.tsx @@ -500,6 +500,7 @@ test('Should refresh charts when tab becomes active after dashboard refresh', as true, // Force refresh 0, // Interval 23, // Dashboard ID + true, // isLazyLoad flag ); }); @@ -540,3 +541,105 @@ test('Should not refresh charts when tab becomes active if no dashboard refresh expect(onRefresh).not.toHaveBeenCalled(); }); + +test('Should not cause infinite refresh loop with nested tabs - regression test', async () => { + jest.clearAllMocks(); + const getChartIdsFromComponent = require('src/dashboard/util/getChartIdsFromComponent'); + getChartIdsFromComponent.mockReset(); + getChartIdsFromComponent.mockReturnValue([201, 202]); + + const props = createProps(); + props.renderType = 'RENDER_TAB_CONTENT'; + props.isComponentVisible = false; + + const initialState = { + dashboardState: { + lastRefreshTime: Date.now() - 1000, // Dashboard was refreshed recently + tabActivationTimes: { + 'TAB-YT6eNksV-': Date.now() - 5000, // Tab was activated before refresh + }, + }, + dashboardInfo: { + id: 23, + dash_edit_perm: true, + }, + }; + + const { rerender } = render(, { + useRedux: true, + useDnd: true, + initialState, + }); + + // Initial state - no refresh should happen + expect(onRefresh).not.toHaveBeenCalled(); + + // Make tab visible - should trigger ONE refresh + rerender(); + + await waitFor( + () => { + expect(onRefresh).toHaveBeenCalledTimes(1); + }, + { timeout: 500 }, + ); + + // Clear the mock to track subsequent calls + jest.clearAllMocks(); + + // REGRESSION TEST: Multiple re-renders should NOT trigger additional refreshes + // This simulates the infinite loop scenario that was happening with nested tabs + for (let i = 0; i < 5; i++) { + rerender(); + await new Promise(resolve => setTimeout(resolve, 20)); + } + + expect(onRefresh).not.toHaveBeenCalled(); +}); + +test('Should use isLazyLoad flag for tab refreshes', async () => { + jest.clearAllMocks(); + const getChartIdsFromComponent = require('src/dashboard/util/getChartIdsFromComponent'); + getChartIdsFromComponent.mockReset(); + getChartIdsFromComponent.mockReturnValue([401, 402]); + + const props = createProps(); + props.renderType = 'RENDER_TAB_CONTENT'; + props.isComponentVisible = true; + + const initialState = { + dashboardState: { + lastRefreshTime: Date.now() - 1000, // Dashboard was refreshed recently + tabActivationTimes: { + 'TAB-YT6eNksV-': Date.now() - 5000, // Tab was activated before refresh + }, + }, + dashboardInfo: { + id: 42, + dash_edit_perm: true, + }, + }; + + render(, { + useRedux: true, + useDnd: true, + initialState, + }); + + // Tab should trigger refresh with isLazyLoad = true + await waitFor( + () => { + expect(onRefresh).toHaveBeenCalled(); + }, + { timeout: 500 }, + ); + + // Verify that isLazyLoad flag is set to true for tab refreshes + expect(onRefresh).toHaveBeenCalledWith( + [401, 402], + true, // force + 0, // interval + 42, // dashboardId + true, // isLazyLoad should be true to prevent infinite loops + ); +});