Skip to content

Commit 20e6dfd

Browse files
sadpandajoesadpandajoe
authored
fix(dashboard): seed default active tab path at hydration (#42075)
1 parent 5067230 commit 20e6dfd

7 files changed

Lines changed: 612 additions & 86 deletions

File tree

‎superset-frontend/src/dashboard/actions/hydrate.test.ts‎

Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
import { HYDRATE_DASHBOARD, hydrateDashboard } from './hydrate';
20+
import {
21+
DASHBOARD_ROOT_TYPE,
22+
TABS_TYPE,
23+
TAB_TYPE,
24+
} from '../util/componentTypes';
25+
import { DASHBOARD_ROOT_ID } from '../util/constants';
26+
27+
/**
28+
* Regression guard for the follow-up to PR #39417 / PR #41832: the default
29+
* active-tab path must be seeded into `dashboardState.activeTabs` at hydration
30+
* time, so every first-render consumer (filter bar, share menus, permalink
31+
* utilities, screenshot download) reads the dashboard's real default tab path
32+
* instead of an empty array.
33+
*
34+
* Before this change, hydrate seeded `activeTabs: activeTabs ||
35+
* dashboardState?.activeTabs || []` (hydrate.ts). With no permalink, no stored
36+
* state and no directPathToChild, that resolved to `[]`, and the live `Tabs`
37+
* component only populated the value from a post-mount effect. These tests
38+
* assert the seeded default path and fail without the hydration-time seeding.
39+
*/
40+
41+
const layoutItem = (
42+
id: string,
43+
type: string,
44+
children: string[],
45+
parents: string[],
46+
) => ({ id, type, children, parents, meta: {} });
47+
48+
const buildDashboard = (
49+
positionData: Record<string, unknown>,
50+
title = 'Test dashboard',
51+
) => ({
52+
id: 1,
53+
dashboard_title: title,
54+
css: '',
55+
published: true,
56+
changed_on: '2024-01-01T00:00:00.000Z',
57+
owners: [],
58+
metadata: {},
59+
position_data: positionData,
60+
});
61+
62+
const hydrate = (
63+
positionData: Record<string, unknown>,
64+
overrides: {
65+
activeTabs?: string[] | null;
66+
dashboardState?: Record<string, unknown>;
67+
} = {},
68+
) => {
69+
const dispatch = jest.fn((action: unknown) => action);
70+
const getState = () =>
71+
({
72+
user: { roles: {}, userId: 1 },
73+
common: { conf: {} },
74+
dashboardState: overrides.dashboardState ?? {},
75+
}) as any;
76+
const action = (
77+
hydrateDashboard({
78+
history: { replace: jest.fn() },
79+
dashboard: buildDashboard(positionData),
80+
charts: [],
81+
dataMask: {},
82+
activeTabs: overrides.activeTabs ?? null,
83+
chartStates: null,
84+
} as any) as any
85+
)(dispatch, getState);
86+
return action;
87+
};
88+
89+
test('seeds the default (first) tab path for a flat ROOT → TABS → TAB layout', () => {
90+
const positionData = {
91+
[DASHBOARD_ROOT_ID]: layoutItem(
92+
DASHBOARD_ROOT_ID,
93+
DASHBOARD_ROOT_TYPE,
94+
['TABS-1'],
95+
[],
96+
),
97+
'TABS-1': layoutItem(
98+
'TABS-1',
99+
TABS_TYPE,
100+
['TAB-1', 'TAB-2'],
101+
[DASHBOARD_ROOT_ID],
102+
),
103+
'TAB-1': layoutItem('TAB-1', TAB_TYPE, [], [DASHBOARD_ROOT_ID, 'TABS-1']),
104+
'TAB-2': layoutItem('TAB-2', TAB_TYPE, [], [DASHBOARD_ROOT_ID, 'TABS-1']),
105+
};
106+
107+
const action = hydrate(positionData);
108+
109+
expect(action.type).toBe(HYDRATE_DASHBOARD);
110+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-1']);
111+
});
112+
113+
test('seeds the recursive default path for nested TABS containers', () => {
114+
const positionData = {
115+
[DASHBOARD_ROOT_ID]: layoutItem(
116+
DASHBOARD_ROOT_ID,
117+
DASHBOARD_ROOT_TYPE,
118+
['TABS-1'],
119+
[],
120+
),
121+
'TABS-1': layoutItem(
122+
'TABS-1',
123+
TABS_TYPE,
124+
['TAB-1', 'TAB-2'],
125+
[DASHBOARD_ROOT_ID],
126+
),
127+
'TAB-1': layoutItem(
128+
'TAB-1',
129+
TAB_TYPE,
130+
['TABS-2'],
131+
[DASHBOARD_ROOT_ID, 'TABS-1'],
132+
),
133+
'TAB-2': layoutItem('TAB-2', TAB_TYPE, [], [DASHBOARD_ROOT_ID, 'TABS-1']),
134+
'TABS-2': layoutItem(
135+
'TABS-2',
136+
TABS_TYPE,
137+
['TAB-1-1', 'TAB-1-2'],
138+
[DASHBOARD_ROOT_ID, 'TABS-1', 'TAB-1'],
139+
),
140+
'TAB-1-1': layoutItem(
141+
'TAB-1-1',
142+
TAB_TYPE,
143+
[],
144+
[DASHBOARD_ROOT_ID, 'TABS-1', 'TAB-1', 'TABS-2'],
145+
),
146+
'TAB-1-2': layoutItem(
147+
'TAB-1-2',
148+
TAB_TYPE,
149+
[],
150+
[DASHBOARD_ROOT_ID, 'TABS-1', 'TAB-1', 'TABS-2'],
151+
),
152+
};
153+
154+
const action = hydrate(positionData);
155+
156+
expect(action.type).toBe(HYDRATE_DASHBOARD);
157+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-1', 'TAB-1-1']);
158+
});
159+
160+
test('seeds the default tab path for an embedded top-level-TABS layout (hideTab scenario)', () => {
161+
// hideTab is a `uiConfig` property (read at DashboardBuilder.tsx, outside
162+
// hydrated state), not a layout/position_data property, so it cannot and
163+
// need not appear in this fixture. The seed derives purely from layout
164+
// shape and is correct whether or not the top-level tab bar renders — this
165+
// is an ordinary top-level-TABS layout mirroring state.test.ts's
166+
// `embeddedLayout`, re-anchoring the #39417 embedded-filter-bar regression
167+
// guard at the layer (hydration) that now owns the seeded value.
168+
const positionData = {
169+
[DASHBOARD_ROOT_ID]: layoutItem(
170+
DASHBOARD_ROOT_ID,
171+
DASHBOARD_ROOT_TYPE,
172+
['TABS-1'],
173+
[],
174+
),
175+
'TABS-1': layoutItem(
176+
'TABS-1',
177+
TABS_TYPE,
178+
['TAB-Company', 'TAB-Desktop'],
179+
[DASHBOARD_ROOT_ID],
180+
),
181+
'TAB-Company': layoutItem(
182+
'TAB-Company',
183+
TAB_TYPE,
184+
[],
185+
[DASHBOARD_ROOT_ID, 'TABS-1'],
186+
),
187+
'TAB-Desktop': layoutItem(
188+
'TAB-Desktop',
189+
TAB_TYPE,
190+
[],
191+
[DASHBOARD_ROOT_ID, 'TABS-1'],
192+
),
193+
};
194+
195+
const action = hydrate(positionData);
196+
197+
expect(action.type).toBe(HYDRATE_DASHBOARD);
198+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-Company']);
199+
});
200+
201+
// Precedence: the layout default only applies
202+
// to a genuinely fresh load. A permalink `activeTabs`, a non-empty stored
203+
// redux value, or a non-empty `directPathToChild` (deep link) must each
204+
// suppress it.
205+
const flatTabsPositionData = {
206+
[DASHBOARD_ROOT_ID]: layoutItem(
207+
DASHBOARD_ROOT_ID,
208+
DASHBOARD_ROOT_TYPE,
209+
['TABS-1'],
210+
[],
211+
),
212+
'TABS-1': layoutItem(
213+
'TABS-1',
214+
TABS_TYPE,
215+
['TAB-1', 'TAB-2'],
216+
[DASHBOARD_ROOT_ID],
217+
),
218+
'TAB-1': layoutItem('TAB-1', TAB_TYPE, [], [DASHBOARD_ROOT_ID, 'TABS-1']),
219+
'TAB-2': layoutItem('TAB-2', TAB_TYPE, [], [DASHBOARD_ROOT_ID, 'TABS-1']),
220+
};
221+
222+
test('a permalink activeTabs param suppresses the layout default', () => {
223+
const action = hydrate(flatTabsPositionData, { activeTabs: ['TAB-2'] });
224+
225+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-2']);
226+
});
227+
228+
test('a non-empty stored redux activeTabs value suppresses the layout default', () => {
229+
const action = hydrate(flatTabsPositionData, {
230+
dashboardState: { activeTabs: ['TAB-2'] },
231+
});
232+
233+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-2']);
234+
});
235+
236+
test('a non-empty directPathToChild (deep link) suppresses the layout default', () => {
237+
const action = hydrate(flatTabsPositionData, {
238+
dashboardState: { directPathToChild: ['TAB-2'] },
239+
});
240+
241+
expect(action.data.dashboardState.activeTabs).toEqual([]);
242+
});
243+
244+
test('an empty stored redux activeTabs value still falls through to the layout default', () => {
245+
// Pins the `.length` guard: a stored `activeTabs: []` is truthy but must
246+
// not be treated as "already populated" — otherwise it would win over the
247+
// layout default and regress to the pre-fix `[]`.
248+
const action = hydrate(flatTabsPositionData, {
249+
dashboardState: { activeTabs: [] },
250+
});
251+
252+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-1']);
253+
});
254+
255+
test('a non-empty stored redux value wins over a non-empty directPathToChild', () => {
256+
// Pins the `||` operand order: stored value is checked before the
257+
// directPathToChild-gated default branch.
258+
const action = hydrate(flatTabsPositionData, {
259+
dashboardState: { activeTabs: ['TAB-2'], directPathToChild: ['TAB-1'] },
260+
});
261+
262+
expect(action.data.dashboardState.activeTabs).toEqual(['TAB-2']);
263+
});
264+
265+
test('a permalink activeTabs: [] (empty but present) wins and seeds []', () => {
266+
// INTENTIONAL legacy-link behavior: unlike the stored-redux operand, the
267+
// permalink param is not `?.length`-normalized, so a permalink that
268+
// deliberately encoded "no active tabs" still wins over the layout
269+
// default. This degrades to the pre-fix one-paint-late correction (the
270+
// live Tabs component resolves the default after mount) rather than
271+
// seeding the layout default — no regression, since that was already
272+
// today's behavior for such links.
273+
const action = hydrate(flatTabsPositionData, { activeTabs: [] });
274+
275+
expect(action.data.dashboardState.activeTabs).toEqual([]);
276+
});

‎superset-frontend/src/dashboard/actions/hydrate.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import {
4949
ROW_TYPE,
5050
} from 'src/dashboard/util/componentTypes';
5151
import findFirstParentContainerId from 'src/dashboard/util/findFirstParentContainer';
52+
import getDefaultActiveTabs from 'src/dashboard/util/getDefaultActiveTabs';
5253
import getEmptyLayout from 'src/dashboard/util/getEmptyLayout';
5354
import getLocationHash from 'src/dashboard/util/getLocationHash';
5455
import newComponentFactory, {
@@ -327,6 +328,19 @@ export const hydrateDashboard =
327328
metadata.cross_filters_enabled as boolean | undefined,
328329
);
329330

331+
// precedence: permalink param > stored redux value > layout default.
332+
// The layout default only applies to a genuinely fresh load: no permalink
333+
// activeTabs, no stored activeTabs, and no deep-link (directPathToChild),
334+
// which the live Tabs component resolves on its own.
335+
const seededActiveTabs =
336+
activeTabs ||
337+
(dashboardState?.activeTabs?.length
338+
? dashboardState.activeTabs
339+
: undefined) ||
340+
(directPathToChild.length
341+
? []
342+
: getDefaultActiveTabs(dashboardLayout.present as DashboardLayout));
343+
330344
return dispatch({
331345
type: HYDRATE_DASHBOARD,
332346
data: {
@@ -390,7 +404,7 @@ export const hydrateDashboard =
390404
lastModifiedTime: dashboard.changed_on,
391405
isRefreshing: false,
392406
isFiltersRefreshing: false,
393-
activeTabs: activeTabs || dashboardState?.activeTabs || [],
407+
activeTabs: seededActiveTabs,
394408
datasetsStatus:
395409
dashboardState?.datasetsStatus || ResourceStatus.Loading,
396410
chartStates: chartStates || dashboardState?.chartStates || {},

0 commit comments

Comments
 (0)