Compare commits

...

3 Commits

Author SHA1 Message Date
Elizabeth Thompson
92cc1a79c8 fix(test): add useRouter to DashboardBuilder tests broken by useLocation() in Header
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-01 20:56:57 +00:00
Elizabeth Thompson
a9132a36d7 chore: drop SavedQueryList changes already covered by #39503
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-01 20:38:01 +00:00
Elizabeth Thompson
36a32e7b49 fix(frontend): prevent duplicate subdirectory prefix in dashboard fullscreen and saved queries navigation
React Router v5 prepends its basename automatically on history.push/replace
and <Link to>. Passing a path already prefixed by applicationRoot() doubled
the subdirectory on deployments using SUPERSET_APP_ROOT.

- Dashboard fullscreen toggle (sc-103933): switch from window.location.pathname
  to useLocation().pathname so history.replace receives a router-relative path
- Saved Queries "+ Query" / label links (sc-103661): remove makeUrl() from
  history.push and <Link to>; retain makeUrl() for window.open and clipboard
  where the full browser-absolute path is required

Adds unit tests (Header.test.tsx, SavedQueryList.test.tsx) and Playwright E2E
specs (fullscreen.spec.ts, saved-queries.spec.ts) covering both fixes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-22 01:21:35 +00:00
5 changed files with 238 additions and 2 deletions

View File

@@ -0,0 +1,117 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
/**
* Fullscreen toggle E2E tests.
*
* Regression tests for subdirectory deployments: when Superset runs at a path
* prefix (e.g. /pcs), the fullscreen toggle must not duplicate the prefix in
* the URL (sc-103933).
*/
import { testWithAssets as test, expect } from '../../helpers/fixtures';
import { DashboardPage } from '../../pages/DashboardPage';
import { apiPostDashboard } from '../../helpers/api/dashboard';
import { TIMEOUT } from '../../utils/constants';
test('toggling fullscreen adds standalone param without duplicating path segments', async ({
page,
testAssets,
}) => {
test.setTimeout(TIMEOUT.SLOW_TEST);
// Create a minimal dashboard to test against
const dashboardResponse = await apiPostDashboard(page, {
dashboard_title: `test_fullscreen_${Date.now()}`,
published: true,
});
expect(dashboardResponse.status()).toBe(201);
const { id: dashboardId } = await dashboardResponse.json();
testAssets.trackDashboard(dashboardId);
const dashboardPage = new DashboardPage(page);
await dashboardPage.gotoById(dashboardId);
await dashboardPage.waitForLoad();
// Record the pathname before toggling (should be something like /superset/dashboard/N/)
const pathBefore = new URL(page.url()).pathname;
// Open the three-dot menu and click "Enter fullscreen"
await dashboardPage.openHeaderActionsMenu();
await page.getByText('Enter fullscreen').click();
// Wait for the URL to update with standalone param
await page.waitForURL(/standalone=1/, { timeout: TIMEOUT.API_RESPONSE });
const urlAfter = new URL(page.url());
// The pathname must not have grown — i.e. no segment was duplicated
expect(urlAfter.pathname).toBe(pathBefore);
// standalone=1 must be present
expect(urlAfter.searchParams.get('standalone')).toBe('1');
// The dashboard header must still be visible (not a blank page)
await expect(
page.locator('[data-test="dashboard-header-container"]'),
).toBeVisible();
});
test('toggling fullscreen off removes standalone param without duplicating path segments', async ({
page,
testAssets,
}) => {
test.setTimeout(TIMEOUT.SLOW_TEST);
const dashboardResponse = await apiPostDashboard(page, {
dashboard_title: `test_fullscreen_exit_${Date.now()}`,
published: true,
});
expect(dashboardResponse.status()).toBe(201);
const { id: dashboardId } = await dashboardResponse.json();
testAssets.trackDashboard(dashboardId);
const dashboardPage = new DashboardPage(page);
// Navigate directly into fullscreen mode
await page.goto(`superset/dashboard/${dashboardId}/?standalone=1`);
await dashboardPage.waitForLoad();
const pathBefore = new URL(page.url()).pathname;
// Open the three-dot menu and click "Exit fullscreen"
await dashboardPage.openHeaderActionsMenu();
await page.getByText('Exit fullscreen').click();
// Wait for standalone param to disappear
await page.waitForFunction(
() => !new URL(window.location.href).searchParams.has('standalone'),
{ timeout: TIMEOUT.API_RESPONSE },
);
const urlAfter = new URL(page.url());
// Pathname must be unchanged — no duplicated segment
expect(urlAfter.pathname).toBe(pathBefore);
expect(urlAfter.searchParams.get('standalone')).toBeNull();
// Dashboard must still be rendered
await expect(
page.locator('[data-test="dashboard-header-container"]'),
).toBeVisible();
});

View File

@@ -0,0 +1,59 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
/**
* Saved Queries list E2E tests.
*
* Regression tests for subdirectory deployments: "+ Query" and query label
* links must navigate to SQL Lab without duplicating the path prefix (sc-103661).
*/
import { test, expect } from '@playwright/test';
import { URL as APP_URLS } from '../../utils/urls';
import { TIMEOUT } from '../../utils/constants';
test('+ Query button navigates to SQL Lab without duplicating path segments', async ({
page,
}) => {
test.setTimeout(TIMEOUT.SLOW_TEST);
await page.goto(APP_URLS.SAVED_QUERIES_LIST);
// Wait for the list to load
await page.waitForSelector('[data-test="saved_query-list-view"]', {
timeout: TIMEOUT.PAGE_LOAD,
});
// Click the "+ Query" button in the submenu (accessible name is "plus Query" due to icon)
await page.getByRole('button', { name: /Query/i }).click();
// Wait for SQL Lab to load
await page.waitForURL(/sqllab/, { timeout: TIMEOUT.PAGE_LOAD });
const url = new URL(page.url());
// No path segment should appear twice consecutively — catches the double-prefix bug
// where React Router prepends the basename a second time (e.g. /superset/superset/sqllab).
expect(url.pathname).not.toMatch(/\/(\w+)\/\1\//);
// SQL Lab editor must be visible (not a blank page)
await expect(page.locator('[data-test="sql-editor-tabs"]')).toBeVisible({
timeout: TIMEOUT.PAGE_LOAD,
});
});

View File

@@ -139,6 +139,7 @@ describe('DashboardBuilder', () => {
...overrideState,
}),
useDnd: true,
useRouter: true,
useTheme: true,
});
}
@@ -473,6 +474,7 @@ test('should render ParentSize wrapper with height 100% for tabs', async () => {
dashboardLayout: undoableDashboardLayoutWithTabs,
}),
useDnd: true,
useRouter: true,
useTheme: true,
});
@@ -506,6 +508,7 @@ test('should maintain layout when switching between tabs', async () => {
dashboardLayout: undoableDashboardLayoutWithTabs,
}),
useDnd: true,
useRouter: true,
useTheme: true,
});

View File

@@ -31,6 +31,20 @@ import { DASHBOARD_HEADER_ID } from '../../util/constants';
import { UPDATE_COMPONENTS } from '../../actions/dashboardLayout';
import { AutoRefreshStatus } from '../../types/autoRefresh';
const mockHistoryReplace = jest.fn();
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useHistory: () => ({
replace: mockHistoryReplace,
}),
useLocation: jest.fn(() => ({
pathname: '/dashboard',
search: '?standalone=1',
hash: '',
state: undefined,
})),
}));
const initialState = {
dashboardInfo: {
id: 1,
@@ -223,6 +237,13 @@ beforeAll(() => {
beforeEach(() => {
jest.clearAllMocks();
const { useLocation } = jest.requireMock('react-router-dom');
useLocation.mockReturnValue({
pathname: '/dashboard',
search: '?standalone=1',
hash: '',
state: undefined,
});
(useUnsavedChangesPrompt as jest.Mock).mockReturnValue({
showModal: false,
@@ -968,3 +989,30 @@ test('should sync theme ref when navigating between dashboards', async () => {
expect(setUnsavedChanges).toHaveBeenCalledTimes(0);
});
});
test('should not duplicate subdirectory prefix when toggling fullscreen', async () => {
const { useLocation } = jest.requireMock('react-router-dom');
// Simulate React Router with basename=/pcs: useLocation returns path relative to basename
useLocation.mockReturnValue({
pathname: '/dashboard',
search: '?standalone=1',
hash: '',
state: undefined,
});
// Simulate browser URL including the subdirectory prefix
window.history.pushState({}, 'Test page', '/pcs/dashboard?standalone=1');
setup();
await openActionsDropdown();
userEvent.click(screen.getByText('Exit fullscreen'));
// history.replace must be called with the Router-relative path, not window.location.pathname.
// If the subdirectory prefix (/pcs) were included, React Router would prepend it again,
// producing /pcs/pcs/dashboard (the bug). The path must start with /dashboard, not /pcs/.
expect(mockHistoryReplace).toHaveBeenCalledWith(
expect.not.stringMatching(/^\/pcs\//),
);
expect(mockHistoryReplace).toHaveBeenCalledWith(
expect.stringMatching(/^\/dashboard(\?|$)/),
);
});

View File

@@ -19,7 +19,7 @@
import type { Dispatch, ReactElement, SetStateAction } from 'react';
import { useState, useEffect, useCallback, useMemo } from 'react';
import { useSelector } from 'react-redux';
import { useHistory } from 'react-router-dom';
import { useHistory, useLocation } from 'react-router-dom';
import { Menu, MenuItem } from '@superset-ui/core/components/Menu';
import { t } from '@apache-superset/core/translation';
import { isEmpty } from 'lodash';
@@ -75,6 +75,7 @@ export const useHeaderActionsMenu = ({
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
const { canExportImage } = usePermissions();
const history = useHistory();
const location = useLocation();
const directPathToChild = useSelector(
(state: RootState) => state.dashboardState.directPathToChild,
);
@@ -101,8 +102,11 @@ export const useHeaderActionsMenu = ({
case MenuKeys.ToggleFullscreen: {
const isCurrentlyStandalone =
Number(getUrlParam(URL_PARAMS.standalone)) === 1;
// Use location.pathname from React Router (relative to basename) rather than
// window.location.pathname to avoid duplicating the subdirectory prefix when
// history.replace prepends it again.
const url = getDashboardUrl({
pathname: window.location.pathname,
pathname: location.pathname,
filters: getActiveFilters(),
hash: window.location.hash,
standalone: isCurrentlyStandalone ? null : 1,
@@ -125,6 +129,7 @@ export const useHeaderActionsMenu = ({
showRefreshModal,
manageEmbedded,
history,
location,
],
);
@@ -133,6 +138,10 @@ export const useHeaderActionsMenu = ({
[dashboardTitle],
);
// window.location.pathname is intentional here: this URL is used for sharing
// (email, embed, copy link) and must be a full browser-absolute path that
// includes the application root. Do NOT replace with useLocation().pathname —
// that would strip the subdirectory prefix and produce a broken share link.
const url = useMemo(
() =>
getDashboardUrl({