From efbbe12db3b3ac3fc1e2f59e5ea9c718f3549016 Mon Sep 17 00:00:00 2001 From: Steven Carter Date: Sun, 23 Aug 2026 01:25:53 -0400 Subject: [PATCH] feat(sync): persist the local cache and sync in the background MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every launch deleted the SQLite cache, then blocked the UI behind a centered "Loading data" overlay while it re-downloaded the entire account over four sequential API round-trips. The cache location was hardcoded, so users could not move it off a synced home directory, and `cargo test` deleted a real installation's database on every run. `auto_sync_interval_minutes` was parsed, defaulted and validated, but never read by any runtime code — no timer ever existed. Storage: - New `[storage] data_dir` config key. A leading `~` is expanded and relative paths resolve against the working directory; unset keeps the platform default. An unusable path fails at startup with the path in the error rather than silently falling back, which would send writes to the database the user was trying to move away from. - The cache is no longer deleted at startup, and the schema is created idempotently, so it survives across launches. - The backend row's UUID is derived from (type, name) instead of being generated per launch. An existing row's UUID is adopted as-is, so a database written by an earlier version keeps its cache instead of having every project and task orphaned under a dead backend_uuid. Duplicate rows left by older debug runs are collapsed before the new unique index is built. - The database file is created 0600 on Unix; it holds the API token. Sync: - Reconciles instead of rebuilding: rows are upserted on (backend_uuid, remote_id) and rows the remote no longer returns are deleted. The delete pass runs before parent re-linking, so completing a parent task cannot cascade its still-open subtasks out of the cache. Local UUIDs stay stable across syncs. - An empty fetch never wipes an entity table. - The four fetches run concurrently; per-resource error handling is unchanged, including sections degrading to empty rather than failing. Interface: - Startup paints cached data immediately and syncs in the background. - Sync state appears as a toast in the lower right of the task list — syncing, synced, or failed — instead of a modal that ate keystrokes. A failed sync now reports as failed rather than as success. - `auto_sync_interval_minutes` drives a real timer; 0 disables it, and `r` still forces a sync. Every terminal outcome records the attempt, so a failing backend is retried once per interval rather than ten times a second. - Task selection follows the selected task across a sync-driven reload, so data arriving out of band cannot move the cursor under the user's hands. Reloads the user asked for — a task operation, a sidebar change, a debug refresh — leave the cursor on the row it was on, so marking an overdue task due today does not drag the cursor along as the task moves out of the Overdue section. Each reload carries its own policy, rather than the list guessing from shared state. Behavior change: a locally completed or deleted task is removed from the cache at the next sync rather than lingering until the next launch, so it can be restored from the list for at most one auto-sync interval. Excluding those rows instead would make them permanent tombstones, since the remote never returns them again. README and the architecture, configuration and development docs described a system that never existed — an in-memory database, a five-minute staleness check, and a dependency list naming crates the project does not use. They now describe the code. Tests no longer touch any user path: storage is constructed against a tempfile directory throughout, and timing decisions are pure functions taking `now`, so nothing sleeps. Claude-Session: https://claude.ai/code/session_01QddAyPzAbdC5bRK7nccDv5 --- CHANGELOG.md | 10 + Cargo.lock | 27 + Cargo.toml | 5 +- README.md | 9 +- docs/ARCHITECTURE.md | 56 +- docs/CONFIGURATION.md | 28 +- docs/DEVELOPMENT.md | 18 +- src/backend_registry.rs | 41 +- src/config.rs | 47 +- src/constants.rs | 3 - src/main.rs | 12 +- src/repositories/backend.rs | 17 + src/storage.rs | 165 ++++- src/sync/mod.rs | 45 +- src/sync/storage.rs | 178 +++++- src/ui/app_component.rs | 295 +++++---- src/ui/components/mod.rs | 3 + src/ui/components/sync_toast.rs | 219 +++++++ src/ui/components/task_list_component.rs | 39 +- src/ui/core/actions.rs | 25 + src/ui/core/mod.rs | 2 +- src/ui/core/task_manager.rs | 24 +- src/ui/mod.rs | 2 +- src/ui/renderer.rs | 14 +- tests/config.rs | 87 +++ tests/storage.rs | 3 + tests/storage/db.rs | 244 +++++++- tests/storage/labels.rs | 8 - tests/storage/migration.rs | 245 ++++++++ tests/storage/projects.rs | 8 - tests/storage/sections.rs | 8 - tests/storage/tasks.rs | 8 - tests/sync.rs | 2 + tests/sync/reconcile.rs | 687 +++++++++++++++++++++ tests/ui/app_component.rs | 206 +++++- tests/ui/components.rs | 3 + tests/ui/components/sync_toast.rs | 299 +++++++++ tests/ui/components/task_list_component.rs | 260 ++++++++ 38 files changed, 3074 insertions(+), 278 deletions(-) create mode 100644 src/ui/components/sync_toast.rs delete mode 100644 tests/storage/labels.rs create mode 100644 tests/storage/migration.rs delete mode 100644 tests/storage/projects.rs delete mode 100644 tests/storage/sections.rs delete mode 100644 tests/storage/tasks.rs create mode 100644 tests/sync.rs create mode 100644 tests/sync/reconcile.rs create mode 100644 tests/ui/components/sync_toast.rs 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" + ); +}