From 13d60f31cfe0fc1b109ad16cad893adc4d8fbccc Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Tue, 2 Mar 2021 15:59:58 +0200 Subject: [PATCH 01/13] feat: add BE pagination to table with pages --- .../src/query/buildQueryContext.ts | 3 + .../superset-ui-core/src/query/types/Query.ts | 2 + .../src/DataTable/DataTable.tsx | 76 +++++++++++-------- .../DataTable/components/SelectPageSize.tsx | 5 +- .../DataTable/components/ServerPagination.tsx | 58 -------------- .../src/DataTable/utils/externalAPIs.ts | 2 +- plugins/plugin-chart-table/src/TableChart.tsx | 19 ++--- plugins/plugin-chart-table/src/buildQuery.ts | 57 ++++++++++---- plugins/plugin-chart-table/src/index.ts | 4 +- .../plugin-chart-table/src/transformProps.ts | 36 ++++----- plugins/plugin-chart-table/src/types.ts | 6 +- 11 files changed, 128 insertions(+), 140 deletions(-) delete mode 100644 plugins/plugin-chart-table/src/DataTable/components/ServerPagination.tsx diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index bf9880d955..e7a817c138 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -7,6 +7,8 @@ const WRAP_IN_ARRAY = (baseQueryObject: QueryObject) => [baseQueryObject]; export type BuildFinalQuerieObjects = (baseQueryObject: QueryObject) => QueryObject[]; +const emptyFunction = () => {}; + export default function buildQueryContext( formData: QueryFormData, options?: @@ -24,5 +26,6 @@ export default function buildQueryContext( queries: buildQuery(buildQueryObject(formData, queryFields)), result_format: formData.result_format || 'json', result_type: formData.result_type || 'full', + setDataMask: formData.setDataMask ?? emptyFunction, }; } diff --git a/packages/superset-ui-core/src/query/types/Query.ts b/packages/superset-ui-core/src/query/types/Query.ts index 6e1b4543fa..36c2ae242a 100644 --- a/packages/superset-ui-core/src/query/types/Query.ts +++ b/packages/superset-ui-core/src/query/types/Query.ts @@ -24,6 +24,7 @@ import { AnnotationLayer } from './AnnotationLayer'; import { QueryFields, QueryFormMetric } from './QueryFormData'; import { Maybe } from '../../types'; import { PostProcessingRule } from './PostProcessing'; +import { SetDataMaskHook } from '../../chart'; export type QueryObjectFilterClause = { col: string; @@ -134,6 +135,7 @@ export interface QueryContext { /** Response format */ result_format: string; queries: QueryObject[]; + setDataMask: SetDataMaskHook; } export default {}; diff --git a/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/plugins/plugin-chart-table/src/DataTable/DataTable.tsx index 4f5e01387d..e7bef4c51b 100644 --- a/plugins/plugin-chart-table/src/DataTable/DataTable.tsx +++ b/plugins/plugin-chart-table/src/DataTable/DataTable.tsx @@ -42,8 +42,7 @@ import SelectPageSize, { SelectPageSizeProps, SizeOption } from './components/Se import SimplePagination from './components/Pagination'; import useSticky from './hooks/useSticky'; import { updateExternalFormData } from './utils/externalAPIs'; -import ServerPagination from './components/ServerPagination'; -import { ServerPage } from '../types'; +import { PAGE_SIZE_OPTIONS } from '../controlPanel'; export interface DataTableProps extends TableOptions { tableClassName?: string; @@ -56,11 +55,11 @@ export interface DataTableProps extends TableOptions { height?: string | number; serverPagination?: boolean; setDataMask: SetDataMaskHook; - currentPage?: number; + ownCurrentState: { pageSize?: number; currentPage?: number }; pageSize?: number; noResults?: string | ((filterString: string) => ReactNode); sticky?: boolean; - showNextButton: boolean; + rowCount: number; wrapperRef?: MutableRefObject; } @@ -73,17 +72,17 @@ export default function DataTable({ tableClassName, columns, data, - currentPage = 0, + ownCurrentState, width: initialWidth = '100%', height: initialHeight = 300, pageSize: initialPageSize = 0, initialState: initialState_ = {}, - pageSizeOptions = [10, 25, 50, 100, 200], + pageSizeOptions = PAGE_SIZE_OPTIONS, maxPageItemCount = 9, sticky: doSticky, searchInput = true, setDataMask, - showNextButton, + rowCount, selectPageSize, noResults: noResultsText = 'No data found', hooks, @@ -98,7 +97,7 @@ export default function DataTable({ doSticky ? useSticky : [], hooks || [], ].flat(); - const resultsSize = data.length; + const resultsSize = serverPagination ? rowCount : data.length; const sortByRef = useRef([]); // cache initial `sortby` so sorting doesn't trigger page reset const pageSizeRef = useRef([initialPageSize, resultsSize]); const hasPagination = initialPageSize > 0 && resultsSize > 0; // pageSize == 0 means no pagination @@ -129,7 +128,16 @@ export default function DataTable({ } return undefined; // eslint-disable-next-line react-hooks/exhaustive-deps - }, [initialHeight, initialWidth, wrapperRef, hasPagination, hasGlobalControl, showNextButton]); + }, [ + initialHeight, + initialWidth, + wrapperRef, + hasPagination, + hasGlobalControl, + paginationRef, + resultsSize, + JSON.stringify(ownCurrentState), + ]); const defaultGlobalFilter: FilterType = useCallback( (rows: Row[], columnIds: IdType[], filterValue: string) => { @@ -224,7 +232,7 @@ export default function DataTable({ ); - // force upate the pageSize when it's been update from the initial state + // force update the pageSize when it's been update from the initial state if ( pageSizeRef.current[0] !== initialPageSize || // when initialPageSize stays as zero, but total number of records changed, @@ -235,16 +243,29 @@ export default function DataTable({ setPageSize(initialPageSize); } - const goToBEPage = (direction: ServerPage) => { - updateExternalFormData( - setDataMask, - direction === ServerPage.NEXT ? currentPage + 1 : currentPage - 1, - pageSize, - ); - }; - const paginationStyle: CSSProperties = sticky.height ? {} : { visibility: 'hidden' }; + let resultPageCount = pageCount; + let resultCurrentPageSize = pageSize; + let resultCurrentPage = pageIndex; + let resultOnPageChange: (page: number) => void = gotoPage; + if (serverPagination) { + const serverPageSize = ownCurrentState.pageSize ?? initialPageSize; + resultPageCount = Math.ceil(rowCount / serverPageSize); + if (!Number.isFinite(resultPageCount)) { + resultPageCount = 0; + } + resultCurrentPageSize = serverPageSize; + const foundPageSizeIndex = pageSizeOptions.findIndex( + ([option]) => option >= resultCurrentPageSize, + ); + if (foundPageSizeIndex === -1) { + resultCurrentPageSize = 0; + } + resultCurrentPage = ownCurrentState.currentPage ?? 0; + resultOnPageChange = (pageNumber: number) => + updateExternalFormData(setDataMask, pageNumber, serverPageSize); + } return (
{hasGlobalControl ? ( @@ -254,7 +275,7 @@ export default function DataTable({ {hasPagination ? ( ({
) : null} {wrapStickyTable ? wrapStickyTable(renderTable) : renderTable()} - {serverPagination && ( - - )} - {!serverPagination && hasPagination ? ( + {hasPagination && resultPageCount > 1 ? ( ) : null} diff --git a/plugins/plugin-chart-table/src/DataTable/components/SelectPageSize.tsx b/plugins/plugin-chart-table/src/DataTable/components/SelectPageSize.tsx index c098e2c5c9..5355b85f42 100644 --- a/plugins/plugin-chart-table/src/DataTable/components/SelectPageSize.tsx +++ b/plugins/plugin-chart-table/src/DataTable/components/SelectPageSize.tsx @@ -17,8 +17,9 @@ * under the License. */ import React from 'react'; +import { formatSelectOptions } from '@superset-ui/chart-controls'; -export type SizeOption = number | [number, string]; +export type SizeOption = [number, string]; export interface SelectPageSizeRendererProps { current: number; @@ -81,7 +82,7 @@ export default React.memo(function SelectPageSize({ options.splice( sizeOptionValues.findIndex(x => x > currentSize), 0, - currentSize, + formatSelectOptions([currentSize])[0], ); } const current = currentSize === undefined ? sizeOptionValues[0] : currentSize; diff --git a/plugins/plugin-chart-table/src/DataTable/components/ServerPagination.tsx b/plugins/plugin-chart-table/src/DataTable/components/ServerPagination.tsx deleted file mode 100644 index 538dba66d0..0000000000 --- a/plugins/plugin-chart-table/src/DataTable/components/ServerPagination.tsx +++ /dev/null @@ -1,58 +0,0 @@ -/** - * 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 React, { CSSProperties } from 'react'; -import { t } from '@superset-ui/core'; -import { ServerPage } from '../../types'; - -export interface ServerPaginationProps { - showNext: boolean; - showPrevious: boolean; - onPageChange: (direction: ServerPage) => void; // `page` next/previous - style?: CSSProperties; -} - -export default React.memo( - React.forwardRef(function ServerPagination( - { style, onPageChange, showNext, showPrevious }: ServerPaginationProps, - ref: React.Ref, - ) { - const getButton = (name: ServerPage, label: string) => ( -
  • - { - e.preventDefault(); - onPageChange(name); - }} - > - {label} - -
    • - ); - return ( -
    -
      - {showPrevious && getButton(ServerPage.PREVIOUS, t('table.previous_page'))} - {showNext && getButton(ServerPage.NEXT, t('table.next_page'))} -
    -
    - ); - }), -); diff --git a/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts b/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts index d87e3ffebd..dbd4cf49a7 100644 --- a/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts +++ b/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts @@ -29,7 +29,7 @@ export const updateExternalFormData = ( extraFormData: { custom_form_data: { row_offset: pageNumber * pageSize, - row_limit: pageSize + 1, + row_limit: pageSize, }, }, currentState: { diff --git a/plugins/plugin-chart-table/src/TableChart.tsx b/plugins/plugin-chart-table/src/TableChart.tsx index 41c1611e29..e57ff8aa3e 100644 --- a/plugins/plugin-chart-table/src/TableChart.tsx +++ b/plugins/plugin-chart-table/src/TableChart.tsx @@ -147,14 +147,14 @@ export default function TableChart( width, data, isRawRecords, - showNextButton, + rowCount = 0, columns: columnsMeta, alignPositiveNegative = false, colorPositiveNegative = false, includeSearch = false, pageSize = 0, serverPagination = false, - currentPage, + ownCurrentState, setDataMask, showCellBars = true, emitFilter = false, @@ -167,11 +167,12 @@ export default function TableChart( const [filters, setFilters] = useState(initialFilters); // only take relevant page size options - const pageSizeOptions = useMemo( - () => - PAGE_SIZE_OPTIONS.filter(([n]) => n <= 2 * data.length || serverPagination) as SizeOption[], - [data.length], - ); + const pageSizeOptions = useMemo(() => { + const getServerPagination = (n: number) => n <= rowCount; + return PAGE_SIZE_OPTIONS.filter(([n]) => + serverPagination ? getServerPagination(n) : n <= 2 * data.length, + ) as SizeOption[]; + }, [data.length, rowCount]); const getValueRange = useCallback( function getValueRange(key: string) { @@ -289,10 +290,10 @@ export default function TableChart( columns={columns} data={data} - showNextButton={showNextButton} + rowCount={rowCount} tableClassName="table table-striped table-condensed" pageSize={pageSize} - currentPage={currentPage} + ownCurrentState={ownCurrentState} pageSizeOptions={pageSizeOptions} width={width} height={height} diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index c4074efc80..1cf5bdfde2 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -26,6 +26,7 @@ import { } from '@superset-ui/core'; import { PostProcessingRule } from '@superset-ui/core/src/query/types/PostProcessing'; import { TableChartFormData } from './types'; +import { updateExternalFormData } from './DataTable/utils/externalAPIs'; /** * Infer query mode from form data. If `all_columns` is set, then raw records mode, @@ -43,7 +44,7 @@ export function getQueryMode(formData: TableChartFormData) { return hasRawColumns ? QueryMode.raw : QueryMode.aggregate; } -export default function buildQuery(formData: TableChartFormData) { +function buildQuery(formData: TableChartFormData) { const { percent_metrics: percentMetrics, order_desc: orderDesc = false } = formData; const queryMode = getQueryMode(formData); const sortByMetric = ensureIsArray(formData.timeseries_limit_metric)[0]; @@ -88,21 +89,49 @@ export default function buildQuery(formData: TableChartFormData) { const moreProps: Partial = {}; if (formDataCopy.server_pagination) { const rowLimit = formDataCopy.extra_form_data?.custom_form_data?.row_limit; - // 1 - means all data - if (rowLimit !== 1) { - moreProps.row_limit = rowLimit ?? formDataCopy.server_page_length + 1; // +1 to determine if exists next page - } + moreProps.row_limit = rowLimit ?? formDataCopy.server_page_length; moreProps.row_offset = formDataCopy?.extra_form_data?.custom_form_data?.row_offset ?? 0; } - return [ - { - ...baseQueryObject, - orderby, - metrics, - post_processing: postProcessing, - ...moreProps, - }, - ]; + let queryObject = { + ...baseQueryObject, + orderby, + metrics, + post_processing: postProcessing, + ...moreProps, + }; + + if ( + formData.server_pagination && + formData?.cachedChanges?.[formData.slice_id] && + JSON.stringify(formData?.cachedChanges?.[formData.slice_id]) !== + JSON.stringify(queryObject.filters) + ) { + queryObject = { ...queryObject, row_offset: 0 }; + updateExternalFormData(formData.setDataMask, 0, queryObject.row_limit ?? 0); + } + // Because we use same buildQuery for all table on the page we need split them by id + formData.setCachedChanges({ [formData.slice_id]: queryObject.filters }); + + if (formData.server_pagination) { + return [ + { ...queryObject }, + { ...queryObject, row_limit: 0, row_offset: 0, post_processing: [], is_rowcount: true }, + ]; + } + return [queryObject]; }); } + +// Use this closure to cache changing of external filters, if we have server pagination we need reset page to 0, after +// external filter changed +const cachedBuildQuery = () => { + let cachedChanges: any = {}; + const setCachedChanges = (_cachedChanges: any) => { + cachedChanges = { ...cachedChanges, ..._cachedChanges }; + }; + return (formData: TableChartFormData) => + buildQuery({ ...formData, cachedChanges, setCachedChanges }); +}; + +export default cachedBuildQuery; diff --git a/plugins/plugin-chart-table/src/index.ts b/plugins/plugin-chart-table/src/index.ts index ff47298054..116484201e 100644 --- a/plugins/plugin-chart-table/src/index.ts +++ b/plugins/plugin-chart-table/src/index.ts @@ -20,7 +20,7 @@ import { t, ChartMetadata, ChartPlugin } from '@superset-ui/core'; import transformProps from './transformProps'; import thumbnail from './images/thumbnail.png'; import controlPanel from './controlPanel'; -import buildQuery from './buildQuery'; +import cachedBuildQuery from './buildQuery'; import { TableChartFormData, TableChartProps } from './types'; // must export something for the module to be exist in dev mode @@ -41,7 +41,7 @@ export default class TableChartPlugin extends ChartPlugin 5000 ? 200 : 0; }; -export default function transformProps(chartProps: TableChartProps): TableChartTransformedProps { +const transformProps = (chartProps: TableChartProps): TableChartTransformedProps => { const { height, width, rawFormData: formData, queriesData, initialValues: filters = {}, - ownCurrentState: { currentPage, pageSize }, + ownCurrentState = {}, hooks: { onAddFilter: onChangeFilter, setDataMask = () => {} }, } = chartProps; @@ -193,14 +193,8 @@ export default function transformProps(chartProps: TableChartProps): TableChartT } = formData; const [metrics, percentMetrics, columns] = processColumns(chartProps); - let data = queriesData?.[0]?.data; - let showNextButton = false; - // We do request +1 items for BE pagination to know if how `next` button - if (serverPagination && data.length === (pageSize ?? serverPageLength) + 1) { - data.pop(); - showNextButton = true; - } - data = processDataRecords(data, columns); + const data = processDataRecords(queriesData?.[0]?.data, columns); + const rowCount = queriesData?.[1]?.data?.[0]?.rowcount as number; return { height, @@ -211,14 +205,14 @@ export default function transformProps(chartProps: TableChartProps): TableChartT serverPagination, metrics, percentMetrics, - currentPage, + ownCurrentState, setDataMask, alignPositiveNegative, colorPositiveNegative, showCellBars, sortDesc, includeSearch, - showNextButton, + rowCount, pageSize: serverPagination ? serverPageLength : getPageSize(pageLength, data.length, columns.length), @@ -226,4 +220,6 @@ export default function transformProps(chartProps: TableChartProps): TableChartT emitFilter: tableFilter === true, onChangeFilter, }; -} +}; + +export default transformProps; diff --git a/plugins/plugin-chart-table/src/types.ts b/plugins/plugin-chart-table/src/types.ts index 69465334cb..cda899c500 100644 --- a/plugins/plugin-chart-table/src/types.ts +++ b/plugins/plugin-chart-table/src/types.ts @@ -74,6 +74,8 @@ export interface TableChartProps extends ChartProps { pageSize?: number; currentPage?: number; }; + cachedChanges: any; + setCachedChanges: Function; rawFormData: TableChartFormData; queriesData: ChartDataResponseResult[]; } @@ -81,9 +83,9 @@ export interface TableChartProps extends ChartProps { export interface TableChartTransformedProps { height: number; width: number; - showNextButton: boolean; + rowCount?: number; serverPagination: boolean; - currentPage?: number; + ownCurrentState: { pageSize?: number; currentPage?: number }; setDataMask: SetDataMaskHook; isRawRecords?: boolean; data: D[]; From 884527de527864a4cdfb5c086709b9add737a36d Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Tue, 2 Mar 2021 16:20:48 +0200 Subject: [PATCH 02/13] test: update tests --- plugins/plugin-chart-table/test/buildQuery.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/plugin-chart-table/test/buildQuery.test.ts b/plugins/plugin-chart-table/test/buildQuery.test.ts index e10c1bbe71..015cc0a9e5 100644 --- a/plugins/plugin-chart-table/test/buildQuery.test.ts +++ b/plugins/plugin-chart-table/test/buildQuery.test.ts @@ -17,7 +17,7 @@ * under the License. */ import { QueryMode } from '@superset-ui/core'; -import buildQuery from '../src/buildQuery'; +import buildQueryCached from '../src/buildQuery'; import { TableChartFormData } from '../src/types'; const basicFormData: TableChartFormData = { @@ -28,7 +28,7 @@ const basicFormData: TableChartFormData = { describe('plugin-chart-table', () => { describe('buildQuery', () => { it('should add post-processing in aggregate mode', () => { - const query = buildQuery({ + const query = buildQueryCached()({ ...basicFormData, query_mode: QueryMode.aggregate, metrics: ['aaa'], @@ -47,7 +47,7 @@ describe('plugin-chart-table', () => { }); it('should not add post-processing when there is not percent metric', () => { - const query = buildQuery({ + const query = buildQueryCached()({ ...basicFormData, query_mode: QueryMode.aggregate, metrics: ['aaa'], @@ -58,7 +58,7 @@ describe('plugin-chart-table', () => { }); it('should not add post-processing in raw records mode', () => { - const query = buildQuery({ + const query = buildQueryCached()({ ...basicFormData, query_mode: QueryMode.raw, metrics: ['aaa'], From 8da67cb81b2f2bcac258fc7e915c3b773cc8d4e7 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 3 Mar 2021 09:47:49 +0200 Subject: [PATCH 03/13] refactor: fix table CR notes --- .../src/chart/types/TransformFunction.ts | 7 ++-- .../src/query/buildQueryContext.ts | 16 ++++++---- .../superset-ui-core/src/query/types/Query.ts | 2 -- .../src/DataTable/DataTable.tsx | 22 ++++++------- plugins/plugin-chart-table/src/TableChart.tsx | 13 +++++--- plugins/plugin-chart-table/src/buildQuery.ts | 25 +++++++++------ plugins/plugin-chart-table/src/consts.ts | 32 +++++++++++++++++++ .../plugin-chart-table/src/controlPanel.tsx | 11 +------ .../plugin-chart-table/src/transformProps.ts | 4 +-- plugins/plugin-chart-table/src/types.ts | 9 +----- 10 files changed, 86 insertions(+), 55 deletions(-) create mode 100644 plugins/plugin-chart-table/src/consts.ts diff --git a/packages/superset-ui-core/src/chart/types/TransformFunction.ts b/packages/superset-ui-core/src/chart/types/TransformFunction.ts index 94e197fa1b..de6495fe07 100644 --- a/packages/superset-ui-core/src/chart/types/TransformFunction.ts +++ b/packages/superset-ui-core/src/chart/types/TransformFunction.ts @@ -1,4 +1,4 @@ -import { QueryFormData, QueryContext } from '../..'; +import { QueryFormData, QueryContext, SetDataMaskHook } from '../..'; import ChartProps from '../models/ChartProps'; import { PlainObject } from './Base'; @@ -10,6 +10,9 @@ export type PreTransformProps = TransformFunction; export type TransformProps = TransformFunction; export type PostTransformProps = TransformFunction; -export type BuildQueryFunction = (formData: T) => QueryContext; +export type BuildQueryFunction = ( + formData: T, + options?: { hooks?: { setDataMask?: SetDataMaskHook } }, +) => QueryContext; export default {}; diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index e7a817c138..bc21c33002 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -2,30 +2,34 @@ import buildQueryObject from './buildQueryObject'; import DatasourceKey from './DatasourceKey'; import { QueryFieldAliases, QueryFormData } from './types/QueryFormData'; import { QueryContext, QueryObject } from './types/Query'; +import { SetDataMaskHook } from '../chart'; -const WRAP_IN_ARRAY = (baseQueryObject: QueryObject) => [baseQueryObject]; +const WRAP_IN_ARRAY = (baseQueryObject: QueryObject, hooks: { setDataMask: SetDataMaskHook }) => [ + baseQueryObject, +]; export type BuildFinalQuerieObjects = (baseQueryObject: QueryObject) => QueryObject[]; -const emptyFunction = () => {}; - export default function buildQueryContext( formData: QueryFormData, options?: | { buildQuery?: BuildFinalQuerieObjects; queryFields?: QueryFieldAliases; + hooks?: { setDataMask: SetDataMaskHook }; } | BuildFinalQuerieObjects, ): QueryContext { - const { queryFields, buildQuery = WRAP_IN_ARRAY } = + const { queryFields, buildQuery = WRAP_IN_ARRAY, hooks = {} } = typeof options === 'function' ? { buildQuery: options, queryFields: {} } : options || {}; return { datasource: new DatasourceKey(formData.datasource).toObject(), force: formData.force || false, - queries: buildQuery(buildQueryObject(formData, queryFields)), + queries: buildQuery(buildQueryObject(formData, queryFields), { + setDataMask: () => {}, + ...hooks, + }), result_format: formData.result_format || 'json', result_type: formData.result_type || 'full', - setDataMask: formData.setDataMask ?? emptyFunction, }; } diff --git a/packages/superset-ui-core/src/query/types/Query.ts b/packages/superset-ui-core/src/query/types/Query.ts index 36c2ae242a..6e1b4543fa 100644 --- a/packages/superset-ui-core/src/query/types/Query.ts +++ b/packages/superset-ui-core/src/query/types/Query.ts @@ -24,7 +24,6 @@ import { AnnotationLayer } from './AnnotationLayer'; import { QueryFields, QueryFormMetric } from './QueryFormData'; import { Maybe } from '../../types'; import { PostProcessingRule } from './PostProcessing'; -import { SetDataMaskHook } from '../../chart'; export type QueryObjectFilterClause = { col: string; @@ -135,7 +134,6 @@ export interface QueryContext { /** Response format */ result_format: string; queries: QueryObject[]; - setDataMask: SetDataMaskHook; } export default {}; diff --git a/plugins/plugin-chart-table/src/DataTable/DataTable.tsx b/plugins/plugin-chart-table/src/DataTable/DataTable.tsx index e7bef4c51b..0048a4a4df 100644 --- a/plugins/plugin-chart-table/src/DataTable/DataTable.tsx +++ b/plugins/plugin-chart-table/src/DataTable/DataTable.tsx @@ -36,13 +36,11 @@ import { Row, } from 'react-table'; import { matchSorter, rankings } from 'match-sorter'; -import { SetDataMaskHook } from '@superset-ui/core'; import GlobalFilter, { GlobalFilterProps } from './components/GlobalFilter'; import SelectPageSize, { SelectPageSizeProps, SizeOption } from './components/SelectPageSize'; import SimplePagination from './components/Pagination'; import useSticky from './hooks/useSticky'; -import { updateExternalFormData } from './utils/externalAPIs'; -import { PAGE_SIZE_OPTIONS } from '../controlPanel'; +import { PAGE_SIZE_OPTIONS } from '../consts'; export interface DataTableProps extends TableOptions { tableClassName?: string; @@ -54,8 +52,8 @@ export interface DataTableProps extends TableOptions { width?: string | number; height?: string | number; serverPagination?: boolean; - setDataMask: SetDataMaskHook; - ownCurrentState: { pageSize?: number; currentPage?: number }; + onServerPaginationChange: (pageNumber: number, pageSize: number) => void; + serverPaginationData: { pageSize?: number; currentPage?: number }; pageSize?: number; noResults?: string | ((filterString: string) => ReactNode); sticky?: boolean; @@ -72,7 +70,7 @@ export default function DataTable({ tableClassName, columns, data, - ownCurrentState, + serverPaginationData, width: initialWidth = '100%', height: initialHeight = 300, pageSize: initialPageSize = 0, @@ -81,7 +79,7 @@ export default function DataTable({ maxPageItemCount = 9, sticky: doSticky, searchInput = true, - setDataMask, + onServerPaginationChange, rowCount, selectPageSize, noResults: noResultsText = 'No data found', @@ -136,7 +134,7 @@ export default function DataTable({ hasGlobalControl, paginationRef, resultsSize, - JSON.stringify(ownCurrentState), + JSON.stringify(serverPaginationData), ]); const defaultGlobalFilter: FilterType = useCallback( @@ -178,7 +176,7 @@ export default function DataTable({ // make setPageSize accept 0 const setPageSize = (size: number) => { if (serverPagination) { - updateExternalFormData(setDataMask, 0, size); + onServerPaginationChange(0, size); } // keep the original size if data is empty if (size || resultsSize !== 0) { @@ -250,7 +248,7 @@ export default function DataTable({ let resultCurrentPage = pageIndex; let resultOnPageChange: (page: number) => void = gotoPage; if (serverPagination) { - const serverPageSize = ownCurrentState.pageSize ?? initialPageSize; + const serverPageSize = serverPaginationData.pageSize ?? initialPageSize; resultPageCount = Math.ceil(rowCount / serverPageSize); if (!Number.isFinite(resultPageCount)) { resultPageCount = 0; @@ -262,9 +260,9 @@ export default function DataTable({ if (foundPageSizeIndex === -1) { resultCurrentPageSize = 0; } - resultCurrentPage = ownCurrentState.currentPage ?? 0; + resultCurrentPage = serverPaginationData.currentPage ?? 0; resultOnPageChange = (pageNumber: number) => - updateExternalFormData(setDataMask, pageNumber, serverPageSize); + onServerPaginationChange(pageNumber, serverPageSize); } return (
    diff --git a/plugins/plugin-chart-table/src/TableChart.tsx b/plugins/plugin-chart-table/src/TableChart.tsx index e57ff8aa3e..08bf8b524c 100644 --- a/plugins/plugin-chart-table/src/TableChart.tsx +++ b/plugins/plugin-chart-table/src/TableChart.tsx @@ -32,7 +32,8 @@ import DataTable, { import Styles from './Styles'; import formatValue from './utils/formatValue'; -import { PAGE_SIZE_OPTIONS } from './controlPanel'; +import { PAGE_SIZE_OPTIONS } from './consts'; +import { updateExternalFormData } from './DataTable/utils/externalAPIs'; type ValueRange = [number, number]; @@ -154,7 +155,7 @@ export default function TableChart( includeSearch = false, pageSize = 0, serverPagination = false, - ownCurrentState, + serverPaginationData, setDataMask, showCellBars = true, emitFilter = false, @@ -285,6 +286,10 @@ export default function TableChart( const columns = useMemo(() => columnsMeta.map(getColumnConfigs), [columnsMeta, getColumnConfigs]); + const handleServerPaginationChange = (pageNumber: number, pageSize: number) => { + updateExternalFormData(setDataMask, pageNumber, pageSize); + }; + return ( @@ -293,12 +298,12 @@ export default function TableChart( rowCount={rowCount} tableClassName="table table-striped table-condensed" pageSize={pageSize} - ownCurrentState={ownCurrentState} + serverPaginationData={serverPaginationData} pageSizeOptions={pageSizeOptions} width={width} height={height} serverPagination={serverPagination} - setDataMask={setDataMask} + onServerPaginationChange={handleServerPaginationChange} // 9 page items in > 340px works well even for 100+ pages maxPageItemCount={width > 340 ? 9 : 7} noResults={(filter: string) => t(filter ? 'No matching records found' : 'No records found')} diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index 1cf5bdfde2..8bfd5ed3b3 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -23,6 +23,7 @@ import { removeDuplicates, ensureIsArray, QueryObject, + SetDataMaskHook, } from '@superset-ui/core'; import { PostProcessingRule } from '@superset-ui/core/src/query/types/PostProcessing'; import { TableChartFormData } from './types'; @@ -44,7 +45,13 @@ export function getQueryMode(formData: TableChartFormData) { return hasRawColumns ? QueryMode.raw : QueryMode.aggregate; } -function buildQuery(formData: TableChartFormData) { +type Hooks = { + setDataMask: SetDataMaskHook; + cachedChanges: any; + setCachedChanges: (newChanges: any) => void; +}; + +function buildQuery(formData: TableChartFormData, hooks: Hooks) { const { percent_metrics: percentMetrics, order_desc: orderDesc = false } = formData; const queryMode = getQueryMode(formData); const sortByMetric = ensureIsArray(formData.timeseries_limit_metric)[0]; @@ -103,15 +110,15 @@ function buildQuery(formData: TableChartFormData) { if ( formData.server_pagination && - formData?.cachedChanges?.[formData.slice_id] && - JSON.stringify(formData?.cachedChanges?.[formData.slice_id]) !== + hooks?.cachedChanges?.[formData.slice_id] && + JSON.stringify(hooks?.cachedChanges?.[formData.slice_id]) !== JSON.stringify(queryObject.filters) ) { queryObject = { ...queryObject, row_offset: 0 }; - updateExternalFormData(formData.setDataMask, 0, queryObject.row_limit ?? 0); + updateExternalFormData(hooks.setDataMask, 0, queryObject.row_limit ?? 0); } // Because we use same buildQuery for all table on the page we need split them by id - formData.setCachedChanges({ [formData.slice_id]: queryObject.filters }); + hooks.setCachedChanges({ [formData.slice_id]: queryObject.filters }); if (formData.server_pagination) { return [ @@ -127,11 +134,11 @@ function buildQuery(formData: TableChartFormData) { // external filter changed const cachedBuildQuery = () => { let cachedChanges: any = {}; - const setCachedChanges = (_cachedChanges: any) => { - cachedChanges = { ...cachedChanges, ..._cachedChanges }; + const setCachedChanges = (newChanges: any) => { + cachedChanges = { ...cachedChanges, ...newChanges }; }; - return (formData: TableChartFormData) => - buildQuery({ ...formData, cachedChanges, setCachedChanges }); + return (formData: TableChartFormData, { hooks }: { hooks: { setDataMask: SetDataMaskHook } }) => + buildQuery({ ...formData }, { ...hooks, cachedChanges, setCachedChanges }); }; export default cachedBuildQuery; diff --git a/plugins/plugin-chart-table/src/consts.ts b/plugins/plugin-chart-table/src/consts.ts new file mode 100644 index 0000000000..e370c4b029 --- /dev/null +++ b/plugins/plugin-chart-table/src/consts.ts @@ -0,0 +1,32 @@ +/** + * 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 { formatSelectOptions } from '@superset-ui/chart-controls'; +import { addLocaleData, t } from '@superset-ui/core'; +import i18n from './i18n'; + +addLocaleData(i18n); + +export const PAGE_SIZE_OPTIONS = formatSelectOptions([ + [0, t('page_size.all')], + 10, + 20, + 50, + 100, + 200, +]); diff --git a/plugins/plugin-chart-table/src/controlPanel.tsx b/plugins/plugin-chart-table/src/controlPanel.tsx index bd2eb9af4d..a5d55ee783 100644 --- a/plugins/plugin-chart-table/src/controlPanel.tsx +++ b/plugins/plugin-chart-table/src/controlPanel.tsx @@ -27,7 +27,6 @@ import { QueryFormColumn, } from '@superset-ui/core'; import { - formatSelectOptions, D3_TIME_FORMAT_OPTIONS, ControlConfig, ColumnOption, @@ -39,18 +38,10 @@ import { } from '@superset-ui/chart-controls'; import i18n from './i18n'; +import { PAGE_SIZE_OPTIONS } from './consts'; addLocaleData(i18n); -export const PAGE_SIZE_OPTIONS = formatSelectOptions([ - [0, t('page_size.all')], - 10, - 20, - 50, - 100, - 200, -]); - const QueryModeLabel = { [QueryMode.aggregate]: t('Aggregate'), [QueryMode.raw]: t('Raw Records'), diff --git a/plugins/plugin-chart-table/src/transformProps.ts b/plugins/plugin-chart-table/src/transformProps.ts index 2c4cb58c43..360300fb89 100644 --- a/plugins/plugin-chart-table/src/transformProps.ts +++ b/plugins/plugin-chart-table/src/transformProps.ts @@ -175,7 +175,7 @@ const transformProps = (chartProps: TableChartProps): TableChartTransformedProps rawFormData: formData, queriesData, initialValues: filters = {}, - ownCurrentState = {}, + ownCurrentState: serverPaginationData = {}, hooks: { onAddFilter: onChangeFilter, setDataMask = () => {} }, } = chartProps; @@ -205,7 +205,7 @@ const transformProps = (chartProps: TableChartProps): TableChartTransformedProps serverPagination, metrics, percentMetrics, - ownCurrentState, + serverPaginationData, setDataMask, alignPositiveNegative, colorPositiveNegative, diff --git a/plugins/plugin-chart-table/src/types.ts b/plugins/plugin-chart-table/src/types.ts index cda899c500..8b47a3c15c 100644 --- a/plugins/plugin-chart-table/src/types.ts +++ b/plugins/plugin-chart-table/src/types.ts @@ -74,8 +74,6 @@ export interface TableChartProps extends ChartProps { pageSize?: number; currentPage?: number; }; - cachedChanges: any; - setCachedChanges: Function; rawFormData: TableChartFormData; queriesData: ChartDataResponseResult[]; } @@ -85,7 +83,7 @@ export interface TableChartTransformedProps { width: number; rowCount?: number; serverPagination: boolean; - ownCurrentState: { pageSize?: number; currentPage?: number }; + serverPaginationData: { pageSize?: number; currentPage?: number }; setDataMask: SetDataMaskHook; isRawRecords?: boolean; data: D[]; @@ -106,9 +104,4 @@ export interface TableChartTransformedProps { onChangeFilter?: ChartProps['hooks']['onAddFilter']; } -export enum ServerPage { - NEXT = 'next', - PREVIOUS = 'previous', -} - export default {}; From ac622d849e1dcc66822f020a6794cda7e0b4b231 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 3 Mar 2021 10:47:33 +0200 Subject: [PATCH 04/13] lint: fix TS --- packages/superset-ui-core/src/query/buildQueryContext.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index bc21c33002..62043f669c 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -4,7 +4,7 @@ import { QueryFieldAliases, QueryFormData } from './types/QueryFormData'; import { QueryContext, QueryObject } from './types/Query'; import { SetDataMaskHook } from '../chart'; -const WRAP_IN_ARRAY = (baseQueryObject: QueryObject, hooks: { setDataMask: SetDataMaskHook }) => [ +const WRAP_IN_ARRAY = (baseQueryObject: QueryObject, hooks?: { setDataMask: SetDataMaskHook }) => [ baseQueryObject, ]; @@ -16,7 +16,7 @@ export default function buildQueryContext( | { buildQuery?: BuildFinalQuerieObjects; queryFields?: QueryFieldAliases; - hooks?: { setDataMask: SetDataMaskHook }; + hooks?: { setDataMask?: SetDataMaskHook }; } | BuildFinalQuerieObjects, ): QueryContext { From 42393cbf07d2d53c43aa447c028ed8ff3c36f404 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 3 Mar 2021 11:30:01 +0200 Subject: [PATCH 05/13] lint: fix TS --- .../superset-ui-core/src/chart/types/TransformFunction.ts | 2 +- packages/superset-ui-core/src/query/buildQueryContext.ts | 8 ++++---- plugins/plugin-chart-table/src/index.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/superset-ui-core/src/chart/types/TransformFunction.ts b/packages/superset-ui-core/src/chart/types/TransformFunction.ts index de6495fe07..5ff11fae6e 100644 --- a/packages/superset-ui-core/src/chart/types/TransformFunction.ts +++ b/packages/superset-ui-core/src/chart/types/TransformFunction.ts @@ -12,7 +12,7 @@ export type PostTransformProps = TransformFunction; export type BuildQueryFunction = ( formData: T, - options?: { hooks?: { setDataMask?: SetDataMaskHook } }, + options: { hooks: { setDataMask: SetDataMaskHook; [key: string]: any } }, ) => QueryContext; export default {}; diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index 62043f669c..2c6bed1d19 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -4,21 +4,21 @@ import { QueryFieldAliases, QueryFormData } from './types/QueryFormData'; import { QueryContext, QueryObject } from './types/Query'; import { SetDataMaskHook } from '../chart'; -const WRAP_IN_ARRAY = (baseQueryObject: QueryObject, hooks?: { setDataMask: SetDataMaskHook }) => [ +const WRAP_IN_ARRAY = (baseQueryObject: QueryObject, hooks?: { setDataMask?: SetDataMaskHook }) => [ baseQueryObject, ]; -export type BuildFinalQuerieObjects = (baseQueryObject: QueryObject) => QueryObject[]; +export type BuildFinalQueryObjects = (baseQueryObject: QueryObject) => QueryObject[]; export default function buildQueryContext( formData: QueryFormData, options?: | { - buildQuery?: BuildFinalQuerieObjects; + buildQuery?: BuildFinalQueryObjects; queryFields?: QueryFieldAliases; hooks?: { setDataMask?: SetDataMaskHook }; } - | BuildFinalQuerieObjects, + | BuildFinalQueryObjects, ): QueryContext { const { queryFields, buildQuery = WRAP_IN_ARRAY, hooks = {} } = typeof options === 'function' ? { buildQuery: options, queryFields: {} } : options || {}; diff --git a/plugins/plugin-chart-table/src/index.ts b/plugins/plugin-chart-table/src/index.ts index 116484201e..898717e477 100644 --- a/plugins/plugin-chart-table/src/index.ts +++ b/plugins/plugin-chart-table/src/index.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { t, ChartMetadata, ChartPlugin } from '@superset-ui/core'; +import { t, ChartMetadata, ChartPlugin, BuildQueryFunction } from '@superset-ui/core'; import transformProps from './transformProps'; import thumbnail from './images/thumbnail.png'; import controlPanel from './controlPanel'; @@ -41,7 +41,7 @@ export default class TableChartPlugin extends ChartPlugin, }); } } From 11451060b2a510ef5c8c13345a6d79dd343ec22f Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 3 Mar 2021 12:14:22 +0200 Subject: [PATCH 06/13] lint: fix TS --- .../registries/ChartBuildQueryRegistrySingleton.ts | 9 +++++++-- .../superset-ui-core/src/query/buildQueryContext.ts | 13 ++++++++----- plugins/plugin-chart-table/src/buildQuery.ts | 5 +++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 3e7da4c38e..9876d56ac1 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -1,7 +1,12 @@ -import { Registry, makeSingleton, OverwritePolicy, QueryContext } from '../..'; +import { Registry, makeSingleton, OverwritePolicy, QueryContext, SetDataMaskHook } from '../..'; // Ideally this would be -type BuildQuery = (formData: any) => QueryContext; +type BuildQuery = ( + formData: any, + { + hooks: { setDataMask }, + }: { hooks: { setDataMask: SetDataMaskHook; [key: string]: any } }, +) => QueryContext; class ChartBuildQueryRegistry extends Registry { constructor() { diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index 2c6bed1d19..c7e869587a 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -4,9 +4,10 @@ import { QueryFieldAliases, QueryFormData } from './types/QueryFormData'; import { QueryContext, QueryObject } from './types/Query'; import { SetDataMaskHook } from '../chart'; -const WRAP_IN_ARRAY = (baseQueryObject: QueryObject, hooks?: { setDataMask?: SetDataMaskHook }) => [ - baseQueryObject, -]; +const WRAP_IN_ARRAY = ( + baseQueryObject: QueryObject, + options?: { hooks?: { setDataMask?: SetDataMaskHook } }, +) => [baseQueryObject]; export type BuildFinalQueryObjects = (baseQueryObject: QueryObject) => QueryObject[]; @@ -26,8 +27,10 @@ export default function buildQueryContext( datasource: new DatasourceKey(formData.datasource).toObject(), force: formData.force || false, queries: buildQuery(buildQueryObject(formData, queryFields), { - setDataMask: () => {}, - ...hooks, + hooks: { + setDataMask: () => {}, + ...hooks, + }, }), result_format: formData.result_format || 'json', result_type: formData.result_type || 'full', diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index 8bfd5ed3b3..b1a3759978 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -51,7 +51,7 @@ type Hooks = { setCachedChanges: (newChanges: any) => void; }; -function buildQuery(formData: TableChartFormData, hooks: Hooks) { +function buildQuery(formData: TableChartFormData, { hooks }: { hooks: Hooks }) { const { percent_metrics: percentMetrics, order_desc: orderDesc = false } = formData; const queryMode = getQueryMode(formData); const sortByMetric = ensureIsArray(formData.timeseries_limit_metric)[0]; @@ -137,8 +137,9 @@ const cachedBuildQuery = () => { const setCachedChanges = (newChanges: any) => { cachedChanges = { ...cachedChanges, ...newChanges }; }; + return (formData: TableChartFormData, { hooks }: { hooks: { setDataMask: SetDataMaskHook } }) => - buildQuery({ ...formData }, { ...hooks, cachedChanges, setCachedChanges }); + buildQuery({ ...formData }, { hooks: { ...hooks, cachedChanges, setCachedChanges } }); }; export default cachedBuildQuery; From 8e2521a2e2b36f3fb0d30ca36da2c020fc6a5a0e Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 3 Mar 2021 12:33:54 +0200 Subject: [PATCH 07/13] lint: fix TS --- .../src/chart/registries/ChartBuildQueryRegistrySingleton.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 9876d56ac1..705758d1a4 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -5,7 +5,7 @@ type BuildQuery = ( formData: any, { hooks: { setDataMask }, - }: { hooks: { setDataMask: SetDataMaskHook; [key: string]: any } }, + }?: { hooks: { setDataMask: SetDataMaskHook; [key: string]: any } }, ) => QueryContext; class ChartBuildQueryRegistry extends Registry { From 56c5ef6c72d05364a62fbf2d692e82581e4aa6e9 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Wed, 3 Mar 2021 14:18:36 +0200 Subject: [PATCH 08/13] lint: fix TS --- .../registries/ChartBuildQueryRegistrySingleton.ts | 4 +--- .../src/chart/types/TransformFunction.ts | 2 +- .../src/DataTable/utils/externalAPIs.ts | 2 +- plugins/plugin-chart-table/src/buildQuery.ts | 10 ++++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 705758d1a4..ea67ec309c 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -3,9 +3,7 @@ import { Registry, makeSingleton, OverwritePolicy, QueryContext, SetDataMaskHook // Ideally this would be type BuildQuery = ( formData: any, - { - hooks: { setDataMask }, - }?: { hooks: { setDataMask: SetDataMaskHook; [key: string]: any } }, + options?: { hooks?: { [key: string]: any; setDataMask?: SetDataMaskHook } }, ) => QueryContext; class ChartBuildQueryRegistry extends Registry { diff --git a/packages/superset-ui-core/src/chart/types/TransformFunction.ts b/packages/superset-ui-core/src/chart/types/TransformFunction.ts index 5ff11fae6e..3e80681484 100644 --- a/packages/superset-ui-core/src/chart/types/TransformFunction.ts +++ b/packages/superset-ui-core/src/chart/types/TransformFunction.ts @@ -12,7 +12,7 @@ export type PostTransformProps = TransformFunction; export type BuildQueryFunction = ( formData: T, - options: { hooks: { setDataMask: SetDataMaskHook; [key: string]: any } }, + options?: { hooks?: { [key: string]: any; setDataMask?: SetDataMaskHook } }, ) => QueryContext; export default {}; diff --git a/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts b/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts index dbd4cf49a7..ab910621c1 100644 --- a/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts +++ b/plugins/plugin-chart-table/src/DataTable/utils/externalAPIs.ts @@ -20,7 +20,7 @@ import { SetDataMaskHook } from '@superset-ui/core'; export const updateExternalFormData = ( - setDataMask: SetDataMaskHook, + setDataMask: SetDataMaskHook = () => {}, pageNumber: number, pageSize: number, ) => diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index b1a3759978..c0680904c4 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -46,7 +46,7 @@ export function getQueryMode(formData: TableChartFormData) { } type Hooks = { - setDataMask: SetDataMaskHook; + setDataMask?: SetDataMaskHook; cachedChanges: any; setCachedChanges: (newChanges: any) => void; }; @@ -115,7 +115,7 @@ function buildQuery(formData: TableChartFormData, { hooks }: { hooks: Hooks }) { JSON.stringify(queryObject.filters) ) { queryObject = { ...queryObject, row_offset: 0 }; - updateExternalFormData(hooks.setDataMask, 0, queryObject.row_limit ?? 0); + updateExternalFormData(hooks?.setDataMask, 0, queryObject.row_limit ?? 0); } // Because we use same buildQuery for all table on the page we need split them by id hooks.setCachedChanges({ [formData.slice_id]: queryObject.filters }); @@ -138,8 +138,10 @@ const cachedBuildQuery = () => { cachedChanges = { ...cachedChanges, ...newChanges }; }; - return (formData: TableChartFormData, { hooks }: { hooks: { setDataMask: SetDataMaskHook } }) => - buildQuery({ ...formData }, { hooks: { ...hooks, cachedChanges, setCachedChanges } }); + return ( + formData: TableChartFormData, + { hooks }: { hooks?: { setDataMask?: SetDataMaskHook } } = { hooks: {} }, + ) => buildQuery({ ...formData }, { hooks: { ...hooks, cachedChanges, setCachedChanges } }); }; export default cachedBuildQuery; From 8d1bf27cf6f1ab987a55d22e0158886d54541038 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 4 Mar 2021 08:50:40 +0200 Subject: [PATCH 09/13] lint: update types --- .../registries/ChartBuildQueryRegistrySingleton.ts | 2 +- plugins/plugin-chart-table/src/buildQuery.ts | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index ea67ec309c..3241dc09c4 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -1,7 +1,7 @@ import { Registry, makeSingleton, OverwritePolicy, QueryContext, SetDataMaskHook } from '../..'; // Ideally this would be -type BuildQuery = ( +export type BuildQuery = ( formData: any, options?: { hooks?: { [key: string]: any; setDataMask?: SetDataMaskHook } }, ) => QueryContext; diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index c0680904c4..514c0ebcc7 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -26,6 +26,7 @@ import { SetDataMaskHook, } from '@superset-ui/core'; import { PostProcessingRule } from '@superset-ui/core/src/query/types/PostProcessing'; +import { BuildQuery } from '@superset-ui/core/src/chart/registries/ChartBuildQueryRegistrySingleton'; import { TableChartFormData } from './types'; import { updateExternalFormData } from './DataTable/utils/externalAPIs'; @@ -45,13 +46,7 @@ export function getQueryMode(formData: TableChartFormData) { return hasRawColumns ? QueryMode.raw : QueryMode.aggregate; } -type Hooks = { - setDataMask?: SetDataMaskHook; - cachedChanges: any; - setCachedChanges: (newChanges: any) => void; -}; - -function buildQuery(formData: TableChartFormData, { hooks }: { hooks: Hooks }) { +const buildQuery: BuildQuery = (formData: TableChartFormData, { hooks } = { hooks: {} }) => { const { percent_metrics: percentMetrics, order_desc: orderDesc = false } = formData; const queryMode = getQueryMode(formData); const sortByMetric = ensureIsArray(formData.timeseries_limit_metric)[0]; @@ -118,7 +113,7 @@ function buildQuery(formData: TableChartFormData, { hooks }: { hooks: Hooks }) { updateExternalFormData(hooks?.setDataMask, 0, queryObject.row_limit ?? 0); } // Because we use same buildQuery for all table on the page we need split them by id - hooks.setCachedChanges({ [formData.slice_id]: queryObject.filters }); + hooks?.setCachedChanges({ [formData.slice_id]: queryObject.filters }); if (formData.server_pagination) { return [ @@ -128,7 +123,7 @@ function buildQuery(formData: TableChartFormData, { hooks }: { hooks: Hooks }) { } return [queryObject]; }); -} +}; // Use this closure to cache changing of external filters, if we have server pagination we need reset page to 0, after // external filter changed From 92d3dce7708698d3c001dd30ab1cc0663c725bd3 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 4 Mar 2021 08:52:12 +0200 Subject: [PATCH 10/13] lint: update types --- .../chart/registries/ChartBuildQueryRegistrySingleton.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 3241dc09c4..0c6f39100f 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -3,7 +3,14 @@ import { Registry, makeSingleton, OverwritePolicy, QueryContext, SetDataMaskHook // Ideally this would be export type BuildQuery = ( formData: any, - options?: { hooks?: { [key: string]: any; setDataMask?: SetDataMaskHook } }, + options?: { + hooks?: { + [key: string]: any; + setDataMask?: SetDataMaskHook; + cachedChanges?: any; + setCachedChanges?: (newChanges: any) => void; + }; + }, ) => QueryContext; class ChartBuildQueryRegistry extends Registry { From 61440414333b9f400d94a4b24e8f280fdb478e28 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 4 Mar 2021 09:15:09 +0200 Subject: [PATCH 11/13] lint: fix TS --- .../ChartBuildQueryRegistrySingleton.ts | 8 +++---- .../src/chart/types/TransformFunction.ts | 9 +++++++- .../src/query/buildQueryContext.ts | 12 ++++++++-- plugins/plugin-chart-table/src/buildQuery.ts | 22 +++++++++---------- 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 0c6f39100f..4c47fe85ed 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -1,14 +1,14 @@ import { Registry, makeSingleton, OverwritePolicy, QueryContext, SetDataMaskHook } from '../..'; // Ideally this would be -export type BuildQuery = ( - formData: any, +export type BuildQuery = ( + formData: T, options?: { hooks?: { [key: string]: any; - setDataMask?: SetDataMaskHook; + setDataMask: SetDataMaskHook; cachedChanges?: any; - setCachedChanges?: (newChanges: any) => void; + setCachedChanges: (newChanges: any) => void; }; }, ) => QueryContext; diff --git a/packages/superset-ui-core/src/chart/types/TransformFunction.ts b/packages/superset-ui-core/src/chart/types/TransformFunction.ts index 3e80681484..39237e8bc8 100644 --- a/packages/superset-ui-core/src/chart/types/TransformFunction.ts +++ b/packages/superset-ui-core/src/chart/types/TransformFunction.ts @@ -12,7 +12,14 @@ export type PostTransformProps = TransformFunction; export type BuildQueryFunction = ( formData: T, - options?: { hooks?: { [key: string]: any; setDataMask?: SetDataMaskHook } }, + options?: { + hooks?: { + [key: string]: any; + setDataMask: SetDataMaskHook; + cachedChanges?: any; + setCachedChanges: (newChanges: any) => void; + }; + }, ) => QueryContext; export default {}; diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index c7e869587a..4465dc42c9 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -6,7 +6,14 @@ import { SetDataMaskHook } from '../chart'; const WRAP_IN_ARRAY = ( baseQueryObject: QueryObject, - options?: { hooks?: { setDataMask?: SetDataMaskHook } }, + options?: { + hooks?: { + [key: string]: any; + setDataMask: SetDataMaskHook; + cachedChanges?: any; + setCachedChanges: (newChanges: any) => void; + }; + }, ) => [baseQueryObject]; export type BuildFinalQueryObjects = (baseQueryObject: QueryObject) => QueryObject[]; @@ -17,7 +24,7 @@ export default function buildQueryContext( | { buildQuery?: BuildFinalQueryObjects; queryFields?: QueryFieldAliases; - hooks?: { setDataMask?: SetDataMaskHook }; + hooks?: { setDataMask: SetDataMaskHook }; } | BuildFinalQueryObjects, ): QueryContext { @@ -29,6 +36,7 @@ export default function buildQueryContext( queries: buildQuery(buildQueryObject(formData, queryFields), { hooks: { setDataMask: () => {}, + setCachedChanges: () => {}, ...hooks, }, }), diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index 514c0ebcc7..0ddc7a243b 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -23,7 +23,6 @@ import { removeDuplicates, ensureIsArray, QueryObject, - SetDataMaskHook, } from '@superset-ui/core'; import { PostProcessingRule } from '@superset-ui/core/src/query/types/PostProcessing'; import { BuildQuery } from '@superset-ui/core/src/chart/registries/ChartBuildQueryRegistrySingleton'; @@ -46,7 +45,7 @@ export function getQueryMode(formData: TableChartFormData) { return hasRawColumns ? QueryMode.raw : QueryMode.aggregate; } -const buildQuery: BuildQuery = (formData: TableChartFormData, { hooks } = { hooks: {} }) => { +const buildQuery: BuildQuery = (formData: TableChartFormData, options) => { const { percent_metrics: percentMetrics, order_desc: orderDesc = false } = formData; const queryMode = getQueryMode(formData); const sortByMetric = ensureIsArray(formData.timeseries_limit_metric)[0]; @@ -105,15 +104,15 @@ const buildQuery: BuildQuery = (formData: TableChartFormData, { hooks } = { hook if ( formData.server_pagination && - hooks?.cachedChanges?.[formData.slice_id] && - JSON.stringify(hooks?.cachedChanges?.[formData.slice_id]) !== + options?.hooks?.cachedChanges?.[formData.slice_id] && + JSON.stringify(options?.hooks?.cachedChanges?.[formData.slice_id]) !== JSON.stringify(queryObject.filters) ) { queryObject = { ...queryObject, row_offset: 0 }; - updateExternalFormData(hooks?.setDataMask, 0, queryObject.row_limit ?? 0); + updateExternalFormData(options?.hooks?.setDataMask, 0, queryObject.row_limit ?? 0); } // Because we use same buildQuery for all table on the page we need split them by id - hooks?.setCachedChanges({ [formData.slice_id]: queryObject.filters }); + options?.hooks?.setCachedChanges({ [formData.slice_id]: queryObject.filters }); if (formData.server_pagination) { return [ @@ -127,16 +126,17 @@ const buildQuery: BuildQuery = (formData: TableChartFormData, { hooks } = { hook // Use this closure to cache changing of external filters, if we have server pagination we need reset page to 0, after // external filter changed -const cachedBuildQuery = () => { +const cachedBuildQuery = (): BuildQuery => { let cachedChanges: any = {}; const setCachedChanges = (newChanges: any) => { cachedChanges = { ...cachedChanges, ...newChanges }; }; - return ( - formData: TableChartFormData, - { hooks }: { hooks?: { setDataMask?: SetDataMaskHook } } = { hooks: {} }, - ) => buildQuery({ ...formData }, { hooks: { ...hooks, cachedChanges, setCachedChanges } }); + return (formData, options) => + buildQuery( + { ...formData }, + { hooks: { ...options?.hooks, setDataMask: () => {}, cachedChanges, setCachedChanges } }, + ); }; export default cachedBuildQuery; From 8c02918df8e598d40003828b8d2b7b4950c7d704 Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 4 Mar 2021 09:58:44 +0200 Subject: [PATCH 12/13] lint: fix TS --- .../src/chart/registries/ChartBuildQueryRegistrySingleton.ts | 1 - packages/superset-ui-core/src/chart/types/TransformFunction.ts | 1 - packages/superset-ui-core/src/query/buildQueryContext.ts | 1 - 3 files changed, 3 deletions(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 4c47fe85ed..6411e4d085 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -5,7 +5,6 @@ export type BuildQuery = ( formData: T, options?: { hooks?: { - [key: string]: any; setDataMask: SetDataMaskHook; cachedChanges?: any; setCachedChanges: (newChanges: any) => void; diff --git a/packages/superset-ui-core/src/chart/types/TransformFunction.ts b/packages/superset-ui-core/src/chart/types/TransformFunction.ts index 39237e8bc8..6abc24e912 100644 --- a/packages/superset-ui-core/src/chart/types/TransformFunction.ts +++ b/packages/superset-ui-core/src/chart/types/TransformFunction.ts @@ -14,7 +14,6 @@ export type BuildQueryFunction = ( formData: T, options?: { hooks?: { - [key: string]: any; setDataMask: SetDataMaskHook; cachedChanges?: any; setCachedChanges: (newChanges: any) => void; diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index 4465dc42c9..28d48502d1 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -8,7 +8,6 @@ const WRAP_IN_ARRAY = ( baseQueryObject: QueryObject, options?: { hooks?: { - [key: string]: any; setDataMask: SetDataMaskHook; cachedChanges?: any; setCachedChanges: (newChanges: any) => void; From 8d3fcd05bb5de80064cf4c764b6805e13169b1fa Mon Sep 17 00:00:00 2001 From: Simcha Shats Date: Thu, 4 Mar 2021 10:15:14 +0200 Subject: [PATCH 13/13] lint: fix TS --- .../registries/ChartBuildQueryRegistrySingleton.ts | 4 +++- .../src/chart/types/TransformFunction.ts | 4 +++- .../superset-ui-core/src/query/buildQueryContext.ts | 5 ++++- plugins/plugin-chart-table/src/buildQuery.ts | 13 ++++++++++--- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts index 6411e4d085..0092de2e3b 100644 --- a/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts +++ b/packages/superset-ui-core/src/chart/registries/ChartBuildQueryRegistrySingleton.ts @@ -4,9 +4,11 @@ import { Registry, makeSingleton, OverwritePolicy, QueryContext, SetDataMaskHook export type BuildQuery = ( formData: T, options?: { + extras?: { + cachedChanges?: any; + }; hooks?: { setDataMask: SetDataMaskHook; - cachedChanges?: any; setCachedChanges: (newChanges: any) => void; }; }, diff --git a/packages/superset-ui-core/src/chart/types/TransformFunction.ts b/packages/superset-ui-core/src/chart/types/TransformFunction.ts index 6abc24e912..ea07000e48 100644 --- a/packages/superset-ui-core/src/chart/types/TransformFunction.ts +++ b/packages/superset-ui-core/src/chart/types/TransformFunction.ts @@ -13,9 +13,11 @@ export type PostTransformProps = TransformFunction; export type BuildQueryFunction = ( formData: T, options?: { + extras?: { + cachedChanges?: any; + }; hooks?: { setDataMask: SetDataMaskHook; - cachedChanges?: any; setCachedChanges: (newChanges: any) => void; }; }, diff --git a/packages/superset-ui-core/src/query/buildQueryContext.ts b/packages/superset-ui-core/src/query/buildQueryContext.ts index 28d48502d1..c4621c2f68 100644 --- a/packages/superset-ui-core/src/query/buildQueryContext.ts +++ b/packages/superset-ui-core/src/query/buildQueryContext.ts @@ -7,9 +7,11 @@ import { SetDataMaskHook } from '../chart'; const WRAP_IN_ARRAY = ( baseQueryObject: QueryObject, options?: { + extras?: { + cachedChanges?: any; + }; hooks?: { setDataMask: SetDataMaskHook; - cachedChanges?: any; setCachedChanges: (newChanges: any) => void; }; }, @@ -33,6 +35,7 @@ export default function buildQueryContext( datasource: new DatasourceKey(formData.datasource).toObject(), force: formData.force || false, queries: buildQuery(buildQueryObject(formData, queryFields), { + extras: {}, hooks: { setDataMask: () => {}, setCachedChanges: () => {}, diff --git a/plugins/plugin-chart-table/src/buildQuery.ts b/plugins/plugin-chart-table/src/buildQuery.ts index 0ddc7a243b..19b264bf64 100644 --- a/plugins/plugin-chart-table/src/buildQuery.ts +++ b/plugins/plugin-chart-table/src/buildQuery.ts @@ -104,8 +104,8 @@ const buildQuery: BuildQuery = (formData: TableChartFormData if ( formData.server_pagination && - options?.hooks?.cachedChanges?.[formData.slice_id] && - JSON.stringify(options?.hooks?.cachedChanges?.[formData.slice_id]) !== + options?.extras?.cachedChanges?.[formData.slice_id] && + JSON.stringify(options?.extras?.cachedChanges?.[formData.slice_id]) !== JSON.stringify(queryObject.filters) ) { queryObject = { ...queryObject, row_offset: 0 }; @@ -135,7 +135,14 @@ const cachedBuildQuery = (): BuildQuery => { return (formData, options) => buildQuery( { ...formData }, - { hooks: { ...options?.hooks, setDataMask: () => {}, cachedChanges, setCachedChanges } }, + { + extras: { cachedChanges }, + hooks: { + ...options?.hooks, + setDataMask: () => {}, + setCachedChanges, + }, + }, ); };