Skip to content

Add controller-level tests for timezone handling in Query#5809

Merged
aerosol merged 5 commits intomasterfrom
query-timezones
Oct 23, 2025
Merged

Add controller-level tests for timezone handling in Query#5809
aerosol merged 5 commits intomasterfrom
query-timezones

Conversation

@ukutaht
Copy link
Copy Markdown
Contributor

@ukutaht ukutaht commented Oct 16, 2025

From conversations with @aerosol we probably want to refactor Query internals to use NaiveDateTime so we don't carry around useless and confusing timezone information in the Query datetimes.

This PR adds some tests that give guardrails and increase confidence in timezone behaviour that is observable to clients:

  • Relative date handling: today should be the current day in the site's timezone, not utc. Previously untested.
  • Ambigous and gap handling in timezone conversions: Currently tested via unit tests internally which also use DateTime over Naive. Doesn't give much confidence for refactoring. New controller-level tests only test observable behaviour, not internals, helping much more with refactoring.
  • Legacy time on page cutoff: the cutoff is shifted to site timezone with ambigous/gap handling. I don't know if this is really necessary, a couple hours here or there with the cutoff should not have noticeable effects on data. We could move this to Naive and ignore any timezones in cutoff handling. But I wanted to cover this behaviour with tests so we can be intentional about it in refactoring.

@ukutaht ukutaht requested a review from aerosol October 16, 2025 09:59
Copy link
Copy Markdown
Member

@aerosol aerosol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! I admit I haven't checked if the comments are true though 😅

@ukutaht
Copy link
Copy Markdown
Contributor Author

ukutaht commented Oct 20, 2025

Nice one! I admit I haven't checked if the comments are true though 😅

Fair, but I've verified that each test has a corresponding piece of production code that will cause the test to fail if the implementation changes. So we know that the tests are actually testing something :)

@ukutaht ukutaht added this pull request to the merge queue Oct 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 20, 2025
@aerosol aerosol added this pull request to the merge queue Oct 23, 2025
@aerosol
Copy link
Copy Markdown
Member

aerosol commented Oct 23, 2025

I'll merge since it fell out of the queue

Merged via the queue into master with commit 9e3102d Oct 23, 2025
15 checks passed
@ukutaht ukutaht deleted the query-timezones branch February 11, 2026 11:47
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.

2 participants