makiolaj/calendula
1
0
Fork 1
Code Actions Releases 32 Activity

fix(ics): round all-day reminders up and stop dropping them on export (#225)

Browse Source 
Review follow-ups on the import work:

- All-day reminder offsets are `days * 1440 - timeOfDay`, so rounding to the
  nearest day lost a day for any producer whose all-day notifications fire after
  noon ("1 day before at 18:00" arrives as -PT6H). Round up instead.
- Export decodes an all-day row's raw provider offset back to its whole-day lead
  time. It is normally negative, and the writer discards a trigger that fires
  after the event, so our own backups came back with no all-day reminder at all.
- A floating EXDATE DATE-TIME is read in the series' zone, not the device's;
  otherwise the exclusion lands on an instant no occurrence has.
- Wire the parsed calendar name up: it had no reader, so the CATEGORIES handling
  was inert. It now preselects a target calendar of that name.
This commit is contained in:
Jean-Luc Makiola
2026-08-27 19:36:45 +02:00
parent c0361686df
commit db4611d284
13 changed files with 254 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt
View File

@@ -91,8 +91,10 @@ interface CalendarDataSource {
* rows are excluded (see [EventExportProjection]). When [calendarIds] is
* given, only those calendars are exported (still intersected with the
* eligible set); `null` exports every eligible calendar.
* [allDayReminderTimeMinutes]: needed to write all-day reminders as whole-day
* lead times rather than raw provider offsets (see [toIcsEvent]).
*/
fun exportableEvents(calendarIds: Set<Long>? = null): List<IcsEvent>
fun exportableEvents(calendarIds: Set<Long>?, allDayReminderTimeMinutes: Int): List<IcsEvent>
/**
* The non-empty `Events.UID_2445` values present in [calendarId] — used to
@@ -752,7 +754,10 @@ class AndroidCalendarDataSource @Inject constructor(
?: emptyList()
}
override fun exportableEvents(calendarIds: Set<Long>?): List<IcsEvent> {
override fun exportableEvents(
calendarIds: Set<Long>?,
allDayReminderTimeMinutes: Int,
): List<IcsEvent> {
// Only the local calendars the app owns and can write — synced calendars
// already have a backup (their server). Exclude the managed special-dates
// mirror calendars: their events are derived from contacts, not authored
@@ -787,6 +792,7 @@ class AndroidCalendarDataSource @Inject constructor(
reader.toIcsEvent(
reminderMinutes = queryReminders(eventId).map { it.minutes },
calendarName = names[calendarId],
allDayReminderTimeMinutes = allDayReminderTimeMinutes,
)
}
} ?: emptyList()

2
app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt
View File

@@ -212,7 +212,7 @@ class CalendarRepositoryImpl @Inject constructor(
}
override suspend fun exportEvents(calendarIds: Set<Long>?) =
withContext(io) { dataSource.exportableEvents(calendarIds) }
withContext(io) { dataSource.exportableEvents(calendarIds, allDayReminderTimeMinutes()) }
override suspend fun importEvents(
targetCalendarId: Long,

3
app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt
View File

@@ -560,7 +560,8 @@ internal fun buildImportedEventValues(
* series, seconds otherwise. An all-day series is never shorter than a day —
* the provider expands a zero-length one into no instances at all, so the
* series would vanish (which is what a literal read of the Fossify family's
* inclusive DTEND used to produce; see `IcsQuirks`).
* inclusive DTEND used to produce; see "Importing foreign .ics" in
* docs/ARCHITECTURE.md).
*/
private fun importDuration(startMillis: Long, endMillis: Long, isAllDay: Boolean): String {
val span = (endMillis - startMillis).coerceAtLeast(0)

23
app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/IcsExportMapper.kt
View File

@@ -5,6 +5,9 @@ import de.jeanlucmakiola.calendula.domain.EventStatus
import de.jeanlucmakiola.calendula.domain.ics.IcsEvent
import de.jeanlucmakiola.calendula.domain.ics.deriveIcsUid
import de.jeanlucmakiola.calendula.domain.ics.parseRfc2445DurationMillis
import java.time.Instant
import java.time.ZoneId
import java.time.ZoneOffset
/**
* Map one Events row (read through [EventExportProjection]) into an [IcsEvent]
@@ -12,14 +15,23 @@ import de.jeanlucmakiola.calendula.domain.ics.parseRfc2445DurationMillis
* [calendarName] the display name of its calendar (emitted as
* `X-CALENDULA-CALENDAR`). Pure given a [ColumnReader] — JVM-tested with
* MapColumnReader.
*
* An all-day row's raw offset has the firing time of day encoded into it and is
* normally *negative* (see [AllDayReminderEncoding]); written out literally it
* would be dropped as a trigger that fires after the event, losing the reminder.
* [allDayReminderTimeMinutes] and [zone] decode it back to the whole-day lead
* time the file should carry, which an import re-encodes for its own device.
*/
internal fun ColumnReader.toIcsEvent(
reminderMinutes: List<Int>,
calendarName: String?,
allDayReminderTimeMinutes: Int,
zone: ZoneId = ZoneId.systemDefault(),
): IcsEvent {
val eventId = getLong(EventExportProjection.IDX_ID)
val dtStart = getLong(EventExportProjection.IDX_DTSTART)
val rrule = getString(EventExportProjection.IDX_RRULE)?.takeIf { it.isNotBlank() }
val isAllDay = getInt(EventExportProjection.IDX_ALL_DAY) != 0
// Recurring rows store DURATION instead of DTEND; reconstruct the end from it
// so the writer can render DTEND. A missing/blank both means a zero-length event.
@@ -40,13 +52,20 @@ internal fun ColumnReader.toIcsEvent(
summary = getString(EventExportProjection.IDX_TITLE).orEmpty(),
start = dtStart.toKotlinInstantFromEpochMillis(),
end = end.toKotlinInstantFromEpochMillis(),
isAllDay = getInt(EventExportProjection.IDX_ALL_DAY) != 0,
isAllDay = isAllDay,
zoneId = getString(EventExportProjection.IDX_EVENT_TIMEZONE)?.takeIf { it.isNotBlank() }
?: "UTC",
recurrenceRule = rrule,
location = getString(EventExportProjection.IDX_LOCATION),
description = getString(EventExportProjection.IDX_DESCRIPTION),
reminderMinutes = reminderMinutes,
reminderMinutes = if (isAllDay) {
val startDate = Instant.ofEpochMilli(dtStart).atZone(ZoneOffset.UTC).toLocalDate()
reminderMinutes.map {
fromProviderAllDayMinutes(it, startDate, zone, allDayReminderTimeMinutes)
}
} else {
reminderMinutes
},
status = status,
availability = mapAvailability(getInt(EventExportProjection.IDX_AVAILABILITY)),
calendarName = calendarName,

34
app/src/main/java/de/jeanlucmakiola/calendula/domain/ics/IcsParser.kt
View File

@@ -10,7 +10,7 @@ import kotlinx.datetime.atStartOfDayIn
import kotlinx.datetime.number
import kotlinx.datetime.toInstant
import kotlinx.datetime.toLocalDateTime
import kotlin.math.roundToInt
import kotlin.math.ceil
import kotlin.time.Instant
/** Milliseconds in a calendar day; all-day times are UTC midnights, so this is exact. */
@@ -55,15 +55,21 @@ data class ParsedIcsEvent(
*
* For a timed event the raw lead time already is that. All-day reminders are
* whole days before the event, fired at the user's configured time of day (see
* `AllDayReminderEncoding`), so a raw offset is rounded to the nearest day —
* which is also what makes Fossify's all-day encoding land correctly: it writes
* "on the day at 09:00" as a *positive* `TRIGGER:P0DT9H0M0S`, i.e. nine hours
* after the UTC midnight, and that rounds to zero days before.
* `AllDayReminderEncoding`), so a raw offset counts the whole days its trigger
* lands *earlier than* the event's UTC midnight — i.e. it rounds **up**.
*
* Rounding to the nearest day instead would lose a day for every producer whose
* all-day notifications fire after noon: a file's raw offset is
* `days × 1440 − timeOfDay`, so "1 day before at 18:00" arrives as
* `TRIGGER:-PT6H` (raw 360) and would read as "on the day". Rounding up also
* lands Fossify's encoding correctly — it writes "on the day at 09:00" as a
* *positive* `TRIGGER:P0DT9H0M0S`, nine hours after the UTC midnight, which
* ceils to zero days before.
*/
fun ParsedIcsEvent.semanticReminderMinutes(): List<Int> = reminderMinutes
.map { raw ->
if (isAllDay) {
(raw.toDouble() / MINUTES_PER_DAY).roundToInt().coerceAtLeast(0) * MINUTES_PER_DAY
ceil(raw.toDouble() / MINUTES_PER_DAY).toInt().coerceAtLeast(0) * MINUTES_PER_DAY
} else {
raw.coerceAtLeast(0)
}
@@ -306,7 +312,7 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
.mapNotNull { (line, token) ->
if (token.isEmpty()) return@mapNotNull null
if (token.contains('T')) {
val instant = parseExDateTime(token, line) ?: return@mapNotNull null
val instant = parseExDateTime(token, line, startZone) ?: return@mapNotNull null
if (start.isAllDay) utcDayCode(instant) else utcStamp(instant)
} else {
val date = parseBasicDate(token) ?: return@mapNotNull null
@@ -320,12 +326,22 @@ class IcsParser(private val deviceZone: TimeZone = TimeZone.currentSystemDefault
.distinct()
}
private fun parseExDateTime(token: String, line: IcsContentLine): Instant? {
/**
* [startZone] is the series' own zone, which a floating value (no `Z`, no
* `TZID`) has to be read in — RFC 5545 ties `EXDATE` to `DTSTART`'s form,
* and reading it in the device's zone instead would put the exclusion on an
* instant no occurrence has.
*/
private fun parseExDateTime(
token: String,
line: IcsContentLine,
startZone: TimeZone,
): Instant? {
val ldt = parseBasicDateTime(token.removeSuffix("Z")) ?: return null
val zone = when {
token.endsWith("Z") -> TimeZone.UTC
else -> line.params["TZID"]?.let { runCatching { TimeZone.of(it) }.getOrNull() }
?: deviceZone
?: startZone
}
return ldt.toInstant(zone)
}

12
app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportScreen.kt
View File

@@ -93,15 +93,11 @@ fun ImportScreen(
}
// Hoisted target calendar so the always-visible top-bar Import action can
// read it without the user scrolling to a bottom button. Defaults to the
// first *local* calendar — the first row the picker shows ("Your calendars"
// group leads) — so the pre-selection lines up with the top of the list;
// falls back to the first calendar if there are no local ones. Re-defaults
// when the "many" list first arrives (keyed on it), then holds the pick.
// read it without the user scrolling to a bottom button (see
// [defaultImportTarget] for the choice). Re-defaults when the "many" list
// first arrives (keyed on it), then holds the pick.
val many = state as? ImportUiState.Many
val defaultTarget = many?.calendars?.let { cals ->
(cals.firstOrNull { it.isLocal } ?: cals.firstOrNull())?.id
}
val defaultTarget = many?.let { defaultImportTarget(it.calendars, it.fileCalendarName) }
var selected by rememberSaveable(defaultTarget) { mutableStateOf(defaultTarget) }
Scaffold(

29
app/src/main/java/de/jeanlucmakiola/calendula/ui/imports/ImportViewModel.kt
View File

@@ -43,6 +43,12 @@ sealed interface ImportUiState {
val events: List<ParsedIcsEvent>,
val warnings: Set<IcsParseWarning>,
val calendars: List<CalendarSource>,
/**
* The calendar the file says its events came from, when it names exactly
* one — used to preselect a matching target. Null when the file names
* none or its events disagree.
*/
val fileCalendarName: String? = null,
) : ImportUiState
data class Done(val summary: IcsImportSummary) : ImportUiState
@@ -92,6 +98,10 @@ class ImportViewModel @Inject constructor(
warnings = parsed.warnings,
calendars = repository.calendars().first()
.filter { it.isEventTarget },
fileCalendarName = parsed.events
.mapNotNull { it.calendarName }
.distinct()
.singleOrNull(),
)
}
}
@@ -113,3 +123,22 @@ class ImportViewModel @Inject constructor(
}
}
}
/**
* The target calendar to preselect for a bulk import.
*
* A calendar named like the one the file came from wins — restoring "Birthdays"
* onto the Birthdays calendar is what the user means. Otherwise the first
* *local* calendar, which is the first row the picker shows ("Your calendars"
* leads), so the pre-selection lines up with the top of the list; failing that
* the first calendar of any kind.
*/
internal fun defaultImportTarget(
calendars: List<CalendarSource>,
fileCalendarName: String?,
): Long? {
val named = fileCalendarName?.let { name ->
calendars.firstOrNull { it.displayName.equals(name, ignoreCase = true) }
}
return (named ?: calendars.firstOrNull { it.isLocal } ?: calendars.firstOrNull())?.id
}

5
app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt
View File

@@ -81,7 +81,10 @@ internal class FakeCalendarDataSource : CalendarDataSource {
eventDetailResult(eventId)
override fun eventColorPalette(calendarId: Long): List<EventColorOption> =
eventColorPaletteResult(calendarId)
override fun exportableEvents(calendarIds: Set<Long>?): List<IcsEvent> {
override fun exportableEvents(
calendarIds: Set<Long>?,
allDayReminderTimeMinutes: Int,
): List<IcsEvent> {
lastExportableEventsCalendarIds = calendarIds
return exportableEventsResult
}

41
app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/IcsExportMapperTest.kt
View File

@@ -4,6 +4,7 @@ import android.provider.CalendarContract
import com.google.common.truth.Truth.assertThat
import de.jeanlucmakiola.calendula.domain.EventStatus
import org.junit.jupiter.api.Test
import java.time.ZoneId
class IcsExportMapperTest {
@@ -20,7 +21,11 @@ class IcsExportMapperTest {
EventExportProjection.IDX_AVAILABILITY to CalendarContract.Events.AVAILABILITY_BUSY,
)
val event = reader.toIcsEvent(reminderMinutes = listOf(10), calendarName = "Personal")
val event = reader.toIcsEvent(
reminderMinutes = listOf(10),
calendarName = "Personal",
allDayReminderTimeMinutes = NINE_AM,
)
assertThat(event.uid).isEqualTo("abc@host")
assertThat(event.summary).isEqualTo("Standup")
@@ -47,7 +52,11 @@ class IcsExportMapperTest {
EventExportProjection.IDX_EVENT_TIMEZONE to "UTC",
)
val event = reader.toIcsEvent(reminderMinutes = emptyList(), calendarName = null)
val event = reader.toIcsEvent(
reminderMinutes = emptyList(),
calendarName = null,
allDayReminderTimeMinutes = NINE_AM,
)
assertThat(event.uid).isEqualTo("7-1000000@calendula")
assertThat(event.recurrenceRule).isEqualTo("FREQ=WEEKLY")
@@ -65,6 +74,32 @@ class IcsExportMapperTest {
EventExportProjection.IDX_EVENT_TIMEZONE to "UTC",
)
assertThat(reader.toIcsEvent(emptyList(), null).isAllDay).isTrue()
assertThat(reader.toIcsEvent(emptyList(), null, NINE_AM).isAllDay).isTrue()
}
@Test
fun `an all-day reminder is exported as its whole-day lead time`() {
// The raw offset encodes the firing time and is normally negative; left
// raw it would be written as a trigger after the event and dropped, so
// the backup would come back with no reminder at all.
val reader = MapColumnReader(
EventExportProjection.IDX_ID to 1L,
EventExportProjection.IDX_TITLE to "Anna Birthday",
EventExportProjection.IDX_DTSTART to 1_782_864_000_000L, // 2026-07-01 UTC, CEST
EventExportProjection.IDX_DTEND to 1_782_950_400_000L,
EventExportProjection.IDX_ALL_DAY to 1,
EventExportProjection.IDX_EVENT_TIMEZONE to "UTC",
)
val onTheDay = reader.toIcsEvent(listOf(-420), null, NINE_AM, BERLIN)
val dayBefore = reader.toIcsEvent(listOf(1_020), null, NINE_AM, BERLIN)
assertThat(onTheDay.reminderMinutes).containsExactly(0)
assertThat(dayBefore.reminderMinutes).containsExactly(1_440)
}
private companion object {
const val NINE_AM = 9 * 60
val BERLIN: ZoneId = ZoneId.of("Europe/Berlin")
}
}

43
app/src/test/java/de/jeanlucmakiola/calendula/domain/ics/IcsFossifyImportTest.kt
View File

@@ -289,6 +289,28 @@ class IcsFossifyImportTest {
assertThat(event.semanticReminderMinutes()).containsExactly(0)
}
@Test
fun `an all-day reminder whose firing time is after noon keeps its full day`() {
// The raw offset is `days * 1440 - timeOfDay`, so an 18:00 notification
// time puts "1 day before" only six hours ahead of the UTC midnight.
// Rounding to the nearest day would read that as "on the day".
val result = parser.parse(
fossify(
"BEGIN:VEVENT",
"SUMMARY:Anna Birthday",
"DTSTART;VALUE=DATE:20260819",
"DTEND;VALUE=DATE:20260820",
"BEGIN:VALARM",
"ACTION:DISPLAY",
"TRIGGER:-P0DT6H0M0S",
"END:VALARM",
"END:VEVENT",
),
)
assertThat(result.events.single().semanticReminderMinutes()).containsExactly(1440)
}
@Test
fun `an all-day reminder rounds to whole days before`() {
val result = parser.parse(
@@ -366,6 +388,27 @@ class IcsFossifyImportTest {
assertThat(result.events.single().exDates).containsExactly("20260820T170000Z")
}
@Test
fun `a floating EXDATE is read in the series' zone, not the device's`() {
// RFC 5545 ties EXDATE to DTSTART's form: with no Z and no TZID the
// value is New York wall time. Reading it in the device's Berlin zone
// would land on an instant no occurrence has, and the excluded
// occurrence would come back.
val result = parser.parse(
fossify(
"BEGIN:VEVENT",
"SUMMARY:Standup",
"DTSTART;TZID=America/New_York:20260819T100000",
"DTEND;TZID=America/New_York:20260819T103000",
"RRULE:FREQ=WEEKLY",
"EXDATE:20260826T100000",
"END:VEVENT",
),
)
assertThat(result.events.single().exDates).containsExactly("20260826T140000Z")
}
@Test
fun `EXDATE is dropped along with an unusable recurrence rule`() {
val result = parser.parse(

55
app/src/test/java/de/jeanlucmakiola/calendula/ui/imports/ImportTargetTest.kt Normal file
View File

@@ -0,0 +1,55 @@
package de.jeanlucmakiola.calendula.ui.imports
import com.google.common.truth.Truth.assertThat
import de.jeanlucmakiola.calendula.domain.CalendarSource
import org.junit.jupiter.api.Test
/** Which calendar a bulk import lands on before the user touches the picker. */
class ImportTargetTest {
private fun cal(id: Long, name: String, local: Boolean = false) = CalendarSource(
id = id,
displayName = name,
accountName = "acc@local",
accountType = if (local) "LOCAL" else "com.google",
color = 0,
isVisibleInSystem = true,
isLocal = local,
)
private val calendars = listOf(
cal(1L, "Work"),
cal(2L, "Personal", local = true),
cal(3L, "Birthdays", local = true),
)
@Test
fun `a calendar named like the file's wins`() {
assertThat(defaultImportTarget(calendars, "Birthdays")).isEqualTo(3L)
}
@Test
fun `the name match ignores case`() {
assertThat(defaultImportTarget(calendars, "birthdays")).isEqualTo(3L)
}
@Test
fun `an unmatched name falls back to the first local calendar`() {
assertThat(defaultImportTarget(calendars, "Holidays")).isEqualTo(2L)
}
@Test
fun `no name at all falls back to the first local calendar`() {
assertThat(defaultImportTarget(calendars, null)).isEqualTo(2L)
}
@Test
fun `with no local calendars the first of any kind is taken`() {
assertThat(defaultImportTarget(listOf(cal(1L, "Work")), null)).isEqualTo(1L)
}
@Test
fun `an empty list preselects nothing`() {
assertThat(defaultImportTarget(emptyList(), "Birthdays")).isNull()
}
}