Fixed Drill-to-detail in TableChart not providing the correct dates b… #36318
Conversation
…y using a Time Range.
dosubot
bot
added
dashboard:drill-down
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #c735a1
Actionable Suggestions - 1
-
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx - 1
- Incorrect time range calculation · Line 303-372
Additional Suggestions - 1
-
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx - 1
-
Missing HTML extraction for non-temporal data · Line 635-641For non-temporal columns, the code no longer calls extractTextFromHTML on dataRecordValue before using it in the filter val, unlike the previous version. This could cause issues if data contains HTML markup, leading to incorrect drill-to-detail filters.
Code suggestion
--- superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx +++ superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx @@ -635,7 +635,7 @@ } else { // Non-temporal columns use exact match drillToDetailFilters.push({ - col: col.key, + col: col.key, op: '==', - val: dataRecordValue as string | number | boolean, + val: extractTextFromHTML(dataRecordValue) as string | number | boolean, formattedVal: formatColumnValue(col, dataRecordValue)[1], }); }
-
Review Details
-
Files reviewed - 1 · Commit Range:
0a918b1..0a918b1- superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
AI Code Review powered by
| 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)); | ||
| } | ||
| } |
There was a problem hiding this comment.
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
|
@msyavuz and I thought this was fixed... but if there was a regression, we should add some tests this time around to harden it :) Thanks for the contribution! |
2 participants
…y using a Time Range.
fix(dashboard): Drill to detail on date-time columns returns correct range of values
SUMMARY
Fix for Issue: 23847
Unexpected Behaviour: For table charts with time grains (year, month, day, hour, etc): when the user drills down by a date-time column they do not get the expected records. For example, if the time grain is month, and the user selects “Drill to detail by month” the resulting table contains records for where the date-time column equals “yyyy-mm-01 00:00:00z”. So, only the records that have year, month, and first day yyyy-mm-01 with exactly the start of the first day 00:00:00z, are returned. In the expected output, we want to include all the records that have the same selected yyyy-mm regardless of the day and time.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before Fix:
After Fix:
TESTING INSTRUCTIONS
How to verify the fix:
ADDITIONAL INFORMATION