This is an automated email from the ASF dual-hosted git repository. jli pushed a commit to branch fix-app-prefix-export in repository https://gitbox.apache.org/repos/asf/superset.git
commit bbb4aaf9217d97bf5de4e573de29a34c5d60d18e Author: Joe Li <[email protected]> AuthorDate: Fri Dec 19 10:45:41 2025 -0800 fix(streaming-export): improve URL prefix guard and flatten tests Fixes: - ensureUrlPrefix now correctly handles sibling paths (/app2 when appRoot is /app) by checking url === appRoot || url.startsWith(`${appRoot}/`) - Add early return for protocol-relative URLs (//example.com) to prevent incorrect prefixing Tests: - Add tests for sibling path and protocol-relative URL edge cases - Flatten nested describe() blocks to top-level test() per project guidelines - Add missing jest.mock('./pathUtils') declaration in export.test.ts 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --- .../useStreamingExport.test.ts | 287 ++++++++++++--------- .../StreamingExportModal/useStreamingExport.ts | 7 +- .../src/explore/exploreUtils/exportChart.test.ts | 251 +++++++++--------- superset-frontend/src/utils/export.test.ts | 93 ++++--- 4 files changed, 345 insertions(+), 293 deletions(-) diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts index 5f2bbb41f5..3d4b631607 100644 --- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts +++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.test.ts @@ -218,172 +218,213 @@ test('sets ERROR status and calls onError when fetch rejects', async () => { }); // URL prefix guard tests - prevent regression of missing app root prefix -describe('URL prefix guard', () => { - const { applicationRoot } = jest.requireMock('src/utils/getBootstrapData'); - const { makeUrl } = jest.requireMock('src/utils/pathUtils'); - - const createMockFetch = () => - jest.fn().mockResolvedValue({ - ok: true, - headers: new Headers({ - 'Content-Disposition': 'attachment; filename="export.csv"', +const { applicationRoot } = jest.requireMock('src/utils/getBootstrapData'); +const { makeUrl } = jest.requireMock('src/utils/pathUtils'); + +const createPrefixTestMockFetch = () => + jest.fn().mockResolvedValue({ + ok: true, + headers: new Headers({ + 'Content-Disposition': 'attachment; filename="export.csv"', + }), + body: { + getReader: () => ({ + read: jest.fn().mockResolvedValue({ done: true, value: undefined }), }), - body: { - getReader: () => ({ - read: jest.fn().mockResolvedValue({ done: true, value: undefined }), - }), - }, + }, + }); + +test('URL prefix guard applies prefix to unprefixed relative URL when app root is configured', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; + + const { result } = renderHook(() => useStreamingExport()); + + act(() => { + result.current.startExport({ + url: '/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); - beforeEach(() => { - applicationRoot.mockReturnValue(''); - makeUrl.mockImplementation((path: string) => path); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); }); - test('applies prefix to unprefixed relative URL when app root is configured', async () => { - const appRoot = '/superset'; - applicationRoot.mockReturnValue(appRoot); - makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + expect(mockFetch).toHaveBeenCalledWith( + '/superset/api/v1/sqllab/export_streaming/', + expect.any(Object), + ); +}); - const mockFetch = createMockFetch(); - global.fetch = mockFetch; +test('URL prefix guard does not double-prefix URL that already has app root', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); - const { result } = renderHook(() => useStreamingExport()); + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; - act(() => { - result.current.startExport({ - url: '/api/v1/sqllab/export_streaming/', - payload: { client_id: 'test-id' }, - exportType: 'csv', - }); - }); + const { result } = renderHook(() => useStreamingExport()); - await waitFor(() => { - expect(mockFetch).toHaveBeenCalledTimes(1); + act(() => { + result.current.startExport({ + url: '/superset/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); - // Guard should have applied the prefix - expect(mockFetch).toHaveBeenCalledWith( - '/superset/api/v1/sqllab/export_streaming/', - expect.any(Object), - ); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); }); - test('does not double-prefix URL that already has app root', async () => { - const appRoot = '/superset'; - applicationRoot.mockReturnValue(appRoot); - makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + expect(mockFetch).toHaveBeenCalledWith( + '/superset/api/v1/sqllab/export_streaming/', + expect.any(Object), + ); +}); - const mockFetch = createMockFetch(); - global.fetch = mockFetch; +test('URL prefix guard leaves URL unchanged when no app root is configured', async () => { + applicationRoot.mockReturnValue(''); - const { result } = renderHook(() => useStreamingExport()); + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; - // URL already has prefix (caller did the right thing) - act(() => { - result.current.startExport({ - url: '/superset/api/v1/sqllab/export_streaming/', - payload: { client_id: 'test-id' }, - exportType: 'csv', - }); - }); + const { result } = renderHook(() => useStreamingExport()); - await waitFor(() => { - expect(mockFetch).toHaveBeenCalledTimes(1); + act(() => { + result.current.startExport({ + url: '/api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); - // Should NOT double-prefix - expect(mockFetch).toHaveBeenCalledWith( - '/superset/api/v1/sqllab/export_streaming/', - expect.any(Object), - ); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); }); - test('leaves URL unchanged when no app root is configured', async () => { - applicationRoot.mockReturnValue(''); + expect(mockFetch).toHaveBeenCalledWith( + '/api/v1/sqllab/export_streaming/', + expect.any(Object), + ); +}); - const mockFetch = createMockFetch(); - global.fetch = mockFetch; +test('URL prefix guard leaves relative URL without leading slash unchanged', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); - const { result } = renderHook(() => useStreamingExport()); + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; - act(() => { - result.current.startExport({ - url: '/api/v1/sqllab/export_streaming/', - payload: { client_id: 'test-id' }, - exportType: 'csv', - }); - }); + const { result } = renderHook(() => useStreamingExport()); - await waitFor(() => { - expect(mockFetch).toHaveBeenCalledTimes(1); + act(() => { + result.current.startExport({ + url: 'api/v1/sqllab/export_streaming/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); - // No prefix should be applied - expect(mockFetch).toHaveBeenCalledWith( - '/api/v1/sqllab/export_streaming/', - expect.any(Object), - ); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); }); - test('leaves relative URL without leading slash unchanged (caller responsibility)', async () => { - // URLs without leading slash are treated as non-relative and passed through. - // Callers should always use absolute paths starting with / - const appRoot = '/superset'; - applicationRoot.mockReturnValue(appRoot); - makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + expect(mockFetch).toHaveBeenCalledWith( + 'api/v1/sqllab/export_streaming/', + expect.any(Object), + ); +}); - const mockFetch = createMockFetch(); - global.fetch = mockFetch; +test('URL prefix guard leaves absolute URLs (https) unchanged', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); - const { result } = renderHook(() => useStreamingExport()); + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; - act(() => { - result.current.startExport({ - url: 'api/v1/sqllab/export_streaming/', - payload: { client_id: 'test-id' }, - exportType: 'csv', - }); - }); + const { result } = renderHook(() => useStreamingExport()); - await waitFor(() => { - expect(mockFetch).toHaveBeenCalledTimes(1); + act(() => { + result.current.startExport({ + url: 'https://external.example.com/api/export/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); - // URL without leading slash is passed through unchanged - guard only handles /paths - expect(mockFetch).toHaveBeenCalledWith( - 'api/v1/sqllab/export_streaming/', - expect.any(Object), - ); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); }); - test('leaves absolute URLs (http/https) unchanged', async () => { - // Absolute URLs should pass through without modification - const appRoot = '/superset'; - applicationRoot.mockReturnValue(appRoot); - makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); + expect(mockFetch).toHaveBeenCalledWith( + 'https://external.example.com/api/export/', + expect.any(Object), + ); +}); + +test('URL prefix guard leaves protocol-relative URLs (//host) unchanged', async () => { + const appRoot = '/superset'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); - const mockFetch = createMockFetch(); - global.fetch = mockFetch; + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; - const { result } = renderHook(() => useStreamingExport()); + const { result } = renderHook(() => useStreamingExport()); - act(() => { - result.current.startExport({ - url: 'https://external.example.com/api/export/', - payload: { client_id: 'test-id' }, - exportType: 'csv', - }); + act(() => { + result.current.startExport({ + url: '//external.example.com/api/export/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); + + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + expect(mockFetch).toHaveBeenCalledWith( + '//external.example.com/api/export/', + expect.any(Object), + ); +}); + +test('URL prefix guard correctly handles sibling paths (prefixes /app2 when appRoot is /app)', async () => { + const appRoot = '/app'; + applicationRoot.mockReturnValue(appRoot); + makeUrl.mockImplementation((path: string) => `${appRoot}${path}`); - await waitFor(() => { - expect(mockFetch).toHaveBeenCalledTimes(1); + const mockFetch = createPrefixTestMockFetch(); + global.fetch = mockFetch; + + const { result } = renderHook(() => useStreamingExport()); + + act(() => { + result.current.startExport({ + url: '/app2/api/v1/export/', + payload: { client_id: 'test-id' }, + exportType: 'csv', }); + }); - // Absolute URLs should not be modified - expect(mockFetch).toHaveBeenCalledWith( - 'https://external.example.com/api/export/', - expect.any(Object), - ); + await waitFor(() => { + expect(mockFetch).toHaveBeenCalledTimes(1); }); + + // /app2 should be prefixed because it's not under /app/ - it's a sibling path + expect(mockFetch).toHaveBeenCalledWith( + '/app/app2/api/v1/export/', + expect.any(Object), + ); }); diff --git a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts index 945827d371..9d2f3b798c 100644 --- a/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts +++ b/superset-frontend/src/components/StreamingExportModal/useStreamingExport.ts @@ -48,6 +48,10 @@ const NEWLINE_BYTE = 10; // '\n' character code */ const ensureUrlPrefix = (url: string): string => { const appRoot = applicationRoot(); + // Protocol-relative URLs (//example.com/...) should pass through unchanged + if (url.startsWith('//')) { + return url; + } // Only process relative URLs (starting with /) if (!url.startsWith('/')) { return url; @@ -57,7 +61,8 @@ const ensureUrlPrefix = (url: string): string => { return url; } // If URL already has the app root prefix, return as-is - if (url.startsWith(appRoot)) { + // Use strict check to avoid false positives with sibling paths (e.g., /app2 when appRoot is /app) + if (url === appRoot || url.startsWith(`${appRoot}/`)) { return url; } // Apply prefix via makeUrl diff --git a/superset-frontend/src/explore/exploreUtils/exportChart.test.ts b/superset-frontend/src/explore/exploreUtils/exportChart.test.ts index ae6de113a1..661e1248b3 100644 --- a/superset-frontend/src/explore/exploreUtils/exportChart.test.ts +++ b/superset-frontend/src/explore/exploreUtils/exportChart.test.ts @@ -40,143 +40,134 @@ jest.mock('@superset-ui/core', () => ({ })); const { ensureAppRoot } = jest.requireMock('src/utils/pathUtils'); +const { getChartMetadataRegistry } = jest.requireMock('@superset-ui/core'); + +// Minimal formData that won't trigger legacy API (useLegacyApi = false) +const baseFormData = { + datasource: '1__table', + viz_type: 'table', +}; + +beforeEach(() => { + jest.clearAllMocks(); + // Default: no prefix + ensureAppRoot.mockImplementation((path: string) => path); + // Default: v1 API (not legacy) + getChartMetadataRegistry.mockReturnValue({ + get: jest.fn().mockReturnValue({ parseMethod: 'json' }), + }); +}); + +// Tests for exportChart URL prefix handling in streaming export +test('exportChart v1 API passes prefixed URL to onStartStreamingExport when app root is configured', async () => { + const appRoot = '/superset'; + ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`); + + const onStartStreamingExport = jest.fn(); -describe('exportChart URL prefix for streaming export', () => { - beforeEach(() => { - jest.clearAllMocks(); - // Default: no prefix - ensureAppRoot.mockImplementation((path: string) => path); + await exportChart({ + formData: baseFormData, + resultFormat: 'csv', + onStartStreamingExport: onStartStreamingExport as unknown as null, }); - // Minimal formData that won't trigger legacy API (useLegacyApi = false) - const baseFormData = { - datasource: '1__table', - viz_type: 'table', - }; + expect(onStartStreamingExport).toHaveBeenCalledTimes(1); + const callArgs = onStartStreamingExport.mock.calls[0][0]; + expect(callArgs.url).toBe('/superset/api/v1/chart/data'); + expect(callArgs.exportType).toBe('csv'); +}); + +test('exportChart v1 API passes unprefixed URL when no app root is configured', async () => { + ensureAppRoot.mockImplementation((path: string) => path); - describe('v1 API endpoint', () => { - test('passes prefixed URL to onStartStreamingExport when app root is configured', async () => { - const appRoot = '/superset'; - ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`); - - const onStartStreamingExport = jest.fn(); - - await exportChart({ - formData: baseFormData, - resultFormat: 'csv', - onStartStreamingExport: onStartStreamingExport as unknown as null, - }); - - expect(onStartStreamingExport).toHaveBeenCalledTimes(1); - const callArgs = onStartStreamingExport.mock.calls[0][0]; - expect(callArgs.url).toBe('/superset/api/v1/chart/data'); - expect(callArgs.exportType).toBe('csv'); - }); - - test('passes unprefixed URL when no app root is configured', async () => { - ensureAppRoot.mockImplementation((path: string) => path); - - const onStartStreamingExport = jest.fn(); - - await exportChart({ - formData: baseFormData, - resultFormat: 'csv', - onStartStreamingExport: onStartStreamingExport as unknown as null, - }); - - expect(onStartStreamingExport).toHaveBeenCalledTimes(1); - const callArgs = onStartStreamingExport.mock.calls[0][0]; - expect(callArgs.url).toBe('/api/v1/chart/data'); - }); - - test('passes nested prefix for deeply nested deployments', async () => { - const appRoot = '/my-company/analytics/superset'; - ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`); - - const onStartStreamingExport = jest.fn(); - - await exportChart({ - formData: baseFormData, - resultFormat: 'xlsx', - onStartStreamingExport: onStartStreamingExport as unknown as null, - }); - - expect(onStartStreamingExport).toHaveBeenCalledTimes(1); - const callArgs = onStartStreamingExport.mock.calls[0][0]; - expect(callArgs.url).toBe( - '/my-company/analytics/superset/api/v1/chart/data', - ); - expect(callArgs.exportType).toBe('xlsx'); - }); + const onStartStreamingExport = jest.fn(); + + await exportChart({ + formData: baseFormData, + resultFormat: 'csv', + onStartStreamingExport: onStartStreamingExport as unknown as null, }); - describe('exportType passthrough', () => { - test('passes csv exportType for CSV exports', async () => { - const onStartStreamingExport = jest.fn(); - - await exportChart({ - formData: baseFormData, - resultFormat: 'csv', - onStartStreamingExport: onStartStreamingExport as unknown as null, - }); - - expect(onStartStreamingExport).toHaveBeenCalledWith( - expect.objectContaining({ - exportType: 'csv', - }), - ); - }); - - test('passes xlsx exportType for Excel exports', async () => { - const onStartStreamingExport = jest.fn(); - - await exportChart({ - formData: baseFormData, - resultFormat: 'xlsx', - onStartStreamingExport: onStartStreamingExport as unknown as null, - }); - - expect(onStartStreamingExport).toHaveBeenCalledWith( - expect.objectContaining({ - exportType: 'xlsx', - }), - ); - }); + expect(onStartStreamingExport).toHaveBeenCalledTimes(1); + const callArgs = onStartStreamingExport.mock.calls[0][0]; + expect(callArgs.url).toBe('/api/v1/chart/data'); +}); + +test('exportChart v1 API passes nested prefix for deeply nested deployments', async () => { + const appRoot = '/my-company/analytics/superset'; + ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`); + + const onStartStreamingExport = jest.fn(); + + await exportChart({ + formData: baseFormData, + resultFormat: 'xlsx', + onStartStreamingExport: onStartStreamingExport as unknown as null, }); - describe('legacy API endpoint (useLegacyApi=true)', () => { - // Legacy API uses getExploreUrl() -> getURIDirectory() -> ensureAppRoot() - // This test ensures the legacy path also correctly prefixes URLs - const { getChartMetadataRegistry } = jest.requireMock('@superset-ui/core'); - - test('passes prefixed URL for legacy viz type with app root configured', async () => { - const appRoot = '/superset'; - ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`); - - // Configure mock to return useLegacyApi: true - getChartMetadataRegistry.mockReturnValue({ - get: jest - .fn() - .mockReturnValue({ useLegacyApi: true, parseMethod: 'json' }), - }); - - const onStartStreamingExport = jest.fn(); - const legacyFormData = { - datasource: '1__table', - viz_type: 'legacy_viz', - }; - - await exportChart({ - formData: legacyFormData, - resultFormat: 'csv', - onStartStreamingExport: onStartStreamingExport as unknown as null, - }); - - expect(onStartStreamingExport).toHaveBeenCalledTimes(1); - const callArgs = onStartStreamingExport.mock.calls[0][0]; - // Legacy path uses getURIDirectory which calls ensureAppRoot - expect(callArgs.url).toContain(appRoot); - expect(callArgs.exportType).toBe('csv'); - }); + expect(onStartStreamingExport).toHaveBeenCalledTimes(1); + const callArgs = onStartStreamingExport.mock.calls[0][0]; + expect(callArgs.url).toBe('/my-company/analytics/superset/api/v1/chart/data'); + expect(callArgs.exportType).toBe('xlsx'); +}); + +test('exportChart passes csv exportType for CSV exports', async () => { + const onStartStreamingExport = jest.fn(); + + await exportChart({ + formData: baseFormData, + resultFormat: 'csv', + onStartStreamingExport: onStartStreamingExport as unknown as null, }); + + expect(onStartStreamingExport).toHaveBeenCalledWith( + expect.objectContaining({ + exportType: 'csv', + }), + ); +}); + +test('exportChart passes xlsx exportType for Excel exports', async () => { + const onStartStreamingExport = jest.fn(); + + await exportChart({ + formData: baseFormData, + resultFormat: 'xlsx', + onStartStreamingExport: onStartStreamingExport as unknown as null, + }); + + expect(onStartStreamingExport).toHaveBeenCalledWith( + expect.objectContaining({ + exportType: 'xlsx', + }), + ); +}); + +test('exportChart legacy API (useLegacyApi=true) passes prefixed URL with app root configured', async () => { + // Legacy API uses getExploreUrl() -> getURIDirectory() -> ensureAppRoot() + const appRoot = '/superset'; + ensureAppRoot.mockImplementation((path: string) => `${appRoot}${path}`); + + // Configure mock to return useLegacyApi: true + getChartMetadataRegistry.mockReturnValue({ + get: jest.fn().mockReturnValue({ useLegacyApi: true, parseMethod: 'json' }), + }); + + const onStartStreamingExport = jest.fn(); + const legacyFormData = { + datasource: '1__table', + viz_type: 'legacy_viz', + }; + + await exportChart({ + formData: legacyFormData, + resultFormat: 'csv', + onStartStreamingExport: onStartStreamingExport as unknown as null, + }); + + expect(onStartStreamingExport).toHaveBeenCalledTimes(1); + const callArgs = onStartStreamingExport.mock.calls[0][0]; + // Legacy path uses getURIDirectory which calls ensureAppRoot + expect(callArgs.url).toContain(appRoot); + expect(callArgs.exportType).toBe('csv'); }); diff --git a/superset-frontend/src/utils/export.test.ts b/superset-frontend/src/utils/export.test.ts index 94ccf9a9ab..4c07c4cbf9 100644 --- a/superset-frontend/src/utils/export.test.ts +++ b/superset-frontend/src/utils/export.test.ts @@ -33,6 +33,11 @@ jest.mock('@superset-ui/core', () => ({ jest.mock('content-disposition'); +// Mock pathUtils for double-prefix prevention tests +jest.mock('./pathUtils', () => ({ + ensureAppRoot: jest.fn((path: string) => path), +})); + let mockBlob: Blob; let mockResponse: Response; let createElementSpy: jest.SpyInstance; @@ -393,42 +398,52 @@ test('handles export with empty IDs array', async () => { ); }); -// Test to prevent double-prefix bug: SupersetClient.getUrl() already adds -// the appRoot prefix, so handleResourceExport should NOT pre-apply it. -// If we use ensureAppRoot() in export.ts, SupersetClient.getUrl() adds the -// prefix again, resulting in double-prefixed URLs like /app/app/api/v1/... -describe('double-prefix prevention', () => { - // Import the mocked ensureAppRoot to control its behavior per test - const { ensureAppRoot } = jest.requireMock('./pathUtils'); - - const prefixTestCases = [ - { name: 'subdirectory prefix', appRoot: '/superset', resource: 'dashboard', ids: [1] }, - { name: 'subdirectory prefix (dataset)', appRoot: '/superset', resource: 'dataset', ids: [1] }, - { name: 'nested prefix', appRoot: '/my-app/superset', resource: 'dataset', ids: [1, 2] }, - ]; - - test.each(prefixTestCases)( - 'endpoint should not include app prefix: $name', - async ({ appRoot, resource, ids }) => { - // Simulate real ensureAppRoot behavior: prepend the appRoot - (ensureAppRoot as jest.Mock).mockImplementation( - (path: string) => `${appRoot}${path}`, - ); - - const doneMock = jest.fn(); - await handleResourceExport(resource, ids, doneMock); - - // The endpoint passed to SupersetClient.get should NOT have the appRoot prefix - // because SupersetClient.getUrl() adds it when building the full URL. - const expectedEndpoint = `/api/v1/${resource}/export/?q=!(${ids.join(',')})`; - - // Explicitly verify no prefix in endpoint - this will fail if ensureAppRoot is used - const callArgs = (SupersetClient.get as jest.Mock).mock.calls.slice(-1)[0][0]; - expect(callArgs.endpoint).not.toContain(appRoot); - expect(callArgs.endpoint).toBe(expectedEndpoint); - - // Reset mock for next test - (ensureAppRoot as jest.Mock).mockImplementation((path: string) => path); - }, - ); -}); +const { ensureAppRoot } = jest.requireMock('./pathUtils'); + +const doublePrefixTestCases = [ + { + name: 'subdirectory prefix', + appRoot: '/superset', + resource: 'dashboard', + ids: [1], + }, + { + name: 'subdirectory prefix (dataset)', + appRoot: '/superset', + resource: 'dataset', + ids: [1], + }, + { + name: 'nested prefix', + appRoot: '/my-app/superset', + resource: 'dataset', + ids: [1, 2], + }, +]; + +test.each(doublePrefixTestCases)( + 'handleResourceExport endpoint should not include app prefix: $name', + async ({ appRoot, resource, ids }) => { + // Simulate real ensureAppRoot behavior: prepend the appRoot + (ensureAppRoot as jest.Mock).mockImplementation( + (path: string) => `${appRoot}${path}`, + ); + + const doneMock = jest.fn(); + await handleResourceExport(resource, ids, doneMock); + + // The endpoint passed to SupersetClient.get should NOT have the appRoot prefix + // because SupersetClient.getUrl() adds it when building the full URL. + const expectedEndpoint = `/api/v1/${resource}/export/?q=!(${ids.join(',')})`; + + // Explicitly verify no prefix in endpoint - this will fail if ensureAppRoot is used + const callArgs = (SupersetClient.get as jest.Mock).mock.calls.slice( + -1, + )[0][0]; + expect(callArgs.endpoint).not.toContain(appRoot); + expect(callArgs.endpoint).toBe(expectedEndpoint); + + // Reset mock for next test + (ensureAppRoot as jest.Mock).mockImplementation((path: string) => path); + }, +);
