From 66b833b7ac41d2c0a407a2857e9607a02759654d Mon Sep 17 00:00:00 2001 From: Ruben Rosario Date: Mon, 22 Jun 2026 12:15:11 +0100 Subject: [PATCH] Refactor bulk move to use Move API + Fix update_list_id UNIQUE violation + File-only logging - Bulk move (new/existing list) now uses SyncAction::Move + move_task API instead of create+delete clone, preserving task IDs and history on Google - Add SyncAction::Move variant and MovePayload struct to domain models - Fix api.move_task parameter: destinationTaskList -> destinationTasklist - Add update_task_list_id method to Db for local DB update - Fix update_list_id: DELETE+INSERT instead of UPDATE for task_lists to avoid UNIQUE constraint violation (duplicate PK on re-sync) - Add list_id_map table and resolve_list_id for UUID->server ID mapping - Replace eprintln! with file-only log_msg to avoid TUI screen corruption - Add debug logging throughout sync engine (push_sync items, CreateList, Move resolution, etc.) - Skip pushing Delete sync for single tasks whose list_id is local UUID - Use trigger_full_sync() instead of trigger_sync() after bulk move operations, called after load_tasks() for clean ordering --- src/app.rs | 128 ++++++++++++++++++++------------------ src/domain/models.rs | 6 ++ src/infrastructure/api.rs | 2 +- src/infrastructure/db.rs | 87 ++++++++++++++++++++++++-- src/main.rs | 95 +++++++++++++++++++++++----- 5 files changed, 236 insertions(+), 82 deletions(-) diff --git a/src/app.rs b/src/app.rs index 928289c..579b00b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -833,12 +833,14 @@ impl App { let task_id = task.id.clone(); let list_id = task.list_id.clone(); self.db.delete_task(&task_id).ok(); - self.db.push_sync( - SyncAction::Delete, - &task_id, - &list_id, - "", - ).ok(); + if !list_id.contains('-') { + self.db.push_sync( + SyncAction::Delete, + &task_id, + &list_id, + "", + ).ok(); + } self.trigger_sync(); self.load_tasks(); if self.selected_task >= self.tasks.len() { @@ -951,6 +953,15 @@ impl App { id: uuid_v4(), title: list_name.to_string(), }; + crate::log_msg(&format!("[task_app] bulk_move_to_new_list: list={} title={} selected={} tasks_len={}", + list.id, list_name, self.selected_tasks.len(), self.tasks.len())); + for &i in self.selected_tasks.iter() { + if i < self.tasks.len() { + crate::log_msg(&format!("[task_app] selected[{}]: task={} list_id={} has_hyphen={}", + i, self.tasks[i].id, self.tasks[i].list_id, self.tasks[i].list_id.contains('-'))); + } + } + self.db.insert_list(&list).ok(); self.db.push_sync( SyncAction::CreateList, @@ -963,43 +974,37 @@ impl App { for &i in &indices { if i >= self.tasks.len() { continue; } let original = &self.tasks[i]; - let new_task = Task { - id: uuid_v4(), - list_id: list.id.clone(), - title: original.title.clone(), - notes: original.notes.clone(), - status: original.status.clone(), - due: original.due, - position: 0, - created_at: None, - updated_at: None, - }; - self.db.insert_task(&new_task).ok(); - self.db.push_sync( - SyncAction::Create, - &new_task.id, - &list.id, - &serde_json::to_string(&new_task).unwrap_or_default(), - ).ok(); - // Delete original - self.db.delete_task(&original.id).ok(); - self.db.push_sync( - SyncAction::Delete, - &original.id, - &original.list_id, - "", - ).ok(); + // Always update local DB immediately + self.db.update_task_list_id(&original.id, &list.id).ok(); + + if !original.list_id.contains('-') { + // Source list has server ID — also push Move sync + let payload = serde_json::to_string(&MovePayload { + destination_list_id: list.id.clone(), + }).unwrap_or_default(); + crate::log_msg(&format!("[task_app] pushing Move: task={} source={} dest={}", + original.id, original.list_id, list.id)); + self.db.push_sync( + SyncAction::Move, + &original.id, + &original.list_id, + &payload, + ).ok(); + } else { + crate::log_msg(&format!("[task_app] skipping Move (source has hyphen): task={} list_id={}", + original.id, original.list_id)); + } } self.clear_selection(); - self.trigger_sync(); self.load_lists(); // Switch to the new list if let Some(pos) = self.lists.iter().position(|l| l.id == list.id) { self.selected_list = pos; } self.load_tasks(); + self.trigger_full_sync(); } fn bulk_mark_uncomplete(&mut self) { @@ -1066,43 +1071,46 @@ impl App { } fn bulk_move_to_existing_list(&mut self, target_list_id: &str) { + crate::log_msg(&format!("[task_app] bulk_move_to_existing_list: target={} selected={} tasks_len={}", + target_list_id, self.selected_tasks.len(), self.tasks.len())); + for &i in self.selected_tasks.iter() { + if i < self.tasks.len() { + crate::log_msg(&format!("[task_app] selected[{}]: task={} list_id={} has_hyphen={}", + i, self.tasks[i].id, self.tasks[i].list_id, self.tasks[i].list_id.contains('-'))); + } + } + let indices: Vec = self.selected_tasks.iter().copied().collect(); for &i in &indices { if i >= self.tasks.len() { continue; } let original = &self.tasks[i]; - let new_task = Task { - id: uuid_v4(), - list_id: target_list_id.to_string(), - title: original.title.clone(), - notes: original.notes.clone(), - status: original.status.clone(), - due: original.due, - position: 0, - created_at: None, - updated_at: None, - }; - self.db.insert_task(&new_task).ok(); - self.db.push_sync( - SyncAction::Create, - &new_task.id, - target_list_id, - &serde_json::to_string(&new_task).unwrap_or_default(), - ).ok(); - self.db.delete_task(&original.id).ok(); - self.db.push_sync( - SyncAction::Delete, - &original.id, - &original.list_id, - "", - ).ok(); + // Always update local DB immediately + self.db.update_task_list_id(&original.id, target_list_id).ok(); + + if !original.list_id.contains('-') { + let payload = serde_json::to_string(&MovePayload { + destination_list_id: target_list_id.to_string(), + }).unwrap_or_default(); + crate::log_msg(&format!("[task_app] pushing Move: task={} source={} dest={}", + original.id, original.list_id, target_list_id)); + self.db.push_sync( + SyncAction::Move, + &original.id, + &original.list_id, + &payload, + ).ok(); + } else { + crate::log_msg(&format!("[task_app] skipping Move (source has hyphen): task={} list_id={}", + original.id, original.list_id)); + } } self.clear_selection(); - self.trigger_sync(); if let Some(pos) = self.lists.iter().position(|l| l.id == target_list_id) { self.selected_list = pos; } self.load_tasks(); + self.trigger_full_sync(); } fn execute_bulk_action(&mut self, action_idx: usize) { @@ -1119,8 +1127,8 @@ impl App { self.show_popup = Some(Popup::Input); } 6 => { + self.load_lists(); self.popup_list_indices = self.lists.iter() - .filter(|l| !l.id.contains('-')) .map(|l| (l.title.clone(), l.id.clone())) .collect(); self.popup_list_selected = 0; diff --git a/src/domain/models.rs b/src/domain/models.rs index 6dcbe08..e960e44 100644 --- a/src/domain/models.rs +++ b/src/domain/models.rs @@ -32,10 +32,16 @@ pub enum SyncAction { Update, Delete, Reorder, + Move, CreateList, DeleteList, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MovePayload { + pub destination_list_id: String, +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SyncQueueItem { pub id: i64, diff --git a/src/infrastructure/api.rs b/src/infrastructure/api.rs index aa9d771..9c689db 100644 --- a/src/infrastructure/api.rs +++ b/src/infrastructure/api.rs @@ -454,7 +454,7 @@ impl ApiClient { req = req.query(&[("previous", p)]); } if let Some(s) = sibling { - req = req.query(&[("destinationTaskList", s)]); + req = req.query(&[("destinationTasklist", s)]); } let resp = req diff --git a/src/infrastructure/db.rs b/src/infrastructure/db.rs index 424aaf3..39b61ec 100644 --- a/src/infrastructure/db.rs +++ b/src/infrastructure/db.rs @@ -40,6 +40,11 @@ impl Db { payload TEXT NOT NULL, created_at TEXT NOT NULL, retries INTEGER NOT NULL DEFAULT 0 + ); + + CREATE TABLE IF NOT EXISTS list_id_map ( + old_id TEXT PRIMARY KEY, + new_id TEXT NOT NULL );", )?; conn.execute_batch( @@ -206,6 +211,15 @@ impl Db { Ok(()) } + pub fn update_task_list_id(&self, task_id: &str, new_list_id: &str) -> SqlResult<()> { + let conn = self.conn.lock().unwrap(); + conn.execute( + "UPDATE tasks SET list_id = ?1, updated_at = ?2 WHERE id = ?3", + params![new_list_id, chrono::Utc::now().format("%Y-%m-%d %H:%M:%S").to_string(), task_id], + )?; + Ok(()) + } + pub fn reorder_task(&self, task_id: &str, new_position: i64) -> SqlResult<()> { let conn = self.conn.lock().unwrap(); let (old_position, list_id): (i64, String) = conn.query_row( @@ -281,6 +295,7 @@ impl Db { SyncAction::Update => "Update", SyncAction::Delete => "Delete", SyncAction::Reorder => "Reorder", + SyncAction::Move => "Move", SyncAction::CreateList => "CreateList", SyncAction::DeleteList => "DeleteList", }; @@ -328,10 +343,19 @@ impl Db { pub fn update_list_id(&self, old_id: &str, new_id: &str) -> SqlResult<()> { let conn = self.conn.lock().unwrap(); - conn.execute( - "UPDATE task_lists SET id = ?1 WHERE id = ?2", - params![new_id, old_id], - )?; + let title: Option = conn.query_row( + "SELECT title FROM task_lists WHERE id = ?1", + params![old_id], + |row| row.get(0), + ).ok(); + conn.execute("DELETE FROM task_lists WHERE id = ?1", params![old_id])?; + conn.execute("DELETE FROM task_lists WHERE id = ?1", params![new_id])?; + if let Some(title) = title { + conn.execute( + "INSERT INTO task_lists (id, title) VALUES (?1, ?2)", + params![new_id, title], + )?; + } conn.execute( "UPDATE tasks SET list_id = ?1 WHERE list_id = ?2", params![new_id, old_id], @@ -340,9 +364,63 @@ impl Db { "UPDATE sync_queue SET list_id = ?1 WHERE list_id = ?2", params![new_id, old_id], )?; + conn.execute( + "INSERT OR REPLACE INTO list_id_map (old_id, new_id) VALUES (?1, ?2)", + params![old_id, new_id], + )?; Ok(()) } + pub fn resolve_list_id(&self, list_id: &str) -> String { + if !list_id.contains('-') { + return list_id.to_string(); + } + let conn = self.conn.lock().unwrap(); + // Check if this list_id has been mapped to a server ID + let result: Option = conn + .query_row( + "SELECT new_id FROM list_id_map WHERE old_id = ?1", + params![list_id], + |row| row.get(0), + ) + .ok(); + // Also check if task_lists has this id directly (already a server ID from old schema) + let result = result.or_else(|| { + conn.query_row( + "SELECT id FROM task_lists WHERE id = ?1 AND id NOT LIKE '%-%'", + params![list_id], + |row| row.get(0), + ) + .ok() + }); + // Also check if task_lists has this list with a different id (same title) + let result = result.or_else(|| { + conn.query_row( + "SELECT id FROM task_lists WHERE title = (SELECT title FROM task_lists WHERE id = ?1) AND id NOT LIKE '%-%'", + params![list_id], + |row| row.get(0), + ) + .ok() + }); + if result.is_none() && list_id.contains('-') { + let log_path = { + let home = std::env::var("HOME").unwrap_or_else(|_| ".".to_string()); + let mut p = std::path::PathBuf::from(home); + p.push(".local/share/task_app/sync.log"); + p + }; + if let Ok(mut f) = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(log_path) + { + use std::io::Write; + let _ = writeln!(f, "[task_app] resolve_list_id: {} NOT FOUND in list_id_map", list_id); + } + } + result.unwrap_or_else(|| list_id.to_string()) + } + #[allow(dead_code)] pub fn update_sync_list_id(&self, old_id: &str, new_id: &str) -> SqlResult<()> { let conn = self.conn.lock().unwrap(); @@ -366,6 +444,7 @@ impl Db { "\"Update\"" | "Update" => SyncAction::Update, "\"Delete\"" | "Delete" => SyncAction::Delete, "\"Reorder\"" | "Reorder" => SyncAction::Reorder, + "\"Move\"" | "Move" => SyncAction::Move, "\"CreateList\"" | "CreateList" => SyncAction::CreateList, "\"DeleteList\"" | "DeleteList" => SyncAction::DeleteList, _ => SyncAction::Update, diff --git a/src/main.rs b/src/main.rs index 3c2a31d..de3b00c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,6 +20,28 @@ use crate::infrastructure::api::ApiClient; use crate::infrastructure::db::Db; use crate::ui::{draw, AppView, NetworkStatus}; +fn log_file_path() -> PathBuf { + let data_dir = dirs_data_dir(); + std::fs::create_dir_all(&data_dir).ok(); + data_dir.join("sync.log") +} + +pub fn log_msg(msg: &str) { + if let Ok(mut f) = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(log_file_path()) + { + use std::io::Write; + let _ = writeln!(f, "{}", msg); + } +} + +fn dirs_data_dir() -> PathBuf { + let home = std::env::var("HOME").unwrap_or_else(|_| ".".to_string()); + PathBuf::from(home).join(".local/share/task_app") +} + fn find_secret_file() -> Option { if let Ok(path) = std::env::var("GOOGLE_CLIENT_SECRET_FILE") { let p = PathBuf::from(&path); @@ -263,6 +285,12 @@ async fn push_sync( let mut all_ok = true; + log_msg(&format!("[task_app] push_sync: {} items in queue", items.len())); + for (idx, item) in items.iter().enumerate() { + log_msg(&format!("[task_app] item[{}]: action={:?} task={} list={} payload_len={}", + idx, item.action, item.task_id, item.list_id, item.payload.len())); + } + // First pass: CreateList items (so list IDs are updated before task operations) for i in 0..items.len() { if items[i].action != SyncAction::CreateList { @@ -274,19 +302,22 @@ async fn push_sync( }); match api.create_list(&list.title).await { Ok(server_list) => { + log_msg(&format!("[task_app] CreateList success: local={} -> server={}", + items[i].task_id, server_list.id)); if server_list.id != items[i].task_id { let _ = db.update_list_id(&items[i].task_id, &server_list.id); - // Update list_id in remaining items of this batch for j in (i + 1)..items.len() { if items[j].list_id == items[i].task_id { + log_msg(&format!("[task_app] -> updating batch item[{}] list_id: {} -> {}", + j, items[j].list_id, server_list.id)); items[j].list_id = server_list.id.clone(); } } } } Err(err) => { - eprintln!("[task_app] Sync failed (retry {}/{}): action=CreateList list={} error={}", - items[i].retries, MAX_SYNC_RETRIES, items[i].task_id, err); + log_msg(&format!("[task_app] Sync failed (retry {}/{}): action=CreateList list={} error={}", + items[i].retries, MAX_SYNC_RETRIES, items[i].task_id, err)); if items[i].retries < MAX_SYNC_RETRIES { let _ = db.push_sync_with_retry( SyncAction::CreateList, @@ -306,11 +337,15 @@ async fn push_sync( if item.action == SyncAction::CreateList { continue; } + let api_list_id = db.resolve_list_id(&item.list_id); + if api_list_id != item.list_id { + log_msg(&format!("[task_app] -> resolved list_id for API call: {} -> {}", item.list_id, api_list_id)); + } let result = match item.action { SyncAction::Create => { let task = serde_json::from_str::(&item.payload).unwrap_or_else(|_| Task { id: item.task_id.clone(), - list_id: item.list_id.clone(), + list_id: api_list_id.clone(), title: String::new(), notes: None, status: TaskStatus::NeedsAction, @@ -319,7 +354,7 @@ async fn push_sync( created_at: None, updated_at: None, }); - api.create_task(&item.list_id, &task).await.map(|server_task| { + api.create_task(&api_list_id, &task).await.map(|server_task| { if server_task.id != item.task_id { let _ = db.update_task_id(&item.task_id, &server_task.id); let _ = db.update_sync_task_id(&item.task_id, &server_task.id); @@ -329,7 +364,7 @@ async fn push_sync( SyncAction::Update => { let task = serde_json::from_str::(&item.payload).unwrap_or_else(|_| Task { id: item.task_id.clone(), - list_id: item.list_id.clone(), + list_id: api_list_id.clone(), title: String::new(), notes: None, status: TaskStatus::NeedsAction, @@ -338,13 +373,22 @@ async fn push_sync( created_at: None, updated_at: None, }); - api.update_task(&item.list_id, &task).await + api.update_task(&api_list_id, &task).await } SyncAction::Delete => { - api.delete_task(&item.list_id, &item.task_id).await + api.delete_task(&api_list_id, &item.task_id).await } SyncAction::Reorder => { - api.move_task(&item.list_id, &item.task_id, None, None).await + api.move_task(&api_list_id, &item.task_id, None, None).await + } + SyncAction::Move => { + let move_payload: MovePayload = serde_json::from_str(&item.payload).unwrap_or_else(|_| MovePayload { + destination_list_id: String::new(), + }); + let resolved_dest = db.resolve_list_id(&move_payload.destination_list_id); + log_msg(&format!("[task_app] Move: task={} source={} dest_raw={} dest_resolved={}", + item.task_id, api_list_id, move_payload.destination_list_id, resolved_dest)); + api.move_task(&api_list_id, &item.task_id, None, Some(&resolved_dest)).await } SyncAction::DeleteList => { api.delete_list(&item.task_id).await @@ -353,19 +397,36 @@ async fn push_sync( }; if let Err(err) = result { - eprintln!("[task_app] Sync failed (retry {}/{}): action={:?} task={} error={}", - item.retries, MAX_SYNC_RETRIES, item.action, item.task_id, err); - if item.retries < MAX_SYNC_RETRIES { + log_msg(&format!("[task_app] Sync failed (retry {}/{}): action={:?} task={} list_id={} error={}", + item.retries, MAX_SYNC_RETRIES, item.action, item.task_id, item.list_id, err)); + let resolved_list_id = db.resolve_list_id(&item.list_id); + if resolved_list_id != item.list_id { + log_msg(&format!("[task_app] -> resolved list_id: {} -> {}", item.list_id, resolved_list_id)); + } + if resolved_list_id.contains('-') { + log_msg("[task_app] -> list_id is local UUID (list not synced), dropping item"); + } else if item.retries < MAX_SYNC_RETRIES { + let re_payload = if item.action == SyncAction::Move { + let move_payload: MovePayload = serde_json::from_str(&item.payload).unwrap_or_else(|_| MovePayload { + destination_list_id: String::new(), + }); + let resolved_dest = db.resolve_list_id(&move_payload.destination_list_id); + serde_json::to_string(&MovePayload { + destination_list_id: resolved_dest, + }).unwrap_or_else(|_| item.payload.clone()) + } else { + item.payload.clone() + }; let _ = db.push_sync_with_retry( item.action.clone(), &item.task_id, - &item.list_id, - &item.payload, + &resolved_list_id, + &re_payload, item.retries + 1, ); } else { - eprintln!("[task_app] Dropping sync item after {} failed attempts: action={:?} task={}", - MAX_SYNC_RETRIES, item.action, item.task_id); + log_msg(&format!("[task_app] Dropping sync item after {} failed attempts: action={:?} task={}", + MAX_SYNC_RETRIES, item.action, item.task_id)); } all_ok = false; } @@ -467,7 +528,7 @@ async fn refresh_calendar( *guard = events; } Err(e) => { - eprintln!("[task_app] Calendar fetch failed: {}", e); + log_msg(&format!("[task_app] Calendar fetch failed: {}", e)); *network_status.lock().await = NetworkStatus::Offline; } }