mirror of
https://github.com/apache/superset.git
synced 2026-05-12 19:35:17 +00:00
fix(viz): correct table chart drill-to-detail temporal boundaries and null handling (#39668)
Co-authored-by: Samuelinto <samuel.mantilla@mail.utoronto.ca> Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -50,6 +50,7 @@ import {
|
||||
getTimeFormatterForGranularity,
|
||||
BinaryQueryObjectFilterClause,
|
||||
extractTextFromHTML,
|
||||
TimeGranularity,
|
||||
} from '@superset-ui/core';
|
||||
import {
|
||||
styled,
|
||||
@@ -309,6 +310,67 @@ function SelectPageSize({
|
||||
const getNoResultsMessage = (filter: string) =>
|
||||
filter ? t('No matching records found') : t('No records found');
|
||||
|
||||
/**
|
||||
* Calculates the inclusive/exclusive temporal range for a bucket.
|
||||
* standard SQL range pattern: [start, end)
|
||||
*/
|
||||
function getTimeRangeFromGranularity(
|
||||
startTime: Date,
|
||||
granularity: TimeGranularity,
|
||||
): [Date, Date] {
|
||||
const time = startTime.getTime();
|
||||
const date = startTime.getUTCDate();
|
||||
const month = startTime.getUTCMonth();
|
||||
const year = startTime.getUTCFullYear();
|
||||
|
||||
// Constants
|
||||
const MS_IN_SECOND = 1000;
|
||||
const MS_IN_MINUTE = 60 * MS_IN_SECOND;
|
||||
const MS_IN_HOUR = 60 * MS_IN_MINUTE;
|
||||
|
||||
switch (granularity) {
|
||||
case TimeGranularity.SECOND:
|
||||
return [startTime, new Date(time + MS_IN_SECOND)];
|
||||
case TimeGranularity.MINUTE:
|
||||
return [startTime, new Date(time + MS_IN_MINUTE)];
|
||||
case TimeGranularity.FIVE_MINUTES:
|
||||
return [startTime, new Date(time + MS_IN_MINUTE * 5)];
|
||||
case TimeGranularity.TEN_MINUTES:
|
||||
return [startTime, new Date(time + MS_IN_MINUTE * 10)];
|
||||
case TimeGranularity.FIFTEEN_MINUTES:
|
||||
return [startTime, new Date(time + MS_IN_MINUTE * 15)];
|
||||
case TimeGranularity.THIRTY_MINUTES:
|
||||
return [startTime, new Date(time + MS_IN_MINUTE * 30)];
|
||||
case TimeGranularity.HOUR:
|
||||
return [startTime, new Date(time + MS_IN_HOUR)];
|
||||
case TimeGranularity.DAY:
|
||||
case TimeGranularity.DATE:
|
||||
return [startTime, new Date(Date.UTC(year, month, date + 1))];
|
||||
case TimeGranularity.WEEK:
|
||||
case TimeGranularity.WEEK_STARTING_SUNDAY:
|
||||
case TimeGranularity.WEEK_STARTING_MONDAY:
|
||||
return [startTime, new Date(Date.UTC(year, month, date + 7))];
|
||||
case TimeGranularity.WEEK_ENDING_SATURDAY:
|
||||
case TimeGranularity.WEEK_ENDING_SUNDAY:
|
||||
// Week-ending buckets are labeled by the bucket's final day.
|
||||
return [
|
||||
new Date(Date.UTC(year, month, date - 6)),
|
||||
new Date(Date.UTC(year, month, date + 1)),
|
||||
];
|
||||
case TimeGranularity.MONTH:
|
||||
return [startTime, new Date(Date.UTC(year, month + 1, 1))];
|
||||
case TimeGranularity.QUARTER:
|
||||
return [
|
||||
startTime,
|
||||
new Date(Date.UTC(year, Math.floor(month / 3) * 3 + 3, 1)),
|
||||
];
|
||||
case TimeGranularity.YEAR:
|
||||
return [startTime, new Date(Date.UTC(year + 1, 0, 1))];
|
||||
default:
|
||||
return [startTime, new Date(Date.UTC(year, month, date + 1))];
|
||||
}
|
||||
}
|
||||
|
||||
export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
props: TableChartTransformedProps<D> & {
|
||||
sticky?: DataTableProps<D>['sticky'];
|
||||
@@ -471,7 +533,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
// so that cross-filters work on the receiving chart
|
||||
const resolvedCol = columnLabelToNameMap[col] ?? col;
|
||||
const val = ensureIsArray(updatedFilters?.[col]);
|
||||
if (!val.length)
|
||||
if (!val.length || val[0] === null || (val[0] instanceof DateWithFormatter && val[0].input === null))
|
||||
return {
|
||||
col: resolvedCol,
|
||||
op: 'IS NULL' as const,
|
||||
@@ -578,15 +640,49 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
|
||||
filteredColumnsMeta.forEach(col => {
|
||||
if (!col.isMetric) {
|
||||
let dataRecordValue = value[col.key];
|
||||
dataRecordValue = extractTextFromHTML(dataRecordValue);
|
||||
const dataRecordValue = value[col.key];
|
||||
|
||||
drillToDetailFilters.push({
|
||||
col: col.key,
|
||||
op: '==',
|
||||
val: dataRecordValue as string | number | boolean,
|
||||
formattedVal: formatColumnValue(col, dataRecordValue)[1],
|
||||
});
|
||||
// FIX: Explicitly handle NULL values for temporal and non-temporal columns
|
||||
// DateWithFormatter objects wrap nulls, so we must check both
|
||||
if (
|
||||
dataRecordValue == null ||
|
||||
(dataRecordValue instanceof DateWithFormatter && dataRecordValue.input == null)
|
||||
) {
|
||||
drillToDetailFilters.push({
|
||||
col: col.key,
|
||||
op: 'IS NULL' as any,
|
||||
val: null,
|
||||
});
|
||||
|
||||
} else if (col.dataType === GenericDataType.Temporal && timeGrain) {
|
||||
const startTime =
|
||||
dataRecordValue instanceof Date
|
||||
? dataRecordValue
|
||||
: new Date(dataRecordValue as string | number);
|
||||
|
||||
const [rangeStartTime, rangeEndTime] = getTimeRangeFromGranularity(
|
||||
startTime,
|
||||
timeGrain,
|
||||
);
|
||||
const timeRangeValue = `${rangeStartTime.toISOString()} : ${rangeEndTime.toISOString()}`;
|
||||
|
||||
drillToDetailFilters.push({
|
||||
col: col.key,
|
||||
op: 'TEMPORAL_RANGE',
|
||||
val: timeRangeValue,
|
||||
grain: timeGrain,
|
||||
formattedVal: formatColumnValue(col, dataRecordValue)[1],
|
||||
});
|
||||
} else {
|
||||
// Non-temporal columns use exact match
|
||||
const sanitizedValue = extractTextFromHTML(dataRecordValue);
|
||||
drillToDetailFilters.push({
|
||||
col: col.key,
|
||||
op: '==',
|
||||
val: sanitizedValue as string | number | boolean,
|
||||
formattedVal: formatColumnValue(col, sanitizedValue)[1],
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
onContextMenu(clientX, clientY, {
|
||||
@@ -600,7 +696,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
filters: [
|
||||
{
|
||||
col: cellPoint.key,
|
||||
op: '==',
|
||||
op: (cellPoint.value == null || (cellPoint.value instanceof DateWithFormatter && cellPoint.value.input == null) ? 'IS NULL' : '==') as any,
|
||||
val: extractTextFromHTML(cellPoint.value),
|
||||
},
|
||||
],
|
||||
@@ -615,6 +711,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
|
||||
isRawRecords,
|
||||
filteredColumnsMeta,
|
||||
getCrossFilterDataMask,
|
||||
timeGrain,
|
||||
]);
|
||||
|
||||
const getHeaderColumns = useCallback(
|
||||
|
||||
@@ -2360,3 +2360,76 @@ describe('plugin-chart-table', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* DRILL-TO-DETAIL FIX VERIFICATION (#23847)
|
||||
*/
|
||||
describe('Drill-to-Detail Temporal Range Logic', () => {
|
||||
const renderChartAndOpenContextMenu = (
|
||||
timeGrain?: TimeGranularity,
|
||||
timestampValue?: string | number | null,
|
||||
) => {
|
||||
const onContextMenu = jest.fn();
|
||||
const data = cloneDeep(testData.basic);
|
||||
|
||||
if (timestampValue !== undefined) {
|
||||
data.queriesData[0].data[0].__timestamp = timestampValue;
|
||||
}
|
||||
|
||||
const props = transformProps({
|
||||
...data,
|
||||
rawFormData: {
|
||||
...data.rawFormData,
|
||||
...(timeGrain ? { time_grain_sqla: timeGrain } : {}),
|
||||
},
|
||||
hooks: { onAddFilter: jest.fn(), onContextMenu, setDataMask: jest.fn() },
|
||||
});
|
||||
render(<TableChart {...props} sticky={false} />);
|
||||
|
||||
const tbody = screen.getAllByRole('rowgroup')[1];
|
||||
fireEvent.contextMenu(tbody.querySelectorAll('td')[0]);
|
||||
|
||||
const [, , { drillToDetail }] = onContextMenu.mock.calls[0];
|
||||
return drillToDetail.find((f: any) => f.col === '__timestamp');
|
||||
};
|
||||
|
||||
test('uses TEMPORAL_RANGE for monthly grain', () => {
|
||||
const filter = renderChartAndOpenContextMenu(TimeGranularity.MONTH);
|
||||
|
||||
expect(filter.op).toBe('TEMPORAL_RANGE');
|
||||
expect(filter.val).toContain(
|
||||
'2020-01-01T12:34:56.000Z : 2020-02-01T00:00:00.000Z',
|
||||
);
|
||||
});
|
||||
|
||||
test('uses the full bucket for week ending sunday grain', () => {
|
||||
const filter = renderChartAndOpenContextMenu(
|
||||
TimeGranularity.WEEK_ENDING_SUNDAY,
|
||||
'2020-01-05T00:00:00',
|
||||
);
|
||||
|
||||
expect(filter.op).toBe('TEMPORAL_RANGE');
|
||||
expect(filter.val).toBe(
|
||||
'2019-12-30T00:00:00.000Z : 2020-01-06T00:00:00.000Z',
|
||||
);
|
||||
});
|
||||
|
||||
test('uses the full bucket for week ending saturday grain', () => {
|
||||
const filter = renderChartAndOpenContextMenu(
|
||||
TimeGranularity.WEEK_ENDING_SATURDAY,
|
||||
'2020-01-04T00:00:00',
|
||||
);
|
||||
|
||||
expect(filter.op).toBe('TEMPORAL_RANGE');
|
||||
expect(filter.val).toBe(
|
||||
'2019-12-29T00:00:00.000Z : 2020-01-05T00:00:00.000Z',
|
||||
);
|
||||
});
|
||||
|
||||
test('correctly handles NULL values by emitting IS NULL instead of 1970 timestamp', () => {
|
||||
const filter = renderChartAndOpenContextMenu(TimeGranularity.MONTH, null);
|
||||
|
||||
expect(filter.op).toBe('IS NULL');
|
||||
expect(filter.val).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user