diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt index 446489c..adaeb0c 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncoding.kt @@ -1,11 +1,10 @@ package de.jeanlucmakiola.calendula.data.calendar -import java.time.Instant +import de.jeanlucmakiola.calendula.domain.reminders.allDayLeadDays import java.time.LocalDate import java.time.LocalTime import java.time.ZoneId import java.time.ZoneOffset -import java.time.temporal.ChronoUnit /** * Translates an all-day reminder between the **semantic** lead time the UI @@ -73,19 +72,14 @@ internal fun nextYearlyOccurrence(month: Int, day: Int, today: LocalDate): Local /** * Recover the semantic whole-day lead time from a raw all-day reminder - * [rawMinutes]. Keys off the **local date** of the encoded fire instant, so it - * returns the right day count regardless of which [timeOfDayMinutes] wrote the - * row — including pre-feature rows (raw multiples of 1440, fired at UTC midnight) - * and rows written under a different timezone. A negative [rawMinutes] (fire - * after DTSTART) folds to day 0. + * [rawMinutes] — the inverse of [toProviderAllDayMinutes], for the form and the + * detail screen. Delegates to [allDayLeadDays] so what is displayed is the day + * the reminder actually fires on; see there for how an encoded row is told from + * a plain one written by another calendar app. */ internal fun fromProviderAllDayMinutes( rawMinutes: Int, startDate: LocalDate, zone: ZoneId, -): Int { - val utcMidnight = startDate.atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() - val fireLocalDate = Instant.ofEpochMilli(utcMidnight - rawMinutes * MILLIS_PER_MINUTE) - .atZone(zone).toLocalDate() - return ChronoUnit.DAYS.between(fireLocalDate, startDate).toInt() * MINUTES_PER_DAY -} + timeOfDayMinutes: Int, +): Int = allDayLeadDays(rawMinutes, startDate, zone, timeOfDayMinutes).toInt() * MINUTES_PER_DAY 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 499b956..17e62ee 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 @@ -54,7 +54,12 @@ import javax.inject.Singleton interface CalendarDataSource { fun calendars(): List fun instances(beginMillis: Long, endMillis: Long): List - fun eventDetail(eventId: Long): EventDetail? + /** + * [allDayReminderTimeMinutes]: the hour all-day reminders fire at, needed to + * decode their stored offsets back to whole-day lead times (see + * [fromProviderAllDayMinutes]). + */ + fun eventDetail(eventId: Long, allDayReminderTimeMinutes: Int): EventDetail? /** * Master/one-off events whose title, description or location contains @@ -670,7 +675,7 @@ class AndroidCalendarDataSource @Inject constructor( } } - override fun eventDetail(eventId: Long): EventDetail? { + override fun eventDetail(eventId: Long, allDayReminderTimeMinutes: Int): EventDetail? { val attendees = queryAttendees(eventId) val reminders = queryReminders(eventId) return resolver.query( @@ -679,7 +684,9 @@ class AndroidCalendarDataSource @Inject constructor( null, null, null, )?.use { c -> if (!c.moveToFirst()) null - else CursorColumnReader(c).toEventDetailCore(attendees, reminders) + else CursorColumnReader(c).toEventDetailCore( + attendees, reminders, allDayReminderTimeMinutes, + ) } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt index 19dcb5a..9d57346 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/CalendarRepositoryImpl.kt @@ -179,7 +179,8 @@ class CalendarRepositoryImpl @Inject constructor( } override suspend fun eventDetail(eventId: Long): EventDetail = withContext(io) { - dataSource.eventDetail(eventId) ?: throw NoSuchEventException(eventId) + dataSource.eventDetail(eventId, allDayReminderTimeMinutes()) + ?: throw NoSuchEventException(eventId) } override suspend fun searchEvents(query: String): List = withContext(io) { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapper.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapper.kt index 3711c81..cbf572d 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapper.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapper.kt @@ -24,6 +24,7 @@ private const val TAG = "EventDetailMapper" internal fun ColumnReader.toEventDetailCore( attendees: List, reminders: List, + allDayReminderTimeMinutes: Int, ): EventDetail? { // DTSTART is epoch millis in UTC, so a series anchored before 1970 (common // for yearly birthdays/anniversaries synced over CalDAV) is legitimately @@ -89,7 +90,13 @@ internal fun ColumnReader.toEventDetailCore( val displayReminders = if (isAllDay) { val startDate = Instant.ofEpochMilli(begin).atZone(ZoneOffset.UTC).toLocalDate() val zone = ZoneId.systemDefault() - reminders.map { it.copy(minutes = fromProviderAllDayMinutes(it.minutes, startDate, zone)) } + reminders.map { + it.copy( + minutes = fromProviderAllDayMinutes( + it.minutes, startDate, zone, allDayReminderTimeMinutes, + ), + ) + } } else { reminders } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt index 53c883b..20d5221 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/data/reminders/ReminderActionReceiver.kt @@ -3,6 +3,7 @@ package de.jeanlucmakiola.calendula.data.reminders import android.content.BroadcastReceiver import android.content.Context import android.content.Intent +import androidx.core.net.toUri import dagger.hilt.android.AndroidEntryPoint import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs import kotlinx.coroutines.CoroutineScope @@ -91,10 +92,19 @@ class ReminderActionReceiver : BroadcastReceiver() { private const val EXTRA_LOCATION = "location" private const val EXTRA_ALL_DAY = "all_day" - /** An explicit intent to this receiver carrying [alert] as extras. */ + /** + * An explicit intent to this receiver carrying [alert] as extras. + * + * The data URI names the reminder and carries no information the extras + * don't. It is what actually keeps two reminders' `PendingIntent`s + * apart: `filterEquals` compares action, component, data, type and + * categories — never extras — so without it a request-code collision + * would have them share one alarm and one payload. + */ fun intent(context: Context, action: String, alert: ReminderAlert): Intent = Intent(context, ReminderActionReceiver::class.java).apply { this.action = action + data = "calendula://reminder/${alert.key}".toUri() putExtra(EXTRA_ALERT_KEY, alert.key) putExtra(EXTRA_EVENT_ID, alert.eventId) putExtra(EXTRA_CALENDAR_ID, alert.calendarId) @@ -110,9 +120,9 @@ class ReminderActionReceiver : BroadcastReceiver() { * PendingIntents stay distinct and don't clobber each other. * * The key is a hash now rather than a small row id, so the shift is what - * keeps the action slot intact; the top three bits it drops only make two - * *different* reminders collide, which the intent extras then separate - * (PendingIntents compare their intents too, not just the request code). + * keeps the action slot intact; the top three bits it drops can make two + * *different* reminders share a code, which the per-reminder data URI in + * [intent] separates. */ fun requestCode(alert: ReminderAlert, action: String): Int { val actionOffset = when (action) { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt b/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt index e098b3f..134c8fb 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt @@ -124,30 +124,59 @@ private fun allDayDate(beginMillis: Long): LocalDate = /** * How many whole days before its occurrence a raw all-day offset means. * - * A plain multiple of 1440 is read at face value. That covers rows from other - * calendar apps, which carry no encoded hour — and it stays right for our own - * rows that happen to land on a multiple, because those encode a wall-clock hour - * equal to the sampled UTC offset, so the day count is the same either way. + * Normally the local date of the encoded fire instant answers it: our own rows + * fold the wanted hour into the offset, so that date *is* the day the reminder + * belongs to, whatever the offset was when it was written. * - * Anything else is one of ours, with an hour folded in: recover the day count the - * way [de.jeanlucmakiola.calendula.data.calendar.fromProviderAllDayMinutes] - * does for display, by asking which local date the encoded instant falls on. - * Keeping the two in step is what makes the notification arrive on the day the - * event screen says it will. + * A plain multiple of 1440 is the exception. Those come from calendar apps that + * store a bare lead time against UTC midnight, and east of UTC both readings + * agree anyway — but west of UTC the instant falls on the previous local date, + * so the day count has to be taken at face value or it comes out one too many. + * + * Except when the row is one of ours after all: our offset lands on a multiple + * whenever the all-day hour equals the zone's UTC offset (20:00 in New York, + * 19:00 an hour further west, and so on), and reading those at face value fired + * them a day late — a "1 day before" arriving on the morning of the event. So + * the hour decides, not the shape of the number: an instant landing on the hour + * the setting names is ours. Within an hour or so of it counts, because a fixed + * offset written in one DST phase drifts by the offset delta in the other. + * + * The two are genuinely indistinguishable in that band — the encodings collide + * exactly there — so the tie goes to the reading that honours the lead time the + * user chose in this app. + * + * [de.jeanlucmakiola.calendula.data.calendar.fromProviderAllDayMinutes] calls + * this for display, so the notification arrives on the day the event screen says + * it will. */ -internal fun allDayLeadDays(rawMinutes: Int, beginMillis: Long, zone: ZoneId): Long { - if (rawMinutes % MINUTES_PER_DAY == 0) return (rawMinutes / MINUTES_PER_DAY).toLong() - val encoded = Instant.ofEpochMilli(beginMillis - rawMinutes * MILLIS_PER_MINUTE) - return ChronoUnit.DAYS.between(encoded.atZone(zone).toLocalDate(), allDayDate(beginMillis)) +internal fun allDayLeadDays( + rawMinutes: Int, + startDate: LocalDate, + zone: ZoneId, + allDayTimeMinutes: Int, +): Long { + val utcMidnight = startDate.atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() + val encoded = Instant.ofEpochMilli(utcMidnight - rawMinutes * MILLIS_PER_MINUTE).atZone(zone) + val minutesFromNamedHour = encoded.toLocalTime().let { + val delta = (it.hour * 60 + it.minute - allDayTimeMinutes).mod(MINUTES_PER_DAY) + minOf(delta, MINUTES_PER_DAY - delta) + } + if (rawMinutes % MINUTES_PER_DAY == 0 && minutesFromNamedHour > NAMED_HOUR_TOLERANCE_MINUTES) { + return (rawMinutes / MINUTES_PER_DAY).toLong() + } + return ChronoUnit.DAYS.between(encoded.toLocalDate(), startDate) } +/** DST drift a row written in the other phase carries, rounded up past Lord Howe's half hour. */ +private const val NAMED_HOUR_TOLERANCE_MINUTES = 90 + private fun allDayAlarmMillis( beginMillis: Long, rawMinutes: Int, zone: ZoneId, allDayTimeMinutes: Int, ): Long = allDayDate(beginMillis) - .minusDays(allDayLeadDays(rawMinutes, beginMillis, zone)) + .minusDays(allDayLeadDays(rawMinutes, allDayDate(beginMillis), zone, allDayTimeMinutes)) .atTime(LocalTime.of(allDayTimeMinutes / 60, allDayTimeMinutes % 60)) .atZone(zone) .toInstant() @@ -214,6 +243,16 @@ fun reminderWatermark(lastScanMillis: Long?, nowMillis: Long): Long = * reminder in time: the plain lookahead plus the longest offset any reminder row * carries, so a "two weeks before" reminder is planned before it comes due * instead of firing late (the limitation Etar's equivalent documents). + * + * The stretch is capped at [MAX_REMINDER_LEAD_MILLIS]. `maxReminderMinutes` is + * whatever the largest row in the whole provider says, across every calendar and + * whoever wrote it — an imported `TRIGGER:-P100W`, or a sync adapter writing + * nonsense, would otherwise make every scan (launch, every provider change, every + * alarm) expand every recurring series over years. A lead beyond the cap is not + * delivered; a year is far past anything the picker composes. */ fun reminderQueryHorizon(lookaheadMillis: Long, maxReminderMinutes: Int): Long = - lookaheadMillis + maxOf(0L, maxReminderMinutes * MILLIS_PER_MINUTE) + lookaheadMillis + (maxReminderMinutes * MILLIS_PER_MINUTE).coerceIn(0L, MAX_REMINDER_LEAD_MILLIS) + +/** Longest reminder offset a scan stretches its query window for — one year. */ +const val MAX_REMINDER_LEAD_MILLIS = 365L * 24 * 60 * 60 * 1000 diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt index 33385ff..83d432e 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/RootScreen.kt @@ -51,6 +51,10 @@ fun RootScreen( ) } + // Whether the app came up already holding it — a launch scan has covered + // that case, so only a grant made during this session owes a re-scan. + val grantedAtLaunch = remember { hasPermission } + val lifecycle = LocalLifecycleOwner.current.lifecycle DisposableEffect(lifecycle) { val obs = LifecycleEventObserver { _, event -> @@ -88,7 +92,10 @@ fun RootScreen( // Runs on entry however the permission was granted — including from // Android's app-settings screen, which only comes back through the // ON_RESUME check above. Cheap once there is nothing left to do. - LaunchedEffect(Unit) { visibilityNotice.reconcile() } + LaunchedEffect(Unit) { + visibilityNotice.reconcile() + if (!grantedAtLaunch) reminderOnboarding.rearmAfterGrant() + } if (onboardingDone == true && noticePending) { CalendarVisibilityNoticeDialog(onDismiss = visibilityNotice::dismiss) } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt index 3a005c0..3f2fc3f 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/calendars/CalendarsViewModel.kt @@ -153,9 +153,12 @@ class CalendarsViewModel @Inject constructor( * reminders. Nothing is patched by hand — the provider notifies and the * observer re-queries. * - * Nothing has to be re-posted on the way back on: reminder delivery plans - * from `Instances` and `Reminders` on every scan (#75), and the provider - * change this write makes triggers one. + * A reminder that came due while the calendar was off stays gone when it is + * switched back on. Delivery plans from `Instances` and `Reminders` on every + * scan (#75), but the watermark has already moved past that moment, so no + * scan returns it again — and on a read-only install the switch never + * reaches the provider at all, so nothing even triggers one. Deliberate: the + * reminder was silenced on purpose, and the event itself is back in view. */ fun setCalendarVisible(id: Long, visible: Boolean) = write { repository.setCalendarsVisible(listOf(id), visible) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt index e20bf7b..f2570c0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/permission/ReminderOnboardingViewModel.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel import de.jeanlucmakiola.calendula.data.prefs.SettingsPrefs +import de.jeanlucmakiola.calendula.data.reminders.ReminderScanner import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map @@ -19,6 +20,7 @@ import javax.inject.Inject @HiltViewModel class ReminderOnboardingViewModel @Inject constructor( private val prefs: SettingsPrefs, + private val scanner: ReminderScanner, ) : ViewModel() { val onboardingDone: StateFlow = prefs.reminderOnboardingDone @@ -34,6 +36,19 @@ class ReminderOnboardingViewModel @Inject constructor( viewModelScope.launch { prefs.setRemindersEnabled(remindersEnabled) prefs.setReminderOnboardingDone() + // Nothing else re-arms the scan: turning reminders off cancels the + // alarm, so a later "on" would sit without one until a provider + // change or the daily worker happened to run (#75). + scanner.scan() } } + + /** + * Re-scan after the calendar permission is granted. The launch scan runs + * before the grant and bails out without arming anything, so without this + * the first alarm waits on the daily worker. + */ + fun rearmAfterGrant() { + scanner.scanInBackground() + } } diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt index 9adcf3a..8388991 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/settings/SettingsViewModel.kt @@ -27,6 +27,7 @@ import de.jeanlucmakiola.calendula.data.prefs.SpecialDatesStalledReason import de.jeanlucmakiola.calendula.data.prefs.ThemeMode import de.jeanlucmakiola.calendula.data.prefs.TimeFormatPref import de.jeanlucmakiola.calendula.data.prefs.WeekStartPref +import de.jeanlucmakiola.calendula.data.reminders.ReminderScanner import de.jeanlucmakiola.calendula.domain.CalendarSource import de.jeanlucmakiola.calendula.domain.EventFormField import de.jeanlucmakiola.calendula.domain.FontRole @@ -68,6 +69,7 @@ class SettingsViewModel @Inject constructor( private val specialDatesEngine: SpecialDatesSyncEngine, specialDatesSpec: SpecialDatesCalendarSpec, private val launcherNameManager: LauncherNameManager, + private val reminderScanner: ReminderScanner, @IoDispatcher private val io: CoroutineDispatcher, @ApplicationContext private val appContext: Context, ) : ViewModel() { @@ -582,8 +584,16 @@ class SettingsViewModel @Inject constructor( viewModelScope.launch { prefs.setDrawerViewOrder(order) } } + /** + * A scan follows the write, in that order. Switching reminders off cancels + * the scan alarm, so switching them back on has to arm a new one — nothing + * else would until a provider change or the daily worker came along (#75). + */ fun setRemindersEnabled(enabled: Boolean) { - viewModelScope.launch { prefs.setRemindersEnabled(enabled) } + viewModelScope.launch { + prefs.setRemindersEnabled(enabled) + reminderScanner.scan() + } } fun setAutofocusEventTitle(enabled: Boolean) { @@ -598,8 +608,12 @@ class SettingsViewModel @Inject constructor( viewModelScope.launch { prefs.setDefaultAllDayReminderMinutes(minutes) } } + /** The armed alarm was planned for the old hour, so re-plan against the new one. */ fun setAllDayReminderTimeMinutes(minutesOfDay: Int) { - viewModelScope.launch { prefs.setAllDayReminderTimeMinutes(minutesOfDay) } + viewModelScope.launch { + prefs.setAllDayReminderTimeMinutes(minutesOfDay) + reminderScanner.scan() + } } fun setSnoozeMinutes(minutes: Int) { diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncodingTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncodingTest.kt index 7ebb69b..0dd4cc4 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncodingTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/AllDayReminderEncodingTest.kt @@ -54,7 +54,8 @@ class AllDayReminderEncodingTest { for (time in listOf(0, nineAm, 20 * 60)) { for (semantic in listOf(0, 1_440, 2_880, 10_080)) { val raw = toProviderAllDayMinutes(semantic, date, berlin, time) - assertThat(fromProviderAllDayMinutes(raw, date, berlin)).isEqualTo(semantic) + assertThat(fromProviderAllDayMinutes(raw, date, berlin, time)) + .isEqualTo(semantic) } } } @@ -64,9 +65,31 @@ class AllDayReminderEncodingTest { fun `pre-feature rows (raw multiple of 1440) still decode to whole days`() { // Reminders written before this feature stored raw N*1440 (fired at UTC // midnight). They must still read back as "N days before". - assertThat(fromProviderAllDayMinutes(1_440, summer, berlin)).isEqualTo(1_440) - assertThat(fromProviderAllDayMinutes(1_440, winter, berlin)).isEqualTo(1_440) - assertThat(fromProviderAllDayMinutes(2_880, summer, berlin)).isEqualTo(2_880) + assertThat(fromProviderAllDayMinutes(1_440, summer, berlin, nineAm)).isEqualTo(1_440) + assertThat(fromProviderAllDayMinutes(1_440, winter, berlin, nineAm)).isEqualTo(1_440) + assertThat(fromProviderAllDayMinutes(2_880, summer, berlin, nineAm)).isEqualTo(2_880) + } + + @Test + fun `an offset that encodes to a multiple still decodes to its own lead time`() { + // New York at 20:00 is the collision: "1 day before" encodes to a plain 0, + // which read at face value would say "at time of event". The screen has to + // show what the reminder does, which is fire the evening before. + val newYork = ZoneId.of("America/New_York") + val eightPm = 20 * 60 + val raw = toProviderAllDayMinutes(1_440, summer, newYork, eightPm) + + assertThat(raw).isEqualTo(0) + assertThat(fromProviderAllDayMinutes(raw, summer, newYork, eightPm)).isEqualTo(1_440) + } + + @Test + fun `a foreign multiple west of UTC keeps its face-value lead time`() { + // No encoded hour in this row, and its instant lands on the evening two + // days out in New York — the local-date reading would say "2 days before". + val newYork = ZoneId.of("America/New_York") + + assertThat(fromProviderAllDayMinutes(1_440, summer, newYork, nineAm)).isEqualTo(1_440) } @Test @@ -74,8 +97,8 @@ class AllDayReminderEncodingTest { val atNine = toProviderAllDayMinutes(1_440, summer, berlin, nineAm) val atEight = toProviderAllDayMinutes(1_440, summer, berlin, 8 * 60) assertThat(atNine).isNotEqualTo(atEight) - assertThat(fromProviderAllDayMinutes(atNine, summer, berlin)).isEqualTo(1_440) - assertThat(fromProviderAllDayMinutes(atEight, summer, berlin)).isEqualTo(1_440) + assertThat(fromProviderAllDayMinutes(atNine, summer, berlin, nineAm)).isEqualTo(1_440) + assertThat(fromProviderAllDayMinutes(atEight, summer, berlin, nineAm)).isEqualTo(1_440) } @Test diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapperTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapperTest.kt index 36d397d..3b4203c 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapperTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/EventDetailMapperTest.kt @@ -79,7 +79,8 @@ class EventDetailMapperTest { private fun MapColumnReader.toDetail( attendees: List = emptyList(), reminders: List = emptyList(), - ) = toEventDetailCore(attendees, reminders) + allDayReminderTimeMinutes: Int = 9 * 60, + ) = toEventDetailCore(attendees, reminders, allDayReminderTimeMinutes) @Test fun `happy path detail maps all fields and embeds matching EventInstance`() { diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt index ccc9b68..89f96ae 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/data/calendar/FakeCalendarDataSource.kt @@ -72,7 +72,8 @@ internal class FakeCalendarDataSource : CalendarDataSource { override fun instances(beginMillis: Long, endMillis: Long): List = instancesResult(beginMillis, endMillis) override fun searchEvents(query: String): List = searchResult(query) - override fun eventDetail(eventId: Long): EventDetail? = eventDetailResult(eventId) + override fun eventDetail(eventId: Long, allDayReminderTimeMinutes: Int): EventDetail? = + eventDetailResult(eventId) override fun eventColorPalette(calendarId: Long): List = eventColorPaletteResult(calendarId) override fun exportableEvents(calendarIds: Set?): List { diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt index 8dcd1c1..cf17e91 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt @@ -184,8 +184,58 @@ class ReminderPlanTest { @Test fun `a zero-day row fires on the event's own date`() { - assertThat(allDayLeadDays(rawMinutes = 0, beginMillis = allDayBegin("2026-07-15"), zone = berlin)) - .isEqualTo(0L) + assertThat( + allDayLeadDays( + rawMinutes = 0, + startDate = LocalDate.parse("2026-07-15"), + zone = berlin, + allDayTimeMinutes = nineAm, + ), + ).isEqualTo(0L) + } + + @Test + fun `our own row that encodes to a multiple is not read at face value`() { + // New York with the all-day hour at 20:00: "1 day before" encodes to a + // plain 0, because 20:00 EDT *is* UTC midnight. Read at face value it + // fired at 20:00 on the day of the event instead of the evening before. + val newYork = ZoneId.of("America/New_York") + val eightPm = 20 * 60 + val row = encodedAllDayMinutes("2026-07-15", days = 1, timeOfDayMinutes = eightPm, zone = newYork) + assertThat(row % 1_440).isEqualTo(0) + + val alarm = plan( + instances = listOf( + instance(1L, beginMillis = allDayBegin("2026-07-15"), isAllDay = true), + ), + minutesByEvent = mapOf(1L to listOf(row)), + zone = newYork, + allDayTimeMinutes = eightPm, + ).single().alarmMillis + + assertThat(firedAt(alarm, newYork).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2026-07-14").atTime(20, 0)) + } + + @Test + fun `such a row keeps its lead time across the DST boundary that wrote it`() { + // Same row, sampled in EDT, on a winter occurrence: its instant lands an + // hour off the named hour, which still has to read as "one of ours". + val newYork = ZoneId.of("America/New_York") + val eightPm = 20 * 60 + val summerRow = encodedAllDayMinutes("2026-07-15", days = 1, timeOfDayMinutes = eightPm, zone = newYork) + + val alarm = plan( + instances = listOf( + instance(1L, beginMillis = allDayBegin("2027-01-15"), isAllDay = true), + ), + minutesByEvent = mapOf(1L to listOf(summerRow)), + zone = newYork, + allDayTimeMinutes = eightPm, + ).single().alarmMillis + + assertThat(firedAt(alarm, newYork).toLocalDateTime()) + .isEqualTo(LocalDate.parse("2027-01-14").atTime(20, 0)) } @Test @@ -437,6 +487,17 @@ class ReminderPlanTest { .isEqualTo(7 * day + 14 * day) } + @Test + fun `the query horizon is capped against an outlier row`() { + // The longest offset comes from any row in the provider — an imported + // TRIGGER:-P100W would otherwise expand every series over two years, on + // every scan. + val hundredWeeks = 100 * 7 * 24 * 60 + + assertThat(reminderQueryHorizon(lookaheadMillis = 7 * day, maxReminderMinutes = hundredWeeks)) + .isEqualTo(7 * day + MAX_REMINDER_LEAD_MILLIS) + } + @Test fun `a first-ever scan starts from now, not from the epoch`() { // Otherwise an install — or the upgrade onto in-house delivery — treats diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d84f2eb..41c4a17 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -175,7 +175,12 @@ an app update or a doze window costs nothing. A first-ever scan claims the present rather than the epoch, and a watermark left in the future by a clock change is clamped. Every trigger runs the same idempotent `scan()`, so there is no ordering between them to get wrong; `BOOT_COMPLETED` and `MY_PACKAGE_REPLACED` -matter because both wipe pending alarms. +matter because both wipe pending alarms. Turning reminders off cancels the alarm +and granting the calendar permission arms none, so those transitions scan too — +without it, switching reminders back on would sit silent until the daily worker. +The window a scan reads is the 7-day lookahead plus the longest reminder offset +in the provider, capped at a year: that offset is whatever the largest row says, +including one imported from a stray `TRIGGER:-P100W`. **All-day reminders fire at the hour the setting names.** The stored offset is not a plain lead time — `AllDayReminderEncoding` folds a wall-clock hour into it, @@ -183,7 +188,13 @@ sampled against one date's UTC offset — so taking it at face value drifts by t offset delta across a DST boundary, and rows from other apps carry no hour at all. The offset is therefore read only for *which day* it means; the hour comes from the global all-day reminder setting, recomposed against each occurrence's -own date. Timed reminders need none of this: `begin` is an absolute instant. +own date. Which day that is comes from the local date the encoded instant falls +on — except for a plain multiple of 1440, read at face value because a foreign +row means literal days from UTC midnight. The two collide where the all-day hour +equals the zone's UTC offset (20:00 in New York), and there the instant landing +on the named hour decides it is ours; the display path decodes through the same +function, so the screen and the notification agree. Timed reminders need none of +this: `begin` is an absolute instant. **One visibility model.** The scan only plans occurrences of calendars with `Calendars.VISIBLE = 1`, and that flag *is* the app's on/off switch: Settings →