diff --git a/cmd/bd/cli_coverage_show_test.go b/cmd/bd/cli_coverage_show_test.go index 688df80f..6eb4d661 100644 --- a/cmd/bd/cli_coverage_show_test.go +++ b/cmd/bd/cli_coverage_show_test.go @@ -1,3 +1,5 @@ +//go:build e2e + package main import ( diff --git a/cmd/bd/show.go b/cmd/bd/show.go index cec5ae93..856db09c 100644 --- a/cmd/bd/show.go +++ b/cmd/bd/show.go @@ -663,8 +663,8 @@ var updateCmd = &cobra.Command{ fmt.Fprintf(os.Stderr, "Error getting %s: %v\n", id, err) continue } - if issue != nil && issue.IsTemplate { - fmt.Fprintf(os.Stderr, "Error: cannot update template %s: templates are read-only; use 'bd molecule instantiate' to create a work item\n", id) + if err := validateIssueUpdatable(id, issue); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) continue } @@ -683,48 +683,21 @@ var updateCmd = &cobra.Command{ } // Handle label operations - // Set labels (replaces all existing labels) - if setLabels, ok := updates["set_labels"].([]string); ok && len(setLabels) > 0 { - // Get current labels - currentLabels, err := store.GetLabels(ctx, id) - if err != nil { - fmt.Fprintf(os.Stderr, "Error getting labels for %s: %v\n", id, err) + var setLabels, addLabels, removeLabels []string + if v, ok := updates["set_labels"].([]string); ok { + setLabels = v + } + if v, ok := updates["add_labels"].([]string); ok { + addLabels = v + } + if v, ok := updates["remove_labels"].([]string); ok { + removeLabels = v + } + if len(setLabels) > 0 || len(addLabels) > 0 || len(removeLabels) > 0 { + if err := applyLabelUpdates(ctx, store, id, actor, setLabels, addLabels, removeLabels); err != nil { + fmt.Fprintf(os.Stderr, "Error updating labels for %s: %v\n", id, err) continue } - // Remove all current labels - for _, label := range currentLabels { - if err := store.RemoveLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error removing label %s from %s: %v\n", label, id, err) - continue - } - } - // Add new labels - for _, label := range setLabels { - if err := store.AddLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error setting label %s on %s: %v\n", label, id, err) - continue - } - } - } - - // Add labels - if addLabels, ok := updates["add_labels"].([]string); ok { - for _, label := range addLabels { - if err := store.AddLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error adding label %s to %s: %v\n", label, id, err) - continue - } - } - } - - // Remove labels - if removeLabels, ok := updates["remove_labels"].([]string); ok { - for _, label := range removeLabels { - if err := store.RemoveLabel(ctx, id, label, actor); err != nil { - fmt.Fprintf(os.Stderr, "Error removing label %s from %s: %v\n", label, id, err) - continue - } - } } // Run update hook (bd-kwro.8) @@ -993,14 +966,8 @@ var closeCmd = &cobra.Command{ if showErr == nil { var issue types.Issue if json.Unmarshal(showResp.Data, &issue) == nil { - // Check if issue is a template (beads-1ra): templates are read-only - if issue.IsTemplate { - fmt.Fprintf(os.Stderr, "Error: cannot close template %s: templates are read-only\n", id) - continue - } - // Check if issue is pinned (bd-6v2) - if !force && issue.Status == types.StatusPinned { - fmt.Fprintf(os.Stderr, "Error: cannot close pinned issue %s (use --force to override)\n", id) + if err := validateIssueClosable(id, &issue, force); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) continue } } @@ -1051,20 +1018,11 @@ var closeCmd = &cobra.Command{ // Get issue for checks issue, _ := store.GetIssue(ctx, id) - // Check if issue is a template (beads-1ra): templates are read-only - if issue != nil && issue.IsTemplate { - fmt.Fprintf(os.Stderr, "Error: cannot close template %s: templates are read-only\n", id) + if err := validateIssueClosable(id, issue, force); err != nil { + fmt.Fprintf(os.Stderr, "%s\n", err) continue } - // Check if issue is pinned (bd-6v2) - if !force { - if issue != nil && issue.Status == types.StatusPinned { - fmt.Fprintf(os.Stderr, "Error: cannot close pinned issue %s (use --force to override)\n", id) - continue - } - } - if err := store.CloseIssue(ctx, id, reason, actor); err != nil { fmt.Fprintf(os.Stderr, "Error closing %s: %v\n", id, err) continue @@ -1291,15 +1249,13 @@ func findRepliesTo(ctx context.Context, issueID string, daemonClient *rpc.Client return "" } // Direct mode - query storage - if sqliteStore, ok := store.(*sqlite.SQLiteStorage); ok { - deps, err := sqliteStore.GetDependenciesWithMetadata(ctx, issueID) - if err != nil { - return "" - } - for _, dep := range deps { - if dep.DependencyType == types.DepRepliesTo { - return dep.ID - } + deps, err := store.GetDependencyRecords(ctx, issueID) + if err != nil { + return "" + } + for _, dep := range deps { + if dep.Type == types.DepRepliesTo { + return dep.DependsOnID } } return "" @@ -1348,7 +1304,25 @@ func findReplies(ctx context.Context, issueID string, daemonClient *rpc.Client, } return replies } - return nil + + allDeps, err := store.GetAllDependencyRecords(ctx) + if err != nil { + return nil + } + + var replies []*types.Issue + for childID, deps := range allDeps { + for _, dep := range deps { + if dep.Type == types.DepRepliesTo && dep.DependsOnID == issueID { + issue, _ := store.GetIssue(ctx, childID) + if issue != nil { + replies = append(replies, issue) + } + } + } + } + + return replies } func init() { diff --git a/cmd/bd/show_unit_helpers.go b/cmd/bd/show_unit_helpers.go new file mode 100644 index 00000000..23c80a1e --- /dev/null +++ b/cmd/bd/show_unit_helpers.go @@ -0,0 +1,68 @@ +package main + +import ( + "context" + "fmt" + + "github.com/steveyegge/beads/internal/storage" + "github.com/steveyegge/beads/internal/types" +) + +func validateIssueUpdatable(id string, issue *types.Issue) error { + if issue == nil { + return nil + } + if issue.IsTemplate { + return fmt.Errorf("Error: cannot update template %s: templates are read-only; use 'bd molecule instantiate' to create a work item", id) + } + return nil +} + +func validateIssueClosable(id string, issue *types.Issue, force bool) error { + if issue == nil { + return nil + } + if issue.IsTemplate { + return fmt.Errorf("Error: cannot close template %s: templates are read-only", id) + } + if !force && issue.Status == types.StatusPinned { + return fmt.Errorf("Error: cannot close pinned issue %s (use --force to override)", id) + } + return nil +} + +func applyLabelUpdates(ctx context.Context, st storage.Storage, issueID, actor string, setLabels, addLabels, removeLabels []string) error { + // Set labels (replaces all existing labels) + if len(setLabels) > 0 { + currentLabels, err := st.GetLabels(ctx, issueID) + if err != nil { + return err + } + for _, label := range currentLabels { + if err := st.RemoveLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + for _, label := range setLabels { + if err := st.AddLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + } + + // Add labels + for _, label := range addLabels { + if err := st.AddLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + + // Remove labels + for _, label := range removeLabels { + if err := st.RemoveLabel(ctx, issueID, label, actor); err != nil { + return err + } + } + + return nil +} diff --git a/cmd/bd/show_unit_helpers_test.go b/cmd/bd/show_unit_helpers_test.go new file mode 100644 index 00000000..e1bef58d --- /dev/null +++ b/cmd/bd/show_unit_helpers_test.go @@ -0,0 +1,139 @@ +package main + +import ( + "context" + "testing" + + "github.com/steveyegge/beads/internal/storage/memory" + "github.com/steveyegge/beads/internal/types" +) + +func TestValidateIssueUpdatable(t *testing.T) { + if err := validateIssueUpdatable("x", nil); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if err := validateIssueUpdatable("x", &types.Issue{IsTemplate: false}); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if err := validateIssueUpdatable("bd-1", &types.Issue{IsTemplate: true}); err == nil { + t.Fatalf("expected error") + } +} + +func TestValidateIssueClosable(t *testing.T) { + if err := validateIssueClosable("x", nil, false); err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if err := validateIssueClosable("bd-1", &types.Issue{IsTemplate: true}, false); err == nil { + t.Fatalf("expected template close error") + } + if err := validateIssueClosable("bd-2", &types.Issue{Status: types.StatusPinned}, false); err == nil { + t.Fatalf("expected pinned close error") + } + if err := validateIssueClosable("bd-2", &types.Issue{Status: types.StatusPinned}, true); err != nil { + t.Fatalf("expected pinned close to succeed with force, got %v", err) + } +} + +func TestApplyLabelUpdates_SetAddRemove(t *testing.T) { + ctx := context.Background() + st := memory.New("") + if err := st.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("SetConfig: %v", err) + } + + issue := &types.Issue{Title: "x", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + if err := st.CreateIssue(ctx, issue, "tester"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + _ = st.AddLabel(ctx, issue.ID, "old1", "tester") + _ = st.AddLabel(ctx, issue.ID, "old2", "tester") + + if err := applyLabelUpdates(ctx, st, issue.ID, "tester", []string{"a", "b"}, []string{"b", "c"}, []string{"a"}); err != nil { + t.Fatalf("applyLabelUpdates: %v", err) + } + labels, _ := st.GetLabels(ctx, issue.ID) + if len(labels) != 2 { + t.Fatalf("expected 2 labels, got %v", labels) + } + // Order is not guaranteed. + foundB := false + foundC := false + for _, l := range labels { + if l == "b" { + foundB = true + } + if l == "c" { + foundC = true + } + if l == "old1" || l == "old2" || l == "a" { + t.Fatalf("unexpected label %q in %v", l, labels) + } + } + if !foundB || !foundC { + t.Fatalf("expected labels b and c, got %v", labels) + } +} + +func TestApplyLabelUpdates_AddRemoveOnly(t *testing.T) { + ctx := context.Background() + st := memory.New("") + issue := &types.Issue{Title: "x", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeTask} + if err := st.CreateIssue(ctx, issue, "tester"); err != nil { + t.Fatalf("CreateIssue: %v", err) + } + + _ = st.AddLabel(ctx, issue.ID, "a", "tester") + if err := applyLabelUpdates(ctx, st, issue.ID, "tester", nil, []string{"b"}, []string{"a"}); err != nil { + t.Fatalf("applyLabelUpdates: %v", err) + } + labels, _ := st.GetLabels(ctx, issue.ID) + if len(labels) != 1 || labels[0] != "b" { + t.Fatalf("expected [b], got %v", labels) + } +} + +func TestFindRepliesToAndReplies_WorksWithMemoryStorage(t *testing.T) { + ctx := context.Background() + st := memory.New("") + if err := st.SetConfig(ctx, "issue_prefix", "test"); err != nil { + t.Fatalf("SetConfig: %v", err) + } + + root := &types.Issue{Title: "root", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "a", Assignee: "b"} + reply1 := &types.Issue{Title: "r1", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "b", Assignee: "a"} + reply2 := &types.Issue{Title: "r2", Status: types.StatusOpen, Priority: 2, IssueType: types.TypeMessage, Sender: "a", Assignee: "b"} + if err := st.CreateIssue(ctx, root, "tester"); err != nil { + t.Fatalf("CreateIssue(root): %v", err) + } + if err := st.CreateIssue(ctx, reply1, "tester"); err != nil { + t.Fatalf("CreateIssue(reply1): %v", err) + } + if err := st.CreateIssue(ctx, reply2, "tester"); err != nil { + t.Fatalf("CreateIssue(reply2): %v", err) + } + + if err := st.AddDependency(ctx, &types.Dependency{IssueID: reply1.ID, DependsOnID: root.ID, Type: types.DepRepliesTo}, "tester"); err != nil { + t.Fatalf("AddDependency(reply1->root): %v", err) + } + if err := st.AddDependency(ctx, &types.Dependency{IssueID: reply2.ID, DependsOnID: reply1.ID, Type: types.DepRepliesTo}, "tester"); err != nil { + t.Fatalf("AddDependency(reply2->reply1): %v", err) + } + + if got := findRepliesTo(ctx, root.ID, nil, st); got != "" { + t.Fatalf("expected root replies-to to be empty, got %q", got) + } + if got := findRepliesTo(ctx, reply2.ID, nil, st); got != reply1.ID { + t.Fatalf("expected reply2 parent %q, got %q", reply1.ID, got) + } + + rootReplies := findReplies(ctx, root.ID, nil, st) + if len(rootReplies) != 1 || rootReplies[0].ID != reply1.ID { + t.Fatalf("expected root replies [%s], got %+v", reply1.ID, rootReplies) + } + r1Replies := findReplies(ctx, reply1.ID, nil, st) + if len(r1Replies) != 1 || r1Replies[0].ID != reply2.ID { + t.Fatalf("expected reply1 replies [%s], got %+v", reply2.ID, r1Replies) + } +}