diff options
| -rw-r--r-- | internal/store/native_tasks.go | 20 | ||||
| -rw-r--r-- | internal/store/native_tasks_test.go | 82 | ||||
| -rw-r--r-- | internal/store/sqlite_test.go | 3 | ||||
| -rw-r--r-- | migrations/030_abandon_stale_recurrences.sql | 23 |
4 files changed, 122 insertions, 6 deletions
diff --git a/internal/store/native_tasks.go b/internal/store/native_tasks.go index ea5212c..955b7ac 100644 --- a/internal/store/native_tasks.go +++ b/internal/store/native_tasks.go @@ -43,7 +43,7 @@ func (s *Store) GetNativeTasks() ([]models.Task, error) { chain_id, chain_position, chain_unlocked, bucket_id, bucket_state, bucket_last_active_at FROM native_tasks - WHERE completed = 0 + WHERE completed = 0 AND abandoned = 0 ORDER BY CASE WHEN due_date IS NULL THEN 1 ELSE 0 END, due_date ASC, priority DESC `) if err != nil { @@ -64,7 +64,7 @@ func (s *Store) GetNativeTasksByDateRange(start, end time.Time) ([]models.Task, chain_id, chain_position, chain_unlocked, bucket_id, bucket_state, bucket_last_active_at FROM native_tasks t - WHERE completed = 0 AND due_date IS NOT NULL AND due_date >= ? AND due_date < ? + WHERE completed = 0 AND abandoned = 0 AND due_date IS NOT NULL AND due_date >= ? AND due_date < ? AND `+visibleChainTaskClause+` ORDER BY due_date ASC, priority DESC `, start, end) @@ -88,7 +88,7 @@ func (s *Store) GetOverdueNativeTasks(before time.Time) ([]models.Task, error) { chain_id, chain_position, chain_unlocked, bucket_id, bucket_state, bucket_last_active_at FROM native_tasks t - WHERE completed = 0 AND due_date IS NOT NULL AND due_date < ? + WHERE completed = 0 AND abandoned = 0 AND due_date IS NOT NULL AND due_date < ? AND `+visibleChainTaskClause+` ORDER BY due_date ASC, priority DESC `, before) @@ -107,7 +107,7 @@ func (s *Store) GetUndatedNativeTasks() ([]models.Task, error) { chain_id, chain_position, chain_unlocked, bucket_id, bucket_state, bucket_last_active_at FROM native_tasks t - WHERE completed = 0 AND due_date IS NULL + WHERE completed = 0 AND abandoned = 0 AND due_date IS NULL AND `+visibleChainTaskClause+` AND bucket_state != 'dormant' ORDER BY priority DESC, created_at ASC @@ -493,6 +493,18 @@ func (s *Store) AdvanceDueRecurringTasks(now time.Time) (int, error) { if err := s.CreateNextIteration(task); err != nil { return 0, err } + // The occurrence being superseded was never done, and now its + // replacement is here -- it's stale, not still owed. Left open it would + // sit in the list as a permanent overdue item and every skipped + // occurrence would add another. Not marked `completed`, since that would + // falsely credit work that never happened. + if !task.Completed { + if _, err := s.db.Exec(` + UPDATE native_tasks SET abandoned = 1, updated_at = ? WHERE id = ? + `, config.Now(), task.ID); err != nil { + return 0, err + } + } } return len(series), nil } diff --git a/internal/store/native_tasks_test.go b/internal/store/native_tasks_test.go index 9a7e55f..9453c86 100644 --- a/internal/store/native_tasks_test.go +++ b/internal/store/native_tasks_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "task-dashboard/internal/config" "task-dashboard/internal/models" _ "github.com/mattn/go-sqlite3" @@ -48,7 +49,8 @@ func newNativeTasksTestStore(t *testing.T) *Store { chain_unlocked BOOLEAN DEFAULT 0, bucket_id TEXT DEFAULT '', bucket_state TEXT DEFAULT '', - bucket_last_active_at DATETIME + bucket_last_active_at DATETIME, + abandoned BOOLEAN DEFAULT 0 ) `); err != nil { t.Fatal(err) @@ -718,3 +720,81 @@ func TestDeleteNativeTask_UnknownID_ReturnsErrNotFound(t *testing.T) { t.Errorf("err = %v, want ErrNativeTaskNotFound", err) } } + +// A weekly task not done before its next iteration lands is stale, not still +// owed -- the successor supersedes it. Before this, the skipped instance stayed +// open forever and showed up as a permanent overdue item, so ignoring one +// occurrence cost a list slot indefinitely. +func TestAdvanceDueRecurringTasks_AbandonsSupersededInstance(t *testing.T) { + s := newNativeTasksTestStore(t) + + due := config.Now().Add(-24 * time.Hour) + task := models.Task{ + ID: "weekly-1", Content: "Take out trash", DueDate: &due, + RecurrenceFreq: "weekly", RecurrenceInterval: 1, + } + if err := s.CreateNativeTask(task); err != nil { + t.Fatal(err) + } + if err := s.SetTaskRecurrence("weekly-1", "weekly", 1, nil); err != nil { + t.Fatal(err) + } + + if _, err := s.AdvanceDueRecurringTasks(config.Now()); err != nil { + t.Fatalf("AdvanceDueRecurringTasks: %v", err) + } + + // The superseded original must no longer be an open, actionable task. + overdue, err := s.GetOverdueNativeTasks(config.Now()) + if err != nil { + t.Fatal(err) + } + for _, o := range overdue { + if o.ID == "weekly-1" { + t.Error("superseded instance weekly-1 is still showing as overdue; it should have been abandoned when its successor was created") + } + } + + // ...and the series should be left with exactly one live occurrence: the + // successor. Scoped to this series because the test store seeds an + // unrelated task of its own. + var seriesID string + if err := s.db.QueryRow(`SELECT recurrence_series_id FROM native_tasks WHERE id = 'weekly-1'`).Scan(&seriesID); err != nil { + t.Fatal(err) + } + var open int + if err := s.db.QueryRow(` + SELECT COUNT(*) FROM native_tasks + WHERE recurrence_series_id = ? AND completed = 0 AND abandoned = 0 + `, seriesID).Scan(&open); err != nil { + t.Fatal(err) + } + if open != 1 { + t.Errorf("live occurrences in series = %d, want exactly 1 (the successor); a skipped occurrence must not keep its slot", open) + } +} + +// Completing normally must not mark anything abandoned -- only the scheduler's +// supersede path does that. +func TestCompleteNativeTask_DoesNotAbandonAnything(t *testing.T) { + s := newNativeTasksTestStore(t) + + due := config.Now() + if err := s.CreateNativeTask(models.Task{ID: "w1", Content: "Chore", DueDate: &due}); err != nil { + t.Fatal(err) + } + if err := s.SetTaskRecurrence("w1", "weekly", 1, nil); err != nil { + t.Fatal(err) + } + if err := s.CompleteNativeTask("w1"); err != nil { + t.Fatal(err) + } + + var abandoned int + if err := s.db.QueryRow(`SELECT COUNT(*) FROM native_tasks WHERE abandoned = 1`).Scan(&abandoned); err != nil { + t.Fatal(err) + } + if abandoned != 0 { + t.Errorf("abandoned = %d, want 0 -- a completed occurrence was done, not skipped", abandoned) + } +} diff --git a/internal/store/sqlite_test.go b/internal/store/sqlite_test.go index 95996b3..2110423 100644 --- a/internal/store/sqlite_test.go +++ b/internal/store/sqlite_test.go @@ -191,7 +191,8 @@ func setupTestStoreWithNativeTasks(t *testing.T) *Store { chain_unlocked BOOLEAN DEFAULT 0, bucket_id TEXT DEFAULT '', bucket_state TEXT DEFAULT '', - bucket_last_active_at DATETIME + bucket_last_active_at DATETIME, + abandoned BOOLEAN DEFAULT 0 ); -- The task queries join against this to check the owning chain's status -- (see visibleChainTaskClause), so it has to exist even for tests that diff --git a/migrations/030_abandon_stale_recurrences.sql b/migrations/030_abandon_stale_recurrences.sql new file mode 100644 index 0000000..f8ec225 --- /dev/null +++ b/migrations/030_abandon_stale_recurrences.sql @@ -0,0 +1,23 @@ +-- A recurring task that wasn't done before its next iteration arrived is stale, +-- not still-owed: the next one supersedes it. Without a way to record that, every +-- skipped occurrence stayed open forever and piled up as permanent overdue items +-- (found live: "Bills" and "Bones's meds" each sitting in the list twice, an +-- unfinished instance alongside its already-generated successor). +-- +-- Deliberately a separate flag rather than reusing `completed`: these were never +-- done, and marking them completed would falsely credit them and pollute +-- completion history. +ALTER TABLE native_tasks ADD COLUMN abandoned BOOLEAN DEFAULT 0; + +-- Backfill: any uncompleted recurring instance that already has a later sibling +-- in its series was superseded before this flag existed. +UPDATE native_tasks +SET abandoned = 1 +WHERE completed = 0 + AND recurrence_series_id != '' + AND due_date IS NOT NULL + AND EXISTS ( + SELECT 1 FROM native_tasks later + WHERE later.recurrence_series_id = native_tasks.recurrence_series_id + AND later.due_date > native_tasks.due_date + ); |