fix(reconcile): send ReconciledAt as a UTC instant, not naked digits by renemadsen · Pull Request #1719 · microting/eform-angular-timeplanning-plugin

renemadsen · 2026-09-17T14:56:21Z

What

The "Afstemt <dato> kl. HH:mm" provenance stamp on a reconciled day read 1-2 hours early for Danish users, year-round — and it jumped: correct at the moment of reconciling, two hours earlier once the grid reloaded.

Why

SetReconciledAsync writes DateTime.Now
  -> on the shipped container (no TZ; aspnet base image defaults to UTC) that IS a UTC instant
  -> column is datetime(6): carries NO timezone
  -> EF materialises it as DateTimeKind.Unspecified
  -> Newtonsoft (default RoundtripKind) emits a suffix only for Kind Utc ("Z") or Local ("+hh:mm")
  -> the wire carries a NAKED "2026-09-17T12:30:00"
  -> new Date(naked) parses as LOCAL in JS
  -> the browser relabels the server's UTC digits as Copenhagen wall clock

The jump had its own cause: the dialog's optimistic stand-in wrote the browser's wall clock, equally naked — right on arrival, an offset away once the stored value replaced it.

The fix

The decision (the user's) is that the column holds UTC, the wire tags it, and each viewer's browser renders it in their own local time. Three parts, so the UTC claim is true by construction rather than by an unset environment variable:

	Change
`TimePlanningPlanningService.SetReconciledAsync`	`DateTime.Now` → `DateTime.UtcNow`, so the column genuinely holds UTC even if ops ever sets `TZ` on the container. Tagging the wire asserts this; `UtcNow` removes the dependency.
`PlanRegistrationHelper` (read projection)	`DateTime.SpecifyKind(…, DateTimeKind.Utc)`, so the JSON carries the trailing `Z`. Null-safe via pattern match — a null stays null.
`workday-entity-dialog.component.ts`	optimistic stand-in is now `toISOString()` — the same shape as the value that replaces it, so neither is relabelled by the viewer's offset and the stamp stops jumping.

Only one projection exists. PlanRegistrationHelper is the sole place ReconciledAt reaches a response model; the mobile gRPC surface does not carry the field at all.

No backfill needed. The container was already UTC, so rows written before this change hold UTC digits too and the new Z is retroactively true for them.

Also corrects the comments that encoded the old, false premise (day-lock.util.ts, its spec, design doc §6.4), including a warning against the plausible-looking "fix" of naming a zone: Angular resolves the DatePipe timezone argument through Date.parse('Jan 01, 1970 00:00:00 ' + tz), which is NaN for an IANA name like 'Europe/Copenhagen' and then silently falls back to the browser's own offset.

Separately, DayLockHelper's pre-existing direction comment was wrong in a way this branch had already copied into the spec twice: at a positive offset UtcNow.Date lags the local date in the first hours after local midnight (at local 00:30 on the 16th at +02:00 it is still the 15th in UTC), not late in the local day, where the two dates agree and the clock makes no difference. Fixed at the origin and in both restatements.

Scope

ReconciledAt only — chosen by the user. The sibling stamps with related problems are recorded as follow-ups below, not touched here.

Testing — what is and is not actually enforced

The one CI-enforced guard is the C# assertion that the projected model's ReconciledAt comes back with Kind == Utc (plus: an unreconciled day stays null). It fails on every host, CI included, if the SpecifyKind is reverted, because EF materialises the offsetless column as Unspecified everywhere. It lives in the existing ReconcileServiceTests fixture, which is already in the shard filters of both workflow files.
The companion freshness assertion (Within(5 minutes) of UtcNow) catches a regression to DateTime.Now, but only on a host at a non-zero offset. It is inert on CI and anywhere at UTC+0.
The frontend specs were reviewed by reading only — never compiled, never run. This repo carries only eform-client/src/app/…: no package.json, no node_modules, no jest config. And angular-unit-test is continue-on-error: true, so it cannot fail this PR regardless of outcome. The frontend assertions were made zone-independent (shape regexes rather than hard-coded digits) precisely because CI runs UTC, where every clock in this chain coincides and a UTC-only test proves almost nothing.
dotnet build TimePlanning.Pn.sln: 0 errors.
Not verified in a browser. Worth a look at the stamp before and after a grid reload.

Recorded follow-ups (not in this PR)

CompareDateTimeField (TimePlanningPlanningService.cs:2134) renders ReconciledAt as naked digits in the admin version-history diff — a string, shared verbatim across ~30 DateTime fields, so no Kind is involved and tagging this one would split it from every sibling.
formatStamp passes 'UTC' for the shift stamps, pinning every viewer to the server's clock.
The absence/handover requestedAtUtc / respondedAtUtc / decidedAtUtc stamps.
Two hand-rolled "matches Newtonsoft.Json's serialization" date formatters: TimePlanningAbsenceRequestGrpcService.cs:214, TimePlanningContentHandoverGrpcService.cs:272.

🤖 Generated with Claude Code

The "Afstemt <dato> kl. HH:mm" stamp read 1-2 hours early for Danish users, year-round, and it also JUMPED: right at the moment of reconciling it showed the correct time, and after the grid reloaded it moved two hours earlier. The chain: SetReconciledAsync wrote DateTime.Now, which on the shipped container (no TZ, aspnet base image defaults to UTC) is in fact a UTC instant. The column is datetime(6), which carries no offset, so EF materialises it as DateTimeKind.Unspecified, and Newtonsoft with the default RoundtripKind emits a suffix only for Kind Utc or Local. The wire therefore carried "2026-09-17T12:30:00" with nothing after it, and new Date(naked) parses as LOCAL, so the browser relabelled the server's UTC digits as Copenhagen wall clock. The jump came from the dialog's optimistic stand-in, which wrote the BROWSER's wall clock, equally naked: right on arrival, an offset away once the stored value replaced it. The decision (the user's) is that the column holds UTC, the wire tags it, and each viewer's browser renders it in their own local time. Three parts, so the UTC claim is true by construction rather than by an unset environment variable: - SetReconciledAsync writes DateTime.UtcNow, so the column genuinely holds UTC even if ops ever sets TZ on the container. - The read projection in PlanRegistrationHelper specifies DateTimeKind.Utc, so the JSON carries the trailing Z. Null stays null. This is the only projection of ReconciledAt into a response model; the mobile gRPC surface does not carry the field. - The optimistic stand-in uses toISOString(), the same shape as the value that replaces it, so neither is relabelled by the viewer's offset and the stamp no longer jumps. Scope is ReconciledAt only. The sibling stamps with related problems (formatStamp's 'UTC' on the shift stamps, the absence/handover *Utc stamps, CompareDateTimeField in the version-history diff) are recorded as follow-ups, not touched here. Also corrects the comments that encoded the old, false premise, in day-lock.util.ts, its spec, and the design doc. DayLockHelper's pre-existing direction comment was wrong in a way this PR had already copied into the spec twice: at a positive offset UtcNow.Date lags the local date in the first hours AFTER local midnight (at local 00:30 on the 16th at +02:00 it is still the 15th in UTC), not late in the local day, where the two dates agree and the clock makes no difference. Fixed at the origin and in both restatements. Existing rows need no backfill: the container was already UTC, so rows written before this change hold UTC digits too and the new Z is retroactively true for them. Guard: ReconcileServiceTests asserts the projected model's ReconciledAt comes back with Kind == Utc and that an unreconciled day stays null. The Kind half fails on every host including CI if the SpecifyKind is reverted, since EF materialises the offsetless column as Unspecified everywhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

🟡 Changes recommended

The newly added backend test uses multiple DateTime.UtcNow.Date reads that can become inconsistent across a UTC midnight boundary and intermittently fail.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR fixes incorrect and “jumping” display of the reconciled-day provenance stamp by ensuring ReconciledAt is treated as a UTC instant end-to-end (write as UTC, project as UTC-kind so JSON emits Z, and keep the client’s optimistic stand-in in the same ISO-UTC shape).

Changes:

Backend: write ReconciledAt using DateTime.UtcNow and project it with DateTimeKind.Utc so JSON roundtrips with a trailing Z.
Frontend: use toISOString() for the optimistic in-dialog stand-in and update unit specs to assert shape rather than zone-dependent digits.
Docs/comments/tests: clarify the timezone rationale (including UtcNow.Date lock behavior) and add a guard test for ReconciledAt.Kind == Utc.

File summaries

File	Description
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs	Writes `ReconciledAt` as `UtcNow` to make the DB value UTC by construction.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/PlanRegistrationHelper.cs	Re-tags `ReconciledAt` as `DateTimeKind.Utc` on projection so JSON includes `Z`.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Infrastructure/Helpers/DayLockHelper.cs	Corrects/clarifies the `UtcNow.Date` vs local-date boundary explanation.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/ReconcileServiceTests.cs	Adds a test asserting projected `ReconciledAt` is UTC-kind and near `UtcNow`.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/workday-entity/workday-entity-dialog.component.ts	Uses `toISOString()` for the optimistic reconcile timestamp to avoid offset relabeling/jumps.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/time-planning-actions/workday-entity/workday-entity-dialog.component.spec.ts	Updates dialog unit tests to accept localized output and assert ISO-UTC stamp shape.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/day-lock.util.ts	Updates documentation to reflect UTC-instant contract and timezone-arg behavior.
eform-client/src/app/plugins/modules/time-planning-pn/components/plannings/day-lock.util.spec.ts	Updates util unit tests to use `Z`-tagged stamps and zone-independent assertions.
docs/superpowers/specs/2026-09-12-reconciled-day-lock-design.md	Updates design spec to reflect UTC instant handling and corrected midnight-boundary reasoning.

Review details

Files reviewed: 9/9 changed files
Comments generated: 1
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+        // UtcNow.Date, not Now.Date: this whole chain is meant to run on one
+        // clock, and SeedReconciledBoundaryAsync has to satisfy CanReconcile,
+        // which compares against UtcNow.Date.
+        var open = await SeedPlain(916, DateTime.UtcNow.Date.AddDays(-1));
+        var boundary = await SeedReconciledBoundaryAsync(916, DateTime.UtcNow.Date.AddDays(-4));
+        var window = new TimePlanningPlanningRequestModel
+        {
+            DateFrom = DateTime.UtcNow.Date.AddDays(-4),
+            DateTo = DateTime.UtcNow.Date
+        };


Copilot AI lite review requested due to automatic review settings September 17, 2026 14:56

Copilot started reviewing on behalf of renemadsen September 17, 2026 14:58 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

renemadsen merged commit d079ba5 into stable Sep 17, 2026
41 checks passed

Navigation Menu

fix(reconcile): send ReconciledAt as a UTC instant, not naked digits - #1719

fix(reconcile): send ReconciledAt as a UTC instant, not naked digits#1719
renemadsen merged 1 commit into
stablefrom
fix/reconciled-at-utc-on-the-wire

renemadsen commented Sep 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

renemadsen commented Sep 17, 2026

What

Why

The fix

Scope

Testing — what is and is not actually enforced

Recorded follow-ups (not in this PR)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants