codeant-ai-for-open-source[bot] commented on code in PR #43275: URL: https://github.com/apache/superset/pull/43275#discussion_r3799889138
########## superset-frontend/src/filters/components/CustomControls/buildQuery.ts: ########## @@ -0,0 +1,54 @@ +/** + * 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 { + buildQueryContext, + BuildQuery, + QueryObject, +} from '@superset-ui/core'; +import { PluginFilterCustomControlsQueryFormData } from './types'; + +const buildQuery: BuildQuery<PluginFilterCustomControlsQueryFormData> = ( + formData: PluginFilterCustomControlsQueryFormData, +) => + buildQueryContext(formData, baseQueryObject => { + const rawCol = + formData.targets?.[0]?.column?.name || + (formData as Record<string, unknown>).groupby || + formData.filterColumn; + const col = Array.isArray(rawCol) + ? rawCol[0] + : typeof rawCol === 'object' && rawCol !== null + ? (rawCol as { label?: string; column_name?: string; sqlExpression?: string }).column_name || + (rawCol as { label?: string }).label || + (rawCol as { sqlExpression?: string }).sqlExpression + : rawCol; + const columns = col ? [String(col)] : baseQueryObject.columns || []; + + const query: QueryObject[] = [ + { + ...baseQueryObject, + columns, + orderby: columns.map(c => [c, true]), + row_limit: 1000, + }, Review Comment: **Suggestion:** Hard-coding `row_limit` to 1000 truncates the option query for columns with more than 1000 distinct values. The custom control has no pagination or search query to recover omitted values, so users cannot select valid values beyond the first 1000 returned rows. [incomplete implementation] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ High-cardinality Generic Filter options are truncated. - ❌ Users cannot select values beyond the first 1000 rows. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3a5db2054b244a55b77c2311f011fada&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=3a5db2054b244a55b77c2311f011fada&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/buildQuery.ts **Line:** 49:49 **Comment:** *Incomplete Implementation: Hard-coding `row_limit` to 1000 truncates the option query for columns with more than 1000 distinct values. The custom control has no pagination or search query to recover omitted values, so users cannot select valid values beyond the first 1000 returned rows. 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%2F43275&comment_hash=643dacf56a30b3be7c54750247cd8f40762b9a5da5498bc064790f093ca87ef5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43275&comment_hash=643dacf56a30b3be7c54750247cd8f40762b9a5da5498bc064790f093ca87ef5&reaction=dislike'>👎</a> ########## superset-frontend/src/filters/components/CustomControls/CustomControlsFilterPlugin.tsx: ########## @@ -0,0 +1,303 @@ +/** + * 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 { 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` : '0px'}; + display: flex; + flex-direction: column; + justifyContent: center; + + .ant-select { + width: 100%; + } + + .ant-input { + width: 100%; + } +`; + +export default function CustomControlsFilterPlugin( + props: CustomControlsTransformedProps, +) { + const { + data, + height, + width, + controlType, + filterColumn, + orientation, + includeAllOption, + multiSelect, + inCanvas = false, + setDataMask = () => {}, + filterState, + } = props; + + // Extract human-readable string for Custom SQL dimensions + const filterColumnLabel = useMemo(() => { + if (!filterColumn) return ''; + if (typeof filterColumn === 'string') return filterColumn; + return ( + filterColumn.label || + filterColumn.column_name || + filterColumn.sqlExpression || + 'Custom SQL' + ); Review Comment: **Suggestion:** For an object-valued `filterColumn` containing both `label` and `column_name`, this lookup uses the label, while the query builder selects `column_name` as the result column. The returned rows are therefore keyed by `column_name`, causing `row[filterColumnLabel]` to be undefined and leaving the control with no options despite the query returning data. Use the same canonical column key as the query builder. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Custom filter controls show no options for aliased columns. - ⚠️ Valid queried values cannot be selected or applied. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8a00ffb665b248dfa6764fde9ae78a09&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=8a00ffb665b248dfa6764fde9ae78a09&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:** 71:76 **Comment:** *Api Mismatch: For an object-valued `filterColumn` containing both `label` and `column_name`, this lookup uses the label, while the query builder selects `column_name` as the result column. The returned rows are therefore keyed by `column_name`, causing `row[filterColumnLabel]` to be undefined and leaving the control with no options despite the query returning data. Use the same canonical column key as the query builder. 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%2F43275&comment_hash=e4916df140b65bea97e5fced64a3507881cc11849288f0cc8fb89144fd710e4c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43275&comment_hash=e4916df140b65bea97e5fced64a3507881cc11849288f0cc8fb89144fd710e4c&reaction=dislike'>👎</a> ########## superset-frontend/src/filters/components/DateTimeFilter/DateTimeFilterPlugin.tsx: ########## @@ -0,0 +1,1028 @@ +/** + * 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' | 'advanced'; + +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, + } = 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); + + // 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(() => { + const value = (filterState.value as string) || NO_TIME_RANGE; + if (!value || value === NO_TIME_RANGE) { + setTriggerDates([null, null]); + return; + } + + // 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; + } + } + + // Async path: resolve formula strings (e.g. "30 days ago : now") + fetchTimeRange(value).then(({ value: resolved, error }) => { + if (!error && resolved) { + const matches = [...resolved.matchAll(RESOLVED_DATE_RE)]; + if (matches.length >= 2) { + setTriggerDates([ + dayjs(matches[0][1]).isValid() ? dayjs(matches[0][1]) : null, + dayjs(matches[1][1]).isValid() ? dayjs(matches[1][1]) : null, + ]); + } + } Review Comment: **Suggestion:** This effect starts a new `fetchTimeRange` request whenever the confirmed filter value changes, but it does not cancel the previous request or verify that the response still belongs to the current value. A slower response for an older range can overwrite `triggerDates` after a newer range has already been applied, leaving the visible picker inconsistent with `filterState.value`. [race condition] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Date picker trigger can display stale resolved dates. - ⚠️ Users may see inconsistent filter state and calendar values. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3bf36f38d6894dfabc251cb880930479&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=3bf36f38d6894dfabc251cb880930479&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:** 456:465 **Comment:** *Race Condition: This effect starts a new `fetchTimeRange` request whenever the confirmed filter value changes, but it does not cancel the previous request or verify that the response still belongs to the current value. A slower response for an older range can overwrite `triggerDates` after a newer range has already been applied, leaving the visible picker inconsistent with `filterState.value`. 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%2F43275&comment_hash=06d06c8e47608bcc03ed3ba4c9753fcb97a57572edee9b405b8f9d274c54681d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43275&comment_hash=06d06c8e47608bcc03ed3ba4c9753fcb97a57572edee9b405b8f9d274c54681d&reaction=dislike'>👎</a> ########## superset-frontend/src/filters/components/DateTimeFilter/transformProps.ts: ########## @@ -0,0 +1,59 @@ +/** + * 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 { ChartProps } from '@superset-ui/core'; +import { noOp } from 'src/utils/common'; + +export default function transformProps(chartProps: ChartProps) { + const { + formData, + height, + hooks, + width, + behaviors, + filterState, + inputRef, + displaySettings, + } = chartProps; + + const { + setDataMask = noOp, + setFocusedFilter = noOp, + unsetFocusedFilter = noOp, + setHoveredFilter = noOp, + unsetHoveredFilter = noOp, + setFilterActive = noOp, + } = hooks; + + return { + formData, + behaviors, + height, + setDataMask, + filterState, + width, + setHoveredFilter, + unsetHoveredFilter, + setFocusedFilter, + unsetFocusedFilter, + setFilterActive, + inputRef, + isOverflowingFilterBar: displaySettings?.isOverflowingFilterBar, + filterBarOrientation: displaySettings?.filterBarOrientation, + }; Review Comment: **Suggestion:** The native filter hook contract provides `clearAllTrigger` and `onClearAllComplete`, but this transform drops both values. `DateTimeFilterPlugin` consequently cannot reset its local `timeRangeValue` when Clear All occurs, so an open editor can continue showing and reapply the previously selected range instead of reflecting the cleared filter. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Clear All fails to reset an open date filter editor. - ⚠️ Applying afterward can restore the previously cleared range. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=02251a3d33db4562bdf51b81a837470f&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=02251a3d33db4562bdf51b81a837470f&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/transformProps.ts **Line:** 43:58 **Comment:** *Api Mismatch: The native filter hook contract provides `clearAllTrigger` and `onClearAllComplete`, but this transform drops both values. `DateTimeFilterPlugin` consequently cannot reset its local `timeRangeValue` when Clear All occurs, so an open editor can continue showing and reapply the previously selected range instead of reflecting the cleared filter. 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%2F43275&comment_hash=48841a8bb786dc900bd823be7c744e3ebf40d9d7cbe0f8ad9d26458a7474988f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43275&comment_hash=48841a8bb786dc900bd823be7c744e3ebf40d9d7cbe0f8ad9d26458a7474988f&reaction=dislike'>👎</a> ########## superset-frontend/src/filters/components/DateTimeFilter/DateTimeFilterPlugin.tsx: ########## @@ -0,0 +1,1028 @@ +/** + * 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' | 'advanced'; + +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, + } = 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); + + // 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(() => { + const value = (filterState.value as string) || NO_TIME_RANGE; + if (!value || value === NO_TIME_RANGE) { + setTriggerDates([null, null]); + return; + } + + // 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; + } + } + + // Async path: resolve formula strings (e.g. "30 days ago : now") + fetchTimeRange(value).then(({ value: resolved, error }) => { + if (!error && resolved) { + const matches = [...resolved.matchAll(RESOLVED_DATE_RE)]; + if (matches.length >= 2) { + setTriggerDates([ + dayjs(matches[0][1]).isValid() ? dayjs(matches[0][1]) : null, + dayjs(matches[1][1]).isValid() ? dayjs(matches[1][1]) : null, + ]); + } + } + }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [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 }, + { col, op: '<=', val: end }, + ]; + } Review Comment: **Suggestion:** The displayed end boundary is documented as exclusive and the resolved range uses a half-open interval, but this converts it to `<=`. Applying a range ending on a date can therefore include the end boundary or produce incorrect results for timestamp columns; preserve the exclusive upper-bound semantics when constructing the filter. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Date/time filters can include records at exclusive boundaries. - ⚠️ Dashboard results differ from the displayed half-open range. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c856e1fc7d5f42bcaa1eba169435e82f&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=c856e1fc7d5f42bcaa1eba169435e82f&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:** 526:530 **Comment:** *Logic Error: The displayed end boundary is documented as exclusive and the resolved range uses a half-open interval, but this converts it to `<=`. Applying a range ending on a date can therefore include the end boundary or produce incorrect results for timestamp columns; preserve the exclusive upper-bound semantics when constructing the filter. 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%2F43275&comment_hash=d8e88028c7629a3f44c320e6ec6c5ba793a7aa15ad6652f5ef8fabf3fb39ca04&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43275&comment_hash=d8e88028c7629a3f44c320e6ec6c5ba793a7aa15ad6652f5ef8fabf3fb39ca04&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]
