fix(dashboard): prevent duplicate screenshot downloads

superset-frontend/src/dashboard/hooks/useDownloadScreenshot.test.ts

@@ -51,13 +51,52 @@ jest.mock('src/utils/urlUtils', () => ({
   getDashboardUrlParams: jest.fn(() => []),
 }));
 
-test('downloadScreenshot calls API with force=true to ensure fresh screenshots', async () => {
-  const mockCacheKey = 'test-cache-key';
+const RETRY_INTERVAL = 3000;
+const DASHBOARD_ID = 123;
+const CACHE_KEY = 'test-cache-key';
+
+const mockPostSuccess = () =>
   (SupersetClient.post as jest.Mock).mockResolvedValue({
-    json: { cache_key: mockCacheKey },
+    json: { cache_key: CACHE_KEY },
   });
 
-  const { result } = renderHook(() => useDownloadScreenshot(123));
+const createResponse = (): Response =>
+  ({
+    headers: { get: () => null },
+    blob: () => Promise.resolve(new Blob(['image-data'])),
+  }) as unknown as Response;
+
+const notReadyError = () => ({ status: 404 });
+
+// Chain several Promise.resolves to drain nested microtasks (.then/.catch/.finally
+// in the hook). setImmediate-based flush would stall under fake timers.
+const flushPromises = async () => {
+  for (let i = 0; i < 10; i += 1) {
+    // eslint-disable-next-line no-await-in-loop
+    await Promise.resolve();
+  }
+};
+
+const triggerDownload = async () => {
+  const { result } = renderHook(() => useDownloadScreenshot(DASHBOARD_ID));
+  await act(async () => {
+    result.current(DownloadScreenshotFormat.PNG);
+    await flushPromises();
+  });
+  return result;
+};
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  // Default: GET hangs so microtask chains don't throw on undefined in tests
+  // that only care about POST behavior.
+  (SupersetClient.get as jest.Mock).mockReturnValue(new Promise(() => {}));
+});
+
+test('downloadScreenshot calls API with force=true to ensure fresh screenshots', async () => {
+  mockPostSuccess();
+
+  const { result } = renderHook(() => useDownloadScreenshot(DASHBOARD_ID));
 
   await act(async () => {
     result.current(DownloadScreenshotFormat.PNG);
@@ -71,3 +110,64 @@ test('downloadScreenshot calls API with force=true to ensure fresh screenshots',
   expect(callArgs.endpoint).toContain('force');
   expect(callArgs.endpoint).toMatch(/force[:%]true|force[:%]!t/);
 });
+
+test('does not issue overlapping GETs while a previous GET is in-flight', async () => {
+  jest.useFakeTimers();
+  mockPostSuccess();
+
+  // GET never resolves within the test — simulates a slow screenshot request.
+  (SupersetClient.get as jest.Mock).mockImplementation(
+    () => new Promise(() => {}),
+  );
+
+  await triggerDownload();
+
+  // First (immediate) GET fires right after POST resolves.
+  expect(SupersetClient.get).toHaveBeenCalledTimes(1);
+
+  // Advance past several retry intervals while the first GET is still pending.
+  await act(async () => {
+    jest.advanceTimersByTime(RETRY_INTERVAL * 5);
+    await flushPromises();
+  });
+
+  // isFetching guard must prevent the interval from stacking new requests.
+  expect(SupersetClient.get).toHaveBeenCalledTimes(1);
+
+  jest.clearAllTimers();
+  jest.useRealTimers();
+});
+
+test('triggers only one download when multiple successful responses race', async () => {
+  jest.useFakeTimers();
+  mockPostSuccess();
+
+  // First GET returns 404 (not ready), then resolves 200 for every subsequent call.
+  // Without the isDownloaded guard any late-arriving 200 would trigger a second click.
+  (SupersetClient.get as jest.Mock)
+    .mockRejectedValueOnce(notReadyError())
+    .mockResolvedValue(createResponse());
+
+  // jsdom does not implement URL.createObjectURL / revokeObjectURL — stub them.
+  Object.assign(window.URL, {
+    createObjectURL: jest.fn(() => 'blob:mock'),
+    revokeObjectURL: jest.fn(),
+  });
+  const clickSpy = jest
+    .spyOn(HTMLAnchorElement.prototype, 'click')
+    .mockImplementation(() => {});
+
+  await triggerDownload();
+
+  // Drive several interval ticks so multiple 200 responses could resolve.
+  await act(async () => {
+    jest.advanceTimersByTime(RETRY_INTERVAL * 5);
+    await flushPromises();
+  });
+
+  expect(clickSpy).toHaveBeenCalledTimes(1);
+
+  clickSpy.mockRestore();
+  jest.clearAllTimers();
+  jest.useRealTimers();
+});

superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts

@@ -74,6 +74,8 @@ export const useDownloadScreenshot = (
   const downloadScreenshot = useCallback(
     (format: DownloadScreenshotFormat) => {
       let retries = 0;
+      let isFetching = false;
+      let isDownloaded = false;
 
       const toastIntervalId = setInterval(
         () =>
@@ -118,6 +120,11 @@ export const useDownloadScreenshot = (
             return response.blob().then(blob => ({ blob, fileName }));
           })
           .then(({ blob, fileName }) => {
+            if (isDownloaded) {
+              return;
+            }
+            isDownloaded = true;
+            stopIntervals('success');
             const url = window.URL.createObjectURL(blob);
             const a = document.createElement('a');
             a.href = url;
@@ -126,7 +133,6 @@ export const useDownloadScreenshot = (
             a.click();
             document.body.removeChild(a);
             window.URL.revokeObjectURL(url);
-            stopIntervals('success');
           })
           .catch(err => {
             if ((err as SupersetApiError).status === 404) {
@@ -135,14 +141,22 @@ export const useDownloadScreenshot = (
           });
 
       const fetchImageWithRetry = (cacheKey: string) => {
+        if (isDownloaded || isFetching) {
+          return;
+        }
         if (retries >= MAX_RETRIES) {
           stopIntervals('failure');
           logging.error('Max retries reached');
           return;
         }
-        checkImageReady(cacheKey).catch(() => {
-          retries += 1;
-        });
+        isFetching = true;
+        checkImageReady(cacheKey)
+          .catch(() => {
+            retries += 1;
+          })
+          .finally(() => {
+            isFetching = false;
+          });
       };
 
       SupersetClient.post({