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

fix(reminders): fire all-day reminders at the hour the setting names (#75)

Browse Source 
Taking the stored offset at face value is what puts an all-day reminder on the
wrong hour, and owning the alarm is what finally allows fixing it.

An all-day occurrence begins at UTC midnight, and the offset in `Reminders` is
not a plain lead time: `AllDayReminderEncoding` folds the wanted wall-clock hour
into it, sampled against one date's UTC offset. Fire at `begin - minutes` and
every occurrence in a different DST phase than the sampled one drifts by the
offset delta — an hour early one way, an hour late the other, which for a yearly
birthday is every second occurrence. Its KDoc documents that drift as inherent to
the provider model. It was, while the provider held the alarm.

Rows written by other calendar apps have the opposite problem: a conventional
1440 carries no hour at all, so it fires at UTC midnight — 02:00 local in summer
Berlin, and a day early west of UTC, where UTC midnight still falls on the
previous local date.

So the offset is now read only for which day it means, and the hour comes from
the one global all-day reminder setting, recomposed against each occurrence's own
date. Plain multiples of 1440 are read at face value, which covers foreign rows
and stays right for our own rows that land on a multiple; anything else keeps the
local-date recovery `fromProviderAllDayMinutes` already uses for display, so the
notification arrives on the day the event screen promises.

Timed reminders are untouched: `begin` is an absolute instant, so `begin -
minutes` is exact in any zone across any boundary. A test pins that too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jean-Luc Makiola
2026-07-26 19:10:17 +02:00
parent 29857495be
commit 40096c473e
2 changed files with 240 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
app/src/main/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlan.kt
View File

@@ -1,5 +1,12 @@
package de.jeanlucmakiola.calendula.domain.reminders
import java.time.Instant
import java.time.LocalDate
import java.time.LocalTime
import java.time.ZoneId
import java.time.ZoneOffset
import java.time.temporal.ChronoUnit
/**
* Works out *when* each reminder has to fire and *which* ones are due, with no
* provider and no clock of its own — the whole decision layer of in-house
@@ -63,15 +70,30 @@ data class ReminderSchedule(
)
private const val MILLIS_PER_MINUTE = 60_000L
private const val MINUTES_PER_DAY = 1_440
/**
* Pair every instance with each of its event's reminder offsets.
*
* The fire instant is `begin − minutes` for timed **and** all-day occurrences
* alike. That is not an oversight: an all-day instance's `begin` is UTC midnight,
* and `AllDayReminderEncoding` deliberately encodes the wanted wall-clock time
* *into* the stored offset measured from exactly that point. Applying a second,
* timezone-dependent correction here would undo it.
* A **timed** occurrence is trivial: `begin` is an absolute instant, so
* `begin − minutes` is exact by construction, in any timezone, across any DST
* boundary.
*
* An **all-day** occurrence is not, and taking the offset at face value is what
* makes reminders land at the wrong hour. Its `begin` is UTC midnight, and the
* stored offset is not a plain lead time — `AllDayReminderEncoding` folds the
* wanted wall-clock hour into it, sampled against *one* date's UTC offset. Fire
* at `begin − minutes` and every occurrence in a different DST phase than the one
* that was sampled drifts by the offset delta, an hour early in one direction and
* an hour late in the other. Rows written by other apps carry no wall-clock at
* all — a conventional `1440` fires at UTC midnight, which is 01:00 or 02:00
* local in Berlin and the wrong day west of UTC.
*
* So the offset is only read for *which day* it means, via [allDayLeadDays], and
* the hour comes from [allDayTimeMinutes] — the one global "show all-day
* reminders at" setting — recomposed against each occurrence's own date in
* [zone]. 09:00 Berlin is then 09:00 Berlin on every occurrence, whatever the
* offset was when the row was written.
*
* [minutesByEvent] may hold duplicate offsets (two identical reminder rows on one
* event); they collapse, because they would otherwise fight over one notification.
@@ -79,16 +101,58 @@ private const val MILLIS_PER_MINUTE = 60_000L
fun planReminders(
instances: List<ReminderEventInstance>,
minutesByEvent: Map<Long, List<Int>>,
zone: ZoneId,
allDayTimeMinutes: Int,
): List<PlannedReminder> = instances.flatMap { instance ->
minutesByEvent[instance.eventId].orEmpty().distinct().map { minutes ->
PlannedReminder(
instance = instance,
minutes = minutes,
alarmMillis = instance.beginMillis - minutes * MILLIS_PER_MINUTE,
alarmMillis = if (instance.isAllDay) {
allDayAlarmMillis(instance.beginMillis, minutes, zone, allDayTimeMinutes)
} else {
instance.beginMillis - minutes * MILLIS_PER_MINUTE
},
)
}
}
/** UTC midnight of an all-day occurrence, as the calendar date it stands for. */
private fun allDayDate(beginMillis: Long): LocalDate =
Instant.ofEpochMilli(beginMillis).atZone(ZoneOffset.UTC).toLocalDate()
/**
* 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.
*
* 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.
*/
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))
}
private fun allDayAlarmMillis(
beginMillis: Long,
rawMinutes: Int,
zone: ZoneId,
allDayTimeMinutes: Int,
): Long = allDayDate(beginMillis)
.minusDays(allDayLeadDays(rawMinutes, beginMillis, zone))
.atTime(LocalTime.of(allDayTimeMinutes / 60, allDayTimeMinutes % 60))
.atZone(zone)
.toInstant()
.toEpochMilli()
/**
* Split [planned] into what is due now and when to wake up next.
*

203
app/src/test/java/de/jeanlucmakiola/calendula/domain/reminders/ReminderPlanTest.kt
View File

@@ -2,6 +2,11 @@ package de.jeanlucmakiola.calendula.domain.reminders
import com.google.common.truth.Truth.assertThat
import org.junit.jupiter.api.Test
import java.time.LocalDate
import java.time.LocalTime
import java.time.ZoneId
import java.time.ZoneOffset
import java.time.ZonedDateTime
/**
* The decision layer of in-house reminder delivery (#75). The watermark rules are
@@ -30,10 +35,18 @@ class ReminderPlanTest {
isAllDay = isAllDay,
)
/** [planReminders] with the fixtures' zone and all-day hour filled in. */
private fun plan(
instances: List<ReminderEventInstance>,
minutesByEvent: Map<Long, List<Int>>,
zone: ZoneId = berlin,
allDayTimeMinutes: Int = nineAm,
) = planReminders(instances, minutesByEvent, zone, allDayTimeMinutes)
@Test
fun `a reminder fires its offset before the occurrence begins`() {
val begin = now + 30 * minute
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L, beginMillis = begin)),
minutesByEvent = mapOf(1L to listOf(10)),
)
@@ -41,35 +54,159 @@ class ReminderPlanTest {
assertThat(planned.map { it.alarmMillis }).containsExactly(begin - 10 * minute)
}
@Test
fun `an all-day offset is measured from the raw begin, not corrected again`() {
// An all-day instance begins at UTC midnight and AllDayReminderEncoding
// already folded the wanted wall-clock time into the stored offset, so a
// second timezone correction here would move the reminder off it.
val utcMidnight = 1_700_006_400_000L
val planned = planReminders(
instances = listOf(instance(1L, beginMillis = utcMidnight, isAllDay = true)),
minutesByEvent = mapOf(1L to listOf(1_020)),
)
// --- all-day: the hour the user picked, on every occurrence -------------
assertThat(planned.single().alarmMillis).isEqualTo(utcMidnight - 1_020 * minute)
private val berlin = ZoneId.of("Europe/Berlin")
private val nineAm = 540
/** UTC midnight of [date] — how the provider stores an all-day occurrence. */
private fun allDayBegin(date: String): Long =
LocalDate.parse(date).atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli()
private fun firedAt(alarmMillis: Long, zone: ZoneId = berlin): ZonedDateTime =
java.time.Instant.ofEpochMilli(alarmMillis).atZone(zone)
/**
* The offset AllDayReminderEncoding would store for "[days] before, at
* [timeOfDayMinutes]" when sampled against [eventDate] — i.e. exactly the row
* the app writes today.
*/
private fun encodedAllDayMinutes(
eventDate: String,
days: Long,
timeOfDayMinutes: Int = nineAm,
zone: ZoneId = berlin,
): Int {
val date = LocalDate.parse(eventDate)
val utcMidnight = date.atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli()
val fire = date.minusDays(days)
.atTime(LocalTime.of(timeOfDayMinutes / 60, timeOfDayMinutes % 60))
.atZone(zone).toInstant().toEpochMilli()
return ((utcMidnight - fire) / minute).toInt()
}
private fun allDayAlarm(eventDate: String, rawMinutes: Int, zone: ZoneId = berlin): Long =
plan(
instances = listOf(
instance(1L, beginMillis = allDayBegin(eventDate), isAllDay = true),
),
minutesByEvent = mapOf(1L to listOf(rawMinutes)),
zone = zone,
allDayTimeMinutes = nineAm,
).single().alarmMillis
@Test
fun `an all-day reminder fires at the hour the setting names`() {
// Winter: Berlin is UTC+1. "1 day before" on the 15th means the 14th, 09:00.
val alarm = allDayAlarm("2026-01-15", encodedAllDayMinutes("2026-01-15", days = 1))
assertThat(firedAt(alarm).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-01-14").atTime(9, 0))
}
@Test
fun `a negative offset fires after the occurrence begins`() {
fun `a summer occurrence of a row written in winter is not an hour early`() {
// The drift this replaces: the offset was sampled at UTC+1, the occurrence
// falls at UTC+2, and firing at `begin - minutes` would land at 08:00.
val winterRow = encodedAllDayMinutes("2026-01-15", days = 1)
val alarm = allDayAlarm("2026-07-15", winterRow)
assertThat(firedAt(alarm).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0))
}
@Test
fun `a winter occurrence of a row written in summer is not an hour late`() {
// The same drift in the other direction: sampled at UTC+2, fires at UTC+1.
val summerRow = encodedAllDayMinutes("2026-07-15", days = 1)
val alarm = allDayAlarm("2026-01-15", summerRow)
assertThat(firedAt(alarm).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-01-14").atTime(9, 0))
}
@Test
fun `every occurrence of a yearly all-day series fires at the same wall clock`() {
// A birthday's offset is sampled once; the series must not walk off it.
val row = encodedAllDayMinutes("2026-07-15", days = 1)
val fired = listOf("2026-07-15", "2027-01-15", "2027-07-15", "2028-01-15")
.map { firedAt(allDayAlarm(it, row)).toLocalTime() }
assertThat(fired.toSet()).containsExactly(LocalTime.of(9, 0))
}
@Test
fun `an all-day reminder on the day itself fires that morning`() {
// "At time of event" on an all-day event encodes to a negative offset.
val begin = now + 30 * minute
val planned = planReminders(
instances = listOf(instance(1L, beginMillis = begin, isAllDay = true)),
minutesByEvent = mapOf(1L to listOf(-420)),
val sameDay = encodedAllDayMinutes("2026-07-15", days = 0)
assertThat(sameDay).isLessThan(0)
val alarm = allDayAlarm("2026-07-15", sameDay)
assertThat(firedAt(alarm).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-07-15").atTime(9, 0))
}
@Test
fun `a plain 1440 row from another calendar app means one day before`() {
// Foreign rows carry no encoded hour. Read at face value they would fire
// at UTC midnight — 02:00 local in summer Berlin.
val alarm = allDayAlarm("2026-07-15", rawMinutes = 1_440)
assertThat(firedAt(alarm).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0))
}
@Test
fun `a plain 1440 row west of UTC still means one day before`() {
// UTC midnight of the 15th is the evening of the 14th in New York, so a
// local-date reading of the offset would put this two days out.
val newYork = ZoneId.of("America/New_York")
val alarm = allDayAlarm("2026-07-15", rawMinutes = 1_440, zone = newYork)
assertThat(firedAt(alarm, newYork).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0))
}
@Test
fun `an encoded row west of UTC fires at the named hour`() {
val newYork = ZoneId.of("America/New_York")
val row = encodedAllDayMinutes("2026-07-15", days = 1, zone = newYork)
val alarm = allDayAlarm("2026-07-15", row, zone = newYork)
assertThat(firedAt(alarm, newYork).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-07-14").atTime(9, 0))
}
@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)
}
@Test
fun `a timed reminder is exact across a DST boundary`() {
// Nothing to re-anchor: begin is an absolute instant either way.
val begin = LocalDate.parse("2026-03-29").atTime(14, 0)
.atZone(berlin).toInstant().toEpochMilli()
val planned = plan(
instances = listOf(instance(1L, beginMillis = begin, isAllDay = false)),
minutesByEvent = mapOf(1L to listOf(30)),
zone = berlin,
allDayTimeMinutes = nineAm,
)
assertThat(planned.single().alarmMillis).isEqualTo(begin + 420 * minute)
assertThat(firedAt(planned.single().alarmMillis).toLocalDateTime())
.isEqualTo(LocalDate.parse("2026-03-29").atTime(13, 30))
}
@Test
fun `every occurrence of a series gets its own reminder`() {
val planned = planReminders(
val planned = plan(
instances = listOf(
instance(1L, beginMillis = now + day),
instance(1L, beginMillis = now + 2 * day),
@@ -84,7 +221,7 @@ class ReminderPlanTest {
@Test
fun `duplicate reminder rows collapse to one`() {
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L)),
minutesByEvent = mapOf(1L to listOf(10, 10)),
)
@@ -94,7 +231,7 @@ class ReminderPlanTest {
@Test
fun `an event with no reminders plans nothing`() {
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L)),
minutesByEvent = emptyMap(),
)
@@ -105,7 +242,7 @@ class ReminderPlanTest {
@Test
fun `the same reminder keeps its key across scans`() {
val plan = {
planReminders(listOf(instance(1L)), mapOf(1L to listOf(10))).single().key
plan(listOf(instance(1L)), mapOf(1L to listOf(10))).single().key
}
assertThat(plan()).isEqualTo(plan())
@@ -113,7 +250,7 @@ class ReminderPlanTest {
@Test
fun `occurrences of one series get different keys`() {
val planned = planReminders(
val planned = plan(
instances = listOf(
instance(1L, beginMillis = now + day),
instance(1L, beginMillis = now + 2 * day),
@@ -126,7 +263,7 @@ class ReminderPlanTest {
@Test
fun `two reminders on one occurrence get different keys`() {
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L)),
minutesByEvent = mapOf(1L to listOf(10, 30)),
)
@@ -136,7 +273,7 @@ class ReminderPlanTest {
@Test
fun `a reminder whose moment has passed since the last scan is due`() {
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L, beginMillis = now + 5 * minute)),
minutesByEvent = mapOf(1L to listOf(10)),
)
@@ -153,7 +290,7 @@ class ReminderPlanTest {
fun `a reminder already covered by the watermark does not fire twice`() {
// The scan runs again (a provider change, a reboot) after the alarm that
// already posted this one. Nothing records "fired" but the watermark.
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L, beginMillis = now + 5 * minute)),
minutesByEvent = mapOf(1L to listOf(10)),
)
@@ -169,7 +306,7 @@ class ReminderPlanTest {
@Test
fun `a reminder exactly on the watermark does not fire again`() {
val begin = now + 5 * minute
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L, beginMillis = begin)),
minutesByEvent = mapOf(1L to listOf(10)),
)
@@ -186,7 +323,7 @@ class ReminderPlanTest {
fun `a missed alarm is caught up by a much later scan`() {
// The device was off over the reminder; the scan on boot must still post it
// while the event is ahead. This is what the provider path could never do.
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L, beginMillis = now + 5 * minute)),
minutesByEvent = mapOf(1L to listOf(60)),
)
@@ -201,7 +338,7 @@ class ReminderPlanTest {
@Test
fun `a reminder for an occurrence that already ended is dropped`() {
val planned = planReminders(
val planned = plan(
instances = listOf(
instance(1L, beginMillis = now - 3 * 60 * minute, endMillis = now - 2 * 60 * minute),
),
@@ -218,7 +355,7 @@ class ReminderPlanTest {
@Test
fun `an occurrence with no end falls back to its begin for relevance`() {
val planned = planReminders(
val planned = plan(
instances = listOf(
instance(1L, beginMillis = now - minute, endMillis = 0L),
),
@@ -234,7 +371,7 @@ class ReminderPlanTest {
@Test
fun `due reminders come out in occurrence order`() {
val planned = planReminders(
val planned = plan(
instances = listOf(
instance(1L, beginMillis = now + 20 * minute),
instance(2L, beginMillis = now + 5 * minute),
@@ -251,7 +388,7 @@ class ReminderPlanTest {
@Test
fun `the next wake-up is the earliest reminder still ahead`() {
val planned = planReminders(
val planned = plan(
instances = listOf(
instance(1L, beginMillis = now + 20 * minute),
instance(2L, beginMillis = now + 90 * minute),
@@ -280,7 +417,7 @@ class ReminderPlanTest {
fun `a reminder beyond the horizon waits for the next scan`() {
// Capping keeps the rolling window honest: the far-off reminder is picked
// up by a later scan rather than pinned to an alarm we may never re-check.
val planned = planReminders(
val planned = plan(
instances = listOf(instance(1L, beginMillis = now + 30 * day)),
minutesByEvent = mapOf(1L to listOf(5)),
)