From cb36d8f78ce688db6f74cac57e7c10cd334e5f6e Mon Sep 17 00:00:00 2001 From: Paul Buetow Date: Thu, 25 Jun 2026 17:54:48 +0300 Subject: Fix rq0 detail identity by UUID --- internal/ui/detail_handlers.go | 2 +- internal/ui/keyactions.go | 4 +- internal/ui/keyhandlers.go | 4 +- internal/ui/table.go | 19 ++++--- internal/ui/table_test.go | 122 +++++++++++++++++++++++++++++++++++++---- internal/ui/taskdetail.go | 50 +++++++++++++++-- 6 files changed, 173 insertions(+), 28 deletions(-) (limited to 'internal') diff --git a/internal/ui/detail_handlers.go b/internal/ui/detail_handlers.go index a53749d..d61f29d 100644 --- a/internal/ui/detail_handlers.go +++ b/internal/ui/detail_handlers.go @@ -145,7 +145,7 @@ func (m *Model) handleDetailUndo() (tea.Model, tea.Cmd) { // done, undo). func (m *Model) closeDetailView() { m.showTaskDetail = false - m.currentTaskDetailID = 0 + m.clearCurrentTaskDetail() m.detailSearching = false m.detailSearchRegex = nil m.detailSearchInput.SetValue("") diff --git a/internal/ui/keyactions.go b/internal/ui/keyactions.go index 9c80585..57a9c2e 100644 --- a/internal/ui/keyactions.go +++ b/internal/ui/keyactions.go @@ -689,9 +689,9 @@ func (m *Model) handleShowTaskDetail() (tea.Model, tea.Cmd) { return m, nil } - if m.taskByID(id) != nil { + if t := m.taskByID(id); t != nil { m.showTaskDetail = true - m.currentTaskDetailID = id + m.setCurrentTaskDetail(t) m.detailSearching = false m.detailSearchRegex = nil m.detailFieldIndex = 0 diff --git a/internal/ui/keyhandlers.go b/internal/ui/keyhandlers.go index 116f850..690db05 100644 --- a/internal/ui/keyhandlers.go +++ b/internal/ui/keyhandlers.go @@ -274,7 +274,7 @@ func (m *Model) handleQuitKey() (tea.Model, tea.Cmd) { } if m.showTaskDetail { m.showTaskDetail = false - m.currentTaskDetailID = 0 + m.clearCurrentTaskDetail() m.detailSearching = false m.detailSearchRegex = nil m.detailSearchInput.SetValue("") @@ -313,7 +313,7 @@ func (m *Model) handleEscapeKey() (tea.Model, tea.Cmd) { } if m.showTaskDetail { m.showTaskDetail = false - m.currentTaskDetailID = 0 + m.clearCurrentTaskDetail() m.detailSearching = false m.detailSearchRegex = nil m.detailSearchInput.SetValue("") diff --git a/internal/ui/table.go b/internal/ui/table.go index 69e8dff..29ec388 100644 --- a/internal/ui/table.go +++ b/internal/ui/table.go @@ -82,15 +82,16 @@ type searchState struct { // Blink fields here are separate from blinkState because they drive a // per-field highlight inside the detail view rather than a table row. type detailViewState struct { - showTaskDetail bool - currentTaskDetailID int - detailSearching bool - detailSearchInput textinput.Model - detailSearchRegex *regexp.Regexp - detailFieldIndex int // currently selected field (-1 = none) - detailBlinkField int // field currently blinking (-1 = none) - detailBlinkOn bool // whether the blink is currently on - detailBlinkCount int // number of blink cycles completed so far + showTaskDetail bool + currentTaskDetailUUID string + currentTaskDetailFallbackID int + detailSearching bool + detailSearchInput textinput.Model + detailSearchRegex *regexp.Regexp + detailFieldIndex int // currently selected field (-1 = none) + detailBlinkField int // field currently blinking (-1 = none) + detailBlinkOn bool // whether the blink is currently on + detailBlinkCount int // number of blink cycles completed so far } // ultraState holds the state for the ultra mode task list and its search UI. diff --git a/internal/ui/table_test.go b/internal/ui/table_test.go index e30e6d7..74a5f57 100644 --- a/internal/ui/table_test.go +++ b/internal/ui/table_test.go @@ -209,7 +209,7 @@ func TestNewWithTaskwarriorRejectsNilClient(t *testing.T) { } } -func TestHandleShowTaskDetailTracksTaskID(t *testing.T) { +func TestHandleShowTaskDetailTracksTaskUUID(t *testing.T) { fake := &fakeTaskwarrior{ tasks: []task.Task{ {ID: 1, UUID: "fake-1", Description: "original detail", Status: "pending"}, @@ -226,14 +226,42 @@ func TestHandleShowTaskDetailTracksTaskID(t *testing.T) { if !m.showTaskDetail { t.Fatalf("detail view was not shown") } - if m.currentTaskDetailID != 1 { - t.Fatalf("current detail task ID = %d, want 1", m.currentTaskDetailID) + if m.currentTaskDetailUUID != "fake-1" { + t.Fatalf("current detail task UUID = %q, want fake-1", m.currentTaskDetailUUID) + } + if m.currentTaskDetailFallbackID != 0 { + t.Fatalf("current detail fallback ID = %d, want 0", m.currentTaskDetailFallbackID) } if got := m.renderTaskDetail(); !strings.Contains(got, "original detail") { t.Fatalf("rendered detail %q does not include original task description", got) } } +func TestHandleShowTaskDetailFallsBackToTaskIDWhenUUIDMissing(t *testing.T) { + fake := &fakeTaskwarrior{ + tasks: []task.Task{ + {ID: 1, Description: "legacy detail", Status: "pending"}, + }, + } + m, err := NewWithTaskwarrior(nil, "firefox", fake) + if err != nil { + t.Fatalf("NewWithTaskwarrior: %v", err) + } + + mv, _ := (&m).handleShowTaskDetail() + m = *mv.(*Model) + + if m.currentTaskDetailUUID != "" { + t.Fatalf("current detail task UUID = %q, want empty fallback", m.currentTaskDetailUUID) + } + if m.currentTaskDetailFallbackID != 1 { + t.Fatalf("current detail fallback ID = %d, want 1", m.currentTaskDetailFallbackID) + } + if got := m.renderTaskDetail(); !strings.Contains(got, "legacy detail") { + t.Fatalf("rendered detail %q does not include legacy task description", got) + } +} + func TestTaskDetailUsesReplacedTaskSlice(t *testing.T) { fake := &fakeTaskwarrior{ tasks: []task.Task{ @@ -260,11 +288,82 @@ func TestTaskDetailUsesReplacedTaskSlice(t *testing.T) { } } +func TestTaskDetailRejectsSameNumericIDDifferentUUIDAfterReload(t *testing.T) { + fake := &fakeTaskwarrior{ + tasks: []task.Task{ + {ID: 1, UUID: "fake-1", Description: "original detail", Status: "pending"}, + }, + } + m, err := NewWithTaskwarrior(nil, "firefox", fake) + if err != nil { + t.Fatalf("NewWithTaskwarrior: %v", err) + } + + mv, _ := (&m).handleShowTaskDetail() + m = *mv.(*Model) + m.tasks = []task.Task{ + {ID: 1, UUID: "fake-2", Description: "wrong detail", Status: "pending"}, + } + + m.refreshCurrentTaskDetail() + + if m.showTaskDetail { + t.Fatalf("detail view stayed open for different UUID with same numeric ID") + } + if m.currentTaskDetailUUID != "" { + t.Fatalf("current detail task UUID = %q, want cleared", m.currentTaskDetailUUID) + } + if m.currentTaskDetailFallbackID != 0 { + t.Fatalf("current detail fallback ID = %d, want 0", m.currentTaskDetailFallbackID) + } + if got := m.renderTaskDetail(); got != "No task selected" { + t.Fatalf("rendered detail = %q, want no task selected", got) + } +} + +func TestTaskDetailUsesSameUUIDAfterNumericIDChanges(t *testing.T) { + fake := &fakeTaskwarrior{ + tasks: []task.Task{ + {ID: 1, UUID: "fake-1", Description: "old detail", Status: "pending"}, + }, + } + m, err := NewWithTaskwarrior(nil, "firefox", fake) + if err != nil { + t.Fatalf("NewWithTaskwarrior: %v", err) + } + + mv, _ := (&m).handleShowTaskDetail() + m = *mv.(*Model) + m.tasks = []task.Task{ + {ID: 7, UUID: "fake-1", Description: "renumbered detail", Status: "pending"}, + } + + m.refreshCurrentTaskDetail() + + if !m.showTaskDetail { + t.Fatalf("detail view closed for same UUID with new numeric ID") + } + current := m.currentDetailTask() + if current == nil { + t.Fatalf("current detail task was nil") + } + if current.ID != 7 { + t.Fatalf("current detail task ID = %d, want 7", current.ID) + } + got := m.renderTaskDetail() + if !strings.Contains(got, "renumbered detail") { + t.Fatalf("rendered detail %q does not include renumbered task description", got) + } + if strings.Contains(got, "old detail") { + t.Fatalf("rendered detail %q still includes stale task description", got) + } +} + func TestRefreshCurrentTaskDetailClosesMissingTask(t *testing.T) { m := Model{ detailViewState: detailViewState{ - showTaskDetail: true, - currentTaskDetailID: 1, + showTaskDetail: true, + currentTaskDetailUUID: "fake-1", }, tasks: []task.Task{ {ID: 2, UUID: "fake-2", Description: "different task", Status: "pending"}, @@ -276,8 +375,11 @@ func TestRefreshCurrentTaskDetailClosesMissingTask(t *testing.T) { if m.showTaskDetail { t.Fatalf("detail view stayed open for missing task") } - if m.currentTaskDetailID != 0 { - t.Fatalf("current detail task ID = %d, want 0", m.currentTaskDetailID) + if m.currentTaskDetailUUID != "" { + t.Fatalf("current detail task UUID = %q, want cleared", m.currentTaskDetailUUID) + } + if m.currentTaskDetailFallbackID != 0 { + t.Fatalf("current detail fallback ID = %d, want 0", m.currentTaskDetailFallbackID) } if got := m.renderTaskDetail(); got != "No task selected" { t.Fatalf("rendered detail = %q, want no task selected", got) @@ -518,8 +620,8 @@ func TestHandleDescEditDoneUpdatesDescriptionAndRemovesTempFile(t *testing.T) { if err != nil { t.Fatalf("New: %v", err) } - m.currentTaskDetailID = 1 m.showTaskDetail = true + m.currentTaskDetailUUID = "x" m.detailDescEditing = true tempFile := filepath.Join(tmp, "desc.txt") @@ -1385,7 +1487,7 @@ func TestHandleRecurrenceModeDetailBlinkTargetsRecurField(t *testing.T) { m := newRecurrenceDetailModel(t, "") m.showTaskDetail = true - m.currentTaskDetailID = m.tasks[0].ID + m.setCurrentTaskDetail(&m.tasks[0]) current := m.currentDetailTask() m.activateRecurEdit(current.ID, current.Recur) m.recurInput.SetValue("daily") @@ -1418,7 +1520,7 @@ func TestHandleRecurrenceModeDetailFallsBackWhenRecurrenceRemoved(t *testing.T) m := newRecurrenceDetailModel(t, "daily") m.showTaskDetail = true - m.currentTaskDetailID = m.tasks[0].ID + m.setCurrentTaskDetail(&m.tasks[0]) m.detailBlinkField = -1 current := m.currentDetailTask() m.activateRecurEdit(current.ID, current.Recur) diff --git a/internal/ui/taskdetail.go b/internal/ui/taskdetail.go index b39af51..bcaed8f 100644 --- a/internal/ui/taskdetail.go +++ b/internal/ui/taskdetail.go @@ -369,16 +369,58 @@ func (m *Model) taskByID(id int) *task.Task { return nil } +func (m *Model) taskByUUID(uuid string) *task.Task { + uuid = strings.TrimSpace(uuid) + if uuid == "" { + return nil + } + for i := range m.tasks { + if m.tasks[i].UUID == uuid { + return &m.tasks[i] + } + } + return nil +} + func (m *Model) currentDetailTask() *task.Task { - if !m.showTaskDetail || m.currentTaskDetailID == 0 { + if !m.showTaskDetail { return nil } - return m.taskByID(m.currentTaskDetailID) + if m.currentTaskDetailUUID != "" { + return m.taskByUUID(m.currentTaskDetailUUID) + } + if m.currentTaskDetailFallbackID != 0 { + return m.taskByID(m.currentTaskDetailFallbackID) + } + return nil +} + +func (m *Model) setCurrentTaskDetail(t *task.Task) { + // UUID is stable across Taskwarrior renumbering. Numeric ID is used only + // for legacy/defensive compatibility when a task has no UUID. + m.currentTaskDetailUUID = strings.TrimSpace(t.UUID) + if m.currentTaskDetailUUID != "" { + m.currentTaskDetailFallbackID = 0 + return + } + m.currentTaskDetailFallbackID = t.ID +} + +func (m *Model) clearCurrentTaskDetail() { + m.currentTaskDetailUUID = "" + m.currentTaskDetailFallbackID = 0 +} + +func (m *Model) hasCurrentTaskDetailIdentity() bool { + if m.currentTaskDetailUUID != "" { + return true + } + return m.currentTaskDetailFallbackID != 0 } // refreshCurrentTaskDetail validates detail state after a reload. func (m *Model) refreshCurrentTaskDetail() { - if m.currentTaskDetailID == 0 { + if !m.hasCurrentTaskDetailIdentity() { return } if m.currentDetailTask() != nil { @@ -387,7 +429,7 @@ func (m *Model) refreshCurrentTaskDetail() { // Task no longer exists, clear detail view m.showTaskDetail = false - m.currentTaskDetailID = 0 + m.clearCurrentTaskDetail() } // detailDescriptionFieldIndex returns the navigable field index for the -- cgit v1.2.3