refactor magic number -1, used for default entity ids

watercooler-labs · Dec 11, 2024 · 8b7e602 · 8b7e602
1 parent 36b3bd9
commit 8b7e602
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 26 deletions.
diff --git a/src/api/client.rs b/src/api/client.rs
@@ -1,5 +1,6 @@
 use std::collections::HashMap;
 
+use crate::constants;
 use crate::credentials;
 use crate::error;
 use crate::models;
@@ -188,7 +189,9 @@ impl ApiClient for V9ApiClient {
                         id: p.id,
                         name: p.name.clone(),
                         workspace_id: p.workspace_id,
-                        client: clients.get(&p.client_id.unwrap_or(-1)).cloned(),
+                        client: clients
+                            .get(&p.client_id.unwrap_or(constants::DEFAULT_ENTITY_ID))
+                            .cloned(),
                         is_private: p.is_private,
                         active: p.active,
                         at: p.at,
@@ -228,8 +231,12 @@ impl ApiClient for V9ApiClient {
                 billable: te.billable,
                 workspace_id: te.workspace_id,
                 tags: te.tags.clone(),
-                project: projects.get(&te.project_id.unwrap_or(-1)).cloned(),
-                task: tasks.get(&te.task_id.unwrap_or(-1)).cloned(),
+                project: projects
+                    .get(&te.project_id.unwrap_or(constants::DEFAULT_ENTITY_ID))
+                    .cloned(),
+                task: tasks
+                    .get(&te.task_id.unwrap_or(constants::DEFAULT_ENTITY_ID))
+                    .cloned(),
                 ..Default::default()
             })
             .collect();

diff --git a/src/commands/edit.rs b/src/commands/edit.rs
@@ -26,7 +26,16 @@ impl EditCommand {
                     })
                     .unwrap();
 
-                api_client.update_time_entry(updated_time_entry).await?;
+                let updated_entry_id = api_client
+                    .update_time_entry(updated_time_entry.clone())
+                    .await;
+                if updated_entry_id.is_err() {
+                    println!("{}", "Failed to update time entry".red());
+                    return Err(updated_entry_id.err().unwrap());
+                }
+
+                println!("{}\n{}", "Time entry updated".green(), updated_time_entry);
+                return Ok(());
             }
         }
         Ok(())

diff --git a/src/commands/start.rs b/src/commands/start.rs
@@ -1,6 +1,7 @@
 use crate::api;
 use crate::commands;
 use crate::config;
+use crate::constants;
 use crate::models;
 use crate::models::Entities;
 use crate::picker::ItemPicker;
@@ -104,7 +105,7 @@ impl StartCommand {
             .and_then(|track_config| track_config.get_default_entry(entities.clone()))
             .unwrap_or_else(|_| TimeEntry::default());
 
-        let workspace_id = if default_time_entry.workspace_id != -1 {
+        let workspace_id = if default_time_entry.workspace_id != constants::DEFAULT_ENTITY_ID {
             default_time_entry.workspace_id
         } else {
             workspace_id

diff --git a/src/config/model.rs b/src/config/model.rs
@@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::error::ConfigError;
 use crate::models::{Entities, ResultWithDefaultError, TimeEntry};
-use crate::utilities;
+use crate::{constants, utilities};
 
 /// BranchConfig optionally determines workspace, description, project, task,
 /// tags, and billable status of a time entry.
@@ -546,16 +546,16 @@ impl TrackConfig {
                 .find(|t| t.name == name && t.project.id == project_id.unwrap())
         });
 
-        let workspace_id = config.workspace.as_ref().map_or(
-            // Default to -1 if workspace is not set
-            Ok(-1),
-            |name| {
-                entities.workspace_id_for_name(name).ok_or_else(|| {
-                    Box::new(ConfigError::WorkspaceNotFound(name.clone()))
-                        as Box<dyn std::error::Error + Send>
-                })
-            },
-        )?;
+        let workspace_id =
+            config
+                .workspace
+                .as_ref()
+                .map_or(Ok(constants::DEFAULT_ENTITY_ID), |name| {
+                    entities.workspace_id_for_name(name).ok_or_else(|| {
+                        Box::new(ConfigError::WorkspaceNotFound(name.clone()))
+                            as Box<dyn std::error::Error + Send>
+                    })
+                })?;
 
         let time_entry = TimeEntry {
             workspace_id,

diff --git a/src/constants.rs b/src/constants.rs
@@ -31,6 +31,7 @@ pub const NO_TASK: &str = "No Task";
 pub const NO_DESCRIPTION: &str = "(no description)";
 pub const DIRECTORY_NOT_FOUND_ERROR: &str = "Directory not found";
 pub const NOT_A_DIRECTORY_ERROR: &str = "Not a directory";
+pub const DEFAULT_ENTITY_ID: i64 = -1;
 
 #[cfg(target_os = "macos")]
 pub const SIMPLE_HOME_PATH: &str = "~/Library/Application Support";

diff --git a/src/models.rs b/src/models.rs
@@ -32,11 +32,17 @@ impl Entities {
             .map(|w| w.id)
     }
     pub fn project_for_name(&self, workspace_id: i64, name: &str) -> Option<Project> {
-        self.projects.values().find(|p| p.workspace_id == workspace_id && p.name == name).cloned()
+        self.projects
+            .values()
+            .find(|p| p.workspace_id == workspace_id && p.name == name)
+            .cloned()
     }
 
     pub fn task_for_name(&self, workspace_id: i64, name: &str) -> Option<Task> {
-        self.tasks.values().find(|t| t.workspace_id == workspace_id && t.name == name).cloned()
+        self.tasks
+            .values()
+            .find(|t| t.workspace_id == workspace_id && t.name == name)
+            .cloned()
     }
 }
 
@@ -180,9 +186,9 @@ impl std::fmt::Display for Project {
 impl Default for Project {
     fn default() -> Self {
         Self {
-            id: -1,
+            id: constants::DEFAULT_ENTITY_ID,
             name: constants::NO_PROJECT.to_string(),
-            workspace_id: -1,
+            workspace_id: constants::DEFAULT_ENTITY_ID,
             client: None,
             is_private: false,
             active: true,
@@ -212,9 +218,9 @@ pub struct Task {
 impl Default for Task {
     fn default() -> Self {
         Self {
-            id: -1,
+            id: constants::DEFAULT_ENTITY_ID,
             name: constants::NO_TASK.to_string(),
-            workspace_id: -1,
+            workspace_id: constants::DEFAULT_ENTITY_ID,
             project: Project::default(),
         }
     }
@@ -280,7 +286,7 @@ impl Default for TimeEntry {
     fn default() -> Self {
         let start = Utc::now();
         Self {
-            id: -1,
+            id: constants::DEFAULT_ENTITY_ID,
             created_with: Some(constants::CLIENT_NAME.to_string()),
             billable: false,
             description: "".to_string(),
@@ -290,7 +296,7 @@ impl Default for TimeEntry {
             stop: None,
             tags: Vec::new(),
             task: None,
-            workspace_id: -1,
+            workspace_id: constants::DEFAULT_ENTITY_ID,
         }
     }
 }
@@ -398,7 +404,7 @@ impl Parcel for TimeEntry {
                     let project_id = if project_parts.len() > 1 {
                         project_parts[1].parse().unwrap()
                     } else {
-                        -1
+                        constants::DEFAULT_ENTITY_ID
                     };
                     time_entry.project = Some(Project {
                         id: project_id,
@@ -416,7 +422,7 @@ impl Parcel for TimeEntry {
                     let task_id = if task_parts.len() > 1 {
                         task_parts[1].parse().unwrap()
                     } else {
-                        -1
+                        constants::DEFAULT_ENTITY_ID
                     };
                     time_entry.task = Some(Task {
                         id: task_id,