Skip to content

Fixed Drill-to-detail in TableChart not providing the correct dates b… #36318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fixed Drill-to-detail in TableChart not providing the correct dates b… #36318

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 104 additions & 10 deletions superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
t,
tn,
extractTextFromHTML,
TimeGranularity,
} from '@superset-ui/core';
import { styled, css, useTheme, SupersetTheme } from '@apache-superset/core/ui';
import { GenericDataType } from '@apache-superset/core/api/core';
Expand Down Expand Up @@ -298,6 +299,78 @@ function SelectPageSize({
const getNoResultsMessage = (filter: string) =>
filter ? t('No matching records found') : t('No records found');

//Calculates the end time based on the granularity
function getEndTimeFromGranularity(
startTime: Date,
granularity?: TimeGranularity,
): Date {
if (!granularity) {
return startTime;
}

const time = startTime.getTime();
const date = startTime.getUTCDate();
const month = startTime.getUTCMonth();
const year = startTime.getUTCFullYear();

// Constants for time calculations
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:
case 'PT1S':
return new Date(time + MS_IN_SECOND);
case TimeGranularity.MINUTE:
case 'PT1M':
return new Date(time + MS_IN_MINUTE);
case TimeGranularity.FIVE_MINUTES:
case 'PT5M':
return new Date(time + MS_IN_MINUTE * 5);
case TimeGranularity.TEN_MINUTES:
case 'PT10M':
return new Date(time + MS_IN_MINUTE * 10);
case TimeGranularity.FIFTEEN_MINUTES:
case 'PT15M':
return new Date(time + MS_IN_MINUTE * 15);
case TimeGranularity.THIRTY_MINUTES:
case 'PT30M':
return new Date(time + MS_IN_MINUTE * 30);
case TimeGranularity.HOUR:
case 'PT1H':
return new Date(time + MS_IN_HOUR);
case TimeGranularity.DAY:
case TimeGranularity.DATE:
case 'P1D':
case 'date':
return new Date(Date.UTC(year, month, date + 1));
case TimeGranularity.WEEK:
case TimeGranularity.WEEK_STARTING_SUNDAY:
case TimeGranularity.WEEK_STARTING_MONDAY:
case 'P1W':
case '1969-12-28T00:00:00Z/P1W':
case '1969-12-29T00:00:00Z/P1W':
return new Date(Date.UTC(year, month, date + 7));
case TimeGranularity.WEEK_ENDING_SATURDAY:
case TimeGranularity.WEEK_ENDING_SUNDAY:
case 'P1W/1970-01-03T00:00:00Z':
case 'P1W/1970-01-04T00:00:00Z':
return new Date(Date.UTC(year, month, date + 1));
case TimeGranularity.MONTH:
case 'P1M':
return new Date(Date.UTC(year, month + 1, 1));
case TimeGranularity.QUARTER:
case 'P3M':
return new Date(Date.UTC(year, Math.floor(month / 3) * 3 + 3, 1));
case TimeGranularity.YEAR:
case 'P1Y':
return new Date(Date.UTC(year + 1, 0, 1));
default:
return new Date(Date.UTC(year, month, date + 1));
}
}
Comment on lines +303 to +372
Copy link
Contributor

@bito-code-review bito-code-review bot Nov 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect time range calculation

The getEndTimeFromGranularity function returns the start of the next period instead of the end of the current period, causing incorrect time ranges in drill-to-detail for temporal columns. For example, for DAY granularity, it returns the start of the next day rather than 23:59:59.999 of the current day. This matches the createTimeRangeFromGranularity utility in @superset-ui/core, which handles inclusive ranges properly.

Code Review Run #c735a1

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them


export default function TableChart<D extends DataRecord = DataRecord>(
props: TableChartTransformedProps<D> & {
sticky?: DataTableProps<D>['sticky'];
Expand Down Expand Up @@ -534,18 +607,39 @@ export default function TableChart<D extends DataRecord = DataRecord>(
clientX: number,
clientY: number,
) => {
const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
filteredColumnsMeta.forEach(col => {
if (!col.isMetric) {
let dataRecordValue = value[col.key];
dataRecordValue = extractTextFromHTML(dataRecordValue);

drillToDetailFilters.push({
col: col.key,
op: '==',
val: dataRecordValue as string | number | boolean,
formattedVal: formatColumnValue(col, dataRecordValue)[1],
});
const dataRecordValue = value[col.key];

// Handle temporal columns differently to support time ranges
if (col.dataType === GenericDataType.Temporal && timeGrain) {
// Make sure the value is a Date
const startTime = dataRecordValue instanceof Date
? dataRecordValue
: new Date(dataRecordValue as string | number);

// Calculate the end time based on the granularity
const endTime = getEndTimeFromGranularity(startTime, timeGrain);

const timeRangeValue = `${startTime.toISOString()} : ${endTime.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
drillToDetailFilters.push({
col: col.key,
op: '==',
val: dataRecordValue as string | number | boolean,
formattedVal: formatColumnValue(col, dataRecordValue)[1],
});
}
}
});
onContextMenu(clientX, clientY, {
Expand Down
Loading