diff --git a/services/core/internal/http/handlers.go b/services/core/internal/http/handlers.go index 33253e3..b07b0a3 100644 --- a/services/core/internal/http/handlers.go +++ b/services/core/internal/http/handlers.go @@ -14,27 +14,27 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" - "github.com/nomadcode/nomadcode-core/internal/adapters/plane" "github.com/nomadcode/nomadcode-core/internal/workflow" + "github.com/nomadcode/nomadcode-core/internal/workitem" ) -type PlaneWorkItemClient interface { - GetWorkItem(ctx context.Context, ref plane.WorkItemRef) (plane.WorkItem, error) +type WorkItemReader interface { + FetchWorkItem(ctx context.Context, ref workitem.Ref) (workitem.WorkItem, error) } type Handler struct { - db *pgxpool.Pool - workflow *workflow.Service - plane PlaneWorkItemClient - logger *slog.Logger + db *pgxpool.Pool + workflow *workflow.Service + workItems WorkItemReader + logger *slog.Logger } -func NewHandler(pool *pgxpool.Pool, workflowService *workflow.Service, planeClient PlaneWorkItemClient, logger *slog.Logger) *Handler { +func NewHandler(pool *pgxpool.Pool, workflowService *workflow.Service, workItemReader WorkItemReader, logger *slog.Logger) *Handler { return &Handler{ - db: pool, - workflow: workflowService, - plane: planeClient, - logger: logger, + db: pool, + workflow: workflowService, + workItems: workItemReader, + logger: logger, } } @@ -85,7 +85,7 @@ type createPlaneTaskRequest struct { } func (h *Handler) CreatePlaneTask(w stdhttp.ResponseWriter, r *stdhttp.Request) { - if h.plane == nil { + if h.workItems == nil { writeError(w, stdhttp.StatusServiceUnavailable, "plane client is not configured") return } @@ -102,13 +102,13 @@ func (h *Handler) CreatePlaneTask(w stdhttp.ResponseWriter, r *stdhttp.Request) return } - workItem, err := h.plane.GetWorkItem(r.Context(), ref) + item, err := h.workItems.FetchWorkItem(r.Context(), ref) if err != nil { h.writeServiceError(w, err) return } - createInput, err := buildPlaneCreateTaskInput(input, workItem) + createInput, err := buildPlaneCreateTaskInput(input, item) if err != nil { writeError(w, stdhttp.StatusBadRequest, err.Error()) return @@ -173,72 +173,32 @@ func (h *Handler) EnqueueTask(w stdhttp.ResponseWriter, r *stdhttp.Request) { }) } -func (input createPlaneTaskRequest) workItemRef() (plane.WorkItemRef, error) { - ref := plane.WorkItemRef{ - WorkspaceSlug: strings.TrimSpace(input.WorkspaceSlug), - ProjectID: strings.TrimSpace(input.ProjectID), - WorkItemID: strings.TrimSpace(input.WorkItemID), +func (input createPlaneTaskRequest) workItemRef() (workitem.Ref, error) { + tenant := strings.TrimSpace(input.WorkspaceSlug) + project := strings.TrimSpace(input.ProjectID) + id := strings.TrimSpace(input.WorkItemID) + if tenant == "" || project == "" || id == "" { + return workitem.Ref{}, errors.New("workspace_slug, project_id, and work_item_id are required") } - if ref.WorkspaceSlug == "" || ref.ProjectID == "" || ref.WorkItemID == "" { - return plane.WorkItemRef{}, errors.New("workspace_slug, project_id, and work_item_id are required") - } - return ref, nil + return workitem.NormalizeRef(workitem.Ref{ + Provider: workitem.ProviderID("plane"), + Tenant: tenant, + Project: project, + ID: id, + URL: strings.TrimSpace(input.ExternalURL), + }) } -func buildPlaneCreateTaskInput(input createPlaneTaskRequest, workItem plane.WorkItem) (workflow.CreateTaskInput, error) { +func buildPlaneCreateTaskInput(input createPlaneTaskRequest, item workitem.WorkItem) (workflow.CreateTaskInput, error) { ref, err := input.workItemRef() if err != nil { return workflow.CreateTaskInput{}, err } - - title := strings.TrimSpace(workItem.Name) - if title == "" { - title = ref.WorkItemID - } - - stateID := strings.TrimSpace(input.StateID) - message := firstNonEmpty(input.Comment, workItem.DescriptionStripped, workItem.Description, workItem.DescriptionHTML, title) - metadata := map[string]string{ - "workspace_slug": ref.WorkspaceSlug, - "project_id": ref.ProjectID, - "work_item_id": ref.WorkItemID, - "state_id": stateID, - "external_url": strings.TrimSpace(input.ExternalURL), - } - payload := map[string]any{ - "message": message, - "plane": metadata, - } - - rawPayload, err := json.Marshal(payload) - if err != nil { - return workflow.CreateTaskInput{}, err - } - rawMetadata, err := json.Marshal(metadata) - if err != nil { - return workflow.CreateTaskInput{}, err - } - - return workflow.CreateTaskInput{ - Title: title, - Source: "plane", - Payload: rawPayload, - External: &workflow.ExternalRefInput{ - Provider: "plane", - ID: ref.WorkItemID, - URL: strings.TrimSpace(input.ExternalURL), - Metadata: rawMetadata, - }, - }, nil -} - -func firstNonEmpty(values ...string) string { - for _, value := range values { - if trimmed := strings.TrimSpace(value); trimmed != "" { - return trimmed - } - } - return "" + return workitem.BuildCreateTaskInput(workitem.TaskCreateInput{ + Ref: ref, + StateID: input.StateID, + Comment: input.Comment, + }, item) } func stringValue(value *string) string { diff --git a/services/core/internal/http/handlers_test.go b/services/core/internal/http/handlers_test.go index cd2af23..6635ca8 100644 --- a/services/core/internal/http/handlers_test.go +++ b/services/core/internal/http/handlers_test.go @@ -4,19 +4,47 @@ import ( "encoding/json" "testing" - "github.com/nomadcode/nomadcode-core/internal/adapters/plane" + "github.com/nomadcode/nomadcode-core/internal/workitem" ) +func TestPlaneTaskRequestBuildsProviderNeutralRef(t *testing.T) { + req := createPlaneTaskRequest{ + WorkspaceSlug: " acme ", + ProjectID: " proj-1 ", + WorkItemID: " work-1 ", + ExternalURL: " https://plane.example/work-1 ", + } + ref, err := req.workItemRef() + if err != nil { + t.Fatalf("workItemRef returned error: %v", err) + } + if ref.Provider != workitem.ProviderID("plane") { + t.Errorf("provider: got %q", ref.Provider) + } + if ref.Tenant != "acme" { + t.Errorf("tenant: got %q", ref.Tenant) + } + if ref.Project != "proj-1" { + t.Errorf("project: got %q", ref.Project) + } + if ref.ID != "work-1" { + t.Errorf("id: got %q", ref.ID) + } + if ref.URL != "https://plane.example/work-1" { + t.Errorf("url: got %q", ref.URL) + } +} + func TestBuildPlaneCreateTaskInputUsesCommentBeforeDescription(t *testing.T) { input, err := buildPlaneCreateTaskInput(createPlaneTaskRequest{ WorkspaceSlug: "general", ProjectID: "project-1", WorkItemID: "work-1", Comment: "operator instruction", - }, plane.WorkItem{ - ID: "work-1", - Name: "Fix issue", - DescriptionStripped: "plane description", + }, workitem.WorkItem{ + Ref: workitem.Ref{Provider: "plane", Tenant: "general", Project: "project-1", ID: "work-1"}, + Title: "Fix issue", + DescriptionText: "plane description", }) if err != nil { t.Fatalf("buildPlaneCreateTaskInput returned error: %v", err) @@ -44,9 +72,9 @@ func TestBuildPlaneCreateTaskInputStoresExternalMetadata(t *testing.T) { WorkItemID: "work-1", StateID: "state-1", ExternalURL: "https://plane.example/work-1", - }, plane.WorkItem{ - ID: "work-1", - Name: "Fix issue", + }, workitem.WorkItem{ + Ref: workitem.Ref{Provider: "plane", Tenant: "general", Project: "project-1", ID: "work-1"}, + Title: "Fix issue", }) if err != nil { t.Fatalf("buildPlaneCreateTaskInput returned error: %v", err) @@ -62,10 +90,12 @@ func TestBuildPlaneCreateTaskInputStoresExternalMetadata(t *testing.T) { if err := json.Unmarshal(input.External.Metadata, &metadata); err != nil { t.Fatalf("decode metadata: %v", err) } - if metadata["workspace_slug"] != "general" || - metadata["project_id"] != "project-1" || - metadata["work_item_id"] != "work-1" || - metadata["state_id"] != "state-1" { + if metadata["provider"] != "plane" || + metadata["tenant"] != "general" || + metadata["project"] != "project-1" || + metadata["id"] != "work-1" || + metadata["state_id"] != "state-1" || + metadata["external_url"] != "https://plane.example/work-1" { t.Fatalf("unexpected metadata: %#v", metadata) } } diff --git a/services/core/internal/workitem/provider.go b/services/core/internal/workitem/provider.go index cd0da55..4183e3b 100644 --- a/services/core/internal/workitem/provider.go +++ b/services/core/internal/workitem/provider.go @@ -5,6 +5,8 @@ import ( "encoding/json" "errors" "strings" + + "github.com/nomadcode/nomadcode-core/internal/workflow" ) var ErrInvalidRef = errors.New("invalid workitem ref") @@ -116,6 +118,69 @@ func (m Mapping) LabelTarget(canonical string) (string, bool) { return target, target != "" } +type TaskCreateInput struct { + Ref Ref + StateID string + Comment string +} + +func firstNonEmpty(ss ...string) string { + for _, s := range ss { + if trimmed := strings.TrimSpace(s); trimmed != "" { + return trimmed + } + } + return "" +} + +func BuildCreateTaskInput(input TaskCreateInput, item WorkItem) (workflow.CreateTaskInput, error) { + ref, err := NormalizeRef(input.Ref) + if err != nil { + return workflow.CreateTaskInput{}, err + } + + title := strings.TrimSpace(item.Title) + if title == "" { + title = ref.ID + } + + message := firstNonEmpty(input.Comment, item.DescriptionText, item.DescriptionHTML, title) + + metadata := map[string]string{ + "provider": string(ref.Provider), + "tenant": ref.Tenant, + "project": ref.Project, + "id": ref.ID, + "state_id": strings.TrimSpace(input.StateID), + "external_url": ref.URL, + } + + payloadBytes, err := json.Marshal(map[string]any{ + "message": message, + "work_item": metadata, + }) + if err != nil { + return workflow.CreateTaskInput{}, err + } + + metadataBytes, err := json.Marshal(metadata) + if err != nil { + return workflow.CreateTaskInput{}, err + } + + return workflow.CreateTaskInput{ + Title: title, + Source: string(ref.Provider), + Payload: payloadBytes, + External: &workflow.ExternalRefInput{ + Provider: string(ref.Provider), + ID: ref.ID, + URL: ref.URL, + Metadata: metadataBytes, + }, + }, nil +} + func NormalizeRef(ref Ref) (Ref, error) { provider := ProviderID(strings.TrimSpace(string(ref.Provider))) id := strings.TrimSpace(ref.ID) diff --git a/services/core/internal/workitem/provider_test.go b/services/core/internal/workitem/provider_test.go index 9488433..2eecaf7 100644 --- a/services/core/internal/workitem/provider_test.go +++ b/services/core/internal/workitem/provider_test.go @@ -118,3 +118,165 @@ func TestMappingReturnsTrimmedProviderTargets(t *testing.T) { t.Error("LabelTarget missing: expected false") } } + +func TestBuildCreateTaskInputUsesCommentBeforeDescription(t *testing.T) { + input := TaskCreateInput{ + Ref: Ref{Provider: "plane", ID: "work-1", Tenant: "acme", Project: "proj-1"}, + StateID: "state-abc", + Comment: "my comment", + } + item := WorkItem{ + Ref: input.Ref, + Title: "Issue Title", + DescriptionText: "description text", + } + + out, err := BuildCreateTaskInput(input, item) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Title != "Issue Title" { + t.Errorf("title: got %q", out.Title) + } + if out.Source != "plane" { + t.Errorf("source: got %q", out.Source) + } + if out.External == nil || out.External.Provider != "plane" { + t.Errorf("external.provider: got %v", out.External) + } + + var payload map[string]any + if err := json.Unmarshal(out.Payload, &payload); err != nil { + t.Fatalf("payload unmarshal: %v", err) + } + if payload["message"] != "my comment" { + t.Errorf("message: got %v, want 'my comment'", payload["message"]) + } +} + +func TestBuildCreateTaskInputStoresProviderNeutralMetadata(t *testing.T) { + input := TaskCreateInput{ + Ref: Ref{Provider: "plane", ID: "work-2", Tenant: "acme", Project: "proj-2", URL: "https://example.com/work/2"}, + StateID: " state-xyz ", + } + item := WorkItem{ + Ref: input.Ref, + Title: "Task", + } + + out, err := BuildCreateTaskInput(input, item) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var payload map[string]any + if err := json.Unmarshal(out.Payload, &payload); err != nil { + t.Fatalf("payload unmarshal: %v", err) + } + wi, ok := payload["work_item"].(map[string]any) + if !ok { + t.Fatalf("work_item missing or wrong type: %v", payload["work_item"]) + } + + checks := map[string]string{ + "provider": "plane", + "tenant": "acme", + "project": "proj-2", + "id": "work-2", + "state_id": "state-xyz", + "external_url": "https://example.com/work/2", + } + for k, want := range checks { + if wi[k] != want { + t.Errorf("work_item[%q]: got %v, want %q", k, wi[k], want) + } + } + + var extMeta map[string]any + if err := json.Unmarshal(out.External.Metadata, &extMeta); err != nil { + t.Fatalf("external metadata unmarshal: %v", err) + } + for k, want := range checks { + if extMeta[k] != want { + t.Errorf("external.metadata[%q]: got %v, want %q", k, extMeta[k], want) + } + } +} + +func TestBuildCreateTaskInputSkipsWhitespaceMessageCandidates(t *testing.T) { + ref := Ref{Provider: "plane", ID: "work-4", Tenant: "acme", Project: "proj-1"} + + // whitespace comment -> falls back to DescriptionText, which is trimmed + out, err := BuildCreateTaskInput( + TaskCreateInput{Ref: ref, Comment: " "}, + WorkItem{Ref: ref, Title: "T", DescriptionText: " description text "}, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + var payload map[string]any + if err := json.Unmarshal(out.Payload, &payload); err != nil { + t.Fatalf("payload unmarshal: %v", err) + } + if payload["message"] != "description text" { + t.Errorf("message: got %v, want 'description text'", payload["message"]) + } + + // comment and DescriptionText both whitespace -> falls back to DescriptionHTML + out, err = BuildCreateTaskInput( + TaskCreateInput{Ref: ref, Comment: " "}, + WorkItem{Ref: ref, Title: "T", DescriptionText: " ", DescriptionHTML: "

html

"}, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if err := json.Unmarshal(out.Payload, &payload); err != nil { + t.Fatalf("payload unmarshal: %v", err) + } + if payload["message"] != "

html

" { + t.Errorf("message: got %v, want '

html

'", payload["message"]) + } + + // all candidates whitespace -> falls back to title (trimmed) + out, err = BuildCreateTaskInput( + TaskCreateInput{Ref: ref, Comment: " "}, + WorkItem{Ref: ref, Title: " My Title ", DescriptionText: " ", DescriptionHTML: " "}, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if err := json.Unmarshal(out.Payload, &payload); err != nil { + t.Fatalf("payload unmarshal: %v", err) + } + if payload["message"] != "My Title" { + t.Errorf("message: got %v, want 'My Title'", payload["message"]) + } +} + +func TestBuildCreateTaskInputFallsBackToRefIDAndRequiresValidRef(t *testing.T) { + input := TaskCreateInput{ + Ref: Ref{Provider: "plane", ID: "work-3"}, + } + item := WorkItem{ + Ref: input.Ref, + Title: " ", + } + + out, err := BuildCreateTaskInput(input, item) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Title != "work-3" { + t.Errorf("title fallback: got %q, want 'work-3'", out.Title) + } + + _, err = BuildCreateTaskInput(TaskCreateInput{Ref: Ref{Provider: "", ID: "work-3"}}, item) + if !errors.Is(err, ErrInvalidRef) { + t.Errorf("missing provider: expected ErrInvalidRef, got %v", err) + } + + _, err = BuildCreateTaskInput(TaskCreateInput{Ref: Ref{Provider: "plane", ID: ""}}, item) + if !errors.Is(err, ErrInvalidRef) { + t.Errorf("missing id: expected ErrInvalidRef, got %v", err) + } +}