EnxDev commented on code in PR #37561:
URL: https://github.com/apache/superset/pull/37561#discussion_r2766076477
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -251,8 +251,19 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
);
const timestampFormatter = useCallback(
- value => getTimeFormatterForGranularity(timeGrain)(value),
- [timeGrain],
+ (value: DataRecordValue) => {
+ if (value == null) {
+ return '';
+ }
+ // In Raw Records mode, don't apply time grain-based formatting
+ if (isRawRecords || !timeGrain) {
+ return String(value);
+ }
+ return getTimeFormatterForGranularity(timeGrain)(
+ value as number | Date | null | undefined,
+ );
+ },
+ [timeGrain, isRawRecords],
Review Comment:
Do you think we can avoid duplicating code here?
##########
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx:
##########
@@ -376,6 +382,52 @@ describe('plugin-chart-table', () => {
expect(percentMetric2?.originalLabel).toBe('metric_2');
});
+ it('should not apply time grain formatting in Raw Records mode', () => {
Review Comment:
Would it make sense to use test instead of it here? Also, do you think
AgGridTable should have tests?
--
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]
