refactor: extract find_task helper to deduplicate CLI task search

The complete(), delete(), and edit() functions each had an identical
loop searching for a task by ID across all lists. Extract a shared
find_task() helper that returns the list ID and task.

https://claude.ai/code/session_013ooJht2HrZUTXgNJFU79cV
This commit is contained in:
Claude 2026-04-16 07:27:16 +00:00
parent 1ad6fddda6
commit 85400b68bc
No known key found for this signature in database

View file

@ -4,6 +4,17 @@ use chrono::{DateTime, Utc};
use uuid::Uuid; use uuid::Uuid;
use crate::output; use crate::output;
use crate::commands::get_repository; use crate::commands::get_repository;
use onyx_core::TaskList;
/// Find a task by ID across all lists, returning the list ID and cloned task.
fn find_task(lists: &[TaskList], task_id: Uuid) -> Option<(Uuid, Task)> {
for list in lists {
if let Some(task) = list.tasks.iter().find(|t| t.id == task_id) {
return Some((list.id, task.clone()));
}
}
None
}
pub fn add(title: String, list_name: Option<String>, date_str: Option<String>, workspace: Option<String>) -> Result<()> { pub fn add(title: String, list_name: Option<String>, date_str: Option<String>, workspace: Option<String>) -> Result<()> {
let (mut repo, _workspace_name) = get_repository(workspace)?; let (mut repo, _workspace_name) = get_repository(workspace)?;
@ -56,25 +67,15 @@ pub fn complete(task_id_str: String, workspace: Option<String>) -> Result<()> {
let task_id = Uuid::parse_str(&task_id_str) let task_id = Uuid::parse_str(&task_id_str)
.context("Invalid task ID")?; .context("Invalid task ID")?;
// Find the task across all lists
let lists = repo.get_lists()?; let lists = repo.get_lists()?;
let mut found = false; let (list_id, mut task) = find_task(&lists, task_id)
.ok_or_else(|| anyhow::anyhow!("Task not found: {}", task_id_str))?;
for list in lists {
if let Some(mut task) = list.tasks.iter().find(|t| t.id == task_id).cloned() {
task.complete(); task.complete();
repo.update_task(list.id, task.clone()) repo.update_task(list_id, task.clone())
.context("Failed to update task")?; .context("Failed to update task")?;
output::success(&format!("Completed task \"{}\"", task.title)); output::success(&format!("Completed task \"{}\"", task.title));
found = true;
break;
}
}
if !found {
anyhow::bail!("Task not found: {}", task_id_str);
}
Ok(()) Ok(())
} }
@ -85,15 +86,11 @@ pub fn delete(task_id_str: String, workspace: Option<String>) -> Result<()> {
let task_id = Uuid::parse_str(&task_id_str) let task_id = Uuid::parse_str(&task_id_str)
.context("Invalid task ID")?; .context("Invalid task ID")?;
// Find the task across all lists
let lists = repo.get_lists()?; let lists = repo.get_lists()?;
let mut found = false; let (list_id, task) = find_task(&lists, task_id)
.ok_or_else(|| anyhow::anyhow!("Task not found: {}", task_id_str))?;
for list in lists { output::warning(&format!("This will delete task \"{}\"", task.title));
if let Some(task) = list.tasks.iter().find(|t| t.id == task_id) {
let title = task.title.clone();
output::warning(&format!("This will delete task \"{}\"", title));
print!("Continue? (y/n): "); print!("Continue? (y/n): ");
use std::io::{self, Write}; use std::io::{self, Write};
io::stdout().flush()?; io::stdout().flush()?;
@ -104,18 +101,10 @@ pub fn delete(task_id_str: String, workspace: Option<String>) -> Result<()> {
return Ok(()); return Ok(());
} }
repo.delete_task(list.id, task_id) repo.delete_task(list_id, task_id)
.context("Failed to delete task")?; .context("Failed to delete task")?;
output::success(&format!("Deleted task \"{}\"", title)); output::success(&format!("Deleted task \"{}\"", task.title));
found = true;
break;
}
}
if !found {
anyhow::bail!("Task not found: {}", task_id_str);
}
Ok(()) Ok(())
} }
@ -126,23 +115,9 @@ pub fn edit(task_id_str: String, workspace: Option<String>) -> Result<()> {
let task_id = Uuid::parse_str(&task_id_str) let task_id = Uuid::parse_str(&task_id_str)
.context("Invalid task ID")?; .context("Invalid task ID")?;
// Find the task across all lists
let lists = repo.get_lists()?; let lists = repo.get_lists()?;
let mut task_list_id = None; let (list_id, task) = find_task(&lists, task_id)
let mut task_to_edit = None; .ok_or_else(|| anyhow::anyhow!("Task not found: {}", task_id_str))?;
for list in lists {
if let Some(task) = list.tasks.iter().find(|t| t.id == task_id).cloned() {
task_list_id = Some(list.id);
task_to_edit = Some(task);
break;
}
}
let (list_id, task) = match (task_list_id, task_to_edit) {
(Some(lid), Some(t)) => (lid, t),
_ => anyhow::bail!("Task not found: {}", task_id_str),
};
// Create temporary file with task content // Create temporary file with task content
let temp_dir = std::env::temp_dir(); let temp_dir = std::env::temp_dir();