codeant-ai-for-open-source[bot] commented on code in PR #43277: URL: https://github.com/apache/superset/pull/43277#discussion_r3800356672
########## superset-frontend/src/dashboard/components/gridComponents/FilterHolder/FilterHolder.tsx: ########## @@ -0,0 +1,583 @@ +/** + * 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 { useCallback, useMemo, useState, ReactNode } from 'react'; +import cx from 'classnames'; +import { useDispatch, useSelector } from 'react-redux'; +import { ResizeCallback, ResizeStartCallback } from 're-resizable'; +import { css, useTheme } from '@apache-superset/core/theme'; +import { t } from '@apache-superset/core/translation'; +import { DataMask, Filter } from '@superset-ui/core'; +import { Button, Select } from '@superset-ui/core/components'; +import { Icons } from '@superset-ui/core/components/Icons'; +import PopoverDropdown from '@superset-ui/core/components/PopoverDropdown'; +import { FilterBarOrientation, LayoutItem, RootState } from 'src/dashboard/types'; +import { updateDataMask } from 'src/dataMask/actions'; +import { Draggable } from 'src/dashboard/components/dnd/DragDroppable'; +import DragHandle from 'src/dashboard/components/dnd/DragHandle'; +import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; +import IconButton from 'src/dashboard/components/IconButton'; +import WithPopoverMenu from 'src/dashboard/components/menu/WithPopoverMenu'; +import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; +import ResizableContainer from 'src/dashboard/components/resizable/ResizableContainer'; +import FilterControl from 'src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControl'; +import { COLUMN_TYPE, ROW_TYPE } from 'src/dashboard/util/componentTypes'; +import { + GRID_BASE_UNIT, + GRID_MIN_COLUMN_COUNT, + GRID_MIN_ROW_UNITS, + GRID_COLUMN_COUNT, +} from 'src/dashboard/util/constants'; + +interface FilterHolderProps { + id: string; + parentId: string; + component: LayoutItem; + parentComponent: LayoutItem; + index: number; + depth: number; + editMode: boolean; + + // grid related + availableColumnCount: number; + columnWidth: number; + onResizeStart: ResizeStartCallback; + onResize: ResizeCallback; + onResizeStop: ResizeCallback; + + // dnd + deleteComponent: (id: string, parentId: string) => void; + updateComponents: (updates: Record<string, LayoutItem>) => void; + handleComponentDrop: (...args: unknown[]) => unknown; +} + +const FilterHolder = ({ + id, + parentId, + component, + parentComponent, + index, + depth, + availableColumnCount, + columnWidth, + onResizeStart, + onResize, + onResizeStop, + editMode, + deleteComponent, + updateComponents, + handleComponentDrop, +}: FilterHolderProps) => { + const theme = useTheme(); + const dispatch = useDispatch(); + + const [isFocused, setIsFocused] = useState(false); + const [stagedDataMask, setStagedDataMask] = useState<DataMask | null>(null); + + const nativeFilters = useSelector( + (state: RootState) => state.nativeFilters?.filters || {}, + ); + const dataMask = useSelector((state: RootState) => state.dataMask || {}); + + const filterId = component.meta?.filterId as string | undefined; + const filter = filterId ? (nativeFilters[filterId] as Filter | undefined) : undefined; + + const titlePosition = (component.meta?.titlePosition as 'top' | 'left') || 'top'; + const applyMode = (component.meta?.applyMode as 'instant' | 'manual') || 'instant'; + const buttonPlacement = + (component.meta?.buttonPlacement as 'bottom' | 'right' | 'stacked_right') || + 'bottom'; + + const filterWithDataMask = useMemo(() => { + if (!filter) return null; + return { + ...filter, + dataMask: stagedDataMask || dataMask[filter.id], + inCanvas: true, + } as Filter & { inCanvas: boolean }; + }, [filter, dataMask, stagedDataMask]); + + const updateMeta = useCallback( + (metaUpdates: Record<string, unknown>) => { + updateComponents({ + [component.id]: { + ...component, + meta: { + ...component.meta, + ...metaUpdates, + }, + }, + }); + }, + [component, updateComponents], + ); + + const handleChangeTitlePosition = useCallback( + (nextPosition: string) => { + updateMeta({ titlePosition: nextPosition }); + }, + [updateMeta], + ); + + const handleChangeApplyMode = useCallback( + (nextApplyMode: string) => { + setStagedDataMask(null); + updateMeta({ applyMode: nextApplyMode }); + }, + [updateMeta], + ); + + const handleChangeButtonPlacement = useCallback( + (nextPlacement: string) => { + updateMeta({ buttonPlacement: nextPlacement }); + }, + [updateMeta], + ); + + const handleSelectFilter = useCallback( + (nextFilterId: string) => { + setStagedDataMask(null); + updateMeta({ filterId: nextFilterId }); + }, + [updateMeta], + ); + + const handleFilterSelectionChange = useCallback( + (targetFilter: Filter, nextDataMask: DataMask) => { + if (applyMode === 'manual') { + setStagedDataMask(nextDataMask); + } else { + dispatch(updateDataMask(targetFilter.id, nextDataMask)); + } + }, + [applyMode, dispatch], + ); + + const handleApplyStagedFilter = useCallback(() => { + if (filter && stagedDataMask) { + dispatch(updateDataMask(filter.id, stagedDataMask)); + setStagedDataMask(null); + } + }, [dispatch, filter, stagedDataMask]); + + const handleClearStagedFilter = useCallback(() => { + if (filter) { + const clearedValue = + filter.filterType === 'filter_range' ? [null, null] : undefined; + const clearedMask: DataMask = { + filterState: { value: clearedValue }, + extraFormData: {}, + }; + dispatch(updateDataMask(filter.id, clearedMask)); + setStagedDataMask(null); Review Comment: **Suggestion:** Manual mode stages ordinary selections until Apply, but Clear dispatches directly to Redux and immediately changes the applied dashboard state. This makes Clear bypass the configured manual-apply workflow and can update charts before the user clicks Apply. Stage the cleared mask and commit it through the same Apply path. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Manual canvas filter clearing changes dashboard results immediately. - ⚠️ Clear behaves differently from ordinary manual selections. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=56ed2962943842adb64be655636c1c25&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=56ed2962943842adb64be655636c1c25&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/dashboard/components/gridComponents/FilterHolder/FilterHolder.tsx **Line:** 181:187 **Comment:** *Api Mismatch: Manual mode stages ordinary selections until Apply, but Clear dispatches directly to Redux and immediately changes the applied dashboard state. This makes Clear bypass the configured manual-apply workflow and can update charts before the user clicks Apply. Stage the cleared mask and commit it through the same Apply path. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=1c7032aacc05bfe4982d305fd3248b75dd7bb50e00bb1cc600e75fa18b1c800b&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=1c7032aacc05bfe4982d305fd3248b75dd7bb50e00bb1cc600e75fa18b1c800b&reaction=dislike'>👎</a> ########## superset-frontend/src/filters/components/CustomControls/CustomControlsFilterPlugin.tsx: ########## @@ -0,0 +1,330 @@ +/** + * 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 { useMemo, useState, useCallback, useEffect } from 'react'; +import { t } from '@apache-superset/core/translation'; +import { styled } from '@apache-superset/core/theme'; +import { DataMask } from '@superset-ui/core'; +import { + Select, + Radio, + Checkbox, + Input, + FormItem, +} from '@superset-ui/core/components'; +import { FilterPluginStyle } from '../common'; +import { CustomControlsTransformedProps } from './types'; + +const Styles = styled.div<{ inCanvas?: boolean }>` + width: 100%; + min-height: 32px; + padding: ${({ inCanvas, theme }) => + inCanvas ? `${theme.sizeUnit * 2}px` : '0'}; + overflow: visible; + + .ant-form-item { + margin-bottom: 0; + } + + .custom-controls-radio-group, + .custom-controls-checkbox-group { + display: flex; + flex-direction: ${({ inCanvas }) => (inCanvas ? 'column' : 'row')}; + gap: 8px; + flex-wrap: wrap; + } +`; + +export default function CustomControlsFilterPlugin( + props: CustomControlsTransformedProps, +) { + const { + data = [], + height, + width, + controlType = 'Dropdown', + filterColumn, + orientation = 'vertical', + includeAllOption = false, + multiSelect, + inCanvas = false, + setDataMask = () => {}, + filterState, + } = props; + + // Extract canonical column name used in queries and query result rows + const filterColumnName = useMemo(() => { + if (!filterColumn) return ''; + if (typeof filterColumn === 'string') return filterColumn; + return ( + filterColumn.column_name || + filterColumn.label || + filterColumn.sqlExpression || + 'Custom SQL' + ); + }, [filterColumn]); + + // Extract human-readable string for display labels + const filterColumnLabel = useMemo(() => { + if (!filterColumn) return ''; + if (typeof filterColumn === 'string') return filterColumn; + return ( + filterColumn.label || + filterColumn.column_name || + filterColumn.sqlExpression || + t('Custom SQL') + ); + }, [filterColumn]); + + // Initial local value is filterState if present + const [localValue, setLocalValue] = useState(() => filterState?.value); + + const options = useMemo(() => { + if (!filterColumnName || !data || data.length === 0) return []; + + const uniqueValues = new Set<string | number>(); + data.forEach(row => { + const val = + row[filterColumnName] ?? + (filterColumnLabel ? row[filterColumnLabel as string] : undefined); + if (val !== undefined && val !== null) { + uniqueValues.add(val as string | number); + } + }); + + let opts = Array.from(uniqueValues).map(val => ({ + label: String(val), + value: val, + })); + + if (includeAllOption) { + opts = [{ label: t('All'), value: 'ALL_SELECTED' }, ...opts]; + } + + return opts; + }, [data, filterColumnName, filterColumnLabel, includeAllOption]); + + const emitFilter = useCallback( + (val: unknown) => { + if (!filterColumnName) return; + + const isAllSelected = + val === 'ALL_SELECTED' || + (Array.isArray(val) && val.includes('ALL_SELECTED')); + const isEmpty = + isAllSelected || + val === undefined || + val === null || + val === '' || + (Array.isArray(val) && val.length === 0); + + let op: 'ILIKE' | 'IN' = 'IN'; + let filterVal: unknown = val; + + if (controlType === 'TextBox') { + op = 'ILIKE'; + filterVal = `%${val}%`; + } else if (Array.isArray(val)) { + op = 'IN'; + filterVal = val.filter(v => v !== 'ALL_SELECTED'); + } else { + op = 'IN'; + filterVal = [val]; + } + + const dataMask: DataMask = { + extraFormData: { + filters: isEmpty + ? [] + : [ + { + col: filterColumnName, + op, + val: filterVal, + }, + ], + }, + filterState: { + value: isEmpty ? null : val, + label: isEmpty ? '' : Array.isArray(val) ? val.join(', ') : String(val), + }, + }; + + setDataMask(dataMask); + }, + [filterColumnName, controlType, setDataMask], + ); + + const handleChange = (val: unknown) => { + setLocalValue(val); + emitFilter(val); + }; + + useEffect(() => { + if (filterState?.value !== undefined) { + setLocalValue(filterState.value); + } + }, [filterState?.value]); + + const effectiveControlType = + ['Dropdown', 'Radio', 'Checkbox', 'TextBox'].includes(controlType) + ? controlType + : 'Dropdown'; + + const renderControl = () => { + if (effectiveControlType === 'TextBox') { + return ( + <Input + placeholder={ + filterColumnLabel + ? t('Filter by %s', filterColumnLabel) + : t('Filter by value') + } + value={localValue as string} + onChange={e => handleChange(e.target.value)} + allowClear + style={{ width: '100%' }} + /> + ); + } + + // In Filter Bar (where space is tight) or Config Modal, render Checkbox/Radio in compact dropdown + if (!inCanvas) { + return ( + <Select + headerPosition="left" + ariaLabel={filterColumnLabel || t('Filter')} + placeholder={ + filterColumnLabel + ? t('Select %s', filterColumnLabel) + : t('Select a value') + } + options={options} + value={localValue as any} + onChange={handleChange} + allowClear + getPopupContainer={() => document.body} + mode={ + effectiveControlType === 'Checkbox' || multiSelect + ? 'multiple' + : 'single' + } + /> + ); + } + + const isExceedingThreshold = options.length > 10; + + // On Canvas (or chart explore), render native full interactive control, + // or convert to Dropdown if count exceeds 10 options + if (effectiveControlType === 'Dropdown' || isExceedingThreshold) { + return ( + <Select + headerPosition="left" + ariaLabel={filterColumnLabel || t('Filter')} + placeholder={ + filterColumnLabel + ? t('Select %s', filterColumnLabel) + : t('Select a value') + } + options={options} + value={localValue as any} + onChange={handleChange} + allowClear + getPopupContainer={() => document.body} + mode={ + effectiveControlType === 'Checkbox' || multiSelect + ? 'multiple' + : 'single' Review Comment: **Suggestion:** The `Checkbox` control is forced into Ant Design multiple-selection mode even when `multiSelect` is false. Consequently, disabling “Allow Multiple Selections” has no effect and users can still select multiple values, contradicting the control-panel configuration. Respect `multiSelect` when choosing the control mode or render a single-select control when it is disabled. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Checkbox filters violate the configured single-selection setting. - ⚠️ Queries receive multiple selected values unexpectedly. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=399ebc07fcb24836ad693d06eb494325&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=399ebc07fcb24836ad693d06eb494325&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/filters/components/CustomControls/CustomControlsFilterPlugin.tsx **Line:** 250:253 **Comment:** *Api Mismatch: The `Checkbox` control is forced into Ant Design multiple-selection mode even when `multiSelect` is false. Consequently, disabling “Allow Multiple Selections” has no effect and users can still select multiple values, contradicting the control-panel configuration. Respect `multiSelect` when choosing the control mode or render a single-select control when it is disabled. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=80bbb5f1cc7087abbac81e7096a032e3e770a6f6d891c2f37b76d2c142f9fdc6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=80bbb5f1cc7087abbac81e7096a032e3e770a6f6d891c2f37b76d2c142f9fdc6&reaction=dislike'>👎</a> ########## superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts: ########## @@ -36,23 +37,66 @@ import { import { useFilterConfiguration } from '../state'; export const useFilters = () => { - const preselectedNativeFilters = useSelector<any, Filters>( + const preselectedNativeFilters = useSelector<RootState, Filters | undefined>( state => state.dashboardState?.preselectNativeFilters, ); + const dashboardLayout = useSelector<RootState, DashboardLayout>( + state => state.dashboardLayout?.present || {}, + ); const filterConfiguration = useFilterConfiguration(); + // Exclude native filters that are already placed on the dashboard canvas + const canvasFilterIds = useMemo(() => { + const ids = new Set<string>(); + Object.values(dashboardLayout).forEach(item => { + if (item?.type === FILTER_TYPE && item?.meta?.filterId) { + ids.add(String(item.meta.filterId)); + } + }); + return ids; + }, [dashboardLayout]); + return useMemo( () => - filterConfiguration.reduce( - (acc, filter: Filter) => ({ - ...acc, - [filter.id]: { - ...filter, - preselect: preselectedNativeFilters?.[filter.id], - }, - }), - {} as Filters, - ), + filterConfiguration + .filter( + (filter): filter is Filter => + filter.type !== 'DIVIDER' && !canvasFilterIds.has(filter.id), + ) Review Comment: **Suggestion:** Removing canvas-bound filters from `useFilters` also removes them from the FilterBar's initialization, selected-mask cleanup, and required-filter lifecycle. A canvas filter rendered by `FilterHolder` therefore does not receive the same default/preselected and required-state processing as a regular native filter, while its data mask can still be present in shared Redux state. Keep canvas filters in the lifecycle data and exclude them only from the bar's visual rendering. [state/lifecycle] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Canvas filters can skip required/default initialization. - ⚠️ Cascading canvas filters can lose lifecycle synchronization. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eafc2c9b22b94582bc178a85fbfb3fc5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=eafc2c9b22b94582bc178a85fbfb3fc5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/dashboard/components/nativeFilters/FilterBar/state.ts **Line:** 61:65 **Comment:** *State Lifecycle: Removing canvas-bound filters from `useFilters` also removes them from the FilterBar's initialization, selected-mask cleanup, and required-filter lifecycle. A canvas filter rendered by `FilterHolder` therefore does not receive the same default/preselected and required-state processing as a regular native filter, while its data mask can still be present in shared Redux state. Keep canvas filters in the lifecycle data and exclude them only from the bar's visual rendering. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=1e56c92e084adab3b249794c6832eac9e2c280798a5f6ebdc769ccb7ad755011&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=1e56c92e084adab3b249794c6832eac9e2c280798a5f6ebdc769ccb7ad755011&reaction=dislike'>👎</a> ########## superset-frontend/src/filters/components/DateTimeFilter/DateTimeFilterPlugin.tsx: ########## @@ -0,0 +1,1005 @@ +/** + * 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 { useCallback, useEffect, useRef, useState, useMemo } from 'react'; +import { t } from '@apache-superset/core/translation'; +import { + NO_TIME_RANGE, + fetchTimeRange, + SEPARATOR, + JsonObject, +} from '@superset-ui/core'; +import { styled, useTheme } from '@apache-superset/core/theme'; +import { + RangePicker, + Button, + Divider, + AntdThemeProvider, + InfoTooltip, + Popover, +} from '@superset-ui/core/components'; +import { + CommonFrame, + CalendarFrame, + CurrentCalendarFrame, + CustomFrame, +} from 'src/explore/components/controls/DateFilterControl/components'; +import { DateFilterTestKey } from 'src/explore/components/controls/DateFilterControl/utils'; +import { FilterPluginStyle } from '../common'; +import { PluginFilterDateTimeProps } from './types'; +import { useLocale } from 'src/hooks/useLocale'; +import dayjs from 'dayjs'; + +// Matches date strings returned by fetchTimeRange, e.g.: +// "2026-04-23 ≤ col < 2026-04-29" +// "2026-04-23 00:00:00 ≤ col < 2026-04-29 00:00:00" +const RESOLVED_DATE_RE = /(\d{4}-\d{2}-\d{2}(?:[T ]\d{2}:\d{2}:\d{2})?)/g; + +/* ------------------------------------------------------------------ */ +/* Frame → Tab mapping */ +/* ------------------------------------------------------------------ */ + +type TabKey = 'basic' | 'last' | 'previous' | 'current' | 'custom'; + +const TAB_CONFIG: { key: TabKey; label: string }[] = [ + { key: 'basic', label: 'Basic' }, + { key: 'last', label: 'Last' }, + { key: 'previous', label: 'Previous' }, + { key: 'current', label: 'Current' }, + { key: 'custom', label: 'Custom' }, +]; + +/* ------------------------------------------------------------------ */ +/* Styled wrappers */ +/* ------------------------------------------------------------------ */ + +const DateTimeFilterStyles = styled(FilterPluginStyle)` + display: flex; + align-items: center; + overflow-x: visible; +`; + +const ControlContainer = styled.div<{ + validateStatus?: 'error' | 'warning' | 'info'; +}>` + display: flex; + height: 100%; + max-width: 100%; + width: 100%; + + & > .ant-picker { + width: 100%; + flex: 1; + } +`; + +const PopoverContent = styled.div` + width: 600px; + max-width: 90vw; + + .tab-nav { + display: flex; + border-bottom: 1px solid ${({ theme }) => theme.colorBorderSecondary}; + padding: 0 8px; + margin-bottom: 0; + } + + .tab-nav-item { + padding: 6px 10px; + cursor: pointer; + font-size: 11px; + font-weight: 500; + letter-spacing: 0.02em; + color: ${({ theme }) => theme.colorTextSecondary}; + border-bottom: 2px solid transparent; + margin-bottom: -1px; + transition: all 0.2s; + + &:hover { + color: ${({ theme }) => theme.colorPrimary}; + } + + &.active { + color: ${({ theme }) => theme.colorPrimary}; + border-bottom-color: ${({ theme }) => theme.colorPrimary}; + } + } + + .tab-body { + padding: 8px 16px; + min-height: 100px; + + .section-title { + font-weight: 600; + font-size: 13px; + line-height: 20px; + margin-bottom: 6px; + letter-spacing: -0.01em; + } + + .control-label { + font-size: 11px; + font-weight: 500; + color: ${({ theme }) => theme.colorTextSecondary}; + margin-bottom: 6px; + text-transform: uppercase; + letter-spacing: 0.03em; + } + + .ant-input { + background: ${({ theme }) => theme.colorBgContainer} !important; + border: 1px solid ${({ theme }) => theme.colorBorder} !important; + color: ${({ theme }) => theme.colorText} !important; + padding: 6px 12px; + font-size: 12px; + border-radius: 4px; + + &:focus { + border-color: ${({ theme }) => theme.colorPrimary} !important; + box-shadow: 0 0 0 2px ${({ theme }) => theme.colorPrimary}22 !important; + } + + &::placeholder { + color: ${({ theme }) => + theme.colorTextPlaceholder || theme.colorTextQuaternary} !important; + } + } + + .ant-row { + margin-top: 8px; + } + .ant-picker { + padding: 4px 17px 4px; + border-radius: 4px; + } + .ant-divider-horizontal { + margin: 16px 0; + border-color: ${({ theme }) => theme.colorBorderSecondary}; + } + .control-anchor-to { + margin-top: 16px; + } + .control-anchor-to-datetime { + width: 217px; + } + } +`; + +const ActualTimeRange = styled.div` + font-size: 12px; + font-weight: 600; + color: ${({ theme }) => theme.colorText}; + padding: 4px 0; + font-family: ${({ theme }) => theme.fontFamilyCode}; + display: flex; + align-items: center; + flex-wrap: nowrap; + gap: 8px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + + .label { + font-size: 11px; + font-weight: 500; + color: ${({ theme }) => theme.colorTextSecondary}; + text-transform: uppercase; + letter-spacing: 0.03em; + flex-shrink: 0; + } +`; + +/** + * Container that holds the inline calendar. + * + * The RangePicker renders two elements: + * 1. The <input> row — we collapse it to zero height so it's invisible. + * 2. The dropdown panel — we un-position it so it flows inline in the div. + * + * pointer-events on the input are set to none so clicks pass through to the + * calendar panels, which explicitly restore pointer-events. + */ +const InlineCalendarContainer = styled.div` + position: relative; + /* Enough height for two calendar months side by side */ + min-height: 290px; + margin-bottom: 12px; + + /* Collapse the picker INPUT element */ + .ant-picker { + position: absolute !important; + top: 0; + left: 0; + width: 0 !important; + height: 0 !important; + padding: 0 !important; + border: none !important; + overflow: hidden !important; + pointer-events: none !important; + opacity: 0 !important; + } + + /* Make the dropdown render statically inside this div */ + .ant-picker-dropdown { + position: static !important; + box-shadow: none !important; + padding: 0 !important; + background: transparent !important; + } + + .ant-picker-panel-container { + box-shadow: none !important; + border: none !important; + background: transparent !important; + } + + .ant-picker-header-view { + font-weight: 600; + font-size: 13px; + letter-spacing: -0.01em; + } + + .ant-picker-content th { + font-size: 11px; + color: ${({ theme }) => theme.colorTextDescription}; + font-weight: 500; + } + + /* Range selection colors — Preset Green */ + .ant-picker-cell-in-view.ant-picker-cell-in-range::before { + background: ${({ theme }) => theme.colorPrimary}22 !important; + } + .ant-picker-cell-in-view.ant-picker-cell-range-start .ant-picker-cell-inner, + .ant-picker-cell-in-view.ant-picker-cell-range-end .ant-picker-cell-inner { + background: ${({ theme }) => theme.colorPrimary} !important; + color: white !important; + } + .ant-picker-cell-in-view.ant-picker-cell-today + .ant-picker-cell-inner::before { + border-color: ${({ theme }) => theme.colorPrimary} !important; + } + + /* Restore click events on the actual calendar UI */ + .ant-picker-panel-container, + .ant-picker-panels, + .ant-picker-panel, + .ant-picker-body, + .ant-picker-content, + .ant-picker-header, + table, + th, + td { + pointer-events: auto !important; + } +`; + +const StatusTag = styled.span` + background: ${({ theme }) => theme.colorSuccessBg}; + color: ${({ theme }) => theme.colorSuccess}; + font-size: 10px; + font-weight: 700; + padding: 2px 6px; + border-radius: 10px; + text-transform: uppercase; + letter-spacing: 0.05em; + margin-right: 8px; + display: inline-flex; + align-items: center; + gap: 4px; + + &::before { + content: ''; + width: 6px; + height: 6px; + background: ${({ theme }) => theme.colorSuccess}; + border-radius: 50%; + } +`; + +const InputWrapper = styled.div` + position: relative; + width: 100%; + + .clear-icon { + position: absolute; + right: 8px; + top: 50%; + transform: translateY(-50%); + cursor: pointer; + color: ${({ theme }) => + theme.colorTextDescription || theme.colorTextTertiary}; + font-size: 12px; + transition: color 0.2s; + + &:hover { + color: ${({ theme }) => theme.colorText}; + } + } +`; + +/* ------------------------------------------------------------------ */ +/* Component */ +/* ------------------------------------------------------------------ */ + +export default function DateTimeFilterPlugin(props: PluginFilterDateTimeProps) { + const theme = useTheme(); + const { + setDataMask, + setHoveredFilter, + unsetHoveredFilter, + setFocusedFilter, + unsetFocusedFilter, + setFilterActive, + width, + height, + filterState, + inputRef, + isOverflowingFilterBar = false, + formData, + clearAllTrigger, + onClearAllComplete, + } = props; + + const col: string = useMemo(() => { + const jsonFormData = formData as JsonObject | undefined; + const rawCol = + jsonFormData?.groupby || + jsonFormData?.column || + (jsonFormData?.targets as JsonObject[] | undefined)?.[0]?.column?.name || + jsonFormData?.columnName; + if (typeof rawCol === 'string') return rawCol.trim(); + if (Array.isArray(rawCol) && rawCol.length > 0) return String(rawCol[0]).trim(); + if (rawCol && typeof rawCol === 'object') { + const colObj = rawCol as Record<string, unknown>; + return String( + colObj.label || colObj.column_name || colObj.sqlExpression || '', + ).trim(); + } + return ''; + }, [formData]); + + // ---- State ---- + const [show, setShow] = useState(false); + const [timeRangeValue, setTimeRangeValue] = useState<string>(NO_TIME_RANGE); + const [triggerDates, setTriggerDates] = useState< + [dayjs.Dayjs | null, dayjs.Dayjs | null] + >([null, null]); + const [evalResponse, setEvalResponse] = useState<string>(''); + const [validTimeRange, setValidTimeRange] = useState(true); + const [activeTab, setActiveTab] = useState<TabKey>('basic'); + // Bump to re-mount the inline RangePicker after the popover finishes animating in + const [calendarKey, setCalendarKey] = useState(0); + const [defaultPickerValue, setDefaultPickerValue] = useState< + [dayjs.Dayjs, dayjs.Dayjs] | undefined + >(undefined); + + const datePickerLocale = useLocale(); + const calendarContainerRef = useRef<HTMLDivElement>(null); + + const suggestedBtnStyle = useMemo( + () => ({ + background: 'none', + border: 'none', + padding: 0, + color: theme.colorPrimary, + cursor: 'pointer', + font: 'inherit', + textDecoration: 'none', + display: 'inline', + }), + [theme.colorPrimary], + ); + + useEffect(() => { + if (clearAllTrigger) { + setTimeRangeValue(NO_TIME_RANGE); + setTriggerDates([null, null]); + setDataMask({ + extraFormData: { filters: [] }, + filterState: { value: null, label: '' }, + }); + onClearAllComplete?.(formData.nativeFilterId); + } + }, [clearAllTrigger, onClearAllComplete, setDataMask, formData.nativeFilterId]); + + // Parse since/until for the inline calendar value + const [since, until] = useMemo(() => { + if ( + timeRangeValue && + timeRangeValue !== NO_TIME_RANGE && + timeRangeValue.includes(SEPARATOR) + ) { + const parts = timeRangeValue.split(SEPARATOR); + return [parts[0]?.trim() || '', parts[1]?.trim() || '']; + } + return ['', '']; + }, [timeRangeValue]); + + const calValue: [dayjs.Dayjs | null, dayjs.Dayjs | null] = useMemo(() => { + // If we have a successful resolved range string (e.g. "2026-04-23 <= col < 2026-04-29"), + // use those dates to drive the calendar highlights even if the input is a formula. + if ( + evalResponse && + !evalResponse.includes('Invalid') && + evalResponse.includes('col') + ) { + const matches = [...evalResponse.matchAll(RESOLVED_DATE_RE)]; + if (matches.length >= 2) { + const start = dayjs(matches[0][1]); + const end = dayjs(matches[1][1]); + if (start.isValid() && end.isValid()) { + return [start, end]; + } + } + } + + // Fallback to direct parsing if it's a fixed date string + return [ + since && dayjs(since).isValid() ? dayjs(since) : null, + until && dayjs(until).isValid() ? dayjs(until) : null, + ]; + }, [since, until, evalResponse]); + + /* ---- Resolve filterState.value → trigger display --------------- */ + // Watch the dashboard's confirmed value and derive actual dates for the + // trigger RangePicker display. This survives re-mounts and page reloads. + useEffect(() => { + let isCurrent = true; + const value = (filterState.value as string) || NO_TIME_RANGE; + if (!value || value === NO_TIME_RANGE) { + setTriggerDates([null, null]); + return undefined; + } + + // Synchronous path: value is already ISO date strings (e.g. "2026-04-01 : 2026-05-01") + if (value.includes(SEPARATOR)) { + const parts = value.split(SEPARATOR); + const s = parts[0]?.trim() ?? ''; + const e = parts[1]?.trim() ?? ''; + const start = s && dayjs(s).isValid() ? dayjs(s) : null; + const end = e && dayjs(e).isValid() ? dayjs(e) : null; + // ONLY use the fast path if BOTH are valid dates. + // If either is a formula (invalid dayjs), we must use the async fetchTimeRange path. + if (start && end) { + setTriggerDates([start, end]); + return undefined; + } + } + + // Async path: resolve formula strings (e.g. "30 days ago : now") + fetchTimeRange(value).then(({ value: resolved, error }) => { + if (!isCurrent) return; + if (!error && resolved) { + const matches = [...resolved.matchAll(RESOLVED_DATE_RE)]; + if (matches.length >= 2) { + const [[, start], [, end]] = matches; + setTriggerDates([ + dayjs(start).isValid() ? dayjs(start) : null, + dayjs(end).isValid() ? dayjs(end) : null, + ]); + } + } + }); + return () => { + isCurrent = false; + }; + }, [filterState.value]); + useEffect(() => { + if (show && activeTab === 'basic') { + // Small delay allows the popover animation to finish before mounting + const timer = setTimeout(() => setCalendarKey(k => k + 1), 80); + return () => clearTimeout(timer); + } + return undefined; + }, [show, activeTab]); + + /* ---- Resolve actual time range preview ------------------------- */ + useEffect(() => { + let isCurrent = true; + if (!timeRangeValue || timeRangeValue === NO_TIME_RANGE) { + setEvalResponse(''); + setValidTimeRange(true); + return undefined; + } + fetchTimeRange(timeRangeValue).then(({ value: resolved, error }) => { + if (!isCurrent) return; + if (error) { + setEvalResponse(error || ''); + setValidTimeRange(false); + } else { + setEvalResponse(resolved || ''); + setValidTimeRange(true); + } + }); + return () => { + isCurrent = false; + }; + }, [timeRangeValue]); + + /* ---- Emit filter ---------------------------------------------- */ + const emitFilter = useCallback( + async (rangeStr: string) => { + const isSet = rangeStr && rangeStr !== NO_TIME_RANGE; + if (!isSet) { + setDataMask({ + extraFormData: { filters: [] }, + filterState: { value: null, label: '' }, + }); + return; + } + + const extra: JsonObject = {}; + + if (!col) { + extra.time_range = rangeStr; + } + + try { + const { value: resolved, error } = await fetchTimeRange(rangeStr); + if (!error && resolved) { + const matches = [...resolved.matchAll(RESOLVED_DATE_RE)]; + if (matches.length >= 2) { + const [[, start], [, end]] = matches; + if (col) { + extra.filters = [ + { col, op: '>=', val: start }, Review Comment: **Suggestion:** The asynchronous resolution is not versioned or cancelled before `setDataMask`. If the user applies a second range while the first `fetchTimeRange` request is still pending, the older response can complete later and overwrite the newer selection. Track the latest request or cancel stale requests before publishing the mask. [race condition] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Rapid datetime changes can apply an older range. - ⚠️ Dashboard charts may refresh with stale filter values. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=96c18ad9b70743a290268d01409aab1b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=96c18ad9b70743a290268d01409aab1b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/filters/components/DateTimeFilter/DateTimeFilterPlugin.tsx **Line:** 553:560 **Comment:** *Race Condition: The asynchronous resolution is not versioned or cancelled before `setDataMask`. If the user applies a second range while the first `fetchTimeRange` request is still pending, the older response can complete later and overwrite the newer selection. Track the latest request or cancel stale requests before publishing the mask. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=d2b954537519ab201e7a0aaaac78d52c630c8de5d45fdf345011430f1d973aea&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43277&comment_hash=d2b954537519ab201e7a0aaaac78d52c630c8de5d45fdf345011430f1d973aea&reaction=dislike'>👎</a> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
