rusackas commented on PR #34564:
URL: https://github.com/apache/superset/pull/34564#issuecomment-3857267801

   ## PR Updated - Rebased and Ready for Review
   
   I've rebased this PR on master and resolved the merge conflicts. Here's the 
analysis:
   
   ### Background
   
   There are multiple timezone adjustments happening in the Calendar Heatmap:
   
   1. **PR #17664** (Dec 2021) - Added offset **subtraction** in 
`getFormattedUTCTime` to fix tooltip timezone
   2. **PR #24989** (Aug 2023) - Added offset **addition** in `afterLoadData` 
(cal-heatmap.js) to fix day offset
   
   These two adjustments were working against each other, causing the day-off 
bug reported in #28931.
   
   ### The Fix
   
   This PR removes the offset subtraction from `getFormattedUTCTime` since 
`afterLoadData` already handles the UTC → local conversion for the cal-heatmap 
library. The double conversion was causing dates to shift by one day.
   
   **Before:** `afterLoadData` adds offset, then `getFormattedUTCTime` 
subtracts offset → double conversion  
   **After:** `afterLoadData` adds offset, `getFormattedUTCTime` formats 
directly → correct dates
   
   ### Changes Made
   
   1. Removed timezone offset calculation from `getFormattedUTCTime`
   2. Updated comments to explain why no offset is needed
   3. Added comprehensive tests for date handling near midnight
   
   ### Addressing @michael-s-molina's Comment
   
   The offset was introduced by PR #17664 to fix tooltip timezone issues. 
However, PR #24989 later added a separate offset adjustment in the data loading 
phase (`afterLoadData`). With both adjustments in place, dates were being 
double-converted, causing the day-off bug. This PR removes the redundant 
adjustment from `getFormattedUTCTime`, leaving the `afterLoadData` adjustment 
to handle the conversion.
   
   cc @kgabryje - would appreciate your review since you authored the original 
PR #17664 that introduced this offset. The current approach keeps your 
`afterLoadData` fix from PR #24989 and removes the now-redundant offset from 
`getFormattedUTCTime`.


-- 
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]

Reply via email to