mirror of
https://github.com/apache/superset.git
synced 2026-04-20 16:44:46 +00:00
fix(dashboard): dashboard filters not inherited in charts in Safari sometimes due to race condition (#38851)
Co-authored-by: madhushree agarwal <madhushree_agarwal@apple.com>
This commit is contained in:
@@ -85,11 +85,11 @@ const getDashboardPageContext = (pageId?: string | null) => {
|
|||||||
return getItem(LocalStorageKeys.DashboardExploreContext, {})[pageId] || null;
|
return getItem(LocalStorageKeys.DashboardExploreContext, {})[pageId] || null;
|
||||||
};
|
};
|
||||||
|
|
||||||
const getDashboardContextFormData = () => {
|
const getDashboardContextFormData = (search: string) => {
|
||||||
const dashboardPageId = getUrlParam(URL_PARAMS.dashboardPageId);
|
const dashboardPageId = getUrlParam(URL_PARAMS.dashboardPageId, search);
|
||||||
const dashboardContext = getDashboardPageContext(dashboardPageId);
|
const dashboardContext = getDashboardPageContext(dashboardPageId);
|
||||||
if (dashboardContext) {
|
if (dashboardContext) {
|
||||||
const sliceId = getUrlParam(URL_PARAMS.sliceId) || 0;
|
const sliceId = getUrlParam(URL_PARAMS.sliceId, search) || 0;
|
||||||
const {
|
const {
|
||||||
colorScheme,
|
colorScheme,
|
||||||
labelsColor,
|
labelsColor,
|
||||||
@@ -141,7 +141,7 @@ export default function ExplorePage() {
|
|||||||
fetchGeneration.current += 1;
|
fetchGeneration.current += 1;
|
||||||
const generation = fetchGeneration.current;
|
const generation = fetchGeneration.current;
|
||||||
const exploreUrlParams = getParsedExploreURLParams(loc);
|
const exploreUrlParams = getParsedExploreURLParams(loc);
|
||||||
const dashboardContextFormData = getDashboardContextFormData();
|
const dashboardContextFormData = getDashboardContextFormData(loc.search);
|
||||||
|
|
||||||
const isStale = () => generation !== fetchGeneration.current;
|
const isStale = () => generation !== fetchGeneration.current;
|
||||||
|
|
||||||
|
|||||||
@@ -22,7 +22,9 @@ import {
|
|||||||
parseUrl,
|
parseUrl,
|
||||||
toQueryString,
|
toQueryString,
|
||||||
getDashboardUrlParams,
|
getDashboardUrlParams,
|
||||||
|
getUrlParam,
|
||||||
} from './urlUtils';
|
} from './urlUtils';
|
||||||
|
import { URL_PARAMS } from '../constants';
|
||||||
|
|
||||||
test('isUrlExternal', () => {
|
test('isUrlExternal', () => {
|
||||||
expect(isUrlExternal('http://google.com')).toBeTruthy();
|
expect(isUrlExternal('http://google.com')).toBeTruthy();
|
||||||
@@ -145,3 +147,45 @@ test('getDashboardUrlParams should exclude multiple parameters when provided', (
|
|||||||
// Restore original location
|
// Restore original location
|
||||||
window.location = originalLocation;
|
window.location = originalLocation;
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('getUrlParam reads from window.location.search by default', () => {
|
||||||
|
const originalLocation = window.location;
|
||||||
|
Object.defineProperty(window, 'location', {
|
||||||
|
value: { ...originalLocation, search: '?dashboard_page_id=from-window' },
|
||||||
|
writable: true,
|
||||||
|
configurable: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(getUrlParam(URL_PARAMS.dashboardPageId)).toBe('from-window');
|
||||||
|
|
||||||
|
Object.defineProperty(window, 'location', {
|
||||||
|
value: originalLocation,
|
||||||
|
writable: true,
|
||||||
|
configurable: true,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('getUrlParam uses provided search string instead of window.location.search (Safari race condition fix)', () => {
|
||||||
|
// Simulate Safari race condition: window.location.search is stale (empty),
|
||||||
|
// but the correct search string is passed in from React Router's useLocation()
|
||||||
|
const originalLocation = window.location;
|
||||||
|
Object.defineProperty(window, 'location', {
|
||||||
|
value: { ...originalLocation, search: '' },
|
||||||
|
writable: true,
|
||||||
|
configurable: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
// Without the search override, window.location.search is stale — returns null (the bug)
|
||||||
|
expect(getUrlParam(URL_PARAMS.dashboardPageId)).toBeNull();
|
||||||
|
|
||||||
|
// With the search override (the fix), returns the correct value
|
||||||
|
expect(
|
||||||
|
getUrlParam(URL_PARAMS.dashboardPageId, '?dashboard_page_id=correct-id'),
|
||||||
|
).toBe('correct-id');
|
||||||
|
|
||||||
|
Object.defineProperty(window, 'location', {
|
||||||
|
value: originalLocation,
|
||||||
|
writable: true,
|
||||||
|
configurable: true,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -38,22 +38,35 @@ export type UrlParamType = 'string' | 'number' | 'boolean' | 'object' | 'rison';
|
|||||||
export type UrlParam = (typeof URL_PARAMS)[keyof typeof URL_PARAMS];
|
export type UrlParam = (typeof URL_PARAMS)[keyof typeof URL_PARAMS];
|
||||||
export function getUrlParam(
|
export function getUrlParam(
|
||||||
param: UrlParam & { type: 'string' },
|
param: UrlParam & { type: 'string' },
|
||||||
|
search?: string,
|
||||||
): string | null;
|
): string | null;
|
||||||
export function getUrlParam(
|
export function getUrlParam(
|
||||||
param: UrlParam & { type: 'number' },
|
param: UrlParam & { type: 'number' },
|
||||||
|
search?: string,
|
||||||
): number | null;
|
): number | null;
|
||||||
export function getUrlParam(
|
export function getUrlParam(
|
||||||
param: UrlParam & { type: 'boolean' },
|
param: UrlParam & { type: 'boolean' },
|
||||||
|
search?: string,
|
||||||
): boolean | null;
|
): boolean | null;
|
||||||
export function getUrlParam(
|
export function getUrlParam(
|
||||||
param: UrlParam & { type: 'object' },
|
param: UrlParam & { type: 'object' },
|
||||||
|
search?: string,
|
||||||
): object | null;
|
): object | null;
|
||||||
export function getUrlParam(param: UrlParam & { type: 'rison' }): object | null;
|
export function getUrlParam(
|
||||||
|
param: UrlParam & { type: 'rison' },
|
||||||
|
search?: string,
|
||||||
|
): string | object | null;
|
||||||
export function getUrlParam(
|
export function getUrlParam(
|
||||||
param: UrlParam & { type: 'rison | string' },
|
param: UrlParam & { type: 'rison | string' },
|
||||||
|
search?: string,
|
||||||
): string | object | null;
|
): string | object | null;
|
||||||
export function getUrlParam({ name, type }: UrlParam): unknown {
|
export function getUrlParam(
|
||||||
const urlParam = new URLSearchParams(window.location.search).get(name);
|
{ name, type }: UrlParam,
|
||||||
|
search?: string,
|
||||||
|
): unknown {
|
||||||
|
const urlParam = new URLSearchParams(search ?? window.location.search).get(
|
||||||
|
name,
|
||||||
|
);
|
||||||
switch (type) {
|
switch (type) {
|
||||||
case 'number':
|
case 'number':
|
||||||
if (!urlParam) {
|
if (!urlParam) {
|
||||||
|
|||||||
Reference in New Issue
Block a user