fix(ics): keep a task's span, drop after-start timed alarms (#225)
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".
This commit is contained in:
@@ -53,7 +53,10 @@ data class ParsedIcsEvent(
|
|||||||
* [reminderMinutes] mapped onto Calendula's model: a lead time in minutes before
|
* [reminderMinutes] mapped onto Calendula's model: a lead time in minutes before
|
||||||
* the event, never negative.
|
* 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
|
* 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
|
* `AllDayReminderEncoding`), so a raw offset counts the whole days its trigger
|
||||||
* lands *earlier than* the event's UTC midnight — i.e. it rounds **up**.
|
* 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.
|
* ceils to zero days before.
|
||||||
*/
|
*/
|
||||||
fun ParsedIcsEvent.semanticReminderMinutes(): List<Int> = reminderMinutes
|
fun ParsedIcsEvent.semanticReminderMinutes(): List<Int> = reminderMinutes
|
||||||
.map { raw ->
|
.mapNotNull { raw ->
|
||||||
if (isAllDay) {
|
if (isAllDay) {
|
||||||
ceil(raw.toDouble() / MINUTES_PER_DAY).toInt().coerceAtLeast(0) * MINUTES_PER_DAY
|
ceil(raw.toDouble() / MINUTES_PER_DAY).toInt().coerceAtLeast(0) * MINUTES_PER_DAY
|
||||||
} else {
|
} else {
|
||||||
raw.coerceAtLeast(0)
|
raw.takeIf { it >= 0 }
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
.distinct()
|
.distinct()
|
||||||
@@ -179,6 +182,7 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
|
|||||||
var summary = ""
|
var summary = ""
|
||||||
var dtStart: IcsDateTime? = null
|
var dtStart: IcsDateTime? = null
|
||||||
var dtEnd: IcsDateTime? = null
|
var dtEnd: IcsDateTime? = null
|
||||||
|
var due: IcsDateTime? = null
|
||||||
var duration: String? = null
|
var duration: String? = null
|
||||||
var rrule: String? = null
|
var rrule: String? = null
|
||||||
var location: 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 }
|
"UID" -> uid = line.value.trim().ifEmpty { null }
|
||||||
"SUMMARY" -> summary = unescapeText(line.value)
|
"SUMMARY" -> summary = unescapeText(line.value)
|
||||||
"DTSTART" -> dtStart = parseIcsDateTime(line, warnings)
|
"DTSTART" -> dtStart = parseIcsDateTime(line, warnings)
|
||||||
// A VTODO has no DTSTART of its own in most producers' output.
|
"DUE" -> if (isTask) due = parseIcsDateTime(line, warnings)
|
||||||
"DUE" -> if (isTask && dtStart == null) dtStart = parseIcsDateTime(line, warnings)
|
|
||||||
"DTEND" -> dtEnd = parseIcsDateTime(line, warnings)
|
"DTEND" -> dtEnd = parseIcsDateTime(line, warnings)
|
||||||
"DURATION" -> duration = line.value.trim()
|
"DURATION" -> duration = line.value.trim()
|
||||||
"RRULE" -> rrule = line.value.trim().ifEmpty { null }
|
"RRULE" -> rrule = line.value.trim().ifEmpty { null }
|
||||||
@@ -244,6 +247,14 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
|
|||||||
warnings.add(IcsParseWarning.ModifiedOccurrenceSkipped)
|
warnings.add(IcsParseWarning.ModifiedOccurrenceSkipped)
|
||||||
return null
|
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 {
|
val start = dtStart ?: run {
|
||||||
warnings.add(IcsParseWarning.EventWithoutStartSkipped)
|
warnings.add(IcsParseWarning.EventWithoutStartSkipped)
|
||||||
return null
|
return null
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package de.jeanlucmakiola.calendula.domain.ics
|
|||||||
import com.google.common.truth.Truth.assertThat
|
import com.google.common.truth.Truth.assertThat
|
||||||
import kotlinx.datetime.TimeZone
|
import kotlinx.datetime.TimeZone
|
||||||
import org.junit.jupiter.api.Test
|
import org.junit.jupiter.api.Test
|
||||||
|
import kotlin.time.Instant
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Reading the Simple Calendar / Fossify export dialect (Codeberg #225).
|
* Reading the Simple Calendar / Fossify export dialect (Codeberg #225).
|
||||||
@@ -176,6 +177,41 @@ class IcsFossifyImportTest {
|
|||||||
assertThat(result.events.single().summary).isEqualTo("File taxes")
|
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
|
@Test
|
||||||
fun `the per-event colour wins over the calendar colour`() {
|
fun `the per-event colour wins over the calendar colour`() {
|
||||||
val result = parser.parse(
|
val result = parser.parse(
|
||||||
@@ -388,6 +424,27 @@ class IcsFossifyImportTest {
|
|||||||
assertThat(result.events.single().semanticReminderMinutes()).containsExactly(10)
|
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
|
@Test
|
||||||
fun `an all-day series' EXDATE day codes carry over`() {
|
fun `an all-day series' EXDATE day codes carry over`() {
|
||||||
val result = parser.parse(
|
val result = parser.parse(
|
||||||
|
|||||||
@@ -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
|
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
|
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
|
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
|
spells out rather than the day its resolved instant lands on in UTC, and an
|
||||||
`VALARM` trigger pointing *after* the start — which is how that family encodes
|
all-day `VALARM` trigger pointing *after* the start — which is how that family
|
||||||
"on the day at 09:00" — is read as zero days before rather than clamped to a lead
|
encodes "on the day at 09:00" — is read as zero days before. On a *timed* event
|
||||||
time of zero.
|
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
|
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
|
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
|
`fromProviderAllDayMinutes` first, and the importing device re-encodes it against
|
||||||
its own setting.
|
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
|
That re-encode is sampled where the reminder will actually *fire*, not at
|
||||||
`DTSTART` (`importedAllDayReminderDate`). An imported series' `DTSTART` is only an
|
`DTSTART` (`importedAllDayReminderDate`). An imported series' `DTSTART` is only an
|
||||||
anchor and is routinely ancient — Fossify writes a year-less contact birthday at
|
anchor and is routinely ancient — Fossify writes a year-less contact birthday at
|
||||||
|
|||||||
Reference in New Issue
Block a user