diff --git a/apps/backend/internal/office/repository/sqlite/blockers.go b/apps/backend/internal/office/repository/sqlite/blockers.go index 03cfbbd445..f7f14899a2 100644 --- a/apps/backend/internal/office/repository/sqlite/blockers.go +++ b/apps/backend/internal/office/repository/sqlite/blockers.go @@ -40,13 +40,19 @@ func (r *Repository) DeleteTaskBlocker(ctx context.Context, taskID, blockerTaskI } // ListTasksBlockedBy returns task IDs that are blocked by the given task. +// This is the reverse direction of ListTaskBlockers; results are ordered by +// insertion time so callers see a stable list. func (r *Repository) ListTasksBlockedBy(ctx context.Context, blockerTaskID string) ([]string, error) { var ids []string err := r.ro.SelectContext(ctx, &ids, r.ro.Rebind( - `SELECT task_id FROM task_blockers WHERE blocker_task_id = ?`), blockerTaskID) + `SELECT task_id FROM task_blockers WHERE blocker_task_id = ? ORDER BY created_at, task_id`), + blockerTaskID) if err != nil { return nil, err } + if ids == nil { + ids = []string{} + } return ids, nil } diff --git a/apps/backend/internal/office/repository/sqlite/blockers_test.go b/apps/backend/internal/office/repository/sqlite/blockers_test.go index 75724e6b4e..bede188bca 100644 --- a/apps/backend/internal/office/repository/sqlite/blockers_test.go +++ b/apps/backend/internal/office/repository/sqlite/blockers_test.go @@ -2,6 +2,7 @@ package sqlite_test import ( "context" + "slices" "testing" "github.com/kandev/kandev/internal/office/models" @@ -39,6 +40,70 @@ func TestTaskBlocker_CRUD(t *testing.T) { } } +func TestListTasksBlockedBy(t *testing.T) { + repo := newTestRepo(t) + ctx := context.Background() + + // No edges yet: empty, non-nil result. + ids, err := repo.ListTasksBlockedBy(ctx, "task-3") + if err != nil { + t.Fatalf("ListTasksBlockedBy: %v", err) + } + if ids == nil { + t.Error("expected non-nil empty slice, got nil") + } + if len(ids) != 0 { + t.Errorf("expected no blocked tasks, got %v", ids) + } + + // task-1 and task-2 are blocked by task-3; task-2 is also blocked by task-4. + for _, b := range []*models.TaskBlocker{ + {TaskID: "task-1", BlockerTaskID: "task-3"}, + {TaskID: "task-2", BlockerTaskID: "task-3"}, + {TaskID: "task-2", BlockerTaskID: "task-4"}, + } { + if err := repo.CreateTaskBlocker(ctx, b); err != nil { + t.Fatalf("create blocker: %v", err) + } + } + + // One blocked task. + ids, err = repo.ListTasksBlockedBy(ctx, "task-4") + if err != nil { + t.Fatalf("ListTasksBlockedBy: %v", err) + } + if !slices.Equal(ids, []string{"task-2"}) { + t.Errorf("blocked by task-4 = %v, want [task-2]", ids) + } + + // Several blocked tasks, ordered by insertion time. + ids, err = repo.ListTasksBlockedBy(ctx, "task-3") + if err != nil { + t.Fatalf("ListTasksBlockedBy: %v", err) + } + if !slices.Equal(ids, []string{"task-1", "task-2"}) { + t.Errorf("blocked by task-3 = %v, want [task-1 task-2]", ids) + } + + // A task that only appears on the blocked side blocks nothing. + ids, err = repo.ListTasksBlockedBy(ctx, "task-1") + if err != nil { + t.Fatalf("ListTasksBlockedBy: %v", err) + } + if len(ids) != 0 { + t.Errorf("blocked by task-1 = %v, want none", ids) + } + + // Deleting an edge removes it from the reverse lookup. + if err := repo.DeleteTaskBlocker(ctx, "task-1", "task-3"); err != nil { + t.Fatalf("DeleteTaskBlocker: %v", err) + } + ids, _ = repo.ListTasksBlockedBy(ctx, "task-3") + if !slices.Equal(ids, []string{"task-2"}) { + t.Errorf("blocked by task-3 after delete = %v, want [task-2]", ids) + } +} + func TestGetChildSummaries(t *testing.T) { repo := newSearchTestRepo(t) ctx := context.Background() diff --git a/apps/backend/internal/task/service/service_office.go b/apps/backend/internal/task/service/service_office.go index 955ac39d64..dd8e629625 100644 --- a/apps/backend/internal/task/service/service_office.go +++ b/apps/backend/internal/task/service/service_office.go @@ -147,15 +147,20 @@ func (s *Service) GetBlockers(ctx context.Context, taskID string) ([]string, err // GetBlocking returns all task IDs that the given task is blocking. // This is the reverse lookup: find all tasks where blockerTaskID = taskID. +// Like GetBlockers, it returns the raw blocker edges: archived tasks are not +// filtered out, so callers that only want active tasks must filter themselves. func (s *Service) GetBlocking(ctx context.Context, taskID string) ([]string, error) { if s.blockers == nil { return nil, fmt.Errorf("blocker repository not configured") } - // We need to search across all tasks. For now, we use a pragmatic approach: - // query all tasks and check. This will be optimized with a dedicated query later. - // For the initial implementation, we return an empty list. - // TODO: Add a GetBlockingTasks query to the blocker repository - return []string{}, nil + ids, err := s.blockers.ListTasksBlockedBy(ctx, taskID) + if err != nil { + return nil, err + } + if ids == nil { + return []string{}, nil + } + return ids, nil } // checkCircularBlocker checks if adding blockerTaskID as a blocker to taskID diff --git a/apps/backend/internal/task/service/service_office_test.go b/apps/backend/internal/task/service/service_office_test.go index b2d337f203..b514c1709f 100644 --- a/apps/backend/internal/task/service/service_office_test.go +++ b/apps/backend/internal/task/service/service_office_test.go @@ -3,6 +3,8 @@ package service import ( "context" "database/sql" + "slices" + "sort" "strings" "testing" @@ -372,6 +374,122 @@ func TestBlocker_CRUD(t *testing.T) { } } +func TestGetBlocking_ReverseLookup(t *testing.T) { + svc, _, _ := createTestService(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + + // No blocker edges at all. + ids, err := svc.GetBlocking(ctx, "task-1") + if err != nil { + t.Fatalf("GetBlocking: %v", err) + } + if ids == nil { + t.Error("expected non-nil empty slice, got nil") + } + if len(ids) != 0 { + t.Errorf("expected no blocked tasks, got %v", ids) + } + + // One blocked task: task-2 is blocked by task-1. + if err := svc.AddBlocker(ctx, "task-2", "task-1"); err != nil { + t.Fatalf("AddBlocker: %v", err) + } + ids, err = svc.GetBlocking(ctx, "task-1") + if err != nil { + t.Fatalf("GetBlocking: %v", err) + } + if len(ids) != 1 || ids[0] != "task-2" { + t.Errorf("expected [task-2], got %v", ids) + } + + // The reverse direction must not be confused with the forward one: + // task-2 blocks nothing, and task-1 is the blocker of task-2. + if ids, err = svc.GetBlocking(ctx, "task-2"); err != nil || len(ids) != 0 { + t.Errorf("GetBlocking(task-2) = %v, %v; want empty", ids, err) + } + if ids, err = svc.GetBlockers(ctx, "task-2"); err != nil || len(ids) != 1 || ids[0] != "task-1" { + t.Errorf("GetBlockers(task-2) = %v, %v; want [task-1]", ids, err) + } + + // Several blocked tasks. + for _, blocked := range []string{"task-3", "task-4"} { + if err := svc.AddBlocker(ctx, blocked, "task-1"); err != nil { + t.Fatalf("AddBlocker(%s): %v", blocked, err) + } + } + ids, err = svc.GetBlocking(ctx, "task-1") + if err != nil { + t.Fatalf("GetBlocking: %v", err) + } + sort.Strings(ids) + want := []string{"task-2", "task-3", "task-4"} + if !slices.Equal(ids, want) { + t.Errorf("expected %v, got %v", want, ids) + } + + // Removing an edge removes it from the reverse lookup too. + if err := svc.RemoveBlocker(ctx, "task-3", "task-1"); err != nil { + t.Fatalf("RemoveBlocker: %v", err) + } + ids, _ = svc.GetBlocking(ctx, "task-1") + sort.Strings(ids) + if !slices.Equal(ids, []string{"task-2", "task-4"}) { + t.Errorf("expected [task-2 task-4] after removal, got %v", ids) + } +} + +func TestGetBlocking_NoRepositoryConfigured(t *testing.T) { + svc, _, _ := createTestService(t) + ctx := context.Background() + + if _, err := svc.GetBlocking(ctx, "task-1"); err == nil { + t.Fatal("expected error when blocker repository is not configured") + } +} + +// Archiving a task does not drop it from the reverse lookup: GetBlocking +// mirrors GetBlockers and reports the raw blocker edges. Callers that only +// want active tasks filter themselves. +func TestGetBlocking_IncludesArchivedTasks(t *testing.T) { + svc, repo := setupOfficeTest(t) + svc.SetBlockerRepository(&mockBlockerRepo{}) + ctx := context.Background() + + blocker, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", Title: "Blocker", ProjectID: "proj-1", + }) + if err != nil { + t.Fatalf("create blocker: %v", err) + } + blocked, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", Title: "Blocked", ProjectID: "proj-1", + BlockedBy: []string{blocker.ID}, + }) + if err != nil { + t.Fatalf("create blocked task: %v", err) + } + + ids, err := svc.GetBlocking(ctx, blocker.ID) + if err != nil { + t.Fatalf("GetBlocking: %v", err) + } + if len(ids) != 1 || ids[0] != blocked.ID { + t.Fatalf("expected [%s], got %v", blocked.ID, ids) + } + + if err := repo.ArchiveTask(ctx, blocked.ID); err != nil { + t.Fatalf("ArchiveTask: %v", err) + } + ids, err = svc.GetBlocking(ctx, blocker.ID) + if err != nil { + t.Fatalf("GetBlocking after archive: %v", err) + } + if len(ids) != 1 || ids[0] != blocked.ID { + t.Errorf("expected archived task %s to still be reported, got %v", blocked.ID, ids) + } +} + func TestBlocker_CircularDetection(t *testing.T) { svc, _, _ := createTestService(t) svc.SetBlockerRepository(&mockBlockerRepo{})