Premium Analytics: type DateRange as a zoned instant - #52065
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 4 files.
|
canStepForward and clampRangeEndToToday still took a plain Date for now while drillDateRange required a TZDate, so the same package asked for the site's clock in one signature and accepted the browser's in two others. Narrow both, plus END_OF_BUCKET alongside the ADD_BY_UNIT it parallels, and zone the call sites the narrowing catches. Re-export TZDate from datetime so a consumer naming a DateRange bound does not take its own direct dependency on @date-fns/tz, and point the three production importers at that boundary. Drop PickerRange, which had become a docblock-less twin of DateRange. Sync the docs the retyping left behind: the startOfDayTZ and endOfDayTZ @return lines, the formatters README parameter tables, and the README sentence that asked callers to pass the site's now by hand.
getDateRangeSpan floors every measurement at 1, so a backwards range fell through differenceInHours to a plausible-looking one-hour window rather than to null, and stepDateRange and canStepForward then acted on it. The range params are decoded from the URL independently with no ordering check, so a hand-edited address reaches this. Pre-existing rather than introduced here: the arithmetic is unchanged by the retyping this branch does. Split out if it is better reviewed alone.
louwie17
left a comment
There was a problem hiding this comment.
Looks great, the main hold back is the unneeded unit test, and also left a suggestion around adding a comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0181D8ao2EdA5EfQMny529pj
Fixes WOOA7S-2100
Proposed changes
Keep Premium Analytics date ranges in the site's timezone so day boundaries and range lengths don't depend on the browser's timezone.
TZDate) throughout date parsing, calculations, and the date picker.Related product discussion/links
Does this pull request change what data or activity we track or use?
No. The values written to the URL and to Stats requests are serialized from the epoch, so they are byte-identical.
Testing instructions
This is a typing change with no user-visible effect, so the useful test is that the date controls still behave.
from/toshould carry the site's offset.