Skip to content

Add support for time zones when calculating alarm trigger times#348

Merged
cketti merged 14 commits into
mainfrom
224-alarm-trigger-calculation
May 11, 2026
Merged

Add support for time zones when calculating alarm trigger times#348
cketti merged 14 commits into
mainfrom
224-alarm-trigger-calculation

Conversation

@cketti
Copy link
Copy Markdown
Contributor

@cketti cketti commented May 8, 2026

Unfortunately, this is a rather large pull request. However, the bulk of it is just moving things around, (hopefully) without changing the behavior. These commits are prefixed with [Refactor].

The functional changes are:

  • Extracted the code to calculate the end date using start + duration to Task.end (for calendar events VEvent.getEndDate() was already doing all the work).
  • Changed AlarmTriggerCalculator.alarmTriggerToMinutes() (previously ICalendar.vAlarmToMin()) to support time zones when calculating alarm trigger times
  • A start time (or end time if the alarm is relative to the end) is now required to return the "alarm minutes". This simplifies the code, but required some changes to tests. (It's also in line with what RFC5545 requires.)

@cketti cketti linked an issue May 8, 2026 that may be closed by this pull request
@cketti cketti marked this pull request as ready for review May 8, 2026 17:59
@cketti cketti requested a review from rfc2822 May 8, 2026 17:59
@rfc2822 rfc2822 requested a review from Copilot May 11, 2026 08:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds time zone–aware alarm trigger minute calculations and centralizes end-time derivation for tasks, aligning alarm handling more closely with RFC5545 requirements and improving DST correctness.

Changes:

  • Introduces AlarmTriggerCalculator.alarmTriggerToMinutes() to compute alarm offsets with time zone/DST-aware behavior (including for Period triggers).
  • Adds Task.end to derive an effective task end (DUE or DTSTART + DURATION) and updates task alarm mapping to use it.
  • Updates calendar/task builders and tests to pass required reference start/end times and validates behavior with new/adjusted test coverage.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/src/main/kotlin/at/bitfire/synctools/util/AlarmTriggerCalculator.kt New shared implementation for alarm trigger offset calculation with time zone/DST support.
lib/src/main/kotlin/at/bitfire/ical4android/Task.kt Adds Task.end helper for deriving an effective end date from due/start+duration.
lib/src/main/kotlin/at/bitfire/synctools/mapping/tasks/DmfsTaskBuilder.kt Switches task alarm minute calculation to AlarmTriggerCalculator and uses task.end.
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/RemindersBuilder.kt Switches event reminder minute calculation to AlarmTriggerCalculator and derives end via getEndDate(true).
lib/src/main/kotlin/at/bitfire/ical4android/ICalendar.kt Removes the old vAlarmToMin helper now replaced by AlarmTriggerCalculator.
lib/src/test/kotlin/at/bitfire/synctools/util/AlarmTriggerCalculatorTest.kt New focused unit tests for alarm trigger minute calculation, including DST/time zone scenarios.
lib/src/test/kotlin/at/bitfire/synctools/mapping/calendar/builder/RemindersBuilderTest.kt Updates tests to include required DTSTART references and validate reminder mapping behavior.
lib/src/test/kotlin/at/bitfire/ical4android/TaskTest.kt Adds test coverage for Task.end across temporal types and DST boundaries.
lib/src/test/kotlin/at/bitfire/ical4android/ICalendarTest.kt Removes vAlarmToMin-specific tests that moved to AlarmTriggerCalculatorTest.

Comment thread lib/src/test/kotlin/at/bitfire/synctools/util/AlarmTriggerCalculatorTest.kt Outdated
…culation

# Conflicts:
#	lib/src/main/kotlin/at/bitfire/synctools/mapping/tasks/DmfsTaskBuilder.kt
@cketti cketti merged commit 0054b9f into main May 11, 2026
8 checks passed
@cketti cketti deleted the 224-alarm-trigger-calculation branch May 11, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ical4j 4.x] Start handling trigger period + test

3 participants