diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx index b1ae8231fe3..61761c0f927 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/components/CustomHeader.tsx @@ -19,9 +19,10 @@ * under the License. */ -import { useRef, useState } from 'react'; +import { useRef, useState, useEffect } from 'react'; import { t } from '@apache-superset/core'; import { ArrowDownOutlined, ArrowUpOutlined } from '@ant-design/icons'; +import { Column } from '@superset-ui/core/components/ThemedAgGridReact'; import FilterIcon from './Filter'; import KebabMenu from './KebabMenu'; import { @@ -29,6 +30,8 @@ import { CustomHeaderParams, SortState, UserProvidedColDef, + FilterInputPosition, + AGGridFilterInstance, } from '../../types'; import CustomPopover from './CustomPopover'; import { @@ -39,6 +42,13 @@ import { MenuContainer, SortIconWrapper, } from '../../styles'; +import { GridApi } from 'ag-grid-community'; +import { + FILTER_POPOVER_OPEN_DELAY, + FILTER_INPUT_POSITIONS, + FILTER_CONDITION_BODY_INDEX, + FILTER_INPUT_SELECTOR, +} from '../../consts'; const getSortIcon = (sortState: SortState[], colId: string | null) => { if (!sortState?.length || !colId) return null; @@ -53,6 +63,43 @@ const getSortIcon = (sortState: SortState[], colId: string | null) => { return null; }; +// Auto-opens filter popover and focuses the correct input after server-side filtering +const autoOpenFilterAndFocus = async ( + column: Column, + api: GridApi, + filterRef: React.RefObject, + setFilterVisible: (visible: boolean) => void, + lastFilteredInputPosition?: FilterInputPosition, +) => { + setFilterVisible(true); + + const filterInstance = (await api.getColumnFilterInstance( + column, + )) as AGGridFilterInstance | null; + const filterEl = filterInstance?.eGui; + + if (!filterEl || !filterRef.current) return; + + filterRef.current.innerHTML = ''; + filterRef.current.appendChild(filterEl); + + if (filterInstance?.eConditionBodies) { + const conditionBodies = filterInstance.eConditionBodies; + const targetIndex = + lastFilteredInputPosition === FILTER_INPUT_POSITIONS.SECOND + ? FILTER_CONDITION_BODY_INDEX.SECOND + : FILTER_CONDITION_BODY_INDEX.FIRST; + const targetBody = conditionBodies[targetIndex]; + + if (targetBody) { + const input = targetBody.querySelector( + FILTER_INPUT_SELECTOR, + ) as HTMLInputElement | null; + input?.focus(); + } + } +}; + const CustomHeader: React.FC = ({ displayName, enableSorting, @@ -61,7 +108,12 @@ const CustomHeader: React.FC = ({ column, api, }) => { - const { initialSortState, onColumnHeaderClicked } = context; + const { + initialSortState, + onColumnHeaderClicked, + lastFilteredColumn, + lastFilteredInputPosition, + } = context; const colId = column?.getColId(); const colDef = column?.getColDef() as CustomColDef; const userColDef = column.getUserProvidedColDef() as UserProvidedColDef; @@ -77,7 +129,6 @@ const CustomHeader: React.FC = ({ const isTimeComparison = !isMain && userColDef?.timeComparisonKey; const sortKey = isMain ? colId.replace('Main', '').trim() : colId; - // Sorting logic const clearSort = () => { onColumnHeaderClicked({ column: { colId: sortKey, sort: null } }); setSort(null, false); @@ -106,7 +157,9 @@ const CustomHeader: React.FC = ({ e.stopPropagation(); setFilterVisible(!isFilterVisible); - const filterInstance = await api.getColumnFilterInstance(column); + const filterInstance = (await api.getColumnFilterInstance( + column, + )) as AGGridFilterInstance | null; const filterEl = filterInstance?.eGui; if (filterEl && filterRef.current) { filterRef.current.innerHTML = ''; @@ -114,6 +167,25 @@ const CustomHeader: React.FC = ({ } }; + // Re-open filter popover after server refresh (delay allows AG Grid to finish rendering) + useEffect(() => { + if (lastFilteredColumn === colId && !isFilterVisible) { + const timeoutId = setTimeout( + () => + autoOpenFilterAndFocus( + column, + api, + filterRef, + setFilterVisible, + lastFilteredInputPosition, + ), + FILTER_POPOVER_OPEN_DELAY, + ); + return () => clearTimeout(timeoutId); + } + return undefined; + }, [lastFilteredColumn, colId, lastFilteredInputPosition]); + const handleMenuClick = (e: React.MouseEvent) => { e.stopPropagation(); setMenuVisible(!isMenuVisible); diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx index 23a02905126..c51f88ef0ef 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx @@ -55,7 +55,9 @@ import Pagination from './components/Pagination'; import SearchSelectDropdown from './components/SearchSelectDropdown'; import { SearchOption, SortByItem } from '../types'; import getInitialSortState, { shouldSort } from '../utils/getInitialSortState'; +import getInitialFilterModel from '../utils/getInitialFilterModel'; import { PAGE_SIZE_OPTIONS } from '../consts'; +import { getCompleteFilterState } from '../utils/filterStateManager'; export interface AgGridState extends Partial { timestamp?: number; @@ -100,6 +102,8 @@ export interface AgGridTableProps { showTotals: boolean; width: number; onColumnStateChange?: (state: AgGridChartStateWithMetadata) => void; + onFilterChanged?: (filterModel: Record) => void; + metricColumns?: string[]; gridRef?: RefObject; chartState?: AgGridChartState; } @@ -137,6 +141,8 @@ const AgGridDataTable: FunctionComponent = memo( showTotals, width, onColumnStateChange, + onFilterChanged, + metricColumns = [], chartState, }) => { const gridRef = useRef(null); @@ -144,14 +150,27 @@ const AgGridDataTable: FunctionComponent = memo( const rowData = useMemo(() => data, [data]); const containerRef = useRef(null); const lastCapturedStateRef = useRef(null); + const filterOperationVersionRef = useRef(0); const searchId = `search-${id}`; + + const initialFilterModel = getInitialFilterModel( + chartState, + serverPaginationData, + serverPagination, + ); + const gridInitialState: GridState = { ...(serverPagination && { sort: { sortModel: getInitialSortState(serverPaginationData?.sortBy || []), }, }), + ...(initialFilterModel && { + filter: { + filterModel: initialFilterModel, + }, + }), }; const defaultColDef = useMemo( @@ -332,6 +351,56 @@ const AgGridDataTable: FunctionComponent = memo( [onColumnStateChange], ); + const handleFilterChanged = useCallback(async () => { + filterOperationVersionRef.current += 1; + const currentVersion = filterOperationVersionRef.current; + + const completeFilterState = await getCompleteFilterState( + gridRef, + metricColumns, + ); + + // Skip stale operations from rapid filter changes + if (currentVersion !== filterOperationVersionRef.current) { + return; + } + + // Reject invalid filter states (e.g., text filter on numeric column) + if (completeFilterState.originalFilterModel) { + const filterModel = completeFilterState.originalFilterModel; + const hasInvalidFilterType = Object.entries(filterModel).some( + ([colId, filter]: [string, any]) => { + if ( + filter?.filterType === 'text' && + metricColumns?.includes(colId) + ) { + return true; + } + return false; + }, + ); + + if (hasInvalidFilterType) { + return; + } + } + + if ( + !isEqual( + serverPaginationData?.agGridFilterModel, + completeFilterState.originalFilterModel, + ) + ) { + if (onFilterChanged) { + onFilterChanged(completeFilterState); + } + } + }, [ + onFilterChanged, + metricColumns, + serverPaginationData?.agGridFilterModel, + ]); + useEffect(() => { if ( hasServerPageLengthChanged && @@ -356,19 +425,14 @@ const AgGridDataTable: FunctionComponent = memo( // This will make columns fill the grid width params.api.sizeColumnsToFit(); - // Restore saved AG Grid state from permalink if available - if (chartState && params.api) { + // Restore saved column state from permalink if available + // Note: filterModel is now handled via gridInitialState for better performance + if (chartState?.columnState && params.api) { try { - if (chartState.columnState) { - params.api.applyColumnState?.({ - state: chartState.columnState as ColumnState[], - applyOrder: true, - }); - } - - if (chartState.filterModel) { - params.api.setFilterModel?.(chartState.filterModel); - } + params.api.applyColumnState?.({ + state: chartState.columnState as ColumnState[], + applyOrder: true, + }); } catch { // Silently fail if state restoration fails } @@ -429,6 +493,7 @@ const AgGridDataTable: FunctionComponent = memo( rowSelection="multiple" animateRows onCellClicked={handleCrossFilter} + onFilterChanged={handleFilterChanged} onStateUpdated={handleGridStateChange} initialState={gridInitialState} maintainColumnOrder @@ -520,6 +585,9 @@ const AgGridDataTable: FunctionComponent = memo( serverPaginationData?.sortBy || [], ), isActiveFilterValue, + lastFilteredColumn: serverPaginationData?.lastFilteredColumn, + lastFilteredInputPosition: + serverPaginationData?.lastFilteredInputPosition, }} /> {serverPagination && ( diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx index add287c6f05..2944477696e 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx @@ -42,6 +42,7 @@ import TimeComparisonVisibility from './AgGridTable/components/TimeComparisonVis import { useColDefs } from './utils/useColDefs'; import { getCrossFilterDataMask } from './utils/getCrossFilterDataMask'; import { StyledChartContainer } from './styles'; +import type { FilterState } from './utils/filterStateManager'; const getGridHeight = (height: number, includeSearch: boolean | undefined) => { let calculatedGridHeight = height; @@ -88,6 +89,15 @@ export default function TableChart( const [searchOptions, setSearchOptions] = useState([]); + // Extract metric column names for SQL conversion + const metricColumns = useMemo( + () => + columns + .filter(col => col.isMetric || col.isPercentMetric) + .map(col => col.key), + [columns], + ); + useEffect(() => { const options = columns .filter(col => col?.dataType === GenericDataType.String) @@ -101,6 +111,29 @@ export default function TableChart( } }, [columns]); + useEffect(() => { + if (!serverPagination || !serverPaginationData || !rowCount) return; + + const currentPage = serverPaginationData.currentPage ?? 0; + const currentPageSize = serverPaginationData.pageSize ?? serverPageLength; + const totalPages = Math.ceil(rowCount / currentPageSize); + + if (currentPage >= totalPages && totalPages > 0) { + const validPage = Math.max(0, totalPages - 1); + const modifiedOwnState = { + ...serverPaginationData, + currentPage: validPage, + }; + updateTableOwnState(setDataMask, modifiedOwnState); + } + }, [ + rowCount, + serverPagination, + serverPaginationData, + serverPageLength, + setDataMask, + ]); + const comparisonColumns = [ { key: 'all', label: t('Display all') }, { key: '#', label: '#' }, @@ -121,6 +154,52 @@ export default function TableChart( [onChartStateChange], ); + const handleFilterChanged = useCallback( + (completeFilterState: FilterState) => { + if (!serverPagination) return; + // Sync chartState immediately with the new filter model to prevent stale state + // This ensures chartState and ownState are in sync + if (onChartStateChange && chartState) { + const filterModel = + completeFilterState.originalFilterModel && + Object.keys(completeFilterState.originalFilterModel).length > 0 + ? completeFilterState.originalFilterModel + : undefined; + const updatedChartState = { + ...chartState, + filterModel, + timestamp: Date.now(), + }; + onChartStateChange(updatedChartState); + } + + // Prepare modified own state for server pagination + const modifiedOwnState = { + ...serverPaginationData, + agGridFilterModel: + completeFilterState.originalFilterModel && + Object.keys(completeFilterState.originalFilterModel).length > 0 + ? completeFilterState.originalFilterModel + : undefined, + agGridSimpleFilters: completeFilterState.simpleFilters, + agGridComplexWhere: completeFilterState.complexWhere, + agGridHavingClause: completeFilterState.havingClause, + lastFilteredColumn: completeFilterState.lastFilteredColumn, + lastFilteredInputPosition: completeFilterState.inputPosition, + currentPage: 0, // Reset to first page when filtering + }; + + updateTableOwnState(setDataMask, modifiedOwnState); + }, + [ + setDataMask, + serverPagination, + serverPaginationData, + onChartStateChange, + chartState, + ], + ); + const filteredColumns = useMemo(() => { if (!isUsingTimeComparison) { return columns; @@ -206,6 +285,8 @@ export default function TableChart( ...serverPaginationData, currentPage: pageNumber, pageSize, + lastFilteredColumn: undefined, + lastFilteredInputPosition: undefined, }; updateTableOwnState(setDataMask, modifiedOwnState); }, @@ -218,6 +299,8 @@ export default function TableChart( ...serverPaginationData, currentPage: 0, pageSize, + lastFilteredColumn: undefined, + lastFilteredInputPosition: undefined, }; updateTableOwnState(setDataMask, modifiedOwnState); }, @@ -230,6 +313,8 @@ export default function TableChart( ...serverPaginationData, searchColumn: searchCol, searchText: '', + lastFilteredColumn: undefined, + lastFilteredInputPosition: undefined, }; updateTableOwnState(setDataMask, modifiedOwnState); } @@ -243,6 +328,8 @@ export default function TableChart( serverPaginationData?.searchColumn || searchOptions[0]?.value, searchText, currentPage: 0, // Reset to first page when searching + lastFilteredColumn: undefined, + lastFilteredInputPosition: undefined, }; updateTableOwnState(setDataMask, modifiedOwnState); }, @@ -255,6 +342,8 @@ export default function TableChart( const modifiedOwnState = { ...serverPaginationData, sortBy, + lastFilteredColumn: undefined, + lastFilteredInputPosition: undefined, }; updateTableOwnState(setDataMask, modifiedOwnState); }, @@ -288,6 +377,8 @@ export default function TableChart( onSearchColChange={handleChangeSearchCol} onSearchChange={handleSearch} onSortChange={handleSortByChange} + onFilterChanged={handleFilterChanged} + metricColumns={metricColumns} id={slice_id} handleCrossFilter={toggleFilter} percentMetrics={percentMetrics} diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts index 2749071368b..293509ff1aa 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/buildQuery.ts @@ -22,6 +22,7 @@ import { ensureIsArray, getColumnLabel, getMetricLabel, + isDefined, isPhysicalColumn, QueryFormColumn, QueryFormMetric, @@ -253,13 +254,23 @@ const buildQuery: BuildQuery = ( }; sortByFromOwnState = sortSource - .map((sortItem: any) => { - const colId = sortItem?.colId || sortItem?.key; - const sortKey = mapColIdToIdentifier(colId); - if (!sortKey) return null; - const isDesc = sortItem?.sort === 'desc' || sortItem?.desc; - return [sortKey, !isDesc] as QueryFormOrderBy; - }) + .map( + (sortItem: { + colId?: string | number; + key?: string | number; + sort?: string; + desc?: boolean; + }) => { + const colId = isDefined(sortItem?.colId) + ? sortItem.colId + : sortItem?.key; + if (!isDefined(colId)) return null; + const sortKey = mapColIdToIdentifier(String(colId)); + if (!sortKey) return null; + const isDesc = sortItem?.sort === 'desc' || sortItem?.desc; + return [sortKey, !isDesc] as QueryFormOrderBy; + }, + ) .filter((item): item is QueryFormOrderBy => item !== null); // Add secondary sort for stable ordering (matches AG Grid's stable sort behavior) @@ -361,6 +372,8 @@ const buildQuery: BuildQuery = ( ...options?.ownState, currentPage: 0, pageSize: queryObject.row_limit ?? 0, + lastFilteredColumn: undefined, + lastFilteredInputPosition: undefined, }; updateTableOwnState(options?.hooks?.setDataMask, modifiedOwnState); } @@ -370,21 +383,6 @@ const buildQuery: BuildQuery = ( }); const extraQueries: QueryObject[] = []; - if ( - metrics?.length && - formData.show_totals && - queryMode === QueryMode.Aggregate - ) { - extraQueries.push({ - ...queryObject, - columns: [], - row_limit: 0, - row_offset: 0, - post_processing: [], - order_desc: undefined, // we don't need orderby stuff here, - orderby: undefined, // because this query will be used for get total aggregation. - }); - } const interactiveGroupBy = formData.extra_form_data?.interactive_groupby; if (interactiveGroupBy && queryObject.columns) { @@ -445,6 +443,70 @@ const buildQuery: BuildQuery = ( ], }; } + // Add AG Grid column filters from ownState (non-metric filters only) + if ( + ownState.agGridSimpleFilters && + ownState.agGridSimpleFilters.length > 0 + ) { + // Get columns that have AG Grid filters + const agGridFilterColumns = new Set( + ownState.agGridSimpleFilters.map( + (filter: { col: string }) => filter.col, + ), + ); + + // Remove existing TEMPORAL_RANGE filters for columns that have new AG Grid filters + // This prevents duplicate filters like "No filter" and actual date ranges + const existingFilters = (queryObject.filters || []).filter(filter => { + // Keep filter if it doesn't have the expected structure + if (!filter || typeof filter !== 'object' || !filter.col) { + return true; + } + // Keep filter if it's not a temporal range filter + if (filter.op !== 'TEMPORAL_RANGE') { + return true; + } + // Remove if this column has an AG Grid filter + return !agGridFilterColumns.has(filter.col); + }); + + queryObject = { + ...queryObject, + filters: [...existingFilters, ...ownState.agGridSimpleFilters], + }; + } + + // Add AG Grid complex WHERE clause from ownState (non-metric filters) + if (ownState.agGridComplexWhere && ownState.agGridComplexWhere.trim()) { + const existingWhere = queryObject.extras?.where; + const combinedWhere = existingWhere + ? `${existingWhere} AND ${ownState.agGridComplexWhere}` + : ownState.agGridComplexWhere; + + queryObject = { + ...queryObject, + extras: { + ...queryObject.extras, + where: combinedWhere, + }, + }; + } + + // Add AG Grid HAVING clause from ownState (metric filters only) + if (ownState.agGridHavingClause && ownState.agGridHavingClause.trim()) { + const existingHaving = queryObject.extras?.having; + const combinedHaving = existingHaving + ? `${existingHaving} AND ${ownState.agGridHavingClause}` + : ownState.agGridHavingClause; + + queryObject = { + ...queryObject, + extras: { + ...queryObject.extras, + having: combinedHaving, + }, + }; + } } if (isDownloadQuery) { @@ -481,6 +543,54 @@ const buildQuery: BuildQuery = ( } } + // Create totals query AFTER all filters (including AG Grid filters) are applied + // This ensures we can properly exclude AG Grid WHERE filters from the totals + if ( + metrics?.length && + formData.show_totals && + queryMode === QueryMode.Aggregate + ) { + // Create a copy of extras without the AG Grid WHERE clause + // AG Grid filters in extras.where can reference calculated columns + // which aren't available in the totals subquery + const totalsExtras = { ...queryObject.extras }; + if (ownState.agGridComplexWhere) { + // Remove AG Grid WHERE clause from totals query + const whereClause = totalsExtras.where; + if (whereClause) { + // Remove the AG Grid filter part from the WHERE clause using string methods + const agGridWhere = ownState.agGridComplexWhere; + let newWhereClause = whereClause; + + // Try to remove with " AND " before + newWhereClause = newWhereClause.replace(` AND ${agGridWhere}`, ''); + // Try to remove with " AND " after + newWhereClause = newWhereClause.replace(`${agGridWhere} AND `, ''); + // If it's the only clause, remove it entirely + if (newWhereClause === agGridWhere) { + newWhereClause = ''; + } + + if (newWhereClause.trim()) { + totalsExtras.where = newWhereClause; + } else { + delete totalsExtras.where; + } + } + } + + extraQueries.push({ + ...queryObject, + columns: [], + extras: totalsExtras, // Use extras with AG Grid WHERE removed + row_limit: 0, + row_offset: 0, + post_processing: [], + order_desc: undefined, // we don't need orderby stuff here, + orderby: undefined, // because this query will be used for get total aggregation. + }); + } + // Now since row limit control is always visible even // in case of server pagination // we must use row limit from form data @@ -506,8 +616,8 @@ const buildQuery: BuildQuery = ( // Use this closure to cache changing of external filters, if we have server pagination we need reset page to 0, after // external filter changed export const cachedBuildQuery = (): BuildQuery => { - let cachedChanges: any = {}; - const setCachedChanges = (newChanges: any) => { + let cachedChanges: Record = {}; + const setCachedChanges = (newChanges: Record) => { cachedChanges = { ...cachedChanges, ...newChanges }; }; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts index 995e43ea672..29d43a17aca 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/consts.ts @@ -27,3 +27,18 @@ export const PAGE_SIZE_OPTIONS = [10, 20, 50, 100, 200]; export const CUSTOM_AGG_FUNCS = { queryTotal: 'Metric total', }; + +export const FILTER_POPOVER_OPEN_DELAY = 200; +export const FILTER_INPUT_SELECTOR = 'input[data-ref="eInput"]'; +export const NOOP_FILTER_COMPARATOR = () => 0; + +export const FILTER_INPUT_POSITIONS = { + FIRST: 'first' as const, + SECOND: 'second' as const, + UNKNOWN: 'unknown' as const, +} as const; + +export const FILTER_CONDITION_BODY_INDEX = { + FIRST: 0, + SECOND: 1, +} as const; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx index cd1f8564bfe..a13502374af 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import { styled, useTheme } from '@apache-superset/core/ui'; +import { styled, useTheme, type SupersetTheme } from '@apache-superset/core/ui'; import { CustomCellRendererProps } from '@superset-ui/core/components/ThemedAgGridReact'; -import { BasicColorFormatterType, InputColumn } from '../types'; +import { BasicColorFormatterType, InputColumn, ValueRange } from '../types'; import { useIsDark } from '../utils/useTableTheme'; const StyledTotalCell = styled.div` @@ -53,8 +53,6 @@ const Bar = styled.div<{ z-index: 1; `; -type ValueRange = [number, number]; - /** * Cell background width calculation for horizontal bar chart */ @@ -64,7 +62,7 @@ function cellWidth({ alignPositiveNegative, }: { value: number; - valueRange: ValueRange; + valueRange: [number, number]; alignPositiveNegative: boolean; }) { const [minValue, maxValue] = valueRange; @@ -89,7 +87,7 @@ function cellOffset({ alignPositiveNegative, }: { value: number; - valueRange: ValueRange; + valueRange: [number, number]; alignPositiveNegative: boolean; }) { if (alignPositiveNegative) { @@ -114,7 +112,7 @@ function cellBackground({ value: number; colorPositiveNegative: boolean; isDarkTheme: boolean; - theme: any; + theme: SupersetTheme | null; }) { if (!colorPositiveNegative) { return 'transparent'; // Use transparent background when colorPositiveNegative is false @@ -134,7 +132,7 @@ export const NumericCellRenderer = ( basicColorFormatters: { [Key: string]: BasicColorFormatterType; }[]; - valueRange: any; + valueRange: ValueRange; alignPositiveNegative: boolean; colorPositiveNegative: boolean; }, diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/stateConversion.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/stateConversion.ts index a8b8d10f4b9..bc04be64221 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/stateConversion.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/stateConversion.ts @@ -25,30 +25,46 @@ import { type AgGridFilterModel, type AgGridFilter, } from '@superset-ui/core'; +import { + getStartOfDay, + getEndOfDay, + FILTER_OPERATORS, + SQL_OPERATORS, + validateColumnName, +} from './utils/agGridFilterConverter'; /** - * AG Grid text filter type to backend operator mapping + * Maps custom server-side date filter operators to normalized operator names. + * Server-side operators (serverEquals, serverBefore, etc.) are custom operators + * used when server_pagination is enabled to bypass client-side filtering. */ -const TEXT_FILTER_OPERATORS: Record = { - equals: '==', - notEqual: '!=', - contains: 'ILIKE', - notContains: 'NOT ILIKE', - startsWith: 'ILIKE', - endsWith: 'ILIKE', +const DATE_FILTER_OPERATOR_MAP: Record = { + // Standard operators + [FILTER_OPERATORS.EQUALS]: FILTER_OPERATORS.EQUALS, + [FILTER_OPERATORS.NOT_EQUAL]: FILTER_OPERATORS.NOT_EQUAL, + [FILTER_OPERATORS.LESS_THAN]: FILTER_OPERATORS.LESS_THAN, + [FILTER_OPERATORS.LESS_THAN_OR_EQUAL]: FILTER_OPERATORS.LESS_THAN_OR_EQUAL, + [FILTER_OPERATORS.GREATER_THAN]: FILTER_OPERATORS.GREATER_THAN, + [FILTER_OPERATORS.GREATER_THAN_OR_EQUAL]: + FILTER_OPERATORS.GREATER_THAN_OR_EQUAL, + [FILTER_OPERATORS.IN_RANGE]: FILTER_OPERATORS.IN_RANGE, + // Custom server-side operators (map to standard equivalents) + [FILTER_OPERATORS.SERVER_EQUALS]: FILTER_OPERATORS.EQUALS, + [FILTER_OPERATORS.SERVER_NOT_EQUAL]: FILTER_OPERATORS.NOT_EQUAL, + [FILTER_OPERATORS.SERVER_BEFORE]: FILTER_OPERATORS.LESS_THAN, + [FILTER_OPERATORS.SERVER_AFTER]: FILTER_OPERATORS.GREATER_THAN, + [FILTER_OPERATORS.SERVER_IN_RANGE]: FILTER_OPERATORS.IN_RANGE, }; /** - * AG Grid number filter type to backend operator mapping + * Blank filter operator types */ -const NUMBER_FILTER_OPERATORS: Record = { - equals: '==', - notEqual: '!=', - lessThan: '<', - lessThanOrEqual: '<=', - greaterThan: '>', - greaterThanOrEqual: '>=', -}; +const BLANK_OPERATORS: Set = new Set([ + FILTER_OPERATORS.BLANK, + FILTER_OPERATORS.NOT_BLANK, + FILTER_OPERATORS.SERVER_BLANK, + FILTER_OPERATORS.SERVER_NOT_BLANK, +]); /** Escapes single quotes in SQL strings: O'Hara → O''Hara */ function escapeStringValue(value: string): string { @@ -56,18 +72,77 @@ function escapeStringValue(value: string): string { } function getTextComparator(type: string, value: string): string { - if (type === 'contains' || type === 'notContains') { + if ( + type === FILTER_OPERATORS.CONTAINS || + type === FILTER_OPERATORS.NOT_CONTAINS + ) { return `%${value}%`; } - if (type === 'startsWith') { + if (type === FILTER_OPERATORS.STARTS_WITH) { return `${value}%`; } - if (type === 'endsWith') { + if (type === FILTER_OPERATORS.ENDS_WITH) { return `%${value}`; } return value; } +/** + * Converts a date filter to SQL clause. + * Handles both standard operators (equals, lessThan, etc.) and + * custom server-side operators (serverEquals, serverBefore, etc.). + * + * @param colId - Column identifier + * @param filter - AG Grid date filter object + * @returns SQL clause string or null if conversion not possible + */ +function convertDateFilterToSQL( + colId: string, + filter: AgGridFilter, +): string | null { + const { type, dateFrom, dateTo } = filter; + + if (!type) return null; + + // Map custom server operators to standard ones + const normalizedType = DATE_FILTER_OPERATOR_MAP[type] || type; + + switch (normalizedType) { + case FILTER_OPERATORS.EQUALS: + if (!dateFrom) return null; + // Full day range for equals + return `(${colId} >= '${getStartOfDay(dateFrom)}' AND ${colId} <= '${getEndOfDay(dateFrom)}')`; + + case FILTER_OPERATORS.NOT_EQUAL: + if (!dateFrom) return null; + // Outside the full day range for not equals + return `(${colId} < '${getStartOfDay(dateFrom)}' OR ${colId} > '${getEndOfDay(dateFrom)}')`; + + case FILTER_OPERATORS.LESS_THAN: + if (!dateFrom) return null; + return `${colId} < '${getStartOfDay(dateFrom)}'`; + + case FILTER_OPERATORS.LESS_THAN_OR_EQUAL: + if (!dateFrom) return null; + return `${colId} <= '${getEndOfDay(dateFrom)}'`; + + case FILTER_OPERATORS.GREATER_THAN: + if (!dateFrom) return null; + return `${colId} > '${getEndOfDay(dateFrom)}'`; + + case FILTER_OPERATORS.GREATER_THAN_OR_EQUAL: + if (!dateFrom) return null; + return `${colId} >= '${getStartOfDay(dateFrom)}'`; + + case FILTER_OPERATORS.IN_RANGE: + if (!dateFrom || !dateTo) return null; + return `${colId} BETWEEN '${getStartOfDay(dateFrom)}' AND '${getEndOfDay(dateTo)}'`; + + default: + return null; + } +} + /** * Converts AG Grid sortModel to backend sortBy format */ @@ -111,11 +186,18 @@ export function convertColumnState( * - Complex: {operator: 'AND', condition1: {type: 'greaterThan', filter: 1}, condition2: {type: 'lessThan', filter: 16}} * → "(column_name > 1 AND column_name < 16)" * - Set: {filterType: 'set', values: ['a', 'b']} → "column_name IN ('a', 'b')" + * - Blank: {filterType: 'text', type: 'blank'} → "column_name IS NULL" + * - Date: {filterType: 'date', type: 'serverBefore', dateFrom: '2024-01-01'} → "column_name < '2024-01-01T00:00:00'" */ function convertFilterToSQL( colId: string, filter: AgGridFilter, ): string | null { + // Validate column name to prevent SQL injection and malformed queries + if (!validateColumnName(colId)) { + return null; + } + // Complex filter: has operator and conditions if ( filter.operator && @@ -137,14 +219,43 @@ function convertFilterToSQL( return `(${conditions.join(` ${filter.operator} `)})`; } + // Handle blank/notBlank operators for all filter types + // These are special operators that check for NULL values + if (filter.type && BLANK_OPERATORS.has(filter.type)) { + if ( + filter.type === FILTER_OPERATORS.BLANK || + filter.type === FILTER_OPERATORS.SERVER_BLANK + ) { + return `${colId} ${SQL_OPERATORS.IS_NULL}`; + } + if ( + filter.type === FILTER_OPERATORS.NOT_BLANK || + filter.type === FILTER_OPERATORS.SERVER_NOT_BLANK + ) { + return `${colId} ${SQL_OPERATORS.IS_NOT_NULL}`; + } + } + if (filter.filterType === 'text' && filter.filter && filter.type) { - const op = TEXT_FILTER_OPERATORS[filter.type]; const escapedFilter = escapeStringValue(String(filter.filter)); const val = getTextComparator(filter.type, escapedFilter); - return op === 'ILIKE' || op === 'NOT ILIKE' - ? `${colId} ${op} '${val}'` - : `${colId} ${op} '${escapedFilter}'`; + // Map text filter types to SQL operators + switch (filter.type) { + case FILTER_OPERATORS.EQUALS: + return `${colId} ${SQL_OPERATORS.EQUALS} '${escapedFilter}'`; + case FILTER_OPERATORS.NOT_EQUAL: + return `${colId} ${SQL_OPERATORS.NOT_EQUALS} '${escapedFilter}'`; + case FILTER_OPERATORS.CONTAINS: + return `${colId} ${SQL_OPERATORS.ILIKE} '${val}'`; + case FILTER_OPERATORS.NOT_CONTAINS: + return `${colId} ${SQL_OPERATORS.NOT_ILIKE} '${val}'`; + case FILTER_OPERATORS.STARTS_WITH: + case FILTER_OPERATORS.ENDS_WITH: + return `${colId} ${SQL_OPERATORS.ILIKE} '${val}'`; + default: + return null; + } } if ( @@ -152,14 +263,28 @@ function convertFilterToSQL( filter.filter !== undefined && filter.type ) { - const op = NUMBER_FILTER_OPERATORS[filter.type]; - return `${colId} ${op} ${filter.filter}`; + // Map number filter types to SQL operators + switch (filter.type) { + case FILTER_OPERATORS.EQUALS: + return `${colId} ${SQL_OPERATORS.EQUALS} ${filter.filter}`; + case FILTER_OPERATORS.NOT_EQUAL: + return `${colId} ${SQL_OPERATORS.NOT_EQUALS} ${filter.filter}`; + case FILTER_OPERATORS.LESS_THAN: + return `${colId} ${SQL_OPERATORS.LESS_THAN} ${filter.filter}`; + case FILTER_OPERATORS.LESS_THAN_OR_EQUAL: + return `${colId} ${SQL_OPERATORS.LESS_THAN_OR_EQUAL} ${filter.filter}`; + case FILTER_OPERATORS.GREATER_THAN: + return `${colId} ${SQL_OPERATORS.GREATER_THAN} ${filter.filter}`; + case FILTER_OPERATORS.GREATER_THAN_OR_EQUAL: + return `${colId} ${SQL_OPERATORS.GREATER_THAN_OR_EQUAL} ${filter.filter}`; + default: + return null; + } } - if (filter.filterType === 'date' && filter.dateFrom && filter.type) { - const op = NUMBER_FILTER_OPERATORS[filter.type]; - const escapedDate = escapeStringValue(filter.dateFrom); - return `${colId} ${op} '${escapedDate}'`; + // Handle date filters with proper date formatting and custom server operators + if (filter.filterType === 'date' && filter.type) { + return convertDateFilterToSQL(colId, filter); } if ( diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts index b4b89a0a9d2..d1a4cc143a1 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/types.ts @@ -191,9 +191,20 @@ export interface SortState { sort: 'asc' | 'desc' | null; } +export type FilterInputPosition = 'first' | 'second' | 'unknown'; + +export interface AGGridFilterInstance { + eGui?: HTMLElement; + eConditionBodies?: HTMLElement[]; + eJoinAnds?: Array<{ eGui?: HTMLElement }>; + eJoinOrs?: Array<{ eGui?: HTMLElement }>; +} + export interface CustomContext { initialSortState: SortState[]; onColumnHeaderClicked: (args: { column: SortState }) => void; + lastFilteredColumn?: string; + lastFilteredInputPosition?: FilterInputPosition; } export interface CustomHeaderParams extends IHeaderParams { @@ -226,19 +237,25 @@ export interface InputColumn { isNumeric: boolean; isMetric: boolean; isPercentMetric: boolean; - config: Record; - formatter?: Function; + config: TableColumnConfig; + formatter?: + | TimeFormatter + | NumberFormatter + | CustomFormatter + | CurrencyFormatter; originalLabel?: string; metricName?: string; } +export type ValueRange = [number, number] | null; + export type CellRendererProps = CustomCellRendererProps & { hasBasicColorFormatters: boolean | undefined; col: InputColumn; basicColorFormatters: { [Key: string]: BasicColorFormatterType; }[]; - valueRange: any; + valueRange: ValueRange; alignPositiveNegative: boolean; colorPositiveNegative: boolean; allowRenderHtml: boolean; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts new file mode 100644 index 00000000000..5fe226ef84d --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/agGridFilterConverter.ts @@ -0,0 +1,726 @@ +/** + * 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. + */ + +export type AgGridFilterType = 'text' | 'number' | 'date' | 'set' | 'boolean'; + +export type AgGridFilterOperator = + | 'equals' + | 'notEqual' + | 'contains' + | 'notContains' + | 'startsWith' + | 'endsWith' + | 'lessThan' + | 'lessThanOrEqual' + | 'greaterThan' + | 'greaterThanOrEqual' + | 'inRange' + | 'blank' + | 'notBlank' + // Custom server-side date filter operators (always pass client-side filtering) + | 'serverEquals' + | 'serverNotEqual' + | 'serverBefore' + | 'serverAfter' + | 'serverInRange' + | 'serverBlank' + | 'serverNotBlank'; + +export type AgGridLogicalOperator = 'AND' | 'OR'; + +export const FILTER_OPERATORS = { + EQUALS: 'equals' as const, + NOT_EQUAL: 'notEqual' as const, + CONTAINS: 'contains' as const, + NOT_CONTAINS: 'notContains' as const, + STARTS_WITH: 'startsWith' as const, + ENDS_WITH: 'endsWith' as const, + LESS_THAN: 'lessThan' as const, + LESS_THAN_OR_EQUAL: 'lessThanOrEqual' as const, + GREATER_THAN: 'greaterThan' as const, + GREATER_THAN_OR_EQUAL: 'greaterThanOrEqual' as const, + IN_RANGE: 'inRange' as const, + BLANK: 'blank' as const, + NOT_BLANK: 'notBlank' as const, + // Custom server-side date filter operators + SERVER_EQUALS: 'serverEquals' as const, + SERVER_NOT_EQUAL: 'serverNotEqual' as const, + SERVER_BEFORE: 'serverBefore' as const, + SERVER_AFTER: 'serverAfter' as const, + SERVER_IN_RANGE: 'serverInRange' as const, + SERVER_BLANK: 'serverBlank' as const, + SERVER_NOT_BLANK: 'serverNotBlank' as const, +} as const; + +export const SQL_OPERATORS = { + EQUALS: '=', + NOT_EQUALS: '!=', + ILIKE: 'ILIKE', + NOT_ILIKE: 'NOT ILIKE', + LESS_THAN: '<', + LESS_THAN_OR_EQUAL: '<=', + GREATER_THAN: '>', + GREATER_THAN_OR_EQUAL: '>=', + BETWEEN: 'BETWEEN', + IS_NULL: 'IS NULL', + IS_NOT_NULL: 'IS NOT NULL', + IN: 'IN', + TEMPORAL_RANGE: 'TEMPORAL_RANGE', +} as const; + +export type FilterValue = string | number | boolean | Date | null; + +// Regex for validating column names. Allows: +// - Alphanumeric chars, underscores, dots, spaces (standard column names) +// - Parentheses for aggregate functions like COUNT(*) +// - % for LIKE patterns, * for wildcards, + - / for computed columns +const COLUMN_NAME_REGEX = /^[a-zA-Z0-9_. ()%*+\-/]+$/; + +export interface AgGridSimpleFilter { + filterType: AgGridFilterType; + type: AgGridFilterOperator; + filter?: FilterValue; + filterTo?: FilterValue; + // Date filter properties + dateFrom?: string | null; + dateTo?: string | null; +} + +export interface AgGridCompoundFilter { + filterType: AgGridFilterType; + operator: AgGridLogicalOperator; + condition1: AgGridSimpleFilter; + condition2: AgGridSimpleFilter; + conditions?: AgGridSimpleFilter[]; +} + +export interface AgGridSetFilter { + filterType: 'set'; + values: FilterValue[]; +} + +export type AgGridFilterModel = Record< + string, + AgGridSimpleFilter | AgGridCompoundFilter | AgGridSetFilter +>; + +export interface SQLAlchemyFilter { + col: string; + op: string; + val: FilterValue | FilterValue[]; +} + +export interface ConvertedFilter { + simpleFilters: SQLAlchemyFilter[]; + complexWhere?: string; + havingClause?: string; +} + +const AG_GRID_TO_SQLA_OPERATOR_MAP: Record = { + [FILTER_OPERATORS.EQUALS]: SQL_OPERATORS.EQUALS, + [FILTER_OPERATORS.NOT_EQUAL]: SQL_OPERATORS.NOT_EQUALS, + [FILTER_OPERATORS.CONTAINS]: SQL_OPERATORS.ILIKE, + [FILTER_OPERATORS.NOT_CONTAINS]: SQL_OPERATORS.NOT_ILIKE, + [FILTER_OPERATORS.STARTS_WITH]: SQL_OPERATORS.ILIKE, + [FILTER_OPERATORS.ENDS_WITH]: SQL_OPERATORS.ILIKE, + [FILTER_OPERATORS.LESS_THAN]: SQL_OPERATORS.LESS_THAN, + [FILTER_OPERATORS.LESS_THAN_OR_EQUAL]: SQL_OPERATORS.LESS_THAN_OR_EQUAL, + [FILTER_OPERATORS.GREATER_THAN]: SQL_OPERATORS.GREATER_THAN, + [FILTER_OPERATORS.GREATER_THAN_OR_EQUAL]: SQL_OPERATORS.GREATER_THAN_OR_EQUAL, + [FILTER_OPERATORS.IN_RANGE]: SQL_OPERATORS.BETWEEN, + [FILTER_OPERATORS.BLANK]: SQL_OPERATORS.IS_NULL, + [FILTER_OPERATORS.NOT_BLANK]: SQL_OPERATORS.IS_NOT_NULL, + // Server-side date filter operators (map to same SQL operators as standard ones) + [FILTER_OPERATORS.SERVER_EQUALS]: SQL_OPERATORS.EQUALS, + [FILTER_OPERATORS.SERVER_NOT_EQUAL]: SQL_OPERATORS.NOT_EQUALS, + [FILTER_OPERATORS.SERVER_BEFORE]: SQL_OPERATORS.LESS_THAN, + [FILTER_OPERATORS.SERVER_AFTER]: SQL_OPERATORS.GREATER_THAN, + [FILTER_OPERATORS.SERVER_IN_RANGE]: SQL_OPERATORS.BETWEEN, + [FILTER_OPERATORS.SERVER_BLANK]: SQL_OPERATORS.IS_NULL, + [FILTER_OPERATORS.SERVER_NOT_BLANK]: SQL_OPERATORS.IS_NOT_NULL, +}; + +/** + * Escapes single quotes in SQL strings to prevent SQL injection + * @param value - String value to escape + * @returns Escaped string safe for SQL queries + */ +function escapeSQLString(value: string): string { + return value.replace(/'/g, "''"); +} + +// Maximum column name length - conservative upper bound that exceeds all common +// database identifier limits (MySQL: 64, PostgreSQL: 63, SQL Server: 128, Oracle: 128) +const MAX_COLUMN_NAME_LENGTH = 255; + +/** + * Validates a column name to prevent SQL injection + * Checks for: non-empty string, length limit, allowed characters + */ +export function validateColumnName(columnName: string): boolean { + if (!columnName || typeof columnName !== 'string') { + return false; + } + + if (columnName.length > MAX_COLUMN_NAME_LENGTH) { + return false; + } + + if (!COLUMN_NAME_REGEX.test(columnName)) { + return false; + } + + return true; +} + +/** + * Validates a filter value for a given operator + * BLANK and NOT_BLANK operators don't require values + * @param value - Filter value to validate + * @param operator - AG Grid filter operator + * @returns True if the value is valid for the operator, false otherwise + */ +function validateFilterValue( + value: FilterValue | undefined, + operator: AgGridFilterOperator, +): boolean { + if ( + operator === FILTER_OPERATORS.BLANK || + operator === FILTER_OPERATORS.NOT_BLANK + ) { + return true; + } + + if (value === undefined) { + return false; + } + + const valueType = typeof value; + if ( + value !== null && + valueType !== 'string' && + valueType !== 'number' && + valueType !== 'boolean' && + !(value instanceof Date) + ) { + return false; + } + + return true; +} + +function formatValueForOperator( + operator: AgGridFilterOperator, + value: FilterValue, +): FilterValue { + if (typeof value === 'string') { + if ( + operator === FILTER_OPERATORS.CONTAINS || + operator === FILTER_OPERATORS.NOT_CONTAINS + ) { + return `%${value}%`; + } + if (operator === FILTER_OPERATORS.STARTS_WITH) { + return `${value}%`; + } + if (operator === FILTER_OPERATORS.ENDS_WITH) { + return `%${value}`; + } + } + return value; +} + +/** + * Convert a date filter to a WHERE clause + * @param columnName - Column name + * @param filter - AG Grid date filter + * @returns WHERE clause string for date filter + */ +function dateFilterToWhereClause( + columnName: string, + filter: AgGridSimpleFilter, +): string { + const { type, dateFrom, dateTo, filter: filterValue, filterTo } = filter; + + // Support both dateFrom/dateTo and filter/filterTo + const fromDate = dateFrom || (filterValue as string); + const toDate = dateTo || (filterTo as string); + + // Convert based on operator type + switch (type) { + case FILTER_OPERATORS.EQUALS: + if (!fromDate) return ''; + // For equals, check if date is within the full day range + return `(${columnName} >= '${getStartOfDay(fromDate)}' AND ${columnName} <= '${getEndOfDay(fromDate)}')`; + + case FILTER_OPERATORS.NOT_EQUAL: + if (!fromDate) return ''; + // For not equals, exclude the full day range + return `(${columnName} < '${getStartOfDay(fromDate)}' OR ${columnName} > '${getEndOfDay(fromDate)}')`; + + case FILTER_OPERATORS.LESS_THAN: + if (!fromDate) return ''; + return `${columnName} < '${getStartOfDay(fromDate)}'`; + + case FILTER_OPERATORS.LESS_THAN_OR_EQUAL: + if (!fromDate) return ''; + return `${columnName} <= '${getEndOfDay(fromDate)}'`; + + case FILTER_OPERATORS.GREATER_THAN: + if (!fromDate) return ''; + return `${columnName} > '${getEndOfDay(fromDate)}'`; + + case FILTER_OPERATORS.GREATER_THAN_OR_EQUAL: + if (!fromDate) return ''; + return `${columnName} >= '${getStartOfDay(fromDate)}'`; + + case FILTER_OPERATORS.IN_RANGE: + if (!fromDate || !toDate) return ''; + return `${columnName} ${SQL_OPERATORS.BETWEEN} '${getStartOfDay(fromDate)}' AND '${getEndOfDay(toDate)}'`; + + case FILTER_OPERATORS.BLANK: + return `${columnName} ${SQL_OPERATORS.IS_NULL}`; + + case FILTER_OPERATORS.NOT_BLANK: + return `${columnName} ${SQL_OPERATORS.IS_NOT_NULL}`; + + default: + return ''; + } +} + +function simpleFilterToWhereClause( + columnName: string, + filter: AgGridSimpleFilter, +): string { + // Check if this is a date filter and handle it specially + if (filter.filterType === 'date') { + return dateFilterToWhereClause(columnName, filter); + } + + const { type, filter: value, filterTo } = filter; + + const operator = AG_GRID_TO_SQLA_OPERATOR_MAP[type]; + if (!operator) { + return ''; + } + + if (!validateFilterValue(value, type)) { + return ''; + } + + if (type === FILTER_OPERATORS.BLANK) { + return `${columnName} ${SQL_OPERATORS.IS_NULL}`; + } + + if (type === FILTER_OPERATORS.NOT_BLANK) { + return `${columnName} ${SQL_OPERATORS.IS_NOT_NULL}`; + } + + if (value === null || value === undefined) { + return ''; + } + + if (type === FILTER_OPERATORS.IN_RANGE && filterTo !== undefined) { + return `${columnName} ${SQL_OPERATORS.BETWEEN} ${value} AND ${filterTo}`; + } + + const formattedValue = formatValueForOperator(type, value!); + + if ( + operator === SQL_OPERATORS.ILIKE || + operator === SQL_OPERATORS.NOT_ILIKE + ) { + return `${columnName} ${operator} '${escapeSQLString(String(formattedValue))}'`; + } + + if (typeof formattedValue === 'string') { + return `${columnName} ${operator} '${escapeSQLString(formattedValue)}'`; + } + + return `${columnName} ${operator} ${formattedValue}`; +} + +function isCompoundFilter( + filter: AgGridSimpleFilter | AgGridCompoundFilter | AgGridSetFilter, +): filter is AgGridCompoundFilter { + return ( + 'operator' in filter && ('condition1' in filter || 'conditions' in filter) + ); +} + +function isSetFilter( + filter: AgGridSimpleFilter | AgGridCompoundFilter | AgGridSetFilter, +): filter is AgGridSetFilter { + return filter.filterType === 'set' && 'values' in filter; +} + +function compoundFilterToWhereClause( + columnName: string, + filter: AgGridCompoundFilter, +): string { + const { operator, condition1, condition2, conditions } = filter; + + if (conditions && conditions.length > 0) { + const clauses = conditions + .map(cond => { + const clause = simpleFilterToWhereClause(columnName, cond); + + return clause; + }) + .filter(clause => clause !== ''); + + if (clauses.length === 0) { + return ''; + } + + if (clauses.length === 1) { + return clauses[0]; + } + + const result = `(${clauses.join(` ${operator} `)})`; + + return result; + } + + const clause1 = simpleFilterToWhereClause(columnName, condition1); + const clause2 = simpleFilterToWhereClause(columnName, condition2); + + if (!clause1 && !clause2) { + return ''; + } + + if (!clause1) { + return clause2; + } + + if (!clause2) { + return clause1; + } + + const result = `(${clause1} ${operator} ${clause2})`; + return result; +} + +/** + * Format a date string to ISO format expected by Superset, preserving local timezone + */ +export function formatDateForSuperset(dateStr: string): string { + // AG Grid typically provides dates in format: "YYYY-MM-DD HH:MM:SS" + // Superset expects: "YYYY-MM-DDTHH:MM:SS" in local timezone (not UTC) + const date = new Date(dateStr); + if (Number.isNaN(date.getTime())) { + return dateStr; // Return as-is if invalid + } + + // Format date in local timezone, not UTC + const year = date.getFullYear(); + const month = String(date.getMonth() + 1).padStart(2, '0'); + const day = String(date.getDate()).padStart(2, '0'); + const hours = String(date.getHours()).padStart(2, '0'); + const minutes = String(date.getMinutes()).padStart(2, '0'); + const seconds = String(date.getSeconds()).padStart(2, '0'); + + const formatted = `${year}-${month}-${day}T${hours}:${minutes}:${seconds}`; + return formatted; +} + +/** + * Get the start of day (00:00:00) for a given date string + */ +export function getStartOfDay(dateStr: string): string { + const date = new Date(dateStr); + date.setHours(0, 0, 0, 0); + return formatDateForSuperset(date.toISOString()); +} + +/** + * Get the end of day (23:59:59) for a given date string + */ +export function getEndOfDay(dateStr: string): string { + const date = new Date(dateStr); + date.setHours(23, 59, 59, 999); + return formatDateForSuperset(date.toISOString()); +} + +// Converts date filters to TEMPORAL_RANGE format for Superset backend +function convertDateFilter( + columnName: string, + filter: AgGridSimpleFilter, +): SQLAlchemyFilter | null { + if (filter.filterType !== 'date') { + return null; + } + + const { type, dateFrom, dateTo } = filter; + + // Handle null/blank checks for date columns + if ( + type === FILTER_OPERATORS.BLANK || + type === FILTER_OPERATORS.SERVER_BLANK + ) { + return { + col: columnName, + op: SQL_OPERATORS.IS_NULL, + val: null, + }; + } + + if ( + type === FILTER_OPERATORS.NOT_BLANK || + type === FILTER_OPERATORS.SERVER_NOT_BLANK + ) { + return { + col: columnName, + op: SQL_OPERATORS.IS_NOT_NULL, + val: null, + }; + } + + // Validate we have at least one date + if (!dateFrom && !dateTo) { + return null; + } + + let temporalRangeValue: string; + + // Convert based on operator type + switch (type) { + case FILTER_OPERATORS.EQUALS: + case FILTER_OPERATORS.SERVER_EQUALS: + if (!dateFrom) { + return null; + } + // For equals, create a range for the entire day (00:00:00 to 23:59:59) + temporalRangeValue = `${getStartOfDay(dateFrom)} : ${getEndOfDay(dateFrom)}`; + break; + + case FILTER_OPERATORS.NOT_EQUAL: + case FILTER_OPERATORS.SERVER_NOT_EQUAL: + // NOT EQUAL for dates is complex, skip for now + return null; + + case FILTER_OPERATORS.LESS_THAN: + case FILTER_OPERATORS.SERVER_BEFORE: + if (!dateFrom) { + return null; + } + // Everything before the start of this date + temporalRangeValue = ` : ${getStartOfDay(dateFrom)}`; + break; + + case FILTER_OPERATORS.LESS_THAN_OR_EQUAL: + if (!dateFrom) { + return null; + } + // Everything up to and including the end of this date + temporalRangeValue = ` : ${getEndOfDay(dateFrom)}`; + break; + + case FILTER_OPERATORS.GREATER_THAN: + case FILTER_OPERATORS.SERVER_AFTER: + if (!dateFrom) { + return null; + } + // Everything after the end of this date + temporalRangeValue = `${getEndOfDay(dateFrom)} : `; + break; + + case FILTER_OPERATORS.GREATER_THAN_OR_EQUAL: + if (!dateFrom) { + return null; + } + // Everything from the start of this date onwards + temporalRangeValue = `${getStartOfDay(dateFrom)} : `; + break; + + case FILTER_OPERATORS.IN_RANGE: + case FILTER_OPERATORS.SERVER_IN_RANGE: + // Range between two dates + if (!dateFrom || !dateTo) { + return null; + } + // From start of first date to end of second date + temporalRangeValue = `${getStartOfDay(dateFrom)} : ${getEndOfDay(dateTo)}`; + break; + + default: + return null; + } + + const result = { + col: columnName, + op: SQL_OPERATORS.TEMPORAL_RANGE, + val: temporalRangeValue, + }; + + return result; +} + +// Converts AG Grid filters to SQLAlchemy format, separating dimension (WHERE) and metric (HAVING) filters +export function convertAgGridFiltersToSQL( + filterModel: AgGridFilterModel, + metricColumns: string[] = [], +): ConvertedFilter { + if (!filterModel || typeof filterModel !== 'object') { + return { + simpleFilters: [], + complexWhere: undefined, + havingClause: undefined, + }; + } + + const metricColumnsSet = new Set(metricColumns); + const simpleFilters: SQLAlchemyFilter[] = []; + const complexWhereClauses: string[] = []; + const complexHavingClauses: string[] = []; + + Object.entries(filterModel).forEach(([columnName, filter]) => { + if (!validateColumnName(columnName)) { + return; + } + + if (!filter || typeof filter !== 'object') { + return; + } + + const isMetric = metricColumnsSet.has(columnName); + + if (isSetFilter(filter)) { + if (!Array.isArray(filter.values) || filter.values.length === 0) { + return; + } + + if (isMetric) { + const values = filter.values + .map(v => (typeof v === 'string' ? `'${escapeSQLString(v)}'` : v)) + .join(', '); + complexHavingClauses.push(`${columnName} IN (${values})`); + } else { + simpleFilters.push({ + col: columnName, + op: SQL_OPERATORS.IN, + val: filter.values, + }); + } + return; + } + + if (isCompoundFilter(filter)) { + const whereClause = compoundFilterToWhereClause(columnName, filter); + if (whereClause) { + if (isMetric) { + complexHavingClauses.push(whereClause); + } else { + complexWhereClauses.push(whereClause); + } + } + return; + } + + const simpleFilter = filter as AgGridSimpleFilter; + + // Check if this is a date filter and handle it specially + if (simpleFilter.filterType === 'date') { + const dateFilter = convertDateFilter(columnName, simpleFilter); + if (dateFilter) { + simpleFilters.push(dateFilter); + return; + } + } + + const { type, filter: value } = simpleFilter; + + if (!type) { + return; + } + + const operator = AG_GRID_TO_SQLA_OPERATOR_MAP[type]; + if (!operator) { + return; + } + + if (type === FILTER_OPERATORS.BLANK) { + if (isMetric) { + complexHavingClauses.push(`${columnName} ${SQL_OPERATORS.IS_NULL}`); + } else { + simpleFilters.push({ + col: columnName, + op: SQL_OPERATORS.IS_NULL, + val: null, + }); + } + return; + } + + if (type === FILTER_OPERATORS.NOT_BLANK) { + if (isMetric) { + complexHavingClauses.push(`${columnName} ${SQL_OPERATORS.IS_NOT_NULL}`); + } else { + simpleFilters.push({ + col: columnName, + op: SQL_OPERATORS.IS_NOT_NULL, + val: null, + }); + } + return; + } + + if (!validateFilterValue(value, type)) { + return; + } + + const formattedValue = formatValueForOperator(type, value!); + + if (isMetric) { + const sqlClause = simpleFilterToWhereClause(columnName, simpleFilter); + if (sqlClause) { + complexHavingClauses.push(sqlClause); + } + } else { + simpleFilters.push({ + col: columnName, + op: operator, + val: formattedValue, + }); + } + }); + + let complexWhere; + if (complexWhereClauses.length === 1) { + [complexWhere] = complexWhereClauses; + } else if (complexWhereClauses.length > 1) { + complexWhere = `(${complexWhereClauses.join(' AND ')})`; + } + + let havingClause; + if (complexHavingClauses.length === 1) { + [havingClause] = complexHavingClauses; + } else if (complexHavingClauses.length > 1) { + havingClause = `(${complexHavingClauses.join(' AND ')})`; + } + + const result = { + simpleFilters, + complexWhere, + havingClause, + }; + + return result; +} diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/filterStateManager.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/filterStateManager.ts new file mode 100644 index 00000000000..85304bb4119 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/filterStateManager.ts @@ -0,0 +1,164 @@ +/** + * 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 type { RefObject } from 'react'; +import { GridApi } from 'ag-grid-community'; +import { convertAgGridFiltersToSQL } from './agGridFilterConverter'; +import type { + AgGridFilterModel, + SQLAlchemyFilter, +} from './agGridFilterConverter'; +import type { AgGridReact } from '@superset-ui/core/components/ThemedAgGridReact'; +import type { FilterInputPosition, AGGridFilterInstance } from '../types'; +import { FILTER_INPUT_POSITIONS, FILTER_CONDITION_BODY_INDEX } from '../consts'; + +export interface FilterState { + originalFilterModel: AgGridFilterModel; + simpleFilters: SQLAlchemyFilter[]; + complexWhere?: string; + havingClause?: string; + lastFilteredColumn?: string; + inputPosition?: FilterInputPosition; +} + +/** + * Detects which input position (first or second) was last modified in a filter. + * Note: activeElement is captured before async operations and passed here to ensure + * we check against the element that was focused when the detection was initiated, + * not what might be focused after async operations complete. + */ +async function detectLastFilteredInput( + gridApi: GridApi, + filterModel: AgGridFilterModel, + activeElement: HTMLElement, +): Promise<{ + lastFilteredColumn?: string; + inputPosition: FilterInputPosition; +}> { + let inputPosition: FilterInputPosition = FILTER_INPUT_POSITIONS.UNKNOWN; + let lastFilteredColumn: string | undefined; + + // Loop through filtered columns to find which one contains the active element + for (const [colId] of Object.entries(filterModel)) { + const filterInstance = (await gridApi.getColumnFilterInstance( + colId, + )) as AGGridFilterInstance | null; + + if (!filterInstance) { + continue; + } + + if (filterInstance.eConditionBodies) { + const conditionBodies = filterInstance.eConditionBodies; + + // Check first condition body + if ( + conditionBodies[FILTER_CONDITION_BODY_INDEX.FIRST]?.contains( + activeElement, + ) + ) { + inputPosition = FILTER_INPUT_POSITIONS.FIRST; + lastFilteredColumn = colId; + break; + } + + // Check second condition body + if ( + conditionBodies[FILTER_CONDITION_BODY_INDEX.SECOND]?.contains( + activeElement, + ) + ) { + inputPosition = FILTER_INPUT_POSITIONS.SECOND; + lastFilteredColumn = colId; + break; + } + } + + if (filterInstance.eJoinAnds) { + for (const joinAnd of filterInstance.eJoinAnds) { + if (joinAnd.eGui?.contains(activeElement)) { + inputPosition = FILTER_INPUT_POSITIONS.FIRST; + lastFilteredColumn = colId; + break; + } + } + if (lastFilteredColumn) break; + } + + if (filterInstance.eJoinOrs) { + for (const joinOr of filterInstance.eJoinOrs) { + if (joinOr.eGui?.contains(activeElement)) { + inputPosition = FILTER_INPUT_POSITIONS.FIRST; + lastFilteredColumn = colId; + break; + } + } + if (lastFilteredColumn) break; + } + } + + return { lastFilteredColumn, inputPosition }; +} + +/** + * Gets complete filter state including SQL conversion and input position detection. + */ +export async function getCompleteFilterState( + gridRef: RefObject, + metricColumns: string[], +): Promise { + // Capture activeElement before any async operations to detect which input + // was focused when the user triggered the filter change + const activeElement = document.activeElement as HTMLElement; + + if (!gridRef.current?.api) { + return { + originalFilterModel: {}, + simpleFilters: [], + complexWhere: undefined, + havingClause: undefined, + lastFilteredColumn: undefined, + inputPosition: FILTER_INPUT_POSITIONS.UNKNOWN, + }; + } + + const filterModel = gridRef.current.api.getFilterModel(); + + // Convert filters to SQL + const convertedFilters = convertAgGridFiltersToSQL( + filterModel, + metricColumns, + ); + + // Detect which input was last modified + const { lastFilteredColumn, inputPosition } = await detectLastFilteredInput( + gridRef.current.api, + filterModel, + activeElement, + ); + + return { + originalFilterModel: filterModel, + simpleFilters: convertedFilters.simpleFilters, + complexWhere: convertedFilters.complexWhere, + havingClause: convertedFilters.havingClause, + lastFilteredColumn, + inputPosition, + }; +} diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getInitialFilterModel.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getInitialFilterModel.ts new file mode 100644 index 00000000000..bfe3c054be4 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/getInitialFilterModel.ts @@ -0,0 +1,42 @@ +/** + * 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 { isEmpty } from 'lodash'; +import type { AgGridChartState } from '@superset-ui/core'; + +const getInitialFilterModel = ( + chartState?: Partial, + serverPaginationData?: Record, + serverPagination?: boolean, +): Record | undefined => { + const chartStateFilterModel = + chartState?.filterModel && !isEmpty(chartState.filterModel) + ? (chartState.filterModel as Record) + : undefined; + + const serverFilterModel = + serverPagination && + serverPaginationData?.agGridFilterModel && + !isEmpty(serverPaginationData.agGridFilterModel) + ? (serverPaginationData.agGridFilterModel as Record) + : undefined; + + return chartStateFilterModel ?? serverFilterModel; +}; + +export default getInitialFilterModel; diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts index 89f652b1cf4..1879260d451 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts @@ -19,7 +19,7 @@ */ import { ColDef } from '@superset-ui/core/components/ThemedAgGridReact'; import { useCallback, useMemo } from 'react'; -import { DataRecord } from '@superset-ui/core'; +import { DataRecord, DataRecordValue } from '@superset-ui/core'; import { GenericDataType } from '@apache-superset/core/api/core'; import { ColorFormatters } from '@superset-ui/chart-controls'; import { extent as d3Extent, max as d3Max } from 'd3-array'; @@ -27,19 +27,22 @@ import { BasicColorFormatterType, CellRendererProps, InputColumn, + ValueRange, } from '../types'; import getCellClass from './getCellClass'; import filterValueGetter from './filterValueGetter'; import dateFilterComparator from './dateFilterComparator'; +import DateWithFormatter from './DateWithFormatter'; import { getAggFunc } from './getAggFunc'; import { TextCellRenderer } from '../renderers/TextCellRenderer'; import { NumericCellRenderer } from '../renderers/NumericCellRenderer'; import CustomHeader from '../AgGridTable/components/CustomHeader'; +import { NOOP_FILTER_COMPARATOR } from '../consts'; import { valueFormatter, valueGetter } from './formatValue'; import getCellStyle from './getCellStyle'; interface InputData { - [key: string]: any; + [key: string]: DataRecordValue; } type UseColDefsProps = { @@ -60,8 +63,6 @@ type UseColDefsProps = { slice_id: number; }; -type ValueRange = [number, number]; - function getValueRange( key: string, alignPositiveNegative: boolean, @@ -113,6 +114,73 @@ const getFilterType = (col: InputColumn) => { } }; +/** + * Filter value getter for temporal columns. + * Returns null for DateWithFormatter objects with null input, + * enabling AG Grid's blank filter to correctly identify null dates. + */ +const dateFilterValueGetter = (params: { + data: Record; + colDef: { field?: string }; +}) => { + const value = params.data?.[params.colDef.field as string]; + // Return null for DateWithFormatter with null input so AG Grid blank filter works + if (value instanceof DateWithFormatter && value.input === null) { + return null; + } + return value; +}; + +/** + * Custom date filter options for server-side pagination. + * Each option has a predicate that always returns true, allowing all rows to pass + * client-side filtering since the actual filtering is handled by the server. + */ +const SERVER_SIDE_DATE_FILTER_OPTIONS = [ + { + displayKey: 'serverEquals', + displayName: 'Equals', + predicate: () => true, + numberOfInputs: 1, + }, + { + displayKey: 'serverNotEqual', + displayName: 'Not Equal', + predicate: () => true, + numberOfInputs: 1, + }, + { + displayKey: 'serverBefore', + displayName: 'Before', + predicate: () => true, + numberOfInputs: 1, + }, + { + displayKey: 'serverAfter', + displayName: 'After', + predicate: () => true, + numberOfInputs: 1, + }, + { + displayKey: 'serverInRange', + displayName: 'In Range', + predicate: () => true, + numberOfInputs: 2, + }, + { + displayKey: 'serverBlank', + displayName: 'Blank', + predicate: () => true, + numberOfInputs: 0, + }, + { + displayKey: 'serverNotBlank', + displayName: 'Not blank', + predicate: () => true, + numberOfInputs: 0, + }, +]; + function getHeaderLabel(col: InputColumn) { let headerLabel: string | undefined; @@ -232,9 +300,16 @@ export const useColDefs = ({ filterValueGetter, }), ...(dataType === GenericDataType.Temporal && { - filterParams: { - comparator: dateFilterComparator, - }, + // Use dateFilterValueGetter so AG Grid correctly identifies null dates for blank filter + filterValueGetter: dateFilterValueGetter, + filterParams: serverPagination + ? { + filterOptions: SERVER_SIDE_DATE_FILTER_OPTIONS, + comparator: NOOP_FILTER_COMPARATOR, + } + : { + comparator: dateFilterComparator, + }, }), cellDataType: getCellDataType(col), defaultAggFunc: getAggFunc(col), diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts index 12647e99a5b..bb9411ca98b 100644 --- a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/buildQuery.test.ts @@ -476,6 +476,597 @@ describe('plugin-chart-ag-grid-table', () => { }); }); + describe('buildQuery - AG Grid server-side filters', () => { + describe('Simple filters', () => { + it('should apply agGridSimpleFilters to query.filters', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridSimpleFilters: [ + { col: 'state', op: '==', val: 'CA' }, + { col: 'city', op: 'ILIKE', val: '%San%' }, + ], + }, + }, + ).queries[0]; + + expect(query.filters).toContainEqual({ + col: 'state', + op: '==', + val: 'CA', + }); + expect(query.filters).toContainEqual({ + col: 'city', + op: 'ILIKE', + val: '%San%', + }); + }); + + it('should append simple filters to existing filters', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + adhoc_filters: [ + { + expressionType: 'SIMPLE', + subject: 'country', + operator: '==', + comparator: 'USA', + clause: 'WHERE', + }, + ], + }, + { + ownState: { + agGridSimpleFilters: [{ col: 'state', op: '==', val: 'CA' }], + }, + }, + ).queries[0]; + + expect(query.filters?.length).toBeGreaterThan(1); + expect(query.filters).toContainEqual({ + col: 'state', + op: '==', + val: 'CA', + }); + }); + + it('should handle empty agGridSimpleFilters array', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridSimpleFilters: [], + }, + }, + ).queries[0]; + + expect(query.filters).toBeDefined(); + }); + + it('should not apply simple filters when server pagination is disabled', () => { + const query = buildQuery(basicFormData, { + ownState: { + agGridSimpleFilters: [{ col: 'state', op: '==', val: 'CA' }], + }, + }).queries[0]; + + expect(query.filters).not.toContainEqual({ + col: 'state', + op: '==', + val: 'CA', + }); + }); + }); + + describe('Complex WHERE clause', () => { + it('should apply agGridComplexWhere to query.extras.where', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridComplexWhere: '(age > 18 AND age < 65)', + }, + }, + ).queries[0]; + + expect(query.extras?.where).toBe('(age > 18 AND age < 65)'); + }); + + it('should combine with existing WHERE clause using AND', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + adhoc_filters: [ + { + expressionType: 'SQL', + clause: 'WHERE', + sqlExpression: 'country = "USA"', + }, + ], + }, + { + ownState: { + agGridComplexWhere: '(age > 18 AND age < 65)', + }, + }, + ).queries[0]; + + expect(query.extras?.where).toContain('country = "USA"'); + expect(query.extras?.where).toContain('(age > 18 AND age < 65)'); + expect(query.extras?.where).toContain(' AND '); + }); + + it('should handle empty agGridComplexWhere', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridComplexWhere: '', + }, + }, + ).queries[0]; + + // Empty string should not set extras.where (undefined or empty string both acceptable) + expect(query.extras?.where || undefined).toBeUndefined(); + }); + + it('should not apply WHERE clause when server pagination is disabled', () => { + const query = buildQuery(basicFormData, { + ownState: { + agGridComplexWhere: '(age > 18)', + }, + }).queries[0]; + + // When server_pagination is disabled, AG Grid filters should not be applied + expect(query.extras?.where || undefined).toBeUndefined(); + }); + }); + + describe('HAVING clause', () => { + it('should apply agGridHavingClause to query.extras.having', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + metrics: ['SUM(revenue)'], + }, + { + ownState: { + agGridHavingClause: 'SUM(revenue) > 1000', + }, + }, + ).queries[0]; + + expect(query.extras?.having).toBe('SUM(revenue) > 1000'); + }); + + it('should combine with existing HAVING clause using AND', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + metrics: ['SUM(revenue)', 'COUNT(*)'], + adhoc_filters: [ + { + expressionType: 'SQL', + clause: 'HAVING', + sqlExpression: 'COUNT(*) > 10', + }, + ], + }, + { + ownState: { + agGridHavingClause: 'SUM(revenue) > 1000', + }, + }, + ).queries[0]; + + expect(query.extras?.having).toContain('COUNT(*) > 10'); + expect(query.extras?.having).toContain('SUM(revenue) > 1000'); + expect(query.extras?.having).toContain(' AND '); + }); + + it('should handle metric filters correctly', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + metrics: ['AVG(score)', 'MAX(points)'], + }, + { + ownState: { + agGridHavingClause: '(AVG(score) >= 90 AND MAX(points) < 100)', + }, + }, + ).queries[0]; + + expect(query.extras?.having).toBe( + '(AVG(score) >= 90 AND MAX(points) < 100)', + ); + }); + + it('should not apply HAVING clause when server pagination is disabled', () => { + const query = buildQuery(basicFormData, { + ownState: { + agGridHavingClause: 'SUM(revenue) > 1000', + }, + }).queries[0]; + + // When server_pagination is disabled, AG Grid filters should not be applied + expect(query.extras?.having || undefined).toBeUndefined(); + }); + }); + + describe('Totals query handling', () => { + it('should exclude AG Grid WHERE filters from totals query', () => { + const queries = buildQuery( + { + ...basicFormData, + server_pagination: true, + show_totals: true, + query_mode: QueryMode.Aggregate, + }, + { + ownState: { + agGridComplexWhere: 'age > 18', + }, + }, + ).queries; + + const mainQuery = queries[0]; + const totalsQuery = queries[2]; // queries[1] is rowcount, queries[2] is totals + + expect(mainQuery.extras?.where).toBe('age > 18'); + expect(totalsQuery.extras?.where).toBeUndefined(); + }); + + it('should preserve non-AG Grid WHERE clauses in totals', () => { + const queries = buildQuery( + { + ...basicFormData, + server_pagination: true, + show_totals: true, + query_mode: QueryMode.Aggregate, + adhoc_filters: [ + { + expressionType: 'SQL', + clause: 'WHERE', + sqlExpression: 'country = "USA"', + }, + ], + }, + { + ownState: { + agGridComplexWhere: 'age > 18', + }, + }, + ).queries; + + const mainQuery = queries[0]; + const totalsQuery = queries[2]; // queries[1] is rowcount, queries[2] is totals + + expect(mainQuery.extras?.where).toContain('country = "USA"'); + expect(mainQuery.extras?.where).toContain('age > 18'); + expect(totalsQuery.extras?.where).toContain('country = "USA"'); + expect(totalsQuery.extras?.where).not.toContain('age > 18'); + }); + + it('should handle totals when AG Grid WHERE is only clause', () => { + const queries = buildQuery( + { + ...basicFormData, + server_pagination: true, + show_totals: true, + query_mode: QueryMode.Aggregate, + }, + { + ownState: { + agGridComplexWhere: 'status = "active"', + }, + }, + ).queries; + + const totalsQuery = queries[2]; // queries[1] is rowcount, queries[2] is totals + + expect(totalsQuery.extras?.where).toBeUndefined(); + }); + + it('should handle totals with empty WHERE clause after removal', () => { + const queries = buildQuery( + { + ...basicFormData, + server_pagination: true, + show_totals: true, + query_mode: QueryMode.Aggregate, + adhoc_filters: [ + { + expressionType: 'SQL', + clause: 'WHERE', + sqlExpression: 'country = "USA"', + }, + ], + }, + { + ownState: { + agGridComplexWhere: 'country = "USA"', + }, + }, + ).queries; + + const totalsQuery = queries[2]; // queries[1] is rowcount, queries[2] is totals + + // After removing AG Grid WHERE, totals should still have the adhoc filter + expect(totalsQuery.extras).toBeDefined(); + }); + + it('should not modify totals query when no AG Grid filters applied', () => { + const queries = buildQuery( + { + ...basicFormData, + server_pagination: true, + show_totals: true, + query_mode: QueryMode.Aggregate, + }, + { + ownState: {}, + }, + ).queries; + + const totalsQuery = queries[2]; // queries[1] is rowcount, queries[2] is totals + + expect(totalsQuery.columns).toEqual([]); + expect(totalsQuery.row_limit).toBe(0); + }); + }); + + describe('Integration - all filter types together', () => { + it('should apply simple, WHERE, and HAVING filters simultaneously', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + metrics: ['SUM(revenue)', 'COUNT(*)'], + }, + { + ownState: { + agGridSimpleFilters: [{ col: 'state', op: '==', val: 'CA' }], + agGridComplexWhere: '(age > 18 AND age < 65)', + agGridHavingClause: 'SUM(revenue) > 1000', + }, + }, + ).queries[0]; + + expect(query.filters).toContainEqual({ + col: 'state', + op: '==', + val: 'CA', + }); + expect(query.extras?.where).toBe('(age > 18 AND age < 65)'); + expect(query.extras?.having).toBe('SUM(revenue) > 1000'); + }); + + it('should combine AG Grid filters with adhoc filters', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + adhoc_filters: [ + { + expressionType: 'SIMPLE', + subject: 'country', + operator: '==', + comparator: 'USA', + clause: 'WHERE', + }, + { + expressionType: 'SQL', + clause: 'WHERE', + sqlExpression: 'region = "West"', + }, + ], + }, + { + ownState: { + agGridSimpleFilters: [ + { col: 'state', op: 'IN', val: ['CA', 'OR', 'WA'] }, + ], + agGridComplexWhere: "status = 'active'", + }, + }, + ).queries[0]; + + expect(query.filters).toContainEqual({ + col: 'state', + op: 'IN', + val: ['CA', 'OR', 'WA'], + }); + expect(query.extras?.where).toContain('region = "West"'); + expect(query.extras?.where).toContain("status = 'active'"); + }); + + it('should reset currentPage to 0 when filtering', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + currentPage: 5, + agGridSimpleFilters: [{ col: 'state', op: '==', val: 'CA' }], + }, + }, + ).queries[0]; + + // The query itself doesn't have page info, but ownState should be updated + expect(query.filters).toContainEqual({ + col: 'state', + op: '==', + val: 'CA', + }); + }); + + it('should include filter metadata in ownState', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridFilterModel: { + state: { filterType: 'text', type: 'equals', filter: 'CA' }, + }, + lastFilteredColumn: 'state', + lastFilteredInputPosition: 'first', + }, + }, + ).queries[0]; + + // Query should be generated with the filter model metadata + expect(query).toBeDefined(); + }); + + it('should handle complex real-world scenario', () => { + const queries = buildQuery( + { + ...basicFormData, + server_pagination: true, + show_totals: true, + query_mode: QueryMode.Aggregate, + groupby: ['state', 'city'], + metrics: ['SUM(revenue)', 'AVG(score)', 'COUNT(*)'], + adhoc_filters: [ + { + expressionType: 'SIMPLE', + subject: 'country', + operator: '==', + comparator: 'USA', + clause: 'WHERE', + }, + ], + }, + { + ownState: { + agGridSimpleFilters: [ + { col: 'state', op: 'IN', val: ['CA', 'NY', 'TX'] }, + ], + agGridComplexWhere: + '(population > 100000 AND growth_rate > 0.05)', + agGridHavingClause: + '(SUM(revenue) > 1000000 AND AVG(score) >= 4.5)', + currentPage: 0, + pageSize: 50, + }, + }, + ).queries; + + const mainQuery = queries[0]; + const totalsQuery = queries[2]; // queries[1] is rowcount, queries[2] is totals + + // Main query should have all filters + expect(mainQuery.filters).toContainEqual({ + col: 'state', + op: 'IN', + val: ['CA', 'NY', 'TX'], + }); + expect(mainQuery.extras?.where).toContain('population > 100000'); + expect(mainQuery.extras?.having).toContain('SUM(revenue) > 1000000'); + + // Totals query should exclude AG Grid WHERE (since it's the only WHERE clause, it should be undefined) + expect(totalsQuery.extras?.where).toBeUndefined(); + }); + }); + + describe('Edge cases', () => { + it('should handle null ownState gracefully', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + {}, + ).queries[0]; + + expect(query).toBeDefined(); + }); + + it('should handle ownState without filter properties', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + currentPage: 0, + pageSize: 20, + }, + }, + ).queries[0]; + + expect(query).toBeDefined(); + }); + + it('should handle filters with special SQL characters', () => { + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridSimpleFilters: [{ col: 'name', op: '==', val: "O'Brien" }], + }, + }, + ).queries[0]; + + expect(query.filters).toContainEqual({ + col: 'name', + op: '==', + val: "O'Brien", + }); + }); + + it('should handle very long filter clauses', () => { + const longWhereClause = Array(50) + .fill(0) + .map((_, i) => `field${i} > ${i}`) + .join(' AND '); + + const query = buildQuery( + { + ...basicFormData, + server_pagination: true, + }, + { + ownState: { + agGridComplexWhere: longWhereClause, + }, + }, + ).queries[0]; + + expect(query.extras?.where).toBe(longWhereClause); + }); + }); + }); + describe('buildQuery - metrics handling in different query modes', () => { test('should not include metrics in raw records mode', () => { const query = buildQuery({ diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts new file mode 100644 index 00000000000..b00e3f2f138 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/agGridFilterConverter.test.ts @@ -0,0 +1,863 @@ +/** + * 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 { convertAgGridFiltersToSQL } from '../../src/utils/agGridFilterConverter'; +import type { + AgGridFilterModel, + AgGridSimpleFilter, + AgGridCompoundFilter, + AgGridSetFilter, +} from '../../src/utils/agGridFilterConverter'; + +describe('agGridFilterConverter', () => { + describe('Empty and invalid inputs', () => { + it('should handle empty filter model', () => { + const result = convertAgGridFiltersToSQL({}); + + expect(result.simpleFilters).toEqual([]); + expect(result.complexWhere).toBeUndefined(); + expect(result.havingClause).toBeUndefined(); + }); + + it('should handle null filter model', () => { + const result = convertAgGridFiltersToSQL(null as any); + + expect(result.simpleFilters).toEqual([]); + expect(result.complexWhere).toBeUndefined(); + expect(result.havingClause).toBeUndefined(); + }); + + it('should skip invalid column names', () => { + const filterModel: AgGridFilterModel = { + valid_column: { + filterType: 'text', + type: 'equals', + filter: 'test', + }, + 'invalid; DROP TABLE users--': { + filterType: 'text', + type: 'equals', + filter: 'malicious', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0].col).toBe('valid_column'); + }); + + it('should skip filters with invalid objects', () => { + const filterModel = { + column1: null, + column2: 'invalid string', + column3: { + filterType: 'text', + type: 'equals', + filter: 'valid', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel as any); + + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0].col).toBe('column3'); + }); + }); + + describe('Simple text filters', () => { + it('should convert equals filter', () => { + const filterModel: AgGridFilterModel = { + name: { + filterType: 'text', + type: 'equals', + filter: 'John', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0]).toEqual({ + col: 'name', + op: '=', + val: 'John', + }); + }); + + it('should convert notEqual filter', () => { + const filterModel: AgGridFilterModel = { + status: { + filterType: 'text', + type: 'notEqual', + filter: 'inactive', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'status', + op: '!=', + val: 'inactive', + }); + }); + + it('should convert contains filter with wildcard', () => { + const filterModel: AgGridFilterModel = { + description: { + filterType: 'text', + type: 'contains', + filter: 'urgent', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'description', + op: 'ILIKE', + val: '%urgent%', + }); + }); + + it('should convert notContains filter with wildcard', () => { + const filterModel: AgGridFilterModel = { + description: { + filterType: 'text', + type: 'notContains', + filter: 'spam', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'description', + op: 'NOT ILIKE', + val: '%spam%', + }); + }); + + it('should convert startsWith filter with trailing wildcard', () => { + const filterModel: AgGridFilterModel = { + email: { + filterType: 'text', + type: 'startsWith', + filter: 'admin', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'email', + op: 'ILIKE', + val: 'admin%', + }); + }); + + it('should convert endsWith filter with leading wildcard', () => { + const filterModel: AgGridFilterModel = { + email: { + filterType: 'text', + type: 'endsWith', + filter: '@example.com', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'email', + op: 'ILIKE', + val: '%@example.com', + }); + }); + }); + + describe('Numeric filters', () => { + it('should convert lessThan filter', () => { + const filterModel: AgGridFilterModel = { + age: { + filterType: 'number', + type: 'lessThan', + filter: 30, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'age', + op: '<', + val: 30, + }); + }); + + it('should convert lessThanOrEqual filter', () => { + const filterModel: AgGridFilterModel = { + price: { + filterType: 'number', + type: 'lessThanOrEqual', + filter: 100, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'price', + op: '<=', + val: 100, + }); + }); + + it('should convert greaterThan filter', () => { + const filterModel: AgGridFilterModel = { + score: { + filterType: 'number', + type: 'greaterThan', + filter: 50, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'score', + op: '>', + val: 50, + }); + }); + + it('should convert greaterThanOrEqual filter', () => { + const filterModel: AgGridFilterModel = { + rating: { + filterType: 'number', + type: 'greaterThanOrEqual', + filter: 4, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'rating', + op: '>=', + val: 4, + }); + }); + + it('should convert inRange filter to BETWEEN', () => { + const filterModel: AgGridFilterModel = { + age: { + filterType: 'number', + type: 'inRange', + filter: 18, + filterTo: 65, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + // inRange creates a simple filter with BETWEEN operator + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0]).toEqual({ + col: 'age', + op: 'BETWEEN', + val: 18, + }); + }); + }); + + describe('Null/blank filters', () => { + it('should convert blank filter to IS NULL', () => { + const filterModel: AgGridFilterModel = { + optional_field: { + filterType: 'text', + type: 'blank', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'optional_field', + op: 'IS NULL', + val: null, + }); + }); + + it('should convert notBlank filter to IS NOT NULL', () => { + const filterModel: AgGridFilterModel = { + required_field: { + filterType: 'text', + type: 'notBlank', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'required_field', + op: 'IS NOT NULL', + val: null, + }); + }); + }); + + describe('Set filters', () => { + it('should convert set filter to IN operator', () => { + const filterModel: AgGridFilterModel = { + status: { + filterType: 'set', + values: ['active', 'pending', 'approved'], + } as AgGridSetFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'status', + op: 'IN', + val: ['active', 'pending', 'approved'], + }); + }); + + it('should handle set filter with numeric values', () => { + const filterModel: AgGridFilterModel = { + priority: { + filterType: 'set', + values: [1, 2, 3], + } as AgGridSetFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'priority', + op: 'IN', + val: [1, 2, 3], + }); + }); + + it('should skip empty set filters', () => { + const filterModel: AgGridFilterModel = { + status: { + filterType: 'set', + values: [], + } as AgGridSetFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(0); + }); + }); + + describe('Compound filters', () => { + it('should combine conditions with AND operator', () => { + const filterModel: AgGridFilterModel = { + age: { + filterType: 'number', + operator: 'AND', + condition1: { + filterType: 'number', + type: 'greaterThanOrEqual', + filter: 18, + }, + condition2: { + filterType: 'number', + type: 'lessThan', + filter: 65, + }, + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.complexWhere).toBe('(age >= 18 AND age < 65)'); + }); + + it('should combine conditions with OR operator', () => { + const filterModel: AgGridFilterModel = { + status: { + filterType: 'text', + operator: 'OR', + condition1: { + filterType: 'text', + type: 'equals', + filter: 'urgent', + }, + condition2: { + filterType: 'text', + type: 'equals', + filter: 'critical', + }, + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.complexWhere).toBe( + "(status = 'urgent' OR status = 'critical')", + ); + }); + + it('should handle compound filter with inRange', () => { + const filterModel: AgGridFilterModel = { + date: { + filterType: 'date', + operator: 'AND', + condition1: { + filterType: 'date', + type: 'inRange', + filter: '2024-01-01', + filterTo: '2024-12-31', + }, + condition2: { + filterType: 'date', + type: 'notBlank', + }, + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.complexWhere).toContain('BETWEEN'); + expect(result.complexWhere).toContain('IS NOT NULL'); + }); + + it('should handle compound filter with invalid conditions gracefully', () => { + const filterModel: AgGridFilterModel = { + field: { + filterType: 'text', + operator: 'AND', + condition1: { + filterType: 'text', + type: 'equals', + filter: 'valid', + }, + condition2: { + filterType: 'text', + type: 'equals', + // Missing filter value - should be skipped + } as AgGridSimpleFilter, + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + // Should only include valid condition + expect(result.complexWhere).toBe("field = 'valid'"); + }); + + it('should handle multi-condition filters (conditions array)', () => { + const filterModel: AgGridFilterModel = { + category: { + filterType: 'text', + operator: 'OR', + conditions: [ + { + filterType: 'text', + type: 'equals', + filter: 'A', + }, + { + filterType: 'text', + type: 'equals', + filter: 'B', + }, + { + filterType: 'text', + type: 'equals', + filter: 'C', + }, + ], + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.complexWhere).toBe( + "(category = 'A' OR category = 'B' OR category = 'C')", + ); + }); + }); + + describe('Metric vs Dimension separation', () => { + it('should put dimension filters in simpleFilters/complexWhere', () => { + const filterModel: AgGridFilterModel = { + state: { + filterType: 'text', + type: 'equals', + filter: 'CA', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel, []); + + expect(result.simpleFilters).toHaveLength(1); + expect(result.havingClause).toBeUndefined(); + }); + + it('should put metric filters in havingClause', () => { + const filterModel: AgGridFilterModel = { + 'SUM(revenue)': { + filterType: 'number', + type: 'greaterThan', + filter: 1000, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel, ['SUM(revenue)']); + + expect(result.simpleFilters).toHaveLength(0); + expect(result.havingClause).toBe('SUM(revenue) > 1000'); + }); + + it('should separate mixed metric and dimension filters', () => { + const filterModel: AgGridFilterModel = { + state: { + filterType: 'text', + type: 'equals', + filter: 'CA', + }, + 'SUM(revenue)': { + filterType: 'number', + type: 'greaterThan', + filter: 1000, + }, + city: { + filterType: 'text', + type: 'startsWith', + filter: 'San', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel, ['SUM(revenue)']); + + expect(result.simpleFilters).toHaveLength(2); + expect(result.simpleFilters[0].col).toBe('state'); + expect(result.simpleFilters[1].col).toBe('city'); + expect(result.havingClause).toBe('SUM(revenue) > 1000'); + }); + + it('should handle metric set filters in HAVING clause', () => { + const filterModel: AgGridFilterModel = { + 'AVG(score)': { + filterType: 'set', + values: [90, 95, 100], + } as AgGridSetFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel, ['AVG(score)']); + + expect(result.simpleFilters).toHaveLength(0); + expect(result.havingClause).toBe('AVG(score) IN (90, 95, 100)'); + }); + + it('should handle metric blank filters in HAVING clause', () => { + const filterModel: AgGridFilterModel = { + 'COUNT(*)': { + filterType: 'number', + type: 'blank', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel, ['COUNT(*)']); + + expect(result.havingClause).toBe('COUNT(*) IS NULL'); + }); + }); + + describe('Multiple filters combination', () => { + it('should handle both simple and compound filters', () => { + const filterModel: AgGridFilterModel = { + status: { + filterType: 'text', + type: 'equals', + filter: 'active', + }, + age: { + filterType: 'number', + operator: 'AND', + condition1: { + filterType: 'number', + type: 'greaterThan', + filter: 18, + }, + condition2: { + filterType: 'number', + type: 'lessThan', + filter: 65, + }, + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + // Simple filter goes to simpleFilters + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0]).toEqual({ + col: 'status', + op: '=', + val: 'active', + }); + + // Compound filter goes to complexWhere + expect(result.complexWhere).toBe('(age > 18 AND age < 65)'); + }); + + it('should combine multiple HAVING filters with AND', () => { + const filterModel: AgGridFilterModel = { + 'SUM(revenue)': { + filterType: 'number', + type: 'greaterThan', + filter: 1000, + }, + 'AVG(score)': { + filterType: 'number', + type: 'greaterThanOrEqual', + filter: 90, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel, [ + 'SUM(revenue)', + 'AVG(score)', + ]); + + expect(result.havingClause).toBe( + '(SUM(revenue) > 1000 AND AVG(score) >= 90)', + ); + }); + + it('should handle single WHERE filter without parentheses', () => { + const filterModel: AgGridFilterModel = { + age: { + filterType: 'number', + operator: 'AND', + condition1: { + filterType: 'number', + type: 'greaterThan', + filter: 18, + }, + condition2: { + filterType: 'number', + type: 'lessThan', + filter: 65, + }, + } as AgGridCompoundFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.complexWhere).toBe('(age > 18 AND age < 65)'); + }); + }); + + describe('SQL injection prevention', () => { + it('should escape single quotes in filter values', () => { + const filterModel: AgGridFilterModel = { + name: { + filterType: 'text', + type: 'equals', + filter: "O'Brien", + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0].val).toBe("O'Brien"); + // The actual escaping happens in SQL generation, but value is preserved + }); + + it('should escape single quotes in complex filters', () => { + const filterModel: AgGridFilterModel = { + description: { + filterType: 'text', + type: 'contains', + filter: "It's working", + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + // For ILIKE filters, wildcards are added but value preserved + expect(result.simpleFilters[0].val).toBe("%It's working%"); + }); + + it('should reject column names with SQL injection attempts', () => { + const filterModel: AgGridFilterModel = { + "name'; DROP TABLE users--": { + filterType: 'text', + type: 'equals', + filter: 'test', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(0); + }); + + it('should reject column names with special characters', () => { + const filterModel: AgGridFilterModel = { + 'column': { + filterType: 'text', + type: 'equals', + filter: 'test', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(0); + }); + + it('should accept valid column names with allowed special characters', () => { + const filterModel: AgGridFilterModel = { + valid_column_123: { + filterType: 'text', + type: 'equals', + filter: 'test', + }, + 'Column Name With Spaces': { + filterType: 'text', + type: 'equals', + filter: 'test2', + }, + 'SUM(revenue)': { + filterType: 'number', + type: 'greaterThan', + filter: 100, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(3); + }); + + it('should handle very long column names', () => { + const longColumnName = 'a'.repeat(300); + const filterModel: AgGridFilterModel = { + [longColumnName]: { + filterType: 'text', + type: 'equals', + filter: 'test', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + // Should reject column names longer than 255 characters + expect(result.simpleFilters).toHaveLength(0); + }); + }); + + describe('Edge cases', () => { + it('should skip filters with missing type', () => { + const filterModel: AgGridFilterModel = { + column: { + filterType: 'text', + filter: 'value', + } as any, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(0); + }); + + it('should skip filters with unknown operator type', () => { + const filterModel: AgGridFilterModel = { + column: { + filterType: 'text', + type: 'unknownOperator' as any, + filter: 'value', + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(0); + }); + + it('should skip filters with invalid value types', () => { + const filterModel: AgGridFilterModel = { + column: { + filterType: 'text', + type: 'equals', + filter: { invalid: 'object' } as any, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters).toHaveLength(0); + }); + + it('should handle boolean filter values', () => { + const filterModel: AgGridFilterModel = { + is_active: { + filterType: 'boolean', + type: 'equals', + filter: true, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0]).toEqual({ + col: 'is_active', + op: '=', + val: true, + }); + }); + + it('should handle null filter values for blank operators', () => { + const filterModel: AgGridFilterModel = { + field: { + filterType: 'text', + type: 'blank', + filter: null, + }, + }; + + const result = convertAgGridFiltersToSQL(filterModel); + + expect(result.simpleFilters[0].val).toBeNull(); + }); + + it('should handle metric filters with set filter', () => { + const filterModel: AgGridFilterModel = { + 'SUM(amount)': { + filterType: 'set', + values: ['100', '200', '300'], + } as AgGridSetFilter, + }; + + const result = convertAgGridFiltersToSQL(filterModel, ['SUM(amount)']); + + expect(result.havingClause).toBe("SUM(amount) IN ('100', '200', '300')"); + }); + }); +}); diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/filterStateManager.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/filterStateManager.test.ts new file mode 100644 index 00000000000..a15e3f1e2f9 --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/filterStateManager.test.ts @@ -0,0 +1,658 @@ +/** + * 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 { getCompleteFilterState } from '../../src/utils/filterStateManager'; +import type { RefObject } from 'react'; +import type { AgGridReact } from '@superset-ui/core/components/ThemedAgGridReact'; +import { FILTER_INPUT_POSITIONS } from '../../src/consts'; + +describe('filterStateManager', () => { + describe('getCompleteFilterState', () => { + it('should return empty state when gridRef.current is null', async () => { + const gridRef = { current: null } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result).toEqual({ + originalFilterModel: {}, + simpleFilters: [], + complexWhere: undefined, + havingClause: undefined, + lastFilteredColumn: undefined, + inputPosition: FILTER_INPUT_POSITIONS.UNKNOWN, + }); + }); + + it('should return empty state when gridRef.current.api is undefined', async () => { + const gridRef = { + current: { api: undefined } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result).toEqual({ + originalFilterModel: {}, + simpleFilters: [], + complexWhere: undefined, + havingClause: undefined, + lastFilteredColumn: undefined, + inputPosition: FILTER_INPUT_POSITIONS.UNKNOWN, + }); + }); + + it('should convert simple filters correctly', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'John' }, + age: { filterType: 'number', type: 'greaterThan', filter: 25 }, + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.originalFilterModel).toEqual(filterModel); + expect(result.simpleFilters).toHaveLength(2); + expect(result.simpleFilters[0]).toEqual({ + col: 'name', + op: '=', + val: 'John', + }); + expect(result.simpleFilters[1]).toEqual({ + col: 'age', + op: '>', + val: 25, + }); + }); + + it('should separate dimension and metric filters', async () => { + const filterModel = { + state: { filterType: 'text', type: 'equals', filter: 'CA' }, + 'SUM(revenue)': { + filterType: 'number', + type: 'greaterThan', + filter: 1000, + }, + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, ['SUM(revenue)']); + + // Dimension filter goes to simpleFilters + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0].col).toBe('state'); + + // Metric filter goes to havingClause + expect(result.havingClause).toBe('SUM(revenue) > 1000'); + }); + + it('should detect first input position when active element is in first condition body', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }; + + const mockInput = document.createElement('input'); + const mockConditionBody1 = document.createElement('div'); + mockConditionBody1.appendChild(mockInput); + + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [mockConditionBody1], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + // Mock activeElement + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('name'); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.FIRST); + }); + + it('should detect second input position when active element is in second condition body', async () => { + const filterModel = { + age: { + filterType: 'number', + operator: 'AND', + condition1: { filterType: 'number', type: 'greaterThan', filter: 18 }, + condition2: { filterType: 'number', type: 'lessThan', filter: 65 }, + }, + }; + + const mockInput = document.createElement('input'); + const mockConditionBody1 = document.createElement('div'); + const mockConditionBody2 = document.createElement('div'); + mockConditionBody2.appendChild(mockInput); + + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [mockConditionBody1, mockConditionBody2], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + // Mock activeElement + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('age'); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.SECOND); + }); + + it('should return unknown position when active element is not in any condition body', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }; + + const mockConditionBody = document.createElement('div'); + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [mockConditionBody], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + // Mock activeElement as something outside the filter + const outsideElement = document.createElement('div'); + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: outsideElement, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.UNKNOWN); + expect(result.lastFilteredColumn).toBeUndefined(); + }); + + it('should handle multiple filtered columns and detect the correct one', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'John' }, + age: { filterType: 'number', type: 'greaterThan', filter: 25 }, + status: { filterType: 'text', type: 'equals', filter: 'active' }, + }; + + const mockInput = document.createElement('input'); + const mockConditionBodyAge = document.createElement('div'); + mockConditionBodyAge.appendChild(mockInput); + + const mockFilterInstanceName = { + eGui: document.createElement('div'), + eConditionBodies: [document.createElement('div')], + }; + + const mockFilterInstanceAge = { + eGui: document.createElement('div'), + eConditionBodies: [mockConditionBodyAge], + }; + + const mockFilterInstanceStatus = { + eGui: document.createElement('div'), + eConditionBodies: [document.createElement('div')], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn((colId: string) => { + if (colId === 'name') return Promise.resolve(mockFilterInstanceName); + if (colId === 'age') return Promise.resolve(mockFilterInstanceAge); + if (colId === 'status') + return Promise.resolve(mockFilterInstanceStatus); + return Promise.resolve(null); + }), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + // Mock activeElement in age filter + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('age'); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.FIRST); + }); + + it('should handle filter instance without eConditionBodies', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }; + + const mockFilterInstance = { + eGui: document.createElement('div'), + // No eConditionBodies property + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.UNKNOWN); + expect(result.lastFilteredColumn).toBeUndefined(); + }); + + it('should handle empty filter model', async () => { + const filterModel = {}; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.originalFilterModel).toEqual({}); + expect(result.simpleFilters).toEqual([]); + expect(result.complexWhere).toBeUndefined(); + expect(result.havingClause).toBeUndefined(); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.UNKNOWN); + }); + + it('should handle compound filters correctly', async () => { + const filterModel = { + age: { + filterType: 'number', + operator: 'AND', + condition1: { + filterType: 'number', + type: 'greaterThanOrEqual', + filter: 18, + }, + condition2: { filterType: 'number', type: 'lessThan', filter: 65 }, + }, + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.complexWhere).toBe('(age >= 18 AND age < 65)'); + }); + + it('should handle set filters correctly', async () => { + const filterModel = { + status: { + filterType: 'set', + values: ['active', 'pending', 'approved'], + }, + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.simpleFilters).toHaveLength(1); + expect(result.simpleFilters[0]).toEqual({ + col: 'status', + op: 'IN', + val: ['active', 'pending', 'approved'], + }); + }); + + it('should break detection loop after finding active element', async () => { + const filterModel = { + col1: { filterType: 'text', type: 'equals', filter: 'a' }, + col2: { filterType: 'text', type: 'equals', filter: 'b' }, + col3: { filterType: 'text', type: 'equals', filter: 'c' }, + }; + + const mockInput = document.createElement('input'); + const mockConditionBody = document.createElement('div'); + mockConditionBody.appendChild(mockInput); + + let callCount = 0; + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn((colId: string) => { + callCount++; + // Return match on col2 + if (colId === 'col2') { + return Promise.resolve({ + eGui: document.createElement('div'), + eConditionBodies: [mockConditionBody], + }); + } + return Promise.resolve({ + eGui: document.createElement('div'), + eConditionBodies: [document.createElement('div')], + }); + }), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('col2'); + // Should not call getColumnFilterInstance for col3 after finding match + expect(callCount).toBeLessThanOrEqual(2); + }); + + it('should handle null filter instance gracefully', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.UNKNOWN); + expect(result.originalFilterModel).toEqual(filterModel); + }); + + it('should maintain filter model reference integrity', async () => { + const originalFilterModel = { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }; + + const mockApi = { + getFilterModel: jest.fn(() => originalFilterModel), + getColumnFilterInstance: jest.fn(() => Promise.resolve(null)), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + const result = await getCompleteFilterState(gridRef, []); + + // Should return the same reference + expect(result.originalFilterModel).toBe(originalFilterModel); + }); + + it('should detect active element in eJoinAnds array', async () => { + const filterModel = { + age: { + filterType: 'number', + operator: 'AND', + condition1: { filterType: 'number', type: 'greaterThan', filter: 18 }, + condition2: { filterType: 'number', type: 'lessThan', filter: 65 }, + }, + }; + + const mockInput = document.createElement('input'); + const mockJoinAndGui = document.createElement('div'); + mockJoinAndGui.appendChild(mockInput); + + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [ + document.createElement('div'), + document.createElement('div'), + ], + eJoinAnds: [{ eGui: mockJoinAndGui }], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + // Mock activeElement in join AND operator + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('age'); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.FIRST); + }); + + it('should detect active element in eJoinOrs array', async () => { + const filterModel = { + status: { + filterType: 'text', + operator: 'OR', + condition1: { filterType: 'text', type: 'equals', filter: 'active' }, + condition2: { filterType: 'text', type: 'equals', filter: 'pending' }, + }, + }; + + const mockInput = document.createElement('input'); + const mockJoinOrGui = document.createElement('div'); + mockJoinOrGui.appendChild(mockInput); + + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [ + document.createElement('div'), + document.createElement('div'), + ], + eJoinOrs: [{ eGui: mockJoinOrGui }], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + // Mock activeElement in join OR operator + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('status'); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.FIRST); + }); + + it('should check condition bodies before join operators', async () => { + const filterModel = { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }; + + const mockInput = document.createElement('input'); + const mockConditionBody2 = document.createElement('div'); + mockConditionBody2.appendChild(mockInput); + + const mockJoinAndGui = document.createElement('div'); + // Input is NOT in join operator, only in condition body + + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [document.createElement('div'), mockConditionBody2], + eJoinAnds: [{ eGui: mockJoinAndGui }], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('name'); + // Should detect SECOND input position, not from join operator + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.SECOND); + }); + + it('should handle multiple eJoinAnds elements', async () => { + const filterModel = { + score: { filterType: 'number', type: 'greaterThan', filter: 90 }, + }; + + const mockInput = document.createElement('input'); + const mockJoinAndGui2 = document.createElement('div'); + mockJoinAndGui2.appendChild(mockInput); + + const mockFilterInstance = { + eGui: document.createElement('div'), + eConditionBodies: [document.createElement('div')], + eJoinAnds: [ + { eGui: document.createElement('div') }, + { eGui: mockJoinAndGui2 }, + { eGui: document.createElement('div') }, + ], + }; + + const mockApi = { + getFilterModel: jest.fn(() => filterModel), + getColumnFilterInstance: jest.fn(() => + Promise.resolve(mockFilterInstance), + ), + }; + + const gridRef = { + current: { api: mockApi } as any, + } as RefObject; + + Object.defineProperty(document, 'activeElement', { + writable: true, + configurable: true, + value: mockInput, + }); + + const result = await getCompleteFilterState(gridRef, []); + + expect(result.lastFilteredColumn).toBe('score'); + expect(result.inputPosition).toBe(FILTER_INPUT_POSITIONS.FIRST); + }); + }); +}); diff --git a/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getInitialFilterModel.test.ts b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getInitialFilterModel.test.ts new file mode 100644 index 00000000000..5a8822c2bed --- /dev/null +++ b/superset-frontend/plugins/plugin-chart-ag-grid-table/test/utils/getInitialFilterModel.test.ts @@ -0,0 +1,412 @@ +/** + * 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 getInitialFilterModel from '../../src/utils/getInitialFilterModel'; +import type { AgGridChartState } from '@superset-ui/core'; + +describe('getInitialFilterModel', () => { + describe('Priority: chartState > serverPaginationData', () => { + it('should prioritize chartState.filterModel over serverPaginationData', () => { + const chartState: Partial = { + filterModel: { + name: { + filterType: 'text', + type: 'equals', + filter: 'from-chart-state', + }, + }, + }; + + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'from-server' }, + }, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toEqual(chartState.filterModel); + }); + + it('should use serverPaginationData when chartState.filterModel is unavailable', () => { + const chartState: Partial = {}; + + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'from-server' }, + }, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toEqual(serverPaginationData.agGridFilterModel); + }); + + it('should use serverPaginationData when chartState is undefined', () => { + const serverPaginationData = { + agGridFilterModel: { + status: { filterType: 'text', type: 'equals', filter: 'active' }, + }, + }; + + const result = getInitialFilterModel( + undefined, + serverPaginationData, + true, + ); + + expect(result).toEqual(serverPaginationData.agGridFilterModel); + }); + }); + + describe('Empty object handling', () => { + it('should return undefined when chartState.filterModel is empty object', () => { + const chartState: Partial = { + filterModel: {}, + }; + + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + // Empty filterModel should be ignored, fall back to server + expect(result).toEqual(serverPaginationData.agGridFilterModel); + }); + + it('should return undefined when serverPaginationData.agGridFilterModel is empty object', () => { + const chartState: Partial = {}; + + const serverPaginationData = { + agGridFilterModel: {}, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toBeUndefined(); + }); + + it('should handle both being empty objects', () => { + const chartState: Partial = { + filterModel: {}, + }; + + const serverPaginationData = { + agGridFilterModel: {}, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toBeUndefined(); + }); + }); + + describe('Undefined/null handling', () => { + it('should return undefined when all inputs are undefined', () => { + const result = getInitialFilterModel(undefined, undefined, true); + + expect(result).toBeUndefined(); + }); + + it('should return undefined when chartState and serverPaginationData are undefined', () => { + const result = getInitialFilterModel(undefined, undefined, false); + + expect(result).toBeUndefined(); + }); + + it('should return undefined when serverPagination is disabled', () => { + const chartState: Partial = {}; + + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + false, + ); + + expect(result).toBeUndefined(); + }); + + it('should use chartState even when serverPagination is disabled', () => { + const chartState: Partial = { + filterModel: { + name: { + filterType: 'text', + type: 'equals', + filter: 'from-chart-state', + }, + }, + }; + + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'from-server' }, + }, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + false, + ); + + // chartState takes priority regardless of serverPagination flag + expect(result).toEqual(chartState.filterModel); + }); + }); + + describe('Complex filter models', () => { + it('should handle complex chartState filter model', () => { + const chartState: Partial = { + filterModel: { + name: { filterType: 'text', type: 'equals', filter: 'John' }, + age: { + filterType: 'number', + operator: 'AND', + condition1: { + filterType: 'number', + type: 'greaterThan', + filter: 18, + }, + condition2: { filterType: 'number', type: 'lessThan', filter: 65 }, + }, + status: { filterType: 'set', values: ['active', 'pending'] }, + }, + }; + + const result = getInitialFilterModel(chartState, undefined, true); + + expect(result).toEqual(chartState.filterModel); + }); + + it('should handle complex serverPaginationData filter model', () => { + const serverPaginationData = { + agGridFilterModel: { + category: { filterType: 'text', type: 'contains', filter: 'tech' }, + revenue: { filterType: 'number', type: 'greaterThan', filter: 1000 }, + }, + }; + + const result = getInitialFilterModel( + undefined, + serverPaginationData, + true, + ); + + expect(result).toEqual(serverPaginationData.agGridFilterModel); + }); + }); + + describe('Real-world scenarios', () => { + it('should handle permalink scenario with chartState', () => { + // User shares a permalink with saved filter state + const chartState: Partial = { + filterModel: { + state: { filterType: 'set', values: ['CA', 'NY', 'TX'] }, + revenue: { filterType: 'number', type: 'greaterThan', filter: 50000 }, + }, + columnState: [], + }; + + const result = getInitialFilterModel(chartState, undefined, true); + + expect(result).toEqual(chartState.filterModel); + expect(result?.state).toBeDefined(); + expect(result?.revenue).toBeDefined(); + }); + + it('should handle fresh page load with server state', () => { + // Fresh page load - no chartState, but has serverPaginationData from ownState + const serverPaginationData = { + agGridFilterModel: { + created_date: { + filterType: 'date', + type: 'greaterThan', + filter: '2024-01-01', + }, + }, + currentPage: 0, + pageSize: 50, + }; + + const result = getInitialFilterModel( + undefined, + serverPaginationData, + true, + ); + + expect(result).toEqual(serverPaginationData.agGridFilterModel); + }); + + it('should handle chart without any filters applied', () => { + // No filters applied anywhere + const chartState: Partial = { + columnState: [], + }; + + const serverPaginationData = { + currentPage: 0, + pageSize: 20, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toBeUndefined(); + }); + + it('should handle transition from no filters to filters via permalink', () => { + // User applies filters, creates permalink, then loads it + const chartState: Partial = { + filterModel: { + name: { filterType: 'text', type: 'startsWith', filter: 'Admin' }, + }, + }; + + const serverPaginationData = { + agGridFilterModel: undefined, // No server state yet + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toEqual(chartState.filterModel); + }); + }); + + describe('Edge cases', () => { + it('should handle null values in serverPaginationData', () => { + const serverPaginationData = { + agGridFilterModel: null as any, + }; + + const result = getInitialFilterModel( + undefined, + serverPaginationData, + true, + ); + + expect(result).toBeUndefined(); + }); + + it('should handle serverPaginationData without agGridFilterModel key', () => { + const serverPaginationData = { + currentPage: 0, + pageSize: 20, + }; + + const result = getInitialFilterModel( + undefined, + serverPaginationData as any, + true, + ); + + expect(result).toBeUndefined(); + }); + + it('should handle chartState with null filterModel', () => { + const chartState: Partial = { + filterModel: null as any, + }; + + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }, + }; + + const result = getInitialFilterModel( + chartState, + serverPaginationData, + true, + ); + + expect(result).toEqual(serverPaginationData.agGridFilterModel); + }); + + it('should handle serverPagination undefined (defaults to false)', () => { + const serverPaginationData = { + agGridFilterModel: { + name: { filterType: 'text', type: 'equals', filter: 'test' }, + }, + }; + + const result = getInitialFilterModel( + undefined, + serverPaginationData, + undefined, + ); + + expect(result).toBeUndefined(); + }); + + it('should preserve filter model structure without modification', () => { + const originalFilterModel = { + complexFilter: { + filterType: 'number', + operator: 'OR' as const, + condition1: { filterType: 'number', type: 'equals', filter: 100 }, + condition2: { filterType: 'number', type: 'equals', filter: 200 }, + }, + }; + + const chartState: Partial = { + filterModel: originalFilterModel, + }; + + const result = getInitialFilterModel(chartState, undefined, true); + + // Should return exact same reference, not a copy + expect(result).toBe(originalFilterModel); + }); + }); +}); diff --git a/superset-frontend/src/components/Chart/ChartRenderer.jsx b/superset-frontend/src/components/Chart/ChartRenderer.jsx index bad557f8852..decee4d3b49 100644 --- a/superset-frontend/src/components/Chart/ChartRenderer.jsx +++ b/superset-frontend/src/components/Chart/ChartRenderer.jsx @@ -357,9 +357,17 @@ class ChartRenderer extends Component { ?.behaviors.find(behavior => behavior === Behavior.DrillToDetail) ? { inContextMenu: this.state.inContextMenu } : {}; - // By pass no result component when server pagination is enabled & the table has a backend search query + // By pass no result component when server pagination is enabled & the table has: + // - a backend search query, OR + // - non-empty AG Grid filter model + const hasSearchText = (ownState?.searchText?.length || 0) > 0; + const hasAgGridFilters = + ownState?.agGridFilterModel && + Object.keys(ownState.agGridFilterModel).length > 0; + const bypassNoResult = !( - formData?.server_pagination && (ownState?.searchText?.length || 0) > 0 + formData?.server_pagination && + (hasSearchText || hasAgGridFilters) ); return ( diff --git a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx index 9e7425dc97c..05c9e199729 100644 --- a/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx +++ b/superset-frontend/src/explore/components/useExploreAdditionalActionsMenu/index.jsx @@ -284,11 +284,12 @@ export const useExploreAdditionalActionsMenu = ( canDownloadCSV ? exportChart({ formData: latestQueryFormData, + ownState, resultType: 'post_processed', resultFormat: 'csv', }) : null, - [canDownloadCSV, latestQueryFormData], + [canDownloadCSV, latestQueryFormData, ownState], ); const exportJson = useCallback( @@ -296,11 +297,12 @@ export const useExploreAdditionalActionsMenu = ( canDownloadCSV ? exportChart({ formData: latestQueryFormData, + ownState, resultType: 'results', resultFormat: 'json', }) : null, - [canDownloadCSV, latestQueryFormData], + [canDownloadCSV, latestQueryFormData, ownState], ); const exportExcel = useCallback( @@ -308,11 +310,12 @@ export const useExploreAdditionalActionsMenu = ( canDownloadCSV ? exportChart({ formData: latestQueryFormData, + ownState, resultType: 'results', resultFormat: 'xlsx', }) : null, - [canDownloadCSV, latestQueryFormData], + [canDownloadCSV, latestQueryFormData, ownState], ); const copyLink = useCallback(async () => { diff --git a/superset/models/helpers.py b/superset/models/helpers.py index a4fb9e3fea1..c180420a91c 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -2146,6 +2146,8 @@ class ExploreMixin: # pylint: disable=too-many-public-methods not in { utils.FilterOperator.ILIKE, utils.FilterOperator.LIKE, + utils.FilterOperator.NOT_ILIKE, + utils.FilterOperator.NOT_LIKE, } ): # For backwards compatibility and edge cases @@ -3144,11 +3146,17 @@ class ExploreMixin: # pylint: disable=too-many-public-methods target_clause_list.append(sqla_col.like(eq)) else: target_clause_list.append(sqla_col.ilike(eq)) - elif op in {utils.FilterOperator.NOT_LIKE}: + elif op in { + utils.FilterOperator.NOT_LIKE, + utils.FilterOperator.NOT_ILIKE, + }: if target_generic_type != GenericDataType.STRING: sqla_col = sa.cast(sqla_col, sa.String) - target_clause_list.append(sqla_col.not_like(eq)) + if op == utils.FilterOperator.NOT_LIKE: + target_clause_list.append(sqla_col.not_like(eq)) + else: + target_clause_list.append(sqla_col.not_ilike(eq)) elif ( op == utils.FilterOperator.TEMPORAL_RANGE and isinstance(eq, str) diff --git a/superset/utils/core.py b/superset/utils/core.py index f089aadf70e..a5c69554559 100644 --- a/superset/utils/core.py +++ b/superset/utils/core.py @@ -269,6 +269,7 @@ class FilterOperator(StrEnum): LIKE = "LIKE" NOT_LIKE = "NOT LIKE" ILIKE = "ILIKE" + NOT_ILIKE = "NOT ILIKE" IS_NULL = "IS NULL" IS_NOT_NULL = "IS NOT NULL" IN = "IN"