aminghadersohi commented on code in PR #45612: URL: https://github.com/apache/superset/pull/45612#discussion_r4363119715
########## 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: Fixed in 6c75c08436dd478fe8adab7c4eb24d8af7a36f80. I reproduced the query-wide range overwriting an independent date column. The suggested no-range guard prevents that case; the change also protects saved/dashboard ranges and outer buckets on other date columns by retaining the resolved overlap on the clicked filter instead of sending a query-wide override when independent temporal filters exist. Regression test: `Sunburst Drill by preserves an independent date filter without a saved time range` in `superset-frontend/src/components/Chart/DrillBy/DrillByTemporalFilters.test.tsx`. The `temporal drill preserves an independent date filter in query and CSV with saved range %s` cases in `DrillByModal.test.tsx` cover absent, No filter, and bounded saved ranges, checking chart-data, CSV export and Edit chart payloads. The helper's two filter-field cases cover dashboard/legacy overrides and nested ranges, including Mixed Timeseries query B. These regressions fail with the starting source and pass with this commit; the complete focused comparison is 17 failing before / 17 passing after. Siblings checked: - superset-frontend/src/components/Chart/DrillBy/drillByTimeRange.ts:191 — fixed in 6c75c08436dd478fe8adab7c4eb24d8af7a36f80 - superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx:419 — fixed in 6c75c08436dd478fe8adab7c4eb24d8af7a36f80 - superset-frontend/src/components/Chart/DrillBy/DrillByTemporalFilters.test.tsx — fixed in 6c75c08436dd478fe8adab7c4eb24d8af7a36f80 - superset-frontend/src/components/Chart/DrillBy/useResultsTableView.tsx:34 — not affected: table reload delegates to the modal's normalized payload; there is no second range override. - superset-frontend/src/explore/exploreUtils/formData.ts:47 — not affected: Edit chart serializes the same normalized form data, asserted by the regression. - superset-frontend/src/explore/exploreUtils/index.ts — not affected: CSV/XLSX export receives the same normalized form data through handleDownload; the CSV regression checks it. - superset/common/query_context_factory.py:437 — not affected: backend rewriting is unchanged; these independent-column drill queries omit its query-wide override. Searches: `rg -n 'applyDrillByTimeRange|getDrillByOuterTimeRange|drilledFormData|handleDownload|handleReload' superset-frontend/src/components/Chart/DrillBy`; `rg -n 'exportChart|postFormData' superset-frontend/src/explore/exploreUtils`. -- 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]
