diff --git a/CHANGELOG.md b/CHANGELOG.md index c3e24e6..98dfa20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- **Configurable cache location** - New `[storage] data_dir` key sets the directory holding the local SQLite cache (`terminalist.db`). Omit it for the platform default; a leading `~` is expanded. See `docs/CONFIGURATION.md` — note that the database holds your API token in plaintext, so it should not live in a shared or cloud-synced directory +- **Working automatic sync** - `[sync] auto_sync_interval_minutes` now actually schedules background syncs. It was previously accepted, documented, and ignored: nothing ever read it, so the app only synced at startup or when you pressed `r` + +### Changed +- **Persistent local cache** - The database is now kept across launches instead of being deleted and rebuilt at every startup, and each sync reconciles it against Todoist (insert/update/delete) rather than rewriting it. Startup no longer waits on a full re-download, and an existing cache from an older version is adopted in place rather than duplicated +- **Non-blocking startup and sync** - The app paints your cached tasks immediately and syncs in the background, so navigation works while a sync runs. The blocking centered "Loading data" overlay is replaced by a small status toast in the corner of the task list: it shows progress, auto-hides a few seconds after a successful sync, and stays up after a failure until you press a key +- **Faster sync** - Projects, tasks, labels, and sections are fetched concurrently rather than one after another +- **Completed and deleted tasks leave the cache sooner** - A task you complete or delete is now removed from the local cache at the next sync, rather than lingering until the next launch. The window in which such a task is still visible locally is now at most one auto-sync interval + ## [0.5.0] - 2026-03-25 ### Added diff --git a/Cargo.lock b/Cargo.lock index 38e61ba..081adba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -774,6 +774,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd2e7510819d6fbf51a5545c8f922716ecfb14df168a3242f7d33e0239efe6a1" +[[package]] +name = "fastrand" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" + [[package]] name = "fern" version = "0.7.1" @@ -2862,6 +2868,12 @@ dependencies = [ "digest", ] +[[package]] +name = "sha1_smol" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbfa15b3dddfee50a0fff136974b3e1bde555604ba463834a7eb7deb6417705d" + [[package]] name = "sha2" version = "0.10.9" @@ -3337,6 +3349,19 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom 0.4.3", + "once_cell", + "rustix", + "windows-sys 0.61.2", +] + [[package]] name = "termina" version = "0.3.3" @@ -3366,6 +3391,7 @@ dependencies = [ "sea-orm", "serde", "serde_json", + "tempfile", "thiserror 2.0.19", "todoist-api", "tokio", @@ -3850,6 +3876,7 @@ dependencies = [ "getrandom 0.4.3", "js-sys", "serde_core", + "sha1_smol", "wasm-bindgen", ] diff --git a/Cargo.toml b/Cargo.toml index eba910c..68b693f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,4 +42,7 @@ dirs = "6.0" log = "0.4" fern = "0.7" once_cell = "1.21" -uuid = { version = "1.23", features = ["v4", "serde"] } +uuid = { version = "1.23", features = ["v4", "v5", "serde"] } + +[dev-dependencies] +tempfile = "3" diff --git a/README.md b/README.md index f15d0e0..83b50b1 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,8 @@ A terminal application for interacting with Todoist, built in Rust with a modern ## Features - **Interactive TUI Interface** - Beautiful terminal user interface with ratatui -- **Local Data Caching** - Fast, responsive UI with in-memory SQLite storage +- **Local Data Caching** - Fast, responsive UI backed by a persistent SQLite + cache that survives restarts - **Smart Sync** - Automatic sync on startup and manual refresh with 'r' - **Project Management** - Browse projects with hierarchical display - **Task Management** - View, navigate, complete, and create tasks @@ -128,8 +129,10 @@ Essential keyboard shortcuts to get started: ## How It Works Terminalist uses a smart sync mechanism: -- **Fast Startup**: In-memory SQLite database for instant loading -- **Auto Sync**: Syncs with Todoist on startup and every 5 minutes +- **Fast Startup**: A file-backed SQLite cache persists across launches, so + the UI paints cached data immediately while a sync runs in the background +- **Auto Sync**: Syncs with Todoist on startup and every + `auto_sync_interval_minutes` (5 minutes by default; set to `0` to disable) - **Manual Sync**: Press `r` to force refresh from Todoist - **Real-time Updates**: Create, modify, and delete tasks/projects immediately diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5a9385f..3cedfb8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -9,9 +9,17 @@ src/ ├── main.rs # Main application entry point ├── lib.rs # Library exports ├── config.rs # Configuration management +├── constants.rs # Shared UI text and other constants +├── theme.rs # Semantic color theme configuration ├── todoist.rs # Todoist API models & display structs -├── sync.rs # Sync service with API integration -├── storage.rs # Storage initialization +├── storage.rs # Persistent SQLite cache initialization +├── sync/ # Sync service with API integration +│ ├── mod.rs # SyncService and shared sync logic +│ ├── storage.rs # Reconciliation (upsert + delete-missing) +│ ├── projects.rs +│ ├── sections.rs +│ ├── tasks.rs +│ └── labels.rs ├── entities/ # Sea-ORM domain entities │ ├── backend.rs # Backend entity (Todoist, etc.) │ ├── label.rs @@ -31,43 +39,69 @@ src/ │ ├── factory.rs │ ├── todoist.rs # Todoist backend implementation │ └── mod.rs -├── backend_registry.rs # Backend registry system +├── backend_registry.rs # Backend registry (derives a stable UUID) ├── icons.rs # Icon service for terminal compatibility ├── logger.rs # Debug logging system ├── utils/ # Utility modules │ ├── mod.rs │ └── datetime.rs # Date/time utilities └── ui/ # Modern Component-Based Architecture + ├── mod.rs ├── app_component.rs # Main application orchestrator ├── renderer.rs # Modern rendering system + ├── layout.rs # Layout calculations ├── core/ # Core architecture components │ ├── actions.rs # Action system for component communication │ ├── component.rs # Component trait and lifecycle │ ├── context.rs # App context │ ├── event_handler.rs # Event processing system - │ └── task_manager.rs # Background async task management + │ ├── task_manager.rs # Background async task management + │ └── mod.rs └── components/ # UI Components ├── badge.rs ├── dialog_component.rs # Unified modal dialog system + ├── dialogs/ # Per-entity dialog content ├── sidebar_component.rs # Project/label navigation + ├── sidebar_item_component.rs + ├── scrollbar_helper.rs + ├── sync_toast.rs # Non-blocking sync status toast ├── task_list_component.rs # Task management and display - └── task_list_item_component.rs + ├── task_list_item_component.rs + └── mod.rs ``` ## Data Management ### Local Storage -- Data is cached locally in a **file-backed SQLite database** -- Database is recreated from scratch on each startup by syncing with the backend +- Data is cached locally in a **file-backed SQLite database** that persists + across launches — it is opened (or created) rather than deleted and rebuilt +- A backend row's UUID is **derived** from `(backend_type, name)` via + `Uuid::new_v5` when the row is new, and **adopted** from the existing row + when one is already there — including the random UUID written by versions + predating the derived scheme. Either way a relaunch resolves to the same + row, so the cache keyed to it is never orphaned or duplicated +- Sync **reconciles** the cache instead of replacing it: each entity type is + upserted on `(backend_uuid, remote_id)`, then any local row whose + `remote_id` the remote no longer returned is deleted. Local UUIDs stay + stable across syncs. An **empty** fetch is the one exception: it is treated + as "nothing to reconcile" rather than "the remote has nothing", so an + empty-but-successful response cannot blank the cache - Uses Sea-ORM for type-safe database operations - Repository pattern provides clean data access layer - UUID-based primary keys for robust entity management ### Sync Behavior -- **First Run**: Automatically syncs all data from Todoist -- **Startup**: Loads local data instantly, then syncs in background if data is older than 5 minutes -- **Manual Sync**: Press `r` to force refresh from Todoist API -- **Sync Indicators**: Sync progress is shown during operations +- **Startup**: Cached data is loaded and painted immediately, and a sync + with the backend starts in the background without blocking the UI +- **Concurrent Fetch**: Projects, tasks, labels, and sections are fetched + from the backend concurrently (`tokio::join!`) rather than one after + another +- **Auto Sync**: After the first sync of the session, a background sync + fires again once `auto_sync_interval_minutes` has elapsed since the last + one; set it to `0` to disable auto-sync entirely +- **Manual Sync**: Press `r` to force refresh from Todoist API at any time +- **Sync Indicators**: A toast in the lower-right corner of the task list + shows syncing/success/failure status without blocking interaction ### Data Types - **Backends**: Abstract backend entity supporting multiple task management services (Todoist, etc.) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index ed83359..c1313fb 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -41,6 +41,13 @@ show_project_colors = false # Show project colors [logging] enabled = false # Enable logging to file +[storage] +# Directory holding the local SQLite cache. Unset = platform default: +# Linux ~/.local/share/terminalist +# macOS ~/Library/Application Support/terminalist +# Windows %APPDATA%\terminalist +# data_dir = "/path/to/dir" + [theme] accent = "Yellow" # Selection highlight color for the sidebar's currently-selected entry success = "Green" # Completed-task checkmark icon, save/create dialog actions @@ -72,7 +79,8 @@ visual language, so they're always red/orange/blue/white regardless of your `[th ### Sync Configuration - **auto_sync_interval_minutes**: How often to automatically sync with Todoist - - Set to `0` to disable automatic syncing (manual sync only with `r` key) + - Set to `0` to disable automatic syncing entirely + - Manual sync with the `r` key always works, regardless of this setting ### Display Configuration @@ -87,6 +95,24 @@ visual language, so they're always red/orange/blue/white regardless of your `[th - **enabled**: Enable debug logging to file for troubleshooting +### Storage Configuration + +- **data_dir**: Directory holding the local SQLite cache (`terminalist.db`). Omit the key to use + the platform default: `~/.local/share/terminalist` on Linux, `~/Library/Application Support/terminalist` + on macOS, `%APPDATA%\terminalist` on Windows. + - A leading `~` is expanded to your home directory. Relative paths resolve against the directory + you launched Terminalist from. + - The directory is created if it does not exist. If it cannot be created or written, + Terminalist exits with an error rather than silently falling back to the default location — + a wrong path must not send writes to your real database. + - The database persists across launches: it is opened (or created) on + startup rather than deleted, and each sync reconciles it with Todoist + instead of rebuilding it from scratch. + - **The database contains your Todoist API token in plaintext.** Terminalist creates the file + with owner-only permissions (`0600`) on Unix, but do not point `data_dir` at a shared, + world-readable, or cloud-synced directory (Dropbox, iCloud Drive, Syncthing, a network + share) — that would copy your token wherever the directory syncs. + ### Theme Configuration Each field accepts either a named color (`"Black"`, `"Red"`, `"Green"`, `"Yellow"`, `"Blue"`, `"Magenta"`, diff --git a/docs/DEVELOPMENT.md b/docs/DEVELOPMENT.md index 117c730..9575c13 100644 --- a/docs/DEVELOPMENT.md +++ b/docs/DEVELOPMENT.md @@ -66,15 +66,9 @@ This is a fully-featured TUI application for Todoist. You can extend it by: ## Dependencies -This project uses the following Rust crates (see `Cargo.toml` for exact versions): - -- `todoist-api = "0.3.0"` - Unofficial Todoist API client -- `ratatui = "0.29"` - Terminal UI framework -- `crossterm = "0.29"` - Cross-platform terminal handling -- `tokio = "1.x"` - Async runtime -- `sqlx = "0.8"` - Database toolkit with SQLite support -- `serde` - Serialization/deserialization -- `chrono = "0.4"` - Date and time handling -- `anyhow = "1.0"` - Error handling -- `toml = "0.8"` - Configuration file parsing -- `dirs = "5.0"` - Platform-specific directory paths \ No newline at end of file +See [`Cargo.toml`](../Cargo.toml) for the full, exact list of dependencies +and their versions — notably `ratatui` for the TUI, `sea-orm` (with the +`sqlx-sqlite` backend) for the SQLite cache, `todoist-api` for the Todoist +client, `tokio` for async, and `toml`/`dirs` for configuration and platform +paths. A version list copied here would go stale again immediately, so this +guide doesn't duplicate one. diff --git a/src/backend_registry.rs b/src/backend_registry.rs index 361ae78..be445b6 100644 --- a/src/backend_registry.rs +++ b/src/backend_registry.rs @@ -6,7 +6,7 @@ use anyhow::Result; use log::{error, info}; -use sea_orm::{ActiveValue, IntoActiveModel}; +use sea_orm::{ActiveValue, EntityTrait, IntoActiveModel}; use std::collections::HashMap; use std::sync::Arc; use tokio::sync::Mutex; @@ -32,6 +32,19 @@ pub struct BackendRegistry { backends: Arc>, } +/// Namespace for deriving stable backend UUIDs (a fixed, arbitrary v4 UUID). +const BACKEND_NAMESPACE: Uuid = Uuid::from_u128(0x6b1c_2f0e_9d47_4a35_8f21_5c7e_3a9b_0d64); + +/// Derive the UUID for a backend from its type and name. +/// +/// Deterministic: the same pair yields the same UUID on every launch. The local cache keys +/// every project, task, label, and section to this value, so a random UUID per launch would +/// silently duplicate the entire dataset against a persistent database. +#[must_use] +pub fn derive_backend_uuid(backend_type: &str, name: &str) -> Uuid { + Uuid::new_v5(&BACKEND_NAMESPACE, format!("{backend_type}:{name}").as_bytes()) +} + impl BackendRegistry { /// Create a new backend registry. /// @@ -155,10 +168,24 @@ impl BackendRegistry { credentials: String, settings: String, ) -> Result { + use sea_orm::sea_query::OnConflict; + // Validate by creating instance first let backend_instance = factory::create_backend(&backend_type, &credentials)?; - let uuid = Uuid::new_v4(); + let storage = self.storage.lock().await; + + // Reuse the row a previous version wrote, whatever UUID it has, so the cache that hangs + // off that `backend_uuid` stays attached to this backend. Releases predating the derived + // UUID wrote a random v4 here; inserting the derived v5 instead would both violate + // `idx_backends_type_name` (the app would fail to start) and, if that constraint were + // widened rather than respected, strand every cached project/task under a dead + // `backend_uuid` — which the repositories' unfiltered `get_all` would then render as a + // fully duplicated task list. + let uuid = match BackendRepository::get_by_type_and_name(&storage.conn, &backend_type, &name).await? { + Some(existing) => existing.uuid, + None => derive_backend_uuid(&backend_type, &name), + }; let backend_model = backend::ActiveModel { uuid: ActiveValue::Set(uuid), @@ -169,8 +196,14 @@ impl BackendRegistry { settings: ActiveValue::Set(settings), }; - let storage = self.storage.lock().await; - BackendRepository::create(&storage.conn, backend_model).await?; + backend::Entity::insert(backend_model) + .on_conflict( + OnConflict::column(backend::Column::Uuid) + .update_columns([backend::Column::Name, backend::Column::Credentials, backend::Column::Settings]) + .to_owned(), + ) + .exec(&storage.conn) + .await?; // Add to in-memory cache let mut backends = self.backends.lock().await; diff --git a/src/config.rs b/src/config.rs index da7a237..ec0695a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -17,6 +17,8 @@ pub struct Config { pub sync: SyncConfig, pub display: DisplayConfig, pub logging: LoggingConfig, + #[serde(skip_serializing_if = "StorageConfig::is_unset")] + pub storage: StorageConfig, pub theme: Theme, } @@ -29,6 +31,7 @@ struct RawConfig { sync: SyncConfig, display: DisplayConfig, logging: LoggingConfig, + storage: StorageConfig, theme: RawTheme, } @@ -81,6 +84,24 @@ pub struct LoggingConfig { pub enabled: bool, } +/// Storage configuration +#[derive(Debug, Clone, Serialize, Deserialize, Default)] +#[serde(default)] +pub struct StorageConfig { + /// Directory holding the local SQLite cache. + /// + /// Unset means the platform data directory (`~/.local/share/terminalist` on Linux). A + /// leading `~` is expanded; relative paths resolve against the working directory. + pub data_dir: Option, +} + +impl StorageConfig { + /// True when no override is configured, so the section is omitted from generated files. + fn is_unset(&self) -> bool { + self.data_dir.is_none() + } +} + impl Default for UiConfig { fn default() -> Self { Self { @@ -149,6 +170,7 @@ impl Config { sync: raw.sync, display: raw.display, logging: raw.logging, + storage: raw.storage, theme, }; @@ -208,6 +230,13 @@ impl Config { anyhow::bail!("Invalid time_format '{}': {}", self.display.time_format, e); } + // Validate storage settings + if let Some(data_dir) = &self.storage.data_dir { + if data_dir.as_os_str().is_empty() || data_dir.to_string_lossy().trim().is_empty() { + anyhow::bail!("storage.data_dir must not be empty; remove the key to use the default location"); + } + } + Ok(()) } @@ -222,7 +251,23 @@ impl Config { chrono::Local::now().format(datetime::TODOIST_DATE_FORMAT) ); - let full_content = header + &toml_content; + // `storage.data_dir` is omitted from serialization when unset, so document it as a + // comment to keep --generate-config a complete reference. The `[storage]` header + // itself is emitted live (not commented out): an empty `[storage]` table deserializes + // to the default, and a live header keeps the commented `data_dir` line inside its own + // section — appending this whole block as a comment after the serialized body would + // place it after `[theme]`, so uncommenting just `data_dir` would silently land it + // inside `[theme]` instead, which discards unknown keys instead of failing. + let storage_docs = "\n\ +[storage]\n\ +# Directory holding the local SQLite cache. Unset = platform default:\n\ +# Linux ~/.local/share/terminalist\n\ +# macOS ~/Library/Application Support/terminalist\n\ +# Windows %APPDATA%\\terminalist\n\ +# A leading ~ is expanded; relative paths resolve against the working directory.\n\ +# data_dir = \"/path/to/dir\"\n"; + + let full_content = header + &toml_content + storage_docs; // Ensure the parent directory exists if let Some(parent) = path.as_ref().parent() { diff --git a/src/constants.rs b/src/constants.rs index a6f8723..178143f 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -27,7 +27,6 @@ pub const SUCCESS_LABEL_DELETED: &str = "✅ Label deleted"; pub const SUCCESS_LABEL_UPDATED: &str = "✅ Label updated"; pub const SUCCESS_TASK_PRIORITY_UPDATED: &str = "✅ Task priority updated to P"; pub const SUCCESS_TASK_RESTORED: &str = "✅ Task restored"; -pub const SUCCESS_SYNC_COMPLETED: &str = "Sync completed successfully"; // Error Messages pub const ERROR_TASK_COMPLETION_FAILED: &str = "❌ Failed to complete task"; @@ -77,8 +76,6 @@ pub const ERROR_NO_API_TOKEN: &str = "❌ Error: TODOIST_API_TOKEN environment v pub const DIALOG_TITLE_DEBUG_LOGS: &str = "🔍 Debug Logs - Press 'Esc', 'G' or 'q' to close"; pub const UI_CANNOT_DELETE_TODAY_VIEW: &str = "Cannot delete the Today view"; pub const UI_NO_TASK_SELECTED_DUE_DATE: &str = "No task selected to set due date"; -pub const UI_LOADING_DATA: &str = "Loading data"; -pub const UI_SYNCING_WITH_TODOIST: &str = "Syncing with Todoist"; pub const UI_LOADING_DATA_FROM_STORAGE: &str = "Loading data from storage"; // Date header format for upcoming view diff --git a/src/main.rs b/src/main.rs index 6bfd2ea..be66687 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ //! //! * `-h, --help` - Show help message //! * `-V, --version` - Show version information -//! * `-d, --debug` - Use file-backed SQLite database for debugging +//! * `-d, --debug` - Skip the startup sync and work from the cached data already on disk //! * `--generate-config` - Generate a default configuration file //! //! # Environment Variables @@ -60,7 +60,7 @@ async fn main() -> Result<()> { println!("OPTIONS:"); println!(" -h, --help Show this help message"); println!(" -V, --version Show version information"); - println!(" -d, --debug Debug mode: keep database file and skip initial sync"); + println!(" -d, --debug Debug mode: skip the startup sync and use the cached data as-is"); println!(" --generate-config Generate a default configuration file"); println!(); println!("ENVIRONMENT VARIABLES:"); @@ -106,12 +106,16 @@ async fn main() -> Result<()> { } // Initialize storage - let local_storage = Arc::new(Mutex::new(storage::LocalStorage::new(debug_mode).await?)); + let data_dir = storage::resolve_data_dir(config.storage.data_dir.as_deref())?; + let local_storage = Arc::new(Mutex::new(storage::LocalStorage::new_at(&data_dir).await?)); // Initialize backend registry let backend_registry = Arc::new(backend_registry::BackendRegistry::new(local_storage.clone())); + backend_registry.load_backends().await?; - // Create initial Todoist backend (DB is always fresh at startup) + // Create/refresh the Todoist backend. This adopts an existing row's UUID when one is already + // there (including the random one older versions wrote) and derives a stable UUID otherwise, + // so a relaunch just refreshes the credentials and keeps the cache attached. let api_token = std::env::var("TODOIST_API_TOKEN")?; let credentials = serde_json::json!({ "api_token": api_token }).to_string(); diff --git a/src/repositories/backend.rs b/src/repositories/backend.rs index 6d1f544..3cf413f 100644 --- a/src/repositories/backend.rs +++ b/src/repositories/backend.rs @@ -29,6 +29,23 @@ impl BackendRepository { .await?) } + /// Get a backend by its `(backend_type, name)` pair. + /// + /// `idx_backends_type_name` makes the pair unique, so at most one row can match. This is + /// the lookup that lets a relaunch adopt whatever UUID an existing row already has — + /// including the random v4 UUID written by releases predating the derived-UUID scheme — + /// instead of asserting a derived one and orphaning the cache keyed to the old value. + pub async fn get_by_type_and_name(conn: &C, backend_type: &str, name: &str) -> Result> + where + C: ConnectionTrait, + { + Ok(backend::Entity::find() + .filter(backend::Column::BackendType.eq(backend_type)) + .filter(backend::Column::Name.eq(name)) + .one(conn) + .await?) + } + /// Get all backends. pub async fn get_all(conn: &C) -> Result> where diff --git a/src/storage.rs b/src/storage.rs index 0b4e1fd..da9f43a 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -1,40 +1,108 @@ use anyhow::{Context, Result}; use sea_orm::{ConnectOptions, ConnectionTrait, Database, DatabaseConnection, DbBackend, Schema, Statement}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::time::Duration; use crate::entities::{backend, label, project, section, task, task_label}; +/// Filename of the SQLite cache inside the data directory. +pub const DB_FILE_NAME: &str = "terminalist.db"; + +/// Resolve the directory that holds the local SQLite cache. +/// +/// `None` yields the platform data directory (`dirs::data_dir()/terminalist`), which is the +/// historical location. A configured path is used as given, except that a leading `~` is +/// expanded to the user's home directory — without that, `data_dir = "~/foo"` would silently +/// create a directory literally named `~` in the current working directory. Relative paths +/// resolve against the process working directory. +/// +/// # Errors +/// +/// Returns an error if the platform data directory (or, for a `~` path, the home directory) +/// cannot be determined. +pub fn resolve_data_dir(configured: Option<&Path>) -> Result { + match configured { + None => Ok(dirs::data_dir() + .context("Failed to determine the platform data directory")? + .join("terminalist")), + Some(path) => expand_tilde(path), + } +} + +/// Expand a leading `~` / `~/` (and `~\` on Windows) to the home directory. +fn expand_tilde(path: &Path) -> Result { + let Some(text) = path.to_str() else { + return Ok(path.to_path_buf()); + }; + + let remainder = if text == "~" { + Some("") + } else { + text.strip_prefix("~/").or_else(|| text.strip_prefix(r"~\")) + }; + + match remainder { + None => Ok(path.to_path_buf()), + Some(remainder) => { + let home = dirs::home_dir().context("Failed to determine the home directory")?; + Ok(if remainder.is_empty() { + home + } else { + home.join(remainder) + }) + } + } +} + +/// Restrict the database file to owner-only access on Unix. +/// +/// `backends.credentials` holds the raw `TODOIST_API_TOKEN` as plaintext JSON, so the cache is +/// a credential file. Best-effort: a filesystem that cannot represent Unix modes (a mounted +/// exFAT/SMB share, which `data_dir` now lets users point at) must not stop the app from +/// starting, so a failure is logged rather than propagated. No-op on Windows. +#[cfg(unix)] +fn restrict_permissions(db_path: &Path) { + use std::os::unix::fs::PermissionsExt; + + if let Err(e) = std::fs::set_permissions(db_path, std::fs::Permissions::from_mode(0o600)) { + log::warn!( + "Failed to restrict permissions on {} (it holds your API token in plaintext): {e}", + db_path.display() + ); + } +} + +#[cfg(not(unix))] +fn restrict_permissions(_db_path: &Path) {} + /// Local storage manager for Todoist data pub struct LocalStorage { pub conn: DatabaseConnection, } impl LocalStorage { - /// Get the database file path using XDG directories - fn get_db_path() -> Result { - // Always use XDG data directory - let data_dir = dirs::data_dir().context("Failed to get XDG data directory")?; - let app_data_dir = data_dir.join("terminalist"); - - // Create directory if it doesn't exist - std::fs::create_dir_all(&app_data_dir).context("Failed to create application data directory")?; - - Ok(app_data_dir.join("terminalist.db")) + /// Open local storage in the platform's default data directory. + pub async fn new() -> Result { + Self::new_at(resolve_data_dir(None)?).await } - /// Initialize the local storage with SQLite database - pub async fn new(debug_mode: bool) -> Result { - let db_path = Self::get_db_path()?; - - // Normal mode: the DB is a throwaway cache, wiped and rebuilt by initial sync each boot. - // Debug mode: keep an existing file so manually-loaded data (e.g. demo_data.sql) survives - // and is shown without syncing. - let existing = db_path.exists(); - if !debug_mode && existing { - std::fs::remove_file(&db_path)?; - } - + /// Open local storage with the SQLite database inside `data_dir`. + /// + /// The directory and the database are created if missing, and the schema is created + /// idempotently, so an existing cache is reused as-is. Data is reconciled against the + /// backend by [`crate::sync::SyncService`] rather than rebuilt, so the cache survives + /// across launches. + /// + /// # Errors + /// + /// Returns an error if the directory cannot be created, or the connection or schema + /// setup fails. + pub async fn new_at(data_dir: impl AsRef) -> Result { + let data_dir = data_dir.as_ref(); + std::fs::create_dir_all(data_dir) + .with_context(|| format!("Failed to create data directory: {}", data_dir.display()))?; + + let db_path = data_dir.join(DB_FILE_NAME); let database_url = format!("sqlite:{}?mode=rwc", db_path.display()); let mut opt = ConnectOptions::new(database_url); @@ -46,18 +114,19 @@ impl LocalStorage { let conn = Database::connect(opt).await?; - // Enable foreign keys for SQLite conn.execute(Statement::from_string( DbBackend::Sqlite, "PRAGMA foreign_keys = ON;".to_owned(), )) .await?; + restrict_permissions(&db_path); + let storage = LocalStorage { conn }; - // A kept debug file already has its schema; only build it for a fresh file. - if !(debug_mode && existing) { - storage.init_schema().await?; - } + storage + .init_schema() + .await + .with_context(|| format!("Failed to set up the database schema in {}", db_path.display()))?; Ok(storage) } @@ -69,24 +138,52 @@ impl LocalStorage { // Create tables in the correct order (parent tables first). let table_statements = vec![ - schema.create_table_from_entity(backend::Entity), - schema.create_table_from_entity(project::Entity), - schema.create_table_from_entity(section::Entity), - schema.create_table_from_entity(label::Entity), - schema.create_table_from_entity(task::Entity), - schema.create_table_from_entity(task_label::Entity), + schema.create_table_from_entity(backend::Entity).if_not_exists().to_owned(), + schema.create_table_from_entity(project::Entity).if_not_exists().to_owned(), + schema.create_table_from_entity(section::Entity).if_not_exists().to_owned(), + schema.create_table_from_entity(label::Entity).if_not_exists().to_owned(), + schema.create_table_from_entity(task::Entity).if_not_exists().to_owned(), + schema.create_table_from_entity(task_label::Entity).if_not_exists().to_owned(), ]; for statement in table_statements { self.conn.execute(backend.build(&statement)).await?; } + // Releases predating `idx_backends_type_name` had no uniqueness guarantee on + // `(backend_type, name)`, and the old `--debug` mode kept the database file while + // inserting a fresh backend row on every launch — so those installations hold a stack of + // duplicate `("todoist", "My Todoist")` rows. `CREATE UNIQUE INDEX` fails outright + // against them, which would abort startup here before the app could do anything about + // it. Collapse the duplicates first, keeping the lowest `rowid`: that is the oldest row, + // the one `add_backend` will adopt and the one the longest-lived cache is keyed to. + // `backends.uuid` is a blob primary key rather than an INTEGER one, so the table has a + // real implicit `rowid` and insertion order is preserved. + // + // The younger rows' cached projects/tasks/labels/sections go with them via + // `ON DELETE CASCADE` (sqlx applies `PRAGMA foreign_keys = ON` to every pooled + // connection by default, and `new_at` sets it explicitly too), which is the + // point: leaving them behind under a `backend_uuid` no longer in `backends` is exactly + // the duplicated-task-list failure the derived-UUID design exists to prevent. Whatever + // survives is reconciled against the remote by the next sync anyway. + // + // This is a no-op once the index exists, so it costs one scan of a tiny table per launch. + self.conn + .execute(Statement::from_string( + DbBackend::Sqlite, + "DELETE FROM backends WHERE rowid NOT IN (SELECT MIN(rowid) FROM backends GROUP BY backend_type, name)" + .to_owned(), + )) + .await + .context("Failed to collapse duplicate backend rows left by an older version")?; + // Create composite unique indexes for (backend_uuid, remote_id) let indexes = vec![ "CREATE UNIQUE INDEX IF NOT EXISTS idx_projects_backend_remote ON projects(backend_uuid, remote_id)", "CREATE UNIQUE INDEX IF NOT EXISTS idx_sections_backend_remote ON sections(backend_uuid, remote_id)", "CREATE UNIQUE INDEX IF NOT EXISTS idx_labels_backend_remote ON labels(backend_uuid, remote_id)", "CREATE UNIQUE INDEX IF NOT EXISTS idx_tasks_backend_remote ON tasks(backend_uuid, remote_id)", + "CREATE UNIQUE INDEX IF NOT EXISTS idx_backends_type_name ON backends(backend_type, name)", ]; for index_sql in indexes { diff --git a/src/sync/mod.rs b/src/sync/mod.rs index f6e127b..4547f1a 100644 --- a/src/sync/mod.rs +++ b/src/sync/mod.rs @@ -52,7 +52,7 @@ use crate::storage::LocalStorage; /// use tokio::sync::Mutex; /// /// # async fn example() -> anyhow::Result<()> { -/// let storage = Arc::new(Mutex::new(LocalStorage::new(false).await?)); +/// let storage = Arc::new(Mutex::new(LocalStorage::new_at("/tmp/terminalist-example").await?)); /// let backend_registry = Arc::new(BackendRegistry::new(storage)); /// // ... initialize and load backends ... /// # let backend_uuid = uuid::Uuid::new_v4(); @@ -195,8 +195,18 @@ impl SyncService { async fn perform_sync(&self) -> Result { info!("🔄 Starting sync process..."); + // Resolve the backend once and fetch all four resources concurrently. + let backend = self.get_backend().await?; + + let (projects_result, tasks_result, labels_result, sections_result) = tokio::join!( + backend.fetch_projects(), + backend.fetch_tasks(), + backend.fetch_labels(), + backend.fetch_sections(), + ); + // Fetch projects from backend - let projects = match self.get_backend().await?.fetch_projects().await { + let projects = match projects_result { Ok(projects) => { info!("✅ Fetched {} projects from backend", projects.len()); projects @@ -210,7 +220,7 @@ impl SyncService { }; // Fetch all tasks from backend - let tasks = match self.get_backend().await?.fetch_tasks().await { + let tasks = match tasks_result { Ok(tasks) => { info!("✅ Fetched {} tasks from backend", tasks.len()); tasks @@ -224,7 +234,7 @@ impl SyncService { }; // Fetch all labels from backend - let labels = match self.get_backend().await?.fetch_labels().await { + let labels = match labels_result { Ok(labels) => { info!("✅ Fetched {} labels from backend", labels.len()); labels @@ -238,7 +248,7 @@ impl SyncService { }; // Fetch all sections from backend - let sections = match self.get_backend().await?.fetch_sections().await { + let sections = match sections_result { Ok(sections) => { info!("✅ Fetched {} sections from backend", sections.len()); sections @@ -274,17 +284,22 @@ impl SyncService { } info!("✅ Stored labels in database"); - // Store sections BEFORE tasks since tasks have foreign key references to sections - if !sections.is_empty() { - if let Err(e) = self.store_sections_batch(&storage, §ions).await { - error!("❌ Failed to store sections: {e}"); - return Ok(SyncStatus::Error { - message: format!("Failed to store sections: {e}"), - }); - } - info!("✅ Stored sections in database"); + // Store sections BEFORE tasks since tasks have foreign key references to sections. + // The empty case — including a sections fetch that failed above and fell back to an + // empty Vec — needs no special case here: `store_sections_batch` treats an empty + // slice as "nothing to reconcile" rather than "delete everything", the same policy + // all four `store_*_batch` functions apply. Keeping the guard in one layer is what + // stops projects/labels/tasks from silently disagreeing with sections about it. + if let Err(e) = self.store_sections_batch(&storage, §ions).await { + error!("❌ Failed to store sections: {e}"); + return Ok(SyncStatus::Error { + message: format!("Failed to store sections: {e}"), + }); + } + if sections.is_empty() { + info!("⚠️ No sections fetched; the cached sections are left untouched"); } else { - info!("⚠️ No sections to store (skipped due to backend issue)"); + info!("✅ Stored sections in database"); } if let Err(e) = self.store_tasks_batch(&storage, &tasks).await { diff --git a/src/sync/storage.rs b/src/sync/storage.rs index 8d62ec3..1141e19 100644 --- a/src/sync/storage.rs +++ b/src/sync/storage.rs @@ -63,13 +63,26 @@ impl SyncService { } /// Store projects in batch - pub(super) async fn store_projects_batch( + /// + /// An empty `projects` slice means "nothing to reconcile", not "the remote has nothing": + /// `is_not_in(vec![])` matches every row, so an empty-but-successful fetch would otherwise + /// blank the cache. A genuinely emptied account therefore keeps a stale local copy until + /// something comes back — stale beats blank. All four `store_*_batch` functions apply this + /// same guard, so the policy lives in one layer rather than at each call site. + /// + /// Widened to `pub` (from `pub(super)`) solely so integration tests under `tests/sync/` + /// can drive it with fixtures; `perform_sync` remains the only production caller. + pub async fn store_projects_batch( &self, storage: &LocalStorage, projects: &[crate::backend::BackendProject], ) -> Result<()> { use sea_orm::sea_query::OnConflict; + if projects.is_empty() { + return Ok(()); + } + let txn = storage.conn.begin().await?; // First pass: Upsert all projects without parent_uuid relationships @@ -100,6 +113,25 @@ impl SyncService { insert.exec(&txn).await?; } + // Reconcile: anything this backend has locally that the remote no longer returns is + // gone. A failed fetch aborts before reaching this point, so `projects` is always the + // authoritative full list. This MUST run before the parent-relinking pass below: pass 1 + // just set every surviving row's `parent_uuid` to NULL, so nothing currently references + // a project that is about to be deleted here. Running this after pass 2 instead would + // let a still-fetched child project get re-linked to a parent that is being deleted in + // this same call (e.g. the parent was completed/archived remotely), and + // `ON DELETE CASCADE` on the self-referential parent relation would then destroy the + // child too, even though the child was in the fetch. The project entity's parent FK has + // no `on_delete` clause (defaults to `NO ACTION`), so in practice that ordering wouldn't + // silently cascade like the task case below does -- the DELETE would fail an FK check and + // roll back the whole transaction instead. Still wrong, so keep the ordering here too. + let seen: Vec = projects.iter().map(|p| p.remote_id.clone()).collect(); + project::Entity::delete_many() + .filter(project::Column::BackendUuid.eq(self.backend_uuid)) + .filter(project::Column::RemoteId.is_not_in(seen)) + .exec(&txn) + .await?; + // Second pass: Update parent_uuid references to use local UUIDs for backend_project in projects { if let Some(remote_parent_id) = &backend_project.parent_remote_id { @@ -123,13 +155,26 @@ impl SyncService { } /// Store labels in batch - pub(super) async fn store_labels_batch( + /// + /// An empty `labels` slice means "nothing to reconcile", not "the remote has nothing": + /// `is_not_in(vec![])` matches every row, so an empty-but-successful fetch would otherwise + /// blank the cache. A genuinely emptied account therefore keeps a stale local copy until + /// something comes back — stale beats blank. All four `store_*_batch` functions apply this + /// same guard, so the policy lives in one layer rather than at each call site. + /// + /// Widened to `pub` (from `pub(super)`) solely so integration tests under `tests/sync/` + /// can drive it with fixtures; `perform_sync` remains the only production caller. + pub async fn store_labels_batch( &self, storage: &LocalStorage, labels: &[crate::backend::BackendLabel], ) -> Result<()> { use sea_orm::sea_query::OnConflict; + if labels.is_empty() { + return Ok(()); + } + let txn = storage.conn.begin().await?; for backend_label in labels { @@ -151,22 +196,47 @@ impl SyncService { insert.exec(&txn).await?; } + // Reconcile: anything this backend has locally that the remote no longer returns is + // gone. A failed fetch aborts before reaching this point, so `labels` is always the + // authoritative full list. + let seen: Vec = labels.iter().map(|l| l.remote_id.clone()).collect(); + label::Entity::delete_many() + .filter(label::Column::BackendUuid.eq(self.backend_uuid)) + .filter(label::Column::RemoteId.is_not_in(seen)) + .exec(&txn) + .await?; + txn.commit().await?; Ok(()) } /// Store tasks in batch - pub(super) async fn store_tasks_batch( - &self, - storage: &LocalStorage, - tasks: &[crate::backend::BackendTask], - ) -> Result<()> { + /// + /// An empty `tasks` slice means "nothing to reconcile", not "the remote has nothing": + /// `is_not_in(vec![])` matches every row, so an empty-but-successful fetch would otherwise + /// blank the cache. A genuinely emptied account therefore keeps a stale local copy until + /// something comes back — stale beats blank. All four `store_*_batch` functions apply this + /// same guard, so the policy lives in one layer rather than at each call site. + /// + /// Widened to `pub` (from `pub(super)`) solely so integration tests under `tests/sync/` + /// can drive it with fixtures; `perform_sync` remains the only production caller. + pub async fn store_tasks_batch(&self, storage: &LocalStorage, tasks: &[crate::backend::BackendTask]) -> Result<()> { use sea_orm::sea_query::OnConflict; + if tasks.is_empty() { + return Ok(()); + } + let txn = storage.conn.begin().await?; - // Track task labels for later processing + // Everything below is driven by the tasks this call actually stores, never by the input + // slice: the loop `continue`s over tasks whose project does not resolve locally, and a + // skipped task must not be treated as seen (its stale row would survive the delete pass + // with stale content and a stale `project_uuid`), must not be relinked, and must not + // become the parent something else is relinked to. let mut task_labels_map: Vec<(Uuid, Vec)> = Vec::new(); + let mut parent_links: Vec<(Uuid, String)> = Vec::new(); + let mut seen: Vec = Vec::with_capacity(tasks.len()); // First pass: Upsert all tasks without parent_uuid relationships for backend_task in tasks { @@ -239,37 +309,56 @@ impl SyncService { if let Some(task) = TaskRepository::get_by_remote_id(&txn, &self.backend_uuid, &backend_task.remote_id).await? { + seen.push(backend_task.remote_id.clone()); task_labels_map.push((task.uuid, label_names)); + if let Some(remote_parent_id) = &backend_task.parent_remote_id { + parent_links.push((task.uuid, remote_parent_id.clone())); + } } } - // Second pass: Update parent_uuid references to use local UUIDs - for backend_task in tasks { - if let Some(remote_parent_id) = &backend_task.parent_remote_id { - if let Some(parent) = - TaskRepository::get_by_remote_id(&txn, &self.backend_uuid, remote_parent_id).await? - { - if let Some(task) = - TaskRepository::get_by_remote_id(&txn, &self.backend_uuid, &backend_task.remote_id).await? - { - let mut active_model: task::ActiveModel = task.into(); - active_model.parent_uuid = ActiveValue::Set(Some(parent.uuid)); - TaskRepository::update(&txn, active_model).await?; - } - } + // Reconcile: anything this backend has locally that the remote no longer returns is + // gone. A failed fetch aborts before reaching this point, so `tasks` is always the + // authoritative full list. `task_labels` rows for deleted tasks cascade automatically. + // + // This MUST run before the parent-relinking pass below, and it MUST run before that pass + // for a specific reason: Todoist's fetch never returns completed tasks (see + // `TodoistBackend::task_to_backend`), so completing a parent task makes it vanish from + // `tasks` while its still-open subtasks remain. Pass 1 already set every surviving row's + // `parent_uuid` to NULL, so at this point nothing references the parent that is about to + // be deleted. If this delete ran after pass 2 instead, pass 2 would look the parent up + // by remote_id against the stale (not yet reconciled) local row and re-link the subtask + // to it, and the task entity's self-referential parent relation is `ON DELETE CASCADE` + // -- deleting the parent would then destroy the subtask too, even though the subtask WAS + // in the fetch. The subtask would reappear on the next sync as a brand-new INSERT with a + // new uuid, breaking the uuid-stability invariant the UI now anchors selection to. + task::Entity::delete_many() + .filter(task::Column::BackendUuid.eq(self.backend_uuid)) + .filter(task::Column::RemoteId.is_not_in(seen)) + .exec(&txn) + .await?; + + // Second pass: Update parent_uuid references to use local UUIDs. The parent lookup runs + // against the post-delete state on purpose, so a parent that dropped out of the fetch + // (completed remotely) or was skipped above simply leaves the child's parent NULL + // instead of relinking it to a row that is gone. + for (task_uuid, remote_parent_id) in parent_links { + if let Some(parent) = TaskRepository::get_by_remote_id(&txn, &self.backend_uuid, &remote_parent_id).await? { + let active_model = task::ActiveModel { + uuid: ActiveValue::Unchanged(task_uuid), + parent_uuid: ActiveValue::Set(Some(parent.uuid)), + ..Default::default() + }; + TaskRepository::update(&txn, active_model).await?; } } - // Delete task-label relationships only for tasks being synced - for backend_task in tasks { - if let Some(task) = - TaskRepository::get_by_remote_id(&txn, &self.backend_uuid, &backend_task.remote_id).await? - { - task_label::Entity::delete_many() - .filter(task_label::Column::TaskUuid.eq(task.uuid)) - .exec(&txn) - .await?; - } + // Delete task-label relationships only for the tasks actually stored above + for (task_uuid, _) in &task_labels_map { + task_label::Entity::delete_many() + .filter(task_label::Column::TaskUuid.eq(*task_uuid)) + .exec(&txn) + .await?; } // Recreate relationships @@ -303,13 +392,26 @@ impl SyncService { } /// Store sections in batch - pub(super) async fn store_sections_batch( + /// + /// An empty `sections` slice means "nothing to reconcile", not "the remote has nothing": + /// `is_not_in(vec![])` matches every row, so an empty-but-successful fetch would otherwise + /// blank the cache. A genuinely emptied account therefore keeps a stale local copy until + /// something comes back — stale beats blank. All four `store_*_batch` functions apply this + /// same guard, so the policy lives in one layer rather than at each call site. + /// + /// Widened to `pub` (from `pub(super)`) solely so integration tests under `tests/sync/` + /// can drive it with fixtures; `perform_sync` remains the only production caller. + pub async fn store_sections_batch( &self, storage: &LocalStorage, sections: &[crate::backend::BackendSection], ) -> Result<()> { use sea_orm::sea_query::OnConflict; + if sections.is_empty() { + return Ok(()); + } + let txn = storage.conn.begin().await?; for backend_section in sections { @@ -340,6 +442,16 @@ impl SyncService { insert.exec(&txn).await?; } + // Reconcile: anything this backend has locally that the remote no longer returns is + // gone. A failed fetch aborts before reaching this point, so `sections` is always the + // authoritative full list, and the empty case returned early above. + let seen: Vec = sections.iter().map(|s| s.remote_id.clone()).collect(); + section::Entity::delete_many() + .filter(section::Column::BackendUuid.eq(self.backend_uuid)) + .filter(section::Column::RemoteId.is_not_in(seen)) + .exec(&txn) + .await?; + txn.commit().await?; Ok(()) } diff --git a/src/ui/app_component.rs b/src/ui/app_component.rs index 8cfc4ea..ba78e4d 100644 --- a/src/ui/app_component.rs +++ b/src/ui/app_component.rs @@ -3,21 +3,23 @@ use crate::constants::*; use crate::entities::{label, project, section, task}; use crate::sync::{SyncService, SyncStatus}; use crate::theme::{self, ThemeWarning}; -use crate::ui::components::{DialogComponent, SidebarComponent, TaskListComponent}; +use crate::ui::components::sync_toast::sync_completed_successfully; +use crate::ui::components::{should_auto_sync, DialogComponent, SidebarComponent, SyncToast, TaskListComponent}; use crate::ui::core::SidebarSelection; use crate::ui::core::{ - actions::{Action, DialogType}, + actions::{Action, DialogType, SelectionPolicy}, event_handler::EventType, task_manager::{TaskId, TaskManager}, Component, }; use crate::utils::datetime; use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; -use log::info; +use log::{error, info}; use ratatui::{ layout::{Constraint, Layout, Rect}, Frame, }; +use std::time::Instant; use tokio::sync::mpsc; use uuid::Uuid; @@ -64,6 +66,7 @@ pub struct AppComponent { sidebar: SidebarComponent, task_list: TaskListComponent, dialog: DialogComponent, + sync_toast: SyncToast, // Application state state: AppState, @@ -79,7 +82,12 @@ pub struct AppComponent { // Simple UI state should_quit: bool, active_sync_task: Option, - is_initial_sync: bool, + /// True from startup until the first local data load lands and establishes the sidebar + /// selection from `config.ui.default_project`. Nothing else may set the initial selection: + /// doing it a second time snaps the user back to the default view and rebuilds the task + /// list underneath them. + initial_selection_pending: bool, + last_sync_attempt_at: Option, // Layout state sidebar_visible: bool, @@ -108,6 +116,7 @@ impl AppComponent { sidebar, task_list, dialog, + sync_toast: SyncToast::new(), state, sync_service, task_manager, @@ -116,7 +125,8 @@ impl AppComponent { config, should_quit: false, active_sync_task: None, - is_initial_sync: false, + initial_selection_pending: false, + last_sync_attempt_at: None, sidebar_width: 30, // Default width screen_width: 100, // Default width screen_height: 50, // Default height @@ -137,6 +147,32 @@ impl AppComponent { self.active_sync_task.is_some() } + /// Whether a tick can change what the sync toast shows — true only while a success + /// toast is counting down to its own expiry. + /// + /// The render loop uses this, not "is the toast visible", to decide whether a tick has + /// to repaint. A failed sync's toast stays up until the user dismisses it, so keying the + /// repaint off visibility would redraw the whole TUI ten times a second for as long as + /// that notice is on screen. + pub fn sync_toast_expires_on_tick(&self) -> bool { + self.sync_toast.expires_on_tick() + } + + /// The sidebar entry the task list is currently showing. + pub fn sidebar_selection(&self) -> &SidebarSelection { + &self.state.sidebar_selection + } + + /// Await the next background action rather than draining whatever has already arrived. + /// + /// Same channel as [`Self::process_background_actions`], which the render loop polls + /// without blocking. Widened for the integration tests under `tests/ui/`, which need to + /// step the component across a background data load deterministically — no polling, no + /// sleeping, no wall-clock dependency. + pub async fn next_background_action(&mut self) -> Option { + self.background_action_rx.recv().await + } + /// Get total number of tasks pub fn total_tasks(&self) -> usize { self.state.tasks.len() @@ -148,21 +184,29 @@ impl AppComponent { } /// Trigger initial sync on startup (unless in debug mode) + /// + /// The initial *selection* is established by the local data load scheduled here, not by the + /// sync finishing. `initial_selection_pending` stays set until that load is handled, so a + /// sync that fails, succeeds, or never happens at all (debug mode) all end up in the same + /// place. pub fn trigger_initial_sync(&mut self) { + if self.active_sync_task.is_some() { + return; + } + + // Paint cached data immediately so the user has something to look at and navigate; the + // background sync refreshes the view again once it completes. Debug mode differs by + // exactly one thing — it skips the network sync — so the pending-selection lifecycle is + // identical on both paths and cannot drift apart. + self.initial_selection_pending = true; + self.schedule_initial_data_fetch(); + if self.sync_service.is_debug_mode() { - info!("AppComponent: Skipping initial sync (debug mode)"); - // In debug mode, just load existing data from database - self.is_initial_sync = true; - self.schedule_initial_data_fetch(); - self.is_initial_sync = false; + info!("AppComponent: Skipping initial sync (debug mode), loading cached data only"); } else { info!("AppComponent: Starting initial sync"); - if self.active_sync_task.is_none() { - self.is_initial_sync = true; - self.start_background_sync(); - // Data fetch will be triggered automatically when sync completes - info!("AppComponent: Initial sync scheduled"); - } + self.start_background_sync(); + info!("AppComponent: Initial sync scheduled"); } } @@ -209,8 +253,13 @@ impl AppComponent { ); } - /// Update all components with current data - fn sync_component_data(&mut self) { + /// Update all components with current data. + /// + /// `selection_policy` is forwarded to the task list's reload and only matters when the + /// data actually changed (a `DataLoaded`/`InitialDataLoaded` handler). Callers that are not + /// reacting to a fresh data load pass `SelectionPolicy::KeepIndex`: the data has not + /// changed, so it is a no-op, and it is the conservative choice. + fn sync_component_data(&mut self, selection_policy: SelectionPolicy) { // Update sidebar self.sidebar.update_data(self.state.projects.clone(), self.state.labels.clone()); self.sidebar.selection = self.state.sidebar_selection.clone(); @@ -225,6 +274,7 @@ impl AppComponent { self.state.projects.clone(), self.state.labels.clone(), self.state.sidebar_selection.clone(), + selection_policy, ); // Update dialog @@ -236,6 +286,9 @@ impl AppComponent { self.state.tasks.clone(), ); self.dialog.set_sync_service(self.sync_service.clone()); + + // Update sync toast + self.sync_toast.update_theme(self.config.theme.clone()); } /// Handle global keyboard shortcuts that aren't component-specific @@ -446,6 +499,12 @@ impl AppComponent { Action::None } Action::StartSync => { + // Show the "syncing" toast regardless of whether this call actually + // starts a new sync: it also covers the async "sync started" + // notification that arrives after `start_background_sync` was already + // called directly (e.g. from `trigger_initial_sync`), which otherwise + // would hit the "already in progress" branch below and never surface. + self.sync_toast.started(); if self.active_sync_task.is_none() { info!("Starting background sync"); self.state.loading = true; @@ -457,30 +516,51 @@ impl AppComponent { } Action::RefreshLocalData => { info!("Refreshing local data from database (debug mode)"); - // Schedule a data fetch directly from local storage without API sync - self.schedule_data_fetch(); + // Schedule a data fetch directly from local storage without API sync. User- + // initiated (the debug-mode `R` key), so the cursor stays on its row. + self.schedule_data_fetch(SelectionPolicy::KeepIndex); Action::None } Action::SyncCompleted(status) => { info!("Sync: Completed with status {:?}", status); self.active_sync_task = None; self.state.loading = false; + let now = Instant::now(); + // Record every terminal attempt, success or failure, so a failure waits + // a full auto-sync interval before retrying instead of re-firing on the + // very next tick (see `should_auto_sync`'s doc comment). + self.last_sync_attempt_at = Some(now); + + if sync_completed_successfully(&status) { + self.sync_toast.succeeded(now); + } else { + error!("Sync: Completed with a non-success status: {:?}", status); + self.sync_toast.failed(); + } - // Extract data from sync status and update components + // Extract data from sync status and update components. The actual data reload + // (if any) happens asynchronously via `update_data_from_sync`'s `DataLoaded`, + // which carries its own policy; this call just repaints components with + // whatever data is already in `self.state`, unchanged, so KeepIndex is a no-op. self.update_data_from_sync(status); - self.sync_component_data(); + self.sync_component_data(SelectionPolicy::KeepIndex); - self.state.info_message = Some(SUCCESS_SYNC_COMPLETED.to_string()); - info!("Sync: Showing completion info dialog"); - Action::ShowDialog(DialogType::Info(self.state.info_message.clone().unwrap())) + Action::None } Action::SyncFailed(error) => { - info!("Sync: Failed with error: {}", error); + error!("Sync: Failed with error: {}", error); self.active_sync_task = None; self.state.loading = false; - self.is_initial_sync = false; // Reset flag on failure - self.state.error_message = Some(error); - Action::ShowDialog(DialogType::Error(self.state.error_message.clone().unwrap_or_default())) + // `initial_selection_pending` is deliberately NOT cleared here. It is owned by + // the local data load, which runs independently of the sync and still has to + // establish the initial selection; clearing it on a sync failure would leave a + // startup with no network stuck on whatever `SidebarSelection::default()` is, + // ignoring `default_project`. + + // Record the attempt, same reasoning as the `SyncCompleted` arm above. + self.last_sync_attempt_at = Some(Instant::now()); + self.sync_toast.failed(); + Action::None } Action::ShowDialog(ref dialog_type) => { info!("Dialog: Showing dialog {:?}", dialog_type); @@ -516,8 +596,8 @@ impl AppComponent { info!("Navigation: Sidebar selection changed to {}", selection_desc); self.state.sidebar_selection = selection.clone(); - // Reload data for the new selection - self.schedule_data_fetch(); + // Reload data for the new selection. User-initiated navigation, so KeepIndex. + self.schedule_data_fetch(SelectionPolicy::KeepIndex); info!("Navigation: Scheduled data fetch for new selection"); Action::None } @@ -745,15 +825,28 @@ impl AppComponent { // Update app state with loaded data self.state.update_data(projects, labels, sections, tasks); - // Set initial sidebar selection based on config (now we have projects loaded) - self.set_initial_sidebar_selection(); - info!("AppComponent: Set initial sidebar selection after initial data load"); - - // Fetch data for the newly selected sidebar item - self.schedule_data_fetch(); - info!("AppComponent: Scheduled data fetch for initial sidebar selection"); + // The initial selection is established exactly once, here, on the first local + // load after startup. Guarding on the flag (and clearing it immediately) is what + // keeps a later reload from snapping the user back to `default_project` and + // rebuilding the task list under them. + if self.initial_selection_pending { + self.initial_selection_pending = false; + + // Set initial sidebar selection based on config (now we have projects loaded) + self.set_initial_sidebar_selection(); + info!("AppComponent: Set initial sidebar selection after initial data load"); + + // Fetch data for the newly selected sidebar item. User-initiated in effect + // (it applies `default_project`, not a task the user was looking at), so + // KeepIndex. + self.schedule_data_fetch(SelectionPolicy::KeepIndex); + info!("AppComponent: Scheduled data fetch for initial sidebar selection"); + } - self.sync_component_data(); + // No prior selection exists yet at startup, so there is nothing to keep or + // follow: KeepIndex is a no-op here, same as for every other non-data-load + // caller of `sync_component_data`. + self.sync_component_data(SelectionPolicy::KeepIndex); info!("InitialData: Updated all component data after initial data load"); Action::None } @@ -762,6 +855,7 @@ impl AppComponent { labels, sections, tasks, + selection_policy, } => { info!( "Data: Loaded {} projects, {} labels, {} sections, {} tasks", @@ -773,7 +867,7 @@ impl AppComponent { // Update app state with loaded data self.state.update_data(projects, labels, sections, tasks); - self.sync_component_data(); + self.sync_component_data(selection_policy); info!("Data: Updated all component data after data load"); Action::None } @@ -799,8 +893,12 @@ impl AppComponent { } Action::RefreshData => { info!("Data: Refreshing UI data after task operation"); - // Schedule a data fetch to reload current view with updated data - self.schedule_data_fetch(); + // Schedule a data fetch to reload current view with updated data. This is + // always user-initiated (a task operation the user just performed), so the + // cursor stays on its row rather than following the task it just changed -- + // e.g. pressing `t` to mark an overdue task due "today" must not drag the + // cursor along with it as it moves out of the Overdue section. + self.schedule_data_fetch(SelectionPolicy::KeepIndex); Action::None } // Help panel scrolling actions @@ -1090,17 +1188,19 @@ impl AppComponent { ); } + /// Refresh the view from local storage after a sync landed new data. + /// + /// Always the selection-preserving path. The initial sync is not special here: startup + /// already scheduled its own initial load (see [`Self::trigger_initial_sync`]), so routing + /// the *completion* through `schedule_initial_data_fetch` too would re-run + /// `set_initial_sidebar_selection` and undo every bit of navigation the user did while the + /// sync was running — which is exactly what the non-blocking sync exists to allow. The + /// auto-sync timer makes that reset recur with no user action at all. fn update_data_from_sync(&mut self, status: SyncStatus) { - // Only proceed if sync was successful + // Only proceed if sync was successful. The user may be navigating while the sync + // lands, so the reload must anchor to the selected task rather than its index. if matches!(status, SyncStatus::Success) { - if self.is_initial_sync { - // For initial sync, use initial data fetch which sets default selection - self.schedule_initial_data_fetch(); - self.is_initial_sync = false; - } else { - // For manual refresh, use regular data fetch to maintain current selection - self.schedule_data_fetch(); - } + self.schedule_data_fetch(SelectionPolicy::FollowTask); } } @@ -1108,14 +1208,21 @@ impl AppComponent { fn schedule_initial_data_fetch(&mut self) { let _task_id = self.task_manager - .spawn_data_load(self.sync_service.clone(), self.state.sidebar_selection.clone(), true); + .spawn_data_load(self.sync_service.clone(), self.state.sidebar_selection.clone(), None); } - /// Schedule a background task to fetch data after navigation or changes - fn schedule_data_fetch(&mut self) { - let _task_id = - self.task_manager - .spawn_data_load(self.sync_service.clone(), self.state.sidebar_selection.clone(), false); + /// Schedule a background task to fetch data after navigation or changes. + /// + /// `selection_policy` is forwarded onto the resulting `Action::DataLoaded` unchanged, so + /// every call site must state its intent explicitly: `KeepIndex` for a user-initiated + /// reload, `FollowTask` for one triggered by a completed sync. See [`SelectionPolicy`]'s + /// doc comment for the full rationale. + fn schedule_data_fetch(&mut self, selection_policy: SelectionPolicy) { + let _task_id = self.task_manager.spawn_data_load( + self.sync_service.clone(), + self.state.sidebar_selection.clone(), + Some(selection_policy), + ); } /// Process background actions from task manager @@ -1135,6 +1242,21 @@ impl AppComponent { info!("Background: Cleaned up {} finished tasks", count); } + // This is the tick path: advance the toast's own timer (so a success toast can + // expire and a spinner can animate) and decide whether the auto-sync interval + // has elapsed. + let now = Instant::now(); + self.sync_toast.tick(now); + if should_auto_sync( + self.last_sync_attempt_at, + now, + self.config.sync.auto_sync_interval_minutes, + self.active_sync_task.is_some(), + ) { + info!("AppComponent: Auto-sync interval elapsed, starting background sync"); + actions.push(Action::StartSync); + } + actions } @@ -1163,6 +1285,9 @@ impl AppComponent { } } EventType::Key(key) => { + // Any keypress dismisses a failed sync toast (no-op otherwise). + self.sync_toast.dismiss(); + // Route keyboard events to components or handle globally if self.dialog.is_visible() { // Dialog has priority when visible @@ -1212,8 +1337,11 @@ impl AppComponent { // Handle app-level actions let _final_action = self.handle_app_action(action).await; - // Update component data after any changes - self.sync_component_data(); + // Update component data after any changes. Key/mouse/resize events handled here never + // carry a fresh data load (`DataLoaded`/`InitialDataLoaded` only arrive through the + // background-action path in the render loop), so this is always a non-data-load + // caller: KeepIndex. + self.sync_component_data(SelectionPolicy::KeepIndex); Ok(()) } @@ -1270,10 +1398,8 @@ impl Component for AppComponent { } self.task_list.render(f, main_chunks[1]); - // Render sync status if syncing or loading - if self.state.loading || self.is_syncing() { - AppComponent::render_sync_status_impl(self, f, rect); - } + // Render the non-blocking sync toast in the task list's lower-right corner. + self.sync_toast.render(f, main_chunks[1]); // Render dialog on top if visible (includes help dialog) if self.dialog.is_visible() { @@ -1281,46 +1407,3 @@ impl Component for AppComponent { } } } - -impl AppComponent { - /// Render sync status indicator - fn render_sync_status_impl(&self, f: &mut Frame, rect: Rect) { - use ratatui::{ - layout::{Alignment, Constraint, Layout}, - style::Style, - text::{Line, Span}, - widgets::{Block, Borders, Clear, Paragraph}, - }; - - // Calculate centered area for the sync indicator - let popup_area = { - let popup_layout = - Layout::vertical([Constraint::Percentage(40), Constraint::Min(3), Constraint::Percentage(40)]) - .split(rect); - - Layout::horizontal([Constraint::Percentage(30), Constraint::Min(30), Constraint::Percentage(30)]) - .split(popup_layout[1])[1] - }; - - let title = if self.state.loading { - UI_LOADING_DATA - } else { - UI_SYNCING_WITH_TODOIST - }; - - let spinner = "⟳"; - let content = Paragraph::new(Line::from(Span::styled( - format!("{} {}…", spinner, title), - Style::default().fg(self.config.theme.warning), - ))) - .alignment(Alignment::Center) - .block( - Block::default() - .borders(Borders::ALL) - .style(Style::default().fg(self.config.theme.warning)), - ); - - f.render_widget(Clear, popup_area); - f.render_widget(content, popup_area); - } -} diff --git a/src/ui/components/mod.rs b/src/ui/components/mod.rs index fab559c..c309687 100644 --- a/src/ui/components/mod.rs +++ b/src/ui/components/mod.rs @@ -14,6 +14,7 @@ //! - [`dialog_component`] - Modal dialog framework //! - [`dialogs`] - Specific dialog implementations (create, edit, confirm) //! - [`sidebar_component`] - Navigation sidebar with project/view selection +//! - [`sync_toast`] - Non-blocking sync status indicator //! - [`task_list_component`] - Main task display and management interface //! - [`task_list_item_component`] - Individual task rendering and interaction //! @@ -44,10 +45,12 @@ pub mod dialog_component; pub mod dialogs; pub mod sidebar_component; pub mod sidebar_item_component; +pub mod sync_toast; pub mod task_list_component; pub mod task_list_item_component; // Public exports for external use pub use dialog_component::DialogComponent; pub use sidebar_component::SidebarComponent; +pub use sync_toast::{should_auto_sync, SyncToast}; pub use task_list_component::TaskListComponent; diff --git a/src/ui/components/sync_toast.rs b/src/ui/components/sync_toast.rs new file mode 100644 index 0000000..f150754 --- /dev/null +++ b/src/ui/components/sync_toast.rs @@ -0,0 +1,219 @@ +//! Non-blocking sync status toast for the Terminalist application. +//! +//! Replaces the old centered "Loading data" overlay that blocked interaction while a +//! sync ran. This toast anchors to the bottom-right corner of the task list area so the +//! user can keep navigating while a sync (initial, manual, or automatic) runs in the +//! background. +//! +//! The state machine is a pure function of `(event, now)`, so it can be unit tested +//! without sleeping or touching a real clock source beyond `Instant` values the test +//! constructs itself. See `should_auto_sync` for the companion auto-sync timer decision. + +use crate::sync::SyncStatus; +use crate::theme::Theme; +use ratatui::{ + layout::{Margin, Rect}, + style::Style, + widgets::{Block, Borders, Clear, Paragraph}, + Frame, +}; +use std::time::{Duration, Instant}; + +/// How long the "Synced" success toast stays visible before it hides itself. +const SUCCESS_TOAST_DURATION: Duration = Duration::from_secs(3); + +const SYNCING_TEXT: &str = "⟳ Syncing…"; +const SUCCEEDED_TEXT: &str = "✓ Synced"; +const FAILED_TEXT: &str = "✗ Sync failed"; + +/// The toast's visual state, driven purely by explicit events and an `Instant` the +/// caller supplies (never by reading the wall clock itself). +#[derive(Debug, Clone, Copy, PartialEq)] +pub enum ToastState { + /// Nothing to show. + Hidden, + /// A sync is currently running. + Syncing, + /// A sync just finished successfully; stays visible until `until`. + Succeeded { until: Instant }, + /// A sync failed. Stays visible until the user dismisses it (any keypress). + Failed, +} + +/// A small, non-blocking status indicator anchored to a corner of its host area. +pub struct SyncToast { + state: ToastState, + theme: Theme, +} + +impl Default for SyncToast { + fn default() -> Self { + Self::new() + } +} + +impl SyncToast { + #[must_use] + pub fn new() -> Self { + Self { + state: ToastState::Hidden, + theme: Theme::default(), + } + } + + pub fn update_theme(&mut self, theme: Theme) { + self.theme = theme; + } + + /// A sync has started (or is still running): show the syncing indicator. + /// + /// Replaces whatever was showing before, including a `Failed` toast: the new sync + /// attempt supersedes the stale failure notice. + pub fn started(&mut self) { + self.state = ToastState::Syncing; + } + + /// A sync completed successfully at `now`. Visible for a short window, then hides + /// itself the next time [`Self::tick`] observes it has expired. + pub fn succeeded(&mut self, now: Instant) { + self.state = ToastState::Succeeded { + until: now + SUCCESS_TOAST_DURATION, + }; + } + + /// A sync failed. Stays visible until [`Self::dismiss`] is called. + pub fn failed(&mut self) { + self.state = ToastState::Failed; + } + + /// Clears a failure notice. No-op unless the toast is currently showing a failure, + /// so it's safe to call unconditionally (e.g. on every keypress). + pub fn dismiss(&mut self) { + if matches!(self.state, ToastState::Failed) { + self.state = ToastState::Hidden; + } + } + + /// Advances the state machine: expires a `Succeeded` toast whose window has passed. + /// Does nothing to `Syncing`/`Failed`/`Hidden`. + pub fn tick(&mut self, now: Instant) { + if let ToastState::Succeeded { until } = self.state { + if now >= until { + self.state = ToastState::Hidden; + } + } + } + + #[must_use] + pub fn is_visible(&self) -> bool { + !matches!(self.state, ToastState::Hidden) + } + + /// Whether a plain tick can change what this toast shows. + /// + /// Only `Succeeded` can: [`Self::tick`] is what expires it. `Syncing`'s text is static, + /// `Failed` clears only on a keypress (which already forces a repaint), and `Hidden` shows + /// nothing. The render loop asks this instead of [`Self::is_visible`] so that a `Failed` + /// toast — which by design never expires — cannot pin the TUI at tick rate indefinitely. + #[must_use] + pub fn expires_on_tick(&self) -> bool { + matches!(self.state, ToastState::Succeeded { .. }) + } + + #[must_use] + pub fn text(&self) -> &str { + match self.state { + ToastState::Hidden => "", + ToastState::Syncing => SYNCING_TEXT, + ToastState::Succeeded { .. } => SUCCEEDED_TEXT, + ToastState::Failed => FAILED_TEXT, + } + } + + /// Renders the toast anchored to the bottom-right corner, inset by 1 cell inside + /// `task_list_area` so it never overlaps the task list's own border. Clamps so a + /// narrow terminal never produces a negative or out-of-bounds `Rect`. + pub fn render(&self, f: &mut Frame, task_list_area: Rect) { + if !self.is_visible() { + return; + } + + let inner = task_list_area.inner(Margin { + horizontal: 1, + vertical: 1, + }); + if inner.width == 0 || inner.height == 0 { + return; + } + + let text = self.text(); + let width = (text.chars().count() as u16).saturating_add(2).min(inner.width); + let height = 3u16.min(inner.height); + if width == 0 || height == 0 { + return; + } + + let color = match self.state { + ToastState::Syncing => self.theme.warning, + ToastState::Succeeded { .. } => self.theme.success, + ToastState::Failed => self.theme.danger, + ToastState::Hidden => return, + }; + + let area = Rect { + x: inner.right() - width, + y: inner.bottom() - height, + width, + height, + }; + + let paragraph = Paragraph::new(text) + .style(Style::default().fg(color)) + .block(Block::default().borders(Borders::ALL).style(Style::default().fg(color))); + + f.render_widget(Clear, area); + f.render_widget(paragraph, area); + } +} + +/// Whether a terminal `Action::SyncCompleted(status)` should be shown to the user as a +/// success. +/// +/// Only `SyncStatus::Success` counts. `SyncStatus::Error` is an explicit failure, and +/// `Idle`/`InProgress` should never actually reach a "completed" handler in the first +/// place; both are treated defensively as a failure (never a panic) rather than risking +/// a false "Synced" toast. +#[must_use] +pub fn sync_completed_successfully(status: &SyncStatus) -> bool { + matches!(status, SyncStatus::Success) +} + +/// Decides whether the auto-sync timer should fire. +/// +/// - `interval_minutes == 0` disables auto-sync entirely. +/// - A sync already in flight never triggers another one. +/// - No prior attempt (`last_sync_attempt_at == None`) never triggers the timer: the +/// startup sync is kicked off explicitly by `trigger_initial_sync`, not by this timer. +/// +/// `last_sync_attempt_at` must be updated after *every* terminal sync outcome — success +/// **and** failure — not just success. Otherwise a sync that fails fast (e.g. a backend +/// resolution error, which fails well within one ~100ms tick) leaves the timestamp +/// stale, `duration_since` keeps reporting the interval has elapsed, and this fires +/// again on the very next tick: a retry storm with no backoff. Recording the attempt +/// unconditionally makes a failure wait a full interval before retrying, which is the +/// correct trade. +#[must_use] +pub fn should_auto_sync( + last_sync_attempt_at: Option, + now: Instant, + interval_minutes: u64, + sync_in_flight: bool, +) -> bool { + if interval_minutes == 0 || sync_in_flight { + return false; + } + match last_sync_attempt_at { + None => false, + Some(last) => now.duration_since(last) >= Duration::from_secs(interval_minutes * 60), + } +} diff --git a/src/ui/components/task_list_component.rs b/src/ui/components/task_list_component.rs index ba98f45..217255e 100644 --- a/src/ui/components/task_list_component.rs +++ b/src/ui/components/task_list_component.rs @@ -13,7 +13,7 @@ use crate::ui::components::scrollbar_helper::ScrollbarHelper; use crate::ui::components::task_list_item_component::{ListItem, TaskItem, TaskListItemType}; use crate::ui::core::SidebarSelection; use crate::ui::core::{ - actions::{Action, DialogType}, + actions::{Action, DialogType, SelectionPolicy}, Component, }; use crate::utils::datetime; @@ -93,7 +93,18 @@ impl TaskListComponent { projects: Vec, labels: Vec, sidebar_selection: SidebarSelection, + selection_policy: SelectionPolicy, ) { + // Anchor selection to the task itself only for FollowTask: a background sync can + // reload the list while the user is navigating, and an index would silently move the + // cursor to a different task. A user-initiated reload (KeepIndex) instead leaves + // `selected_index` exactly where it was, even if a different task now occupies that + // row -- e.g. an overdue task marked "due today" moving out of the row it was on. + let anchor = match selection_policy { + SelectionPolicy::FollowTask => self.get_selected_task().map(|task| task.uuid), + SelectionPolicy::KeepIndex => None, + }; + self.tasks = tasks; self.sections = sections; self.projects = projects; @@ -102,9 +113,35 @@ impl TaskListComponent { // Build the flat list of items from the hierarchical task data self.build_item_list(); + if let Some(uuid) = anchor { + self.restore_selection(uuid); + } self.update_list_state(); } + /// Move selection back to `uuid` if that task is still in the list. + /// + /// If it is gone (completed or deleted elsewhere), `selected_index` is left unchanged. + /// `update_list_state`'s clamp then only helps if that stale index is now out of range, + /// forcing it down to the last selectable row; if the stale index still happens to be + /// in range, selection silently lands on whatever task now occupies that slot. + fn restore_selection(&mut self, uuid: Uuid) { + let index = self.selectable_task_uuids().position(|candidate| candidate == uuid); + if let Some(index) = index { + self.selected_index = index; + } + } + + /// UUIDs of selectable task rows, in logical-index order (i.e. the same order + /// `logical_to_physical_index`/`get_selected_task` walk). Section headers and + /// separators are not selectable and are skipped, matching `TaskListItemType::is_selectable`. + fn selectable_task_uuids(&self) -> impl Iterator + '_ { + self.items.iter().filter_map(|item| match item { + TaskListItemType::Task(task_item) => Some(task_item.task.uuid), + _ => None, + }) + } + /// Build the flat list of items from task data fn build_item_list(&mut self) { self.items.clear(); diff --git a/src/ui/core/actions.rs b/src/ui/core/actions.rs index d7dc924..aa40841 100644 --- a/src/ui/core/actions.rs +++ b/src/ui/core/actions.rs @@ -1,6 +1,30 @@ use crate::sync::SyncStatus; use uuid::Uuid; +/// Whether a task-list reload keeps the cursor on the same row or moves it to follow the +/// task that was selected before the reload. +/// +/// This travels with each reload (as a field on [`Action::DataLoaded`]) rather than living as +/// mutable state on the app component, because the reload is handled by shared code +/// (`sync_component_data`/`TaskListComponent::update_data`) that serves every reload origin +/// and cannot infer the caller's intent on its own. A mutable "pending policy" flag would risk +/// a later reload from a different origin consuming a stale value. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SelectionPolicy { + /// Leave `selected_index` exactly where it was. Use this for user-initiated reloads: after + /// a task operation (`Action::RefreshData`), a sidebar navigation, or a debug-mode local + /// refresh. The user is looking at a specific row and expects the cursor to stay there even + /// if a different task now occupies it — e.g. pressing `t` to mark an overdue task due + /// "today" must not drag the cursor along with it as it moves out of the Overdue section. + KeepIndex, + /// Re-anchor selection to the previously selected task's UUID, so the cursor follows that + /// task even though its position in the list may have changed. Use this only for reloads + /// triggered by data arriving out-of-band, i.e. a completed sync (manual `r` or the + /// auto-sync timer): the user did not initiate the reload and may be navigating while it + /// lands, so an index-based selection could silently jump to an unrelated task. + FollowTask, +} + /// Represents the currently selected item in the sidebar #[derive(Debug, Clone, PartialEq, Default)] pub enum SidebarSelection { @@ -74,6 +98,7 @@ pub enum Action { labels: Vec, sections: Vec, tasks: Vec, + selection_policy: SelectionPolicy, }, SearchTasks(String), // Query for task search SearchResultsLoaded { diff --git a/src/ui/core/mod.rs b/src/ui/core/mod.rs index d4a0f6d..e2acc3a 100644 --- a/src/ui/core/mod.rs +++ b/src/ui/core/mod.rs @@ -33,7 +33,7 @@ pub mod event_handler; pub mod task_manager; // Re-export core types for easier access from other modules -pub use actions::{Action, DialogType, SidebarSelection}; +pub use actions::{Action, DialogType, SelectionPolicy, SidebarSelection}; pub use component::Component; pub use context::AppContext; pub use event_handler::{EventHandler, EventType}; diff --git a/src/ui/core/task_manager.rs b/src/ui/core/task_manager.rs index 259e621..20feffc 100644 --- a/src/ui/core/task_manager.rs +++ b/src/ui/core/task_manager.rs @@ -1,4 +1,4 @@ -use super::actions::{Action, SidebarSelection}; +use super::actions::{Action, SelectionPolicy, SidebarSelection}; use crate::constants::UI_LOADING_DATA_FROM_STORAGE; use crate::sync::{SyncService, SyncStatus}; use std::collections::HashMap; @@ -180,12 +180,18 @@ impl TaskManager { self.tasks.len() } - /// Spawn a background data loading operation + /// Spawn a background data loading operation. + /// + /// `selection_policy` doubles as the initial-load-vs-reload switch, since the two states + /// are mutually exclusive: `None` means this is the startup load (no prior selection + /// exists yet to keep or follow), and the result is sent as `Action::InitialDataLoaded`. + /// `Some(policy)` means this is a reload of an already-showing view, and the result is + /// sent as `Action::DataLoaded` carrying that policy. pub fn spawn_data_load( &mut self, sync_service: SyncService, sidebar_selection: SidebarSelection, - is_initial_load: bool, + selection_policy: Option, ) -> TaskId { let task_id = self.next_task_id; self.next_task_id += 1; @@ -228,20 +234,20 @@ impl TaskManager { tasks: tasks.clone(), }; - let action = if is_initial_load { - Action::InitialDataLoaded { + let action = match selection_policy { + None => Action::InitialDataLoaded { projects, labels, sections, tasks, - } - } else { - Action::DataLoaded { + }, + Some(selection_policy) => Action::DataLoaded { projects, labels, sections, tasks, - } + selection_policy, + }, }; let _ = action_sender.send(action); diff --git a/src/ui/mod.rs b/src/ui/mod.rs index d70ba5c..3690c5f 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -36,7 +36,7 @@ //! //! # async fn example() -> anyhow::Result<()> { //! let (config, theme_warnings) = Config::load()?; -//! let storage = Arc::new(Mutex::new(LocalStorage::new(false).await?)); +//! let storage = Arc::new(Mutex::new(LocalStorage::new_at("/tmp/terminalist-example").await?)); //! let backend_registry = Arc::new(BackendRegistry::new(storage)); //! // ... initialize and load backends ... //! # let backend_uuid = uuid::Uuid::new_v4(); diff --git a/src/ui/renderer.rs b/src/ui/renderer.rs index 09e4f7b..9b7a861 100644 --- a/src/ui/renderer.rs +++ b/src/ui/renderer.rs @@ -98,6 +98,12 @@ where needs_render = true; } EventType::Tick => { + // Sampled *before* the tick is processed: `process_background_actions` advances + // the toast's own timer, so a success toast that expires on this tick would + // already report "nothing left to change" by the time we asked afterwards, and + // the frame that erases it would never be drawn. + let toast_may_change = app.sync_toast_expires_on_tick(); + // Process background actions on tick (less frequent) let background_actions = app.process_background_actions(); @@ -115,7 +121,13 @@ where } } } - // Don't render on every tick - only when there are actual background actions + // Don't render on every tick unconditionally - only when there were actual + // background actions, or a success toast is counting down and needs the frames + // to expire on screen. A "Syncing" toast is static and a "Failed" toast never + // expires on its own, so neither is allowed to pin the loop at tick rate. + if toast_may_change { + needs_render = true; + } } EventType::Render => { needs_render = true; diff --git a/tests/config.rs b/tests/config.rs index 8817248..7fbce44 100644 --- a/tests/config.rs +++ b/tests/config.rs @@ -179,3 +179,90 @@ fn test_generate_default_config_includes_theme_section() { assert!(toml_str.contains("accent = \"Yellow\"")); assert!(toml_str.contains("danger = \"Red\"")); } + +#[test] +fn test_default_config_has_no_storage_override() { + let config = Config::default(); + assert!(config.storage.data_dir.is_none()); +} + +#[test] +fn test_load_from_file_reads_storage_data_dir() { + use std::fs; + use std::path::PathBuf; + + let path = std::env::temp_dir().join("terminalist_test_storage_dir.toml"); + fs::write(&path, "[storage]\ndata_dir = \"/tmp/terminalist-test\"\n").unwrap(); + + let (config, warnings) = Config::load_from_file(&path).unwrap(); + + assert_eq!(config.storage.data_dir, Some(PathBuf::from("/tmp/terminalist-test"))); + assert!(warnings.is_empty()); + + let _ = fs::remove_file(&path); +} + +#[test] +fn test_config_without_storage_section_still_loads() { + use std::fs; + + // Backward compatibility: every config file written before this feature existed. + let path = std::env::temp_dir().join("terminalist_test_no_storage.toml"); + fs::write(&path, "[ui]\nsidebar_width = 35\n").unwrap(); + + let (config, _) = Config::load_from_file(&path).unwrap(); + + assert_eq!(config.ui.sidebar_width, 35); + assert!(config.storage.data_dir.is_none()); + + let _ = fs::remove_file(&path); +} + +#[test] +fn test_empty_storage_data_dir_fails_validation() { + use std::path::PathBuf; + + let mut config = Config::default(); + config.storage.data_dir = Some(PathBuf::from(" ")); + assert!(config.validate().is_err()); + + config.storage.data_dir = Some(PathBuf::from("")); + assert!(config.validate().is_err()); + + config.storage.data_dir = Some(PathBuf::from("/tmp/terminalist-test")); + assert!(config.validate().is_ok()); +} + +#[test] +fn test_generated_default_config_still_serializes() { + // Guards the toml `None` trap: if an Option field errors instead of being skipped, + // --generate-config breaks entirely. + let toml_str = toml::to_string_pretty(&Config::default()).unwrap(); + assert!(toml_str.contains("[ui]")); + assert!(!toml_str.contains("data_dir")); +} + +#[test] +fn test_generate_default_config_documents_storage_section() { + use std::fs; + + let path = std::env::temp_dir().join("terminalist_test_generated_storage.toml"); + let _ = fs::remove_file(&path); + + Config::generate_default_config(&path).unwrap(); + let written = fs::read_to_string(&path).unwrap(); + + // The section header is live (not commented out), so uncommenting only `data_dir` + // lands the key inside `[storage]` rather than silently inside whatever section + // precedes it in the generated file. + assert!(written.contains("\n[storage]\n")); + assert!(!written.contains("# [storage]")); + assert!(written.contains("# data_dir =")); + + // An empty `[storage]` table (the value still commented out) must deserialize to the + // default, i.e. no override. + let (config, _) = Config::load_from_file(&path).unwrap(); + assert!(config.storage.data_dir.is_none()); + + let _ = fs::remove_file(&path); +} diff --git a/tests/storage.rs b/tests/storage.rs index 1ddd1b1..b0be934 100644 --- a/tests/storage.rs +++ b/tests/storage.rs @@ -1,2 +1,5 @@ #[path = "storage/db.rs"] mod db; + +#[path = "storage/migration.rs"] +mod migration; diff --git a/tests/storage/db.rs b/tests/storage/db.rs index 9c97db4..0823ab2 100644 --- a/tests/storage/db.rs +++ b/tests/storage/db.rs @@ -1,8 +1,244 @@ +use std::path::{Path, PathBuf}; +use terminalist::storage::{resolve_data_dir, DB_FILE_NAME}; + +#[test] +fn test_resolve_data_dir_defaults_to_platform_directory() { + let expected = dirs::data_dir().expect("platform data dir").join("terminalist"); + assert_eq!(resolve_data_dir(None).unwrap(), expected); +} + +#[test] +fn test_resolve_data_dir_passes_through_absolute_paths() { + let absolute = if cfg!(windows) { + PathBuf::from(r"C:\tmp\terminalist-test") + } else { + PathBuf::from("/tmp/terminalist-test") + }; + + assert_eq!(resolve_data_dir(Some(&absolute)).unwrap(), absolute); +} + +#[test] +fn test_resolve_data_dir_passes_through_relative_paths() { + let relative = Path::new("terminalist-data"); + assert_eq!(resolve_data_dir(Some(relative)).unwrap(), relative); +} + +#[test] +fn test_resolve_data_dir_expands_leading_tilde() { + let Some(home) = dirs::home_dir() else { + return; // No home directory in this environment; nothing to assert. + }; + + assert_eq!(resolve_data_dir(Some(Path::new("~"))).unwrap(), home); + assert_eq!( + resolve_data_dir(Some(Path::new("~/terminalist-dev"))).unwrap(), + home.join("terminalist-dev") + ); +} + +#[test] +fn test_resolve_data_dir_only_treats_leading_tilde_as_special() { + // A tilde anywhere but the start is an ordinary path character. + let embedded = Path::new("backups/~archive"); + assert_eq!(resolve_data_dir(Some(embedded)).unwrap(), embedded); +} + +#[test] +fn test_db_file_name_is_stable() { + // The filename is an implementation detail of the data directory, but tests and + // docs both depend on this exact value. + assert_eq!(DB_FILE_NAME, "terminalist.db"); +} + +#[test] +fn test_configured_data_dir_flows_from_config_to_resolution() { + // Pins the config -> resolve_data_dir seam: the config layer, the resolver, and + // `new_at` are each tested in isolation elsewhere, but nothing else joins them. If + // main.rs stopped passing `config.storage.data_dir` through, this is what would catch it. + let tmp = tempfile::tempdir().unwrap(); + let toml = tmp.path().join("terminalist.toml"); + let configured_dir = tmp.path().join("cache"); + std::fs::write(&toml, format!("[storage]\ndata_dir = {:?}\n", configured_dir)).unwrap(); + + let (config, _) = terminalist::config::Config::load_from_file(&toml).unwrap(); + + assert_eq!( + resolve_data_dir(config.storage.data_dir.as_deref()).unwrap(), + configured_dir + ); +} + +use sea_orm::{ConnectionTrait, DbBackend, Statement}; use terminalist::storage::LocalStorage; +/// Close the pool explicitly. Windows cannot delete a file with an open handle, so a +/// reopen-after-delete test is flaky unless the previous connection is closed first. +async fn close(storage: LocalStorage) { + storage.conn.close().await.expect("close connection"); +} + +async fn create_marker(storage: &LocalStorage) { + storage + .conn + .execute(Statement::from_string( + DbBackend::Sqlite, + "CREATE TABLE marker (id INTEGER)".to_owned(), + )) + .await + .expect("create marker table"); +} + +async fn marker_exists(storage: &LocalStorage) -> bool { + storage + .conn + .query_one(Statement::from_string( + DbBackend::Sqlite, + "SELECT name FROM sqlite_master WHERE type='table' AND name='marker'".to_owned(), + )) + .await + .expect("query sqlite_master") + .is_some() +} + +#[tokio::test] +async fn test_new_at_opens_the_database_in_the_given_directory() { + let tmp = tempfile::tempdir().unwrap(); + + let storage = LocalStorage::new_at(tmp.path()).await.unwrap(); + close(storage).await; + + // The whole point of this feature: the configured directory is where the database + // lives, so nothing ever touches the platform default path. + assert!(tmp.path().join(DB_FILE_NAME).exists()); + assert_ne!(resolve_data_dir(None).unwrap(), tmp.path()); +} + #[tokio::test] -async fn test_local_storage_creation() { - // Test that we can create local storage (use in-memory database for tests) - let result = LocalStorage::new(false).await; - assert!(result.is_ok(), "LocalStorage should be created successfully"); +async fn test_new_at_creates_missing_directories() { + let tmp = tempfile::tempdir().unwrap(); + let nested = tmp.path().join("nested").join("data"); + + let storage = LocalStorage::new_at(&nested).await.unwrap(); + close(storage).await; + + assert!(nested.join(DB_FILE_NAME).exists()); +} + +#[tokio::test] +async fn test_reopening_preserves_data() { + let tmp = tempfile::tempdir().unwrap(); + + let first = LocalStorage::new_at(tmp.path()).await.unwrap(); + create_marker(&first).await; + close(first).await; + + // A normal open must no longer wipe the cache — this is the whole feature. + let second = LocalStorage::new_at(tmp.path()).await.unwrap(); + assert!(marker_exists(&second).await, "reopening must preserve the cache"); + close(second).await; +} + +/// The cache holds `backends.credentials` — the raw `TODOIST_API_TOKEN` — as plaintext JSON, +/// and `[storage] data_dir` now lets users relocate it, so the file must not be world-readable. +#[cfg(unix)] +#[tokio::test] +async fn test_database_file_is_readable_only_by_its_owner() { + use std::os::unix::fs::PermissionsExt; + + let tmp = tempfile::tempdir().unwrap(); + let storage = LocalStorage::new_at(tmp.path()).await.unwrap(); + close(storage).await; + + let mode = std::fs::metadata(tmp.path().join(DB_FILE_NAME)).unwrap().permissions().mode(); + + assert_eq!( + mode & 0o777, + 0o600, + "the database stores the API token in plaintext and must be owner-only" + ); +} + +#[tokio::test] +async fn test_schema_creation_is_idempotent() { + let tmp = tempfile::tempdir().unwrap(); + + // Second open re-runs init_schema against existing tables; without + // if_not_exists this fails with "table projects already exists". + let first = LocalStorage::new_at(tmp.path()).await.unwrap(); + close(first).await; + let second = LocalStorage::new_at(tmp.path()).await.unwrap(); + close(second).await; +} + +#[test] +fn test_backend_uuid_is_deterministic() { + use terminalist::backend_registry::derive_backend_uuid; + + // The same (type, name) must yield the same UUID on every launch, or a + // persistent cache duplicates every row under a fresh backend_uuid. + assert_eq!( + derive_backend_uuid("todoist", "My Todoist"), + derive_backend_uuid("todoist", "My Todoist") + ); + assert_ne!( + derive_backend_uuid("todoist", "My Todoist"), + derive_backend_uuid("todoist", "Work") + ); + assert_ne!( + derive_backend_uuid("todoist", "My Todoist"), + derive_backend_uuid("ticktick", "My Todoist") + ); +} + +#[tokio::test] +async fn test_relaunch_upsert_refreshes_credentials_without_re_enabling() { + use sea_orm::EntityTrait; + use std::sync::Arc; + use terminalist::backend_registry::{derive_backend_uuid, BackendRegistry}; + use terminalist::entities::backend; + use tokio::sync::Mutex; + + let tmp = tempfile::tempdir().unwrap(); + let storage = Arc::new(Mutex::new(LocalStorage::new_at(tmp.path()).await.unwrap())); + let registry = BackendRegistry::new(storage.clone()); + + let credentials_a = serde_json::json!({ "api_token": "token-a" }).to_string(); + let uuid = registry + .add_backend( + "todoist".to_string(), + "My Todoist".to_string(), + credentials_a, + "{}".to_string(), + ) + .await + .unwrap(); + + registry.disable_backend(&uuid).await.unwrap(); + + // Simulate a relaunch with the same (type, name) but a rotated token. + let credentials_b = serde_json::json!({ "api_token": "token-b" }).to_string(); + registry + .add_backend( + "todoist".to_string(), + "My Todoist".to_string(), + credentials_b.clone(), + "{}".to_string(), + ) + .await + .unwrap(); + + let locked = storage.lock().await; + let rows = backend::Entity::find().all(&locked.conn).await.unwrap(); + + assert_eq!(rows.len(), 1, "relaunch must upsert the existing row, not duplicate it"); + assert_eq!(rows[0].uuid, derive_backend_uuid("todoist", "My Todoist")); + assert_eq!( + rows[0].credentials, credentials_b, + "relaunch must refresh the rotated token" + ); + assert!( + !rows[0].is_enabled, + "relaunch must not silently re-enable a backend the user disabled" + ); } diff --git a/tests/storage/labels.rs b/tests/storage/labels.rs deleted file mode 100644 index a694b81..0000000 --- a/tests/storage/labels.rs +++ /dev/null @@ -1,8 +0,0 @@ -use terminalist::storage::LocalStorage; - -#[tokio::test] -async fn test_labels_storage_creation() { - // Test that we can create local storage for labels - let result = LocalStorage::new(false).await; - assert!(result.is_ok(), "LocalStorage should be created successfully"); -} diff --git a/tests/storage/migration.rs b/tests/storage/migration.rs new file mode 100644 index 0000000..b25119a --- /dev/null +++ b/tests/storage/migration.rs @@ -0,0 +1,245 @@ +//! Upgrade tests: opening a database that a *pre*-persistent-cache release left behind. +//! +//! Before this branch, every launch deleted `terminalist.db` and rebuilt it, so the file on +//! disk when the process exited always had the old shape: a `backends` row whose `uuid` is a +//! random v4 (there was no derived UUID and no `idx_backends_type_name`), with every cached +//! project/task keyed to that random UUID. `--debug` kept the file *and* inserted a fresh v4 +//! row on every launch, so those installations accumulated duplicate `("todoist", "My Todoist")` +//! rows. +//! +//! These tests reproduce both shapes with raw SQL — deliberately *not* through +//! `LocalStorage::new_at`, which would produce the new shape — and then start the app against +//! them the way `main.rs` does. Every test works inside a `tempfile` directory; none of them +//! ever touches a real user path. + +use sea_orm::{ConnectionTrait, Database, DatabaseConnection, DbBackend, Statement}; +use std::path::Path; +use std::sync::Arc; +use tokio::sync::Mutex; +use uuid::Uuid; + +use terminalist::backend_registry::{derive_backend_uuid, BackendRegistry}; +use terminalist::repositories::{BackendRepository, ProjectRepository, TaskRepository}; +use terminalist::storage::{LocalStorage, DB_FILE_NAME}; + +/// SQLite stores `Uuid` columns as 16-byte blobs, so raw-SQL fixtures must bind blob literals. +fn blob(uuid: &Uuid) -> String { + format!("X'{}'", uuid.simple()) +} + +async fn exec(conn: &DatabaseConnection, sql: &str) { + conn.execute(Statement::from_string(DbBackend::Sqlite, sql.to_owned())) + .await + .unwrap_or_else(|e| panic!("fixture statement failed: {e}\n{sql}")); +} + +/// Create the exact table set the pre-branch release created — same DDL, but **without** +/// `idx_backends_type_name`, which did not exist yet. +async fn create_legacy_schema(conn: &DatabaseConnection) { + for sql in [ + r#"CREATE TABLE "backends" ( "uuid" uuid_text NOT NULL PRIMARY KEY, "backend_type" varchar NOT NULL, "name" varchar NOT NULL, "is_enabled" boolean NOT NULL, "credentials" varchar NOT NULL, "settings" varchar NOT NULL )"#, + r#"CREATE TABLE "projects" ( "uuid" uuid_text NOT NULL PRIMARY KEY, "backend_uuid" uuid_text NOT NULL, "remote_id" varchar NOT NULL, "name" varchar NOT NULL, "is_favorite" boolean NOT NULL, "is_inbox_project" boolean NOT NULL, "order_index" integer NOT NULL, "parent_uuid" uuid_text, FOREIGN KEY ("parent_uuid") REFERENCES "projects" ("uuid"), FOREIGN KEY ("backend_uuid") REFERENCES "backends" ("uuid") ON DELETE CASCADE )"#, + r#"CREATE TABLE "sections" ( "uuid" uuid_text NOT NULL PRIMARY KEY, "backend_uuid" uuid_text NOT NULL, "remote_id" varchar NOT NULL, "name" varchar NOT NULL, "project_uuid" uuid_text NOT NULL, "order_index" integer NOT NULL, FOREIGN KEY ("project_uuid") REFERENCES "projects" ("uuid") ON DELETE CASCADE, FOREIGN KEY ("backend_uuid") REFERENCES "backends" ("uuid") ON DELETE CASCADE )"#, + r#"CREATE TABLE "labels" ( "uuid" uuid_text NOT NULL PRIMARY KEY, "backend_uuid" uuid_text NOT NULL, "remote_id" varchar NOT NULL, "name" varchar NOT NULL, "order_index" integer NOT NULL, "is_favorite" boolean NOT NULL, FOREIGN KEY ("backend_uuid") REFERENCES "backends" ("uuid") ON DELETE CASCADE )"#, + r#"CREATE TABLE "tasks" ( "uuid" uuid_text NOT NULL PRIMARY KEY, "backend_uuid" uuid_text NOT NULL, "remote_id" varchar NOT NULL, "content" varchar NOT NULL, "description" varchar, "project_uuid" uuid_text NOT NULL, "section_uuid" uuid_text, "parent_uuid" uuid_text, "priority" integer NOT NULL, "order_index" integer NOT NULL, "due_date" varchar, "due_datetime" varchar, "is_recurring" boolean NOT NULL, "deadline" varchar, "duration" varchar, "is_completed" boolean NOT NULL, "is_deleted" boolean NOT NULL, FOREIGN KEY ("project_uuid") REFERENCES "projects" ("uuid") ON DELETE CASCADE, FOREIGN KEY ("section_uuid") REFERENCES "sections" ("uuid") ON DELETE SET NULL, FOREIGN KEY ("parent_uuid") REFERENCES "tasks" ("uuid") ON DELETE CASCADE, FOREIGN KEY ("backend_uuid") REFERENCES "backends" ("uuid") ON DELETE CASCADE )"#, + r#"CREATE TABLE "task_labels" ( "task_uuid" uuid_text NOT NULL, "label_uuid" uuid_text NOT NULL, CONSTRAINT "pk-task_labels" PRIMARY KEY ("task_uuid", "label_uuid"), FOREIGN KEY ("task_uuid") REFERENCES "tasks" ("uuid") ON DELETE CASCADE, FOREIGN KEY ("label_uuid") REFERENCES "labels" ("uuid") ON DELETE CASCADE )"#, + "CREATE UNIQUE INDEX idx_projects_backend_remote ON projects(backend_uuid, remote_id)", + "CREATE UNIQUE INDEX idx_sections_backend_remote ON sections(backend_uuid, remote_id)", + "CREATE UNIQUE INDEX idx_labels_backend_remote ON labels(backend_uuid, remote_id)", + "CREATE UNIQUE INDEX idx_tasks_backend_remote ON tasks(backend_uuid, remote_id)", + ] { + exec(conn, sql).await; + } +} + +async fn insert_legacy_backend(conn: &DatabaseConnection, uuid: &Uuid) { + exec( + conn, + &format!( + r#"INSERT INTO backends (uuid, backend_type, name, is_enabled, credentials, settings) + VALUES ({}, 'todoist', 'My Todoist', 1, '{{"api_token":"legacy-token"}}', '{{}}')"#, + blob(uuid) + ), + ) + .await; +} + +/// Seed one project plus one task under `backend_uuid`, named after `suffix` so tests can tell +/// which backend row's cache survived. +async fn insert_legacy_project_and_task(conn: &DatabaseConnection, backend_uuid: &Uuid, suffix: &str) -> (Uuid, Uuid) { + let project_uuid = Uuid::new_v4(); + let task_uuid = Uuid::new_v4(); + + exec( + conn, + &format!( + r#"INSERT INTO projects (uuid, backend_uuid, remote_id, name, is_favorite, is_inbox_project, order_index, parent_uuid) + VALUES ({}, {}, 'p-{suffix}', 'Project {suffix}', 0, 0, 0, NULL)"#, + blob(&project_uuid), + blob(backend_uuid) + ), + ) + .await; + + exec( + conn, + &format!( + r#"INSERT INTO tasks (uuid, backend_uuid, remote_id, content, description, project_uuid, section_uuid, + parent_uuid, priority, order_index, due_date, due_datetime, is_recurring, deadline, + duration, is_completed, is_deleted) + VALUES ({}, {}, 't-{suffix}', 'Task {suffix}', NULL, {}, NULL, NULL, 1, 0, NULL, NULL, 0, NULL, NULL, 0, 0)"#, + blob(&task_uuid), + blob(backend_uuid), + blob(&project_uuid) + ), + ) + .await; + + (project_uuid, task_uuid) +} + +async fn legacy_connection(dir: &Path) -> DatabaseConnection { + let db_path = dir.join(DB_FILE_NAME); + let conn = Database::connect(format!("sqlite:{}?mode=rwc", db_path.display())) + .await + .expect("open fixture database"); + exec(&conn, "PRAGMA foreign_keys = ON;").await; + conn +} + +/// Do exactly what `main.rs` does on startup, against `dir`. +async fn start_app(dir: &Path) -> anyhow::Result<(Arc>, Uuid)> { + let storage = Arc::new(Mutex::new(LocalStorage::new_at(dir).await?)); + let registry = BackendRegistry::new(storage.clone()); + registry.load_backends().await?; + let uuid = registry + .add_backend( + "todoist".to_string(), + "My Todoist".to_string(), + r#"{"api_token":"fresh-token"}"#.to_string(), + "{}".to_string(), + ) + .await?; + Ok((storage, uuid)) +} + +#[tokio::test] +async fn upgrading_a_pre_persistent_cache_database_adopts_the_legacy_backend_row() { + let tmp = tempfile::tempdir().expect("tempdir"); + let legacy_uuid = Uuid::new_v4(); + + let fixture = legacy_connection(tmp.path()).await; + create_legacy_schema(&fixture).await; + insert_legacy_backend(&fixture, &legacy_uuid).await; + insert_legacy_project_and_task(&fixture, &legacy_uuid, "one").await; + insert_legacy_project_and_task(&fixture, &legacy_uuid, "two").await; + fixture.close().await.expect("close fixture connection"); + + // Startup must not fail: the derived v5 UUID does not collide with the legacy v4 one, so an + // insert keyed to the derived UUID would violate `idx_backends_type_name` and abort `main`. + let (storage, backend_uuid) = start_app(tmp.path()).await.expect("startup must succeed on upgrade"); + + assert_eq!( + backend_uuid, + legacy_uuid, + "the existing row's identity must be adopted, not replaced by the derived UUID {}", + derive_backend_uuid("todoist", "My Todoist") + ); + + let guard = storage.lock().await; + let backends = BackendRepository::get_all(&guard.conn).await.expect("query backends"); + assert_eq!(backends.len(), 1, "expected exactly one backend row, got {backends:?}"); + assert_eq!( + backends[0].uuid, legacy_uuid, + "the legacy row must be the surviving one" + ); + + // The cache hangs off `backend_uuid`; adopting keeps it attached instead of orphaning it + // under a dead UUID (which would render as a fully duplicated task list). + let mut projects = ProjectRepository::get_all(&guard.conn) + .await + .expect("query projects") + .into_iter() + .map(|p| p.remote_id) + .collect::>(); + projects.sort(); + assert_eq!(projects, vec!["p-one".to_string(), "p-two".to_string()]); + + let mut tasks = TaskRepository::get_all(&guard.conn) + .await + .expect("query tasks") + .into_iter() + .map(|t| t.remote_id) + .collect::>(); + tasks.sort(); + assert_eq!(tasks, vec!["t-one".to_string(), "t-two".to_string()]); +} + +#[tokio::test] +async fn upgrading_a_debug_mode_database_with_duplicate_backend_rows_starts_and_keeps_the_oldest() { + let tmp = tempfile::tempdir().expect("tempdir"); + // `--debug` kept the file and inserted a fresh v4 row per launch, so these accumulated. + let oldest = Uuid::new_v4(); + let middle = Uuid::new_v4(); + let newest = Uuid::new_v4(); + + let fixture = legacy_connection(tmp.path()).await; + create_legacy_schema(&fixture).await; + for uuid in [&oldest, &middle, &newest] { + insert_legacy_backend(&fixture, uuid).await; + } + insert_legacy_project_and_task(&fixture, &oldest, "oldest").await; + insert_legacy_project_and_task(&fixture, &middle, "middle").await; + insert_legacy_project_and_task(&fixture, &newest, "newest").await; + fixture.close().await.expect("close fixture connection"); + + // `CREATE UNIQUE INDEX idx_backends_type_name` fails outright against duplicate rows, which + // would kill the app inside `LocalStorage::new_at` before anything else could run. + let (storage, backend_uuid) = start_app(tmp.path()) + .await + .expect("startup must survive duplicate legacy backend rows"); + + assert_eq!(backend_uuid, oldest, "the oldest row is the one the cache is keyed to"); + + let guard = storage.lock().await; + let backends = BackendRepository::get_all(&guard.conn).await.expect("query backends"); + assert_eq!( + backends.len(), + 1, + "duplicates must be collapsed to a single row, got {backends:?}" + ); + assert_eq!(backends[0].uuid, oldest); + + // The younger rows' caches cascade away with them, so nothing is left duplicated. + let projects = ProjectRepository::get_all(&guard.conn) + .await + .expect("query projects") + .into_iter() + .map(|p| p.remote_id) + .collect::>(); + assert_eq!(projects, vec!["p-oldest".to_string()]); + + let tasks = TaskRepository::get_all(&guard.conn) + .await + .expect("query tasks") + .into_iter() + .map(|t| t.remote_id) + .collect::>(); + assert_eq!(tasks, vec!["t-oldest".to_string()]); +} + +#[tokio::test] +async fn a_fresh_database_still_gets_the_derived_backend_uuid() { + // The adoption path must not weaken the derived-UUID guarantee for new installations. + let tmp = tempfile::tempdir().expect("tempdir"); + + let (storage, first) = start_app(tmp.path()).await.expect("first launch"); + assert_eq!(first, derive_backend_uuid("todoist", "My Todoist")); + drop(storage); + + let (storage, second) = start_app(tmp.path()).await.expect("second launch"); + assert_eq!(second, first, "relaunch must reuse the same identity"); + let guard = storage.lock().await; + let backends = BackendRepository::get_all(&guard.conn).await.expect("query backends"); + assert_eq!(backends.len(), 1); +} diff --git a/tests/storage/projects.rs b/tests/storage/projects.rs deleted file mode 100644 index ef94a1a..0000000 --- a/tests/storage/projects.rs +++ /dev/null @@ -1,8 +0,0 @@ -use terminalist::storage::LocalStorage; - -#[tokio::test] -async fn test_projects_storage_creation() { - // Test that we can create local storage for projects - let result = LocalStorage::new(false).await; - assert!(result.is_ok(), "LocalStorage should be created successfully"); -} diff --git a/tests/storage/sections.rs b/tests/storage/sections.rs deleted file mode 100644 index f9f7697..0000000 --- a/tests/storage/sections.rs +++ /dev/null @@ -1,8 +0,0 @@ -use terminalist::storage::LocalStorage; - -#[tokio::test] -async fn test_sections_storage_creation() { - // Test that we can create local storage for sections - let result = LocalStorage::new(false).await; - assert!(result.is_ok(), "LocalStorage should be created successfully"); -} diff --git a/tests/storage/tasks.rs b/tests/storage/tasks.rs deleted file mode 100644 index 25a8d35..0000000 --- a/tests/storage/tasks.rs +++ /dev/null @@ -1,8 +0,0 @@ -use terminalist::storage::LocalStorage; - -#[tokio::test] -async fn test_tasks_storage_creation() { - // Test that we can create local storage for tasks - let result = LocalStorage::new(false).await; - assert!(result.is_ok(), "LocalStorage should be created successfully"); -} diff --git a/tests/sync.rs b/tests/sync.rs new file mode 100644 index 0000000..532c74a --- /dev/null +++ b/tests/sync.rs @@ -0,0 +1,2 @@ +#[path = "sync/reconcile.rs"] +mod reconcile; diff --git a/tests/sync/reconcile.rs b/tests/sync/reconcile.rs new file mode 100644 index 0000000..5760dd2 --- /dev/null +++ b/tests/sync/reconcile.rs @@ -0,0 +1,687 @@ +//! Reconciliation tests for the `store_*_batch` functions in `src/sync/storage.rs`. +//! +//! These drive `SyncService::store_projects_batch` / `store_labels_batch` / +//! `store_sections_batch` / `store_tasks_batch` directly with fixture data (bypassing the +//! network-fetching `SyncService::sync()`), then assert against the repositories. Every test +//! opens storage via `LocalStorage::new_at` in a fresh `tempfile` directory — never a user path. + +use sea_orm::{EntityTrait, PaginatorTrait}; +use std::path::Path; +use std::sync::Arc; +use tokio::sync::Mutex; +use uuid::Uuid; + +use terminalist::backend::{BackendLabel, BackendProject, BackendSection, BackendTask}; +use terminalist::backend_registry::BackendRegistry; +use terminalist::entities::task_label; +use terminalist::repositories::{LabelRepository, ProjectRepository, SectionRepository, TaskRepository}; +use terminalist::storage::LocalStorage; +use terminalist::sync::SyncService; + +/// Open a fresh local cache, register a fake "todoist" backend for it (no network calls: the +/// underlying `TodoistWrapper::new` only builds an HTTP client, it never dials out), and return +/// a ready-to-use `SyncService` alongside the storage handle and the backend's UUID. +async fn new_sync_service(dir: &Path) -> (SyncService, Arc>, Uuid) { + let storage = LocalStorage::new_at(dir).await.expect("open local storage"); + let storage = Arc::new(Mutex::new(storage)); + + let registry = Arc::new(BackendRegistry::new(storage.clone())); + let backend_uuid = registry + .add_backend( + "todoist".to_string(), + "reconcile-test-backend".to_string(), + r#"{"api_token":"fake-token"}"#.to_string(), + "{}".to_string(), + ) + .await + .expect("register fake backend"); + + let sync_service = SyncService::new(registry, backend_uuid, false) + .await + .expect("construct sync service"); + + (sync_service, storage, backend_uuid) +} + +fn project(remote_id: &str, name: &str) -> BackendProject { + BackendProject { + remote_id: remote_id.to_string(), + name: name.to_string(), + is_favorite: false, + is_inbox: false, + order_index: 0, + parent_remote_id: None, + } +} + +fn project_with_parent(remote_id: &str, name: &str, parent_remote_id: &str) -> BackendProject { + BackendProject { + parent_remote_id: Some(parent_remote_id.to_string()), + ..project(remote_id, name) + } +} + +fn label(remote_id: &str, name: &str) -> BackendLabel { + BackendLabel { + remote_id: remote_id.to_string(), + name: name.to_string(), + order_index: 0, + is_favorite: false, + } +} + +fn section(remote_id: &str, project_remote_id: &str, name: &str) -> BackendSection { + BackendSection { + remote_id: remote_id.to_string(), + name: name.to_string(), + project_remote_id: project_remote_id.to_string(), + order_index: 0, + } +} + +fn task(remote_id: &str, project_remote_id: &str, content: &str, labels: Vec) -> BackendTask { + BackendTask { + remote_id: remote_id.to_string(), + content: content.to_string(), + description: None, + project_remote_id: project_remote_id.to_string(), + section_remote_id: None, + parent_remote_id: None, + priority: 1, + order_index: 0, + due_date: None, + due_datetime: None, + is_recurring: false, + deadline: None, + duration: None, + is_completed: false, + labels, + } +} + +fn task_with_parent(remote_id: &str, project_remote_id: &str, content: &str, parent_remote_id: &str) -> BackendTask { + BackendTask { + parent_remote_id: Some(parent_remote_id.to_string()), + ..task(remote_id, project_remote_id, content, vec![]) + } +} + +fn remote_ids(rows: &[T], f: impl Fn(&T) -> &str) -> Vec<&str> { + rows.iter().map(f).collect() +} + +// 1. A row absent from the second fetch is deleted. +#[tokio::test] +async fn project_missing_from_second_sync_is_deleted() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox"), project("p2", "Work")]) + .await + .expect("first sync"); + drop(guard); + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("second sync"); + let remaining = ProjectRepository::get_all(&guard.conn).await.expect("query projects"); + drop(guard); + + let ids = remote_ids(&remaining, |p| p.remote_id.as_str()); + assert_eq!(ids, vec!["p1"], "project p2 should have been deleted, got {ids:?}"); +} + +// 1b. ...but an EMPTY fetch is not treated as "the remote has nothing". `is_not_in(vec![])` +// matches every row, so without a guard a transient empty-but-200 response would blank the +// user's list (and cascade their tasks away with it) until a later sync repaired it. Each +// `store_*_batch` treats an empty slice as "nothing to reconcile" and returns early; that is +// the single layer where this policy lives, so all four entity types agree. +// +// The accepted trade: an account whose last project/label/task is genuinely deleted keeps a +// stale local copy until something else comes back. Stale beats blank. +#[tokio::test] +async fn empty_fetch_leaves_existing_rows_untouched() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox"), project("p2", "Work")]) + .await + .expect("store projects"); + sync_service + .store_labels_batch(&guard, &[label("l1", "urgent")]) + .await + .expect("store labels"); + sync_service + .store_sections_batch(&guard, &[section("s1", "p1", "Backlog")]) + .await + .expect("store sections"); + sync_service + .store_tasks_batch(&guard, &[task("t1", "p1", "Buy milk", vec!["urgent".to_string()])]) + .await + .expect("store tasks"); + drop(guard); + + // Every fetch comes back empty — the shape of a transient backend hiccup that still + // answers 200. + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[]) + .await + .expect("empty project fetch"); + sync_service.store_labels_batch(&guard, &[]).await.expect("empty label fetch"); + sync_service + .store_sections_batch(&guard, &[]) + .await + .expect("empty section fetch"); + sync_service.store_tasks_batch(&guard, &[]).await.expect("empty task fetch"); + + let projects = ProjectRepository::get_all(&guard.conn).await.expect("query projects"); + let labels = LabelRepository::get_all(&guard.conn).await.expect("query labels"); + let sections = SectionRepository::get_all(&guard.conn).await.expect("query sections"); + let tasks = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + drop(guard); + + assert_eq!( + remote_ids(&projects, |p| p.remote_id.as_str()), + vec!["p1", "p2"], + "an empty fetch must not wipe the cached projects" + ); + assert_eq!( + remote_ids(&labels, |l| l.remote_id.as_str()), + vec!["l1"], + "an empty fetch must not wipe the cached labels" + ); + assert_eq!( + remote_ids(§ions, |s| s.remote_id.as_str()), + vec!["s1"], + "an empty fetch must not wipe the cached sections" + ); + assert_eq!( + remote_ids(&tasks, |t| t.remote_id.as_str()), + vec!["t1"], + "an empty fetch must not wipe the cached tasks" + ); +} + +// 2. A row present in both is updated, not duplicated, and KEEPS its local uuid. +#[tokio::test] +async fn project_present_in_both_syncs_is_updated_not_duplicated_and_keeps_uuid() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Old Name")]) + .await + .expect("first sync"); + let original = ProjectRepository::get_by_remote_id(&guard.conn, &backend_uuid, "p1") + .await + .expect("query project") + .expect("project p1 stored after first sync"); + drop(guard); + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "New Name")]) + .await + .expect("second sync"); + let all = ProjectRepository::get_all(&guard.conn).await.expect("query projects"); + drop(guard); + + assert_eq!(all.len(), 1, "expected exactly one project row, got {all:?}"); + assert_eq!(all[0].remote_id, "p1"); + assert_eq!(all[0].uuid, original.uuid, "local uuid must survive the update"); + assert_eq!(all[0].name, "New Name", "the update should still apply"); +} + +// 3. Deleting a project removes its tasks but not another project's tasks. +#[tokio::test] +async fn deleting_a_project_cascades_its_tasks_but_not_another_projects_tasks() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Project One"), project("p2", "Project Two")]) + .await + .expect("store projects"); + sync_service + .store_tasks_batch( + &guard, + &[ + task("t1", "p1", "Task in project one", vec![]), + task("t2", "p2", "Task in project two", vec![]), + ], + ) + .await + .expect("store tasks"); + drop(guard); + + // Second project sync: p1 is gone remotely, p2 survives. We never re-sync tasks here, so + // any task-list survivorship is caused purely by the project delete pass's FK cascade. + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p2", "Project Two")]) + .await + .expect("second project sync"); + let remaining_projects = ProjectRepository::get_all(&guard.conn).await.expect("query projects"); + let remaining_tasks = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + drop(guard); + + let project_ids = remote_ids(&remaining_projects, |p| p.remote_id.as_str()); + assert_eq!( + project_ids, + vec!["p2"], + "project p1 should have been deleted, got {project_ids:?}" + ); + + let task_ids = remote_ids(&remaining_tasks, |t| t.remote_id.as_str()); + assert_eq!( + task_ids, + vec!["t2"], + "task t1 should have cascade-deleted with project p1, and task t2 (under surviving \ + project p2) should be untouched, got {task_ids:?}" + ); +} + +// 4. Syncing the same fixture twice leaves exactly one copy of every row. +#[tokio::test] +async fn syncing_the_same_fixture_twice_leaves_one_copy_of_every_row() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let projects = [project("p1", "Inbox")]; + let labels = [label("l1", "urgent")]; + let sections = [section("s1", "p1", "Backlog")]; + let tasks = [task("t1", "p1", "Buy milk", vec!["urgent".to_string()])]; + + for _ in 0..2 { + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &projects) + .await + .expect("store projects"); + sync_service.store_labels_batch(&guard, &labels).await.expect("store labels"); + sync_service + .store_sections_batch(&guard, §ions) + .await + .expect("store sections"); + sync_service.store_tasks_batch(&guard, &tasks).await.expect("store tasks"); + } + + let guard = storage.lock().await; + let all_projects = ProjectRepository::get_all(&guard.conn).await.expect("query projects"); + let all_labels = LabelRepository::get_all(&guard.conn).await.expect("query labels"); + let all_sections = SectionRepository::get_all(&guard.conn).await.expect("query sections"); + let all_tasks = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + let task_label_count = task_label::Entity::find().count(&guard.conn).await.expect("count task_labels"); + drop(guard); + + assert_eq!(remote_ids(&all_projects, |p| p.remote_id.as_str()), vec!["p1"]); + assert_eq!(remote_ids(&all_labels, |l| l.remote_id.as_str()), vec!["l1"]); + assert_eq!(remote_ids(&all_sections, |s| s.remote_id.as_str()), vec!["s1"]); + assert_eq!(remote_ids(&all_tasks, |t| t.remote_id.as_str()), vec!["t1"]); + assert_eq!(task_label_count, 1, "exactly one task-label relationship should remain"); +} + +// Extra: the label delete pass mirrors the project one. +#[tokio::test] +async fn label_missing_from_second_sync_is_deleted() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_labels_batch(&guard, &[label("l1", "urgent"), label("l2", "later")]) + .await + .expect("first sync"); + drop(guard); + + let guard = storage.lock().await; + sync_service + .store_labels_batch(&guard, &[label("l1", "urgent")]) + .await + .expect("second sync"); + let remaining = LabelRepository::get_all(&guard.conn).await.expect("query labels"); + drop(guard); + + let ids = remote_ids(&remaining, |l| l.remote_id.as_str()); + assert_eq!(ids, vec!["l1"], "label l2 should have been deleted, got {ids:?}"); +} + +// Extra: the section delete pass mirrors the project one. (A *non-empty* fetch that omits a +// section deletes it; an empty one is guarded — see `empty_fetch_leaves_existing_rows_untouched`.) +#[tokio::test] +async fn section_missing_from_second_sync_is_deleted() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("store project"); + sync_service + .store_sections_batch(&guard, &[section("s1", "p1", "Backlog"), section("s2", "p1", "Doing")]) + .await + .expect("first sync"); + drop(guard); + + let guard = storage.lock().await; + sync_service + .store_sections_batch(&guard, &[section("s1", "p1", "Backlog")]) + .await + .expect("second sync"); + let remaining = SectionRepository::get_all(&guard.conn).await.expect("query sections"); + drop(guard); + + let ids = remote_ids(&remaining, |s| s.remote_id.as_str()); + assert_eq!(ids, vec!["s1"], "section s2 should have been deleted, got {ids:?}"); +} + +// Extra: deleting a task directly (not via a project cascade) also removes its task_labels rows. +#[tokio::test] +async fn task_missing_from_second_sync_is_deleted_and_its_label_link_goes_with_it() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("store project"); + sync_service + .store_labels_batch(&guard, &[label("l1", "urgent")]) + .await + .expect("store label"); + sync_service + .store_tasks_batch( + &guard, + &[ + task("t1", "p1", "Keep me", vec![]), + task("t2", "p1", "Delete me", vec!["urgent".to_string()]), + ], + ) + .await + .expect("first sync"); + drop(guard); + + let guard = storage.lock().await; + sync_service + .store_tasks_batch(&guard, &[task("t1", "p1", "Keep me", vec![])]) + .await + .expect("second sync"); + let remaining_tasks = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + let task_label_count = task_label::Entity::find().count(&guard.conn).await.expect("count task_labels"); + drop(guard); + + let ids = remote_ids(&remaining_tasks, |t| t.remote_id.as_str()); + assert_eq!(ids, vec!["t1"], "task t2 should have been deleted, got {ids:?}"); + assert_eq!( + task_label_count, 0, + "deleting task t2 should cascade-remove its task_labels row" + ); +} + +// Regression (fix round 1): the delete-missing pass must run BEFORE the parent-relinking pass, +// not after. Todoist's fetch never returns completed tasks (see +// `TodoistBackend::task_to_backend`'s `is_completed: false` comment), so completing a parent +// task makes it vanish from the very next fetch while its still-open subtasks remain. Pass 1 +// nulls every surviving row's `parent_uuid`; if the delete pass ran after pass 2 instead, pass 2 +// would re-link the still-fetched subtask to the stale (not yet reconciled) parent row, and +// `ON DELETE CASCADE` on the task entity's self-referential parent relation would then destroy +// the subtask along with its parent -- even though the subtask WAS in the fetch. It would +// reappear on the next sync as a fresh INSERT with a NEW uuid, violating the uuid-stability +// invariant the UI anchors selection to. +#[tokio::test] +async fn completing_a_parent_task_does_not_cascade_delete_its_still_fetched_subtask() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("store project"); + sync_service + .store_tasks_batch( + &guard, + &[ + task("parent", "p1", "Parent task", vec![]), + task_with_parent("child", "p1", "Child task", "parent"), + ], + ) + .await + .expect("first sync"); + let child_before = TaskRepository::get_by_remote_id(&guard.conn, &backend_uuid, "child") + .await + .expect("query child task") + .expect("child task stored after first sync"); + drop(guard); + + // Second sync: the parent was completed remotely, so it drops out of the fetch. The child + // is still open and still names "parent" as its parent, exactly as Todoist would report it. + let guard = storage.lock().await; + sync_service + .store_tasks_batch(&guard, &[task_with_parent("child", "p1", "Child task", "parent")]) + .await + .expect("second sync"); + let remaining_tasks = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + drop(guard); + + let ids = remote_ids(&remaining_tasks, |t| t.remote_id.as_str()); + assert_eq!( + ids, + vec!["child"], + "parent task should be gone (it was completed / dropped from the fetch) but the still-fetched \ + child must survive, got {ids:?}" + ); + assert_eq!( + remaining_tasks[0].uuid, child_before.uuid, + "the surviving child task must keep its original local uuid, not be recreated" + ); +} + +// Sibling of the above for projects: a parent project dropping out of the fetch (e.g. archived +// remotely) must not take a still-fetched child project down with it. The project entity's +// self-referential parent relation has no `on_delete` clause (defaults to `NO ACTION`), so +// getting the ordering wrong here fails the whole transaction with an FK violation rather than +// silently cascading -- fails safe, but still wrong, and this pins the correct ordering directly. +#[tokio::test] +async fn removing_a_parent_project_does_not_cascade_delete_its_still_fetched_child_project() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch( + &guard, + &[ + project("parent", "Parent Project"), + project_with_parent("child", "Child Project", "parent"), + ], + ) + .await + .expect("first sync"); + let child_before = ProjectRepository::get_by_remote_id(&guard.conn, &backend_uuid, "child") + .await + .expect("query child project") + .expect("child project stored after first sync"); + drop(guard); + + // Second sync: the parent project is gone from the fetch, but the child is still there and + // still names "parent" as its parent. + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project_with_parent("child", "Child Project", "parent")]) + .await + .expect("second sync"); + let remaining_projects = ProjectRepository::get_all(&guard.conn).await.expect("query projects"); + drop(guard); + + let ids = remote_ids(&remaining_projects, |p| p.remote_id.as_str()); + assert_eq!( + ids, + vec!["child"], + "parent project should be gone but the still-fetched child must survive, got {ids:?}" + ); + assert_eq!( + remaining_projects[0].uuid, child_before.uuid, + "the surviving child project must keep its original local uuid, not be recreated" + ); +} + +// Regression (fix round 2): a task whose project does not resolve locally is skipped by the +// upsert loop, so the delete pass must not treat it as "seen". Building the keep-set from the +// input slice let a skipped task's pre-existing row survive with stale content and a stale +// `project_uuid` — a row the sync claimed to have written but never touched. +#[tokio::test] +async fn a_task_skipped_because_its_project_is_missing_does_not_keep_a_stale_row() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, _backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("store project"); + sync_service + .store_tasks_batch( + &guard, + &[ + task("t1", "p1", "Original content", vec![]), + task("t2", "p1", "Untouched", vec![]), + ], + ) + .await + .expect("first sync"); + drop(guard); + + // Second sync: t1 now claims a project that was never stored locally (the free-tier case the + // `continue` exists for), so it is skipped and never upserted. + let guard = storage.lock().await; + sync_service + .store_tasks_batch( + &guard, + &[ + task("t1", "p-not-local", "Updated content", vec![]), + task("t2", "p1", "Untouched", vec![]), + ], + ) + .await + .expect("second sync"); + let remaining = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + drop(guard); + + let ids = remote_ids(&remaining, |t| t.remote_id.as_str()); + assert_eq!( + ids, + vec!["t2"], + "a task the upsert loop skipped must not survive the delete pass with stale content, got {remaining:?}" + ); +} + +// Sibling of the above for the parent-relinking pass: that pass must be driven by the tasks +// actually stored, not by the input slice, so a skipped task can neither be relinked itself nor +// become the parent a surviving task is relinked to. +#[tokio::test] +async fn a_skipped_task_is_never_relinked_as_a_parent() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, backend_uuid) = new_sync_service(tmp.path()).await; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("store project"); + sync_service + .store_tasks_batch( + &guard, + &[ + task("parent", "p1", "Parent task", vec![]), + task_with_parent("child", "p1", "Child task", "parent"), + ], + ) + .await + .expect("first sync"); + drop(guard); + + // The parent moved to a project that is not cached locally, so it is skipped; the child is + // still fetched and still names it as its parent. + let guard = storage.lock().await; + sync_service + .store_tasks_batch( + &guard, + &[ + task("parent", "p-not-local", "Parent task", vec![]), + task_with_parent("child", "p1", "Child task", "parent"), + ], + ) + .await + .expect("second sync"); + let remaining = TaskRepository::get_all(&guard.conn).await.expect("query tasks"); + let child = TaskRepository::get_by_remote_id(&guard.conn, &backend_uuid, "child") + .await + .expect("query child") + .expect("child survives"); + drop(guard); + + let ids = remote_ids(&remaining, |t| t.remote_id.as_str()); + assert_eq!( + ids, + vec!["child"], + "the skipped parent must not survive, and the fetched child must, got {ids:?}" + ); + assert_eq!( + child.parent_uuid, None, + "the child must not be relinked to a parent row that no longer exists" + ); +} + +// The positive case for the parent-relinking pass, pinned explicitly because every other test +// around it asserts the *absence* of a link: a subtask must actually end up pointing at its +// parent's local uuid, and must still point at it after a second identical sync (pass 1 nulls +// `parent_uuid` on every surviving row, so pass 2 has to re-establish the link each time). +#[tokio::test] +async fn a_subtask_is_linked_to_its_parents_local_uuid() { + let tmp = tempfile::tempdir().expect("tempdir"); + let (sync_service, storage, backend_uuid) = new_sync_service(tmp.path()).await; + + let fixture = [ + task("parent", "p1", "Parent task", vec![]), + task_with_parent("child", "p1", "Child task", "parent"), + ]; + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Inbox")]) + .await + .expect("store project"); + drop(guard); + + for pass in 1..=2 { + let guard = storage.lock().await; + sync_service.store_tasks_batch(&guard, &fixture).await.expect("store tasks"); + let parent = TaskRepository::get_by_remote_id(&guard.conn, &backend_uuid, "parent") + .await + .expect("query parent") + .expect("parent stored"); + let child = TaskRepository::get_by_remote_id(&guard.conn, &backend_uuid, "child") + .await + .expect("query child") + .expect("child stored"); + drop(guard); + + assert_eq!( + child.parent_uuid, + Some(parent.uuid), + "the child must be linked to its parent's local uuid on sync pass {pass}" + ); + } +} diff --git a/tests/ui/app_component.rs b/tests/ui/app_component.rs index ab49d3c..b8a0728 100644 --- a/tests/ui/app_component.rs +++ b/tests/ui/app_component.rs @@ -1,4 +1,15 @@ -use terminalist::ui::app_component::AppState; +use std::path::Path; +use std::sync::Arc; +use tokio::sync::Mutex; + +use terminalist::backend::BackendProject; +use terminalist::backend_registry::BackendRegistry; +use terminalist::config::Config; +use terminalist::storage::LocalStorage; +use terminalist::sync::{SyncService, SyncStatus}; +use terminalist::ui::app_component::{AppComponent, AppState}; +use terminalist::ui::core::actions::{Action, SelectionPolicy}; +use terminalist::ui::core::{Component, SidebarSelection}; #[test] fn test_app_state_default() { @@ -10,3 +21,196 @@ fn test_app_state_default() { "Default AppState should have no error message" ); } + +// --- Selection stability across a completed sync ----------------------------------------- +// +// These drive a real `AppComponent` over a `LocalStorage` in a `tempfile` directory (never a +// user path) with a `SyncService` in debug mode, so `trigger_initial_sync` schedules the local +// data load without ever touching the network. Background actions are awaited on the component's +// own channel, so every step is deterministic: no polling, no sleeping, no wall-clock timing. + +fn project(remote_id: &str, name: &str) -> BackendProject { + BackendProject { + remote_id: remote_id.to_string(), + name: name.to_string(), + is_favorite: false, + is_inbox: false, + order_index: 0, + parent_remote_id: None, + } +} + +/// Build an `AppComponent` backed by a seeded cache in `dir`, opening on `default_project`. +async fn new_app(dir: &Path, default_project: &str) -> AppComponent { + let storage = Arc::new(Mutex::new(LocalStorage::new_at(dir).await.expect("open local storage"))); + let registry = Arc::new(BackendRegistry::new(storage.clone())); + let backend_uuid = registry + .add_backend( + "todoist".to_string(), + "app-component-test".to_string(), + r#"{"api_token":"fake-token"}"#.to_string(), + "{}".to_string(), + ) + .await + .expect("register fake backend"); + + // `debug_mode = true` keeps `trigger_initial_sync` off the network; the sync completion + // these tests care about is injected as an action instead. + let sync_service = SyncService::new(registry, backend_uuid, true) + .await + .expect("construct sync service"); + + let guard = storage.lock().await; + sync_service + .store_projects_batch(&guard, &[project("p1", "Alpha"), project("p2", "Beta")]) + .await + .expect("seed projects"); + drop(guard); + + let mut config = Config::default(); + config.ui.default_project = default_project.to_string(); + + AppComponent::new(sync_service, config, Vec::new()) +} + +/// Feed one action through the same path the render loop uses. +async fn step(app: &mut AppComponent, action: Action) { + let processed = app.update(action); + let _ = app.handle_app_action(processed).await; +} + +/// Await the next background action and feed it in, returning it for assertions. +async fn pump(app: &mut AppComponent) -> Action { + let action = app.next_background_action().await.expect("a background action"); + step(app, action.clone()).await; + action +} + +/// Startup: load the cache, then navigate away from the configured default view. +async fn start_and_navigate_away(app: &mut AppComponent) { + app.trigger_initial_sync(); + + let initial = pump(app).await; + assert!( + matches!(initial, Action::InitialDataLoaded { .. }), + "startup should load the cache first, got {initial:?}" + ); + assert_eq!( + app.sidebar_selection(), + &SidebarSelection::Upcoming, + "the initial load should honour default_project" + ); + pump(app).await; // the follow-up fetch for the newly established selection + + // The user navigates while the sync is still running — the whole point of the branch. + step(app, Action::NavigateToSidebar(SidebarSelection::Project(0))).await; + pump(app).await; + assert_eq!(app.sidebar_selection(), &SidebarSelection::Project(0)); +} + +#[tokio::test] +async fn a_completed_sync_does_not_reset_the_sidebar_selection() { + let tmp = tempfile::tempdir().expect("tempdir"); + let mut app = new_app(tmp.path(), "upcoming").await; + + start_and_navigate_away(&mut app).await; + + // The background sync finishes. It must refresh the view, not re-run startup. + step(&mut app, Action::SyncCompleted(SyncStatus::Success)).await; + + let refresh = app.next_background_action().await.expect("post-sync refresh"); + match &refresh { + Action::DataLoaded { selection_policy, .. } => { + assert_eq!( + *selection_policy, + SelectionPolicy::FollowTask, + "a sync-delivered reload must anchor the task-list cursor to the selected task, \ + since the user may be navigating while it lands" + ); + } + other => panic!("a completed sync must refresh through the selection-preserving path, got {other:?}"), + } + step(&mut app, refresh).await; + + assert_eq!( + app.sidebar_selection(), + &SidebarSelection::Project(0), + "a completed sync must not snap the selection back to default_project" + ); +} + +#[tokio::test] +async fn refresh_data_after_a_task_operation_does_not_move_the_cursor() { + // Regression coverage at the origin, not just the mechanism: `Action::RefreshData` (sent + // after a task operation completes, e.g. pressing `t` to mark an overdue task due today) + // must reload with `SelectionPolicy::KeepIndex`, not `FollowTask`. A component-level test + // alone would still pass if `RefreshData` were wired to `FollowTask` by mistake. + let tmp = tempfile::tempdir().expect("tempdir"); + let mut app = new_app(tmp.path(), "upcoming").await; + + start_and_navigate_away(&mut app).await; + + // Simulate what `spawn_task_operation` sends once a task operation (e.g. SetTaskDueToday) + // completes: a plain `Action::RefreshData`, not a sync. + step(&mut app, Action::RefreshData).await; + + let reload = app.next_background_action().await.expect("reload after RefreshData"); + match reload { + Action::DataLoaded { selection_policy, .. } => { + assert_eq!( + selection_policy, + SelectionPolicy::KeepIndex, + "a reload triggered by a task operation must leave the task-list cursor on its row, \ + not drag it along with the task that moved" + ); + } + other => panic!("expected Action::DataLoaded after RefreshData, got {other:?}"), + } +} + +#[tokio::test] +async fn a_stray_initial_data_load_does_not_reset_the_sidebar_selection() { + // Belt and braces for the auto-sync timer: whatever reaches the component later, the + // initial selection is established exactly once. + let tmp = tempfile::tempdir().expect("tempdir"); + let mut app = new_app(tmp.path(), "upcoming").await; + + start_and_navigate_away(&mut app).await; + + step( + &mut app, + Action::InitialDataLoaded { + projects: Vec::new(), + labels: Vec::new(), + sections: Vec::new(), + tasks: Vec::new(), + }, + ) + .await; + + assert_eq!( + app.sidebar_selection(), + &SidebarSelection::Project(0), + "only the first initial load may set the selection" + ); +} + +#[tokio::test] +async fn a_sync_failure_does_not_strand_the_pending_initial_selection() { + // The sync and the local load are independent now. A sync that fails fast (a bad token, no + // network) must not stop the load that follows it from applying `default_project`. + let tmp = tempfile::tempdir().expect("tempdir"); + let mut app = new_app(tmp.path(), "upcoming").await; + + app.trigger_initial_sync(); + step(&mut app, Action::SyncFailed("no network".to_string())).await; + + let initial = pump(&mut app).await; + assert!(matches!(initial, Action::InitialDataLoaded { .. }), "got {initial:?}"); + + assert_eq!( + app.sidebar_selection(), + &SidebarSelection::Upcoming, + "a failed sync must not leave startup stuck on the fallback selection" + ); +} diff --git a/tests/ui/components.rs b/tests/ui/components.rs index aade189..7260a4c 100644 --- a/tests/ui/components.rs +++ b/tests/ui/components.rs @@ -10,6 +10,9 @@ mod dialogs; #[path = "components/sidebar_component.rs"] mod sidebar_component; +#[path = "components/sync_toast.rs"] +mod sync_toast; + #[path = "components/task_list_component.rs"] mod task_list_component; diff --git a/tests/ui/components/sync_toast.rs b/tests/ui/components/sync_toast.rs new file mode 100644 index 0000000..31ee3ec --- /dev/null +++ b/tests/ui/components/sync_toast.rs @@ -0,0 +1,299 @@ +use ratatui::{backend::TestBackend, layout::Rect, Terminal}; +use std::time::{Duration, Instant}; +use terminalist::sync::SyncStatus; +use terminalist::ui::components::sync_toast::sync_completed_successfully; +use terminalist::ui::components::{should_auto_sync, SyncToast}; + +// --- SyncToast state machine ----------------------------------------------------- + +#[test] +fn started_is_visible_with_syncing_text() { + let mut toast = SyncToast::new(); + toast.started(); + + assert!(toast.is_visible()); + assert!(toast.text().contains("Syncing"), "text was: {:?}", toast.text()); +} + +#[test] +fn succeeded_stays_visible_briefly_then_expires() { + let now = Instant::now(); + let mut toast = SyncToast::new(); + toast.succeeded(now); + assert!(toast.is_visible(), "should be visible immediately after succeeding"); + + toast.tick(now + Duration::from_secs(2)); + assert!(toast.is_visible(), "should still be visible 2s after succeeding"); + + toast.tick(now + Duration::from_secs(4)); + assert!(!toast.is_visible(), "should be hidden 4s after succeeding"); +} + +#[test] +fn failed_stays_visible_until_dismissed() { + let now = Instant::now(); + let mut toast = SyncToast::new(); + toast.failed(); + + assert!(toast.is_visible()); + toast.tick(now + Duration::from_secs(60)); + assert!( + toast.is_visible(), + "a failure must not auto-expire, even after a long time" + ); +} + +// The render loop keys its tick repaint off this, not off `is_visible`. `Failed` never expires +// by design, so repainting whenever the toast is visible pins the TUI at tick rate (10 Hz) +// forever after any failed sync. +#[test] +fn only_a_succeeded_toast_expires_on_tick() { + let now = Instant::now(); + + let hidden = SyncToast::new(); + assert!(!hidden.expires_on_tick(), "a hidden toast has nothing to expire"); + + let mut syncing = SyncToast::new(); + syncing.started(); + assert!( + !syncing.expires_on_tick(), + "the syncing text is static; a tick cannot change it" + ); + + let mut succeeded = SyncToast::new(); + succeeded.succeeded(now); + assert!(succeeded.expires_on_tick(), "a success toast expires on its own"); + + let mut failed = SyncToast::new(); + failed.failed(); + assert!( + !failed.expires_on_tick(), + "a failure clears only on a keypress, which already forces a repaint" + ); +} + +#[test] +fn a_failed_toast_never_asks_for_tick_repaints_however_long_it_stays_up() { + let now = Instant::now(); + let mut toast = SyncToast::new(); + toast.failed(); + + for minutes in [0, 1, 5, 60] { + toast.tick(now + Duration::from_secs(minutes * 60)); + assert!(toast.is_visible(), "the failure notice must stay up"); + assert!( + !toast.expires_on_tick(), + "a sticky failure must never force a repaint at {minutes} minutes in" + ); + } +} + +#[test] +fn a_succeeded_toast_stops_asking_for_repaints_once_it_has_expired() { + let now = Instant::now(); + let mut toast = SyncToast::new(); + toast.succeeded(now); + + // Still counting down: the loop must keep drawing so the toast can disappear on time. + toast.tick(now + Duration::from_secs(1)); + assert!(toast.expires_on_tick()); + + // Expired: the erase frame is driven by the value sampled *before* this tick, and from + // here on the loop goes back to idling. + toast.tick(now + Duration::from_secs(4)); + assert!(!toast.is_visible()); + assert!(!toast.expires_on_tick()); +} + +#[test] +fn dismiss_clears_a_failure() { + let mut toast = SyncToast::new(); + toast.failed(); + assert!(toast.is_visible()); + + toast.dismiss(); + assert!(!toast.is_visible()); +} + +#[test] +fn dismiss_is_a_no_op_when_not_failed() { + let now = Instant::now(); + let mut toast = SyncToast::new(); + toast.succeeded(now); + + toast.dismiss(); + assert!(toast.is_visible(), "dismiss() should only clear a Failed toast"); +} + +#[test] +fn started_while_failed_replaces_it_with_syncing() { + let mut toast = SyncToast::new(); + toast.failed(); + assert!(toast.is_visible()); + + toast.started(); + assert!(toast.is_visible()); + assert!(toast.text().contains("Syncing"), "text was: {:?}", toast.text()); +} + +// --- should_auto_sync -------------------------------------------------------------- + +#[test] +fn should_auto_sync_never_when_interval_is_zero() { + let now = Instant::now(); + let last = now - Duration::from_secs(3600); + + assert!(!should_auto_sync(Some(last), now, 0, false)); +} + +#[test] +fn should_auto_sync_never_while_a_sync_is_in_flight() { + let now = Instant::now(); + let last = now - Duration::from_secs(3600); + + assert!(!should_auto_sync(Some(last), now, 5, true)); +} + +#[test] +fn should_auto_sync_false_when_never_synced() { + let now = Instant::now(); + + assert!( + !should_auto_sync(None, now, 5, false), + "the startup sync is triggered explicitly, not by the timer" + ); +} + +#[test] +fn should_auto_sync_false_before_the_interval_elapses() { + let now = Instant::now(); + let interval_minutes = 5; + let last = now - Duration::from_secs(interval_minutes * 60 - 1); + + assert!(!should_auto_sync(Some(last), now, interval_minutes, false)); +} + +#[test] +fn should_auto_sync_true_once_the_interval_elapses() { + let now = Instant::now(); + let interval_minutes = 5; + let last = now - Duration::from_secs(interval_minutes * 60); + + assert!(should_auto_sync(Some(last), now, interval_minutes, false)); +} + +/// Regression test for a retry-storm bug: a sync that fails fast (e.g. a backend +/// resolution error, which resolves well within one ~100ms tick) must not cause the +/// auto-sync timer to fire again on the very next tick. The fix is that `AppComponent` +/// updates `last_sync_attempt_at` on *every* terminal outcome, not just success — so by +/// the time the failed attempt is recorded, `now` and the recorded attempt are the same +/// instant (the worst case: zero elapsed time), and the timer must still treat that as +/// "not yet". +#[test] +fn should_auto_sync_does_not_immediately_refire_after_a_same_instant_failed_attempt() { + let now = Instant::now(); + let interval_minutes = 5; + + // The failed attempt is recorded at the same instant it's checked (elapsed == 0): + // the worst case for a fast-failing sync (e.g. get_backend() resolution failure). + assert!( + !should_auto_sync(Some(now), now, interval_minutes, false), + "a just-failed attempt must not immediately refire the timer" + ); + + // It also must not refire on ticks shortly after, only once a full interval has + // actually elapsed since that failed attempt. + let almost_there = now + Duration::from_secs(interval_minutes * 60 - 1); + assert!(!should_auto_sync(Some(now), almost_there, interval_minutes, false)); + + let interval_elapsed = now + Duration::from_secs(interval_minutes * 60); + assert!( + should_auto_sync(Some(now), interval_elapsed, interval_minutes, false), + "a failed attempt should retry once, and only once, a full interval has passed" + ); +} + +// --- sync_completed_successfully ---------------------------------------------------- + +#[test] +fn sync_completed_successfully_is_true_only_for_success() { + assert!(sync_completed_successfully(&SyncStatus::Success)); +} + +#[test] +fn sync_completed_successfully_is_false_for_an_error_status() { + assert!(!sync_completed_successfully(&SyncStatus::Error { + message: "boom".to_string(), + })); +} + +#[test] +fn sync_completed_successfully_is_false_and_does_not_panic_for_unexpected_statuses() { + // `Idle`/`InProgress` should never actually reach a "completed" handler, but this + // must degrade defensively (treated as failure) rather than panicking. + assert!(!sync_completed_successfully(&SyncStatus::Idle)); + assert!(!sync_completed_successfully(&SyncStatus::InProgress)); +} + +// --- render bounds ------------------------------------------------------------------- + +/// Renders a visible toast into a `task_list_area` covering the whole given terminal +/// size and asserts it doesn't panic. Returns the terminal so the caller can inspect +/// the buffer if needed. +fn render_into(width: u16, height: u16, toast: &SyncToast) -> Terminal { + let backend = TestBackend::new(width, height); + let mut terminal = Terminal::new(backend).expect("terminal should construct"); + terminal + .draw(|f| { + let area = Rect::new(0, 0, width, height); + toast.render(f, area); + }) + .expect("render should not panic on a small terminal"); + terminal +} + +#[test] +fn render_does_not_panic_on_a_very_small_terminal() { + let mut toast = SyncToast::new(); + toast.started(); + + // Small enough that the 1-cell border inset leaves almost nothing to work with. + for (width, height) in [(20, 5), (3, 3), (2, 2), (1, 1), (0, 0)] { + render_into(width, height, &toast); + } +} + +#[test] +fn render_respects_the_1_cell_border_inset_on_a_small_terminal() { + let mut toast = SyncToast::new(); + toast.started(); + + let width = 20u16; + let height = 5u16; + let terminal = render_into(width, height, &toast); + let buffer = terminal.backend().buffer(); + + // The toast must be inset by 1 cell from the edge of `task_list_area` (that's + // where the task list's own border lives), so the outermost ring of cells must + // stay untouched: top row, bottom row, left column, right column. + for x in 0..width { + assert_eq!(buffer[(x, 0)].symbol(), " ", "top row must stay blank"); + assert_eq!(buffer[(x, height - 1)].symbol(), " ", "bottom row must stay blank"); + } + for y in 0..height { + assert_eq!(buffer[(0, y)].symbol(), " ", "left column must stay blank"); + assert_eq!(buffer[(width - 1, y)].symbol(), " ", "right column must stay blank"); + } +} + +#[test] +fn render_is_a_no_op_when_hidden() { + let toast = SyncToast::new(); + assert!(!toast.is_visible()); + + let terminal = render_into(20, 5, &toast); + let buffer = terminal.backend().buffer(); + for cell in buffer.content() { + assert_eq!(cell.symbol(), " ", "a hidden toast should paint nothing"); + } +} diff --git a/tests/ui/components/task_list_component.rs b/tests/ui/components/task_list_component.rs index 5c3cb46..2d0932b 100644 --- a/tests/ui/components/task_list_component.rs +++ b/tests/ui/components/task_list_component.rs @@ -1,7 +1,267 @@ +use terminalist::entities::task; +use terminalist::ui::components::task_list_item_component::ListItem; use terminalist::ui::components::TaskListComponent; +use terminalist::ui::core::actions::SelectionPolicy; +use terminalist::ui::core::SidebarSelection; +use terminalist::utils::datetime; +use uuid::Uuid; #[test] fn test_task_list_component_creation() { // Test that TaskListComponent can be created without panicking let _task_list = TaskListComponent::new(); } + +/// Build a minimal, valid task::Model for tests. Every task gets its own random uuid +/// and project_uuid, and no due date/section/parent, so it always renders as a plain +/// root-level row regardless of sidebar view. +fn make_task(content: &str) -> task::Model { + task::Model { + uuid: Uuid::new_v4(), + backend_uuid: Uuid::new_v4(), + remote_id: String::new(), + content: content.to_string(), + description: None, + project_uuid: Uuid::new_v4(), + section_uuid: None, + parent_uuid: None, + priority: 1, + order_index: 0, + due_date: None, + due_datetime: None, + is_recurring: false, + deadline: None, + duration: None, + is_completed: false, + is_deleted: false, + } +} + +/// Like `make_task`, but with a due date set, for driving `SidebarSelection::Today`'s +/// Overdue/Today sectioning. +fn make_task_due(content: &str, due_date: &str) -> task::Model { + task::Model { + due_date: Some(due_date.to_string()), + ..make_task(content) + } +} + +// SidebarSelection::Project(0) with an empty projects vec falls back to the +// unsectioned "simple items" builder, so every task in `tasks` becomes a plain +// selectable row in list order without needing due dates or matching projects/sections. +fn load(component: &mut TaskListComponent, tasks: Vec, policy: SelectionPolicy) { + component.update_data( + tasks, + Vec::new(), + Vec::new(), + Vec::new(), + SidebarSelection::Project(0), + policy, + ); +} + +// SidebarSelection::Today groups tasks under "Overdue"/"Today" Header rows (with a +// Separator between them when both are present) via build_today_items, so this is what +// actually exercises non-selectable rows in the item list. +fn load_today(component: &mut TaskListComponent, tasks: Vec, policy: SelectionPolicy) { + component.update_data( + tasks, + Vec::new(), + Vec::new(), + Vec::new(), + SidebarSelection::Today, + policy, + ); +} + +#[test] +fn selection_follows_the_task_across_a_reload() { + let mut component = TaskListComponent::new(); + + let task_a = make_task("Task A"); + let task_b = make_task("Task B"); + let task_b_uuid = task_b.uuid; + + load( + &mut component, + vec![task_a.clone(), task_b.clone()], + SelectionPolicy::FollowTask, + ); + + // Select task B (logical index 1: task A, task B). + component.selected_index = 1; + assert_eq!(component.get_selected_task().map(|t| t.uuid), Some(task_b_uuid)); + + // Reload with a new task inserted above task B, using the policy a sync-delivered + // reload uses: FollowTask. + let task_new = make_task("Newly synced task"); + load( + &mut component, + vec![task_new, task_a, task_b], + SelectionPolicy::FollowTask, + ); + + // Selection must still be task B, not whatever now sits at logical index 1. + assert_eq!( + component.get_selected_task().map(|t| t.uuid), + Some(task_b_uuid), + "selection should follow task B, not stay pinned to its old index" + ); +} + +#[test] +fn keep_index_leaves_the_cursor_on_the_row_across_a_reload() { + // Pins the regression: pressing `t` to mark an overdue task due "today" must not drag + // the cursor along with it. That reload is user-initiated (a task operation, not a + // sync), so it uses KeepIndex, and `selected_index` must stay put even though a + // different task now occupies that row. + let mut component = TaskListComponent::new(); + + let task_a = make_task("Task A"); + let task_b = make_task("Task B"); + let task_b_uuid = task_b.uuid; + + load( + &mut component, + vec![task_a.clone(), task_b.clone()], + SelectionPolicy::FollowTask, + ); + + // Select task B (logical index 1: task A, task B). + component.selected_index = 1; + assert_eq!(component.get_selected_task().map(|t| t.uuid), Some(task_b_uuid)); + + // Reload with a new task inserted between task A and task B -- i.e. right above task B, + // at the row task B used to occupy (logical index 1) -- using KeepIndex. + let task_new = make_task("Task moved into this section"); + let task_new_uuid = task_new.uuid; + load( + &mut component, + vec![task_a, task_new, task_b], + SelectionPolicy::KeepIndex, + ); + + // selected_index must be untouched... + assert_eq!( + component.selected_index, 1, + "KeepIndex must leave selected_index unchanged" + ); + // ...so the selected task is now whichever task occupies row 1, not task B. + assert_eq!( + component.get_selected_task().map(|t| t.uuid), + Some(task_new_uuid), + "KeepIndex should leave the cursor on row 1, now occupied by the newly inserted task, not follow task B" + ); +} + +#[test] +fn selection_survives_the_selected_task_disappearing() { + let mut component = TaskListComponent::new(); + + let task_a = make_task("Task A"); + let task_b = make_task("Task B"); + + load( + &mut component, + vec![task_a.clone(), task_b], + SelectionPolicy::FollowTask, + ); + + // Select task B (logical index 1). + component.selected_index = 1; + assert_eq!( + component.get_selected_task().map(|t| t.content.clone()), + Some("Task B".to_string()) + ); + + // Reload without task B at all (e.g. completed/deleted elsewhere). + load(&mut component, vec![task_a], SelectionPolicy::FollowTask); + + // No panic, and the selection must land on a valid, in-bounds row. + assert_eq!(component.selected_index, 0); + assert!(component.get_selected_task().is_some()); +} + +#[test] +fn empty_reload_resets_selection_without_panicking() { + let mut component = TaskListComponent::new(); + + let task_a = make_task("Task A"); + load(&mut component, vec![task_a], SelectionPolicy::FollowTask); + component.selected_index = 0; + assert!(component.get_selected_task().is_some()); + + // Reload with no tasks at all. + load(&mut component, Vec::new(), SelectionPolicy::FollowTask); + + assert_eq!(component.selected_index, 0); + assert!(component.get_selected_task().is_none()); + assert!(component.items.is_empty()); +} + +#[test] +fn selection_follows_the_task_across_a_reload_with_section_headers() { + let mut component = TaskListComponent::new(); + + // Safely in the past regardless of when this test runs, so it always lands in the + // Overdue bucket. + let overdue_task = make_task_due("Overdue task", "2000-01-01"); + let overdue_task_uuid = overdue_task.uuid; + let today_task = make_task_due("Today task", &datetime::format_today()); + + load_today( + &mut component, + vec![overdue_task.clone(), today_task.clone()], + SelectionPolicy::FollowTask, + ); + + // Confirm the fixture genuinely produced header/separator rows (Header("Overdue"), + // Task, Separator, Header("Today"), Task) rather than collapsing into a flat list — + // otherwise this test would be indistinguishable from the flat-list cases above and + // wouldn't exercise the logical/physical index distinction at all. + assert_eq!( + component.items.len(), + 5, + "expected 2 tasks plus 3 non-selectable rows (Overdue header, separator, Today header), got {:?}", + component.items + ); + let non_selectable_count = component.items.iter().filter(|item| !item.is_selectable()).count(); + assert_eq!( + non_selectable_count, 3, + "expected 3 non-selectable rows in the item list, got {:?}", + component.items + ); + + // Select the overdue task: it sits right below the Overdue header, at physical index 1 + // but logical index 0 (only the 2 Task rows count logically). Deliberately NOT the last + // selectable row in the list — see below for why that matters. + component.selected_index = 0; + assert_eq!(component.get_selected_task().map(|t| t.uuid), Some(overdue_task_uuid)); + + // Reload with a new overdue task added ahead of it. It lands in the earlier (Overdue) + // section, so the previously selected overdue task shifts from physical index 1 to + // physical index 2 (logical index 0 to logical index 1), while the still-last today task + // keeps sitting at the highest logical/physical index either way. Anchoring the + // previously-selected task to anything but its own uuid (e.g. mixing up a physical + // position with a logical one) would land selection on the wrong task here — and, + // because it is not the last selectable row, `update_list_state`'s out-of-range clamp + // cannot coincidentally paper over the mistake by forcing it back onto the right task. + let new_overdue_task = make_task_due("New overdue task", "2000-01-02"); + load_today( + &mut component, + vec![new_overdue_task, overdue_task, today_task], + SelectionPolicy::FollowTask, + ); + + assert_eq!( + component.items.len(), + 6, + "expected 3 tasks plus 3 non-selectable rows after the reload, got {:?}", + component.items + ); + assert_eq!( + component.get_selected_task().map(|t| t.uuid), + Some(overdue_task_uuid), + "selection should follow the overdue task even though header rows shifted its physical position" + ); +}