Add an optional start time to calendar entries - #268
Conversation
Entries were always all-day: the date column held a bare date and the ICS import zeroed the time off DTSTART. Appointments that happen at a set hour had nowhere to put it. Adds a nullable time column rather than widening date to DATETIME. Every consumer keys days by Y-m-d - sync reconciliation, the reservation table decorations, the reminder query - and each would otherwise have to strip a time component it never asked for. Null means all-day, which is what every existing entry already says. The ICS import now keeps the time where the feed states one, recognising all-day events by DTSTART's bare-date form. On a multi-day event only the starting day carries it; the days it runs through are whole days. Times are tracked per date so two VEVENTs merging under one UID keep their own, and reconciliation compares them by wall clock so a re-sync does not report every timed entry as updated. Manual entries created over a range follow the same rule. Shown after the title in the day popover and the reminder list, and sorted after the all-day entries within a day.
The hint started with "End date"/"Enddatum", repeating the label right above it and pushing the part that actually matters - that the field may be left alone - to the middle of the sentence. The neighbouring time field already reads "Optional - ...", so both optional fields now announce themselves the same way.
|
Moin, habs mir jetzt angesehen.
LG 😊 |
Addresses three points from review.
Times were stored as whatever wall clock the feed happened to carry. A
Google export publishes UTC instants, so a 13:00 appointment was filed
and shown as 11:00. The ICS parser now keeps each property's parameters
alongside its value and resolves DTSTART/DTEND properly: a trailing Z as
UTC, a TZID as the zone it names, a bare value as local time. Conversion
happens before the dates are derived, because the zone decides the
calendar day as much as the clock - 22:30Z belongs to the next day here.
Multi-day events lost their closing day. DTEND is exclusive only for
all-day events per RFC 5545; when it carries a time it is the moment the
event stops, so its day is still covered - unless it stops exactly at
midnight. "Aug 14 13:00 - Aug 16 14:00" now yields all three days.
Entries gain an optional end time. On a range the first day carries the
start and the last one the end, with the days between running all day,
which keeps one row per day while letting the closing day say when the
event stops instead of showing nothing. Both the manual form and the ICS
sync make that same split.
The zone comes from a new appTimezone parameter rather than php.ini's
date.timezone, which differs between SAPIs - the Apache config sets it
from ${TZ} while the CLI stays on UTC, so the same feed stored different
times depending on whether the web form or the calendars:sync cron ran
it. The lastSyncedAt and confirmedAt stamps went through the same split
and are now taken in that zone too.
The zone was read from an appTimezone parameter fed by ${TZ}. That works
only when the deployment sets it, and it splits into two half-configured
states: TZ in Symfony's .env reaches the parameter but never php.ini,
which is parsed long before .env is loaded.
It now lives on the AppSettings singleton, where web requests and the
cron commands read the same row and cannot disagree by construction.
Configuring it needs no redeploy, and a fresh install gets Europe/Berlin
without anyone setting anything. A container test confirmed the previous
approach did hold up under Docker - crond does pass TZ to its jobs, and
conf.ini covers both SAPIs - but only while TZ is set, which nothing
enforces.
The migration defaults existing rows to Europe/Berlin rather than reading
php.ini: that value differs per SAPI, so there is no single existing one
to carry over. Deployments on another zone set it once in the settings UI.
CalendarEntrySyncService resolves the zone once per import, before any
entity changes are queued - getSettings() creates and flushes the row on
first access, which must not land mid-import.
The three help texts all opened with "Optional -", so the eye had nothing to tell them apart, and the end time's text explained a multi-day rule while the edit modal it appears in has no end date field at all - that only exists when creating an entry. Each text now says one thing, and nothing claims a behaviour the surrounding form cannot reach. The fields shared one row of equal columns, which left the date and time inputs narrower than their own content while the calendar and title had width to spare. Creating an entry has six of them, too many for a modal-lg without squeezing, so it now puts what the entry is on the first row and when it happens on the second; editing has neither the calendar picker nor the end date and still fits one row. Splitting the two also lets the creating form order its dates as they read - date, time, until, end time - instead of placing the end time before the field naming the day it belongs to.
Both date inputs render as native date pickers, so the browser shows its
own tt.mm.jjjj placeholder and drops the one the form sets. With no
labels either, nothing on screen said which of the two dates was the end
one - the help text described what the field does ("Mehrtägig") without
naming what it is. It now leads with "Bis", and fits on one line while
doing so.
|
Alle drei Punkte Punkte angegangen, bitte kritisch prüfen.
Dazu kommt ein zweiter Branch, da steckt eine Entscheidung drin, die über den Kalender hinausgeht. 1. ZeitzoneDer Parser hat die Property-Parameter bisher komplett verworfen ( Kurios daran: die Der Parser behält die Parameter jetzt unter einem eigenen Schlüssel und beherrscht alle drei RFC-5545-Formen: Wichtig war, die Umrechnung vor der Datumsermittlung zu machen, nicht erst bei der Anzeige: die Zone entscheidet auch über den Kalendertag. 2. Verlorener letzter TagDer Code hat Bei mir wurde aus Ausnahme ist ein Ende glatt um Mitternacht, das weiter zum Vortag zählt. Sonst bekäme ein Termin von 18:00 bis 00:00 fälschlich noch einen Eintrag am Folgetag. 3. EndzeitpunktIch habe deinen zweiten Vorschlag genommen, nicht den ersten — das Enddatum zu sperren hätte Punkt 2 für Termine aus der Quelle ja nicht gelöst.
Angezeigt wird Dein Beispiel habe ich einmal komplett durch den reparierten Import geschickt. 12.08. 18:00 bis 14.08. 19:00 ergibt jetzt:
Beide Fehler auf einmal erledigt: die 18:00 stehen richtig da, und der 14. ist wieder dabei — mit der Endzeit statt einer leeren Zeile. Die Zeitzone kommt nicht mehr aus der php.iniDas ist der Teil, den ich dir erklären möchte, bevor du den Code liest. Beim Nachstellen bin ich darauf gestoßen, dass Eine Umgebungsvariable als Grundlage fand ich deshalb zu wackelig. Sie muss im Deployment gesetzt sein, und es gibt eine Falle: Die Zone liegt jetzt als Feld auf Das passt auch zu der Richtung, die in Nebenbei habe ich das Docker-Setup gegengeprüft, weil ich es nicht raten wollte. Dort war der SAPI-Bruch nie das Problem: Zweiter Branch:
|
2 participants
Calendar entries can now carry an optional start time. Left empty they stay
all-day entries, which is what every entry was before and still is by default.
Storage
The time lives in its own nullable
TIMEcolumn rather than wideningdateto
DATETIME. Everything downstream keys days byY-m-d- syncreconciliation, the reservation table decorations, the reminder query - and
each of them would otherwise have to strip a time component it never asked
for.
nullalready says "all day", and that is exactly what existing rowssay without touching them.
Migration
Version20260729140000adds the column,down()drops it again.Calendar sync
CalendarEntrySyncServicereads the start time fromDTSTART:DTSTART;VALUE=DATE:20260801).The parser drops the parameters, so the plain
Ymdshape of the value iswhat identifies them - the same signal
resolveDates()already relies on.runs through are whole days by definition. Tracked per date rather than per
event, so two VEVENTs merging under one UID keep their own.
Times are compared by wall clock, not by object identity - otherwise every
re-sync would report every timed entry as "updated".
Known limitation: time zones
The wall-clock digits are taken as they stand. A feed sending
DTSTART;TZID=Europe/Berlin:20260801T140000loses the zone during parsing butkeeps the 14:00 it meant, which is the time that should be displayed. An
explicit UTC value (trailing
Z) is read as UTC, and the application runs inUTC, so that is what gets shown.
For a feed in a different zone than the one the instance runs in, the
displayed time will therefore be off. Carrying the zone through would mean
teaching the ICS parser to keep
DTSTARTparameters, which is a larger changethan this one - worth doing if it turns out to matter in practice, but the
common case here is a feed from the same zone.
Display and ordering
The popover and the reminder list show the time next to the title when there
is one. Within a calendar and day, all-day entries sort first (null time),
followed by the timed ones in clock order.
In the form the time input sits with the start date, not with the date range,
mirroring that only the first day of a range carries it.
Multi-day entries
The unmapped
dateTofield creates one entry per day of the range instead ofteaching the entity and every consumer about ranges - the same approach the
sync already takes for multi-day ICS events.
Wording of the existing end-date hint
The end-date field predates this branch, but its hint led with
"End date"/"Enddatum", repeating the label right above it and pushing the part
that matters - that the field may be left alone - into the middle of the
sentence. It now leads with "Optional", matching the time field this branch
adds:
Tests
Six functional tests cover the sync path: a timed event stores its time, an
all-day event stores none, a multi-day event puts the time on the first day
only, re-syncing reports unchanged, a changed time updates the entry, and an
event losing its time becomes all-day again.