Add an optional start time to calendar entries - #268
Open
MeisterAdebar wants to merge 2 commits into
Open
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.