This is an automated email from the ASF dual-hosted git repository.
sadpandajoe pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 8141d666d6d fix(dashboard): show loading spinner for charts outside an
in-flight auto-refresh batch (#44272)
8141d666d6d is described below
commit 8141d666d6decade7de8406a0cc40c705289517d
Author: Joe Li <[email protected]>
AuthorDate: Wed Sep 23 16:17:57 2026 -0700
fix(dashboard): show loading spinner for charts outside an in-flight
auto-refresh batch (#44272)
Co-authored-by: Claude Sonnet 5 <[email protected]>
Co-authored-by: Evan Rusackas <[email protected]>
Co-authored-by: Evan Rusackas <[email protected]>
---
.../Header/useHeaderAutoRefresh.test.tsx | 58 ++++++++++++++++--
.../components/Header/useHeaderAutoRefresh.ts | 2 +-
.../components/gridComponents/Chart/Chart.test.tsx | 59 ++++++++++++++++++
.../components/gridComponents/Chart/Chart.tsx | 4 +-
.../dashboard/contexts/AutoRefreshContext.test.tsx | 69 ++++++++++++++++++++++
.../src/dashboard/contexts/AutoRefreshContext.tsx | 31 +++++++++-
6 files changed, 213 insertions(+), 10 deletions(-)
diff --git
a/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.test.tsx
b/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.test.tsx
index 65d962c5ec0..af87623ae8f 100644
---
a/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.test.tsx
+++
b/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.test.tsx
@@ -23,11 +23,15 @@ import { ReactNode } from 'react';
import { LOG_ACTIONS_FORCE_REFRESH_DASHBOARD } from 'src/logger/LogUtils';
import { useHeaderAutoRefresh } from './useHeaderAutoRefresh';
+const mockStartAutoRefresh = jest.fn();
+const mockEndAutoRefresh = jest.fn();
+const mockSetRefreshInFlight = jest.fn();
+
jest.mock('src/dashboard/contexts/AutoRefreshContext', () => ({
useAutoRefreshContext: () => ({
- startAutoRefresh: jest.fn(),
- endAutoRefresh: jest.fn(),
- setRefreshInFlight: jest.fn(),
+ startAutoRefresh: mockStartAutoRefresh,
+ endAutoRefresh: mockEndAutoRefresh,
+ setRefreshInFlight: mockSetRefreshInFlight,
}),
}));
@@ -45,8 +49,10 @@ jest.mock('src/dashboard/hooks/useRealTimeDashboard', () =>
({
}),
}));
+const mockUseAutoRefreshTabPause = jest.fn();
jest.mock('src/dashboard/hooks/useAutoRefreshTabPause', () => ({
- useAutoRefreshTabPause: jest.fn(),
+ useAutoRefreshTabPause: (...args: unknown[]) =>
+ mockUseAutoRefreshTabPause(...args),
}));
const createWrapper = (conf: Record<string, unknown> = {}) => {
@@ -54,6 +60,8 @@ const createWrapper = (conf: Record<string, unknown> = {}) =>
{
charts: {
1: { latestQueryFormData: { datasource: '1__table' } },
2: { latestQueryFormData: { datasource: '2__table' } },
+ // A chart on a tab that has never been visited has no query data yet.
+ 3: { latestQueryFormData: {} },
},
dashboardInfo: {
common: { conf },
@@ -152,3 +160,45 @@ test('forceRefresh normalizes a negative config value to 0
(unstaggered)', async
expect.objectContaining({ interval: 0 }),
);
});
+
+test('a silent refresh reports only the affected chart ids to
startAutoRefresh, not the whole dashboard', async () => {
+ mockStartAutoRefresh.mockClear();
+ mockUseAutoRefreshTabPause.mockClear();
+ const { props } = renderHeaderAutoRefresh(
+ {},
+ { chartIds: [1, 2], timedRefreshImmuneSlices: [2] },
+ );
+
+ const { onRefresh: handleTabVisibilityRefresh } =
+ mockUseAutoRefreshTabPause.mock.calls[0][0];
+
+ await act(async () => {
+ await handleTabVisibilityRefresh();
+ });
+
+ expect(props.onRefresh).toHaveBeenCalledTimes(1);
+ expect(mockStartAutoRefresh).toHaveBeenCalledWith([1]);
+ expect(mockStartAutoRefresh).not.toHaveBeenCalledWith([1, 2]);
+});
+
+test('a silent refresh excludes charts with no previous query data from both
startAutoRefresh and onRefresh', async () => {
+ mockStartAutoRefresh.mockClear();
+ mockUseAutoRefreshTabPause.mockClear();
+ const { props } = renderHeaderAutoRefresh(
+ {},
+ { chartIds: [1, 2, 3], timedRefreshImmuneSlices: [2] },
+ );
+
+ const { onRefresh: handleTabVisibilityRefresh } =
+ mockUseAutoRefreshTabPause.mock.calls[0][0];
+
+ await act(async () => {
+ await handleTabVisibilityRefresh();
+ });
+
+ expect(mockStartAutoRefresh).toHaveBeenCalledTimes(1);
+ expect(mockStartAutoRefresh).toHaveBeenCalledWith([1]);
+ expect(props.onRefresh).toHaveBeenCalledTimes(1);
+ const [refreshedChartIds] = props.onRefresh.mock.calls[0];
+ expect(refreshedChartIds).toEqual([1]);
+});
diff --git
a/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.ts
b/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.ts
index 16692be3479..59945281381 100644
--- a/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.ts
+++ b/superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.ts
@@ -158,7 +158,7 @@ export const useHeaderAutoRefresh = ({
}
if (suppressSpinners) {
- startAutoRefresh();
+ startAutoRefresh(chartsToRefresh);
setStatus(AutoRefreshStatus.Fetching);
setFetchStartTime(Date.now());
}
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx
index c21388b2b53..a8870bcc30c 100644
---
a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx
+++
b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.test.tsx
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { useEffect } from 'react';
import { act, fireEvent, render } from 'spec/helpers/testing-library';
import { FeatureFlag, VizType } from '@superset-ui/core';
import * as redux from 'redux';
@@ -27,6 +28,10 @@ import mockDatasource from 'spec/fixtures/mockDatasource';
import chartQueries, {
sliceId as queryId,
} from 'spec/fixtures/mockChartQueries';
+import {
+ AutoRefreshProvider,
+ useAutoRefreshContext,
+} from 'src/dashboard/contexts/AutoRefreshContext';
import Chart from './Chart';
let capturedChartContainerProps: Record<string, unknown> = {};
@@ -109,6 +114,32 @@ function setup(
});
}
+function StartAutoRefreshFor({ chartIds }: { chartIds: number[] }) {
+ const { startAutoRefresh } = useAutoRefreshContext();
+ useEffect(() => {
+ startAutoRefresh(chartIds);
+ }, [chartIds, startAutoRefresh]);
+ return null;
+}
+
+function setupDuringUnrelatedAutoRefresh(
+ refreshingChartIds: number[],
+ overrideProps: Record<string, unknown> = {},
+ overrideState: Record<string, unknown> = {},
+) {
+ return render(
+ <AutoRefreshProvider>
+ <StartAutoRefreshFor chartIds={refreshingChartIds} />
+ <Chart {...props} {...overrideProps} />
+ </AutoRefreshProvider>,
+ {
+ useRedux: true,
+ useRouter: true,
+ initialState: { ...defaultState, ...overrideState },
+ },
+ );
+}
+
const refreshChart = jest.fn();
const logEvent = jest.fn();
const changeFilter = jest.fn();
@@ -134,6 +165,34 @@ afterEach(() => {
jest.clearAllMocks();
});
+test('shows the loading spinner for a chart that starts loading outside the
in-flight auto-refresh batch', () => {
+ setupDuringUnrelatedAutoRefresh([queryId + 1], undefined, {
+ charts: {
+ ...defaultState.charts,
+ [queryId]: {
+ ...defaultState.charts[queryId],
+ chartStatus: 'loading',
+ },
+ },
+ });
+
+ expect(capturedChartContainerProps.suppressLoadingSpinner).toBe(false);
+});
+
+test('suppresses the loading spinner for a chart included in the in-flight
auto-refresh batch', () => {
+ setupDuringUnrelatedAutoRefresh([queryId], undefined, {
+ charts: {
+ ...defaultState.charts,
+ [queryId]: {
+ ...defaultState.charts[queryId],
+ chartStatus: 'loading',
+ },
+ },
+ });
+
+ expect(capturedChartContainerProps.suppressLoadingSpinner).toBe(true);
+});
+
test('should render a SliceHeader', () => {
const { getByTestId, container } = setup();
expect(getByTestId('slice-header')).toBeInTheDocument();
diff --git
a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx
b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx
index 2b0456a731b..9bffafc7026 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx
@@ -57,7 +57,7 @@ import {
convertChartStateToOwnState,
hasChartStateConverter,
} from '../../../util/chartStateConverter';
-import { useIsAutoRefreshing } from
'src/dashboard/contexts/AutoRefreshContext';
+import { useIsChartAutoRefreshing } from
'src/dashboard/contexts/AutoRefreshContext';
import SliceHeader from '../../SliceHeader';
import MissingChart from '../../MissingChart';
@@ -254,7 +254,7 @@ const Chart = (props: ChartProps) => {
(state.dashboardInfo?.metadata as JsonObject)?.show_chart_timestamps ??
false,
);
- const suppressLoadingSpinner = useIsAutoRefreshing();
+ const suppressLoadingSpinner = useIsChartAutoRefreshing(props.id);
const isCached: boolean[] = useMemo(
() =>
diff --git
a/superset-frontend/src/dashboard/contexts/AutoRefreshContext.test.tsx
b/superset-frontend/src/dashboard/contexts/AutoRefreshContext.test.tsx
index 6f6eae21fbb..9a0254383a0 100644
--- a/superset-frontend/src/dashboard/contexts/AutoRefreshContext.test.tsx
+++ b/superset-frontend/src/dashboard/contexts/AutoRefreshContext.test.tsx
@@ -22,6 +22,7 @@ import {
AutoRefreshProvider,
useAutoRefreshContext,
useIsAutoRefreshing,
+ useIsChartAutoRefreshing,
useIsRefreshInFlight,
} from './AutoRefreshContext';
@@ -135,3 +136,71 @@ test('useIsRefreshInFlight hook returns correct value
inside provider', () => {
expect(result.current.isRefreshInFlight).toBe(true);
});
+
+test('autoRefreshingChartIds starts empty', () => {
+ const { result } = renderHook(() => useAutoRefreshContext(), { wrapper });
+ expect(result.current.autoRefreshingChartIds).toEqual([]);
+});
+
+test('startAutoRefresh(chartIds) records only the affected chart ids', () => {
+ const { result } = renderHook(() => useAutoRefreshContext(), { wrapper });
+
+ act(() => {
+ result.current.startAutoRefresh([1, 2, 3]);
+ });
+
+ expect(result.current.isAutoRefreshing).toBe(true);
+ expect(result.current.autoRefreshingChartIds).toEqual([1, 2, 3]);
+});
+
+test('startAutoRefresh() with no chart ids records an empty batch', () => {
+ const { result } = renderHook(() => useAutoRefreshContext(), { wrapper });
+
+ act(() => {
+ result.current.startAutoRefresh();
+ });
+
+ expect(result.current.isAutoRefreshing).toBe(true);
+ expect(result.current.autoRefreshingChartIds).toEqual([]);
+});
+
+test('endAutoRefresh clears autoRefreshingChartIds', () => {
+ const { result } = renderHook(() => useAutoRefreshContext(), { wrapper });
+
+ act(() => {
+ result.current.startAutoRefresh([1, 2, 3]);
+ });
+ expect(result.current.autoRefreshingChartIds).toEqual([1, 2, 3]);
+
+ act(() => {
+ result.current.endAutoRefresh();
+ });
+ expect(result.current.autoRefreshingChartIds).toEqual([]);
+});
+
+test('useIsChartAutoRefreshing only reports true for charts in the in-flight
batch', () => {
+ const { result } = renderHook(
+ () => ({
+ context: useAutoRefreshContext(),
+ isChart1AutoRefreshing: useIsChartAutoRefreshing(1),
+ isChart2AutoRefreshing: useIsChartAutoRefreshing(2),
+ }),
+ { wrapper },
+ );
+
+ expect(result.current.isChart1AutoRefreshing).toBe(false);
+ expect(result.current.isChart2AutoRefreshing).toBe(false);
+
+ act(() => {
+ result.current.context.startAutoRefresh([1]);
+ });
+
+ expect(result.current.isChart1AutoRefreshing).toBe(true);
+ expect(result.current.isChart2AutoRefreshing).toBe(false);
+
+ act(() => {
+ result.current.context.endAutoRefresh();
+ });
+
+ expect(result.current.isChart1AutoRefreshing).toBe(false);
+});
diff --git a/superset-frontend/src/dashboard/contexts/AutoRefreshContext.tsx
b/superset-frontend/src/dashboard/contexts/AutoRefreshContext.tsx
index c6b0143e882..e03dd7dcb2c 100644
--- a/superset-frontend/src/dashboard/contexts/AutoRefreshContext.tsx
+++ b/superset-frontend/src/dashboard/contexts/AutoRefreshContext.tsx
@@ -29,15 +29,17 @@ import {
export interface AutoRefreshContextValue {
isAutoRefreshing: boolean;
isRefreshInFlight: boolean;
+ autoRefreshingChartIds: number[];
setIsAutoRefreshing: (value: boolean) => void;
setRefreshInFlight: (value: boolean) => void;
- startAutoRefresh: () => void;
+ startAutoRefresh: (chartIds?: number[]) => void;
endAutoRefresh: () => void;
}
const AutoRefreshContext = createContext<AutoRefreshContextValue>({
isAutoRefreshing: false,
isRefreshInFlight: false,
+ autoRefreshingChartIds: [],
setIsAutoRefreshing: () => {},
setRefreshInFlight: () => {},
startAutoRefresh: () => {},
@@ -57,25 +59,37 @@ export const AutoRefreshProvider:
FC<AutoRefreshProviderProps> = ({
}) => {
const [isAutoRefreshing, setIsAutoRefreshing] = useState(false);
const [isRefreshInFlight, setRefreshInFlight] = useState(false);
+ const [autoRefreshingChartIds, setAutoRefreshingChartIds] = useState<
+ number[]
+ >([]);
- const startAutoRefresh = useCallback(() => {
+ const startAutoRefresh = useCallback((chartIds: number[] = []) => {
+ setAutoRefreshingChartIds(chartIds);
setIsAutoRefreshing(true);
}, []);
const endAutoRefresh = useCallback(() => {
setIsAutoRefreshing(false);
+ setAutoRefreshingChartIds([]);
}, []);
const value = useMemo(
() => ({
isAutoRefreshing,
isRefreshInFlight,
+ autoRefreshingChartIds,
setIsAutoRefreshing,
setRefreshInFlight,
startAutoRefresh,
endAutoRefresh,
}),
- [isAutoRefreshing, isRefreshInFlight, startAutoRefresh, endAutoRefresh],
+ [
+ isAutoRefreshing,
+ isRefreshInFlight,
+ autoRefreshingChartIds,
+ startAutoRefresh,
+ endAutoRefresh,
+ ],
);
return (
@@ -98,4 +112,15 @@ export const useIsRefreshInFlight = (): boolean => {
return isRefreshInFlight;
};
+/**
+ * Whether the given chart is part of the currently in-flight silent
+ * auto-refresh batch. Unlike useIsAutoRefreshing, this is scoped to a single
+ * chart so charts loading for unrelated reasons (e.g. mounting into a
+ * freshly-selected tab) still show their own spinner.
+ */
+export const useIsChartAutoRefreshing = (chartId: number): boolean => {
+ const { autoRefreshingChartIds } = useContext(AutoRefreshContext);
+ return autoRefreshingChartIds.includes(chartId);
+};
+
export default AutoRefreshContext;
