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/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. 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..7fb9a4594 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -182,13 +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)? + 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 a2008d1f7..eb40e19aa 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,9 @@ 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> { +/// 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); } @@ -185,201 +201,236 @@ fn validate_recurrence(start_at: NaiveDateTime, rule: &Recurrence) -> Result<(), if weekdays.is_empty() { return Err(RecurrenceError::EmptyByWeekdays); } - if !weekdays.contains(&start_at.weekday()) { - return Err(RecurrenceError::StartWeekdayMismatch); - } } Ok(()) } -/// Expand the rule into concrete occurrence datetimes, in chronological order. +/// 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); + } + Ok(()) +} + +/// 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 a rule about to be published 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; + 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); + } + generate(local_start, rule, zone) +} - // Generation cap: one past the limit so the outer check below sees overflow. - let gen_cap = MAX_RECURRING_OCCURRENCES + 1; +/// 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) +} - let week_interval = match rule.frequency { - Frequency::Biweekly => 2 * rule.interval, - _ => rule.interval, +/// 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, + // 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. 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, - 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))) + }) + // `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() } +#[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 +625,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 +647,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 +668,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 +685,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 +706,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 +730,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 +755,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 +779,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 +803,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 +814,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 +824,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 +851,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 +866,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 +883,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 +897,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..d181554dd --- /dev/null +++ b/domain/src/coaching_session/recurrence_dst_tests.rs @@ -0,0 +1,357 @@ +//! 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 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; + +use super::*; + +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::>() + ); +} + +/// 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/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..4aa59fc76 100644 --- a/domain/src/emails_tests.rs +++ b/domain/src/emails_tests.rs @@ -2,6 +2,8 @@ use super::*; use crate::{coaching_sessions, organizations, users, Id}; use base64::engine::general_purpose::STANDARD; use base64::Engine; +#[cfg(feature = "mock")] +use chrono::DurationRound; use chrono::NaiveDate; use mockito::{Server, ServerGuard}; #[cfg(feature = "mock")] @@ -821,6 +823,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 +1191,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 +1442,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 +1503,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 +1546,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 +1589,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 +1664,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 +1953,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 +2383,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 +2478,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 +2566,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 +4174,284 @@ 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}"); +} + +/// 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.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..0b35804f8 --- /dev/null +++ b/domain/src/gateway/ical_oracle_tests.rs @@ -0,0 +1,241 @@ +//! 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 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; + +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() + .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:?}" + ); + } +} + +/// 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:?}" + ); + } +} 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(()) + } +}