🐛 fix a bug where we didn't handle recurrence exceptions, causing duplicate events whenever a recurring event was moved

This commit is contained in:
_ 2025-08-12 23:25:34 +00:00
parent 3044deb89f
commit 5310f19383
2 changed files with 75 additions and 24 deletions

View file

@ -4,7 +4,9 @@ use chrono::{DateTime, TimeZone as _, Utc};
use clap::Parser as _; use clap::Parser as _;
use icalendar::{Component as _, EventLike as _}; use icalendar::{Component as _, EventLike as _};
use serde::Deserialize; use serde::Deserialize;
use std::{io::Write as _, path::PathBuf, str::FromStr as _, time::Duration}; use std::{
collections::BTreeSet, io::Write as _, path::PathBuf, str::FromStr as _, time::Duration,
};
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
@ -294,6 +296,13 @@ struct ICal {
cal: icalendar::Calendar, cal: icalendar::Calendar,
} }
/// Used to link recurrence exceptions to the original events they replace
#[derive(Eq, Ord, PartialOrd, PartialEq)]
struct RecurrenceKey<'a> {
recurrence_id: DatePerhapsTime,
uid: &'a str,
}
impl ICal { impl ICal {
fn read_from_str(s: &str) -> Result<Self> { fn read_from_str(s: &str) -> Result<Self> {
let cal = s.parse().map_err(|s| anyhow!("parse error {s}"))?; let cal = s.parse().map_err(|s| anyhow!("parse error {s}"))?;
@ -316,8 +325,11 @@ impl ICal {
}) })
} }
/// Returns an unsorted list of event instances for this calendar
fn event_instances(&self, params: &Parameters) -> Result<Vec<EventInstance<'_>>> { fn event_instances(&self, params: &Parameters) -> Result<Vec<EventInstance<'_>>> {
let mut instances = vec![]; let mut instances = vec![];
let mut recurrence_exceptions = BTreeSet::new();
for ev in self.events() { for ev in self.events() {
let eis = match event_instances(params, ev) let eis = match event_instances(params, ev)
.with_context(|| format!("Failed to process event with UID '{:?}'", ev.get_uid())) .with_context(|| format!("Failed to process event with UID '{:?}'", ev.get_uid()))
@ -327,8 +339,7 @@ impl ICal {
if ev.get_last_modified().context("Event has no timestamp")? if ev.get_last_modified().context("Event has no timestamp")?
< params.ignore_before < params.ignore_before
{ {
// FIXME: Use tracing tracing::warn!("Ignoring error from very old event {e:?}");
eprintln!("Ignoring error from very old event {e:?}");
continue; continue;
} else { } else {
Err(e)? Err(e)?
@ -338,8 +349,42 @@ impl ICal {
for ei in eis { for ei in eis {
instances.push(ei); instances.push(ei);
} }
if let Some(recurrence_id) = ev.get_recurrence_id() {
// This is a recurrence exception and we must handle it specially by later deleting the original event it replaces
let recurrence_id = normalize_date_perhaps_time(&recurrence_id, params.tz)
.context("We should be able to normalize recurrence IDs")?;
let uid = ev
.get_uid()
.context("Every recurrence exception should have a UID")?;
recurrence_exceptions.insert(RecurrenceKey { recurrence_id, uid });
}
} }
// Find all recurring events that are replaced with recurrence exceptions and delete the originals.
// There is probably a not-linear-time way to do this, but this should be fine.
instances.retain(|ev| {
if ev.ev.get_recurrence_id().is_some() {
// This is a recurrence exception, exceptions never delete themselves
return true;
}
let uid = ev
.ev
.get_uid()
.context(
"Every recurring event should have a UID so we can apply recurrence exceptions",
)
.unwrap();
let key = RecurrenceKey {
recurrence_id: ev.dtstart,
uid,
};
!recurrence_exceptions.contains(&key)
});
Ok(instances) Ok(instances)
} }
} }

View file

@ -147,28 +147,34 @@ END:VCALENDAR
tz: chrono_tz::America::Chicago, tz: chrono_tz::America::Chicago,
}; };
let instances = ical.event_instances(&params)?; let instances = ical.event_instances(&params)?;
assert_eq!(
[
instances[0].dtstart,
instances[1].dtstart,
instances[2].dtstart,
],
[
DatePerhapsTime {
dt: chicago_time(2025, 7, 8, 18, 0, 0),
all_day: false,
},
DatePerhapsTime {
dt: chicago_time(2025, 9, 9, 18, 0, 0),
all_day: false,
},
DatePerhapsTime {
dt: chicago_time(2025, 8, 14, 18, 0, 0),
all_day: false,
},
]
);
for instance in &instances {
assert_eq!(instance.ev.get_summary(), Some("coil perm brush zippy"));
}
assert_eq!(instances.len(), 3); assert_eq!(instances.len(), 3);
let expected_time = DatePerhapsTime {
dt: chicago_time(2025, 7, 8, 18, 0, 0),
all_day: false,
};
assert_eq!(instances[0].dtstart, expected_time);
assert_eq!(instances[0].ev.get_summary(), Some("coil perm brush zippy"));
let expected_time = DatePerhapsTime {
dt: chicago_time(2025, 8, 14, 18, 0, 0),
all_day: false,
};
assert_eq!(instances[1].dtstart, expected_time);
assert_eq!(instances[1].ev.get_summary(), Some("coil perm brush zippy"));
let expected_time = DatePerhapsTime {
dt: chicago_time(2025, 9, 9, 18, 0, 0),
all_day: false,
};
assert_eq!(instances[2].dtstart, expected_time);
assert_eq!(instances[2].ev.get_summary(), Some("coil perm brush zippy"));
Ok(()) Ok(())
} }