fix(rpc): return JSON data for dep add/remove in daemon mode (#961)
* test(rpc): add failing tests for dep add/remove JSON output (GH#952)
- Add TestDepAdd_JSONOutput: verify Response.Data contains JSON
- Add TestDepRemove_JSONOutput: verify Response.Data contains JSON
Tests prove bug exists: handlers return Response{Success: true}
without populating Data field.
* fix(rpc): return JSON data for dep add/remove in daemon mode (GH#952)
Problem:
- bd dep add/remove --json returned empty output in daemon mode
- handleDepAdd() and handleSimpleStoreOp() didn't populate Response.Data
Solution:
- handleDepAdd: add Data field with JSON result
- handleSimpleStoreOp: add optional responseData callback parameter
- handleDepRemove: provide response data callback
- Label handlers: pass nil (maintain current behavior)
This commit is contained in:
committed by
GitHub
parent
cbefb11c0f
commit
65214b7693
@@ -80,12 +80,20 @@ func (s *Server) handleDepAdd(req *Request) Response {
|
||||
title, assignee := s.lookupIssueMeta(ctx, depArgs.FromID)
|
||||
s.emitMutation(MutationUpdate, depArgs.FromID, title, assignee)
|
||||
|
||||
return Response{Success: true}
|
||||
result := map[string]interface{}{
|
||||
"status": "added",
|
||||
"issue_id": depArgs.FromID,
|
||||
"depends_on_id": depArgs.ToID,
|
||||
"type": depArgs.DepType,
|
||||
}
|
||||
data, _ := json.Marshal(result)
|
||||
return Response{Success: true, Data: data}
|
||||
}
|
||||
|
||||
// Generic handler for simple store operations with standard error handling
|
||||
func (s *Server) handleSimpleStoreOp(req *Request, argsPtr interface{}, argDesc string,
|
||||
opFunc func(context.Context, storage.Storage, string) error, issueID string) Response {
|
||||
opFunc func(context.Context, storage.Storage, string) error, issueID string,
|
||||
responseData func() map[string]interface{}) Response {
|
||||
if err := json.Unmarshal(req.Args, argsPtr); err != nil {
|
||||
return Response{
|
||||
Success: false,
|
||||
@@ -113,28 +121,42 @@ func (s *Server) handleSimpleStoreOp(req *Request, argsPtr interface{}, argDesc
|
||||
title, assignee := s.lookupIssueMeta(ctx, issueID)
|
||||
s.emitMutation(MutationUpdate, issueID, title, assignee)
|
||||
|
||||
if responseData != nil {
|
||||
data, _ := json.Marshal(responseData())
|
||||
return Response{Success: true, Data: data}
|
||||
}
|
||||
return Response{Success: true}
|
||||
}
|
||||
|
||||
func (s *Server) handleDepRemove(req *Request) Response {
|
||||
var depArgs DepRemoveArgs
|
||||
return s.handleSimpleStoreOp(req, &depArgs, "dep remove", func(ctx context.Context, store storage.Storage, actor string) error {
|
||||
return store.RemoveDependency(ctx, depArgs.FromID, depArgs.ToID, actor)
|
||||
}, depArgs.FromID)
|
||||
return s.handleSimpleStoreOp(req, &depArgs, "dep remove",
|
||||
func(ctx context.Context, store storage.Storage, actor string) error {
|
||||
return store.RemoveDependency(ctx, depArgs.FromID, depArgs.ToID, actor)
|
||||
},
|
||||
depArgs.FromID,
|
||||
func() map[string]interface{} {
|
||||
return map[string]interface{}{
|
||||
"status": "removed",
|
||||
"issue_id": depArgs.FromID,
|
||||
"depends_on_id": depArgs.ToID,
|
||||
}
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
func (s *Server) handleLabelAdd(req *Request) Response {
|
||||
var labelArgs LabelAddArgs
|
||||
return s.handleSimpleStoreOp(req, &labelArgs, "label add", func(ctx context.Context, store storage.Storage, actor string) error {
|
||||
return store.AddLabel(ctx, labelArgs.ID, labelArgs.Label, actor)
|
||||
}, labelArgs.ID)
|
||||
}, labelArgs.ID, nil)
|
||||
}
|
||||
|
||||
func (s *Server) handleLabelRemove(req *Request) Response {
|
||||
var labelArgs LabelRemoveArgs
|
||||
return s.handleSimpleStoreOp(req, &labelArgs, "label remove", func(ctx context.Context, store storage.Storage, actor string) error {
|
||||
return store.RemoveLabel(ctx, labelArgs.ID, labelArgs.Label, actor)
|
||||
}, labelArgs.ID)
|
||||
}, labelArgs.ID, nil)
|
||||
}
|
||||
|
||||
func (s *Server) handleCommentList(req *Request) Response {
|
||||
|
||||
172
internal/rpc/server_labels_deps_comments_test.go
Normal file
172
internal/rpc/server_labels_deps_comments_test.go
Normal file
@@ -0,0 +1,172 @@
|
||||
package rpc
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestDepAdd_JSONOutput verifies that handleDepAdd returns JSON data in Response.Data.
|
||||
// This test is expected to FAIL until the bug is fixed (GH#952 Issue 2).
|
||||
func TestDepAdd_JSONOutput(t *testing.T) {
|
||||
_, client, store, cleanup := setupTestServerWithStore(t)
|
||||
defer cleanup()
|
||||
|
||||
// Create two test issues for the dependency relationship
|
||||
createArgs1 := &CreateArgs{
|
||||
Title: "Issue that depends on another",
|
||||
IssueType: "task",
|
||||
Priority: 2,
|
||||
}
|
||||
resp1, err := client.Create(createArgs1)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create first issue: %v", err)
|
||||
}
|
||||
var issue1 struct{ ID string }
|
||||
if err := json.Unmarshal(resp1.Data, &issue1); err != nil {
|
||||
t.Fatalf("Failed to unmarshal first issue: %v", err)
|
||||
}
|
||||
|
||||
createArgs2 := &CreateArgs{
|
||||
Title: "Issue being depended upon",
|
||||
IssueType: "task",
|
||||
Priority: 2,
|
||||
}
|
||||
resp2, err := client.Create(createArgs2)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create second issue: %v", err)
|
||||
}
|
||||
var issue2 struct{ ID string }
|
||||
if err := json.Unmarshal(resp2.Data, &issue2); err != nil {
|
||||
t.Fatalf("Failed to unmarshal second issue: %v", err)
|
||||
}
|
||||
|
||||
// Add dependency: issue1 depends on issue2
|
||||
depArgs := &DepAddArgs{
|
||||
FromID: issue1.ID,
|
||||
ToID: issue2.ID,
|
||||
DepType: "blocks",
|
||||
}
|
||||
resp, err := client.AddDependency(depArgs)
|
||||
if err != nil {
|
||||
t.Fatalf("AddDependency failed: %v", err)
|
||||
}
|
||||
|
||||
// BUG: Response.Data is nil when it should contain JSON
|
||||
if resp.Data == nil {
|
||||
t.Errorf("resp.Data is nil; expected JSON output with {status, issue_id, depends_on_id, type}")
|
||||
}
|
||||
|
||||
// Verify JSON structure matches expected format
|
||||
if resp.Data != nil {
|
||||
var result struct {
|
||||
Status string `json:"status"`
|
||||
IssueID string `json:"issue_id"`
|
||||
DependsOnID string `json:"depends_on_id"`
|
||||
Type string `json:"type"`
|
||||
}
|
||||
if err := json.Unmarshal(resp.Data, &result); err != nil {
|
||||
t.Errorf("Failed to unmarshal response data: %v", err)
|
||||
}
|
||||
if result.Status != "added" {
|
||||
t.Errorf("Expected status='added', got %q", result.Status)
|
||||
}
|
||||
if result.IssueID != issue1.ID {
|
||||
t.Errorf("Expected issue_id=%q, got %q", issue1.ID, result.IssueID)
|
||||
}
|
||||
if result.DependsOnID != issue2.ID {
|
||||
t.Errorf("Expected depends_on_id=%q, got %q", issue2.ID, result.DependsOnID)
|
||||
}
|
||||
if result.Type != "blocks" {
|
||||
t.Errorf("Expected type='blocks', got %q", result.Type)
|
||||
}
|
||||
}
|
||||
|
||||
// Silence unused variable warning
|
||||
_ = store
|
||||
}
|
||||
|
||||
// TestDepRemove_JSONOutput verifies that handleDepRemove returns JSON data in Response.Data.
|
||||
// This test is expected to FAIL until the bug is fixed (GH#952 Issue 2).
|
||||
func TestDepRemove_JSONOutput(t *testing.T) {
|
||||
_, client, store, cleanup := setupTestServerWithStore(t)
|
||||
defer cleanup()
|
||||
|
||||
// Create two test issues
|
||||
createArgs1 := &CreateArgs{
|
||||
Title: "Issue with dependency to remove",
|
||||
IssueType: "task",
|
||||
Priority: 2,
|
||||
}
|
||||
resp1, err := client.Create(createArgs1)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create first issue: %v", err)
|
||||
}
|
||||
var issue1 struct{ ID string }
|
||||
if err := json.Unmarshal(resp1.Data, &issue1); err != nil {
|
||||
t.Fatalf("Failed to unmarshal first issue: %v", err)
|
||||
}
|
||||
|
||||
createArgs2 := &CreateArgs{
|
||||
Title: "Dependency target issue",
|
||||
IssueType: "task",
|
||||
Priority: 2,
|
||||
}
|
||||
resp2, err := client.Create(createArgs2)
|
||||
if err != nil {
|
||||
t.Fatalf("Failed to create second issue: %v", err)
|
||||
}
|
||||
var issue2 struct{ ID string }
|
||||
if err := json.Unmarshal(resp2.Data, &issue2); err != nil {
|
||||
t.Fatalf("Failed to unmarshal second issue: %v", err)
|
||||
}
|
||||
|
||||
// First add a dependency so we can remove it
|
||||
addArgs := &DepAddArgs{
|
||||
FromID: issue1.ID,
|
||||
ToID: issue2.ID,
|
||||
DepType: "blocks",
|
||||
}
|
||||
_, err = client.AddDependency(addArgs)
|
||||
if err != nil {
|
||||
t.Fatalf("AddDependency (setup) failed: %v", err)
|
||||
}
|
||||
|
||||
// Now remove the dependency
|
||||
removeArgs := &DepRemoveArgs{
|
||||
FromID: issue1.ID,
|
||||
ToID: issue2.ID,
|
||||
}
|
||||
resp, err := client.RemoveDependency(removeArgs)
|
||||
if err != nil {
|
||||
t.Fatalf("RemoveDependency failed: %v", err)
|
||||
}
|
||||
|
||||
// BUG: Response.Data is nil when it should contain JSON
|
||||
if resp.Data == nil {
|
||||
t.Errorf("resp.Data is nil; expected JSON output with {status, issue_id, depends_on_id}")
|
||||
}
|
||||
|
||||
// Verify JSON structure matches expected format
|
||||
if resp.Data != nil {
|
||||
var result struct {
|
||||
Status string `json:"status"`
|
||||
IssueID string `json:"issue_id"`
|
||||
DependsOnID string `json:"depends_on_id"`
|
||||
}
|
||||
if err := json.Unmarshal(resp.Data, &result); err != nil {
|
||||
t.Errorf("Failed to unmarshal response data: %v", err)
|
||||
}
|
||||
if result.Status != "removed" {
|
||||
t.Errorf("Expected status='removed', got %q", result.Status)
|
||||
}
|
||||
if result.IssueID != issue1.ID {
|
||||
t.Errorf("Expected issue_id=%q, got %q", issue1.ID, result.IssueID)
|
||||
}
|
||||
if result.DependsOnID != issue2.ID {
|
||||
t.Errorf("Expected depends_on_id=%q, got %q", issue2.ID, result.DependsOnID)
|
||||
}
|
||||
}
|
||||
|
||||
// Silence unused variable warning
|
||||
_ = store
|
||||
}
|
||||
Reference in New Issue
Block a user