aminghadersohi commented on code in PR #45612: URL: https://github.com/apache/superset/pull/45612#discussion_r4363580860
########## superset-frontend/src/components/Chart/DrillBy/drillByTimeRange.ts: ########## @@ -0,0 +1,209 @@ +/** + * 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 rison from 'rison'; +import { + BinaryQueryObjectFilterClause, + ContextMenuFilters, + JsonObject, + NO_TIME_RANGE, + QueryFormData, + SupersetClient, + buildQueryObject, + ensureIsArray, + getColumnLabel, +} from '@superset-ui/core'; + +type DrillByConfig = ContextMenuFilters['drillBy']; + +const TIME_RANGE_SEPARATOR = ' : '; + +/** + * Return the innermost temporal bucket filter a Drill by was started from, + * i.e. the last TEMPORAL_RANGE drill filter. Each further drill narrows the + * previous one, so the last bucket is the one to query. + */ +export const getDrillByTimeFilter = ( + configs: DrillByConfig[], +): BinaryQueryObjectFilterClause | undefined => { + let timeFilter: BinaryQueryObjectFilterClause | undefined; + configs.forEach(config => { + ensureIsArray(config?.filters).forEach(filter => { + if ( + filter.op === 'TEMPORAL_RANGE' && + typeof filter.val === 'string' && + filter.val + ) { + timeFilter = filter; + } + }); + }); + return timeFilter; +}; + +/** + * Return the time range of the innermost temporal bucket a Drill by was + * started from. + */ +export const getDrillByTimeRange = ( + configs: DrillByConfig[], +): string | undefined => + getDrillByTimeFilter(configs)?.val as string | undefined; + +/** + * Return the time range the chart displayed before the drill, using the same + * precedence as the chart's own query: a dashboard override, a legacy + * dashboard time filter or the chart's time_range, and otherwise the chart's + * TEMPORAL_RANGE filter on the drilled column. Returns undefined when the + * chart is not limited in time. + */ +export const getDrillByOuterTimeRange = ( + formData: QueryFormData, + column?: BinaryQueryObjectFilterClause['col'], +): string | undefined => { + let timeRange: unknown; + try { + const queryObject = buildQueryObject(formData); + const columnLabel = + column === undefined ? undefined : getColumnLabel(column); + timeRange = + queryObject.time_range ?? + ensureIsArray(queryObject.filters).find( + (filter): filter is BinaryQueryObjectFilterClause => + filter.op === 'TEMPORAL_RANGE' && + (columnLabel === undefined || + getColumnLabel(filter.col) === columnLabel), + )?.val; + } catch { + return undefined; + } + return typeof timeRange === 'string' && + timeRange.trim() && + timeRange !== NO_TIME_RANGE + ? timeRange + : undefined; +}; + +const TIMEZONE_SUFFIX_RE = /(Z|[+-]\d{2}:?\d{2})$/; + +interface TimeBound { + value: string; + time: number; +} + +const toBound = (value: string | undefined): TimeBound | undefined => { + if (!value) { + return undefined; + } + const time = new Date( + TIMEZONE_SUFFIX_RE.test(value) ? value : `${value}Z`, + ).getTime(); + return Number.isNaN(time) ? undefined : { value, time }; +}; + +/** + * Intersect a bucket time range ("since : until", naive ISO datetimes) with + * resolved outer bounds. An empty outer bound is unbounded on that side. A + * bucket that does not overlap the outer range yields an empty range. + */ +export const intersectTimeRanges = ( + bucket: string, + outerSince: string | undefined, + outerUntil: string | undefined, +): string => { + const [bucketSince, bucketUntil] = bucket.split(TIME_RANGE_SEPARATOR); + let since = toBound(bucketSince); + let until = toBound(bucketUntil); + if (!since || !until) { + return bucket; + } + const outerStart = toBound(outerSince); + const outerEnd = toBound(outerUntil); + if (outerStart && outerStart.time > since.time) { + since = outerStart; + } + if (outerEnd && outerEnd.time < until.time) { + until = outerEnd; + } + return since.time < until.time + ? `${since.value}${TIME_RANGE_SEPARATOR}${until.value}` + : `${since.value}${TIME_RANGE_SEPARATOR}${since.value}`; +}; + +/** + * Resolve the outer time range (which may be relative, e.g. "Last year") and + * return the part of the bucket that lies inside it, so a bucket that is + * only partly inside the chart's or dashboard's range does not return rows + * the chart never displayed. Falls back to the bucket if the range cannot be + * resolved. + */ +export const fetchDrillByTimeRange = async ( + bucket: string, + outerTimeRange: string, +): Promise<string> => { + try { + const { json } = await SupersetClient.get({ + endpoint: `/api/v1/time_range/?q=${rison.encode_uri(outerTimeRange)}`, + }); + const result = ensureIsArray(json?.result)[0]; + if (!result) { + return bucket; + } + return intersectTimeRanges(bucket, result.since, result.until); + } catch { + return bucket; + } +}; + +/** + * Make a drilled query keep the clicked temporal bucket. + * + * The backend replaces the value of every TEMPORAL_RANGE filter with the + * query's time_range, so a bucket sent only as a TEMPORAL_RANGE filter is + * widened to the chart's (or dashboard's) whole time range. Carrying the + * bucket as the query's time_range keeps it. A dashboard time range arrives + * through extra_form_data, which overrides time_range, so it is replaced + * there as well. Legacy dashboard filters can also override the range + * through extra_filters, so their reserved __time_range value is replaced. + * + * TEMPORAL_RANGE is kept, rather than a >= / < pair, because only the + * TEMPORAL_RANGE path applies the dataset's timezone or Hours offset to the + * bounds. + */ +export const applyDrillByTimeRange = <T extends JsonObject>( + formData: T, + timeRange: string | undefined, +): T => { + if (!timeRange) { + return formData; + } + const extraFormData = formData.extra_form_data as JsonObject | undefined; + const extraFilters = formData.extra_filters as QueryFormData['extra_filters']; Review Comment: I overstated the saved/dashboard-range coverage in my earlier reply. That change could reactivate the other date column's saved comparator even when the chart had overridden it, and nested drills could lose the outer bucket's clip. Fixed in df626c43a4e33b496ec911c53399d2faca7dd9a2: I materialize the original chart's effective query-wide range on its existing temporal filters before removing that override. This includes saved ranges, `No filter`, native dashboard ranges and legacy ranges. Appended drill filters remain additional constraints. Every temporal bucket retains its clipped range through nested drills; same-column buckets intersect, and Mixed Timeseries reads the clicked query's filter field. Regression test: `Mixed Timeseries query B drills clip against its own saved temporal range` in superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx. Additional regressions: `temporal drill retains the effective independent date range in query CSV and Explore: %j`, `nested temporal drills retain every outer bucket clip with independent date filters: %s`, and `Mixed Timeseries query B drills clip against its own saved temporal range` in superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx. I replaced the helper test that asserted the mismatch with `query-wide ranges retain the chart row set on independent dates in %s`, which also compares exact row arrays. With the production source pinned to 6c75c08436dd478fe8adab7c4eb24d8af7a36f80, the focused range/label set has 17 failures and one passing compatibility case. All pass with this commit; Docker Jest has 806 passed / 1 skipped, and Docker Python has 253 passed. Siblings checked: - superset-frontend/src/components/Chart/DrillBy/drillByTimeRange.ts — fixed in df626c43a4e33b496ec911c53399d2faca7dd9a2 - superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx — fixed in df626c43a4e33b496ec911c53399d2faca7dd9a2 - superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx — not affected: CSV/XLSX and reload share the modal's normalized data, with format-only wrappers. - superset-frontend/src/explore/exploreUtils/formData.ts — not affected: Edit chart POST carries those same filters; the regression asserts its payload. - superset/commands/explore/get.py — not affected: the unchanged native/legacy merge preserves materialized filters, pinned by test_drilled_temporal_ranges_survive_explore_filter_merge in tests/unit_tests/utils/test_core.py. - superset-frontend/src/explore/actions/saveModalActions.ts — not affected: saved-from-drill charts serialize the merged form data unchanged. - superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterOption/index.tsx — not affected: Explore pills read the merged comparator; the payload and merge contracts are covered, and its Jest suite passes. - superset-frontend/src/components/Chart/DrillDetail/utils.ts — not affected: separate detail payload builder and unchanged detail-event filters; the drill-by modal exposes only Drill by. Its suites pass. Remaining: fresh live UI/SQL acceptance was not run because the localhost health endpoint was unavailable. The existing time-range-resolution error fallback is unchanged. -- 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]
