diff --git a/internal/db/activity.go b/internal/db/activity.go index 78b06427..6b8e3707 100644 --- a/internal/db/activity.go +++ b/internal/db/activity.go @@ -54,6 +54,7 @@ func (db *DB) AddLog(log *models.Log) error { // GetLogs retrieves logs for an issue, including work session logs func (db *DB) GetLogs(issueID string, limit int) ([]models.Log, error) { + issueID = NormalizeIssueID(issueID) // Get logs that are either: // 1. Directly assigned to this issue (issue_id = ?) // 2. Work session logs (issue_id = '') from sessions where this issue is tagged @@ -267,6 +268,7 @@ func (db *DB) AddHandoff(handoff *models.Handoff) error { // GetLatestHandoff retrieves the latest handoff for an issue func (db *DB) GetLatestHandoff(issueID string) (*models.Handoff, error) { + issueID = NormalizeIssueID(issueID) var handoff models.Handoff var doneJSON, remainingJSON, decisionsJSON, uncertainJSON string @@ -784,6 +786,7 @@ func (db *DB) AddGitSnapshot(snapshot *models.GitSnapshot) error { // GetStartSnapshot returns the start snapshot for an issue func (db *DB) GetStartSnapshot(issueID string) (*models.GitSnapshot, error) { + issueID = NormalizeIssueID(issueID) var snapshot models.GitSnapshot err := db.conn.QueryRow(` diff --git a/internal/db/bare_id_relations_test.go b/internal/db/bare_id_relations_test.go new file mode 100644 index 00000000..a96f1c70 --- /dev/null +++ b/internal/db/bare_id_relations_test.go @@ -0,0 +1,105 @@ +package db + +import ( + "testing" + + "github.com/marcus/td/internal/models" +) + +// Relation getters must accept a bare id the same way GetIssue does. Before +// they normalized, `td show abc123` resolved the issue but returned no +// dependencies, logs, handoff, or git snapshot, because those queries ran +// against the raw `abc123` while the rows are keyed `td-abc123`. Each caller +// discards the error, so the omission was silent. See marcus/td#199 for the +// same defect on the write path. +func TestRelationGettersAcceptBareIDs(t *testing.T) { + dir := t.TempDir() + database, err := Initialize(dir) + if err != nil { + t.Fatalf("Initialize failed: %v", err) + } + defer func() { _ = database.Close() }() + + blocker := &models.Issue{Title: "Blocker", Type: models.TypeTask, Priority: models.PriorityP2} + if err := database.CreateIssue(blocker); err != nil { + t.Fatalf("CreateIssue(blocker) failed: %v", err) + } + dependent := &models.Issue{Title: "Dependent", Type: models.TypeTask, Priority: models.PriorityP2} + if err := database.CreateIssue(dependent); err != nil { + t.Fatalf("CreateIssue(dependent) failed: %v", err) + } + + if err := database.AddDependency(dependent.ID, blocker.ID, "depends_on"); err != nil { + t.Fatalf("AddDependency failed: %v", err) + } + if err := database.AddLog(&models.Log{IssueID: dependent.ID, Message: "entry"}); err != nil { + t.Fatalf("AddLog failed: %v", err) + } + + bareDependent := stripPrefix(t, dependent.ID) + bareBlocker := stripPrefix(t, blocker.ID) + + t.Run("GetDependencies", func(t *testing.T) { + prefixed, err := database.GetDependencies(dependent.ID) + if err != nil { + t.Fatalf("GetDependencies(prefixed) failed: %v", err) + } + if len(prefixed) != 1 { + t.Fatalf("GetDependencies(prefixed) returned %d deps, want 1", len(prefixed)) + } + bare, err := database.GetDependencies(bareDependent) + if err != nil { + t.Fatalf("GetDependencies(bare) failed: %v", err) + } + if len(bare) != len(prefixed) { + t.Errorf("bare id returned %d deps, prefixed returned %d; the two must agree", + len(bare), len(prefixed)) + } + }) + + t.Run("GetBlockedBy", func(t *testing.T) { + prefixed, err := database.GetBlockedBy(blocker.ID) + if err != nil { + t.Fatalf("GetBlockedBy(prefixed) failed: %v", err) + } + if len(prefixed) != 1 { + t.Fatalf("GetBlockedBy(prefixed) returned %d, want 1", len(prefixed)) + } + bare, err := database.GetBlockedBy(bareBlocker) + if err != nil { + t.Fatalf("GetBlockedBy(bare) failed: %v", err) + } + if len(bare) != len(prefixed) { + t.Errorf("bare id returned %d, prefixed returned %d; the two must agree", + len(bare), len(prefixed)) + } + }) + + t.Run("GetLogs", func(t *testing.T) { + prefixed, err := database.GetLogs(dependent.ID, 0) + if err != nil { + t.Fatalf("GetLogs(prefixed) failed: %v", err) + } + if len(prefixed) == 0 { + t.Fatal("GetLogs(prefixed) returned no logs, want at least 1") + } + bare, err := database.GetLogs(bareDependent, 0) + if err != nil { + t.Fatalf("GetLogs(bare) failed: %v", err) + } + if len(bare) != len(prefixed) { + t.Errorf("bare id returned %d logs, prefixed returned %d; the two must agree", + len(bare), len(prefixed)) + } + }) +} + +// stripPrefix returns the id without its td- prefix, which is what a user types +// when they copy an id out of a listing and drop the prefix. +func stripPrefix(t *testing.T, id string) string { + t.Helper() + if len(id) <= len(idPrefix) || id[:len(idPrefix)] != idPrefix { + t.Fatalf("expected %q to carry the %q prefix", id, idPrefix) + } + return id[len(idPrefix):] +} diff --git a/internal/db/issue_relations.go b/internal/db/issue_relations.go index 0f455ef9..1b34eae3 100644 --- a/internal/db/issue_relations.go +++ b/internal/db/issue_relations.go @@ -402,6 +402,7 @@ func (db *DB) GetIssueDependencyRelations(issueID string) ([]models.IssueDepende // GetDependencies returns what an issue depends on func (db *DB) GetDependencies(issueID string) ([]string, error) { + issueID = NormalizeIssueID(issueID) rows, err := db.conn.Query(` SELECT depends_on_id FROM issue_dependencies WHERE issue_id = ? AND relation_type = 'depends_on' `, issueID) @@ -426,6 +427,7 @@ func (db *DB) GetDependencies(issueID string) ([]string, error) { // GetBlockedBy returns what issues are blocked by this issue func (db *DB) GetBlockedBy(issueID string) ([]string, error) { + issueID = NormalizeIssueID(issueID) rows, err := db.conn.Query(` SELECT issue_id FROM issue_dependencies WHERE depends_on_id = ? AND relation_type = 'depends_on' `, issueID) @@ -699,6 +701,7 @@ func (db *DB) UnlinkFile(issueID, filePath string) error { // GetLinkedFiles returns files linked to an issue func (db *DB) GetLinkedFiles(issueID string) ([]models.IssueFile, error) { + issueID = NormalizeIssueID(issueID) rows, err := db.conn.Query(` SELECT CAST(id AS TEXT), issue_id, file_path, role, linked_sha, linked_at FROM issue_files WHERE issue_id = ? ORDER BY role, file_path