Skip to content

Fix DST timezone handling for viewed event weeks#228

Open
wslany wants to merge 5 commits intoschej-it:mainfrom
wslany:codex/dst-regression-proof
Open

Fix DST timezone handling for viewed event weeks#228
wslany wants to merge 5 commits intoschej-it:mainfrom
wslany:codex/dst-regression-proof

Conversation

@wslany
Copy link

@wslany wslany commented Mar 25, 2026

Fixes #227

This PR is intentionally split into several commits:

  1. 4ceda5a adds the DST regression proof, focused helper tests, and frontend CI.
  2. ece4641 applies the actual fix that makes the remaining regression pass.
  3. 5b76640 cleanup that removes a deprecated Vite CJS warning
  4. 55dd935 adds another regression test showing the correctness for fall-back DST changes, and another test for the non-edge case where the standard offset is kept during non-DST viewed weeks

I split it this way so review can follow a clear red/green sequence:

  • first, prove the bug exists and that the regression test catches it,
  • then, apply the smallest fix that turns the remaining failing regression green.

The first commit also includes unit tests for the new helper functions so the extracted date/offset logic is documented directly in tests, not only in implementation.

@wslany
Copy link
Author

wslany commented Mar 25, 2026

Red proof for commit 4ceda5a.

At this point the branch contains:

  • the failing DST regression test,
  • focused helper tests for the new date/offset helpers,
  • frontend CI wiring.

So the first commit is intentionally structured with:

  • helper tests green,
  • the user-facing DST regression still red.

Local result at commit:
npm run test:unit

  • 4 passed
  • 1 failed

Failing case:
shows the viewed event week using that week's timezone offset

That failing result is intentional here: it demonstrates the bug exists and that the regression test really catches it before the fix is applied.

failing DST regression test

@wslany
Copy link
Author

wslany commented Mar 25, 2026

Green proof for commit ece4641.

This second commit wires the scheduling view and timezone selector to the new reference-date helpers so both the scheduling math and the visible GMT label are derived from the viewed event week instead of the current machine date.

Local result at commit:
npm run test:unit

  • 5 passed

I also verified:
npm run build

  • build succeeds

So the red/green sequence is now:

  • commit 1: helper tests green, DST regression red
  • commit 2: all tests green after the actual fix

succeeding DST regression test

@wslany wslany force-pushed the codex/dst-regression-proof branch from 03c6413 to 5b76640 Compare March 25, 2026 15:53
@wslany
Copy link
Author

wslany commented Mar 25, 2026

I added one small follow-up cleanup commit: vitest.config.js is now vitest.config.mjs, which removes the deprecated Vite CJS warning from the test output without changing behavior.

image

@wslany
Copy link
Author

wslany commented Mar 26, 2026

I added one more small follow-up test commit so the viewed-week timezone tests now cover:

  • a normal non-DST case,
  • the spring-forward transition,
  • the fall-back transition.

That gives a bit more confidence that the fix is not just handling one DST edge case, but the general viewed-week offset behavior.

image

@maxwellward
Copy link
Contributor

Looks good to me!

@wslany wslany requested a review from maxwellward March 26, 2026 17:44
@wslany
Copy link
Author

wslany commented Mar 26, 2026

@wslany wslany requested a review from maxwellward

Sorry, my mistake, please ignore the re-request. And many thanks for the feedback!

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.

Showing wrong timezone when scheduling across a DST change

2 participants