From 9f1265760b1e2fafb6b4ad4b807347f028225617 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Tue, 20 Oct 2026 15:45:35 -0500 Subject: [PATCH 1/4] fix: expand recurring series in the zone their invites are published in Series slots were stepped in naive UTC while the .ics anchors the RRULE in the coach's zone, so after a DST change every occurrence was stored an hour off the calendar and per-occurrence overrides and cancels named instances no client held. - Expand on local wall time in the series' zone and resolve each slot to UTC per RFC 5545 (skipped time uses the pre-gap offset, repeated time its first pass). - Validate weekdays and month days locally, so evening series no longer 422. - Reject monthly series starting after day 28 and starts in the second pass of a repeated hour. - Persist the published zone on SeriesRule (backfilled from the coach) and use it for every invite under the series UID. - Emit WKST=MO. - Check slots against the rrule crate's expansion of the emitted invite. --- Cargo.lock | 1 + domain/Cargo.toml | 3 + domain/src/coaching_session.rs | 3 +- domain/src/coaching_session/recurrence.rs | 351 +++++++++--------- .../coaching_session/recurrence_dst_tests.rs | 302 +++++++++++++++ domain/src/coaching_session_series.rs | 75 +++- domain/src/emails.rs | 66 +++- domain/src/emails_tests.rs | 274 +++++++++++++- domain/src/gateway/ical.rs | 15 + domain/src/gateway/ical_oracle_tests.rs | 204 ++++++++++ domain/src/gateway/ical_tests.rs | 2 +- migration/src/lib.rs | 2 + ...021_000000_add_timezone_to_series_rules.rs | 46 +++ 13 files changed, 1151 insertions(+), 193 deletions(-) create mode 100644 domain/src/coaching_session/recurrence_dst_tests.rs create mode 100644 domain/src/gateway/ical_oracle_tests.rs create mode 100644 migration/src/m20261021_000000_add_timezone_to_series_rules.rs diff --git a/Cargo.lock b/Cargo.lock index 225f9993f..71c0b1f85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1264,6 +1264,7 @@ dependencies = [ "mockito", "rand 0.8.6", "reqwest", + "rrule", "rusty-s3", "sea-orm", "secrecy", diff --git a/domain/Cargo.toml b/domain/Cargo.toml index 88036d8ab..8376652e4 100644 --- a/domain/Cargo.toml +++ b/domain/Cargo.toml @@ -43,6 +43,9 @@ features = ["debug-print", "runtime-tokio-native-tls", "sqlx-postgres"] [dev-dependencies] mockito = "1.6" +# RFC 5545 expander used as an oracle: series slots must equal what a client expands +# from the exact DTSTART;TZID + RRULE the invite carries. +rrule = "0.14" # In-memory SQLite: executes the purge's real queries in-process, which the mock cannot. sea-orm = { version = "2.0.3", features = ["sqlx-sqlite", "schema-sync"] } # `test-util` (not part of `full`) provides the paused clock the job-scheduler tests diff --git a/domain/src/coaching_session.rs b/domain/src/coaching_session.rs index 128501f19..70df98ec4 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -187,8 +187,9 @@ pub async fn bulk_create_recurring( pub(crate) fn series_slots( start_at: NaiveDateTime, recurrence: &Recurrence, + zone: chrono_tz::Tz, ) -> Result, Error> { - expand_recurrence(start_at, recurrence)? + expand_recurrence(start_at, recurrence, zone)? .into_iter() .map(|d| SessionDate::new(d).map(SessionDate::into_inner)) .collect() diff --git a/domain/src/coaching_session/recurrence.rs b/domain/src/coaching_session/recurrence.rs index a2008d1f7..794d9a8a3 100644 --- a/domain/src/coaching_session/recurrence.rs +++ b/domain/src/coaching_session/recurrence.rs @@ -11,11 +11,12 @@ //! - [`MAX_RECURRING_SPAN_DAYS`] caps the calendar span from the first to the last //! event (covers leap-year monthly). //! -//! Times are operated on as [`NaiveDateTime`]; recurrence expansion in naive time -//! can drift up to an hour across DST transitions, which matches how -//! `coaching_sessions.date` is stored today. +//! Expansion runs on local wall time in the series' zone and resolves each slot to UTC, +//! the way RFC 5545 clients expand the `DTSTART;TZID` + `RRULE` the invite carries, so +//! the stored sessions and the calendar's instances stay the same instants across DST. -use chrono::{Datelike, Duration, Months, NaiveDateTime, Weekday}; +use chrono::{Datelike, Duration, Months, NaiveDateTime, Offset, TimeZone, Weekday}; +use chrono_tz::Tz; use serde::{Deserialize, Serialize}; use utoipa::ToSchema; @@ -27,6 +28,13 @@ const MAX_RECURRING_OCCURRENCES: usize = 365; /// Maximum calendar span (in days) from the first occurrence to the last. const MAX_RECURRING_SPAN_DAYS: i64 = 366; +/// Latest day of the month a monthly series may start on. Clients skip months without +/// the start day while some clamp to the month's end, so later days cannot render alike. +const MAX_MONTHLY_START_DAY: u32 = 28; + +/// How far back to look for the offset in effect before a spring-forward gap. +const GAP_LOOKBACK_HOURS: i64 = 3; + /// How often the rule repeats. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "lowercase")] @@ -122,6 +130,8 @@ pub enum RecurrenceError { ByWeekdaysOnNonWeekly, EmptyByWeekdays, StartWeekdayMismatch, + MonthlyStartDayTooLate, + AmbiguousStartAt, TooManyOccurrences { count: usize }, SpanTooLong { days: i64 }, NoOccurrencesGenerated, @@ -153,6 +163,13 @@ impl RecurrenceError { Self::StartWeekdayMismatch => { "`start_at`'s weekday must be included in `by_weekdays`".into() } + Self::MonthlyStartDayTooLate => format!( + "Monthly series must start on day {MAX_MONTHLY_START_DAY} or earlier in the coach's timezone." + ), + Self::AmbiguousStartAt => { + "`start_at` is the repeated hour of a daylight saving change; choose another time" + .into() + } Self::TooManyOccurrences { count } => format!( "recurrence would produce {count} occurrences (max {MAX_RECURRING_OCCURRENCES})" ), @@ -167,10 +184,13 @@ impl RecurrenceError { } } -/// Validate the structural correctness of a [`Recurrence`] rule against the -/// given `start_at`. Does not enforce the occurrence/span caps — those are -/// applied in [`expand_recurrence`] after generation. -fn validate_recurrence(start_at: NaiveDateTime, rule: &Recurrence) -> Result<(), RecurrenceError> { +/// Validate the structural correctness of a [`Recurrence`] rule against the series' +/// local start. Does not enforce the occurrence/span caps — those are applied in +/// [`expand_recurrence`] after generation. +fn validate_recurrence( + local_start: NaiveDateTime, + rule: &Recurrence, +) -> Result<(), RecurrenceError> { if rule.interval < 1 { return Err(RecurrenceError::InvalidInterval); } @@ -185,201 +205,196 @@ fn validate_recurrence(start_at: NaiveDateTime, rule: &Recurrence) -> Result<(), if weekdays.is_empty() { return Err(RecurrenceError::EmptyByWeekdays); } - if !weekdays.contains(&start_at.weekday()) { + if !weekdays.contains(&local_start.weekday()) { return Err(RecurrenceError::StartWeekdayMismatch); } } + if rule.frequency == Frequency::Monthly && local_start.day() > MAX_MONTHLY_START_DAY { + return Err(RecurrenceError::MonthlyStartDayTooLate); + } Ok(()) } -/// Expand the rule into concrete occurrence datetimes, in chronological order. +/// The wall time `utc` shows in `zone`. +fn to_local(zone: Tz, utc: NaiveDateTime) -> NaiveDateTime { + zone.from_utc_datetime(&utc).naive_local() +} + +/// The UTC instant of a local wall time, resolved as RFC 5545 §3.3.5 does: a repeated +/// time is its first occurrence, and a skipped time uses the offset before the gap. +fn to_utc(zone: Tz, local: NaiveDateTime) -> NaiveDateTime { + zone.from_local_datetime(&local) + .earliest() + .map(|resolved| resolved.naive_utc()) + .unwrap_or_else(|| local - offset_before_gap(zone, local)) +} + +/// The UTC offset in effect just before the gap that swallowed `local`. +fn offset_before_gap(zone: Tz, local: NaiveDateTime) -> Duration { + let before = local - Duration::hours(GAP_LOOKBACK_HOURS); + let seconds = zone + .from_local_datetime(&before) + .earliest() + .map_or(0, |resolved| resolved.offset().fix().local_minus_utc()); + Duration::seconds(seconds.into()) +} + +/// Expand the rule into concrete occurrence datetimes (UTC), in chronological order. /// -/// Validates the rule, generates events according to the frequency, then enforces -/// [`MAX_RECURRING_OCCURRENCES`] and [`MAX_RECURRING_SPAN_DAYS`]. The first -/// occurrence is always equal to `start_at`. +/// Steps run on local wall time in `zone`, so every slot keeps the start's local time of +/// day across DST, and weekdays and month days are the ones the coach sees. `until` is +/// compared against each resolved instant, as a client reads `UNTIL`. Validates the rule, +/// generates events according to the frequency, then enforces +/// [`MAX_RECURRING_OCCURRENCES`] and [`MAX_RECURRING_SPAN_DAYS`]. The first occurrence +/// is always equal to `start_at`. pub fn expand_recurrence( start_at: NaiveDateTime, rule: &Recurrence, + zone: Tz, ) -> Result, RecurrenceError> { - validate_recurrence(start_at, rule)?; - - let target_count = rule.count.map(|c| c as usize); - let until = rule.until; - - // Generation cap: one past the limit so the outer check below sees overflow. - let gen_cap = MAX_RECURRING_OCCURRENCES + 1; - - let week_interval = match rule.frequency { - Frequency::Biweekly => 2 * rule.interval, - _ => rule.interval, + let local_start = to_local(zone, start_at); + validate_recurrence(local_start, rule)?; + // The second pass through a repeated hour: the invite's `DTSTART;TZID` would name the first. + if to_utc(zone, local_start) != start_at { + return Err(RecurrenceError::AmbiguousStartAt); + } + + let bounds = Bounds { + resolve: |local| to_utc(zone, local), + until: rule.until, + // One past the limit so the outer check below sees overflow. + limit: rule + .count + .map_or(MAX_RECURRING_OCCURRENCES + 1, |c| c as usize) + .min(MAX_RECURRING_OCCURRENCES + 1), }; let occurrences = match rule.frequency { - Frequency::Daily => generate_stepped( - start_at, - Duration::days(rule.interval as i64), - target_count, - until, - gen_cap, - ), + Frequency::Daily => { + generate_stepped(local_start, Duration::days(rule.interval as i64), &bounds) + } Frequency::Weekly | Frequency::Biweekly => match &rule.by_weekdays { Some(weekdays) => generate_weekly_with_weekdays( - start_at, - week_interval, + local_start, + rule.effective_interval(), weekdays, - target_count, - until, - gen_cap, + &bounds, ), None => generate_stepped( - start_at, - Duration::weeks(week_interval as i64), - target_count, - until, - gen_cap, + local_start, + Duration::weeks(rule.effective_interval() as i64), + &bounds, ), }, - Frequency::Monthly => { - generate_monthly(start_at, rule.interval, target_count, until, gen_cap) - } + Frequency::Monthly => generate_monthly(local_start, rule.interval, &bounds), }; - if occurrences.is_empty() { + let (Some(first), Some(last)) = (occurrences.first(), occurrences.last()) else { return Err(RecurrenceError::NoOccurrencesGenerated); - } + }; if occurrences.len() > MAX_RECURRING_OCCURRENCES { return Err(RecurrenceError::TooManyOccurrences { count: occurrences.len(), }); } - let span = occurrences - .last() - .unwrap() - .signed_duration_since(*occurrences.first().unwrap()); - if span.num_days() > MAX_RECURRING_SPAN_DAYS { - return Err(RecurrenceError::SpanTooLong { - days: span.num_days(), - }); + let span_days = last.signed_duration_since(*first).num_days(); + if span_days > MAX_RECURRING_SPAN_DAYS { + return Err(RecurrenceError::SpanTooLong { days: span_days }); } Ok(occurrences) } +/// Shared limits for the generators: how a local slot becomes an instant, the `until` +/// instant it must not pass, and how many slots to emit at most. +struct Bounds NaiveDateTime> { + resolve: F, + until: Option, + limit: usize, +} + +impl NaiveDateTime> Bounds { + /// The slot's instant, or `None` once it is past `until`. + fn admit(&self, local: NaiveDateTime) -> Option { + let instant = (self.resolve)(local); + self.until + .is_none_or(|end| instant <= end) + .then_some(instant) + } +} + /// Generic stepped generator for `daily` and `weekly`-without-`by_weekdays`. -fn generate_stepped( - start_at: NaiveDateTime, +fn generate_stepped NaiveDateTime>( + local_start: NaiveDateTime, step: Duration, - target_count: Option, - until: Option, - gen_cap: usize, + bounds: &Bounds, ) -> Vec { - let mut out = Vec::new(); - let mut current = start_at; - let limit = target_count.unwrap_or(gen_cap).min(gen_cap); - while out.len() < limit { - if let Some(end) = until { - if current > end { - break; - } - } - out.push(current); - current = match current.checked_add_signed(step) { - Some(next) => next, - None => break, - }; - } - out + std::iter::successors(Some(local_start), |local| local.checked_add_signed(step)) + .map_while(|local| bounds.admit(local)) + .take(bounds.limit) + .collect() } /// Weekly recurrence with explicit weekdays. Walks `week_interval`-aligned weeks -/// starting from the Monday of `start_at`'s week and emits each listed weekday -/// (skipping any occurrence that falls before `start_at` itself). -fn generate_weekly_with_weekdays( - start_at: NaiveDateTime, +/// starting from the Monday of the start's week (RFC 5545 `WKST=MO`) and emits each +/// listed weekday, skipping any occurrence that falls before the start itself. +fn generate_weekly_with_weekdays NaiveDateTime>( + local_start: NaiveDateTime, week_interval: u32, weekdays: &[Weekday], - target_count: Option, - until: Option, - gen_cap: usize, + bounds: &Bounds, ) -> Vec { let mut sorted: Vec = weekdays.to_vec(); sorted.sort_by_key(|w| w.num_days_from_monday()); sorted.dedup(); - let time_of_day = start_at.time(); + let time_of_day = local_start.time(); let monday_of_start_week = - start_at.date() - Duration::days(start_at.weekday().num_days_from_monday() as i64); - - let mut out = Vec::new(); - let limit = target_count.unwrap_or(gen_cap).min(gen_cap); - let mut week_offset: i64 = 0; - // Safety cap on outer loop iterations to bound worst-case work even when - // `until` is far in the future and weekday filters mean few hits per week. - let max_weeks = (gen_cap as i64) + 8; - while out.len() < limit && week_offset < max_weeks * (week_interval as i64) { - let week_monday = - match monday_of_start_week.checked_add_signed(Duration::days(week_offset * 7)) { - Some(d) => d, - None => break, - }; - for wd in &sorted { - let date = match week_monday - .checked_add_signed(Duration::days(wd.num_days_from_monday() as i64)) - { - Some(d) => d, - None => return out, - }; - let dt = date.and_time(time_of_day); - if dt < start_at { - continue; - } - if let Some(end) = until { - if dt > end { - return out; - } - } - out.push(dt); - if out.len() >= limit { - return out; - } - } - week_offset += week_interval as i64; - } - out + local_start.date() - Duration::days(local_start.weekday().num_days_from_monday() as i64); + // Bounds worst-case work when `until` is far out and the weekday filter hits rarely. + let max_weeks = (bounds.limit as i64) + 8; + + (0..max_weeks) + .map_while(|week| { + monday_of_start_week.checked_add_signed(Duration::weeks(week * week_interval as i64)) + }) + .flat_map(|monday| { + sorted.iter().filter_map(move |wd| { + monday.checked_add_signed(Duration::days(wd.num_days_from_monday() as i64)) + }) + }) + .map(|date| date.and_time(time_of_day)) + .filter(|local| *local >= local_start) + .map_while(|local| bounds.admit(local)) + .take(bounds.limit) + .collect() } -/// Monthly recurrence. Day-of-month is taken from `start_at`; chrono's -/// `checked_add_months` automatically clamps to the last day of the target -/// month when the day doesn't exist (e.g. Jan 31 + 1 month → Feb 28). -fn generate_monthly( - start_at: NaiveDateTime, +/// Monthly recurrence on the start's day of the month. Validation keeps that day at or +/// below [`MAX_MONTHLY_START_DAY`], so `checked_add_months` never has to clamp. +fn generate_monthly NaiveDateTime>( + local_start: NaiveDateTime, interval: u32, - target_count: Option, - until: Option, - gen_cap: usize, + bounds: &Bounds, ) -> Vec { - let mut out = Vec::new(); - let limit = target_count.unwrap_or(gen_cap).min(gen_cap); - let mut i: u32 = 0; - while out.len() < limit { - let candidate = match start_at.checked_add_months(Months::new(i * interval)) { - Some(dt) => dt, - None => break, - }; - if let Some(end) = until { - if candidate > end { - break; - } - } - out.push(candidate); - i += 1; - } - out + (0u32..) + .map_while(|i| { + i.checked_mul(interval) + .and_then(|months| local_start.checked_add_months(Months::new(months))) + }) + .map_while(|local| bounds.admit(local)) + .take(bounds.limit) + .collect() } +#[cfg(test)] +#[path = "recurrence_dst_tests.rs"] +mod dst_tests; + #[cfg(test)] mod tests { use super::*; use chrono::{NaiveDate, NaiveTime}; + use chrono_tz::UTC; fn dt(y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { NaiveDate::from_ymd_opt(y, m, d) @@ -574,7 +589,7 @@ mod tests { count: Some(4), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -596,7 +611,7 @@ mod tests { count: Some(3), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -617,7 +632,7 @@ mod tests { count: None, until: Some(dt(2026, 6, 3, 9, 0)), }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!(occ.len(), 3); assert_eq!(occ.last().unwrap(), &dt(2026, 6, 3, 9, 0)); } @@ -634,7 +649,7 @@ mod tests { count: Some(3), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -655,7 +670,7 @@ mod tests { count: Some(3), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -679,7 +694,7 @@ mod tests { count: Some(6), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -704,7 +719,7 @@ mod tests { count: Some(5), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -728,7 +743,7 @@ mod tests { count: Some(4), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -752,7 +767,7 @@ mod tests { count: Some(3), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + let occ = expand_recurrence(start, &r, UTC).unwrap(); assert_eq!( occ, vec![ @@ -763,10 +778,9 @@ mod tests { ); } + /// Clients disagree on months without the start day, so such a series cannot exist. #[test] - fn expand_monthly_clamps_day_for_short_months() { - // Jan 31 → Feb 28 (2026 is not a leap year) → Mar 31 → Apr 30. - let start = dt(2026, 1, 31, 10, 0); + fn expand_monthly_rejects_start_days_after_28() { let r = Recurrence { frequency: Frequency::Monthly, interval: 1, @@ -774,14 +788,17 @@ mod tests { count: Some(4), until: None, }; - let occ = expand_recurrence(start, &r).unwrap(); + [29, 30, 31].into_iter().for_each(|day| { + let err = expand_recurrence(dt(2026, 1, day, 10, 0), &r, UTC).unwrap_err(); + assert_eq!(err, RecurrenceError::MonthlyStartDayTooLate, "day {day}"); + }); assert_eq!( - occ, + expand_recurrence(dt(2026, 1, 28, 10, 0), &r, UTC).unwrap(), vec![ - dt(2026, 1, 31, 10, 0), + dt(2026, 1, 28, 10, 0), dt(2026, 2, 28, 10, 0), - dt(2026, 3, 31, 10, 0), - dt(2026, 4, 30, 10, 0), + dt(2026, 3, 28, 10, 0), + dt(2026, 4, 28, 10, 0), ] ); } @@ -798,7 +815,7 @@ mod tests { count: Some((MAX_RECURRING_OCCURRENCES as u32) + 5), until: None, }; - let err = expand_recurrence(start, &r).unwrap_err(); + let err = expand_recurrence(start, &r, UTC).unwrap_err(); assert!(matches!(err, RecurrenceError::TooManyOccurrences { .. })); } @@ -813,7 +830,7 @@ mod tests { count: Some(60), until: None, }; - let err = expand_recurrence(start, &r).unwrap_err(); + let err = expand_recurrence(start, &r, UTC).unwrap_err(); assert!(matches!(err, RecurrenceError::SpanTooLong { .. })); } @@ -830,7 +847,7 @@ mod tests { count: None, until: Some(dt(2027, 3, 1, 10, 0)), }; - let err = expand_recurrence(start, &r).unwrap_err(); + let err = expand_recurrence(start, &r, UTC).unwrap_err(); assert!(matches!(err, RecurrenceError::SpanTooLong { .. })); } @@ -844,7 +861,7 @@ mod tests { count: None, until: Some(dt(2026, 5, 1, 10, 0)), }; - let err = expand_recurrence(start, &r).unwrap_err(); + let err = expand_recurrence(start, &r, UTC).unwrap_err(); assert_eq!(err, RecurrenceError::NoOccurrencesGenerated); } } diff --git a/domain/src/coaching_session/recurrence_dst_tests.rs b/domain/src/coaching_session/recurrence_dst_tests.rs new file mode 100644 index 000000000..8c379019f --- /dev/null +++ b/domain/src/coaching_session/recurrence_dst_tests.rs @@ -0,0 +1,302 @@ +//! Expansion in the series' zone: wall time held across DST, weekdays and month days +//! read locally, and the RFC 5545 rules for skipped and repeated local times. + +use super::*; +use chrono::{Days, NaiveDate}; +use chrono_tz::America::Chicago; +use chrono_tz::Asia::{Kolkata, Tokyo}; +use chrono_tz::Australia::Sydney; +use chrono_tz::Europe::London; +use chrono_tz::UTC; + +fn wall(y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { + NaiveDate::from_ymd_opt(y, m, d) + .unwrap() + .and_hms_opt(h, min, 0) + .unwrap() +} + +/// The instant of an unambiguous local wall time. +fn at(zone: Tz, y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { + zone.from_local_datetime(&wall(y, m, d, h, min)) + .single() + .unwrap() + .naive_utc() +} + +fn utc(y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { + wall(y, m, d, h, min) +} + +fn counted(frequency: Frequency, count: u32) -> Recurrence { + Recurrence { + frequency, + interval: 1, + by_weekdays: None, + count: Some(count), + until: None, + } +} + +fn on(weekdays: &[Weekday], frequency: Frequency, count: u32) -> Recurrence { + Recurrence { + by_weekdays: Some(weekdays.to_vec()), + ..counted(frequency, count) + } +} + +/// The `k`th local slot of a rule with no weekday filter. +fn nth_local(start: NaiveDateTime, frequency: Frequency, k: u32) -> NaiveDateTime { + let date = match frequency { + Frequency::Daily => start.date() + Days::new(k.into()), + Frequency::Weekly => start.date() + Days::new(7 * u64::from(k)), + Frequency::Biweekly => start.date() + Days::new(14 * u64::from(k)), + Frequency::Monthly => start.date().checked_add_months(Months::new(k)).unwrap(), + }; + date.and_time(start.time()) +} + +/// Every frequency, started shortly before a transition, keeps its local time of day and +/// local date step on both sides of it, in zones north and south of the equator. +#[test] +fn every_frequency_keeps_wall_time_across_both_transitions() { + let starts = [ + (Chicago, wall(2026, 10, 20, 10, 0)), + (Chicago, wall(2027, 3, 2, 10, 0)), + (London, wall(2026, 10, 13, 10, 0)), + (London, wall(2027, 3, 16, 10, 0)), + (Sydney, wall(2027, 3, 23, 10, 0)), + (Sydney, wall(2027, 9, 21, 10, 0)), + (Kolkata, wall(2026, 10, 20, 10, 0)), + (UTC, wall(2026, 10, 20, 10, 0)), + ]; + let cadences = [ + (Frequency::Daily, 30), + (Frequency::Weekly, 6), + (Frequency::Biweekly, 4), + (Frequency::Monthly, 3), + ]; + + for (zone, local_start) in starts { + for (frequency, count) in cadences { + let start = zone.from_local_datetime(&local_start).unwrap().naive_utc(); + let slots = expand_recurrence(start, &counted(frequency, count), zone).unwrap(); + + let expected: Vec = (0..count) + .map(|k| to_utc(zone, nth_local(local_start, frequency, k))) + .collect(); + assert_eq!(slots, expected, "{zone} {frequency:?} from {local_start}"); + assert!( + slots + .iter() + .all(|slot| to_local(zone, *slot).time() == local_start.time()), + "{zone} {frequency:?} drifted off {local_start}" + ); + } + } +} + +/// The production series behind the report: weekly Thursdays at 11:00 Chicago through +/// the year. After Nov 1 the slot is 17:00Z, where the calendar holds it, not 16:00Z. +#[test] +fn chicago_weekly_series_lands_where_the_calendar_holds_it() { + let r = Recurrence { + until: Some(utc(2027, 1, 1, 5, 59)), + count: None, + ..on(&[Weekday::Thu], Frequency::Weekly, 1) + }; + let slots = expand_recurrence(utc(2026, 9, 10, 16, 0), &r, Chicago).unwrap(); + + assert_eq!(slots.len(), 17); + assert_eq!(slots[7], utc(2026, 10, 29, 16, 0)); + assert_eq!(slots[8], utc(2026, 11, 5, 17, 0)); + assert!(slots.iter().all(|slot| { + let local = to_local(Chicago, *slot); + local.weekday() == Weekday::Thu && local.time() == wall(2026, 1, 1, 11, 0).time() + })); +} + +/// Thursday 7 PM Chicago is Friday in UTC. The weekday is the coach's, so this is +/// accepted and stays on Thursday evenings. +#[test] +fn evening_weekday_is_read_in_the_series_zone() { + let start = at(Chicago, 2026, 10, 22, 19, 0); + assert_eq!(start.weekday(), Weekday::Fri); + + let slots = + expand_recurrence(start, &on(&[Weekday::Thu], Frequency::Weekly, 4), Chicago).unwrap(); + assert_eq!( + slots, + vec![ + at(Chicago, 2026, 10, 22, 19, 0), + at(Chicago, 2026, 10, 29, 19, 0), + at(Chicago, 2026, 11, 5, 19, 0), + at(Chicago, 2026, 11, 12, 19, 0), + ] + ); +} + +/// Picking the UTC weekday as well used to slip the other day back by one locally. +#[test] +fn evening_weekdays_stay_on_the_chosen_local_days() { + let start = at(Chicago, 2026, 10, 22, 19, 0); + let rule = on(&[Weekday::Thu, Weekday::Fri], Frequency::Weekly, 4); + + let slots = expand_recurrence(start, &rule, Chicago).unwrap(); + assert_eq!( + slots, + vec![ + at(Chicago, 2026, 10, 22, 19, 0), + at(Chicago, 2026, 10, 23, 19, 0), + at(Chicago, 2026, 10, 29, 19, 0), + at(Chicago, 2026, 10, 30, 19, 0), + ] + ); +} + +/// Monday 08:00 in Tokyo is Sunday in UTC. +#[test] +fn morning_weekday_ahead_of_utc_is_read_in_the_series_zone() { + let start = at(Tokyo, 2026, 10, 19, 8, 0); + assert_eq!(start.weekday(), Weekday::Sun); + + let slots = + expand_recurrence(start, &on(&[Weekday::Mon], Frequency::Weekly, 3), Tokyo).unwrap(); + assert!(slots + .iter() + .all(|slot| to_local(Tokyo, *slot).weekday() == Weekday::Mon)); +} + +/// The start weekday is still required, in local terms. +#[test] +fn start_weekday_mismatch_is_judged_locally() { + let start = at(Chicago, 2026, 10, 22, 19, 0); + let err = + expand_recurrence(start, &on(&[Weekday::Fri], Frequency::Weekly, 4), Chicago).unwrap_err(); + assert_eq!(err, RecurrenceError::StartWeekdayMismatch); +} + +/// Weeks are Monday-aligned, so a biweekly Sunday belongs to the week before it. +#[test] +fn biweekly_with_sunday_aligns_weeks_on_monday() { + let start = utc(2027, 1, 4, 10, 0); + let rule = on(&[Weekday::Mon, Weekday::Sun], Frequency::Biweekly, 4); + + assert_eq!( + expand_recurrence(start, &rule, UTC).unwrap(), + vec![ + utc(2027, 1, 4, 10, 0), + utc(2027, 1, 10, 10, 0), + utc(2027, 1, 18, 10, 0), + utc(2027, 1, 24, 10, 0), + ] + ); +} + +/// The day of the month is the coach's: the 28th at 7 PM Chicago is the 29th in UTC and +/// is allowed, while the 29th at 8 AM Tokyo is the 28th in UTC and is not. +#[test] +fn monthly_start_day_is_judged_locally() { + let start = at(Chicago, 2027, 1, 28, 19, 0); + assert_eq!(start.day(), 29); + assert_eq!( + expand_recurrence(start, &counted(Frequency::Monthly, 3), Chicago).unwrap(), + vec![ + at(Chicago, 2027, 1, 28, 19, 0), + at(Chicago, 2027, 2, 28, 19, 0), + at(Chicago, 2027, 3, 28, 19, 0), + ] + ); + + let start = at(Tokyo, 2027, 1, 29, 8, 0); + assert_eq!(start.day(), 28); + assert_eq!( + expand_recurrence(start, &counted(Frequency::Monthly, 3), Tokyo).unwrap_err(), + RecurrenceError::MonthlyStartDayTooLate + ); +} + +/// A skipped local time uses the offset before the gap: 02:30 on the spring-forward day +/// is 03:30 daylight time. +#[test] +fn skipped_local_time_uses_the_offset_before_the_gap() { + let start = at(Chicago, 2027, 3, 12, 2, 30); + let slots = expand_recurrence(start, &counted(Frequency::Daily, 4), Chicago).unwrap(); + assert_eq!( + slots, + vec![ + utc(2027, 3, 12, 8, 30), + utc(2027, 3, 13, 8, 30), + utc(2027, 3, 14, 8, 30), + utc(2027, 3, 15, 7, 30), + ] + ); + assert_eq!( + to_local(Chicago, slots[2]), + wall(2027, 3, 14, 3, 30), + "the skipped slot lands an hour later on the wall clock" + ); +} + +/// A repeated local time is its first occurrence. +#[test] +fn repeated_local_time_is_its_first_occurrence() { + let start = at(Chicago, 2026, 10, 30, 1, 30); + let slots = expand_recurrence(start, &counted(Frequency::Daily, 4), Chicago).unwrap(); + assert_eq!( + slots, + vec![ + utc(2026, 10, 30, 6, 30), + utc(2026, 10, 31, 6, 30), + utc(2026, 11, 1, 6, 30), + utc(2026, 11, 2, 7, 30), + ] + ); +} + +/// 01:30 CST on Nov 1 is the second pass through 01:30. Its `DTSTART;TZID` would be read +/// as the first, so the series cannot start there; the first pass is fine. +#[test] +fn start_in_the_second_pass_of_a_repeated_hour_is_rejected() { + let second_pass = utc(2026, 11, 1, 7, 30); + assert_eq!( + expand_recurrence(second_pass, &counted(Frequency::Weekly, 3), Chicago).unwrap_err(), + RecurrenceError::AmbiguousStartAt + ); + + let first_pass = utc(2026, 11, 1, 6, 30); + let slots = expand_recurrence(first_pass, &counted(Frequency::Weekly, 3), Chicago).unwrap(); + assert_eq!(slots[0], first_pass); +} + +/// `until` bounds the resolved instant: the post-DST slot is 17:00Z, so an `until` of +/// 16:59:59 excludes it. +#[test] +fn until_is_compared_against_the_resolved_instant() { + let start = at(Chicago, 2026, 10, 29, 11, 0); + let through = |until| Recurrence { + until: Some(until), + count: None, + ..counted(Frequency::Weekly, 1) + }; + + let included = expand_recurrence(start, &through(utc(2026, 11, 5, 17, 0)), Chicago).unwrap(); + assert_eq!(included, vec![start, utc(2026, 11, 5, 17, 0)]); + + let cutoff = utc(2026, 11, 5, 17, 0) - Duration::seconds(1); + let excluded = expand_recurrence(start, &through(cutoff), Chicago).unwrap(); + assert_eq!(excluded, vec![start]); +} + +/// UTC has no transitions, so expansion there is exactly the old naive stepping. +#[test] +fn utc_expansion_is_plain_stepping() { + let start = utc(2026, 10, 20, 15, 0); + let slots = expand_recurrence(start, &counted(Frequency::Weekly, 4), UTC).unwrap(); + assert_eq!( + slots, + (0..4) + .map(|k| start + Duration::weeks(k)) + .collect::>() + ); +} diff --git a/domain/src/coaching_session_series.rs b/domain/src/coaching_session_series.rs index 88cf3a246..9007efc41 100644 --- a/domain/src/coaching_session_series.rs +++ b/domain/src/coaching_session_series.rs @@ -11,9 +11,11 @@ use crate::coaching_sessions; use crate::duration::Duration; use crate::emails; use crate::error::{DomainErrorKind, EntityErrorKind, Error, InternalErrorKind}; +use crate::gateway::ical; use crate::gateway::tiptap::TiptapDocument; use crate::Id; use chrono::NaiveDateTime; +use chrono_tz::Tz; use entity_api::coaching_session_series; use log::{debug, warn}; use sea_orm::{DatabaseConnection, TransactionTrait}; @@ -34,6 +36,29 @@ pub struct SeriesRule { pub start_at: NaiveDateTime, pub recurrence: Recurrence, pub duration_minutes: i16, + /// IANA zone the series was published in: its invites are written in it and its + /// slots expanded in it, so a coach changing zone later cannot split the two. + #[serde(default = "default_timezone")] + pub timezone: String, +} + +fn default_timezone() -> String { + chrono_tz::UTC.name().to_string() +} + +impl SeriesRule { + /// The zone to expand and publish in. An unknown name falls back to UTC, as an + /// invite for it would. + pub fn zone(&self) -> Tz { + self.timezone + .parse() + .map_or(chrono_tz::UTC, ical::emitted_zone) + } + + /// The original start of every occurrence, in the form `ical_recurrence_id` stores. + pub(crate) fn slots(&self) -> Result, Error> { + coaching_session::series_slots(self.start_at, &self.recurrence, self.zone()) + } } /// Create a series and materialize its sessions in a single transaction. @@ -72,8 +97,9 @@ pub async fn create_with_sessions( }); } let coach_id = relationship.coach_id; + let zone = publication_zone(db, coach_id).await?; - let dates = coaching_session::expand_recurrence(start_at, &recurrence)?; + let dates = coaching_session::expand_recurrence(start_at, &recurrence, zone)?; let resolved_duration = entity_api::coaching_session::resolve_duration(db, coach_id, requested_duration).await?; @@ -81,6 +107,7 @@ pub async fn create_with_sessions( start_at, recurrence, duration_minutes: resolved_duration.minutes(), + timezone: zone.name().to_string(), }; let rule_json = serde_json::to_value(&rule)?; @@ -115,6 +142,12 @@ pub async fn create_with_sessions( Ok((series, sessions)) } +/// The zone a series published now by this coach is written in. +async fn publication_zone(db: &DatabaseConnection, coach_id: Id) -> Result { + let coach = entity_api::user::find_by_id_without_roles(db, coach_id).await?; + Ok(emails::publication_zone(&coach)) +} + /// List every series owned by the given coaching relationship. pub async fn find_by_relationship( db: &DatabaseConnection, @@ -159,7 +192,9 @@ pub async fn reschedule( return Err(RecurrenceError::StartAtInPast.into()); } - let new_dates = coaching_session::expand_recurrence(new_start_at, &new_recurrence)?; + // A reschedule re-publishes the series, so it adopts the coach's zone as it is now. + let zone = publication_zone(db, coach_id).await?; + let new_dates = coaching_session::expand_recurrence(new_start_at, &new_recurrence, zone)?; // Resolve the duration for the re-materialized sessions. A reschedule moves // the meetings; it must not silently restretch them. So when the caller @@ -183,6 +218,7 @@ pub async fn reschedule( start_at: new_start_at, recurrence: new_recurrence, duration_minutes: resolved_duration.minutes(), + timezone: zone.name().to_string(), }; let future_sessions = @@ -440,6 +476,27 @@ mod tests { } } + /// The coach row the series reads its publication zone from. + fn coach_row(timezone: &str) -> entity::users::Model { + let now = chrono::Utc::now(); + entity::users::Model { + id: Id::new_v4(), + email: "coach@example.com".into(), + first_name: "Coach".into(), + last_name: "One".into(), + display_name: None, + password: None, + github_username: None, + github_profile_url: None, + timezone: timezone.into(), + default_coaching_session_duration_minutes: 60, + roles: vec![], + invite_status: None, + created_at: now.into(), + updated_at: now.into(), + } + } + fn start() -> NaiveDateTime { // Relative to now so `reschedule`'s past-guard (and date rollover) // can't make this flaky; a fixed calendar date eventually goes stale. @@ -519,7 +576,9 @@ mod tests { let db = MockDatabase::new(DatabaseBackend::Postgres) // 1. authz → coaching_relationship::find_by_id .append_query_results(vec![vec![relationship.clone()]]) - // 2. resolve_duration → user::find_by_id with related user_roles + // 2. publication zone → the coach + .append_query_results(vec![vec![coach_row("UTC")]]) + // 3. resolve_duration → user::find_by_id with related user_roles .append_query_results::<(entity::users::Model, Option), _, _>( vec![vec![(coach, None)]], ) @@ -571,6 +630,7 @@ mod tests { }; let db = MockDatabase::new(DatabaseBackend::Postgres) .append_query_results(vec![vec![relationship]]) + .append_query_results(vec![vec![coach_row("UTC")]]) .into_connection(); let bad_rule = Recurrence { frequency: Frequency::Weekly, @@ -613,6 +673,7 @@ mod tests { start_at: start(), recurrence: weekly_rule_count(3), duration_minutes: 60, + timezone: "UTC".into(), })?, ical_sequence: 0, created_by_user_id: Id::new_v4(), @@ -663,6 +724,8 @@ mod tests { ]; let db = MockDatabase::new(DatabaseBackend::Postgres) + // 0. publication zone → the coach + .append_query_results(vec![vec![coach_row("UTC")]]) // 1. duration omitted → find_by_id on the series to read its stored rule .append_query_results(vec![vec![existing_series.clone()]]) // 2. find_future_sessions_by_series_id → 2 future rows @@ -737,6 +800,7 @@ mod tests { start_at, recurrence: weekly_rule_count(3), duration_minutes: 60, + timezone: "UTC".into(), })?; let existing_series = Model { id: series_id, @@ -767,6 +831,7 @@ mod tests { }; let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![coach_row("UTC")]]) .append_query_results(vec![vec![existing_series.clone()]]) .append_query_results(vec![vec![future_session.clone()]]) .into_connection(); @@ -817,6 +882,7 @@ mod tests { start_at, recurrence: weekly_rule_count(3), duration_minutes: 60, + timezone: "UTC".into(), })?; // A field written by some other version of this struct. stored_rule["legacy_field"] = serde_json::json!("ignored"); @@ -850,6 +916,7 @@ mod tests { }; let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![coach_row("UTC")]]) .append_query_results(vec![vec![existing_series.clone()]]) .append_query_results(vec![vec![future_session]]) .into_connection(); @@ -998,6 +1065,7 @@ mod tests { start_at: start(), recurrence: weekly_rule_count(1), duration_minutes: 60, + timezone: "UTC".into(), })?, ical_sequence: 0, created_by_user_id: Id::new_v4(), @@ -1024,6 +1092,7 @@ mod tests { }; let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![coach_row("UTC")]]) .append_query_results(vec![vec![existing_series.clone()]]) // The pre-lock snapshot still shows SEQUENCE 1. .append_query_results(vec![vec![session(1)]]) diff --git a/domain/src/emails.rs b/domain/src/emails.rs index db0d5d7ef..23e3131f5 100644 --- a/domain/src/emails.rs +++ b/domain/src/emails.rs @@ -591,6 +591,35 @@ fn anchor_tz(coach: &users::Model) -> chrono_tz::Tz { coach.timezone.parse().unwrap_or(chrono_tz::UTC) } +/// The zone a series this coach publishes now is written in, after the `VTIMEZONE` +/// fallback, so expansion and the emitted `DTSTART;TZID` always agree. +pub(crate) fn publication_zone(coach: &users::Model) -> Tz { + ical::emitted_zone(anchor_tz(coach)) +} + +/// The zone a series' invites are written in: the one it was published in. +fn series_zone(series: &coaching_session_series::Model) -> Result { + Ok(serde_json::from_value::(series.rule.clone())?.zone()) +} + +/// The zone for an invite about `session`: its series' when it is an occurrence of one, +/// so `RECURRENCE-ID` names an instance in the zone the calendar expanded. +async fn invite_zone( + db: &DatabaseConnection, + session: &coaching_sessions::Model, + coach: &users::Model, +) -> Result { + match ( + session.coaching_session_series_id, + session.ical_recurrence_id, + ) { + (Some(series_id), Some(_)) => { + series_zone(&coaching_session_series::find_by_id(db, series_id).await?) + } + _ => Ok(anchor_tz(coach)), + } +} + /// A globally unique, stable `UID` for a calendar event. Stability is the whole /// mechanism: a client matches an update to the event it supersedes by `UID`. fn ics_uid(id: Id) -> String { @@ -782,10 +811,10 @@ fn build_occurrence_reschedule_ics( coachee: &users::Model, session: &coaching_sessions::Model, series_id: Id, + series_zone: Tz, description: String, dtstamp: chrono::NaiveDateTime, ) -> Result { - let anchor_tz = anchor_tz(coach); let invite = ical::IcsInvite { uid: ics_uid(series_id), sequence: session.ical_sequence, @@ -793,7 +822,7 @@ fn build_occurrence_reschedule_ics( status: ical::EventStatus::Confirmed, summary: session_summary(coach, coachee), description, - anchor_tz, + anchor_tz: series_zone, dtstamp, // The NEW start; `recurrence_id` keeps addressing the original slot. start: session.date, @@ -880,6 +909,7 @@ async fn send_single_session_invite_email( }); let dtstamp = chrono::Utc::now().naive_utc(); + let series_zone = invite_zone(db, session, coach).await?; // A session inside a series is addressed as an override of its occurrence; one that // predates `ical_recurrence_id` goes out self-contained under its own `UID`. let ics_body = build_session_ics( @@ -891,6 +921,7 @@ async fn send_single_session_invite_email( coachee, session, series_id, + series_zone, description, dtstamp, ) @@ -1340,10 +1371,10 @@ fn build_occurrence_cancel_ics( coachee: &users::Model, session: &coaching_sessions::Model, series_id: Id, + series_zone: Tz, description: String, dtstamp: chrono::NaiveDateTime, ) -> Result { - let anchor_tz = anchor_tz(coach); let invite = ical::IcsInvite { uid: ics_uid(series_id), // Already bumped by the caller inside the delete transaction, so the cancellation @@ -1353,7 +1384,7 @@ fn build_occurrence_cancel_ics( status: ical::EventStatus::Cancelled, summary: session_summary(coach, coachee), description, - anchor_tz, + anchor_tz: series_zone, dtstamp, start: session.date, duration_minutes: session.duration_minutes, @@ -1401,13 +1432,15 @@ async fn send_session_cancelled_email_to_recipient( } /// Send session-cancelled emails to both coach and coachee. Takes no database handle: -/// a cancellation loads no topics, goals, or actions. +/// a cancellation loads no topics, goals, or actions. `series_zone` is the zone from +/// [`invite_zone`], resolved by the caller. async fn send_session_cancelled_email( config: &Config, coach: &users::Model, coachee: &users::Model, session: &coaching_sessions::Model, organization: &organizations::Model, + series_zone: Tz, ) -> Result<(), Error> { info!( "Initiating session cancelled emails for session: {} (coach: {}, coachee: {})", @@ -1422,7 +1455,15 @@ async fn send_session_cancelled_email( session, SESSION_CANCELLED_DESCRIPTION.to_string(), |series_id, description| { - build_occurrence_cancel_ics(coach, coachee, session, series_id, description, dtstamp) + build_occurrence_cancel_ics( + coach, + coachee, + session, + series_id, + series_zone, + description, + dtstamp, + ) }, |description| { build_session_cancel_ics( @@ -1497,7 +1538,8 @@ pub async fn notify_session_cancelled( let coachee = user::find_by_id_without_roles(db, relationship.coachee_id).await?; let org = organization::find_by_id(db, relationship.organization_id).await?; - send_session_cancelled_email(config, &coach, &coachee, session, &org).await + let series_zone = invite_zone(db, session, &coach).await?; + send_session_cancelled_email(config, &coach, &coachee, session, &org, series_zone).await } .await; @@ -1592,7 +1634,6 @@ fn build_series_invite_ics( description: String, dtstamp: chrono::NaiveDateTime, ) -> Result { - let anchor_tz = anchor_tz(coach); let rule: SeriesRule = serde_json::from_value(series.rule.clone())?; let invite = ical::IcsInvite { uid: ics_uid(series.id), @@ -1601,7 +1642,7 @@ fn build_series_invite_ics( status: ical::EventStatus::Confirmed, summary: session_summary(coach, coachee), description, - anchor_tz, + anchor_tz: rule.zone(), dtstamp, start: first_session.date, duration_minutes: first_session.duration_minutes, @@ -1627,9 +1668,9 @@ fn build_series_snapshot_ics( description: String, dtstamp: chrono::NaiveDateTime, ) -> Result { - let anchor_tz = anchor_tz(coach); let rule: SeriesRule = serde_json::from_value(series.rule.clone())?; - let slots = coaching_session::series_slots(rule.start_at, &rule.recurrence)?; + let anchor_tz = rule.zone(); + let slots = rule.slots()?; let start = *slots.first().ok_or_else(|| Error { source: None, error_kind: DomainErrorKind::Internal(InternalErrorKind::Other( @@ -2103,7 +2144,6 @@ fn build_series_cancel_ics( description: String, dtstamp: chrono::NaiveDateTime, ) -> Result { - let anchor_tz = anchor_tz(coach); let rule: SeriesRule = serde_json::from_value(series.rule.clone())?; let invite = ical::IcsInvite { uid: ics_uid(series.id), @@ -2114,7 +2154,7 @@ fn build_series_cancel_ics( status: ical::EventStatus::Cancelled, summary: session_summary(coach, coachee), description, - anchor_tz, + anchor_tz: rule.zone(), dtstamp, start: first_session.date, duration_minutes: first_session.duration_minutes, diff --git a/domain/src/emails_tests.rs b/domain/src/emails_tests.rs index 9b944fefc..c25765305 100644 --- a/domain/src/emails_tests.rs +++ b/domain/src/emails_tests.rs @@ -2,7 +2,7 @@ use super::*; use crate::{coaching_sessions, organizations, users, Id}; use base64::engine::general_purpose::STANDARD; use base64::Engine; -use chrono::NaiveDate; +use chrono::{DurationRound, NaiveDate}; use mockito::{Server, ServerGuard}; #[cfg(feature = "mock")] use sea_orm::{DatabaseBackend, MockDatabase}; @@ -821,6 +821,17 @@ fn mock_description_loaders() -> DatabaseConnection { .into_connection() } +/// The description loaders, then the series an occurrence invite reads its zone from. +#[cfg(feature = "mock")] +fn mock_occurrence_loaders(series: coaching_session_series::Model) -> DatabaseConnection { + MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results::(vec![vec![]]) + .append_query_results::(vec![vec![]]) + .append_query_results::(vec![vec![]]) + .append_query_results(vec![vec![series]]) + .into_connection() +} + /// mockito has no negative body matcher, so absence of a template variable rides on /// `match_request`. An unreadable or unparsable body fails the match. fn reject_template_variables(keys: &'static [&'static str]) -> impl Fn(&mockito::Request) -> bool { @@ -1178,12 +1189,13 @@ async fn test_send_session_rescheduled_email_omits_recurrence_variables() { async fn test_send_occurrence_rescheduled_email_addresses_the_occurrence() { let mut server = setup_test_server().await; let config = create_full_config_with_mock(&server.url()); - let db = mock_description_loaders(); + let series = create_test_series(); + let series_id = series.id; + let db = mock_occurrence_loaders(series); let capture = IcsCapture::default(); let coach = create_test_user_with("Alex", "Smith", "alex@example.com", "UTC"); let coachee = create_test_user_with("Jane", "Doe", "jane@example.com", "UTC"); - let series_id = Id::new_v4(); let original_start = NaiveDate::from_ymd_opt(2026, 3, 4) .unwrap() .and_hms_opt(15, 0, 0) @@ -1428,7 +1440,9 @@ async fn test_send_session_cancelled_email() { .create_async() .await; - let result = send_session_cancelled_email(&config, &coach, &coachee, &session, &org).await; + let result = + send_session_cancelled_email(&config, &coach, &coachee, &session, &org, anchor_tz(&coach)) + .await; assert!(result.is_ok()); // The send swallows errors, so the mock assertions are what give this test teeth. @@ -1487,7 +1501,9 @@ async fn test_send_session_cancelled_email_omits_when_variables() { .create_async() .await; - let result = send_session_cancelled_email(&config, &coach, &coachee, &session, &org).await; + let result = + send_session_cancelled_email(&config, &coach, &coachee, &session, &org, anchor_tz(&coach)) + .await; assert!(result.is_ok()); mock.assert_async().await; @@ -1528,6 +1544,7 @@ fn test_build_occurrence_cancel_ics_addresses_the_series_uid() { &coachee, &session, series_id, + chrono_tz::America::New_York, SESSION_CANCELLED_DESCRIPTION.to_string(), dtstamp, ) @@ -1570,6 +1587,7 @@ fn test_build_occurrence_reschedule_ics_keeps_original_recurrence_id() { &coachee, &session, series_id, + chrono_tz::America::New_York, "Join this session: https://app/x".into(), dtstamp, ) @@ -1644,7 +1662,9 @@ async fn test_cancelling_a_legacy_series_member_addresses_it_by_its_own_uid() { .create_async() .await; - let result = send_session_cancelled_email(&config, &coach, &coachee, &session, &org).await; + let result = + send_session_cancelled_email(&config, &coach, &coachee, &session, &org, anchor_tz(&coach)) + .await; assert!(result.is_ok()); mock.assert_async().await; @@ -1931,7 +1951,9 @@ async fn test_send_occurrence_cancelled_email_uses_session_template() { .create_async() .await; - let result = send_session_cancelled_email(&config, &coach, &coachee, &session, &org).await; + let result = + send_session_cancelled_email(&config, &coach, &coachee, &session, &org, anchor_tz(&coach)) + .await; assert!(result.is_ok()); // The send swallows errors, so the mock assertions are what give this test teeth. @@ -2359,6 +2381,7 @@ fn create_test_series() -> coaching_session_series::Model { "start_at": "2026-09-15T15:00:00", "recurrence": { "frequency": "weekly", "interval": 1 }, "duration_minutes": 60, + "timezone": "America/New_York", }), ical_sequence: 0, created_by_user_id: Id::new_v4(), @@ -2453,6 +2476,7 @@ fn snapshot_series(start_at: NaiveDateTime, ical_sequence: i32) -> coaching_sess "start_at": start_at, "recurrence": { "frequency": "weekly", "interval": 1, "count": 6 }, "duration_minutes": 60, + "timezone": "America/Chicago", }), ical_sequence, ..create_test_series() @@ -2540,7 +2564,10 @@ fn assert_snapshot( assert_eq!(events.len(), 1 + overrides.len(), "{ics}"); let master = events[0]; assert!(master.contains(&format!("DTSTART;{tz}:{dtstart}")), "{ics}"); - assert!(master.contains("RRULE:FREQ=WEEKLY;COUNT=6"), "{ics}"); + assert!( + master.contains("RRULE:FREQ=WEEKLY;WKST=MO;COUNT=6"), + "{ics}" + ); assert!(master.contains(&format!("SEQUENCE:{sequence}")), "{ics}"); assert!(master.contains("LOCATION:https://meet.example/series")); assert_eq!(ics.matches("EXDATE").count(), exdates.len(), "{ics}"); @@ -4145,3 +4172,234 @@ async fn test_session_reminder_missing_template_id() { other => panic!("Expected Config error, got: {other:?}"), } } + +// ───── stored series zone vs the coach's current zone ───── + +/// Weekly Thursdays at 11:00 Chicago, published before its coach moved to New York. +fn chicago_series() -> coaching_session_series::Model { + coaching_session_series::Model { + rule: serde_json::json!({ + "start_at": "2026-09-10T16:00:00", + "recurrence": { + "frequency": "weekly", + "interval": 1, + "by_weekdays": ["Thu"], + "until": "2027-01-01T05:59:59", + }, + "duration_minutes": 60, + "timezone": "America/Chicago", + }), + ..create_test_series() + } +} + +fn relocated_coach() -> users::Model { + create_test_user_with("Alex", "Smith", "alex@example.com", "America/New_York") +} + +/// An occurrence reads its zone from the series; anything else keeps the coach's, and +/// a legacy member has no occurrence to address, so it reads nothing. +#[cfg(feature = "mock")] +#[tokio::test] +async fn invite_zone_is_the_series_zone_for_an_occurrence_only() { + let coach = relocated_coach(); + let series = chicago_series(); + + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![series.clone()]]) + .into_connection(); + let member = create_test_series_session(series.id, at(2026, 11, 5, 17)); + assert_eq!( + invite_zone(&db, &member, &coach).await.unwrap(), + chrono_tz::America::Chicago + ); + + let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection(); + let legacy = coaching_sessions::Model { + ical_recurrence_id: None, + ..member.clone() + }; + assert_eq!( + invite_zone(&db, &legacy, &coach).await.unwrap(), + chrono_tz::America::New_York + ); + assert_eq!( + invite_zone(&db, &create_test_session(), &coach) + .await + .unwrap(), + chrono_tz::America::New_York + ); + assert!(db.into_transaction_log().is_empty()); +} + +/// Moving one post-DST occurrence names the slot in the zone the calendar expanded the +/// series in, not the coach's new one: 11:00 Chicago on Nov 5 is 17:00Z. +#[cfg(feature = "mock")] +#[tokio::test] +async fn occurrence_reschedule_uses_the_stored_series_zone() { + let mut server = setup_test_server().await; + let config = create_full_config_with_mock(&server.url()); + let series = chicago_series(); + let capture = IcsCapture::default(); + let coach = relocated_coach(); + let coachee = create_test_user_with("Jane", "Doe", "jane@example.com", "UTC"); + let slot = at(2026, 11, 5, 17); + let session = coaching_sessions::Model { + date: at(2026, 11, 5, 19), + ..create_test_series_session(series.id, slot) + }; + let db = mock_occurrence_loaders(series); + + let mock = server + .mock("POST", "/emails") + .match_request(capture.recorder()) + .with_status(200) + .with_body(r#"{"id":"email_test"}"#) + .expect(2) + .create_async() + .await; + + send_session_rescheduled_email( + &db, + &config, + &coach, + &coachee, + &session, + &create_test_organization(), + slot, + ) + .await + .unwrap(); + mock.assert_async().await; + + let ics = capture.ics(); + assert!( + ics.contains("RECURRENCE-ID;TZID=America/Chicago:20261105T110000"), + "{ics}" + ); + assert!( + ics.contains("DTSTART;TZID=America/Chicago:20261105T130000"), + "{ics}" + ); + assert!(!ics.contains("America/New_York"), "{ics}"); +} + +/// Cancelling an occurrence addresses it in the stored zone too, through the same +/// lookup the send path uses. +#[cfg(feature = "mock")] +#[tokio::test] +async fn occurrence_cancel_uses_the_stored_series_zone() { + let mut server = setup_test_server().await; + let config = create_full_config_with_mock(&server.url()); + let series = chicago_series(); + let capture = IcsCapture::default(); + let coach = relocated_coach(); + let coachee = create_test_user_with("Jane", "Doe", "jane@example.com", "UTC"); + let org = create_test_organization(); + // Upcoming, or the notifier treats the delete as housekeeping. + let slot = (chrono::Utc::now() + chrono::Duration::days(30)) + .naive_utc() + .duration_trunc(chrono::TimeDelta::hours(1)) + .unwrap(); + let session = create_test_series_session(series.id, slot); + let relationship = coaching_relationships::Model { + id: session.coaching_relationship_id, + organization_id: org.id, + coach_id: coach.id, + coachee_id: coachee.id, + slug: "test".into(), + created_at: chrono::Utc::now().into(), + updated_at: chrono::Utc::now().into(), + }; + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![relationship]]) + .append_query_results(vec![vec![coach.clone()]]) + .append_query_results(vec![vec![coachee]]) + .append_query_results(vec![vec![org]]) + .append_query_results(vec![vec![series]]) + .into_connection(); + + let mock = server + .mock("POST", "/emails") + .match_request(capture.recorder()) + .with_status(200) + .with_body(r#"{"id":"email_test"}"#) + .expect(2) + .create_async() + .await; + + notify_session_cancelled(&db, &config, &session).await; + mock.assert_async().await; + + let local = Utc + .from_utc_datetime(&slot) + .with_timezone(&chrono_tz::America::Chicago) + .format("%Y%m%dT%H%M%S"); + let ics = capture.ics(); + assert!( + ics.contains(&format!("RECURRENCE-ID;TZID=America/Chicago:{local}")), + "{ics}" + ); + assert!(ics.contains("METHOD:CANCEL"), "{ics}"); + assert!(!ics.contains("America/New_York"), "{ics}"); +} + +/// The snapshot of a Chicago series across Nov 1, built for a coach now in New York: +/// the master, its EXDATE and its override all sit at 11:00 Chicago on both sides of the +/// change, which is where the calendar holds every instance. +#[test] +fn snapshot_across_dst_uses_the_stored_series_zone() { + let series = chicago_series(); + let rule: SeriesRule = serde_json::from_value(series.rule.clone()).unwrap(); + let slots = rule.slots().unwrap(); + let nov_5 = slots + .iter() + .position(|s| *s == at(2026, 11, 5, 17)) + .unwrap(); + let nov_12 = slots + .iter() + .position(|s| *s == at(2026, 11, 12, 17)) + .unwrap(); + + let members: Vec = slots + .iter() + .enumerate() + .filter(|(i, _)| *i != nov_12) + .map(|(i, slot)| coaching_sessions::Model { + duration_minutes: 60, + date: match i == nov_5 { + true => at(2026, 11, 5, 19), + false => *slot, + }, + ..create_test_series_session(series.id, *slot) + }) + .collect(); + + let coachee = create_test_user_with("Jane", "Doe", "jane@example.com", "UTC"); + let ics = unfold( + &build_series_snapshot_ics( + &relocated_coach(), + &coachee, + &series, + &members, + "desc".into(), + at(2026, 10, 20, 12), + ) + .unwrap(), + ); + + assert!( + ics.contains("DTSTART;TZID=America/Chicago:20260910T110000"), + "{ics}" + ); + assert!( + ics.contains("EXDATE;TZID=America/Chicago:20261112T110000"), + "{ics}" + ); + assert!( + ics.contains("RECURRENCE-ID;TZID=America/Chicago:20261105T110000"), + "{ics}" + ); + assert_eq!(ics.matches("RECURRENCE-ID").count(), 1, "{ics}"); + assert!(!ics.contains("America/New_York"), "{ics}"); +} diff --git a/domain/src/gateway/ical.rs b/domain/src/gateway/ical.rs index 4500f0f5e..5b91a639b 100644 --- a/domain/src/gateway/ical.rs +++ b/domain/src/gateway/ical.rs @@ -15,6 +15,10 @@ use vtimezones_rs::VTIMEZONES; #[path = "ical_tests.rs"] mod tests; +#[cfg(test)] +#[path = "ical_oracle_tests.rs"] +mod oracle_tests; + /// VCALENDAR-level `METHOD`. #[derive(PartialEq, Eq)] pub enum Method { @@ -106,6 +110,15 @@ fn vtimezone_block(tz: chrono_tz::Tz) -> Option { Some(inner.replace("\r\n", "\n").replace('\n', "\r\n")) } +/// The zone an invite anchored to `tz` is actually written in: `tz` itself, or UTC when +/// no `VTIMEZONE` block exists for it. Series expansion must use this same zone. +pub fn emitted_zone(tz: chrono_tz::Tz) -> chrono_tz::Tz { + match tz == chrono_tz::UTC || vtimezone_block(tz).is_some() { + true => tz, + false => chrono_tz::UTC, + } +} + /// Insert the `VTIMEZONE` block immediately before the first `BEGIN:VEVENT`. fn splice_vtimezone(serialized: &str, block: &str) -> Result { let marker = "BEGIN:VEVENT"; @@ -139,6 +152,8 @@ fn rrule_value(rule: &Recurrence) -> String { let effective_interval = rule.effective_interval(); let mut parts = vec![format!("FREQ={freq}")]; + // Explicit, so every client aligns multi-week steps on Monday as the expander does. + parts.push("WKST=MO".to_string()); if effective_interval > 1 { parts.push(format!("INTERVAL={effective_interval}")); } diff --git a/domain/src/gateway/ical_oracle_tests.rs b/domain/src/gateway/ical_oracle_tests.rs new file mode 100644 index 000000000..4604d6e93 --- /dev/null +++ b/domain/src/gateway/ical_oracle_tests.rs @@ -0,0 +1,204 @@ +//! The series' stored slots must be the instances a calendar client expands from the +//! invite. An independent RFC 5545 implementation reads the exact `DTSTART` and `RRULE` +//! this builder emits, and must produce the same instants `expand_recurrence` does. + +use super::*; +use crate::coaching_session::{expand_recurrence, Frequency, Recurrence}; +use chrono::{NaiveDate, Weekday}; +use chrono_tz::America::Chicago; +use chrono_tz::Asia::{Kolkata, Tokyo}; +use chrono_tz::Australia::Sydney; +use chrono_tz::Europe::London; +use rrule::RRuleSet; + +fn wall(y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { + NaiveDate::from_ymd_opt(y, m, d) + .unwrap() + .and_hms_opt(h, min, 0) + .unwrap() +} + +/// The instant of a local wall time, first pass when it repeats. +fn at(zone: chrono_tz::Tz, local: NaiveDateTime) -> NaiveDateTime { + zone.from_local_datetime(&local) + .earliest() + .unwrap() + .naive_utc() +} + +fn series_invite( + zone: chrono_tz::Tz, + start: NaiveDateTime, + rule: Recurrence, +) -> IcsInvite<'static> { + IcsInvite { + uid: "series-1@myrefactor.com".into(), + sequence: 0, + method: Method::Request, + status: EventStatus::Confirmed, + summary: "Jane / Alex".into(), + description: "desc".into(), + anchor_tz: zone, + dtstamp: wall(2026, 10, 1, 12, 0), + start, + duration_minutes: 60, + organizer: Participant::new("Refactor Coach", "hello@mail.myrefactor.com"), + attendees: vec![Participant::new("Coachee Quinn", "coachee@example.com")], + location_url: None, + recurrence: Some(rule), + recurrence_id: None, + exdates: Vec::new(), + } +} + +/// The `DTSTART` and `RRULE` lines of the emitted VEVENT, as a client reads them. +fn emitted_rule(ics: &str) -> String { + let unfolded = ics.replace("\r\n ", "").replace("\r\n\t", ""); + let vevent = &unfolded[unfolded.find("BEGIN:VEVENT").unwrap()..]; + let line = |prefix: &str| { + vevent + .lines() + .find(|line| line.starts_with(prefix)) + .unwrap() + .trim() + .to_string() + }; + format!("{}\n{}", line("DTSTART"), line("RRULE:")) +} + +/// What a client expands from the invite, as UTC instants. +fn client_instances( + zone: chrono_tz::Tz, + start: NaiveDateTime, + rule: &Recurrence, +) -> Vec { + let ics = build(&series_invite(zone, start, rule.clone())).unwrap(); + let set: RRuleSet = emitted_rule(&ics).parse().unwrap(); + set.all(400) + .dates + .into_iter() + .map(|instance| instance.with_timezone(&Utc).naive_utc()) + .collect() +} + +fn rule(frequency: Frequency, count: u32) -> Recurrence { + Recurrence { + frequency, + interval: 1, + by_weekdays: None, + count: Some(count), + until: None, + } +} + +fn on(weekdays: &[Weekday], frequency: Frequency, count: u32) -> Recurrence { + Recurrence { + by_weekdays: Some(weekdays.to_vec()), + ..rule(frequency, count) + } +} + +#[test] +fn series_slots_equal_the_instances_a_client_expands() { + let cases: Vec<(chrono_tz::Tz, NaiveDateTime, Recurrence)> = vec![ + // Every frequency across fall-back and spring-forward. + ( + Chicago, + wall(2026, 10, 20, 10, 0), + rule(Frequency::Daily, 30), + ), + ( + Chicago, + wall(2026, 10, 20, 10, 0), + rule(Frequency::Weekly, 8), + ), + ( + Chicago, + wall(2026, 10, 20, 10, 0), + rule(Frequency::Biweekly, 6), + ), + ( + Chicago, + wall(2026, 10, 20, 10, 0), + rule(Frequency::Monthly, 7), + ), + (Chicago, wall(2027, 3, 2, 10, 0), rule(Frequency::Weekly, 6)), + (London, wall(2026, 10, 13, 9, 0), rule(Frequency::Weekly, 6)), + (London, wall(2027, 3, 16, 9, 0), rule(Frequency::Daily, 20)), + (Sydney, wall(2027, 3, 23, 9, 0), rule(Frequency::Weekly, 6)), + (Sydney, wall(2027, 9, 21, 9, 0), rule(Frequency::Monthly, 4)), + ( + Kolkata, + wall(2026, 10, 20, 18, 30), + rule(Frequency::Weekly, 6), + ), + ( + chrono_tz::UTC, + wall(2026, 10, 20, 15, 0), + rule(Frequency::Weekly, 6), + ), + // The production series, `until`-terminated. + ( + Chicago, + wall(2026, 9, 10, 11, 0), + Recurrence { + until: Some(wall(2027, 1, 1, 5, 59)), + count: None, + ..on(&[Weekday::Thu], Frequency::Weekly, 1) + }, + ), + // Weekdays whose UTC day differs from the local one. + ( + Chicago, + wall(2026, 10, 22, 19, 0), + on(&[Weekday::Thu, Weekday::Fri], Frequency::Weekly, 8), + ), + ( + Tokyo, + wall(2026, 10, 19, 8, 0), + on(&[Weekday::Mon, Weekday::Wed], Frequency::Biweekly, 6), + ), + // Multi-week steps with Sunday, which depend on WKST. + ( + chrono_tz::UTC, + wall(2027, 1, 4, 10, 0), + on(&[Weekday::Mon, Weekday::Sun], Frequency::Biweekly, 6), + ), + ( + Chicago, + wall(2027, 1, 4, 10, 0), + on(&[Weekday::Mon, Weekday::Sun], Frequency::Biweekly, 6), + ), + // Every three weeks, and a monthly start on the 28th in the evening. + ( + Chicago, + wall(2026, 10, 20, 10, 0), + Recurrence { + interval: 3, + ..rule(Frequency::Weekly, 6) + }, + ), + ( + Chicago, + wall(2027, 1, 28, 19, 0), + rule(Frequency::Monthly, 4), + ), + // A skipped and a repeated local time. + (Chicago, wall(2027, 3, 12, 2, 30), rule(Frequency::Daily, 4)), + ( + Chicago, + wall(2026, 10, 30, 1, 30), + rule(Frequency::Daily, 4), + ), + ]; + + for (zone, local_start, recurrence) in cases { + let start = at(zone, local_start); + let slots = expand_recurrence(start, &recurrence, zone).unwrap(); + assert_eq!( + slots, + client_instances(zone, start, &recurrence), + "{zone} from {local_start}: {recurrence:?}" + ); + } +} diff --git a/domain/src/gateway/ical_tests.rs b/domain/src/gateway/ical_tests.rs index 589f85fc6..37d69017f 100644 --- a/domain/src/gateway/ical_tests.rs +++ b/domain/src/gateway/ical_tests.rs @@ -93,7 +93,7 @@ fn recurring_biweekly_rrule() { Some(rule), )) .unwrap(); - assert!(out.contains("RRULE:FREQ=WEEKLY;INTERVAL=2")); + assert!(out.contains("RRULE:FREQ=WEEKLY;WKST=MO;INTERVAL=2")); } #[test] diff --git a/migration/src/lib.rs b/migration/src/lib.rs index 80ff27210..0de1a2b8b 100644 --- a/migration/src/lib.rs +++ b/migration/src/lib.rs @@ -56,6 +56,7 @@ mod m20260922_000000_create_coaching_session_images; mod m20261005_000000_add_transcript_participants; mod m20261009_000000_index_transcript_participant_links; mod m20261010_000000_add_transcript_suggestions; +mod m20261021_000000_add_timezone_to_series_rules; pub struct Migrator; @@ -121,6 +122,7 @@ impl MigratorTrait for Migrator { Box::new(m20261005_000000_add_transcript_participants::Migration), Box::new(m20261009_000000_index_transcript_participant_links::Migration), Box::new(m20261010_000000_add_transcript_suggestions::Migration), + Box::new(m20261021_000000_add_timezone_to_series_rules::Migration), ] } } diff --git a/migration/src/m20261021_000000_add_timezone_to_series_rules.rs b/migration/src/m20261021_000000_add_timezone_to_series_rules.rs new file mode 100644 index 000000000..1911daa67 --- /dev/null +++ b/migration/src/m20261021_000000_add_timezone_to_series_rules.rs @@ -0,0 +1,46 @@ +use sea_orm_migration::prelude::*; + +/// Records on each series rule the zone its invites were published in, so slots are +/// expanded in the same zone the calendar expands its `RRULE` in. Existing series take +/// the coach's current zone, which is the one every invite so far was written in. +#[derive(DeriveMigrationName)] +pub struct Migration; + +#[async_trait::async_trait] +impl MigrationTrait for Migration { + async fn up(&self, manager: &SchemaManager) -> Result<(), DbErr> { + // A zone Postgres does not know falls back to UTC, as the invite builder does. + manager + .get_connection() + .execute_unprepared( + "UPDATE refactor_platform.coaching_session_series AS s + SET rule = jsonb_set( + s.rule, + '{timezone}', + to_jsonb(CASE + WHEN u.timezone IN (SELECT name FROM pg_timezone_names) THEN u.timezone + ELSE 'UTC' + END) + ) + FROM refactor_platform.coaching_relationships AS r + JOIN refactor_platform.users AS u ON u.id = r.coach_id + WHERE r.id = s.coaching_relationship_id + AND NOT (s.rule ? 'timezone')", + ) + .await?; + + Ok(()) + } + + async fn down(&self, manager: &SchemaManager) -> Result<(), DbErr> { + manager + .get_connection() + .execute_unprepared( + "UPDATE refactor_platform.coaching_session_series + SET rule = rule - 'timezone'", + ) + .await?; + + Ok(()) + } +} From 4e71ab1531a42f37770ad80c1a77f4e4a1ac54da Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Tue, 20 Oct 2026 15:45:35 -0500 Subject: [PATCH 2/4] docs: add DST cases to the .ics manual test plan --- .../ics-calendar-invites-manual-test-plan.md | 64 ++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/test-plans/ics-calendar-invites-manual-test-plan.md b/docs/test-plans/ics-calendar-invites-manual-test-plan.md index 526d43725..6af19ad72 100644 --- a/docs/test-plans/ics-calendar-invites-manual-test-plan.md +++ b/docs/test-plans/ics-calendar-invites-manual-test-plan.md @@ -141,7 +141,7 @@ above. Watch for: exactly one `BEGIN:VTIMEZONE` (before `BEGIN:VEVENT`), `DTSTAR ### H4: Biweekly series 1. Create a biweekly series. -2. **Expect:** `RRULE:FREQ=WEEKLY;INTERVAL=2`; occurrences land every 2 weeks. +2. **Expect:** `RRULE:FREQ=WEEKLY;WKST=MO;INTERVAL=2`; occurrences land every 2 weeks. ### H5: Rich single-session description 1. Single session WITH a title, one or more topics, at least one in-progress goal, and at @@ -455,6 +455,68 @@ session moving or being cancelled. --- +## Series across a DST change + +The app and the calendar must hold every occurrence at the same instant on both sides of +a DST change. Series are expanded in the zone stored on the series (`rule.timezone`), at +the local wall time of the first session, which is how calendars read the +`DTSTART;TZID` + `RRULE`. A UTC coach never exercises this, so run these with the coach's +timezone set to **`America/Chicago`** and pick dates that span **Nov 1, 2026** (fall back) +or **Mar 14, 2027** (spring forward). Verify each case in **Google Calendar** as well as +in the app. + +### HD1: Weekly series keeps its wall time across DST +1. Create a weekly series on Thursdays at **11:00 AM Chicago**, starting before Nov 1 and + running at least three weeks past it. +2. **Expect** in the app: every session shows 11:00 AM, before and after Nov 1. +3. **Expect** in Google Calendar: every instance at 11:00 AM, matching the app one for one. +4. The series response's `rule.timezone` is `America/Chicago`. + +### HD2: Move one occurrence after DST +1. In the HD1 series, move the first occurrence after Nov 1 from 11:00 AM to 1:00 PM. +2. **Expect** in the `.ics`: `RECURRENCE-ID;TZID=America/Chicago:T110000` and + `DTSTART;TZID=America/Chicago:T130000`. +3. **Expect** in Google Calendar: that instance moves to 1:00 PM, and Gmail does **not** + say "Removed from Google Calendar". + +### HD3: Cancel one occurrence after DST +1. In the HD1 series, delete a different occurrence after Nov 1. +2. **Expect** in the `.ics`: `METHOD:CANCEL` with `RECURRENCE-ID;TZID=America/Chicago:T110000`. +3. **Expect** in Google Calendar: that instance disappears; its neighbours stay at 11:00 AM. + +### HD4: Resend the series after HD2 and HD3 +1. Resend the HD1 series invite. +2. **Expect** in the `.ics`: an `EXDATE;TZID=America/Chicago:T110000` and one + override with `RECURRENCE-ID;TZID=America/Chicago:T110000`. +3. **Expect** in Google Calendar: the moved instance at 1:00 PM, the deleted one absent, + every other instance at 11:00 AM. + +### HD5: Evening weekly series (local weekday) +1. Create a weekly series on Thursdays at **7:00 PM Chicago** (Friday in UTC). +2. **Expect:** it is created (no 422), and every session in the app and in Google + Calendar is a Thursday at 7:00 PM, including after Nov 1. + +### HD6: Spring forward +1. Create a weekly series at 10:00 AM Chicago starting in early March 2027 and running past + Mar 14. +2. **Expect:** every session shows 10:00 AM in the app and in Google Calendar on both sides + of Mar 14. + +### SD1: Monthly series on the 29th to 31st is refused (sad path) +1. Via the API, create a monthly series whose first session is on the 30th in the coach's + timezone. +2. **Expect:** `422` with `"error":"validation_error"` and + `"message":"Monthly series must start on day 28 or earlier in the coach's timezone."` +3. A monthly series on the 28th at 7:00 PM Chicago (the 29th in UTC) is accepted. + +### SD2: Coach changes timezone after publishing (sad path) +1. Create the HD1 series, then change the coach's timezone to `America/New_York`. +2. Move one occurrence after Nov 1. +3. **Expect:** the `.ics` still uses `TZID=America/Chicago` and the `RECURRENCE-ID` still + names 11:00 AM Chicago, so Google moves the right instance. + +--- + ## Not testable yet (later phases: expect NOTHING to happen) - Nothing. All planned phases (0-11) are implemented. From c9df36405113bf162497ca6478d31d14dfefd223 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Tue, 20 Oct 2026 15:51:45 -0500 Subject: [PATCH 3/4] test: gate the DurationRound import on the mock feature --- domain/src/emails_tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/domain/src/emails_tests.rs b/domain/src/emails_tests.rs index c25765305..b5d914f75 100644 --- a/domain/src/emails_tests.rs +++ b/domain/src/emails_tests.rs @@ -2,7 +2,9 @@ use super::*; use crate::{coaching_sessions, organizations, users, Id}; use base64::engine::general_purpose::STANDARD; use base64::Engine; -use chrono::{DurationRound, NaiveDate}; +#[cfg(feature = "mock")] +use chrono::DurationRound; +use chrono::NaiveDate; use mockito::{Server, ServerGuard}; #[cfg(feature = "mock")] use sea_orm::{DatabaseBackend, MockDatabase}; From 8e69aab9763e74f3b08117874928fac28ea10956 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Tue, 20 Oct 2026 16:43:38 -0500 Subject: [PATCH 4/4] fix: expand stored series without the checks only new rules must pass Snapshots expanded stored rules through the publish-time validation, so a monthly series on day 29-31, or one accepted under the old UTC weekday check, could no longer be resent. Stored rules now expand with structural checks only, and months without the start day are skipped as RFC 5545 clients skip them. Also orders the new test files' imports external-first. --- domain/src/coaching_session.rs | 4 +- domain/src/coaching_session/recurrence.rs | 62 +++++++++++++++---- .../coaching_session/recurrence_dst_tests.rs | 57 ++++++++++++++++- domain/src/emails_tests.rs | 50 +++++++++++++++ domain/src/gateway/ical_oracle_tests.rs | 41 +++++++++++- 5 files changed, 196 insertions(+), 18 deletions(-) diff --git a/domain/src/coaching_session.rs b/domain/src/coaching_session.rs index 70df98ec4..7fb9a4594 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -182,14 +182,14 @@ pub async fn bulk_create_recurring( .await?) } -/// The original start of every occurrence a series rule produces, truncated exactly as +/// The original start of every occurrence a published series rule produces, truncated as /// [`bulk_create_recurring`] stores them, so they compare equal to `ical_recurrence_id`. pub(crate) fn series_slots( start_at: NaiveDateTime, recurrence: &Recurrence, zone: chrono_tz::Tz, ) -> Result, Error> { - expand_recurrence(start_at, recurrence, zone)? + recurrence::expand_published(start_at, recurrence, zone)? .into_iter() .map(|d| SessionDate::new(d).map(SessionDate::into_inner)) .collect() diff --git a/domain/src/coaching_session/recurrence.rs b/domain/src/coaching_session/recurrence.rs index 794d9a8a3..eb40e19aa 100644 --- a/domain/src/coaching_session/recurrence.rs +++ b/domain/src/coaching_session/recurrence.rs @@ -184,13 +184,9 @@ impl RecurrenceError { } } -/// Validate the structural correctness of a [`Recurrence`] rule against the series' -/// local start. Does not enforce the occurrence/span caps — those are applied in -/// [`expand_recurrence`] after generation. -fn validate_recurrence( - local_start: NaiveDateTime, - rule: &Recurrence, -) -> Result<(), RecurrenceError> { +/// The checks a rule needs to be expanded at all. Does not enforce the occurrence/span +/// caps; those are applied after generation. +fn validate_structure(rule: &Recurrence) -> Result<(), RecurrenceError> { if rule.interval < 1 { return Err(RecurrenceError::InvalidInterval); } @@ -205,9 +201,23 @@ fn validate_recurrence( if weekdays.is_empty() { return Err(RecurrenceError::EmptyByWeekdays); } - if !weekdays.contains(&local_start.weekday()) { - return Err(RecurrenceError::StartWeekdayMismatch); - } + } + Ok(()) +} + +/// Validate a rule about to be published against the series' local start: its +/// structure, plus the rules that keep a new series rendering alike in every client. +fn validate_recurrence( + local_start: NaiveDateTime, + rule: &Recurrence, +) -> Result<(), RecurrenceError> { + validate_structure(rule)?; + if rule + .by_weekdays + .as_ref() + .is_some_and(|weekdays| !weekdays.contains(&local_start.weekday())) + { + return Err(RecurrenceError::StartWeekdayMismatch); } if rule.frequency == Frequency::Monthly && local_start.day() > MAX_MONTHLY_START_DAY { return Err(RecurrenceError::MonthlyStartDayTooLate); @@ -239,7 +249,8 @@ fn offset_before_gap(zone: Tz, local: NaiveDateTime) -> Duration { Duration::seconds(seconds.into()) } -/// Expand the rule into concrete occurrence datetimes (UTC), in chronological order. +/// Expand a rule about to be published into concrete occurrence datetimes (UTC), in +/// chronological order. /// /// Steps run on local wall time in `zone`, so every slot keeps the start's local time of /// day across DST, and weekdays and month days are the ones the coach sees. `until` is @@ -258,7 +269,29 @@ pub fn expand_recurrence( if to_utc(zone, local_start) != start_at { return Err(RecurrenceError::AmbiguousStartAt); } + generate(local_start, rule, zone) +} + +/// Expand a rule that is already published, into the instances its invite produced. +/// +/// Skips the checks only a new rule must pass (start weekday, monthly start day, a start +/// in a repeated hour): a series stored before they existed still has to resolve, the +/// way a calendar resolves its `RRULE`, or it could never be resent. +pub(crate) fn expand_published( + start_at: NaiveDateTime, + rule: &Recurrence, + zone: Tz, +) -> Result, RecurrenceError> { + validate_structure(rule)?; + generate(to_local(zone, start_at), rule, zone) +} +/// Generate the slots from the local start, then enforce the occurrence and span caps. +fn generate( + local_start: NaiveDateTime, + rule: &Recurrence, + zone: Tz, +) -> Result, RecurrenceError> { let bounds = Bounds { resolve: |local| to_utc(zone, local), until: rule.until, @@ -369,8 +402,9 @@ fn generate_weekly_with_weekdays NaiveDateTime>( .collect() } -/// Monthly recurrence on the start's day of the month. Validation keeps that day at or -/// below [`MAX_MONTHLY_START_DAY`], so `checked_add_months` never has to clamp. +/// Monthly recurrence on the start's day of the month. A month without that day is +/// skipped, as RFC 5545 skips it; only a stored rule can reach one, since new rules +/// start on day [`MAX_MONTHLY_START_DAY`] or earlier. fn generate_monthly NaiveDateTime>( local_start: NaiveDateTime, interval: u32, @@ -381,6 +415,8 @@ fn generate_monthly NaiveDateTime>( i.checked_mul(interval) .and_then(|months| local_start.checked_add_months(Months::new(months))) }) + // `checked_add_months` clamps to the month's end; a clamped day is no occurrence. + .filter(|local| local.day() == local_start.day()) .map_while(|local| bounds.admit(local)) .take(bounds.limit) .collect() diff --git a/domain/src/coaching_session/recurrence_dst_tests.rs b/domain/src/coaching_session/recurrence_dst_tests.rs index 8c379019f..d181554dd 100644 --- a/domain/src/coaching_session/recurrence_dst_tests.rs +++ b/domain/src/coaching_session/recurrence_dst_tests.rs @@ -1,7 +1,6 @@ //! Expansion in the series' zone: wall time held across DST, weekdays and month days //! read locally, and the RFC 5545 rules for skipped and repeated local times. -use super::*; use chrono::{Days, NaiveDate}; use chrono_tz::America::Chicago; use chrono_tz::Asia::{Kolkata, Tokyo}; @@ -9,6 +8,8 @@ use chrono_tz::Australia::Sydney; use chrono_tz::Europe::London; use chrono_tz::UTC; +use super::*; + fn wall(y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { NaiveDate::from_ymd_opt(y, m, d) .unwrap() @@ -300,3 +301,57 @@ fn utc_expansion_is_plain_stepping() { .collect::>() ); } + +/// A stored rule from before the 29-31 rule still expands, the way a client reads it: +/// months without the start day are skipped, never clamped to the month's end. +#[test] +fn stored_late_month_rule_skips_short_months() { + let start = utc(2026, 1, 31, 10, 0); + let r = counted(Frequency::Monthly, 4); + + assert_eq!( + expand_recurrence(start, &r, UTC).unwrap_err(), + RecurrenceError::MonthlyStartDayTooLate + ); + assert_eq!( + expand_published(start, &r, UTC).unwrap(), + vec![ + utc(2026, 1, 31, 10, 0), + utc(2026, 3, 31, 10, 0), + utc(2026, 5, 31, 10, 0), + utc(2026, 7, 31, 10, 0), + ] + ); +} + +/// A stored start in the second pass of a repeated hour resolves as its `DTSTART;TZID` +/// does, to the first pass, instead of failing. +#[test] +fn stored_start_in_a_repeated_hour_resolves_to_the_first_pass() { + let second_pass = utc(2026, 11, 1, 7, 30); + let slots = expand_published(second_pass, &counted(Frequency::Weekly, 2), Chicago).unwrap(); + assert_eq!( + slots, + vec![utc(2026, 11, 1, 6, 30), utc(2026, 11, 8, 7, 30)] + ); +} + +/// A stored evening rule accepted under the old UTC weekday check, whose local start +/// day is not listed, still expands instead of failing. +#[test] +fn stored_rule_whose_local_start_weekday_is_unlisted_still_expands() { + let start = at(Chicago, 2026, 10, 22, 19, 0); + let rule = on(&[Weekday::Fri], Frequency::Weekly, 2); + + assert_eq!( + expand_recurrence(start, &rule, Chicago).unwrap_err(), + RecurrenceError::StartWeekdayMismatch + ); + assert_eq!( + expand_published(start, &rule, Chicago).unwrap(), + vec![ + at(Chicago, 2026, 10, 23, 19, 0), + at(Chicago, 2026, 10, 30, 19, 0), + ] + ); +} diff --git a/domain/src/emails_tests.rs b/domain/src/emails_tests.rs index b5d914f75..4aa59fc76 100644 --- a/domain/src/emails_tests.rs +++ b/domain/src/emails_tests.rs @@ -4405,3 +4405,53 @@ fn snapshot_across_dst_uses_the_stored_series_zone() { assert_eq!(ics.matches("RECURRENCE-ID").count(), 1, "{ics}"); assert!(!ics.contains("America/New_York"), "{ics}"); } + +/// A monthly series on the 31st, stored before new series were limited to day 28, can +/// still be resent: its snapshot skips the months without a 31st, as calendars do. +#[test] +fn snapshot_of_a_stored_late_month_series_still_builds() { + let series = coaching_session_series::Model { + rule: serde_json::json!({ + "start_at": "2026-12-31T16:00:00", + "recurrence": { "frequency": "monthly", "interval": 1, "count": 4 }, + "duration_minutes": 60, + "timezone": "America/Chicago", + }), + ..create_test_series() + }; + let rule: SeriesRule = serde_json::from_value(series.rule.clone()).unwrap(); + let slots = rule.slots().unwrap(); + assert_eq!( + slots, + vec![ + at(2026, 12, 31, 16), + at(2027, 1, 31, 16), + at(2027, 3, 31, 15), + at(2027, 5, 31, 15), + ] + ); + + let members: Vec = slots + .iter() + .map(|slot| coaching_sessions::Model { + duration_minutes: 60, + ..create_test_series_session(series.id, *slot) + }) + .collect(); + let coachee = create_test_user_with("Jane", "Doe", "jane@example.com", "UTC"); + let ics = unfold( + &build_series_snapshot_ics( + &relocated_coach(), + &coachee, + &series, + &members, + "desc".into(), + at(2026, 12, 1, 12), + ) + .unwrap(), + ); + + assert!(ics.contains("RRULE:FREQ=MONTHLY;WKST=MO;COUNT=4"), "{ics}"); + assert!(!ics.contains("EXDATE"), "{ics}"); + assert!(!ics.contains("RECURRENCE-ID"), "{ics}"); +} diff --git a/domain/src/gateway/ical_oracle_tests.rs b/domain/src/gateway/ical_oracle_tests.rs index 4604d6e93..0b35804f8 100644 --- a/domain/src/gateway/ical_oracle_tests.rs +++ b/domain/src/gateway/ical_oracle_tests.rs @@ -2,8 +2,6 @@ //! invite. An independent RFC 5545 implementation reads the exact `DTSTART` and `RRULE` //! this builder emits, and must produce the same instants `expand_recurrence` does. -use super::*; -use crate::coaching_session::{expand_recurrence, Frequency, Recurrence}; use chrono::{NaiveDate, Weekday}; use chrono_tz::America::Chicago; use chrono_tz::Asia::{Kolkata, Tokyo}; @@ -11,6 +9,10 @@ use chrono_tz::Australia::Sydney; use chrono_tz::Europe::London; use rrule::RRuleSet; +use super::*; +use crate::coaching_session::recurrence::expand_published; +use crate::coaching_session::{expand_recurrence, Frequency, Recurrence}; + fn wall(y: i32, m: u32, d: u32, h: u32, min: u32) -> NaiveDateTime { NaiveDate::from_ymd_opt(y, m, d) .unwrap() @@ -202,3 +204,38 @@ fn series_slots_equal_the_instances_a_client_expands() { ); } } + +/// Stored rules that new series can no longer have still expand to what a client holds. +#[test] +fn stored_slots_equal_the_instances_a_client_expands() { + let cases: Vec<(chrono_tz::Tz, NaiveDateTime, Recurrence)> = vec![ + ( + chrono_tz::UTC, + wall(2026, 1, 31, 10, 0), + rule(Frequency::Monthly, 6), + ), + ( + Chicago, + wall(2026, 10, 30, 19, 0), + rule(Frequency::Monthly, 6), + ), + ( + Chicago, + wall(2026, 12, 31, 9, 0), + Recurrence { + interval: 2, + ..rule(Frequency::Monthly, 3) + }, + ), + ]; + + for (zone, local_start, recurrence) in cases { + let start = at(zone, local_start); + let slots = expand_published(start, &recurrence, zone).unwrap(); + assert_eq!( + slots, + client_instances(zone, start, &recurrence), + "{zone} from {local_start}: {recurrence:?}" + ); + } +}