From 4129d8cce8662dd85c9ed371819141b7e1f2e8ce Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Mon, 31 Aug 2026 19:11:25 +0200 Subject: [PATCH] fix(ics): keep a task's span, drop after-start timed alarms (#225) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DUE was read only when a VTODO had no DTSTART, and never as the end. A task carrying both imported as a zero-length event instead of the span between them. It is now resolved after the property sweep, so a DUE ahead of the DTSTART isn't mistaken for the start either. A timed VALARM firing after the start is a follow-up alarm and models no lead time — clamping it to zero invented a reminder at the start that the file never asked for. Dropped again, as the insert path did before. The clamp stays for all-day, where it's how the Fossify family writes "on the day at 09:00". --- .../calendula/domain/ics/IcsParser.kt | 21 +++++-- .../domain/ics/IcsFossifyImportTest.kt | 57 +++++++++++++++++++ docs/ARCHITECTURE.md | 19 +++++-- 3 files changed, 88 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsParser.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsParser.kt index 4310f54..c49c5ff 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsParser.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsParser.kt @@ -53,7 +53,10 @@ data class ParsedIcsEvent( * [reminderMinutes] mapped onto Calendula's model: a lead time in minutes before * the event, never negative. * - * For a timed event the raw lead time already is that. All-day reminders are + * For a timed event the raw lead time already is that, and a trigger that fires + * *after* the start models no lead time at all — it is dropped rather than + * clamped, which would invent a reminder at the start the file never asked for + * (a legal `TRIGGER:PT30M` follow-up alarm is the case). All-day reminders are * whole days before the event, fired at the user's configured time of day (see * `AllDayReminderEncoding`), so a raw offset counts the whole days its trigger * lands *earlier than* the event's UTC midnight — i.e. it rounds **up**. @@ -67,11 +70,11 @@ data class ParsedIcsEvent( * ceils to zero days before. */ fun ParsedIcsEvent.semanticReminderMinutes(): List = reminderMinutes - .map { raw -> + .mapNotNull { raw -> if (isAllDay) { ceil(raw.toDouble() / MINUTES_PER_DAY).toInt().coerceAtLeast(0) * MINUTES_PER_DAY } else { - raw.coerceAtLeast(0) + raw.takeIf { it >= 0 } } } .distinct() @@ -179,6 +182,7 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault var summary = "" var dtStart: IcsDateTime? = null var dtEnd: IcsDateTime? = null + var due: IcsDateTime? = null var duration: String? = null var rrule: String? = null var location: String? = null @@ -207,8 +211,7 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault "UID" -> uid = line.value.trim().ifEmpty { null } "SUMMARY" -> summary = unescapeText(line.value) "DTSTART" -> dtStart = parseIcsDateTime(line, warnings) - // A VTODO has no DTSTART of its own in most producers' output. - "DUE" -> if (isTask && dtStart == null) dtStart = parseIcsDateTime(line, warnings) + "DUE" -> if (isTask) due = parseIcsDateTime(line, warnings) "DTEND" -> dtEnd = parseIcsDateTime(line, warnings) "DURATION" -> duration = line.value.trim() "RRULE" -> rrule = line.value.trim().ifEmpty { null } @@ -244,6 +247,14 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault warnings.add(IcsParseWarning.ModifiedOccurrenceSkipped) return null } + // Most producers give a VTODO only a DUE, which is then the moment the + // task sits at. One that carries a DTSTART too means the span between + // them, so DUE stands in for the end it has no DTEND for. Resolved after + // the sweep rather than inside it: property order isn't guaranteed, and + // a DUE ahead of the DTSTART would otherwise be taken for the start. + if (isTask && due != null) { + if (dtStart == null) dtStart = due else if (dtEnd == null) dtEnd = due + } val start = dtStart ?: run { warnings.add(IcsParseWarning.EventWithoutStartSkipped) return null diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsFossifyImportTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsFossifyImportTest.kt index 070f25e..f7bc50b 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsFossifyImportTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsFossifyImportTest.kt @@ -3,6 +3,7 @@ package de.jeanlucmakiola.calendula.domain.ics import com.google.common.truth.Truth.assertThat import kotlinx.datetime.TimeZone import org.junit.jupiter.api.Test +import kotlin.time.Instant /** * Reading the Simple Calendar / Fossify export dialect (Codeberg #225). @@ -176,6 +177,41 @@ class IcsFossifyImportTest { assertThat(result.events.single().summary).isEqualTo("File taxes") } + @Test + fun `a task with both DTSTART and DUE spans between them`() { + val result = parser.parse( + fossify( + "BEGIN:VTODO", + "SUMMARY:Write report", + "DTSTART:20260901T090000Z", + "DUE:20260901T170000Z", + "END:VTODO", + ), + ) + + val task = result.events.single() + assertThat(task.start).isEqualTo(Instant.parse("2026-09-01T09:00:00Z")) + assertThat(task.end).isEqualTo(Instant.parse("2026-09-01T17:00:00Z")) + } + + @Test + fun `a DUE ahead of the DTSTART is still the end`() { + // Property order isn't guaranteed; DUE must not be taken for the start. + val result = parser.parse( + fossify( + "BEGIN:VTODO", + "SUMMARY:Write report", + "DUE:20260901T170000Z", + "DTSTART:20260901T090000Z", + "END:VTODO", + ), + ) + + val task = result.events.single() + assertThat(task.start).isEqualTo(Instant.parse("2026-09-01T09:00:00Z")) + assertThat(task.end).isEqualTo(Instant.parse("2026-09-01T17:00:00Z")) + } + @Test fun `the per-event colour wins over the calendar colour`() { val result = parser.parse( @@ -388,6 +424,27 @@ class IcsFossifyImportTest { assertThat(result.events.single().semanticReminderMinutes()).containsExactly(10) } + @Test + fun `a timed trigger that fires after the start is dropped, not clamped`() { + // A legal RFC 5545 follow-up alarm. Modelling it as a lead time of zero + // would invent a notification at the start the file never asked for. + val result = parser.parse( + fossify( + "BEGIN:VEVENT", + "SUMMARY:Standup", + "DTSTART:20260819T080000Z", + "DTEND:20260819T081500Z", + "BEGIN:VALARM", + "ACTION:DISPLAY", + "TRIGGER:PT30M", + "END:VALARM", + "END:VEVENT", + ), + ) + + assertThat(result.events.single().semanticReminderMinutes()).isEmpty() + } + @Test fun `an all-day series' EXDATE day codes carry over`() { val result = parser.parse( diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 4c86337..780f683 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -395,10 +395,11 @@ property is a tag list and must not outrank something that names the calendar outright — bare `EXDATE` day codes on a timed series are resolved against the series' own time of day (as is a floating `EXDATE` DATE-TIME — RFC 5545 ties it to `DTSTART`'s zone, not the device's) while an all-day `EXDATE` keeps the day it -spells out rather than the day its resolved instant lands on in UTC, and a -`VALARM` trigger pointing *after* the start — which is how that family encodes -"on the day at 09:00" — is read as zero days before rather than clamped to a lead -time of zero. +spells out rather than the day its resolved instant lands on in UTC, and an +all-day `VALARM` trigger pointing *after* the start — which is how that family +encodes "on the day at 09:00" — is read as zero days before. On a *timed* event +the same shape means a follow-up alarm and is dropped: clamping it to zero would +invent a reminder at the start that the file never asked for. All-day reminder offsets are **whole days, rounded up** in both directions. A file's raw offset is `days × 1440 − timeOfDay`, so rounding to the nearest day @@ -410,6 +411,16 @@ after the event — so `toIcsEvent` decodes it back through `fromProviderAllDayMinutes` first, and the importing device re-encodes it against its own setting. +**A backup carries the series' deleted occurrences.** `EXDATE` lives on the +master row, so the export query's `ORIGINAL_ID IS NULL` filter — which does drop +`RECURRENCE-ID` overrides — never hid it; it simply wasn't read. It is now part +of `EventExportProjection`, and `IcsWriter` marks an all-day series' exclusions +`VALUE=DATE`, because RFC 5545 ties `EXDATE`'s value type to `DTSTART`'s and a +bare day code without it reads as a malformed DATE-TIME. Sync adapters disagree +on whether an all-day exclusion is `yyyyMMdd` or a padded midnight stamp, so +`exportExDates` drops the time part on the way out. Without this, an in-app +backup→restore resurrected every occurrence the user had deleted. + That re-encode is sampled where the reminder will actually *fire*, not at `DTSTART` (`importedAllDayReminderDate`). An imported series' `DTSTART` is only an anchor and is routinely ancient — Fossify writes a year-less contact birthday at