From 5b660c3c1ca0a0ac5049d2f20628c15e3e8853cb Mon Sep 17 00:00:00 2001 From: Joakim Date: Tue, 6 Jan 2026 14:54:01 +0100 Subject: [PATCH] Fix working set IDs to match display order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: BuildWorkingSet() now accepts []*Task instead of *Filter Problem: - Working set IDs were assigned based on database query order (unsorted) - Reports displayed tasks in sorted order (by urgency) - Result: IDs didn't match displayed task positions (ID 1 wasn't first task) Solution: - Changed BuildWorkingSet() to accept pre-sorted task slice - Reports now pass sorted tasks to BuildWorkingSet() - IDs are assigned sequentially to match display order (1, 2, 3...) Behavior: - Reports rebuild working set on every execution with fresh IDs - Task operations (done, modify, info) use saved working set IDs - After completing a task, re-running report renumbers remaining tasks Example: Before: opal list shows ID 1 = low urgency task (wrong) After: opal list shows ID 1 = highest urgency task (correct) Tested scenarios: ✓ List report: IDs 1-N match urgency order ✓ Next report: IDs 1-5 match top urgent tasks ✓ Task completion: IDs renumber correctly after removal ✓ Multiple operations: Use saved working set (correct behavior) ✓ Different reports: Each builds own sequential IDs --- opal-task/cmd/modify.go | 8 ++++++-- opal-task/cmd/reports.go | 6 +++--- opal-task/internal/engine/ws.go | 11 ++++------- opal-task/internal/engine/ws_test.go | 29 ++++++++++++++++++++++------ 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/opal-task/cmd/modify.go b/opal-task/cmd/modify.go index 36b380c..c001f95 100644 --- a/opal-task/cmd/modify.go +++ b/opal-task/cmd/modify.go @@ -50,8 +50,12 @@ func modifyTasks(filterArgs, modifierArgs []string) error { // Load working set for ID resolution ws, err := engine.LoadWorkingSet() if err != nil { - // If no working set exists yet, build one - ws, err = engine.BuildWorkingSet(filter) + // If no working set exists yet, build one from filtered tasks + tasks, err := engine.GetTasks(filter) + if err != nil { + return fmt.Errorf("failed to get tasks: %w", err) + } + ws, err = engine.BuildWorkingSet(tasks) if err != nil { return fmt.Errorf("failed to build working set: %w", err) } diff --git a/opal-task/cmd/reports.go b/opal-task/cmd/reports.go index 3c4007b..e927a2d 100644 --- a/opal-task/cmd/reports.go +++ b/opal-task/cmd/reports.go @@ -47,14 +47,14 @@ func runReport(reportName string, filters []string) error { return err } - // Execute the report + // Execute the report (returns sorted tasks) tasks, err := report.Execute(filters) if err != nil { return fmt.Errorf("failed to execute report: %w", err) } - // Build working set for display IDs - ws, err := engine.BuildWorkingSet(report.BaseFilter) + // Build working set from sorted tasks - IDs will match display order + ws, err := engine.BuildWorkingSet(tasks) if err != nil { return fmt.Errorf("failed to build working set: %w", err) } diff --git a/opal-task/internal/engine/ws.go b/opal-task/internal/engine/ws.go index 447514c..20a25a8 100644 --- a/opal-task/internal/engine/ws.go +++ b/opal-task/internal/engine/ws.go @@ -14,13 +14,10 @@ type WorkingSet struct { byID map[int]uuid.UUID // display_id -> UUID } -// BuildWorkingSet creates a working set from filter results and persists to DB -func BuildWorkingSet(filter *Filter) (*WorkingSet, error) { - tasks, err := GetTasks(filter) - if err != nil { - return nil, err - } - +// BuildWorkingSet creates a working set from a list of tasks and persists to DB +// Tasks should be pre-sorted in the desired display order +// IDs are assigned sequentially (1, 2, 3, ...) based on task order in the slice +func BuildWorkingSet(tasks []*Task) (*WorkingSet, error) { ws := &WorkingSet{ byUUID: make(map[uuid.UUID]*Task), byID: make(map[int]uuid.UUID), diff --git a/opal-task/internal/engine/ws_test.go b/opal-task/internal/engine/ws_test.go index 4173b50..5000270 100644 --- a/opal-task/internal/engine/ws_test.go +++ b/opal-task/internal/engine/ws_test.go @@ -10,8 +10,13 @@ func TestBuildWorkingSet(t *testing.T) { task2, _ := CreateTask("Task 2") task3, _ := CreateTask("Task 3") - // Build working set with default filter - ws, err := BuildWorkingSet(DefaultFilter()) + // Get tasks and build working set + tasks, err := GetTasks(DefaultFilter()) + if err != nil { + t.Fatalf("Failed to get tasks: %v", err) + } + + ws, err := BuildWorkingSet(tasks) if err != nil { t.Fatalf("Failed to build working set: %v", err) } @@ -52,7 +57,12 @@ func TestLoadWorkingSet(t *testing.T) { CreateTask("Load test 1") CreateTask("Load test 2") - ws1, err := BuildWorkingSet(DefaultFilter()) + tasks, err := GetTasks(DefaultFilter()) + if err != nil { + t.Fatalf("Failed to get tasks: %v", err) + } + + ws1, err := BuildWorkingSet(tasks) if err != nil { t.Fatalf("Failed to build working set: %v", err) } @@ -82,7 +92,12 @@ func TestWorkingSetWithFilter(t *testing.T) { // Build working set with high priority filter filter, _ := ParseFilter([]string{"priority:H"}) - ws, err := BuildWorkingSet(filter) + tasks, err := GetTasks(filter) + if err != nil { + t.Fatalf("Failed to get tasks: %v", err) + } + + ws, err := BuildWorkingSet(tasks) if err != nil { t.Fatalf("Failed to build filtered working set: %v", err) } @@ -105,7 +120,8 @@ func TestGetTaskByDisplayID(t *testing.T) { CreateTask("Display ID test 2") CreateTask("Display ID test 3") - ws, _ := BuildWorkingSet(DefaultFilter()) + tasks, _ := GetTasks(DefaultFilter()) + ws, _ := BuildWorkingSet(tasks) // Test valid display IDs for i := 1; i <= ws.Size(); i++ { @@ -130,7 +146,8 @@ func TestWorkingSetGetTasks(t *testing.T) { CreateTask("GetTasks test 1") CreateTask("GetTasks test 2") - ws, _ := BuildWorkingSet(DefaultFilter()) + taskList, _ := GetTasks(DefaultFilter()) + ws, _ := BuildWorkingSet(taskList) tasks := ws.GetTasks() if len(tasks) != ws.Size() {