Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions superset-frontend/src/dashboard/actions/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
},
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -130,20 +130,30 @@ 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 (
lastRefreshTime &&
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);
});
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
});

Expand Down Expand Up @@ -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(<Tab {...props} />, {
useRedux: true,
useDnd: true,
initialState,
});

// Initial state - no refresh should happen
expect(onRefresh).not.toHaveBeenCalled();

// Make tab visible - should trigger ONE refresh
rerender(<Tab {...props} isComponentVisible />);

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(<Tab {...props} isComponentVisible />);
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(<Tab {...props} />, {
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
);
});
Loading