Skip to content

Commit

Permalink
fix(dashboard): race condition between hydrating dashboard and set ac…
Browse files Browse the repository at this point in the history
…tive tabs (#17084)

* fix(dashboard): race condition between hydrating dashboard and set active tabs

* Fix linting

* Change variable name
  • Loading branch information
kgabryje committed Oct 13, 2021
1 parent 40e9add commit 3ad7483
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 16 deletions.
13 changes: 6 additions & 7 deletions superset-frontend/src/dashboard/containers/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useEffect, FC } from 'react';
import React, { useEffect, useRef, FC } from 'react';
import { t } from '@superset-ui/core';
import { useDispatch } from 'react-redux';
import { useParams } from 'react-router-dom';
Expand Down Expand Up @@ -57,17 +57,16 @@ const DashboardPage: FC = () => {
const { result: datasets, error: datasetsApiError } = useDashboardDatasets(
idOrSlug,
);
const isDashboardHydrated = useRef(false);

const error = dashboardApiError || chartsApiError;
const readyToRender = Boolean(dashboard && charts);
const { dashboard_title, css } = dashboard || {};

useEffect(() => {
if (readyToRender) {
dispatch(hydrateDashboard(dashboard, charts));
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [readyToRender]);
if (readyToRender && !isDashboardHydrated.current) {
isDashboardHydrated.current = true;
dispatch(hydrateDashboard(dashboard, charts));
}

useEffect(() => {
if (dashboard_title) {
Expand Down
12 changes: 4 additions & 8 deletions superset-frontend/src/dashboard/reducers/dashboardState.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,12 @@ export default function dashboardStateReducer(state = {}, action) {
};
},
[SET_ACTIVE_TABS]() {
const prevActiveTabs = state.activeTabs ?? [];
const newActiveTabs = action.prevTabId
? [
...prevActiveTabs.filter(tabId => tabId !== action.prevTabId),
action.tabId,
]
: [...prevActiveTabs, action.tabId];
const newActiveTabs = new Set(state.activeTabs);
newActiveTabs.delete(action.prevTabId);
newActiveTabs.add(action.tabId);
return {
...state,
activeTabs: newActiveTabs,
activeTabs: Array.from(newActiveTabs),
};
},
[SET_FOCUSED_FILTER_FIELD]() {
Expand Down
38 changes: 38 additions & 0 deletions superset-frontend/src/dashboard/reducers/dashboardState.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* 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.
*/

import dashboardStateReducer from './dashboardState';
import { setActiveTabs } from '../actions/dashboardState';

describe('DashboardState reducer', () => {
it('SET_ACTIVE_TABS', () => {
expect(
dashboardStateReducer({ activeTabs: [] }, setActiveTabs('tab1')),
).toEqual({ activeTabs: ['tab1'] });
expect(
dashboardStateReducer({ activeTabs: ['tab1'] }, setActiveTabs('tab1')),
).toEqual({ activeTabs: ['tab1'] });
expect(
dashboardStateReducer(
{ activeTabs: ['tab1'] },
setActiveTabs('tab2', 'tab1'),
),
).toEqual({ activeTabs: ['tab2'] });
});
});
7 changes: 6 additions & 1 deletion superset-frontend/src/reduxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,12 @@ export function initEnhancer(
const composeEnhancers =
process.env.WEBPACK_MODE === 'development'
? /* eslint-disable-next-line no-underscore-dangle, dot-notation */
window['__REDUX_DEVTOOLS_EXTENSION_COMPOSE__'] || compose
window['__REDUX_DEVTOOLS_EXTENSION_COMPOSE__']
? /* eslint-disable-next-line no-underscore-dangle, dot-notation */
window['__REDUX_DEVTOOLS_EXTENSION_COMPOSE__']({
trace: true,
})
: compose
: compose;

return persist
Expand Down

0 comments on commit 3ad7483

Please sign in to comment.