Keep treatments tracked when the local day differs from UTC's - #879
Open
CoryMCodes wants to merge 1 commit into
Open
Keep treatments tracked when the local day differs from UTC's#879CoryMCodes wants to merge 1 commit into
CoryMCodes wants to merge 1 commit into
Conversation
Treatments follow a user forward via a Tracking record, not via the check-in
itself: adding one writes it onto today's check-in and creates a Tracking so
Checkin::Creator seeds it into every check-in after that. Only the second half
is gated on `isTodaysCheckin`, so when that answered `false` the treatment was
saved for today and silently never tracked.
A check-in's `date` is a calendar day wearing a UTC costume. The client asks for
one by the browser's local day ("2022-01-24", routes/checkin/index.js) and
Api::V1::CheckinsController#create stores that day stamped with the server's own
UTC wall clock, so it comes back as "2022-01-24T01:00:00.000+00:00". Only the
24th means anything. Reading it as an instant re-dates the check-in for anyone
off UTC - in Los Angeles it becomes the 23rd - and `isTodaysCheckin` said no.
The window this hits is a function of the clock, not of the treatment, which is
why some treatments appeared to stay permanently and others did not:
UTC none Asia/Kolkata 6h (00:00-06:00)
Europe/Berlin 1h America/Los_Angeles 8h (16:00-24:00)
America/New_York 5h Australia/Sydney 11h (00:00-11:00)
Read the day as written instead, via moment.parseZone, and treat a check-in with
no date as never being today so a half-loaded record cannot be tracked against.
Also anchor the tracking lookup window to now. TrackingsController#create stamps
`start_at` with the server's clock, so anchoring the lookup to the check-in's
stored date puts it a day out for the same users, and `untrack` then cannot find
the tracking it is meant to remove - a treatment the user deleted would come
back tomorrow. Without this, fixing the above only makes that reachable.
The tests pin moment's default zone, because this bug is invisible in UTC and CI
runs in UTC; without pinning, a regression would go unnoticed.
Reported twice by users: treatments not staying on the treatments page for
future check-ins, and having to re-enter past treatments every time.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Treatments follow a user forward via a Tracking record, not via the check-in itself: adding one writes it onto today's check-in and creates a Tracking so Checkin::Creator seeds it into every check-in after that. Only the second half is gated on
isTodaysCheckin, so when that answeredfalsethe treatment was saved for today and silently never tracked.A check-in's
dateis a calendar day wearing a UTC costume. The client asks for one by the browser's local day ("2022-01-24", routes/checkin/index.js) and Api::V1::CheckinsController#create stores that day stamped with the server's own UTC wall clock, so it comes back as "2022-01-24T01:00:00.000+00:00". Only the 24th means anything. Reading it as an instant re-dates the check-in for anyone off UTC - in Los Angeles it becomes the 23rd - andisTodaysCheckinsaid no.The window this hits is a function of the clock, not of the treatment, which is why some treatments appeared to stay permanently and others did not:
UTC none Asia/Kolkata 6h (00:00-06:00)
Europe/Berlin 1h America/Los_Angeles 8h (16:00-24:00)
America/New_York 5h Australia/Sydney 11h (00:00-11:00)
Read the day as written instead, via moment.parseZone, and treat a check-in with no date as never being today so a half-loaded record cannot be tracked against.
Also anchor the tracking lookup window to now. TrackingsController#create stamps
start_atwith the server's clock, so anchoring the lookup to the check-in's stored date puts it a day out for the same users, anduntrackthen cannot find the tracking it is meant to remove - a treatment the user deleted would come back tomorrow. Without this, fixing the above only makes that reachable.The tests pin moment's default zone, because this bug is invisible in UTC and CI runs in UTC; without pinning, a regression would go unnoticed.
Reported twice by users: treatments not staying on the treatments page for future check-ins, and having to re-enter past treatments every time.
fixes #581