From fafd1734eb83cb17e14fedc97f7b97dee0427681 Mon Sep 17 00:00:00 2001 From: simonsan <14062932+simonsan@users.noreply.github.com> Date: Sun, 18 Feb 2024 15:51:49 +0100 Subject: [PATCH] WIP Signed-off-by: simonsan <14062932+simonsan@users.noreply.github.com> --- Cargo.lock | 31 ++++++++ Cargo.toml | 1 + TESTING.md | 19 +++++ crates/cli/src/setup.rs | 4 +- crates/core/src/config.rs | 78 +++++++++++---------- crates/core/src/domain/activity.rs | 23 +++--- crates/core/src/domain/category.rs | 17 +++-- crates/core/src/domain/priority.rs | 3 +- crates/core/src/domain/project.rs | 16 +++-- crates/core/src/domain/tag.rs | 16 +++-- crates/core/src/domain/task.rs | 12 +++- crates/core/src/error.rs | 8 +-- crates/core/src/lib.rs | 4 +- crates/core/src/service/activity_store.rs | 28 ++++---- crates/core/src/service/activity_tracker.rs | 6 +- crates/core/src/storage.rs | 30 ++++---- crates/core/src/storage/file.rs | 20 +++--- crates/core/src/storage/in_memory.rs | 39 ++++++----- crates/core/tests/activity_store.rs | 24 +++---- src/commands.rs | 24 +++---- src/commands/begin.rs | 73 ++++++++++++++++--- src/commands/craft.rs | 1 + src/commands/end.rs | 2 +- src/commands/hold.rs | 1 + src/commands/now.rs | 1 + tests/cli.rs | 6 +- 26 files changed, 323 insertions(+), 164 deletions(-) create mode 100644 TESTING.md diff --git a/Cargo.lock b/Cargo.lock index 1d4977d6..0c182a7c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -774,6 +774,21 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "insta" +version = "1.34.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d64600be34b2fcfc267740a243fa7744441bb4947a619ac4e5bb6507f35fbfc" +dependencies = [ + "console", + "lazy_static", + "linked-hash-map", + "serde", + "similar", + "toml 0.5.11", + "yaml-rust", +] + [[package]] name = "itertools" version = "0.12.1" @@ -826,6 +841,12 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "linked-hash-map" +version = "0.5.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" + [[package]] name = "linux-raw-sys" version = "0.4.13" @@ -976,6 +997,7 @@ dependencies = [ "directories", "eyre", "human-panic", + "insta", "once_cell", "pace_cli", "pace_core", @@ -2039,6 +2061,15 @@ dependencies = [ "glob", ] +[[package]] +name = "yaml-rust" +version = "0.4.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56c1936c4cc7a1c9ab21a1ebb602eb942ba868cbd44a99cb7cdc5892335e1c85" +dependencies = [ + "linked-hash-map", +] + [[package]] name = "zerocopy" version = "0.7.32" diff --git a/Cargo.toml b/Cargo.toml index 4f0534ec..16fa83f5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ dialoguer = { version = "0.11.0", features = ["history", "fuzzy-select"] } directories = "5.0.1" eyre = { workspace = true } human-panic = "1.2.3" +insta = { version = "1.34.0", features = ["toml"] } pace_cli = { workspace = true } pace_core = { workspace = true } serde = "1" diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 00000000..58fd7f11 --- /dev/null +++ b/TESTING.md @@ -0,0 +1,19 @@ +# Testing strategy + +## Unit tests + +- TODO + +## Snapshot tests + +- Use toml snapshots for checking if the formatting is right and not the weird + double table thing + +## Property-based tests + +- Implement Arbitrary for all serializable, storage-related types + +## Integration tests + +- CLI +- API diff --git a/crates/cli/src/setup.rs b/crates/cli/src/setup.rs index 9830743c..81185c1f 100644 --- a/crates/cli/src/setup.rs +++ b/crates/cli/src/setup.rs @@ -306,7 +306,7 @@ pub(crate) fn confirmation_or_break(prompt: &str) -> Result<()> { /// /// Returns `Ok(())` if the setup assistant succeeds pub fn craft_setup(term: &Term) -> Result<()> { - let default_config_content = PaceConfig::default(); + let mut config = PaceConfig::default(); let config_paths = get_config_paths("pace.toml") .into_iter() @@ -328,7 +328,7 @@ pub fn craft_setup(term: &Term) -> Result<()> { let final_paths = prompt_activity_log_path(&activity_log_paths)?; - let config = default_config_content.with_activity_log(final_paths.activity_log_path()); + config.add_activity_log_path(final_paths.activity_log_path()); let final_paths = prompt_config_file_path(final_paths, config_paths.as_slice())?; diff --git a/crates/core/src/config.rs b/crates/core/src/config.rs index fcae4c1b..18e930dc 100644 --- a/crates/core/src/config.rs +++ b/crates/core/src/config.rs @@ -8,7 +8,10 @@ use serde_derive::{Deserialize, Serialize}; use directories::ProjectDirs; -use crate::error::{PaceErrorKind, PaceResult}; +use crate::{ + domain::priority::ItemPriority, + error::{PaceErrorKind, PaceResult}, +}; /// The pace configuration file /// @@ -45,12 +48,8 @@ impl PaceConfig { /// # Arguments /// /// `activity_log` - The path to the activity log file - #[must_use] - pub fn with_activity_log(self, activity_log: impl AsRef) -> Self { - let mut new_config = self; - new_config.general.activity_log_file_path = - activity_log.as_ref().to_string_lossy().to_string(); - new_config + pub fn add_activity_log_path(&mut self, activity_log: impl AsRef) { + self.general.activity_log_file_path = activity_log.as_ref().to_string_lossy().to_string(); } } @@ -58,47 +57,54 @@ impl PaceConfig { #[derive(Debug, Deserialize, Default, Serialize, Getters, MutGetters, Clone)] #[getset(get = "pub")] pub struct GeneralConfig { - /// The storage type for the activity log - log_storage: String, - /// The path to the activity log file + /// Default is operating system dependent + /// Use `pace craft setup` to set this value initially #[getset(get = "pub", get_mut = "pub")] activity_log_file_path: String, - /// The format for the activity log - log_format: String, - - /// If IDs should be autogenerated for activities + /// If IDs should be autogenerated for activities, otherwise it's a hard error + /// Default: `true` autogenerate_ids: bool, /// The default category separator + /// Default: `::` category_separator: String, - /// The default category - default_priority: String, + /// The default priority + /// Default: `medium` + default_priority: ItemPriority, + + /// The format for the activity log + /// Default: `toml` + log_format: String, + + /// The storage type for the activity log + /// Default: `file` + log_storage: String, } /// The review configuration for the pace application #[derive(Debug, Deserialize, Default, Serialize, Getters, Clone)] #[getset(get = "pub")] pub struct ReviewConfig { - /// The format for the review - review_format: String, - /// The directory to store the review files review_directory: String, + + /// The format for the review + review_format: String, } /// The export configuration for the pace application #[derive(Debug, Deserialize, Default, Serialize, Getters, Clone)] #[getset(get = "pub")] pub struct ExportConfig { - /// If the export should include tags - export_include_tags: bool, - /// If the export should include descriptions export_include_descriptions: bool, + /// If the export should include tags + export_include_tags: bool, + /// The time format within the export export_time_format: String, } @@ -107,21 +113,18 @@ pub struct ExportConfig { #[derive(Debug, Deserialize, Default, Serialize, Getters, Clone)] #[getset(get = "pub")] pub struct DatabaseConfig { + /// The connection string for the database + connection_string: String, // `type` is a reserved keyword in Rust + /// The type of database #[serde(rename = "type")] - db_type: String, // `type` is a reserved keyword in Rust - - /// The connection string for the database - connection_string: String, + db_type: String, } /// The pomodoro configuration for the pace application #[derive(Debug, Deserialize, Default, Serialize, Getters, Clone, Copy)] #[getset(get = "pub")] pub struct PomodoroConfig { - /// The duration of a work session in minutes - work_duration_minutes: u32, - /// The duration of a short break in minutes break_duration_minutes: u32, @@ -130,34 +133,37 @@ pub struct PomodoroConfig { /// The number of work sessions before a long break sessions_before_long_break: u32, + + /// The duration of a work session in minutes + work_duration_minutes: u32, } /// The inbox configuration for the pace application #[derive(Debug, Deserialize, Default, Serialize, Getters, Clone)] #[getset(get = "pub")] pub struct InboxConfig { - /// The maximum items the inbox should hold - max_size: u32, + /// The default time to auto-archive items in the inbox (in days) + auto_archive_after_days: u32, /// The default category for items in the inbox default_priority: String, - /// The default time to auto-archive items in the inbox (in days) - auto_archive_after_days: u32, + /// The maximum items the inbox should hold + max_size: u32, } /// The auto-archival configuration for the pace application #[derive(Debug, Deserialize, Default, Serialize, Getters, Clone)] #[getset(get = "pub")] pub struct AutoArchivalConfig { - /// If auto-archival is enabled - enabled: bool, - /// The default auto-archival time after which items should be archived (in days) archive_after_days: u32, /// The path to the archive file archive_path: String, + + /// If auto-archival is enabled + enabled: bool, } /// Get the current directory and then search upwards in the directory hierarchy for a file name diff --git a/crates/core/src/domain/activity.rs b/crates/core/src/domain/activity.rs index f2d51490..64a7bb8e 100644 --- a/crates/core/src/domain/activity.rs +++ b/crates/core/src/domain/activity.rs @@ -60,26 +60,30 @@ enum PomodoroCycle { #[derive(Merge)] pub struct Activity { /// The activity's unique identifier - #[builder(default = Some(ActivityId::default()), setter(strip_option))] + #[builder(default = Some(ActivityGuid::default()), setter(strip_option))] #[getset(get_copy, get_mut = "pub")] - id: Option, + #[serde(rename = "id", skip_serializing_if = "Option::is_none")] + guid: Option, /// The category of the activity // TODO: We had it as a struct before with an ID, but it's questionable if we should go for this // TODO: Reconsider when we implement the project management part // category: Category, #[builder(default)] + #[serde(skip_serializing_if = "Option::is_none")] category: Option, /// The description of the activity // This needs to be an Optional, because we use the whole activity struct // as well for intermissions, which don't have a description #[builder(default, setter(strip_option))] + #[serde(skip_serializing_if = "Option::is_none")] description: Option, /// The end date and time of the activity #[builder(default, setter(strip_option))] #[getset(get = "pub", get_mut = "pub")] + #[serde(skip_serializing_if = "Option::is_none")] end: Option, /// The start date and time of the activity @@ -91,6 +95,7 @@ pub struct Activity { /// The duration of the activity #[builder(default, setter(strip_option))] #[getset(get = "pub", get_mut = "pub")] + #[serde(skip_serializing_if = "Option::is_none")] duration: Option, /// The kind of activity @@ -110,18 +115,20 @@ pub struct Activity { // Pomodoro-specific attributes /// The pomodoro cycle of the activity #[builder(default, setter(strip_option))] + #[serde(skip_serializing_if = "Option::is_none")] pomodoro_cycle: Option, // Intermission-specific attributes /// The intermission periods of the activity #[builder(default, setter(strip_option))] + #[serde(skip_serializing_if = "Option::is_none")] intermission_periods: Option>, } impl Default for Activity { fn default() -> Self { Self { - id: Some(ActivityId::default()), + guid: Some(ActivityGuid::default()), category: Some("Uncategorized".to_string()), description: Some("This is an example activity".to_string()), end: None, @@ -136,15 +143,15 @@ impl Default for Activity { /// The unique identifier of an activity #[derive(Debug, Clone, Serialize, Deserialize, Ord, PartialEq, PartialOrd, Eq, Copy, Hash)] -pub struct ActivityId(Ulid); +pub struct ActivityGuid(Ulid); -impl Display for ActivityId { +impl Display for ActivityGuid { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { self.0.fmt(f) } } -impl Default for ActivityId { +impl Default for ActivityGuid { fn default() -> Self { Self(Ulid::new()) } @@ -168,14 +175,14 @@ impl Display for Activity { } } -impl rusqlite::types::FromSql for ActivityId { +impl rusqlite::types::FromSql for ActivityGuid { fn column_result(value: rusqlite::types::ValueRef<'_>) -> rusqlite::types::FromSqlResult { let bytes = <[u8; 16]>::column_result(value)?; Ok(Self(Ulid::from(u128::from_be_bytes(bytes)))) } } -impl rusqlite::types::ToSql for ActivityId { +impl rusqlite::types::ToSql for ActivityGuid { fn to_sql(&self) -> rusqlite::Result> { Ok(rusqlite::types::ToSqlOutput::from(self.0.to_string())) } diff --git a/crates/core/src/domain/category.rs b/crates/core/src/domain/category.rs index fa3a83c8..a9a011ff 100644 --- a/crates/core/src/domain/category.rs +++ b/crates/core/src/domain/category.rs @@ -9,11 +9,13 @@ use ulid::Ulid; pub struct Category { /// The category description #[builder(default, setter(strip_option))] + #[serde(skip_serializing_if = "Option::is_none")] description: Option, /// The category id - #[builder(default = Some(CategoryId::default()), setter(strip_option))] - id: Option, + #[builder(default = Some(CategoryGuid::default()), setter(strip_option))] + #[serde(rename = "id")] + guid: Option, /// The category name name: String, @@ -21,6 +23,7 @@ pub struct Category { /// The category's subcategories // TODO: Add support for subcategories #[builder(default, setter(strip_option))] + #[serde(skip_serializing_if = "Option::is_none")] subcategories: Option>, } @@ -55,9 +58,9 @@ pub fn extract_categories(category_string: &str, separator: &str) -> (Category, /// The category id #[derive(Debug, Serialize, Deserialize, Clone, Copy)] -pub struct CategoryId(Ulid); +pub struct CategoryGuid(Ulid); -impl Default for CategoryId { +impl Default for CategoryGuid { fn default() -> Self { Self(Ulid::new()) } @@ -66,7 +69,7 @@ impl Default for CategoryId { impl Default for Category { fn default() -> Self { Self { - id: Some(CategoryId::default()), + guid: Some(CategoryGuid::default()), name: "Uncategorized".to_string(), description: Some("Uncategorized category".to_string()), subcategories: Option::default(), @@ -74,14 +77,14 @@ impl Default for Category { } } -impl rusqlite::types::FromSql for CategoryId { +impl rusqlite::types::FromSql for CategoryGuid { fn column_result(value: rusqlite::types::ValueRef<'_>) -> rusqlite::types::FromSqlResult { let bytes = <[u8; 16]>::column_result(value)?; Ok(Self(Ulid::from(u128::from_be_bytes(bytes)))) } } -impl rusqlite::types::ToSql for CategoryId { +impl rusqlite::types::ToSql for CategoryGuid { fn to_sql(&self) -> rusqlite::Result> { Ok(rusqlite::types::ToSqlOutput::from(self.0.to_string())) } diff --git a/crates/core/src/domain/priority.rs b/crates/core/src/domain/priority.rs index cce5a243..9d563267 100644 --- a/crates/core/src/domain/priority.rs +++ b/crates/core/src/domain/priority.rs @@ -1,8 +1,9 @@ use serde_derive::{Deserialize, Serialize}; -#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] +#[derive(Debug, Serialize, Default, Deserialize, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] pub enum ItemPriority { High, + #[default] Medium, Low, } diff --git a/crates/core/src/domain/project.rs b/crates/core/src/domain/project.rs index 5c065fe2..b8204810 100644 --- a/crates/core/src/domain/project.rs +++ b/crates/core/src/domain/project.rs @@ -11,22 +11,22 @@ pub struct ProjectConfig { } #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub struct ProjectId(Ulid); +pub struct ProjectGuid(Ulid); -impl Default for ProjectId { +impl Default for ProjectGuid { fn default() -> Self { Self(Ulid::new()) } } -impl rusqlite::types::FromSql for ProjectId { +impl rusqlite::types::FromSql for ProjectGuid { fn column_result(value: rusqlite::types::ValueRef<'_>) -> rusqlite::types::FromSqlResult { let bytes = <[u8; 16]>::column_result(value)?; Ok(Self(Ulid::from(u128::from_be_bytes(bytes)))) } } -impl rusqlite::types::ToSql for ProjectId { +impl rusqlite::types::ToSql for ProjectGuid { fn to_sql(&self) -> rusqlite::Result> { Ok(rusqlite::types::ToSqlOutput::from(self.0.to_string())) } @@ -35,17 +35,21 @@ impl rusqlite::types::ToSql for ProjectId { #[derive(Debug, TypedBuilder, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] pub struct Project { #[builder(default, setter(strip_option))] - id: Option, + #[serde(rename = "id", skip_serializing_if = "Option::is_none")] + guid: Option, name: String, + #[serde(skip_serializing_if = "Option::is_none")] description: Option, // TODO: Broken Eq impl // #[serde(skip)] // next_actions: BinaryHeap, + #[serde(skip_serializing_if = "Option::is_none")] finished: Option>, + #[serde(skip_serializing_if = "Option::is_none")] archived: Option>, root_tasks_file: String, @@ -54,7 +58,7 @@ pub struct Project { #[derive(Serialize, Deserialize, Debug, TypedBuilder)] struct Subproject { #[builder(default, setter(strip_option))] - id: Option, + id: Option, name: String, description: String, tasks_file: String, diff --git a/crates/core/src/domain/tag.rs b/crates/core/src/domain/tag.rs index a4213220..21b90e59 100644 --- a/crates/core/src/domain/tag.rs +++ b/crates/core/src/domain/tag.rs @@ -4,22 +4,22 @@ use typed_builder::TypedBuilder; use ulid::Ulid; #[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Hash, Clone)] -pub struct TagId(Ulid); +pub struct TagGuid(Ulid); -impl Default for TagId { +impl Default for TagGuid { fn default() -> Self { Self(Ulid::new()) } } -impl rusqlite::types::FromSql for TagId { +impl rusqlite::types::FromSql for TagGuid { fn column_result(value: rusqlite::types::ValueRef<'_>) -> rusqlite::types::FromSqlResult { let bytes = <[u8; 16]>::column_result(value)?; Ok(Self(Ulid::from(u128::from_be_bytes(bytes)))) } } -impl rusqlite::types::ToSql for TagId { +impl rusqlite::types::ToSql for TagGuid { fn to_sql(&self) -> rusqlite::Result> { Ok(rusqlite::types::ToSqlOutput::from(self.0.to_string())) } @@ -28,12 +28,14 @@ impl rusqlite::types::ToSql for TagId { #[derive(Debug, TypedBuilder, Serialize, Deserialize, PartialEq, Eq, Hash, Clone)] pub struct Tag { #[builder(default, setter(strip_option))] - id: Option, + #[serde(rename = "id", skip_serializing_if = "Option::is_none")] + guid: Option, + text: String, } impl Tag { - pub fn new(id: Option, text: String) -> Self { - Self { id, text } + pub fn new(guid: Option, text: String) -> Self { + Self { guid, text } } } diff --git a/crates/core/src/domain/task.rs b/crates/core/src/domain/task.rs index ea6f1113..83aedfb8 100644 --- a/crates/core/src/domain/task.rs +++ b/crates/core/src/domain/task.rs @@ -19,13 +19,23 @@ impl Default for TaskId { #[derive(Debug, TypedBuilder, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Clone)] pub struct Task { created_at: NaiveDateTime, + description: String, + + #[builder(default, setter(strip_option))] + #[serde(skip_serializing_if = "Option::is_none")] finished_at: Option, + #[builder(default, setter(strip_option))] - id: Option, + #[serde(rename = "id", skip_serializing_if = "Option::is_none")] + guid: Option, + priority: ItemPriority, + status: ItemStatus, + tags: Vec, + title: String, // TODO: It would be nice to have a way to track the number of pomodoro cycles for each task } diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs index bd52bd5e..fa874a80 100644 --- a/crates/core/src/error.rs +++ b/crates/core/src/error.rs @@ -4,7 +4,7 @@ use displaydoc::Display; use std::{error::Error, path::PathBuf}; use thiserror::Error; -use crate::domain::activity::ActivityId; +use crate::domain::activity::ActivityGuid; /// Result type that is being returned from test functions and methods that can fail and thus have errors. pub type TestResult = Result>; @@ -92,7 +92,7 @@ pub enum ActivityLogErrorKind { /// No activities found in the activity log NoActivitiesFound, /// Activity with ID {0} not found - FailedToReadActivity(ActivityId), + FailedToReadActivity(ActivityGuid), /// Negative duration for activity NegativeDuration, /// There are no activities to hold @@ -108,13 +108,13 @@ pub enum ActivityLogErrorKind { /// Cache not available CacheNotAvailable, /// Activity with id '{0}' not found - ActivityNotFound(ActivityId), + ActivityNotFound(ActivityGuid), /// Activity with id '{0}' can't be removed from the activity log ActivityCantBeRemoved(usize), /// This activity has no id ActivityIdNotSet, /// Activity with id '{0}' already in use, can't create a new activity with the same id - ActivityIdAlreadyInUse(ActivityId), + ActivityIdAlreadyInUse(ActivityGuid), } trait PaceErrorMarker: Error {} diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 1dc6ff01..6474d33d 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -17,12 +17,12 @@ pub use crate::{ ReviewConfig, }, domain::{ - activity::{Activity, ActivityId, ActivityKind}, + activity::{Activity, ActivityGuid, ActivityKind}, activity_log::ActivityLog, filter::{ActivityFilter, FilteredActivities}, time::{extract_time_or_now, parse_time_from_user_input}, }, - error::{PaceError, PaceOptResult, PaceResult, TestResult}, + error::{PaceError, PaceErrorKind, PaceOptResult, PaceResult, TestResult}, service::activity_store::ActivityStore, storage::{ file::TomlActivityStorage, get_storage_from_config, in_memory::InMemoryActivityStorage, diff --git a/crates/core/src/service/activity_store.rs b/crates/core/src/service/activity_store.rs index fc5a96f9..cb7a6886 100644 --- a/crates/core/src/service/activity_store.rs +++ b/crates/core/src/service/activity_store.rs @@ -5,7 +5,7 @@ use serde_derive::{Deserialize, Serialize}; use crate::{ domain::{ - activity::{Activity, ActivityId}, + activity::{Activity, ActivityGuid}, filter::FilteredActivities, }, error::{PaceOptResult, PaceResult}, @@ -27,9 +27,9 @@ pub struct ActivityStore { /// TODO: Optimization for later to make lookup faster #[derive(Serialize, Deserialize, Debug, Default)] struct ActivityStoreCache { - activity_ids: HashSet, - activities_by_id: BTreeMap, - last_entries: VecDeque, + activity_ids: HashSet, + activities_by_id: BTreeMap, + last_entries: VecDeque, } impl ActivityStore { @@ -56,7 +56,7 @@ impl SyncStorage for ActivityStore { } impl ActivityReadOps for ActivityStore { - fn read_activity(&self, activity_id: ActivityId) -> PaceResult { + fn read_activity(&self, activity_id: ActivityGuid) -> PaceResult { self.storage.read_activity(activity_id) } @@ -69,29 +69,33 @@ impl ActivityReadOps for ActivityStore { } impl ActivityWriteOps for ActivityStore { - fn create_activity(&self, activity: Activity) -> PaceResult { + fn create_activity(&self, activity: Activity) -> PaceResult { self.storage.create_activity(activity) } - fn update_activity(&self, activity_id: ActivityId, activity: Activity) -> PaceResult { + fn update_activity( + &self, + activity_id: ActivityGuid, + activity: Activity, + ) -> PaceResult { self.storage.update_activity(activity_id, activity) } - fn delete_activity(&self, activity_id: ActivityId) -> PaceResult { + fn delete_activity(&self, activity_id: ActivityGuid) -> PaceResult { self.storage.delete_activity(activity_id) } } impl ActivityStateManagement for ActivityStore { - fn begin_activity(&self, activity: Activity) -> PaceResult { + fn begin_activity(&self, activity: Activity) -> PaceResult { self.storage.begin_activity(activity) } fn end_single_activity( &self, - activity_id: ActivityId, + activity_id: ActivityGuid, end_time: Option, - ) -> PaceResult { + ) -> PaceResult { self.storage.end_single_activity(activity_id, end_time) } @@ -123,7 +127,7 @@ impl ActivityQuerying for ActivityStore { todo!("Implement find_activities_in_date_range for ActivityStore") } - fn list_activities_by_id(&self) -> PaceOptResult> { + fn list_activities_by_id(&self) -> PaceOptResult> { todo!("Implement list_activities_by_id for ActivityStore") } } diff --git a/crates/core/src/service/activity_tracker.rs b/crates/core/src/service/activity_tracker.rs index ff312679..2d8c197e 100644 --- a/crates/core/src/service/activity_tracker.rs +++ b/crates/core/src/service/activity_tracker.rs @@ -2,11 +2,11 @@ use std::collections::BTreeMap; -use crate::domain::activity::{Activity, ActivityId}; +use crate::domain::activity::{Activity, ActivityGuid}; // This struct represents the overall structure for tracking activities and their intermissions. #[derive(Default, Debug, Clone)] struct ActivityTracker { - activities: BTreeMap, - intermissions: BTreeMap>, + activities: BTreeMap, + intermissions: BTreeMap>, } diff --git a/crates/core/src/storage.rs b/crates/core/src/storage.rs index 4f647455..cc90ff5b 100644 --- a/crates/core/src/storage.rs +++ b/crates/core/src/storage.rs @@ -5,7 +5,7 @@ use chrono::{NaiveDate, NaiveDateTime}; use crate::{ config::PaceConfig, domain::{ - activity::{Activity, ActivityId}, + activity::{Activity, ActivityGuid}, activity_log::ActivityLog, filter::{ActivityFilter, FilteredActivities}, review::ActivityStats, @@ -107,7 +107,7 @@ pub trait ActivityReadOps { /// # Returns /// /// The activity that was read from the storage backend. If no activity is found, it should return `Ok(None)`. - fn read_activity(&self, activity_id: ActivityId) -> PaceResult; + fn read_activity(&self, activity_id: ActivityGuid) -> PaceResult; /// List activities from the storage backend. /// @@ -143,7 +143,7 @@ pub trait ActivityWriteOps: ActivityReadOps { /// # Returns /// /// If the activity was created successfully it should return the ID of the created activity. - fn create_activity(&self, activity: Activity) -> PaceResult; + fn create_activity(&self, activity: Activity) -> PaceResult; /// Update an existing activity in the storage backend. /// @@ -168,7 +168,11 @@ pub trait ActivityWriteOps: ActivityReadOps { /// # Returns /// /// If the activity was updated successfully it should return the activity before it was updated. - fn update_activity(&self, activity_id: ActivityId, activity: Activity) -> PaceResult; + fn update_activity( + &self, + activity_id: ActivityGuid, + activity: Activity, + ) -> PaceResult; /// Delete an activity from the storage backend. /// @@ -183,7 +187,7 @@ pub trait ActivityWriteOps: ActivityReadOps { /// # Returns /// /// If the activity was deleted successfully it should return the activity that was deleted. - fn delete_activity(&self, activity_id: ActivityId) -> PaceResult; + fn delete_activity(&self, activity_id: ActivityGuid) -> PaceResult; } /// Managing Activity State @@ -206,7 +210,7 @@ pub trait ActivityStateManagement: ActivityReadOps + ActivityWriteOps { /// # Returns /// /// If the activity was started successfully it should return the ID of the started activity. - fn begin_activity(&self, activity: Activity) -> PaceResult { + fn begin_activity(&self, activity: Activity) -> PaceResult { self.create_activity(activity) } @@ -226,9 +230,9 @@ pub trait ActivityStateManagement: ActivityReadOps + ActivityWriteOps { /// If the activity was ended successfully it should return the ID of the ended activity. fn end_single_activity( &self, - activity_id: ActivityId, + activity_id: ActivityGuid, end_time: Option, - ) -> PaceResult; + ) -> PaceResult; /// End all unfinished activities in the storage backend. /// @@ -343,7 +347,7 @@ pub trait ActivityQuerying: ActivityReadOps { /// /// A collection of the activities that were loaded from the storage backend by their ID in a `BTreeMap`. /// If no activities are found, it should return `Ok(None)`. - fn list_activities_by_id(&self) -> PaceOptResult>; + fn list_activities_by_id(&self) -> PaceOptResult>; } /// Tagging Activities @@ -366,7 +370,7 @@ pub trait ActivityTagging { /// # Returns /// /// If the tag was added successfully it should return `Ok(())`. - fn add_tag_to_activity(&self, activity_id: ActivityId, tag: &str) -> PaceResult<()>; + fn add_tag_to_activity(&self, activity_id: ActivityGuid, tag: &str) -> PaceResult<()>; /// Remove a tag from an activity. /// @@ -382,7 +386,7 @@ pub trait ActivityTagging { /// # Returns /// /// If the tag was removed successfully it should return `Ok(())`. - fn remove_tag_from_activity(&self, activity_id: ActivityId, tag: &str) -> PaceResult<()>; + fn remove_tag_from_activity(&self, activity_id: ActivityGuid, tag: &str) -> PaceResult<()>; } /// Archiving Activities @@ -406,7 +410,7 @@ pub trait ActivityArchiving { /// # Returns /// /// If the activity was archived successfully it should return `Ok(())`. - fn archive_activity(&self, activity_id: ActivityId) -> PaceResult<()>; + fn archive_activity(&self, activity_id: ActivityGuid) -> PaceResult<()>; /// Unarchive an activity. /// @@ -421,7 +425,7 @@ pub trait ActivityArchiving { /// # Returns /// /// If the activity was unarchived successfully it should return `Ok(())`. - fn unarchive_activity(&self, activity_id: ActivityId) -> PaceResult<()>; + fn unarchive_activity(&self, activity_id: ActivityGuid) -> PaceResult<()>; } /// Generate Statistics for Activities diff --git a/crates/core/src/storage/file.rs b/crates/core/src/storage/file.rs index c72a8b62..d5b5d461 100644 --- a/crates/core/src/storage/file.rs +++ b/crates/core/src/storage/file.rs @@ -9,7 +9,7 @@ use chrono::{NaiveDate, NaiveDateTime}; use crate::{ domain::{ - activity::{Activity, ActivityId}, + activity::{Activity, ActivityGuid}, activity_log::ActivityLog, filter::{ActivityFilter, FilteredActivities}, }, @@ -120,7 +120,7 @@ impl ActivityStorage for TomlActivityStorage { } impl ActivityReadOps for TomlActivityStorage { - fn read_activity(&self, activity_id: ActivityId) -> PaceResult { + fn read_activity(&self, activity_id: ActivityGuid) -> PaceResult { self.cache.read_activity(activity_id) } @@ -143,9 +143,9 @@ impl ActivityStateManagement for TomlActivityStorage { fn end_single_activity( &self, - activity_id: ActivityId, + activity_id: ActivityGuid, end_time: Option, - ) -> PaceResult { + ) -> PaceResult { self.cache.end_single_activity(activity_id, end_time) } @@ -158,21 +158,25 @@ impl ActivityStateManagement for TomlActivityStorage { } impl ActivityWriteOps for TomlActivityStorage { - fn create_activity(&self, activity: Activity) -> PaceResult { + fn create_activity(&self, activity: Activity) -> PaceResult { self.cache.create_activity(activity) } - fn update_activity(&self, activity_id: ActivityId, activity: Activity) -> PaceResult { + fn update_activity( + &self, + activity_id: ActivityGuid, + activity: Activity, + ) -> PaceResult { self.cache.update_activity(activity_id, activity) } - fn delete_activity(&self, activity_id: ActivityId) -> PaceResult { + fn delete_activity(&self, activity_id: ActivityGuid) -> PaceResult { self.cache.delete_activity(activity_id) } } impl ActivityQuerying for TomlActivityStorage { - fn list_activities_by_id(&self) -> PaceOptResult> { + fn list_activities_by_id(&self) -> PaceOptResult> { todo!("Implement `activities_by_id` for `TomlActivityStorage`") } diff --git a/crates/core/src/storage/in_memory.rs b/crates/core/src/storage/in_memory.rs index be3282b8..2906d4c4 100644 --- a/crates/core/src/storage/in_memory.rs +++ b/crates/core/src/storage/in_memory.rs @@ -7,7 +7,7 @@ use rayon::prelude::{ use crate::{ domain::{ - activity::{Activity, ActivityId}, + activity::{Activity, ActivityGuid}, activity_log::ActivityLog, filter::{ActivityFilter, FilteredActivities}, }, @@ -92,7 +92,7 @@ impl SyncStorage for InMemoryActivityStorage { } impl ActivityReadOps for InMemoryActivityStorage { - fn read_activity(&self, activity_id: ActivityId) -> PaceResult { + fn read_activity(&self, activity_id: ActivityGuid) -> PaceResult { let Ok(activities) = self.activities.lock() else { return Err(ActivityLogErrorKind::MutexHasBeenPoisoned.into()); }; @@ -102,7 +102,7 @@ impl ActivityReadOps for InMemoryActivityStorage { .par_iter() .find_first(|activity| { activity - .id() + .guid() .as_ref() .map_or(false, |orig_activity_id| *orig_activity_id == activity_id) }) @@ -147,21 +147,22 @@ impl ActivityReadOps for InMemoryActivityStorage { } impl ActivityWriteOps for InMemoryActivityStorage { - fn create_activity(&self, activity: Activity) -> PaceResult { + fn create_activity(&self, activity: Activity) -> PaceResult { let Ok(mut activities) = self.activities.lock() else { return Err(ActivityLogErrorKind::MutexHasBeenPoisoned.into()); }; - let Some(activity_id) = activity.id() else { + let Some(activity_id) = activity.guid() else { return Err(ActivityLogErrorKind::ActivityIdNotSet.into()); }; // Search for the activity in the list of activities to see if the ID is already in use. - if activities - .activities() - .par_iter() - .any(|activity| activity.id().as_ref().map_or(false, |id| id == activity_id)) - { + if activities.activities().par_iter().any(|activity| { + activity + .guid() + .as_ref() + .map_or(false, |id| id == activity_id) + }) { return Err(ActivityLogErrorKind::ActivityIdAlreadyInUse(*activity_id).into()); } @@ -174,12 +175,12 @@ impl ActivityWriteOps for InMemoryActivityStorage { fn update_activity( &self, - activity_id: ActivityId, + activity_id: ActivityGuid, mut activity: Activity, ) -> PaceResult { // First things, first. Replace new activity's id with the original ID we are looking for. // To make sure we are not accidentally changing the ID. - let _ = activity.id_mut().replace(activity_id); + let _ = activity.guid_mut().replace(activity_id); let Ok(mut activities) = self.activities.lock() else { return Err(ActivityLogErrorKind::MutexHasBeenPoisoned.into()); @@ -190,7 +191,7 @@ impl ActivityWriteOps for InMemoryActivityStorage { .par_iter_mut() .find_first(|activity| { activity - .id() + .guid() .as_ref() .map_or(false, |orig_activity_id| *orig_activity_id == activity_id) }) @@ -205,7 +206,7 @@ impl ActivityWriteOps for InMemoryActivityStorage { Ok(original_activity) } - fn delete_activity(&self, activity_id: ActivityId) -> PaceResult { + fn delete_activity(&self, activity_id: ActivityGuid) -> PaceResult { let Ok(mut activities) = self.activities.lock() else { return Err(ActivityLogErrorKind::MutexHasBeenPoisoned.into()); }; @@ -215,7 +216,7 @@ impl ActivityWriteOps for InMemoryActivityStorage { .par_iter() .position_first(|activity| { activity - .id() + .guid() .as_ref() .map_or(false, |orig_activity_id| *orig_activity_id == activity_id) }) @@ -235,9 +236,9 @@ impl ActivityWriteOps for InMemoryActivityStorage { impl ActivityStateManagement for InMemoryActivityStorage { fn end_single_activity( &self, - activity_id: ActivityId, + activity_id: ActivityGuid, end_time: Option, - ) -> PaceResult { + ) -> PaceResult { let Ok(mut activities) = self.activities.lock() else { return Err(ActivityLogErrorKind::MutexHasBeenPoisoned.into()); }; @@ -249,7 +250,7 @@ impl ActivityStateManagement for InMemoryActivityStorage { .par_iter_mut() .find_first(|activity| { activity - .id() + .guid() .as_ref() .map_or(false, |orig_activity_id| *orig_activity_id == activity_id) }) @@ -355,7 +356,7 @@ impl ActivityQuerying for InMemoryActivityStorage { fn list_activities_by_id( &self, - ) -> PaceOptResult> { + ) -> PaceOptResult> { todo!("Implement list_activities_by_id for InMemoryActivityStorage") } } diff --git a/crates/core/tests/activity_store.rs b/crates/core/tests/activity_store.rs index 316308b9..d6a6d682 100644 --- a/crates/core/tests/activity_store.rs +++ b/crates/core/tests/activity_store.rs @@ -1,7 +1,7 @@ // Test the ActivityStore implementation with a InMemoryStorage backend. use pace_core::{ - Activity, ActivityFilter, ActivityId, ActivityLog, ActivityReadOps, ActivityStore, + Activity, ActivityFilter, ActivityGuid, ActivityLog, ActivityReadOps, ActivityStore, ActivityWriteOps, InMemoryActivityStorage, TestResult, }; use rstest::{fixture, rstest}; @@ -31,7 +31,7 @@ fn activity_log_with_content() -> (Vec, ActivityLog) { #[fixture] fn activity_store_with_item( activity_log_empty: ActivityLog, -) -> TestResult<(ActivityId, Activity, ActivityStore)> { +) -> TestResult<(ActivityGuid, Activity, ActivityStore)> { let store = ActivityStore::new(Box::new(InMemoryActivityStorage::new_with_activity_log( activity_log_empty, ))); @@ -58,7 +58,7 @@ fn test_activity_store_create_activity_passes(activity_log_empty: ActivityLog) - .build(); let og_activity = activity.clone(); - let og_activity_id = activity.id().expect("Activity ID should be set."); + let og_activity_id = activity.guid().expect("Activity ID should be set."); let activity_id = store.create_activity(activity)?; @@ -80,10 +80,10 @@ fn test_activity_store_create_activity_fails( activity_log, ))); - let id = activities[0].id().expect("Activity ID should be set."); + let id = activities[0].guid().expect("Activity ID should be set."); let activity = Activity::builder() - .id(id) + .guid(id) .description("Test Description".to_string()) .category(Some("Test::Category".to_string())) .build(); @@ -93,7 +93,7 @@ fn test_activity_store_create_activity_fails( #[rstest] fn test_activity_store_read_activity_passes( - activity_store_with_item: TestResult<(ActivityId, Activity, ActivityStore)>, + activity_store_with_item: TestResult<(ActivityGuid, Activity, ActivityStore)>, ) -> TestResult<()> { let (og_activity_id, og_activity, store) = activity_store_with_item?; @@ -110,7 +110,7 @@ fn test_activity_store_read_activity_fails(activity_log_empty: ActivityLog) { activity_log_empty, ))); - let activity_id = ActivityId::default(); + let activity_id = ActivityGuid::default(); assert!(store.read_activity(activity_id).is_err()); } @@ -141,7 +141,7 @@ fn test_activity_store_list_active_activities_passes( #[rstest] fn test_activity_store_update_activity_passes( - activity_store_with_item: TestResult<(ActivityId, Activity, ActivityStore)>, + activity_store_with_item: TestResult<(ActivityGuid, Activity, ActivityStore)>, ) -> TestResult<()> { let (og_activity_id, og_activity, store) = activity_store_with_item?; @@ -159,7 +159,7 @@ fn test_activity_store_update_activity_passes( let stored_activity = store.read_activity(og_activity_id)?; - _ = new_activity.id_mut().replace(og_activity_id); + _ = new_activity.guid_mut().replace(og_activity_id); assert_eq!(stored_activity, new_activity); @@ -168,7 +168,7 @@ fn test_activity_store_update_activity_passes( #[rstest] fn test_activity_store_delete_activity_passes( - activity_store_with_item: TestResult<(ActivityId, Activity, ActivityStore)>, + activity_store_with_item: TestResult<(ActivityGuid, Activity, ActivityStore)>, ) -> TestResult<()> { let (og_activity_id, og_activity, store) = activity_store_with_item?; @@ -190,7 +190,7 @@ fn test_activity_store_delete_activity_fails( activity_log, ))); - let activity_id = ActivityId::default(); + let activity_id = ActivityGuid::default(); assert!(store.delete_activity(activity_id).is_err()); } @@ -209,7 +209,7 @@ fn test_activity_store_update_activity_fails( .category(Some("test".to_string())) .build(); - let activity_id = ActivityId::default(); + let activity_id = ActivityGuid::default(); assert!(store.update_activity(activity_id, new_activity).is_err()); } diff --git a/src/commands.rs b/src/commands.rs index 03ab787e..d2d4e76b 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -10,18 +10,18 @@ //! See the `impl Configurable` below for how to specify the path to the //! application's configuration file. -mod begin; -mod end; -mod export; -// TODO: mod import; -mod craft; -mod hold; -mod now; -mod pomo; -mod resume; -mod review; -mod set; -mod tasks; +pub mod begin; +pub mod end; +pub mod export; +// TODO: pub mod import; +pub mod craft; +pub mod hold; +pub mod now; +pub mod pomo; +pub mod resume; +pub mod review; +pub mod set; +pub mod tasks; use abscissa_core::{config::Override, Command, Configurable, FrameworkError, Runnable}; use clap::builder::{styling::AnsiColor, Styles}; diff --git a/src/commands/begin.rs b/src/commands/begin.rs index 240ea46f..18e4cbd1 100644 --- a/src/commands/begin.rs +++ b/src/commands/begin.rs @@ -8,7 +8,7 @@ use crate::prelude::PACE_APP; use pace_core::{ extract_time_or_now, get_storage_from_config, Activity, ActivityKind, ActivityStateManagement, - ActivityStorage, ActivityStore, SyncStorage, + ActivityStorage, ActivityStore, PaceConfig, SyncStorage, }; /// `begin` subcommand @@ -41,7 +41,7 @@ pub struct BeginCmd { impl Runnable for BeginCmd { /// Start the application. fn run(&self) { - if let Err(err) = self.inner_run() { + if let Err(err) = self.inner_run(&PACE_APP.config()) { status_err!("{}", err); PACE_APP.shutdown(Shutdown::Crash); }; @@ -49,7 +49,25 @@ impl Runnable for BeginCmd { } impl BeginCmd { - pub fn inner_run(&self) -> Result<()> { + /// Create a new instance of the `begin` subcommand + pub fn new( + category: impl Into>, + time: impl Into>, + description: String, + tags: impl Into>>, + projects: impl Into>>, + ) -> Self { + Self { + category: category.into(), + time: time.into(), + description, + tags: tags.into(), + _projects: projects.into(), + } + } + + /// Inner run implementation for the begin command + pub fn inner_run(&self, config: &PaceConfig) -> Result<()> { let Self { category, time, @@ -87,14 +105,53 @@ impl BeginCmd { .category(category.clone()) .build(); - let activity_store = ActivityStore::new(get_storage_from_config(&PACE_APP.config())?); + let activity_store = ActivityStore::new(get_storage_from_config(config)?); activity_store.setup_storage()?; - let _activity = activity_store.begin_activity(activity.clone())?; - activity_store.sync()?; + let activity_id = activity_store.begin_activity(activity.clone())?; - println!("{activity}"); + if let Some(og_activity_id) = activity.guid() { + if activity_id == *og_activity_id { + activity_store.sync()?; + println!("{activity}"); + return Ok(()); + } + } - Ok(()) + eyre::bail!("Failed to start {activity}"); } } + +// TODO!: Test pace-rs begin command +// #[cfg(test)] +// mod tests { + +// use std::env::temp_dir; + +// use abscissa_core::fs::create_dir_all; +// use eyre::Ok; + +// use super::*; + +// #[test] +// fn test_begin_subcommand_creates_activity_passes() -> Result<()> { +// let temp_dir = temp_dir(); +// let temp_file = temp_dir.join("activity_log.toml"); +// create_dir_all(temp_dir)?; + +// let cmd = BeginCmd::new( +// "Test::Category".to_string(), +// "22:00".to_string(), +// "Test description".to_string(), +// None, +// None, +// ); + +// let mut pace_config = PaceConfig::default(); +// pace_config.add_activity_log_path(temp_file.to_string_lossy().to_string()); + +// cmd.inner_run(&pace_config)?; + +// Ok(()) +// } +// } diff --git a/src/commands/craft.rs b/src/commands/craft.rs index 92ea9d4f..efdad050 100644 --- a/src/commands/craft.rs +++ b/src/commands/craft.rs @@ -24,6 +24,7 @@ pub enum CraftSubCmd { Completions(completions::CompletionsCmd), } +/// `craft` subcommand #[derive(Command, Debug, Parser, Runnable)] pub struct CraftCmd { #[clap(subcommand)] diff --git a/src/commands/end.rs b/src/commands/end.rs index f5523bd1..d59ed1e4 100644 --- a/src/commands/end.rs +++ b/src/commands/end.rs @@ -33,7 +33,7 @@ impl Runnable for EndCmd { } impl EndCmd { - pub fn inner_run(&self) -> Result<()> { + fn inner_run(&self) -> Result<()> { let Self { time, only_last, .. } = self; diff --git a/src/commands/hold.rs b/src/commands/hold.rs index 8f2ae782..f85e9556 100644 --- a/src/commands/hold.rs +++ b/src/commands/hold.rs @@ -26,6 +26,7 @@ impl Runnable for HoldCmd { } impl HoldCmd { + /// Inner run implementation for the hold command pub fn inner_run(&self) -> Result<()> { // TODO!: Implement hold command // diff --git a/src/commands/now.rs b/src/commands/now.rs index 0151a893..25d25735 100644 --- a/src/commands/now.rs +++ b/src/commands/now.rs @@ -23,6 +23,7 @@ impl Runnable for NowCmd { } impl NowCmd { + /// Inner run implementation for the now command pub fn inner_run(&self) -> Result<()> { let activity_store = ActivityStore::new(get_storage_from_config(&PACE_APP.config())?); diff --git a/tests/cli.rs b/tests/cli.rs index 2aedced3..a474fad6 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,8 +1,9 @@ use assert_cmd::Command; -// use pace_core::ActivityLog; use predicates::prelude::predicate; -// use tempfile::tempdir; // use similar_asserts::assert_eq; +// use tempfile::tempdir; + +// use pace_core::ActivityLog; pub type TestResult = Result>; @@ -79,6 +80,7 @@ fn test_help_command_passes() -> TestResult<()> { // let activity_log = toml::from_str::(&contents)?; +// insta::assert_toml_snapshot!(activity_log); // assert_eq!(activity_log.activities().len(), 1); // let activity = activity_log.activities().front().unwrap();