Skip to content

Commit

Permalink
perf(native-filters): Load native filters after charts (#14443)
Browse files Browse the repository at this point in the history
* fix:fix get permission function

* refactor: filter default value

* refactor: update default value loading

* refactor: apply defaultValues

* lint: fix lint

* lint: fix lint

* test: fix test

* refactor: use extraFormData for reload charts

* feat: load native filters after after charts

* feat: load filters after charts

* fix: revert changes

* test: fix timers

* test: fix tests
  • Loading branch information
simcha90 authored May 4, 2021
1 parent 9a22fb0 commit 582900c
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ describe('getFormDataWithExtraFilters', () => {
const filterId = 'native-filter-1';
const mockChart = {
id: chartId,
chartAlert: null,
chartStatus: null,
chartUpdateEndTime: null,
chartUpdateStartTime: 1,
lastRendered: 1,
latestQueryFormData: {},
sliceFormData: null,
queryController: null,
queriesResponse: null,
triggerQuery: false,
formData: {
viz_type: 'filter_select',
filters: [
Expand All @@ -43,7 +53,7 @@ describe('getFormDataWithExtraFilters', () => {
const mockArgs: GetFormDataWithExtraFiltersArguments = {
chartConfiguration: {},
charts: {
[chartId]: mockChart,
[chartId as number]: mockChart,
},
chart: mockChart,
filters: {
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/src/chart/Chart.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ class Chart extends React.PureComponent {
showMessage={false}
>
<Styles
data-ui-anchor="chart"
className="chart-container"
data-test="chart-container"
height={height}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import React, { FC, SyntheticEvent, useEffect, useState } from 'react';
import { Sticky, StickyContainer } from 'react-sticky';
import { TabContainer } from 'react-bootstrap';
import { JsonObject, styled } from '@superset-ui/core';

import ErrorBoundary from 'src/components/ErrorBoundary';
import BuilderComponentPane from 'src/dashboard/components/BuilderComponentPane';
import DashboardHeader from 'src/dashboard/containers/DashboardHeader';
Expand All @@ -31,7 +30,6 @@ import DragDroppable from 'src/dashboard/components/dnd/DragDroppable';
import DashboardComponent from 'src/dashboard/containers/DashboardComponent';
import ToastPresenter from 'src/messageToasts/containers/ToastPresenter';
import WithPopoverMenu from 'src/dashboard/components/menu/WithPopoverMenu';

import getDirectPathToTabIndex from 'src/dashboard/util/getDirectPathToTabIndex';
import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
import { URL_PARAMS } from 'src/constants';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import mockDatasource, {
import FilterBar, { FILTER_BAR_TEST_ID } from '.';
import { FILTERS_CONFIG_MODAL_TEST_ID } from '../FiltersConfigModal/FiltersConfigModal';

jest.useFakeTimers();
// @ts-ignore
mockCore.makeApi = jest.fn();

Expand Down Expand Up @@ -244,9 +245,11 @@ describe('FilterBar', () => {
expect(screen.getByRole('img', { name: 'filter' })).toBeInTheDocument();
});

it('should render the filter control name', () => {
it('should render the filter control name', async () => {
renderWrapper();
expect(screen.getByText('test')).toBeInTheDocument();
expect(
await screen.findByText('test', {}, { timeout: 2000 }),
).toBeInTheDocument();
});

it('should toggle', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import { useImmer } from 'use-immer';
import { areObjectsEqual } from 'src/reduxUtils';
import { testWithId } from 'src/utils/testUtils';
import { Filter } from 'src/dashboard/components/nativeFilters/types';
import Loading from 'src/components/Loading';
import { getInitialDataMask } from 'src/dataMask/reducer';
import {
getOnlyExtraFormData,
mapParentFiltersToChildren,
Expand All @@ -46,11 +48,11 @@ import {
useFilters,
useFilterSets,
useFilterUpdates,
useInitialization,
} from './state';
import EditSection from './FilterSets/EditSection';
import Header from './Header';
import FilterControls from './FilterControls/FilterControls';
import { getInitialDataMask } from '../../../../dataMask/reducer';

const BAR_WIDTH = `250px`;

Expand Down Expand Up @@ -216,6 +218,9 @@ const FilterBar: React.FC<FiltersBarProps> = ({
getOnlyExtraFormData(dataMaskApplied),
{ ignoreUndefined: true },
) || dataSelectedValues.length !== dataAppliedValues.length;

const isInitialized = useInitialization();

return (
<BarWrapper {...getFilterBarTestId()} className={cx({ open: filtersOpen })}>
<CollapsedBar
Expand All @@ -238,7 +243,9 @@ const FilterBar: React.FC<FiltersBarProps> = ({
dataMaskSelected={dataMaskSelected}
dataMaskApplied={dataMaskApplied}
/>
{isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? (
{!isInitialized ? (
<Loading />
) : isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? (
<StyledTabs
centered
onChange={setTab as HandlerFunction}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import {
DataMaskStateWithId,
DataMaskWithId,
} from 'src/dataMask/types';
import { useEffect } from 'react';
import { RootState } from 'src/dashboard/types';
import { useEffect, useState } from 'react';
import { ChartsState, RootState } from 'src/dashboard/types';
import { NATIVE_FILTER_PREFIX } from '../FiltersConfigModal/utils';

export const useFilterSets = () =>
Expand Down Expand Up @@ -72,3 +72,48 @@ export const useFilterUpdates = (
});
}, [dataMaskApplied, dataMaskSelected, filters, setDataMaskSelected]);
};

// Load filters after charts loaded
export const useInitialization = () => {
const [isInitialized, setIsInitialized] = useState<boolean>(false);
const charts = useSelector<RootState, ChartsState>(state => state.charts);

// We need to know how much charts now shown on dashboard to know how many of all charts should be loaded
let numberOfLoadingCharts = 0;
if (!isInitialized) {
numberOfLoadingCharts = document.querySelectorAll(
'[data-ui-anchor="chart"]',
).length;
}
useEffect(() => {
if (isInitialized) {
return;
}

// For some dashboards may be there are no charts on first page,
// so we check up to 1 sec if there is at least on chart to load
let filterTimeout: NodeJS.Timeout;
if (numberOfLoadingCharts === 0) {
filterTimeout = setTimeout(() => {
setIsInitialized(true);
}, 1000);
}

// @ts-ignore
if (numberOfLoadingCharts > 0 && filterTimeout !== undefined) {
clearTimeout(filterTimeout);
}

const numberOfLoadedCharts = Object.values(charts).filter(
({ chartStatus }) => chartStatus !== 'loading',
).length;
if (
numberOfLoadingCharts > 0 &&
numberOfLoadedCharts >= numberOfLoadingCharts
) {
setIsInitialized(true);
}
}, [charts, isInitialized, numberOfLoadingCharts]);

return isInitialized;
};
8 changes: 5 additions & 3 deletions superset-frontend/src/dashboard/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { chart } from 'src/chart/chartReducer';
import componentTypes from 'src/dashboard/util/componentTypes';
import { DataMaskStateWithId } from '../dataMask/types';
import { NativeFiltersState } from './reducers/types';
import { ChartState } from '../explore/types';

export type ChartReducerInitialState = typeof chart;

Expand All @@ -34,8 +35,7 @@ export interface ChartQueryPayload extends Partial<ChartReducerInitialState> {
}

/** Chart state of redux */
export type Chart = {
id: number;
export type Chart = ChartState & {
formData: {
viz_type: string;
};
Expand All @@ -54,11 +54,13 @@ export type DashboardInfo = {
metadata: { show_native_filters: boolean; chart_configuration: JsonObject };
};

export type ChartsState = { [key: string]: Chart };

/** Root state of redux */
export type RootState = {
datasources: JsonObject;
sliceEntities: JsonObject;
charts: { [key: string]: Chart };
charts: ChartsState;
dashboardLayout: DashboardLayoutState;
dashboardFilters: {};
dashboardState: DashboardState;
Expand Down

0 comments on commit 582900c

Please sign in to comment.