From 2daaf944892857ae90fc3f8fc1746730b2aeb7c3 Mon Sep 17 00:00:00 2001 From: Cory Musick Date: Tue, 25 Aug 2026 16:09:47 +0000 Subject: [PATCH] Keep treatments tracked when the local day differs from UTC's 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) --- .../app/components/checkin/trackables-step.js | 13 +- frontend/app/utils/checkin-day.js | 45 +++++++ .../checkin/trackables-step-test.js | 107 +++++++++++++++ frontend/tests/unit/utils/checkin-day-test.js | 123 ++++++++++++++++++ 4 files changed, 284 insertions(+), 4 deletions(-) create mode 100644 frontend/app/utils/checkin-day.js create mode 100644 frontend/tests/unit/components/checkin/trackables-step-test.js create mode 100644 frontend/tests/unit/utils/checkin-day-test.js diff --git a/frontend/app/components/checkin/trackables-step.js b/frontend/app/components/checkin/trackables-step.js index c1ce37361..030273851 100644 --- a/frontend/app/components/checkin/trackables-step.js +++ b/frontend/app/components/checkin/trackables-step.js @@ -1,7 +1,7 @@ import Ember from 'ember'; import DS from 'ember-data'; -import moment from 'moment'; import TrackablesFromType from 'flaredown/mixins/trackables-from-type'; +import { isCheckinToday } from 'flaredown/utils/checkin-day'; const { get, @@ -24,14 +24,19 @@ export default Component.extend(TrackablesFromType, { setupTracking() { this.get('tracking').setup({ - at: new Date(this.get('checkin.date')), + // TrackingsController#create stamps `start_at` with the server's own clock, + // so the window we look existing trackings up in has to be anchored to now. + // Anchoring it to the check-in's stored date puts it a day out for anyone + // whose local day differs from UTC's, and `untrack` then misses the tracking + // it is meant to remove. These lookups only happen on today's check-in. + at: new Date(), trackableType: this.get('trackableType').capitalize() }); }, checkin: alias('model.checkin'), - isTodaysCheckin: computed('checkin', function() { - return moment(this.get('checkin.date')).isSame(new Date(), 'day'); + isTodaysCheckin: computed('checkin.date', function() { + return isCheckinToday(this.get('checkin.date')); }), sortedTrackeds: computed('trackeds.{[],@each.position}', function() { diff --git a/frontend/app/utils/checkin-day.js b/frontend/app/utils/checkin-day.js new file mode 100644 index 000000000..a9d69b61a --- /dev/null +++ b/frontend/app/utils/checkin-day.js @@ -0,0 +1,45 @@ +import Ember from 'ember'; +import moment from 'moment'; + +const { + isBlank, +} = Ember; + +const CALENDAR_DAY = 'YYYY-MM-DD'; + +/* + A check-in's `date` is a calendar day, not an instant. + + The client asks for a check-in by the browser's local day ("2022-01-24", see + 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 looking like + "2022-01-24T01:00:00.000+00:00". Only the 24th is meaningful - the 01:00 and the + +00:00 are an artefact of when the server happened to handle the request. + + Reading that string as an instant re-dates the check-in for anyone who is not on + UTC: in Los Angeles it becomes the 23rd, in Sydney the 25th. `moment.parseZone` + keeps the wall clock exactly as written, so the day survives the round trip. +*/ + +// The calendar day a check-in was made for, as 'YYYY-MM-DD', or null when the +// check-in has no usable date yet. +export function checkinCalendarDay(checkinDate) { + if (isBlank(checkinDate)) { return null; } + + // ISO_8601 covers everything that reaches here - the API's timestamp and the + // bare 'YYYY-MM-DD' a freshly created record carries until it is saved - and + // keeps moment off its unreliable fall back to `new Date(string)`. + const day = moment.parseZone(checkinDate, moment.ISO_8601); + + return day.isValid() ? day.format(CALENDAR_DAY) : null; +} + +// Whether a check-in is the one for the user's current calendar day. A check-in +// with no date is never today, so a half-loaded record cannot be mistaken for one. +// +// `now` exists so tests can pin the clock; callers should omit it. +export function isCheckinToday(checkinDate, now = new Date()) { + const day = checkinCalendarDay(checkinDate); + + return day !== null && day === moment(now).format(CALENDAR_DAY); +} diff --git a/frontend/tests/unit/components/checkin/trackables-step-test.js b/frontend/tests/unit/components/checkin/trackables-step-test.js new file mode 100644 index 000000000..8eaa2f266 --- /dev/null +++ b/frontend/tests/unit/components/checkin/trackables-step-test.js @@ -0,0 +1,107 @@ +import Ember from 'ember'; +import moment from 'moment-timezone'; +import { moduleForComponent, test } from 'ember-qunit'; + +/* + Adding a treatment to today's check-in has two halves: the treatment is written + onto the check-in itself, and a Tracking is created so Checkin::Creator seeds it + into every check-in from then on. Only the second half is gated on + `isTodaysCheckin`, so when that answers `false` the treatment silently stops + following the user forward - which is what the two support reports describe. +*/ + +let trackingSetupOptions; + +moduleForComponent('checkin/trackables-step', 'Unit | Component | checkin/trackables-step', { + unit: true, + + beforeEach() { + trackingSetupOptions = null; + + this.register('service:tracking', Ember.Service.extend({ + setup(options) { trackingSetupOptions = options; } + })); + this.register('service:selectableData', Ember.Service.extend()); + }, + + afterEach() { + moment.tz.setDefault(); // back to the runner's real zone + } +}); + +function componentWithCheckinDate(context, date) { + return context.subject({ + trackableType: 'treatment', + parentView: { model: { checkin: { date } } } + }); +} + +// The API stamps the server's UTC wall clock onto the day the user picked, so an +// evening check-in in Los Angeles comes back carrying a UTC time that belongs to +// the following day. See utils/checkin-day. +function apiDateFor(day, utcClock) { + return `${day}T${utcClock}.000+00:00`; +} + +test('an evening check-in behind UTC is recognised as today', function(assert) { + moment.tz.setDefault('America/Los_Angeles'); + + const component = componentWithCheckinDate( + this, + apiDateFor(moment().format('YYYY-MM-DD'), '01:00:00') + ); + + assert.ok( + component.get('isTodaysCheckin'), + 'treatments added after 16:00 in Los Angeles must still be tracked' + ); +}); + +test('a morning check-in ahead of UTC is recognised as today', function(assert) { + moment.tz.setDefault('Australia/Sydney'); + + const component = componentWithCheckinDate( + this, + apiDateFor(moment().format('YYYY-MM-DD'), '23:00:00') + ); + + assert.ok( + component.get('isTodaysCheckin'), + 'treatments added before 11:00 in Sydney must still be tracked' + ); +}); + +test('a check-in from another day is not recognised as today', function(assert) { + const component = componentWithCheckinDate(this, apiDateFor('2016-01-06', '03:04:05')); + + assert.notOk( + component.get('isTodaysCheckin'), + 'back-filling an old check-in must not change what gets tracked going forward' + ); +}); + +test('a check-in with no date yet is not recognised as today', function(assert) { + const component = componentWithCheckinDate(this, null); + + assert.notOk(component.get('isTodaysCheckin')); +}); + +test('looks trackings up against the same clock the server stamps them with', function(assert) { + // TrackingsController#create sets `start_at` from the server's own clock, so the + // window we search for existing trackings has to be anchored to now too. Anchoring + // it to the check-in's stored date puts it a day out for anyone off UTC, and + // `untrack` then cannot find the tracking it is supposed to remove. + const before = Date.now(); + const component = componentWithCheckinDate(this, apiDateFor('2016-01-06', '03:04:05')); + + component.setupTracking(); + + const at = trackingSetupOptions.at; + + assert.ok(at instanceof Date, 'passes a Date'); + assert.ok( + at.getTime() >= before && at.getTime() <= Date.now(), + 'asks for the trackings active now, not the ones active at the check-in date' + ); + assert.equal(trackingSetupOptions.trackableType, 'Treatment'); +}); diff --git a/frontend/tests/unit/utils/checkin-day-test.js b/frontend/tests/unit/utils/checkin-day-test.js new file mode 100644 index 000000000..58b425480 --- /dev/null +++ b/frontend/tests/unit/utils/checkin-day-test.js @@ -0,0 +1,123 @@ +import moment from 'moment-timezone'; +import { module, test } from 'qunit'; +import { checkinCalendarDay, isCheckinToday } from 'flaredown/utils/checkin-day'; + +/* + A check-in's `date` is a calendar day wearing a UTC costume. + + The client asks for a check-in using the browser's local day ("2022-01-24", see + routes/checkin/index.js) and Api::V1::CheckinsController#create stamps the + server's UTC wall clock onto it, so the API hands back something like + "2022-01-24T01:00:00.000+00:00". The 01:00 and the +00:00 carry no meaning; only + the 24th does. + + These tests pin moment's default zone because the bug is invisible in UTC - which + is exactly why it survived so long, and why CI (which runs in UTC) would not + otherwise catch a regression. +*/ + +module('Unit | Utility | checkin day', { + afterEach() { + moment.tz.setDefault(); // back to the runner's real zone + } +}); + +test('reads the day as written for an evening check-in behind UTC', function(assert) { + moment.tz.setDefault('America/Los_Angeles'); + + // Checked in at 17:00 on the 24th in Los Angeles, when UTC had already ticked + // over to 01:00 on the 25th. + assert.equal(checkinCalendarDay('2022-01-24T01:00:00.000+00:00'), '2022-01-24'); +}); + +test('reads the day as written for a morning check-in ahead of UTC', function(assert) { + moment.tz.setDefault('Australia/Sydney'); + + // Checked in at 10:00 on the 24th in Sydney, when UTC was still 23:00 on the 23rd. + assert.equal(checkinCalendarDay('2022-01-24T23:00:00.000+00:00'), '2022-01-24'); +}); + +test('leaves the day alone for users already on UTC', function(assert) { + moment.tz.setDefault('UTC'); + + assert.equal(checkinCalendarDay('2022-01-24T01:00:00.000+00:00'), '2022-01-24'); + assert.equal(checkinCalendarDay('2022-01-24T23:00:00.000+00:00'), '2022-01-24'); +}); + +test('reads a bare day, as set on a record that has not reached the server yet', function(assert) { + moment.tz.setDefault('America/Los_Angeles'); + + // mixins/checkin-by-date.js#routeToNewCheckin creates the record with a plain + // 'YYYY-MM-DD' before the API fills in its own timestamp. + assert.equal(checkinCalendarDay('2022-01-24'), '2022-01-24'); +}); + +test('has no day to report when the date is missing or unparseable', function(assert) { + assert.equal(checkinCalendarDay(null), null); + assert.equal(checkinCalendarDay(undefined), null); + assert.equal(checkinCalendarDay(''), null); + assert.equal(checkinCalendarDay(' '), null); + assert.equal(checkinCalendarDay('not a date'), null); +}); + +test("an evening check-in behind UTC is still today's check-in", function(assert) { + moment.tz.setDefault('America/Los_Angeles'); + + const now = new Date('2022-01-25T01:00:00.000Z'); // 17:00 on the 24th in Los Angeles + + assert.ok( + isCheckinToday('2022-01-24T01:00:00.000+00:00', now), + 'a treatment added at 17:00 must still be tracked for future check-ins' + ); +}); + +test("a morning check-in ahead of UTC is still today's check-in", function(assert) { + moment.tz.setDefault('Australia/Sydney'); + + const now = new Date('2022-01-23T23:00:00.000Z'); // 10:00 on the 24th in Sydney + + assert.ok( + isCheckinToday('2022-01-24T23:00:00.000+00:00', now), + 'a treatment added at 10:00 must still be tracked for future check-ins' + ); +}); + +test("a past check-in is not today's check-in", function(assert) { + moment.tz.setDefault('America/Los_Angeles'); + + const now = new Date('2022-01-25T01:00:00.000Z'); // 17:00 on the 24th in Los Angeles + + assert.notOk( + isCheckinToday('2022-01-23T01:00:00.000+00:00', now), + 'back-filling yesterday must not change what gets tracked going forward' + ); +}); + +test("a future check-in is not today's check-in", function(assert) { + moment.tz.setDefault('Australia/Sydney'); + + const now = new Date('2022-01-23T23:00:00.000Z'); // 10:00 on the 24th in Sydney + + assert.notOk(isCheckinToday('2022-01-25T23:00:00.000+00:00', now)); +}); + +test('an unknown date is never today', function(assert) { + const now = new Date('2022-01-25T01:00:00.000Z'); + + assert.notOk(isCheckinToday(null, now), 'an unloaded check-in must not be tracked against'); + assert.notOk(isCheckinToday('', now)); + assert.notOk(isCheckinToday('not a date', now)); + + // moment(undefined) is *now*, so a half-loaded check-in used to look like + // today's and could have had trackings created against it. + assert.notOk(isCheckinToday(undefined)); +}); + +test('falls back to the current time when no clock is supplied', function(assert) { + moment.tz.setDefault('America/Los_Angeles'); + + const todayInLosAngeles = moment().format('YYYY-MM-DD'); + + assert.ok(isCheckinToday(`${todayInLosAngeles}T01:00:00.000+00:00`)); + assert.notOk(isCheckinToday(`${todayInLosAngeles}T01:00:00.000+00:00`, new Date('2000-01-01T12:00:00.000Z'))); +});