diff --git a/src/ical/builder.rs b/src/ical/builder.rs index 59a3bd0..11ca71d 100644 --- a/src/ical/builder.rs +++ b/src/ical/builder.rs @@ -7,6 +7,7 @@ use ics::properties::{Completed, Created, LastModified, PercentComplete, Status, use ics::{ICalendar, ToDo}; use crate::item::Item; +use crate::task::CompletionStatus; use crate::settings::{ORG_NAME, PRODUCT_NAME}; fn ical_product_id() -> String { @@ -30,14 +31,17 @@ pub fn build_from(item: &Item) -> Result> { match item { Item::Task(t) => { - if t.completed() { - todo.push(PercentComplete::new("100")); - t.completion_date().map(|dt| todo.push( - Completed::new(format_date_time(dt)) - )); - todo.push(Status::completed()); - } else { - todo.push(Status::needs_action()); + match t.completion_status() { + CompletionStatus::Uncompleted => { + todo.push(Status::needs_action()); + }, + CompletionStatus::Completed(completion_date) => { + todo.push(PercentComplete::new("100")); + completion_date.as_ref().map(|dt| todo.push( + Completed::new(format_date_time(dt)) + )); + todo.push(Status::completed()); + } } }, _ => { @@ -108,12 +112,9 @@ mod tests { let now = Utc::now(); let s_now = format_date_time(&now); - let mut task = Item::Task(Task::new( + let task = Item::Task(Task::new( String::from("This is a task with ÜTF-8 characters"), completed, &cal_id )); - if completed { - task.unwrap_task_mut().set_completed_on(Some(now)); - } let ical = build_from(&task).unwrap(); (s_now, task.uid().to_string(), ical) diff --git a/src/ical/parser.rs b/src/ical/parser.rs index ca4fc1a..93be645 100644 --- a/src/ical/parser.rs +++ b/src/ical/parser.rs @@ -9,6 +9,7 @@ use crate::Item; use crate::item::SyncStatus; use crate::item::ItemId; use crate::Task; +use crate::task::CompletionStatus; use crate::Event; @@ -53,13 +54,15 @@ pub fn parse(content: &str, item_id: ItemId, sync_status: SyncStatus) -> Result< uid = prop.value.clone(); } if prop.name == "DTSTAMP" { - // this property specifies the date and time that the information associated with - // the calendar component was last revised in the calendar store. + // The property can be specified once, but is not mandatory + // "This property specifies the date and time that the information associated with + // the calendar component was last revised in the calendar store." last_modified = parse_date_time_from_property(&prop.value) } if prop.name == "COMPLETED" { - // this property specifies the date and time that the information associated with - // the calendar component was last revised in the calendar store. + // The property can be specified once, but is not mandatory + // "This property defines the date and time that a to-do was + // actually completed." completion_date = parse_date_time_from_property(&prop.value) } if prop.name == "CREATED" { @@ -79,13 +82,17 @@ pub fn parse(content: &str, item_id: ItemId, sync_status: SyncStatus) -> Result< Some(dt) => dt, None => return Err(format!("Missing DTSTAMP for item {}, but this is required by RFC5545", item_id).into()), }; - if completion_date.is_none() && completed || - completion_date.is_some() && !completed - { - log::warn!("Inconsistant iCal data: completion date is {:?} but completion status is {:?}", completion_date, completed); - } + let completion_status = match completed { + false => { + if completion_date.is_some() { + log::warn!("Task {:?} has an inconsistent content: its STATUS is not completed, yet it has a COMPLETED timestamp at {:?}", uid, completion_date); + } + CompletionStatus::Uncompleted + }, + true => CompletionStatus::Completed(completion_date), + }; - Item::Task(Task::new_with_parameters(name, uid, item_id, sync_status, creation_date, last_modified, completion_date)) + Item::Task(Task::new_with_parameters(name, uid, item_id, completion_status, sync_status, creation_date, last_modified)) }, }; @@ -160,7 +167,7 @@ END:VTODO END:VCALENDAR "#; - const EXAMPLE_ICAL_COMPLETED: &str = r#"BEGIN:VCALENDAR +const EXAMPLE_ICAL_COMPLETED: &str = r#"BEGIN:VCALENDAR VERSION:2.0 PRODID:-//Nextcloud Tasks v0.13.6 BEGIN:VTODO @@ -174,6 +181,20 @@ COMPLETED:20210402T081557 STATUS:COMPLETED END:VTODO END:VCALENDAR +"#; + +const EXAMPLE_ICAL_COMPLETED_WITHOUT_A_COMPLETION_DATE: &str = r#"BEGIN:VCALENDAR +VERSION:2.0 +PRODID:-//Nextcloud Tasks v0.13.6 +BEGIN:VTODO +UID:19960401T080045Z-4000F192713-0052@example.com +CREATED:20210321T001600 +LAST-MODIFIED:20210402T081557 +DTSTAMP:20210402T081557 +SUMMARY:Clean up your room or Mom will be angry +STATUS:COMPLETED +END:VTODO +END:VCALENDAR "#; const EXAMPLE_MULTIPLE_ICAL: &str = r#"BEGIN:VCALENDAR @@ -214,12 +235,13 @@ END:VCALENDAR assert_eq!(task.id(), &item_id); assert_eq!(task.uid(), "0633de27-8c32-42be-bcb8-63bc879c6185@some-domain.com"); assert_eq!(task.completed(), false); + assert_eq!(task.completion_status(), &CompletionStatus::Uncompleted); assert_eq!(task.sync_status(), &sync_status); assert_eq!(task.last_modified(), &Utc.ymd(2021, 03, 21).and_hms(0, 16, 0)); } #[test] - fn test_ical_completion_parsing() { + fn test_completed_ical_parsing() { let version_tag = VersionTag::from(String::from("test-tag")); let sync_status = SyncStatus::Synced(version_tag); let item_id: ItemId = "http://some.id/for/testing".parse().unwrap(); @@ -228,6 +250,20 @@ END:VCALENDAR let task = item.unwrap_task(); assert_eq!(task.completed(), true); + assert_eq!(task.completion_status(), &CompletionStatus::Completed(Some(Utc.ymd(2021, 04, 02).and_hms(8, 15, 57)))); + } + + #[test] + fn test_completed_without_date_ical_parsing() { + let version_tag = VersionTag::from(String::from("test-tag")); + let sync_status = SyncStatus::Synced(version_tag); + let item_id: ItemId = "http://some.id/for/testing".parse().unwrap(); + + let item = parse(EXAMPLE_ICAL_COMPLETED_WITHOUT_A_COMPLETION_DATE, item_id.clone(), sync_status.clone()).unwrap(); + let task = item.unwrap_task(); + + assert_eq!(task.completed(), true); + assert_eq!(task.completion_status(), &CompletionStatus::Completed(None)); } #[test] diff --git a/src/lib.rs b/src/lib.rs index efb6525..a5d0697 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,7 @@ pub use item::Item; pub use item::ItemId; pub use item::VersionTag; pub use item::SyncStatus; -mod task; +pub mod task; pub use task::Task; mod event; pub use event::Event; diff --git a/src/task.rs b/src/task.rs index 275df87..d5d6ddb 100644 --- a/src/task.rs +++ b/src/task.rs @@ -6,6 +6,26 @@ use crate::item::ItemId; use crate::item::SyncStatus; use crate::calendar::CalendarId; +/// RFC5545 defines the completion as several optional fields, yet some combinations make no sense. +/// This enum provides an API that forbids such impossible combinations. +/// +/// * `COMPLETED` is an optional timestamp that tells whether this task is completed +/// * `STATUS` is an optional field, that can be set to `NEEDS-ACTION`, `COMPLETED`, or others. +/// Even though having a `COMPLETED` date but a `STATUS:NEEDS-ACTION` is theorically possible, it obviously makes no sense. This API ensures this cannot happen +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +pub enum CompletionStatus { + Completed(Option>), + Uncompleted, +} +impl CompletionStatus { + pub fn is_completed(&self) -> bool { + match self { + CompletionStatus::Completed(_) => true, + _ => false, + } + } +} + /// A to-do task #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Task { @@ -23,13 +43,15 @@ pub struct Task { creation_date: Option>, /// The last time this item was modified last_modified: DateTime, + /// The completion status of this task + completion_status: CompletionStatus, /// The display name of the task name: String, - /// The time it was completed. Set to None if this task has not been completed - completion_date: Option>, + } + impl Task { /// Create a brand new Task that is not on a server yet. /// This will pick a new (random) task ID. @@ -39,20 +61,23 @@ impl Task { let new_uid = Uuid::new_v4().to_hyphenated().to_string(); let new_creation_date = Some(Utc::now()); let new_last_modified = Utc::now(); - let new_completion_date = if completed { Some(Utc::now()) } else { None }; - Self::new_with_parameters(name, new_uid, new_item_id, new_sync_status, new_creation_date, new_last_modified, new_completion_date) + let new_completion_status = if completed { + CompletionStatus::Completed(Some(Utc::now())) + } else { CompletionStatus::Uncompleted }; + Self::new_with_parameters(name, new_uid, new_item_id, new_completion_status, new_sync_status, new_creation_date, new_last_modified) } /// Create a new Task instance, that may be synced already pub fn new_with_parameters(name: String, uid: String, id: ItemId, - sync_status: SyncStatus, creation_date: Option>, last_modified: DateTime, completion_date: Option>) -> Self + completion_status: CompletionStatus, + sync_status: SyncStatus, creation_date: Option>, last_modified: DateTime) -> Self { Self { id, uid, name, + completion_status, sync_status, - completion_date, creation_date, last_modified, } @@ -61,16 +86,16 @@ impl Task { pub fn id(&self) -> &ItemId { &self.id } pub fn uid(&self) -> &str { &self.uid } pub fn name(&self) -> &str { &self.name } - pub fn completed(&self) -> bool { self.completion_date.is_some() } + pub fn completed(&self) -> bool { self.completion_status.is_completed() } pub fn sync_status(&self) -> &SyncStatus { &self.sync_status } pub fn last_modified(&self) -> &DateTime { &self.last_modified } pub fn creation_date(&self) -> Option<&DateTime> { self.creation_date.as_ref() } - pub fn completion_date(&self) -> Option<&DateTime> { self.completion_date.as_ref() } + pub fn completion_status(&self) -> &CompletionStatus { &self.completion_status } pub fn has_same_observable_content_as(&self, other: &Task) -> bool { self.id == other.id && self.name == other.name - && self.completion_date == other.completion_date + && self.completion_status == other.completion_status && self.last_modified == other.last_modified // sync status must be the same variant, but we ignore its embedded version tag && std::mem::discriminant(&self.sync_status) == std::mem::discriminant(&other.sync_status) @@ -113,16 +138,16 @@ impl Task { self.name = new_name; } - /// Set the completion date (or pass None to un-complete the task) - pub fn set_completed_on(&mut self, new_completion_date: Option>) { + /// Set the completion status + pub fn set_completion_status(&mut self, new_completion_status: CompletionStatus) { self.update_sync_status(); self.update_last_modified(); - self.completion_date = new_completion_date; + self.completion_status = new_completion_status; } #[cfg(feature = "local_calendar_mocks_remote_calendars")] /// Set the completion status, but forces a "master" SyncStatus, just like CalDAV servers are always "masters" - pub fn mock_remote_calendar_set_completed_on(&mut self, nnew_completion_date: Option>) { + pub fn mock_remote_calendar_set_completion_status(&mut self, new_completion_status: CompletionStatus) { self.sync_status = SyncStatus::random_synced(); - self.completion_date = new_completion_date; + self.completion_status = new_completion_status; } } diff --git a/tests/scenarii.rs b/tests/scenarii.rs index a91912e..ab1e77f 100644 --- a/tests/scenarii.rs +++ b/tests/scenarii.rs @@ -12,6 +12,8 @@ use std::path::PathBuf; use std::sync::{Arc, Mutex}; use std::error::Error; +use chrono::Utc; + use my_tasks::calendar::CalendarId; use my_tasks::calendar::SupportedComponents; use my_tasks::traits::CalDavSource; @@ -23,6 +25,7 @@ use my_tasks::Item; use my_tasks::ItemId; use my_tasks::SyncStatus; use my_tasks::Task; +use my_tasks::task::CompletionStatus; use my_tasks::calendar::cached_calendar::CachedCalendar; use my_tasks::Provider; use my_tasks::mock_behaviour::MockBehaviour; @@ -373,7 +376,11 @@ pub fn scenarii_basic() -> Vec { initial_state: LocatedState::None, local_changes_to_apply: Vec::new(), remote_changes_to_apply: vec![ChangeToApply::Create(third_cal.clone(), Item::Task( - Task::new_with_parameters(String::from("Task Q, created on the server"), false, id_q, SyncStatus::random_synced() ) + Task::new_with_parameters( + String::from("Task Q, created on the server"), + id_q.to_string(), id_q, + CompletionStatus::Uncompleted, + SyncStatus::random_synced(), Some(Utc::now()), Utc::now() ) ))], after_sync: LocatedState::BothSynced( ItemState{ calendar: third_cal.clone(), @@ -389,7 +396,11 @@ pub fn scenarii_basic() -> Vec { id: id_r.clone(), initial_state: LocatedState::None, local_changes_to_apply: vec![ChangeToApply::Create(third_cal.clone(), Item::Task( - Task::new_with_parameters(String::from("Task R, created locally"), false, id_r, SyncStatus::NotSynced ) + Task::new_with_parameters( + String::from("Task R, created locally"), + id_r.to_string(), id_r, + CompletionStatus::Uncompleted, + SyncStatus::NotSynced, Some(Utc::now()), Utc::now() ) ))], remote_changes_to_apply: Vec::new(), after_sync: LocatedState::BothSynced( ItemState{ @@ -563,7 +574,11 @@ pub fn scenarii_transient_task() -> Vec { initial_state: LocatedState::None, local_changes_to_apply: vec![ ChangeToApply::Create(cal, Item::Task( - Task::new_with_parameters(String::from("A transient task that will be deleted before the sync"), false, id_transient, SyncStatus::NotSynced ) + Task::new_with_parameters( + String::from("A transient task that will be deleted before the sync"), + id_transient.to_string(), id_transient, + CompletionStatus::Uncompleted, + SyncStatus::NotSynced, Some(Utc::now()), Utc::now() ) )), ChangeToApply::Rename(String::from("A new name")), @@ -612,12 +627,21 @@ async fn populate_test_provider(scenarii: &[ItemScenario], mock_behaviour: Arc (s, SyncStatus::random_synced()), }; + let now = Utc::now(); + let completion_status = match state.completed { + false => CompletionStatus::Uncompleted, + true => CompletionStatus::Completed(Some(now)), + }; + let new_item = Item::Task( Task::new_with_parameters( state.name.clone(), - state.completed, + item.id.to_string(), item.id.clone(), + completion_status, sync_status, + Some(now), + now, )); match required_state { @@ -713,10 +737,14 @@ where } }, ChangeToApply::SetCompletion(new_status) => { + let completion_status = match new_status { + false => CompletionStatus::Uncompleted, + true => CompletionStatus::Completed(Some(Utc::now())), + }; if is_remote { - task.mock_remote_calendar_set_completed(new_status.clone()); + task.mock_remote_calendar_set_completion_status(completion_status); } else { - task.set_completed(new_status.clone()); + task.set_completion_status(completion_status); } }, ChangeToApply::Remove => {