Skip to content

Fix DST day generation for specific-times events#229

Merged
jonyTF merged 3 commits intoschej-it:mainfrom
wslany:codex/dst-duplicate-sunday
Mar 27, 2026
Merged

Fix DST day generation for specific-times events#229
jonyTF merged 3 commits intoschej-it:mainfrom
wslany:codex/dst-duplicate-sunday

Conversation

@wslany
Copy link
Copy Markdown
Contributor

@wslany wslany commented Mar 25, 2026

Fixes #218

This PR is intentionally split into several commits:

  1. b51ee6e adds failing regression tests for DST transitions in specific-times events.
  2. bb6f52a fixes day generation so displayed days are derived from each date’s own local day boundary instead of one shared offset.
  3. 7f5f342 adds a non-DST control case.

The current bug affects both DST directions:

  • spring-forward can collapse later dates and drop the last day
  • fall-back can keep the old offset and shift later day boundaries by one hour

The fix avoids exact-24-hour assumptions and compares local calendar days instead.

@wslany
Copy link
Copy Markdown
Contributor Author

wslany commented Mar 25, 2026

Red proof for commit b51ee6e.

This first commit adds a focused regression test for the specific-times DST bug class before applying any fix.

It now demonstrates that the current logic is wrong in both DST directions:

  • spring-forward: later dates collapse and the last day disappears
  • fall-back: later dates keep the old offset and normalize to the wrong day boundary

Local result at commit b51ee6e:
npm run test:unit

  • 2 failed

Failing cases:

  • keeps one civil day per selected date across spring-forward DST changes
  • keeps one civil day per selected date across fall-back DST changes

That red state is intentional here: it shows the regression test really detects the bug before the implementation is changed.

image image

@wslany
Copy link
Copy Markdown
Contributor Author

wslany commented Mar 25, 2026

Green proof for commit bb6f52a.

This second commit fixes specific-times day generation by deriving each displayed day from that date’s own local day boundary in the selected timezone, instead of reusing one shared offset captured from the first date.

It also avoids exact-24-hour assumptions when checking day continuity, so both DST directions are handled by the same logic.

Local result at commit bb6f52a:
npm run test:unit

  • 2 passed

So the red/green sequence is now:

  • commit 1: both DST regressions fail
  • commit 2: both DST regressions pass
image

@wslany
Copy link
Copy Markdown
Contributor Author

wslany commented Mar 26, 2026

I added one more follow-up test commit with a non-DST control case, so the suite now checks:

  • normal periods
  • spring-forward DST
  • fall-back DST

That gives a little more confidence that the fix is not only handling edge cases, but also preserving ordinary day generation.

image

@maxwellward
Copy link
Copy Markdown
Contributor

The tests currently test assuming curTimezone is set to { value: "America/Los_Angeles" }, but it's technically possible for curTimezone to be {} at line 1478 (initially set at line 1098) as a fallback to the browser timezone setting.

Can we add a test to check for this case?

@wslany
Copy link
Copy Markdown
Contributor Author

wslany commented Mar 26, 2026

The tests currently test assuming curTimezone is set to { value: "America/Los_Angeles" }, but it's technically possible for curTimezone to be {} at line 1478 (initially set at line 1098) as a fallback to the browser timezone setting.

Can we add a test to check for this case?

Thanks, good catch.

I added a follow-up test for the curTimezone = {} path in commit 002cb94.

The helper already handled that case correctly, so this ended up being a test-only change: it now verifies that we fall back to the browser/local timezone path when curTimezone is empty.

Current local result:
npm run test:unit

  • 4 passed

@maxwellward
Copy link
Copy Markdown
Contributor

Looks good to me :)

wslany added 3 commits March 27, 2026 16:42
# Conflicts:
#	frontend/src/utils/date_utils.js
#	frontend/src/utils/date_utils.test.js
# Conflicts:
#	frontend/src/utils/date_utils.js
# Conflicts:
#	frontend/src/utils/date_utils.test.js
@wslany wslany force-pushed the codex/dst-duplicate-sunday branch from 002cb94 to a9c6007 Compare March 27, 2026 15:46
@wslany
Copy link
Copy Markdown
Contributor Author

wslany commented Mar 27, 2026

@jonyTF Hi, I fixed the merge conflicts. Many thanks for your great work!

@jonyTF
Copy link
Copy Markdown
Member

jonyTF commented Mar 27, 2026

great, thanks, will merge now :)

@jonyTF jonyTF merged commit 97a6239 into schej-it:main Mar 27, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Daylight Savings Time appears to be causing duplicate Sunday entries

3 participants