diff --git a/mvp/blocker-phases/04-payment-timezone.md b/mvp/blocker-phases/04-payment-timezone.md index 0ef8694..e43acf7 100644 --- a/mvp/blocker-phases/04-payment-timezone.md +++ b/mvp/blocker-phases/04-payment-timezone.md @@ -30,3 +30,18 @@ the in-memory `Kind` tag changes on read. anywhere else a server-frozen instant feeds a client countdown (e.g. `dispute_window_ends_at`, BNPL `settled_at`) — worth a grep for other `DateTime` (not `DateTimeOffset`) properties before considering this class of bug fully closed, not just the one already reported. + +**Follow-up from that grep (done, not yet fixed).** Swept every other `DateTime`/`DateTime?` entity property +(`BaseEntity.CreatedAt`/`ModifiedAt` already correctly use `DateTimeOffset` and are excluded). Most are +internal/audit-only (payout batches, webhook events, ASP.NET Identity tables) and never reach a client. Four +are real, lower-severity instances of the same bug — never wired to a live countdown, so the failure mode is +"can show the wrong day near a Tehran midnight boundary," not "actively expires while showing time left": +- `Booking.DisputeWindowEndsAt` (`Entities/Booking/Booking.cs:95`) — rendered via `formatShamsiDate` in + `BookingDetailView.tsx:444`. +- `Booking.ConfirmedAt` / `CancelledAt` / `CompletedAt` (`Entities/Booking/Booking.cs:73,74,91`) — booking + timeline timestamps shown to the client. + +All four are configured in `BookingConfig.cs` (no `.HasConversion(...)` today) and would take the exact same +fix as this phase: `.HasConversion(new UtcDateTimeConverter())`. Deliberately left unfixed here — scoped out +of this phase on request, tracked so it isn't dropped. Pick up as a follow-up phase (or fold into +[13-booking-lifecycle.md](13-booking-lifecycle.md), which already touches `Booking` timezone handling). diff --git a/mvp/fix-plan.md b/mvp/fix-plan.md index 03e13c9..025a579 100644 --- a/mvp/fix-plan.md +++ b/mvp/fix-plan.md @@ -19,7 +19,7 @@ whatever order you prefer. | 01 | [admin-rbac](blocker-phases/01-admin-rbac.md) | Admin can't do anything (the #1 leverage item) | — | ✅ Done | | 02 | [reviews-moderation](blocker-phases/02-reviews-moderation.md) | Reviews can never go live | 01 | ✅ Done (no separate fix needed — unblocked by 01) | | 03 | [address-edit-bug](blocker-phases/03-address-edit-bug.md) | Editing an address wipes recipient name/phone | — | ✅ Done | -| 04 | [payment-timezone](blocker-phases/04-payment-timezone.md) | The 30-minute payment countdown can lie | — | — | +| 04 | [payment-timezone](blocker-phases/04-payment-timezone.md) | The 30-minute payment countdown can lie | — | ✅ Done (follow-up filed below) | | 05 | [bnpl-setup](blocker-phases/05-bnpl-setup.md) | Installments (BNPL) don't work at all | — | — | | 06 | [catalog-admin-page](blocker-phases/06-catalog-admin-page.md) | No admin page for service categories/pricing | — | — | | 07 | [card-payment-redirect](blocker-phases/07-card-payment-redirect.md) | Card payment can never complete | — | — | @@ -32,6 +32,26 @@ whatever order you prefer. | 14 | [partner-center](blocker-phases/14-partner-center.md) | Partner/business-center accounts are fake | benefits from 01 | — | | 15 | [debug-mode-production](blocker-phases/15-debug-mode-production.md) | Turn off dev mode on the live site (§B.2) | do last, deliberately | — | +## Follow-ups filed (not yet phases of their own) + +- **Same timezone bug as 04, lower severity, not fixed.** Phase 04 fixed `BookingRequest.PaymentDeadlineAt`/ + `NurseResponseDeadlineAt` — a `DateTime` (not `DateTimeOffset`) read back from SQL Server's `datetime2` + loses its `Kind` tag (comes back `Unspecified`), so JSON serialization drops the trailing `Z` and a client + `Date.parse()` misreads it as local time. A grep for every other bare `DateTime`/`DateTime?` entity property + found four more real instances, all on `Booking` (`server/src/Core/Baya.Domain/Entities/Booking/Booking.cs`): + `DisputeWindowEndsAt` (:95, rendered via `formatShamsiDate` in + `client/src/components/booking/BookingDetailView/BookingDetailView.tsx:444`), and `ConfirmedAt`/ + `CancelledAt`/`CompletedAt` (:73,74,91), booking timeline timestamps shown to the client. Lower severity than + 04 — nothing here drives a live countdown, so the failure mode is "can show the wrong calendar day near a + Tehran (UTC+3:30) midnight boundary," not "actively expires while still showing time left." (Everything + else with a bare `DateTime` — payout batches, webhook events, ASP.NET Identity tables — is internal/audit-only + and never reaches a client, so it's excluded.) Fix is the same pattern phase 04 used: apply the existing + `UtcDateTimeConverter` (`server/src/Infrastructure/Baya.Infrastructure.Persistence/ValueConversion/ + UtcDateTimeConverter.cs`) to these four properties in `BookingConfig.cs`. No migration needed. Deliberately + left undone — pick up as its own small phase, or fold into whatever eventually addresses 13b's timezone + decision (booking-lifecycle's "today's visits" fix), since both are the same missing + `Asia/Tehran`/UTC-boundary discipline. + ## Suggested order 1. **01 (admin RBAC)** — unlocks 02, and makes 09/10/11/14 independently testable via the seeded diff --git a/server/src/Infrastructure/Baya.Infrastructure.Persistence/Configuration/BookingConfig/BookingRequestConfig.cs b/server/src/Infrastructure/Baya.Infrastructure.Persistence/Configuration/BookingConfig/BookingRequestConfig.cs index b94296e..0d6b5b3 100644 --- a/server/src/Infrastructure/Baya.Infrastructure.Persistence/Configuration/BookingConfig/BookingRequestConfig.cs +++ b/server/src/Infrastructure/Baya.Infrastructure.Persistence/Configuration/BookingConfig/BookingRequestConfig.cs @@ -1,4 +1,5 @@ using Baya.Domain.Entities.Booking; +using Baya.Infrastructure.Persistence.ValueConversion; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Metadata.Builders; @@ -16,7 +17,11 @@ internal sealed class BookingRequestConfig : IEntityTypeConfiguration r.CustomerNotes).HasMaxLength(1000); builder.Property(r => r.Status).HasMaxLength(50).IsRequired(); builder.Property(r => r.NurseRejectionReason).HasMaxLength(500); - builder.Property(r => r.NurseResponseDeadlineAt).IsRequired(); + + // datetime2 loses Kind on read (comes back Unspecified); re-tag as Utc so JSON serialization keeps + // the trailing Z and clients don't misparse the deadline as local time (mvp/blocker-phases/04). + builder.Property(r => r.NurseResponseDeadlineAt).HasConversion(new UtcDateTimeConverter()).IsRequired(); + builder.Property(r => r.PaymentDeadlineAt).HasConversion(new UtcDateTimeConverter()); // Inbox lists read on (party, status), actionable-first; the two deadline indexes let the expiry // sweep select stale rows through a covering index instead of scanning the table. diff --git a/server/src/Infrastructure/Baya.Infrastructure.Persistence/ValueConversion/UtcDateTimeConverter.cs b/server/src/Infrastructure/Baya.Infrastructure.Persistence/ValueConversion/UtcDateTimeConverter.cs new file mode 100644 index 0000000..0c1d655 --- /dev/null +++ b/server/src/Infrastructure/Baya.Infrastructure.Persistence/ValueConversion/UtcDateTimeConverter.cs @@ -0,0 +1,14 @@ +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; + +namespace Baya.Infrastructure.Persistence.ValueConversion; + +/// +/// SQL Server's datetime2 carries no timezone, so EF's provider returns +/// on read even though the value was always written as UTC — leaving it that way makes System.Text.Json omit the +/// trailing Z, and clients then misparse the instant as local time. Re-tags the as +/// on read; the value itself is never altered. +/// +internal sealed class UtcDateTimeConverter() + : ValueConverter( + v => v, + v => DateTime.SpecifyKind(v, DateTimeKind.Utc));