max / goingson
18 files changed,
+379 insertions,
-65 deletions
| @@ -173,6 +173,11 @@ pub trait TaskRepository: Send + Sync { | |||
| 173 | 173 | /// Lists unscheduled tasks due on a specific date. | |
| 174 | 174 | async fn list_unscheduled_due_on_date(&self, user_id: UserId, date: NaiveDate) -> Result<Vec<Task>>; | |
| 175 | 175 | ||
| 176 | + | /// Lists unscheduled tasks whose `due` falls within a UTC instant window | |
| 177 | + | /// (inclusive upper bound). Lets callers honor a user-local day boundary | |
| 178 | + | /// instead of an implicit UTC one. | |
| 179 | + | async fn list_unscheduled_due_between(&self, user_id: UserId, start: DateTime<Utc>, end: DateTime<Utc>) -> Result<Vec<Task>>; | |
| 180 | + | ||
| 176 | 181 | /// Updates a task's time-block schedule. | |
| 177 | 182 | async fn update_schedule(&self, id: TaskId, user_id: UserId, start: Option<DateTime<Utc>>, duration: Option<i32>) -> Result<Option<Task>>; | |
| 178 | 183 |
| @@ -840,6 +840,11 @@ impl TaskRepository for SqliteTaskRepository { | |||
| 840 | 840 | } | |
| 841 | 841 | ||
| 842 | 842 | #[tracing::instrument(skip_all)] | |
| 843 | + | async fn list_unscheduled_due_between(&self, user_id: UserId, start: DateTime<Utc>, end: DateTime<Utc>) -> Result<Vec<Task>> { | |
| 844 | + | task_repo_state::list_unscheduled_due_between(&self.pool, user_id, start, end).await | |
| 845 | + | } | |
| 846 | + | ||
| 847 | + | #[tracing::instrument(skip_all)] | |
| 843 | 848 | async fn update_schedule(&self, id: TaskId, user_id: UserId, start: Option<DateTime<Utc>>, duration: Option<i32>) -> Result<Option<Task>> { | |
| 844 | 849 | task_repo_state::update_schedule(&self.pool, id, user_id, start, duration).await | |
| 845 | 850 | } |
| @@ -183,6 +183,25 @@ pub(crate) async fn list_unscheduled_due_on_date( | |||
| 183 | 183 | query_tasks(pool, &sql, &[user_id.to_string(), user_id.to_string(), date_start, date_end]).await | |
| 184 | 184 | } | |
| 185 | 185 | ||
| 186 | + | /// List unscheduled tasks whose `due` falls within an explicit UTC instant | |
| 187 | + | /// window (inclusive upper bound). The caller maps a user-local civil day to | |
| 188 | + | /// this range so the boundary respects the user's timezone, not UTC midnight. | |
| 189 | + | pub(crate) async fn list_unscheduled_due_between( | |
| 190 | + | pool: &SqlitePool, | |
| 191 | + | user_id: UserId, | |
| 192 | + | start: DateTime<Utc>, | |
| 193 | + | end: DateTime<Utc>, | |
| 194 | + | ) -> Result<Vec<Task>> { | |
| 195 | + | let start_str = format_datetime(&start); | |
| 196 | + | let end_str = format_datetime(&end); | |
| 197 | + | ||
| 198 | + | let sql = format!( | |
| 199 | + | "SELECT {} FROM tasks t LEFT JOIN projects p ON t.project_id = p.id AND p.user_id = ? LEFT JOIN contacts ct ON ct.id = t.contact_id WHERE t.user_id = ? AND t.status != 'Deleted' AND t.status != 'Completed' AND t.scheduled_start IS NULL AND t.due IS NOT NULL AND datetime(t.due) >= datetime(?) AND datetime(t.due) <= datetime(?) ORDER BY t.urgency DESC, t.due ASC", | |
| 200 | + | TASK_SELECT_COLUMNS | |
| 201 | + | ); | |
| 202 | + | query_tasks(pool, &sql, &[user_id.to_string(), user_id.to_string(), start_str, end_str]).await | |
| 203 | + | } | |
| 204 | + | ||
| 186 | 205 | /// Update the scheduled start time and duration for a task. | |
| 187 | 206 | pub(crate) async fn update_schedule( | |
| 188 | 207 | pool: &SqlitePool, |
| @@ -335,6 +335,22 @@ pub(crate) async fn log_manual_time( | |||
| 335 | 335 | ) -> Result<TimeSession> { | |
| 336 | 336 | use chrono::Duration; | |
| 337 | 337 | ||
| 338 | + | // Verify the task exists and belongs to this user *before* any write. The | |
| 339 | + | // session INSERT below binds `task_id` unconditionally and the task UPDATE | |
| 340 | + | // is user-scoped, so without this precheck a foreign/bogus id would leave an | |
| 341 | + | // orphan time_sessions row while silently no-op'ing the cache update. | |
| 342 | + | let owns_task: Option<(i64,)> = sqlx::query_as( | |
| 343 | + | "SELECT 1 FROM tasks WHERE id = ? AND user_id = ?" | |
| 344 | + | ) | |
| 345 | + | .bind(task_id.to_string()) | |
| 346 | + | .bind(user_id.to_string()) | |
| 347 | + | .fetch_optional(pool) | |
| 348 | + | .await | |
| 349 | + | .map_err(CoreError::database)?; | |
| 350 | + | if owns_task.is_none() { | |
| 351 | + | return Err(CoreError::not_found("task", task_id)); | |
| 352 | + | } | |
| 353 | + | ||
| 338 | 354 | let id = TimeSessionId::new(); | |
| 339 | 355 | let started_at = date; | |
| 340 | 356 | let ended_at = date + Duration::minutes(minutes as i64); | |
| @@ -359,13 +375,14 @@ pub(crate) async fn log_manual_time( | |||
| 359 | 375 | .await | |
| 360 | 376 | .map_err(CoreError::database)?; | |
| 361 | 377 | ||
| 362 | - | // Update task's cached actual_minutes | |
| 378 | + | // Update task's cached actual_minutes. NB: the `tasks` table has no | |
| 379 | + | // `updated_at` column (only `created_at`), so this must not reference one — | |
| 380 | + | // the working stop_timer path updates the same cache the same way. | |
| 363 | 381 | sqlx::query( | |
| 364 | - | "UPDATE tasks SET actual_minutes = COALESCE(actual_minutes, 0) + ?, updated_at = ? | |
| 382 | + | "UPDATE tasks SET actual_minutes = COALESCE(actual_minutes, 0) + ? | |
| 365 | 383 | WHERE id = ? AND user_id = ?" | |
| 366 | 384 | ) | |
| 367 | 385 | .bind(minutes) | |
| 368 | - | .bind(&created_str) | |
| 369 | 386 | .bind(task_id.to_string()) | |
| 370 | 387 | .bind(user_id.to_string()) | |
| 371 | 388 | .execute(pool) |
| @@ -411,3 +411,42 @@ async fn test_snooze_started_task_succeeds() { | |||
| 411 | 411 | assert!(snoozed.is_some()); | |
| 412 | 412 | assert!(snoozed.unwrap().snoozed_until.is_some()); | |
| 413 | 413 | } | |
| 414 | + | ||
| 415 | + | // Backs GO-6: the windowed due query must include a task whose `due` lies | |
| 416 | + | // inside the UTC window and exclude one just outside it. The day_planning | |
| 417 | + | // command maps a user-local civil day to this window so the boundary respects | |
| 418 | + | // the user's timezone, not UTC midnight. | |
| 419 | + | #[tokio::test] | |
| 420 | + | async fn test_list_unscheduled_due_between_respects_window() { | |
| 421 | + | let pool = common::setup_test_db().await; | |
| 422 | + | let user_id = common::create_test_user(&pool).await; | |
| 423 | + | let repo = SqliteTaskRepository::new(pool); | |
| 424 | + | ||
| 425 | + | // UTC-5 local day 2026-06-11 -> [05:00Z 11th, 05:00Z 12th]. | |
| 426 | + | let win_start = "2026-06-11T05:00:00Z".parse::<chrono::DateTime<Utc>>().unwrap(); | |
| 427 | + | let win_end = "2026-06-12T04:59:59Z".parse::<chrono::DateTime<Utc>>().unwrap(); | |
| 428 | + | ||
| 429 | + | // Due 02:00Z on the 12th = 21:00 local on the 11th -> inside the window. | |
| 430 | + | let inside = repo | |
| 431 | + | .create(user_id, NewTask::builder("Due late local 11th") | |
| 432 | + | .due("2026-06-12T02:00:00Z".parse::<chrono::DateTime<Utc>>().unwrap()) | |
| 433 | + | .build()) | |
| 434 | + | .await | |
| 435 | + | .expect("create inside"); | |
| 436 | + | ||
| 437 | + | // Due 06:00Z on the 12th = 01:00 local on the 12th -> outside the window. | |
| 438 | + | let _outside = repo | |
| 439 | + | .create(user_id, NewTask::builder("Due early local 12th") | |
| 440 | + | .due("2026-06-12T06:00:00Z".parse::<chrono::DateTime<Utc>>().unwrap()) | |
| 441 | + | .build()) | |
| 442 | + | .await | |
| 443 | + | .expect("create outside"); | |
| 444 | + | ||
| 445 | + | let found = repo | |
| 446 | + | .list_unscheduled_due_between(user_id, win_start, win_end) | |
| 447 | + | .await | |
| 448 | + | .expect("query window"); | |
| 449 | + | ||
| 450 | + | assert_eq!(found.len(), 1, "only the in-window task should match"); | |
| 451 | + | assert_eq!(found[0].id, inside.id); | |
| 452 | + | } |
| @@ -2,7 +2,7 @@ | |||
| 2 | 2 | ||
| 3 | 3 | mod common; | |
| 4 | 4 | ||
| 5 | - | use goingson_core::TaskRepository; | |
| 5 | + | use goingson_core::{CoreError, TaskId, TaskRepository}; | |
| 6 | 6 | use goingson_db_sqlite::SqliteTaskRepository; | |
| 7 | 7 | ||
| 8 | 8 | #[tokio::test] | |
| @@ -252,3 +252,44 @@ async fn test_task_with_active_session_has_active_session_populated() { | |||
| 252 | 252 | assert!(task.active_session.is_some(), "Task with running timer should have active_session"); | |
| 253 | 253 | assert!(task.has_active_timer()); | |
| 254 | 254 | } | |
| 255 | + | ||
| 256 | + | #[tokio::test] | |
| 257 | + | async fn test_log_manual_time_happy_path() { | |
| 258 | + | let pool = common::setup_test_db().await; | |
| 259 | + | let user_id = common::create_test_user(&pool).await; | |
| 260 | + | let task_id = common::create_test_task(&pool, user_id).await; | |
| 261 | + | let repo = SqliteTaskRepository::new(pool); | |
| 262 | + | ||
| 263 | + | let session = repo | |
| 264 | + | .log_manual_time(task_id, user_id, 30, chrono::Utc::now()) | |
| 265 | + | .await | |
| 266 | + | .expect("logging manual time on an owned task should succeed"); | |
| 267 | + | assert_eq!(session.duration_minutes, Some(30)); | |
| 268 | + | ||
| 269 | + | let task = repo.get_by_id(task_id, user_id).await.unwrap().unwrap(); | |
| 270 | + | assert_eq!(task.actual_minutes, 30, "cache should reflect the logged minutes"); | |
| 271 | + | ||
| 272 | + | let sessions = repo.list_time_sessions(task_id, user_id).await.unwrap(); | |
| 273 | + | assert_eq!(sessions.len(), 1); | |
| 274 | + | } | |
| 275 | + | ||
| 276 | + | // GO-5 regression: logging time against a task the caller doesn't own (here a | |
| 277 | + | // nonexistent id) must be rejected with NOT_FOUND and must NOT leave an orphan | |
| 278 | + | // time_sessions row. | |
| 279 | + | #[tokio::test] | |
| 280 | + | async fn test_log_manual_time_foreign_task_rejected_no_orphan() { | |
| 281 | + | let pool = common::setup_test_db().await; | |
| 282 | + | let user_id = common::create_test_user(&pool).await; | |
| 283 | + | let repo = SqliteTaskRepository::new(pool); | |
| 284 | + | ||
| 285 | + | let bogus_task = TaskId::new(); | |
| 286 | + | let result = repo | |
| 287 | + | .log_manual_time(bogus_task, user_id, 30, chrono::Utc::now()) | |
| 288 | + | .await; | |
| 289 | + | ||
| 290 | + | assert!(matches!(result, Err(CoreError::NotFound { .. })), "expected NOT_FOUND, got {result:?}"); | |
| 291 | + | ||
| 292 | + | // No session row should have been written for the bogus task. | |
| 293 | + | let sessions = repo.list_time_sessions(bogus_task, user_id).await.unwrap(); | |
| 294 | + | assert!(sessions.is_empty(), "a rejected log must not leave an orphan session"); | |
| 295 | + | } |
| @@ -223,13 +223,18 @@ async function syncAllEmailAccounts() { | |||
| 223 | 223 | GoingsOn.ui.showToast('No email accounts configured', 'info'); | |
| 224 | 224 | return; | |
| 225 | 225 | } | |
| 226 | - | GoingsOn.ui.showToast('Syncing email accounts...', 'info'); | |
| 226 | + | // Persistent progress modal: this loops a blocking IMAP fetch per | |
| 227 | + | // account and can run for minutes. Without it the app looks frozen. | |
| 228 | + | const plural = accounts.length === 1 ? 'account' : 'accounts'; | |
| 229 | + | GoingsOn.emails.showSyncProgressModal(`Syncing ${accounts.length} email ${plural}...`); | |
| 227 | 230 | for (const account of accounts) { | |
| 228 | 231 | await GoingsOn.api.emailAccounts.sync(account.id, false); | |
| 229 | 232 | } | |
| 233 | + | GoingsOn.ui.closeModal(); | |
| 230 | 234 | GoingsOn.ui.showToast('Email sync complete!', 'success'); | |
| 231 | 235 | GoingsOn.emails.load(); | |
| 232 | 236 | } catch (err) { | |
| 237 | + | GoingsOn.ui.closeModal(); | |
| 233 | 238 | GoingsOn.ui.showToast('Email sync failed: ' + GoingsOn.utils.getErrorMessage(err), 'error', { | |
| 234 | 239 | action: { label: 'Retry', fn: syncAllEmailAccounts }, | |
| 235 | 240 | duration: 8000, |
| @@ -550,7 +550,13 @@ | |||
| 550 | 550 | * @param {boolean} [fullSync=false] - true for full re-sync, false for new-only | |
| 551 | 551 | */ | |
| 552 | 552 | async function syncAccount(id, fullSync = false) { | |
| 553 | - | GoingsOn.ui.showToast(fullSync ? 'Starting full sync...' : 'Starting sync...', 'info'); | |
| 553 | + | // A first full sync pulls thousands of messages over minutes. Show a | |
| 554 | + | // persistent progress modal (reuses the OAuth-waiting pattern) so the | |
| 555 | + | // app doesn't look frozen; the modal also blocks the Sync buttons behind | |
| 556 | + | // it, preventing the double-click-queues-a-second-sync problem. | |
| 557 | + | showSyncProgressModal(fullSync | |
| 558 | + | ? 'Full sync in progress. This can take a few minutes for the first run.' | |
| 559 | + | : 'Syncing new messages...'); | |
| 554 | 560 | ||
| 555 | 561 | try { | |
| 556 | 562 | const result = await GoingsOn.api.emailAccounts.sync(id, fullSync); | |
| @@ -583,6 +589,8 @@ | |||
| 583 | 589 | GoingsOn.emails.load(); | |
| 584 | 590 | } | |
| 585 | 591 | } catch (err) { | |
| 592 | + | // Dismiss the progress modal before surfacing the error toast. | |
| 593 | + | GoingsOn.ui.closeModal(); | |
| 586 | 594 | GoingsOn.ui.showToast(GoingsOn.utils.getErrorMessage(err, 'Sync failed'), 'error', { | |
| 587 | 595 | action: { label: 'Retry', fn: () => syncAccount(id, fullSync) }, | |
| 588 | 596 | duration: 8000, | |
| @@ -590,6 +598,23 @@ | |||
| 590 | 598 | } | |
| 591 | 599 | } | |
| 592 | 600 | ||
| 601 | + | /** | |
| 602 | + | * Persistent spinner modal shown while an email sync runs. Replaced by the | |
| 603 | + | * Sync Results modal on success, or dismissed on error. Mirrors | |
| 604 | + | * showOAuthWaitingModal. | |
| 605 | + | * @param {string} message - Status line describing the in-progress sync | |
| 606 | + | */ | |
| 607 | + | function showSyncProgressModal(message) { | |
| 608 | + | const content = ` | |
| 609 | + | <div class="oauth-waiting"> | |
| 610 | + | <div class="oauth-waiting-title">Syncing email...</div> | |
| 611 | + | <div class="oauth-waiting-body">${esc(message)}</div> | |
| 612 | + | <div class="spinner oauth-waiting-spinner"></div> | |
| 613 | + | </div> | |
| 614 | + | `; | |
| 615 | + | GoingsOn.ui.openModal('Email Sync', content); | |
| 616 | + | } | |
| 617 | + | ||
| 593 | 618 | // ============ OAuth Flow ============ | |
| 594 | 619 | ||
| 595 | 620 | // Store OAuth state during flow | |
| @@ -788,6 +813,7 @@ | |||
| 788 | 813 | deleteAccount, | |
| 789 | 814 | testAccount, | |
| 790 | 815 | syncAccount, | |
| 816 | + | showSyncProgressModal, | |
| 791 | 817 | // OAuth | |
| 792 | 818 | startOAuth, | |
| 793 | 819 | cancelOAuth, |
| @@ -679,11 +679,12 @@ | |||
| 679 | 679 | let eventRecord; | |
| 680 | 680 | try { eventRecord = await GoingsOn.api.events.get(id); } catch (_) { /* fetch failed; fall through */ } | |
| 681 | 681 | const isRecurring = !!(eventRecord && eventRecord.recurrence && eventRecord.recurrence !== 'None'); | |
| 682 | + | // Recurring deletes still need the scope picker (which occurrences?). | |
| 683 | + | // Non-recurring deletes are optimistic + undo, so they skip the hard | |
| 684 | + | // confirm (GO-13): the deferred API call means the undo toast is the real | |
| 685 | + | // recovery, and a "cannot be undone" modal would be redundant and false. | |
| 682 | 686 | if (isRecurring) { | |
| 683 | - | // Recurring scope warning replaces the standard confirm dialog. | |
| 684 | 687 | if (!(await confirmRecurringScope(eventRecord, 'delete'))) return; | |
| 685 | - | } else { | |
| 686 | - | if (!await GoingsOn.ui.confirmDelete('event')) return; | |
| 687 | 688 | } | |
| 688 | 689 | ||
| 689 | 690 | GoingsOn.cache.invalidate('events'); |
| @@ -220,7 +220,7 @@ async function cycleGoalStatus(id) { | |||
| 220 | 220 | * @param {string} id - Goal ID | |
| 221 | 221 | */ | |
| 222 | 222 | async function deleteGoal(id) { | |
| 223 | - | const confirmed = await GoingsOn.ui.confirmDelete('Delete this goal?'); | |
| 223 | + | const confirmed = await GoingsOn.ui.confirmDelete('goal'); | |
| 224 | 224 | if (!confirmed) return; | |
| 225 | 225 | ||
| 226 | 226 | await GoingsOn.ui.apiCall( |
| @@ -198,28 +198,22 @@ | |||
| 198 | 198 | * @param {string} id - Project ID to delete | |
| 199 | 199 | */ | |
| 200 | 200 | async function deleteProject(id) { | |
| 201 | + | // High-destructiveness tier (GO-13): deleting a project cascades to all | |
| 202 | + | // its tasks, so this is a deliberate, irreversible action — a hard confirm | |
| 203 | + | // with an immediate delete and no undo toast. (Tasks/events, which are | |
| 204 | + | // low-destructiveness, use the opposite model: optimistic + undo, no | |
| 205 | + | // confirm.) Keeping the confirm makes "this cannot be undone" honest. | |
| 201 | 206 | if (!await GoingsOn.ui.confirmDelete('project')) return; | |
| 202 | 207 | ||
| 203 | - | GoingsOn.cache.invalidate('projects'); | |
| 204 | - | const cachedProjects = GoingsOn.state.projects; | |
| 205 | - | const removedProject = cachedProjects.find(p => p.id === id); | |
| 206 | - | GoingsOn.state.set('projects', cachedProjects.filter(p => p.id !== id)); | |
| 207 | - | ||
| 208 | - | GoingsOn.ui.showUndoToast('Project deleted', { | |
| 209 | - | onConfirm: async () => { | |
| 210 | - | try { | |
| 211 | - | await GoingsOn.api.projects.delete(id); | |
| 212 | - | } catch (err) { | |
| 213 | - | GoingsOn.ui.showToast('Failed to delete project', 'error'); | |
| 214 | - | load(); | |
| 215 | - | } | |
| 216 | - | }, | |
| 217 | - | onUndo: () => { | |
| 218 | - | if (removedProject) { | |
| 219 | - | GoingsOn.state.set('projects', [...GoingsOn.state.projects, removedProject]); | |
| 220 | - | } | |
| 221 | - | }, | |
| 222 | - | }); | |
| 208 | + | try { | |
| 209 | + | await GoingsOn.api.projects.delete(id); | |
| 210 | + | GoingsOn.cache.invalidate('projects'); | |
| 211 | + | GoingsOn.state.set('projects', GoingsOn.state.projects.filter(p => p.id !== id)); | |
| 212 | + | GoingsOn.ui.showToast('Project deleted', 'success'); | |
| 213 | + | } catch (err) { | |
| 214 | + | GoingsOn.ui.showToast('Failed to delete project', 'error'); | |
| 215 | + | load(); | |
| 216 | + | } | |
| 223 | 217 | } | |
| 224 | 218 | ||
| 225 | 219 | /** |
| @@ -646,8 +646,10 @@ | |||
| 646 | 646 | * @param {string} id - Task ID to delete | |
| 647 | 647 | */ | |
| 648 | 648 | async function deleteTask(id) { | |
| 649 | - | if (!await GoingsOn.ui.confirmDelete('task')) return; | |
| 650 | - | ||
| 649 | + | // No hard confirm (GO-13): this is an optimistic delete whose API call is | |
| 650 | + | // deferred until the undo window expires, so the undo toast IS the | |
| 651 | + | // recovery. A "this cannot be undone" modal here would be both redundant | |
| 652 | + | // and false. Confirms are reserved for immediate, non-undoable deletes. | |
| 651 | 653 | GoingsOn.cache.invalidate('tasks'); | |
| 652 | 654 | // Hide task from UI immediately | |
| 653 | 655 | const cachedTasks = GoingsOn.state.tasks; |
| @@ -321,7 +321,11 @@ | |||
| 321 | 321 | * Remove focus from all tasks after confirmation. | |
| 322 | 322 | */ | |
| 323 | 323 | async function clearAllFocus() { | |
| 324 | - | const confirmed = await GoingsOn.ui.confirmDelete('Clear focus from all tasks?'); | |
| 324 | + | const confirmed = await GoingsOn.ui.showConfirmDialog( | |
| 325 | + | 'Clear focus', | |
| 326 | + | 'Remove focus from all tasks?', | |
| 327 | + | { confirmText: 'Clear', danger: true } | |
| 328 | + | ); | |
| 325 | 329 | if (!confirmed) return; | |
| 326 | 330 | ||
| 327 | 331 | try { |
| @@ -14,6 +14,16 @@ use tracing::{debug, error, info, warn}; | |||
| 14 | 14 | /// Check interval for automated backups (1 minute) | |
| 15 | 15 | const CHECK_INTERVAL_SECS: u64 = 60; | |
| 16 | 16 | ||
| 17 | + | /// Build a unique backup filename. The second-granular timestamp keeps files | |
| 18 | + | /// human-sortable; the short random suffix prevents a manual backup and the | |
| 19 | + | /// scheduler firing in the same second from colliding and silently overwriting | |
| 20 | + | /// each other (GO-11). | |
| 21 | + | fn backup_filename(now: chrono::DateTime<Utc>) -> String { | |
| 22 | + | let stamp = now.format("%Y%m%d-%H%M%S"); | |
| 23 | + | let suffix = &uuid::Uuid::new_v4().simple().to_string()[..8]; | |
| 24 | + | format!("goingson-backup-{stamp}-{suffix}.json.gz") | |
| 25 | + | } | |
| 26 | + | ||
| 17 | 27 | /// Starts the background backup scheduler that creates automatic backups | |
| 18 | 28 | /// based on user settings and prunes old backups. | |
| 19 | 29 | pub async fn start_backup_scheduler(app: tauri::AppHandle, cancel: CancellationToken) { | |
| @@ -105,9 +115,7 @@ async fn check_and_backup(app: &tauri::AppHandle, state: &Arc<AppState>) -> Resu | |||
| 105 | 115 | std::fs::create_dir_all(&backup_dir) | |
| 106 | 116 | .map_err(|e| format!("Failed to create backup directory: {}", e))?; | |
| 107 | 117 | ||
| 108 | - | // Generate timestamped filename | |
| 109 | - | let timestamp = now.format("%Y%m%d-%H%M%S"); | |
| 110 | - | let filename = format!("goingson-backup-{}.json.gz", timestamp); | |
| 118 | + | let filename = backup_filename(now); | |
| 111 | 119 | let file_path = backup_dir.join(&filename); | |
| 112 | 120 | ||
| 113 | 121 | // Fetch all data | |
| @@ -180,7 +188,7 @@ fn prune_old_backups(backup_dir: &std::path::Path, max_to_keep: usize) -> Result | |||
| 180 | 188 | entry | |
| 181 | 189 | .metadata() | |
| 182 | 190 | .ok() | |
| 183 | - | .and_then(|m| m.created().ok()) | |
| 191 | + | .and_then(|m| m.created().or_else(|_| m.modified()).ok()) | |
| 184 | 192 | .map(|created| (entry.path(), created)) | |
| 185 | 193 | }) | |
| 186 | 194 | .collect(); | |
| @@ -215,8 +223,7 @@ pub async fn create_backup_now( | |||
| 215 | 223 | std::fs::create_dir_all(&backup_dir) | |
| 216 | 224 | .map_err(|e| format!("Failed to create backup directory: {}", e))?; | |
| 217 | 225 | ||
| 218 | - | let timestamp = now.format("%Y%m%d-%H%M%S"); | |
| 219 | - | let filename = format!("goingson-backup-{}.json.gz", timestamp); | |
| 226 | + | let filename = backup_filename(now); | |
| 220 | 227 | let file_path = backup_dir.join(&filename); | |
| 221 | 228 | ||
| 222 | 229 | // Fetch all data | |
| @@ -268,3 +275,19 @@ pub async fn create_backup_now( | |||
| 268 | 275 | size_bytes, | |
| 269 | 276 | }) | |
| 270 | 277 | } | |
| 278 | + | ||
| 279 | + | #[cfg(test)] | |
| 280 | + | mod tests { | |
| 281 | + | use super::*; | |
| 282 | + | ||
| 283 | + | #[test] | |
| 284 | + | fn backup_filename_is_unique_within_the_same_second() { | |
| 285 | + | // GO-11: a manual backup and the scheduler firing in the same second | |
| 286 | + | // must not produce the same filename (which would silently overwrite). | |
| 287 | + | let now = Utc::now(); | |
| 288 | + | let a = backup_filename(now); | |
| 289 | + | let b = backup_filename(now); | |
| 290 | + | assert_ne!(a, b, "same-second backups must get distinct filenames"); | |
| 291 | + | assert!(a.starts_with("goingson-backup-") && a.ends_with(".json.gz")); | |
| 292 | + | } | |
| 293 | + | } |
| @@ -3,7 +3,7 @@ | |||
| 3 | 3 | //! Provides functionality for viewing and managing a daily timeline | |
| 4 | 4 | //! of scheduled tasks and events, including conflict detection. | |
| 5 | 5 | ||
| 6 | - | use chrono::{DateTime, Duration, Utc}; | |
| 6 | + | use chrono::{DateTime, Duration, Local, NaiveDate, NaiveDateTime, TimeZone, Utc}; | |
| 7 | 7 | use serde::{Deserialize, Serialize}; | |
| 8 | 8 | use std::sync::Arc; | |
| 9 | 9 | use tauri::State; | |
| @@ -37,6 +37,37 @@ pub struct ScheduleTaskInput { | |||
| 37 | 37 | pub duration: Option<i32>, | |
| 38 | 38 | } | |
| 39 | 39 | ||
| 40 | + | // ============ Helpers ============ | |
| 41 | + | ||
| 42 | + | /// Map a user-local civil date to the half-open UTC instant window `[start, end)` | |
| 43 | + | /// that the date spans in timezone `tz`. | |
| 44 | + | /// | |
| 45 | + | /// The frontend sends `date` as a *local* calendar day (`utils.js` derives it | |
| 46 | + | /// from local `getFullYear/getMonth/getDate`), but events and time sessions are | |
| 47 | + | /// stored as UTC instants. Interpreting the date as UTC midnight misattributes | |
| 48 | + | /// anything near local midnight for any non-UTC user. In production `tz` is | |
| 49 | + | /// `Local` — correct here because this is a single-user desktop app whose | |
| 50 | + | /// process runs on the user's machine (same convention as `event.rs`). | |
| 51 | + | /// | |
| 52 | + | /// Generic over `TimeZone` so tests can pin a fixed offset instead of depending | |
| 53 | + | /// on the host's timezone. The end bound is computed from the *next* local | |
| 54 | + | /// midnight (not `start + 24h`) so it stays correct across DST-length days. On | |
| 55 | + | /// the rare DST spring-forward gap at midnight, falls back to treating the | |
| 56 | + | /// civil time as UTC so the call never panics. | |
| 57 | + | fn local_day_to_utc_window<Tz: TimeZone>(date: NaiveDate, tz: &Tz) -> (DateTime<Utc>, DateTime<Utc>) { | |
| 58 | + | let to_utc = |civil: NaiveDateTime| -> DateTime<Utc> { | |
| 59 | + | tz.from_local_datetime(&civil) | |
| 60 | + | .earliest() | |
| 61 | + | .map(|dt| dt.with_timezone(&Utc)) | |
| 62 | + | .unwrap_or_else(|| DateTime::<Utc>::from_naive_utc_and_offset(civil, Utc)) | |
| 63 | + | }; | |
| 64 | + | let start = date.and_hms_opt(0, 0, 0).expect("00:00:00 is a valid time"); | |
| 65 | + | let end = (date + Duration::days(1)) | |
| 66 | + | .and_hms_opt(0, 0, 0) | |
| 67 | + | .expect("00:00:00 is a valid time"); | |
| 68 | + | (to_utc(start), to_utc(end)) | |
| 69 | + | } | |
| 70 | + | ||
| 40 | 71 | // ============ Commands ============ | |
| 41 | 72 | ||
| 42 | 73 | /// Retrieves the day planning view for a specific date. | |
| @@ -69,14 +100,15 @@ pub async fn get_day_planning( | |||
| 69 | 100 | None => false, | |
| 70 | 101 | }; | |
| 71 | 102 | ||
| 72 | - | // Fetch events for the date + expand recurring events | |
| 73 | - | let day_start = parsed_date.and_hms_opt(0, 0, 0) | |
| 74 | - | .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc)) | |
| 75 | - | .unwrap_or_else(Utc::now); | |
| 76 | - | let day_end = day_start + Duration::days(1) - Duration::seconds(1); | |
| 103 | + | // Resolve the requested local day to a UTC instant window. `day_end` is the | |
| 104 | + | // inclusive (second-granular) upper bound used by the overlap/`due` queries, | |
| 105 | + | // mirroring the prior end-of-day-minus-one-second behavior; `day_end_excl` | |
| 106 | + | // is the half-open bound for `get_time_summary`. | |
| 107 | + | let (day_start, day_end_excl) = local_day_to_utc_window(parsed_date, &Local); | |
| 108 | + | let day_end = day_end_excl - Duration::seconds(1); | |
| 77 | 109 | ||
| 78 | 110 | let (date_events, recurring) = tokio::join!( | |
| 79 | - | state.events.list_for_date(DESKTOP_USER_ID, parsed_date), | |
| 111 | + | state.events.list_between(DESKTOP_USER_ID, day_start, day_end), | |
| 80 | 112 | state.events.list_recurring(DESKTOP_USER_ID), | |
| 81 | 113 | ); | |
| 82 | 114 | let mut events = date_events?; | |
| @@ -98,7 +130,7 @@ pub async fn get_day_planning( | |||
| 98 | 130 | events.sort_by_key(|e| e.start_time); | |
| 99 | 131 | ||
| 100 | 132 | let unscheduled_tasks = state.tasks | |
| 101 | - | .list_unscheduled_due_on_date(DESKTOP_USER_ID, parsed_date) | |
| 133 | + | .list_unscheduled_due_between(DESKTOP_USER_ID, day_start, day_end) | |
| 102 | 134 | .await?; | |
| 103 | 135 | ||
| 104 | 136 | let mut timeline_items: Vec<TimelineItem> = events.iter().map(|event| { | |
| @@ -131,12 +163,11 @@ pub async fn get_day_planning( | |||
| 131 | 163 | ||
| 132 | 164 | let conflicts = detect_conflicts(&timeline_items); | |
| 133 | 165 | ||
| 134 | - | // Calculate time tracked for the requested date | |
| 135 | - | let day_start = parsed_date.and_hms_opt(0, 0, 0).expect("midnight is valid"); | |
| 136 | - | let day_end = parsed_date.succ_opt().unwrap_or(parsed_date).and_hms_opt(0, 0, 0).expect("midnight is valid"); | |
| 137 | - | let day_start_utc = chrono::DateTime::<Utc>::from_naive_utc_and_offset(day_start, Utc); | |
| 138 | - | let day_end_utc = chrono::DateTime::<Utc>::from_naive_utc_and_offset(day_end, Utc); | |
| 139 | - | let summaries = state.tasks.get_time_summary(DESKTOP_USER_ID, day_start_utc, day_end_utc).await?; | |
| 166 | + | // Time tracked for the requested local day. `get_time_summary` filters on a | |
| 167 | + | // half-open `[start, end)` window, so use the exclusive upper bound. | |
| 168 | + | let summaries = state.tasks | |
| 169 | + | .get_time_summary(DESKTOP_USER_ID, day_start, day_end_excl) | |
| 170 | + | .await?; | |
| 140 | 171 | let time_tracked_today: i32 = summaries.iter().map(|s| s.total_minutes).sum(); | |
| 141 | 172 | ||
| 142 | 173 | Ok(DayPlanningResponse { | |
| @@ -189,7 +220,15 @@ pub async fn schedule_task( | |||
| 189 | 220 | .get_by_linked_task(DESKTOP_USER_ID, id) | |
| 190 | 221 | .await?; | |
| 191 | 222 | ||
| 192 | - | if let Some(existing) = existing_event { | |
| 223 | + | // Snapshot the prior schedule so a failed linked-event write can be rolled | |
| 224 | + | // back — otherwise the task is left scheduled with no calendar event (GO-10). | |
| 225 | + | // This is a compensating undo, not a DB transaction (tasks and events are | |
| 226 | + | // separate repos): it covers every in-process failure, but a crash between | |
| 227 | + | // the two writes can still leave them inconsistent. | |
| 228 | + | let prior_start = task.scheduled_start; | |
| 229 | + | let prior_duration = task.scheduled_duration; | |
| 230 | + | ||
| 231 | + | let event_result = if let Some(existing) = existing_event { | |
| 193 | 232 | let update_event = UpdateEvent { | |
| 194 | 233 | project_id: task.project_id, | |
| 195 | 234 | title: task.description.clone(), | |
| @@ -206,7 +245,8 @@ pub async fn schedule_task( | |||
| 206 | 245 | }; | |
| 207 | 246 | state.events | |
| 208 | 247 | .update(existing.id, DESKTOP_USER_ID, update_event) | |
| 209 | - | .await?; | |
| 248 | + | .await | |
| 249 | + | .map(|_| ()) | |
| 210 | 250 | } else { | |
| 211 | 251 | let new_event = NewEvent { | |
| 212 | 252 | user_id: Some(DESKTOP_USER_ID), | |
| @@ -225,7 +265,17 @@ pub async fn schedule_task( | |||
| 225 | 265 | }; | |
| 226 | 266 | state.events | |
| 227 | 267 | .create(DESKTOP_USER_ID, new_event) | |
| 228 | - | .await?; | |
| 268 | + | .await | |
| 269 | + | .map(|_| ()) | |
| 270 | + | }; | |
| 271 | + | ||
| 272 | + | if let Err(e) = event_result { | |
| 273 | + | // Best-effort restore of the previous schedule; surface the original | |
| 274 | + | // event-write error regardless of whether the undo itself succeeds. | |
| 275 | + | let _ = state.tasks | |
| 276 | + | .update_schedule(id, DESKTOP_USER_ID, prior_start, prior_duration) | |
| 277 | + | .await; | |
| 278 | + | return Err(e.into()); | |
| 229 | 279 | } | |
| 230 | 280 | ||
| 231 | 281 | Ok(TaskResponse::from(updated_task)) | |
| @@ -257,3 +307,60 @@ pub async fn unschedule_task( | |||
| 257 | 307 | } | |
| 258 | 308 | ||
| 259 | 309 | // Tests for detect_conflicts live in crates/core/src/day_planning.rs | |
| 310 | + | ||
| 311 | + | #[cfg(test)] | |
| 312 | + | mod tests { | |
| 313 | + | use super::*; | |
| 314 | + | use chrono::FixedOffset; | |
| 315 | + | ||
| 316 | + | fn utc(s: &str) -> DateTime<Utc> { | |
| 317 | + | s.parse::<DateTime<Utc>>().expect("valid RFC3339 instant") | |
| 318 | + | } | |
| 319 | + | ||
| 320 | + | fn ymd(y: i32, m: u32, d: u32) -> NaiveDate { | |
| 321 | + | NaiveDate::from_ymd_opt(y, m, d).expect("valid date") | |
| 322 | + | } | |
| 323 | + | ||
| 324 | + | // UTC-5 (US Eastern, standard): a local civil day maps to a window shifted | |
| 325 | + | // +5h into UTC. | |
| 326 | + | #[test] | |
| 327 | + | fn local_day_window_offsets_into_utc() { | |
| 328 | + | let est = FixedOffset::west_opt(5 * 3600).unwrap(); | |
| 329 | + | let (start, end) = local_day_to_utc_window(ymd(2026, 6, 11), &est); | |
| 330 | + | assert_eq!(start, utc("2026-06-11T05:00:00Z")); | |
| 331 | + | assert_eq!(end, utc("2026-06-12T05:00:00Z")); | |
| 332 | + | } | |
| 333 | + | ||
| 334 | + | // The GO-6 case: an instant stored at 02:00Z on the 12th is 21:00 local on | |
| 335 | + | // the 11th in UTC-5, so it must land in the 11th's window, not the 12th's. | |
| 336 | + | #[test] | |
| 337 | + | fn instant_near_local_midnight_lands_on_correct_local_day() { | |
| 338 | + | let est = FixedOffset::west_opt(5 * 3600).unwrap(); | |
| 339 | + | let instant = utc("2026-06-12T02:00:00Z"); | |
| 340 | + | ||
| 341 | + | let (s11, e11) = local_day_to_utc_window(ymd(2026, 6, 11), &est); | |
| 342 | + | let (s12, e12) = local_day_to_utc_window(ymd(2026, 6, 12), &est); | |
| 343 | + | ||
| 344 | + | assert!(instant >= s11 && instant < e11, "belongs to the 11th's local day"); | |
| 345 | + | assert!(!(instant >= s12 && instant < e12), "must not be the 12th's local day"); | |
| 346 | + | } | |
| 347 | + | ||
| 348 | + | // UTC users see no shift (regression guard for the common case). | |
| 349 | + | #[test] | |
| 350 | + | fn utc_day_window_is_plain_midnight() { | |
| 351 | + | let utc_tz = FixedOffset::east_opt(0).unwrap(); | |
| 352 | + | let (start, end) = local_day_to_utc_window(ymd(2026, 6, 11), &utc_tz); | |
| 353 | + | assert_eq!(start, utc("2026-06-11T00:00:00Z")); | |
| 354 | + | assert_eq!(end, utc("2026-06-12T00:00:00Z")); | |
| 355 | + | } | |
| 356 | + | ||
| 357 | + | // Positive offset (UTC+9, Tokyo): the window shifts the other way, into the | |
| 358 | + | // prior UTC day. | |
| 359 | + | #[test] | |
| 360 | + | fn east_of_utc_shifts_window_back() { | |
| 361 | + | let jst = FixedOffset::east_opt(9 * 3600).unwrap(); | |
| 362 | + | let (start, end) = local_day_to_utc_window(ymd(2026, 6, 11), &jst); | |
| 363 | + | assert_eq!(start, utc("2026-06-10T15:00:00Z")); | |
| 364 | + | assert_eq!(end, utc("2026-06-11T15:00:00Z")); | |
| 365 | + | } | |
| 366 | + | } |
| @@ -286,14 +286,33 @@ async fn sync_jmap_account_inner( | |||
| 286 | 286 | if account.needs_token_refresh() { | |
| 287 | 287 | let refresh_lock = state.token_refresh_lock(raw_id); | |
| 288 | 288 | let _guard = refresh_lock.lock().await; | |
| 289 | - | // Re-check after acquiring lock — another task may have refreshed already | |
| 289 | + | // Re-read the account after acquiring the lock (GO-8): a prior waiter may | |
| 290 | + | // have refreshed and persisted a new token while we waited, leaving the | |
| 291 | + | // in-memory `account` and its expiry stale. Passing the fresh copy lets | |
| 292 | + | // refresh_if_needed correctly no-op instead of firing a redundant refresh. | |
| 293 | + | let fresh = state.email_accounts | |
| 294 | + | .get_by_id(id, DESKTOP_USER_ID) | |
| 295 | + | .await? | |
| 296 | + | .or_api_err(|| ApiError::auth("Email account no longer exists"))?; | |
| 290 | 297 | let token_manager = TokenManager::from_env(); | |
| 291 | - | if let Ok(Some((new_token, new_refresh, expires_at))) = token_manager.refresh_if_needed(account).await { | |
| 292 | - | state.email_accounts | |
| 293 | - | .update_oauth_tokens(id, DESKTOP_USER_ID, &new_token, new_refresh.as_deref(), expires_at) | |
| 294 | - | .await?; | |
| 295 | - | let _ = CredentialStore::update_oauth_tokens(raw_id, &new_token, new_refresh.as_deref()); | |
| 296 | - | access_token = new_token; | |
| 298 | + | match token_manager.refresh_if_needed(&fresh).await { | |
| 299 | + | Ok(Some((new_token, new_refresh, expires_at))) => { | |
| 300 | + | state.email_accounts | |
| 301 | + | .update_oauth_tokens(id, DESKTOP_USER_ID, &new_token, new_refresh.as_deref(), expires_at) | |
| 302 | + | .await?; | |
| 303 | + | let _ = CredentialStore::update_oauth_tokens(raw_id, &new_token, new_refresh.as_deref()); | |
| 304 | + | access_token = new_token; | |
| 305 | + | } | |
| 306 | + | // Already fresh (a prior waiter refreshed): adopt the latest token, | |
| 307 | + | // keychain first since refreshes write there before the DB. | |
| 308 | + | Ok(None) => { | |
| 309 | + | if let Some(c) = CredentialStore::get_oauth(raw_id) { | |
| 310 | + | access_token = c.access_token; | |
| 311 | + | } else if let Some(tok) = fresh.oauth2_access_token.clone() { | |
| 312 | + | access_token = tok; | |
| 313 | + | } | |
| 314 | + | } | |
| 315 | + | Err(_) => {} | |
| 297 | 316 | } | |
| 298 | 317 | } | |
| 299 | 318 |
| @@ -523,8 +523,12 @@ pub async fn save_backup_settings( | |||
| 523 | 523 | ) -> Result<BackupSettingsResponse, ApiError> { | |
| 524 | 524 | let settings = goingson_core::NewBackupSettings { | |
| 525 | 525 | auto_backup_enabled: input.auto_backup_enabled, | |
| 526 | - | backup_frequency_minutes: input.backup_frequency_minutes, | |
| 527 | - | max_backups_to_keep: input.max_backups_to_keep, | |
| 526 | + | // Clamp nonsensical values (GO-9): a non-positive frequency would back up | |
| 527 | + | // on every scheduler tick (~60s), and a negative retention casts to a | |
| 528 | + | // huge usize in prune_old_backups so nothing is ever pruned. 0 retention | |
| 529 | + | // is allowed and means "keep all". | |
| 530 | + | backup_frequency_minutes: input.backup_frequency_minutes.max(1), | |
| 531 | + | max_backups_to_keep: input.max_backups_to_keep.max(0), | |
| 528 | 532 | }; | |
| 529 | 533 | ||
| 530 | 534 | let saved = state |
| @@ -118,6 +118,9 @@ pub async fn log_manual_time( | |||
| 118 | 118 | if input.minutes < 1 { | |
| 119 | 119 | return Err(ApiError::validation("minutes", "Minutes must be at least 1")); | |
| 120 | 120 | } | |
| 121 | + | // Ownership of `task_id` is verified inside the repo method (it must precede | |
| 122 | + | // the session INSERT to avoid an orphan row); a foreign/bogus id surfaces as | |
| 123 | + | // a NOT_FOUND error here. | |
| 121 | 124 | Ok(state.tasks.log_manual_time(input.task_id, DESKTOP_USER_ID, input.minutes, input.date).await?) | |
| 122 | 125 | } | |
| 123 | 126 |