[scheduler] Normalize all-day recurring occurrences to whole days#22792
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@rita-codes @zannager I'd appreciate a review on this PR. Thanks! |
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
rita-codes
left a comment
There was a problem hiding this comment.
The fix looks correct and the choice to mirror processEvent is the right call, occurrences should carry the same all-day shape as the base event.
One thing before merging: the all-day flattening (allDay ? startOfDay(start) : start / allDay ? endOfDay(end) : end) now lives verbatim in two places, here and in processEvent (processEvent.ts:65-70). Two copies of the same "all-day = whole day" rule can drift apart over time. Could you extract it into a small shared util?
I think that would be nice, thank u 🙌
|
@rita-codes Good call, thanks. Extracted the all-day normalization into a shared |
|
@rita-codes One question before I wrap up: the new util is currently covered only indirectly through its two callers (the |
Thanks for the quick fix 🙌 The new helper Non-blocking: a small unit test for |
|
@rita-codes Added the dedicated unit tests for |
…i#22792) Co-authored-by: Rita <[email protected]>
Closes #22764
Summary
For an all-day event, the base event correctly ignores the time of day, so its
displayTimezoneis flattened to the start and end of the day. Its recurring occurrences did not do the same. When an all-day event was authored with non-midnight times (for example09:00to18:00), every generated occurrence kept that raw sub-day span instead of covering the whole day. This breaks the "all-day equals whole day" convention and makes the repeats inconsistent with the first instance.This is an edge case: it only triggers when an all-day event carries non-midnight times. Events created through the picker are already midnight-aligned, so they never hit it.
Root cause
processEventnormalizes only the base event'sdisplayTimezonetostartOfDayandendOfDayfor all-day events. It leavesdataTimezonewith the raw authored times. WhengetRecurringEventOccurrencesForVisibleDaysbuilds each occurrence inaddOccurrence, it projects the occurrence start and end into the display timezone but does not re-apply that all-day flattening, so the raw sub-day span leaks into the occurrence'sdisplayTimezone.Fix
In
addOccurrence, when the event is all-day, wrap the display-timezone start instartOfDayand the end inendOfDay. This mirrors exactly whatprocessEventalready does for the base event, so each occurrence'sdisplayTimezonebecomes identical in shape to the base instance (00:00:00.000to23:59:59.999[minutesInDay0to1439]).dataTimezoneis deliberately left untouched, for two reasons:dataTimezoneis also raw for all-day events.dataTimezoneto write values back into the user's model. Normalizing it there (the alternative considered in the issue) would silently overwrite the authored times on those flows. Keeping the change todisplayTimezoneonly avoids that risk, and rendering readsdisplayTimezoneexclusively, so the visible bug is fully fixed.getOccurrenceEndis unchanged.Steps to reproduce
Author an all-day recurring event with non-midnight times and expand it:
Before this change, each occurrence reports
displayTimezone.start.minutesInDay = 540andend = 1080(09:00to18:00). After, it reports0and1439(whole day), matching the base instance. InEventCalendarPremiumandEventTimelinePremium, views that derive an event's duration fromdisplayTimezoneshowed the repeats as 9-hour blocks instead of whole days; now they match the first instance.Testing
Added two regression tests to
getRecurringEventOccurrencesForVisibleDays.test.ts, covering a single-day and a multi-day all-day series authored with sub-day times. Both assert the occurrences span the whole day on each end. They fail on the current code (expected 540 to equal 0) and pass with the fix. Full package suite stays green (445 tests), typecheck, eslint, and prettier all pass.Changelog
Fix all-day recurring occurrences keeping a raw sub-day duration instead of spanning the whole day in
@mui/x-scheduler-internals-premium.