From 582900c170fd1b433dde94e383a0f443496820c5 Mon Sep 17 00:00:00 2001 From: simcha90 <56388545+simcha90@users.noreply.github.com> Date: Tue, 4 May 2021 16:07:40 +0300 Subject: [PATCH] perf(native-filters): Load native filters after charts (#14443) * 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 --- .../util/getFormDataWithExtraFilters_spec.ts | 12 ++++- superset-frontend/src/chart/Chart.jsx | 1 + .../DashboardBuilder/DashboardBuilder.tsx | 2 - .../FilterBar/FilterBar.test.tsx | 7 ++- .../nativeFilters/FilterBar/index.tsx | 11 ++++- .../nativeFilters/FilterBar/state.ts | 49 ++++++++++++++++++- superset-frontend/src/dashboard/types.ts | 8 +-- 7 files changed, 78 insertions(+), 12 deletions(-) diff --git a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts index 38c29e0f5d00c..6561b60b6ab1e 100644 --- a/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts +++ b/superset-frontend/spec/javascripts/dashboard/util/getFormDataWithExtraFilters_spec.ts @@ -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: [ @@ -43,7 +53,7 @@ describe('getFormDataWithExtraFilters', () => { const mockArgs: GetFormDataWithExtraFiltersArguments = { chartConfiguration: {}, charts: { - [chartId]: mockChart, + [chartId as number]: mockChart, }, chart: mockChart, filters: { diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx index 0ca590a1309cb..d9f7e93ccf7b2 100644 --- a/superset-frontend/src/chart/Chart.jsx +++ b/superset-frontend/src/chart/Chart.jsx @@ -214,6 +214,7 @@ class Chart extends React.PureComponent { showMessage={false} > { 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', () => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx index 77f47d171dca6..534866b57d343 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx @@ -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, @@ -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`; @@ -216,6 +218,9 @@ const FilterBar: React.FC = ({ getOnlyExtraFormData(dataMaskApplied), { ignoreUndefined: true }, ) || dataSelectedValues.length !== dataAppliedValues.length; + + const isInitialized = useInitialization(); + return ( = ({ dataMaskSelected={dataMaskSelected} dataMaskApplied={dataMaskApplied} /> - {isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? ( + {!isInitialized ? ( + + ) : isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) ? ( @@ -72,3 +72,48 @@ export const useFilterUpdates = ( }); }, [dataMaskApplied, dataMaskSelected, filters, setDataMaskSelected]); }; + +// Load filters after charts loaded +export const useInitialization = () => { + const [isInitialized, setIsInitialized] = useState(false); + const charts = useSelector(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; +}; diff --git a/superset-frontend/src/dashboard/types.ts b/superset-frontend/src/dashboard/types.ts index f01c2ac66fd0a..a37cc0f938702 100644 --- a/superset-frontend/src/dashboard/types.ts +++ b/superset-frontend/src/dashboard/types.ts @@ -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; @@ -34,8 +35,7 @@ export interface ChartQueryPayload extends Partial { } /** Chart state of redux */ -export type Chart = { - id: number; +export type Chart = ChartState & { formData: { viz_type: string; }; @@ -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;