Conversation
Reviewer's GuideReporting and export boundaries now follow each organization’s configured IANA timezone, using aware half-open datetime filters and timezone-aware aggregation/date rendering; month defaults and scheduled reports resolve ranges from the organization’s local calendar, with Los Angeles retained as the fallback. Sequence diagram for organization-local report boundariessequenceDiagram
participant Caller as Report caller
participant View as Report view or resolver
participant Resolver as organization_time_zone
participant Selectors as Report selectors
participant DB as Note database
participant Export as NoteResource
Caller->>View: Request report range
View->>Resolver: organization_time_zone(organization)
Resolver-->>View: Organization ZoneInfo or default
View->>Selectors: note_list_for_org(start_date, end_date, tz)
Selectors->>DB: Filter interacted_at >= local midnight
Selectors->>DB: Filter interacted_at < next local midnight
DB-->>Selectors: Matching notes
Selectors->>DB: TruncDate(interacted_at, tz=tz)
Selectors-->>View: Report data
View->>Export: NoteResource(tz=tz)
Export-->>Caller: CSV dates rendered in organization timezone
Flow diagram for local month range selectionflowchart TD
Now[Current instant] --> LocalDate[Convert now to organization timezone]
LocalDate --> Month[Select current or previous local calendar month]
Month --> Dates[Inclusive start and end dates]
Dates --> Bounds[Build aware half-open datetime range]
Bounds --> Query[Filter organization notes]
Query --> Buckets[Aggregate dates with TruncDate using organization timezone]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="apps/betterangels-backend/reports/tasks.py" line_range="54" />
<code_context>
# Calculate the date range for the previous month
- start_date, end_date = get_previous_month_range()
+ start_date, end_date = get_previous_month_range(tz=organization_time_zone(report.organization))
month_str = start_date.strftime("%m")
year_str = start_date.strftime("%Y")
</code_context>
<issue_to_address>
**issue (bug_risk):** A scheduled report that runs at or just after 00:00 UTC on the first day of a month can select the month before the immediately preceding calendar month for an America/Los_Angeles organization. For example, at 00:00 UTC on January 1 the organization's local date is still December 31, so the function reports November instead of December.
**Triggers:** When the Celery schedule fires before the organization's local month has rolled over.
**Suggested fix:** Base the scheduled report period on the intended scheduled run month, or schedule/guard execution using each organization's local timezone rather than deriving the period from the task's current UTC instant.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
6511ba7 to
9f6c400
Compare
9e6a8a2 to
b3e85c7
Compare
9f6c400 to
69df125
Compare
b3e85c7 to
2b78694
Compare
69df125 to
f0f5a36
Compare
f0f5a36 to
642496a
Compare
642496a to
5ad17a3
Compare
5ad17a3 to
d7d311d
Compare
d7d311d to
036347b
Compare
2b78694 to
ca8f224
Compare
|
🚀 Expo continuous deployment is ready for betterangels!
iOS Simulator Build: Simulator Build Link |
|
🔍 [shelter-web] Preview available at: https://shelter.dev.betterangels.la/branches/DEV-warnings-report-timezones Last updated: 2026-09-01T19:53:59.271Z |
|
🔍 [betterangels-admin] Preview available at: https://admin.dev.betterangels.la/branches/DEV-warnings-report-timezones Last updated: 2026-09-01T19:53:59.268Z |
ca8f224 to
4f0643e
Compare
…me zone Report ranges were built on UTC calendar days, so "January" ran from 4pm on 31 December to 4pm on 31 January in Los Angeles. A note logged on the last evening of a month fell into the next month's report. In production 9 of 3,850 live notes sit in that window, all on the last evening of a month. The monthly schedule had the same fault. `hour` was documented as UTC, so a report due on the 1st fired at 00:00 UTC — 5pm on the last day of the month locally — and missed everything logged that evening. `set_next_run` now places `day_of_month`/`hour` on the operating zone's calendar, and a data migration recomputes `next_run_at` for active reports. Separately, when a report ran decided what it contained. `send_scheduled_report` derived its month from `timezone.now()`, so a schedule due 1 September but retried on 2 October emailed September — with a September subject — and August was never sent. The period now comes from the run being serviced, and `next_run_at` is recomputed from `day_of_month` rather than by adding a month to a stored instant, so a late, early or retried run all produce the same month. Exports are split by who they are for. The admin's `NoteResource` labelled rows with `interacted_at.date()` — the UTC date, ignoring every zone — and now uses `timezone.localtime`, so it follows the zone `TimezoneMiddleware` activates from the browsing user's cookie, which is what someone downloading from the admin wants. A report is a record rather than a view, so `ReportNoteResource` labels rows on the same calendar the range was cut on; otherwise the same month exports as two different files depending on who asked, and neither matches the emailed copy. The zone travels inside the values rather than beside them. `local_window` widens an inclusive pair of dates into the aware, half-open range the ORM filters on, and `note_list_for_org` takes that range — the same shape `shelters.selectors.reports.daily_occupancy` already uses. Nothing passes a `tzinfo` argument. `SHELTER_SCHEDULE_TIME_ZONE` was already this value under a name describing one of its callers, so it moves to `common.constants.OPERATING_TIME_ZONE` and reports uses the same one — two hardcoded zones become one. `hmis`'s `LOS_ANGELES_TZ` stays: it is the vendor API's convention for parsing their naive datetimes, not our operating zone. `TIME_ZONE` remains UTC. Storage and display are unaffected — datetimes are stored and served in UTC and localized per viewer by each client. Only the boundaries of an aggregate are local, because those decide which rows are counted rather than how an instant is shown. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…a constant
Replaces the hardcoded `OPERATING_TIME_ZONE` this branch introduced with
Django's own request-scoped mechanism, fixing the same bugs with no zone named
anywhere in application code.
`TIME_ZONE` supplies the default -- `USE_TZ` is what keeps storage UTC, and
conflating the two is what made the framework's own knob look unavailable.
`TimezoneMiddleware` was already in MIDDLEWARE reading a `django_timezone`
cookie that only the Django admin ever set; the web fetch client now publishes
it too, so the chart, the totals and the downloaded CSV agree with the calendar
the reader is on. Every conversion downstream then takes no argument at all:
`TruncDate("interacted_at")`, `make_aware`, `localdate`, `localtime`.
`set_next_run` deliberately reads `get_default_timezone()` instead. It runs
from an admin save and from Celery alike, so following the request's zone would
let a report set to 8am by a remote admin drift to 8am here on its first
reschedule; a schedule fires once, globally, and has no viewer.
Shelter hours and occupancy days stay pinned to the site's zone for the same
reason -- a shelter is open on its own clock whoever is looking it up. The Expo
app deliberately does not publish the cookie: it runs no reports queries, and
`openNow` must ignore the viewer.
Deletes `OPERATING_TIME_ZONE`, `local_window`, `local_today` and
`ReportNoteResource`. `NoteResource` already serves both audiences, following
the viewer on a request and the site zone in Celery.
Five tests outside `reports/` were asserting UTC results from naive
`time_machine.travel(...)` strings, silently riding on the process TZ. Their
travel targets are now explicit UTC.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4d0ed23 to
eb2e8f0
Compare
The bug
Calendar days were cut on UTC, in three places that have to agree and didn't.
Report ranges. Bare
dateobjects handed tointeracted_atfilters were coerced to naive midnight and read withsettings.TIME_ZONE, so every range ran UTC midnight to UTC midnight — Dec 31 4pm → Jan 31 4pm in Los Angeles. A note logged at 5pm on 31 January was counted in February's report, and the CSV labelled it02/01/2025. That coercion also raised the ~40RuntimeWarning: received a naive datetimewarnings this PR set out to clear.The schedule.
ScheduledReport.hourwas read on UTC's calendar, so the one active report — due on the 1st at hour 0 — fired at00:00 UTC, which is 5pm on the last day of the month in Los Angeles. It went out before the month it reports on had ended locally.The period.
send_scheduled_reportderived its month fromtimezone.now(), so a schedule due 1 September but retried on 2 October emailed September, with a September subject, and August was never sent at all.The fix: let Django resolve the calendar
USE_TZ = Trueis what keeps storage UTC.TIME_ZONEis only the calendar to use when nobody said otherwise — we had been reading it as the former, which is why the zone looked like something we had to carry by hand.common/middleware/timezone.pywas already inMIDDLEWARE, already reading adjango_timezonecookie, already callingtimezone.activate()— and onlytemplates/admin/base.htmlever set that cookie.createWebFetchClientnow sets it too, which is the entire frontend change.With the zone activated at the boundary, every conversion resolves itself:
TruncDate("interacted_at")get_current_timezone_name()timezone.make_aware(...)timezone.localdate()/localtime()timezone.get_default_timezone()settings.TIME_ZONE, ignoring the active zoneSHELTER_SCHEDULE_TIME_ZONEis deleted — the one hardcoded zone in application code this touches.hmis/api_bridge.py'sLOS_ANGELES_TZstays: it is the zone the HMIS vendor emits its naive timestamps in, a property of their system rather than of our deployment, and it must not followTIME_ZONE.Three decisions worth checking
set_next_runignores the request and usesget_default_timezone(). It runs from an admin save and from Celery after a send, so following the active zone would let a report set to 8am by a remote admin drift to 8am here on its first reschedule. A schedule fires once, globally — it has no viewer. Pinned bytest_the_schedule_ignores_the_timezone_of_whoever_saved_it.Shelters stay pinned,
openNowand occupancy alike: a shelter is open on its own clock whoever is looking it up. That is also why the Expo app does not publish the cookie — it runs no reports queries, and this is the one mobile-facing thing that reads a calendar.The CSV date column follows the viewer.
NoteResource.dehydrate_interacted_atgoes fromnote.interacted_at.date()totimezone.localtime(note.interacted_at)— the viewer's zone on a request,settings.TIME_ZONEin Celery. One class serves both audiences.Why the boundary and the firing time move together
Fixing only the ranges is worse than fixing neither:
Case B skips a month, so a data migration recomputes
next_run_atfor active reports —set_next_runonly corrects itself after a send, which is one wrong-month email too late. The period itself now comes from the run being serviced rather than the wall clock, so a late, early or retried run all produce the same month.Verified rather than assumed
connection.timezone_namereturns"UTC"wheneverUSE_TZ(django/db/backends/base/base.py:162), confirmed under an override..isoformat()on the UTC-aware value, neverlocaltime.app.conf.timezone == "UTC",enable_utc == True, noCELERY_TIMEZONE, and Celery's Django fixup never readssettings.TIME_ZONE.crontab(hour=7)stays where it was..toISOString(),hmis/api_bridge.pysetstzinfoexplicitly, and every otherstrptimeyields adate/time.EXPLAIN: the half-open window keeps thenotes_note_interacted_atindex, whereinteracted_at__datewould have cost a Seq Scan. That is whynote_list_for_orgbuilds a window instead of using the__datelookup.Tests
1561 passed, 31 skipped, and zero
RuntimeWarning: received a naive datetime. The new coverage exercises the mechanism end-to-end — a request carryingdjango_timezonethrough both the GraphQL resolver and the DRF export — rather than the selector in isolation. Every new assertion was mutation-tested: revert the behaviour, confirm it fails, restore.Five tests outside
reports/were asserting UTC results from naivetime_machine.travel(...)strings, silently riding on the processTZ. Their travel targets are now explicit UTC rather than worked around.Known, not fixed here
DateCountType.dateis a bareString!, so a bucketed date reaches the client with no indication of which calendar produced it — and that calendar is now per-viewer. Harmless while every org is in LA County; the fix when it matters is an optionaltimeZoneargument the response echoes back.clients/models.pycomputes age fromtimezone.now().date(), i.e. UTC's calendar — a latent off-by-one for seven hours a day.timezone.localdate()is the fix.celery.py'scrontab(minute=0, hour=7), # 7 AM UTC, corresponds to midnight PTis only true during daylight saving.Summary by Sourcery
Align reporting boundaries, exports, and monthly schedules with the appropriate local calendar instead of UTC.
New Features:
Bug Fixes:
Enhancements:
Tests: