fix(console): show prefs-page times in the browser's timezone

docs/design-decisions/022-zulip-prefs-form-design.md

@@ -44,14 +44,12 @@ Admission to being a reviewer is unchanged; only authentication moved.
 | Editable field set + validation | `core.forms.ReviewerPreferenceForm` |
 | Formset assembly, ownership scoping, label catalog | `core.services.reviewer_prefs` |
 | Field markup | `templates/shared/_reviewer_prefs_fields.html` |
-| Timezone for local times | `zulip_bot.services.user_timezone` |
+| Timezone for local times | The browser, via `prefs_form.js` and the posted `timezone` field |
 | The page itself | `console.views.prefs` + `templates/console/prefs.html` |
 
 `core.services.reviewer_prefs` reads `syncer.models.LabelDef` at module scope, matching the existing
-`core/admin.py` precedent. Timezone resolution lives in `zulip_bot` because its authoritative source
-is Zulip's own user record and `core/services` otherwise carries no app dependencies; console access
-stays independent of Zulip *reachability* — an unlinked or unreachable Zulip falls through to
-`core.User.timezone`, then the Django default.
+`core/admin.py` precedent. The page needs nothing from Zulip: times are shown and read in the
+browser's zone (see "Times are in the browser's zone" below).
 
 ### Invariants
 
@@ -99,10 +97,30 @@ Nothing about the page expires. The Django session bounds an editing window inst
 `SESSION_SAVE_EVERY_REQUEST = True` slides the two-week `SESSION_COOKIE_AGE` on activity so a long
 edit cannot POST into a lapsed cookie; the `beforeunload` dirty guard is the backstop.
 
+### Times are in the browser's zone
+
+- The server renders every time in its default zone (`TIME_ZONE`), and the form posts back, in a
+  hidden `timezone` field, the zone its times are in. `prefs_form.js` rewrites each `away_until` from
+  the stored instant in its `data-utc` into the browser's zone
+  (`Intl.DateTimeFormat().resolvedOptions().timeZone`), and that field with it. So the server reads a
+  naive `away_until` in exactly the zone it was shown in, whichever that was.
+- The "saved at" time is a `<time datetime>`, localized by `console/base.html` like every other
+  console timestamp. Zulip messages already show times in each reader's zone (`<time:…>` tags), so
+  no reviewer-facing surface picks a zone on the server.
+- A posted zone this server cannot load saves nothing, with a banner saying why, rather than reading
+  the times in some other zone.
+- Without JS the page stays in the server's zone and names it.
+- This replaced a server-side resolver, `zulip_bot.services.user_timezone` (Zulip's zone →
+  `core.User.timezone` → the default). It cost a Zulip API call per render and fell back to UTC for
+  anyone without a Zulip zone. It also rendered the page outside the zone it validated in, so times
+  showed in UTC under the reviewer's zone name, and every save moved `away_until` by that zone's UTC
+  offset (#219).
+
 ### Front-end assets
 
 `console/static/console/prefs_form.{css,js}` are console-owned. The JS is progressive enhancement
-only — an unsaved-changes guard and the per-row "clear away time" buttons — with **no countdown**,
+only — showing times in the browser's zone, an unsaved-changes guard and the per-row "clear away
+time" buttons — with **no countdown**,
 which is what let the expiry helpers stay behind in `zulip_bot/static/zulip_bot/expiry.js` for the
 close-pr / label-pr token pages that still need them. (Those pages import them as a *sibling* module,
 and a relative ES import must resolve both as a served path in the browser and an on-disk path under
@@ -120,8 +138,8 @@ and cover both apps' modules; CI runs them (`.github/workflows/ci.yml`).
   reviewers". Intended (invariant 2), but user-visible.
 - Editing scope widened from a token snapshot to all current rows, which is why ownership filtering
   must be applied on POST as well as GET.
-- `core.User.timezone` is still only ever set by hand in the Django admin, so in practice the
-  timezone comes from Zulip's user record per render, or the project default. See Follow-Ups.
+- `core.User.timezone` is read by nothing since the browser supplies the zone; it was only ever set
+  by hand in the Django admin. See Follow-Ups.
 
 ## Operational Notes
 
@@ -136,20 +154,20 @@ and cover both apps' modules; CI runs them (`.github/workflows/ci.yml`).
   `templates/zulip_bot/prefs_form.html`, `templates/zulip_bot/prefs_invalid.html`, and the token test
   module (its form-behavior coverage moved to `console/tests/test_prefs_form_fields.py`, its
   field-accounting guard to `core/tests/test_reviewer_preference_form.py`, and its timezone-chain
-  coverage to `zulip_bot/tests/test_user_timezone.py`).
+  coverage to `zulip_bot/tests/test_user_timezone.py`, since removed with that chain).
 - **Pre-flight for the retirement:** every `ReviewerPreference` row's user must have a usable
   `github_login` — under GitHub OAuth that, not `zulip_user_id`, is what would lock someone out.
   Registration always sets it (it goes through GitHub OAuth), as does `import_reviewer_topics`.
 - **Tests.** `console/tests/test_prefs.py` (auth, admission gate, no-row-creation, cross-reviewer POST,
-  timezone), `console/tests/test_prefs_form_fields.py` (fields, validation, label catalog),
-  `zulip_bot/tests/test_user_timezone.py`, `core/tests/test_reviewer_preference_form.py`, and the
-  vitest specs above.
+  posted-timezone handling), `console/tests/test_prefs_form_fields.py` (fields, validation, label
+  catalog), `core/tests/test_reviewer_preference_form.py`, and the vitest specs above (which cover
+  the browser's half of the timezone round trip).
 
 ## Follow-Ups
 
-- **Persist a browser-reported timezone.** `core.User.timezone` is admin-only today; storing
-  `Intl.DateTimeFormat().resolvedOptions().timeZone` on first visit would make the fallback real and
-  let us drop the per-render Zulip API round-trip.
+- **Drop `core.User.timezone`.** Nothing reads it now that the page uses the browser's zone (which
+  closed the earlier follow-up, "persist a browser-reported timezone"). Removing the column needs a
+  migration and a `scripts/backup_policy.py` check.
 - Optional confirmation DM after a successful preferences change (stage C3 of the original plan).
 - Structured logging for saves/rejections (stage C1) — never token or form contents.
 

qb_site/console/AGENTS.md

@@ -90,17 +90,24 @@
 - Ownership scoping lives in the shared builder: the queryset is narrowed to the supplied rows *and*
   their owner on GET and POST alike, so a posted `form-<n>-id` naming someone else's row fails
   validation. Never widen that queryset at a call site.
-- Timezone comes from `zulip_bot.services.user_timezone.resolve_user_timezone_name` (Zulip's zone →
-  `core.User.timezone` → Django default) and is what naive `away_until` input means. The page renders
-  no countdown: the session bounds it, and `SESSION_SAVE_EVERY_REQUEST` slides that window.
+- Times are in the browser's zone (design doc 022). The server renders them in its default zone and
+  the form posts back, in the hidden `timezone` field, the zone they are in; `prefs_form.js` rewrites
+  each `away_until` (from the stored instant in its `data-utc`) and that field into the browser's
+  zone. A POST's naive `away_until` is read in the posted zone, so it is always the zone it was shown
+  in. A posted zone the server cannot load (`_zone_or_none`) saves nothing rather than misreading the
+  times. Rendering stays inside `timezone.override`: the `date` filter and the widget convert at
+  render time. There is no Zulip lookup.
+- The page renders no countdown: the session bounds it, and `SESSION_SAVE_EVERY_REQUEST` slides that
+  window.
 - Styling/JS: `templates/console/prefs.html` extends `console/base.html` and uses its `extra_css` /
   `extra_js` blocks to load `console/prefs_form.css|js`, both console-owned. The JS is progressive
-  enhancement only (unsaved-changes guard, "clear away time" buttons, and the excluded-labels picker:
-  a filterable checkbox list over the label catalog the template emits with `json_script`, writing
-  one label per line into the hidden textarea that is still the submitted field; design doc 057)
-  with **no countdown** — the session bounds this page, not a link TTL — which is why the expiry
-  helpers stayed behind in `zulip_bot/static/zulip_bot/expiry.js` for the close-pr / label-pr token
-  pages. Its vitest spec lives with those, in `qb_site/zulip_bot/frontend/tests/`.
+  enhancement only (times in the browser's zone, unsaved-changes guard, "clear away time" buttons,
+  and the excluded-labels picker: a filterable checkbox list over the label catalog the template
+  emits with `json_script`, writing one label per line into the hidden textarea that is still the
+  submitted field; design doc 057) with **no countdown** — the session bounds this page, not a link
+  TTL — which is why the expiry helpers stayed behind in `zulip_bot/static/zulip_bot/expiry.js` for
+  the close-pr / label-pr token pages. Its vitest spec lives with those, in
+  `qb_site/zulip_bot/frontend/tests/`.
 
 ## Accept / decline / assign-anyway / unassign (the load-bearing handlers)
 - All `POST`. Accept and decline re-validate live state via the single
@@ -169,9 +176,8 @@
 - This app is step `[12/13]` of `scripts/repo_check_compose.sh`. It was missing from that script until
   the prefs work added it, so console tests had never run in CI — if you add an app, add its step.
 - View tests mock `console.views.GitHubOAuthClient`, `console.views.assign_reviewer_and_record`,
-  `console.views.GitHubAssignmentClient` (unassign), `console.views._enqueue_pr_sync`, and
-  `console.views.resolve_user_timezone_name` (prefs), and seed the session directly; no real GitHub
-  or Zulip calls. `tests/test_prefs.py` pins the prefs invariants (no row creation, admission gate at
-  sign-in *and* per view, cross-reviewer POST rejection, timezone interpretation) and
-  `tests/test_prefs_form_fields.py` covers fields, validation and the label catalog. Canonical full run
-  stays `bash scripts/repo_check_compose.sh`.
+  `console.views.GitHubAssignmentClient` (unassign) and `console.views._enqueue_pr_sync`, and seed
+  the session directly; no real GitHub or Zulip calls. `tests/test_prefs.py` pins the prefs invariants
+  (no row creation, admission gate at sign-in *and* per view, cross-reviewer POST rejection, reading
+  times in the posted zone) and `tests/test_prefs_form_fields.py` covers fields, validation and the
+  label catalog. Canonical full run stays `bash scripts/repo_check_compose.sh`.

qb_site/console/static/console/prefs_form.js

@@ -1,7 +1,7 @@
 // Reviewer preferences form behavior for /console/preferences/ (design doc 022).
 //
-// Progressive enhancement only: an unsaved-changes guard, the per-row "clear away time" buttons and
-// the excluded-labels picker. There is deliberately no countdown — the console session bounds this
+// Progressive enhancement only: showing times in the browser's zone, an unsaved-changes guard, the
+// per-row "clear away time" buttons and the excluded-labels picker. There is deliberately no countdown — the console session bounds this
 // page, not a link TTL — which is why the expiry helpers stayed behind in
 // `zulip_bot/static/zulip_bot/expiry.js` with the close-pr / label-pr token pages.
 
@@ -11,6 +11,64 @@ function serializeForm(form) {
 
 const fold = (name) => name.toLowerCase();
 
+// The browser's IANA zone, or "" when it will not say.
+function browserTimeZone() {
+  try {
+    return Intl.DateTimeFormat().resolvedOptions().timeZone || "";
+  } catch {
+    return "";
+  }
+}
+
+// `date` as a `datetime-local` value ("YYYY-MM-DDTHH:MM") on the wall clock of `timeZone`.
+export function toDatetimeLocalValue(date, timeZone) {
+  const format = new Intl.DateTimeFormat("en-US", {
+    timeZone,
+    year: "numeric",
+    month: "2-digit",
+    day: "2-digit",
+    hour: "2-digit",
+    minute: "2-digit",
+    hourCycle: "h23", // midnight is 00, never 24
+  });
+  const parts = {};
+  for (const { type, value } of format.formatToParts(date)) {
+    parts[type] = value;
+  }
+  return `${parts.year}-${parts.month}-${parts.day}T${parts.hour}:${parts.minute}`;
+}
+
+// The page's times are in the zone its `timezone` field names: the server's default, as rendered.
+// Rewrite each `away_until` from the stored instant in its `data-utc` into the browser's zone, and
+// post that zone back, so the server reads the times in the zone they were shown in (design doc
+// 022). A page already in the browser's zone (a re-render after a failed save) keeps what was typed.
+// All or nothing: if any time cannot be converted, the page stays in the server's zone, which still
+// agrees with every time on it.
+export function localizeTimes(root = document, browserZone = browserTimeZone()) {
+  const field = root.getElementById("prefs-timezone");
+  if (!(field instanceof HTMLInputElement) || !browserZone) {
+    return;
+  }
+  if (field.value !== browserZone) {
+    let rewrites;
+    try {
+      rewrites = [...root.querySelectorAll("input[data-utc]")].map((input) => [
+        input,
+        toDatetimeLocalValue(new Date(input.dataset.utc), browserZone),
+      ]);
+    } catch {
+      return;
+    }
+    for (const [input, value] of rewrites) {
+      input.value = value;
+    }
+    field.value = browserZone;
+  }
+  for (const name of root.querySelectorAll(".js-timezone-name")) {
+    name.textContent = browserZone;
+  }
+}
+
 // Excluded-labels picker (design doc 057): a filter box over a short scrolling list of checkboxes,
 // one per repository label (the JSON block the template renders), plus a removable chip per chosen
 // label. Real checkboxes rather than a pop-up suggestion list: the browsers' own <datalist> shows
@@ -215,7 +273,7 @@ export function mountLabelPicker(container, root = document) {
   };
 }
 
-export function mountPrefsForm(root = document) {
+export function mountPrefsForm(root = document, browserZone = browserTimeZone()) {
   const container = root.getElementById("prefs-root");
   if (!container) {
     return () => undefined;
@@ -226,7 +284,9 @@ export function mountPrefsForm(root = document) {
     return () => undefined;
   }
 
-  // Before the snapshot below. Mounting never rewrites the textarea, so the page starts clean.
+  // Both before the snapshot below, so neither counts as an unsaved change. Mounting never rewrites
+  // the textarea, so the page starts clean.
+  localizeTimes(root, browserZone);
   const unmountPickers = [...root.querySelectorAll(".js-label-picker")].map((picker) => mountLabelPicker(picker, root));
 
   let dirty = false;