From 8e3a17fdc35cd971b766de18bfe72e8e20a2beb5 Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Thu, 2 Jul 2026 22:40:26 +0200 Subject: [PATCH] refactor(calendar): share the insert values builder + reminder seeding insertManagedEvent duplicated ~45 lines of insertEvent (ContentValues build and the reminder-seeding loop). Both now go through buildEventInsertValues in EventWriteMapper and a shared seedReminders, differing only in the UID they stamp and the colour/attendee columns insertEvent layers on top. Co-Authored-By: Claude Fable 5 --- .../data/calendar/CalendarDataSource.kt | 112 ++++++------------ .../data/calendar/EventWriteMapper.kt | 36 ++++++ 2 files changed, 69 insertions(+), 79 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt index 2f56ffa..3a8e854 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarDataSource.kt @@ -416,47 +416,15 @@ class AndroidCalendarDataSource @Inject constructor( } ?: emptyList() override fun insertManagedEvent(form: EventForm, uid: String, allDayReminderTimeMinutes: Int): Long { - val times = form.toWriteTimes(ZoneId.systemDefault()) - val values = ContentValues().apply { - put( - CalendarContract.Events.CALENDAR_ID, - requireNotNull(form.calendarId) { "EventForm.calendarId is required" }, - ) - // Deterministic UID = the reconciliation key. A re-sync reads it back - // (queryManagedEvents) and diffs on it, so a contact never doubles. - put(CalendarContract.Events.UID_2445, uid) - put(CalendarContract.Events.TITLE, form.title.trim()) - put(CalendarContract.Events.ALL_DAY, if (form.isAllDay) 1 else 0) - put(CalendarContract.Events.DTSTART, times.dtStartMillis) - if (form.rrule == null) { - put(CalendarContract.Events.DTEND, times.dtEndMillis) - } else { - put(CalendarContract.Events.RRULE, form.rrule) - put(CalendarContract.Events.DURATION, times.toRfc2445Duration(form.isAllDay)) - } - put(CalendarContract.Events.EVENT_TIMEZONE, times.timezone) - put(CalendarContract.Events.AVAILABILITY, form.availability.toProviderValue()) - put(CalendarContract.Events.ACCESS_LEVEL, form.accessLevel.toProviderValue()) - form.location.trim().takeIf { it.isNotEmpty() } - ?.let { put(CalendarContract.Events.EVENT_LOCATION, it) } - form.description.trim().takeIf { it.isNotEmpty() } - ?.let { put(CalendarContract.Events.DESCRIPTION, it) } - } - val eventId = resolver.insert(CalendarContract.Events.CONTENT_URI, values) + // Deterministic UID = the reconciliation key. A re-sync reads it back + // (queryManagedEvents) and diffs on it, so a contact never doubles. + val values = buildEventInsertValues(form, uid, form.toWriteTimes(ZoneId.systemDefault())) + val eventId = resolver.insert(CalendarContract.Events.CONTENT_URI, values.toContentValues()) ?.let(ContentUris::parseId) ?: throw WriteFailedException("insert managed event into calendar id=${form.calendarId}") // Seeded once, then never re-touched by sync — so a user who moves a // birthday reminder keeps their change. - encodedReminders(form, allDayReminderTimeMinutes).forEach { minutes -> - val reminder = ContentValues().apply { - put(CalendarContract.Reminders.EVENT_ID, eventId) - put(CalendarContract.Reminders.MINUTES, minutes) - put(CalendarContract.Reminders.METHOD, CalendarContract.Reminders.METHOD_ALERT) - } - if (resolver.insert(CalendarContract.Reminders.CONTENT_URI, reminder) == null) { - Log.w(TAG, "Failed to attach reminder ($minutes min) to managed event $eventId") - } - } + seedReminders(eventId, form, allDayReminderTimeMinutes) return eventId } @@ -779,35 +747,15 @@ class AndroidCalendarDataSource @Inject constructor( .distinct() override fun insertEvent(form: EventForm, allDayReminderTimeMinutes: Int): Long { - val times = form.toWriteTimes(ZoneId.systemDefault()) - val values = ContentValues().apply { - put( - CalendarContract.Events.CALENDAR_ID, - requireNotNull(form.calendarId) { "EventForm.calendarId is required" }, - ) - // A globally-unique UID so a later .ics backup/restore can identify - // the event and not duplicate it on re-import (the provider leaves - // this null for events it didn't sync). Older rows without one fall - // back to a stable synthesised UID at export time (deriveIcsUid). - put(CalendarContract.Events.UID_2445, "${UUID.randomUUID()}@calendula") - put(CalendarContract.Events.TITLE, form.title.trim()) - put(CalendarContract.Events.ALL_DAY, if (form.isAllDay) 1 else 0) - put(CalendarContract.Events.DTSTART, times.dtStartMillis) - // The provider's invariant: recurring rows carry RRULE+DURATION - // (and no DTEND), one-off rows carry DTEND. - if (form.rrule == null) { - put(CalendarContract.Events.DTEND, times.dtEndMillis) - } else { - put(CalendarContract.Events.RRULE, form.rrule) - put(CalendarContract.Events.DURATION, times.toRfc2445Duration(form.isAllDay)) - } - put(CalendarContract.Events.EVENT_TIMEZONE, times.timezone) - put(CalendarContract.Events.AVAILABILITY, form.availability.toProviderValue()) - put(CalendarContract.Events.ACCESS_LEVEL, form.accessLevel.toProviderValue()) - form.location.trim().takeIf { it.isNotEmpty() } - ?.let { put(CalendarContract.Events.EVENT_LOCATION, it) } - form.description.trim().takeIf { it.isNotEmpty() } - ?.let { put(CalendarContract.Events.DESCRIPTION, it) } + // A globally-unique UID so a later .ics backup/restore can identify + // the event and not duplicate it on re-import (the provider leaves + // this null for events it didn't sync). Older rows without one fall + // back to a stable synthesised UID at export time (deriveIcsUid). + val values = buildEventInsertValues( + form, + uid = "${UUID.randomUUID()}@calendula", + times = form.toWriteTimes(ZoneId.systemDefault()), + ).toContentValues().apply { // A null colour just leaves both columns unset (the event inherits // its calendar's colour), so only the key/raw cases are written. when { @@ -819,19 +767,7 @@ class AndroidCalendarDataSource @Inject constructor( val uri = resolver.insert(CalendarContract.Events.CONTENT_URI, values) ?: throw WriteFailedException("insert event into calendar id=${form.calendarId}") val eventId = ContentUris.parseId(uri) - // Best effort (spec §8): the event exists at this point — a reminder - // that fails to attach is logged, not surfaced as a failed create. - encodedReminders(form, allDayReminderTimeMinutes) - .forEach { minutes -> - val reminder = ContentValues().apply { - put(CalendarContract.Reminders.EVENT_ID, eventId) - put(CalendarContract.Reminders.MINUTES, minutes) - put(CalendarContract.Reminders.METHOD, CalendarContract.Reminders.METHOD_ALERT) - } - if (resolver.insert(CalendarContract.Reminders.CONTENT_URI, reminder) == null) { - Log.w(TAG, "Failed to attach reminder ($minutes min) to event $eventId") - } - } + seedReminders(eventId, form, allDayReminderTimeMinutes) // Guests are best-effort like reminders: a row that fails to attach is // logged, not surfaced as a failed create. Calendula never sends an // invitation — it only writes the rows; the backend decides delivery. @@ -839,6 +775,24 @@ class AndroidCalendarDataSource @Inject constructor( return eventId } + /** + * Attach [form]'s reminders to a freshly inserted event. Best effort + * (spec §8): the event exists at this point — a reminder that fails to + * attach is logged, not surfaced as a failed create. + */ + private fun seedReminders(eventId: Long, form: EventForm, allDayReminderTimeMinutes: Int) { + encodedReminders(form, allDayReminderTimeMinutes).forEach { minutes -> + val reminder = ContentValues().apply { + put(CalendarContract.Reminders.EVENT_ID, eventId) + put(CalendarContract.Reminders.MINUTES, minutes) + put(CalendarContract.Reminders.METHOD, CalendarContract.Reminders.METHOD_ALERT) + } + if (resolver.insert(CalendarContract.Reminders.CONTENT_URI, reminder) == null) { + Log.w(TAG, "Failed to attach reminder ($minutes min) to event $eventId") + } + } + } + override fun updateEvent( eventId: Long, original: EventForm, diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt index 2c3928c..dc830aa 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventWriteMapper.kt @@ -49,6 +49,42 @@ internal fun EventWriteTimes.toRfc2445Duration(isAllDay: Boolean): String = if ( "P${(dtEndMillis - dtStartMillis) / 1_000L}S" } +/** + * Column values for a brand-new Events row: identity, times (the provider's + * invariant — recurring rows carry RRULE+DURATION and no DTEND, one-off rows + * carry DTEND), availability/access level and trimmed optional text. Shared by + * the user-event and managed-event insert paths, which differ only in the + * [uid] they stamp; colour and attendees are user-event concerns the caller + * layers on top. + */ +internal fun buildEventInsertValues( + form: EventForm, + uid: String, + times: EventWriteTimes, +): Map = buildMap { + put( + CalendarContract.Events.CALENDAR_ID, + requireNotNull(form.calendarId) { "EventForm.calendarId is required" }, + ) + put(CalendarContract.Events.UID_2445, uid) + put(CalendarContract.Events.TITLE, form.title.trim()) + put(CalendarContract.Events.ALL_DAY, if (form.isAllDay) 1 else 0) + put(CalendarContract.Events.DTSTART, times.dtStartMillis) + if (form.rrule == null) { + put(CalendarContract.Events.DTEND, times.dtEndMillis) + } else { + put(CalendarContract.Events.RRULE, form.rrule) + put(CalendarContract.Events.DURATION, times.toRfc2445Duration(form.isAllDay)) + } + put(CalendarContract.Events.EVENT_TIMEZONE, times.timezone) + put(CalendarContract.Events.AVAILABILITY, form.availability.toProviderValue()) + put(CalendarContract.Events.ACCESS_LEVEL, form.accessLevel.toProviderValue()) + form.location.trim().takeIf { it.isNotEmpty() } + ?.let { put(CalendarContract.Events.EVENT_LOCATION, it) } + form.description.trim().takeIf { it.isNotEmpty() } + ?.let { put(CalendarContract.Events.DESCRIPTION, it) } +} + /** * Dirty-checked column values for updating an existing Events row: only what * the user actually changed is written, so untouched fields can't stomp