From bef5727aef819bb885e31b1171d559cf3a759f7f Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 24 Apr 2026 19:36:27 +0200 Subject: [PATCH 1/9] [#74316] add JSON work package inspect Implement the first agent-facing CLI slice for work package inspection. Add machine-readable JSON output, include direct children on demand, preserve custom field data and schema labels, and return structured JSON errors for the inspect command. https://community.openproject.org/projects/cli/work_packages/74316 --- cmd/inspect/inspect.go | 16 +- cmd/inspect/work_package.go | 67 ++++- cmd/inspect/work_package_test.go | 226 +++++++++++++++ components/presenter/json.go | 16 ++ components/presenter/json_test.go | 66 +++++ components/resources/work_packages/details.go | 141 ++++++++++ .../resources/work_packages/details_test.go | 266 ++++++++++++++++++ components/resources/work_packages/schema.go | 50 ++++ dtos/project.go | 12 +- dtos/work_package.go | 60 +++- dtos/work_package_schema.go | 39 +++ dtos/work_package_test.go | 61 ++++ models/project.go | 5 +- models/work_package_details.go | 42 +++ 14 files changed, 1053 insertions(+), 14 deletions(-) create mode 100644 cmd/inspect/work_package_test.go create mode 100644 components/presenter/json.go create mode 100644 components/presenter/json_test.go create mode 100644 components/resources/work_packages/details.go create mode 100644 components/resources/work_packages/details_test.go create mode 100644 components/resources/work_packages/schema.go create mode 100644 dtos/work_package_schema.go create mode 100644 dtos/work_package_test.go create mode 100644 models/work_package_details.go diff --git a/cmd/inspect/inspect.go b/cmd/inspect/inspect.go index b956ab4..35592d3 100644 --- a/cmd/inspect/inspect.go +++ b/cmd/inspect/inspect.go @@ -29,7 +29,21 @@ func init() { &listAvailableTypes, "types", false, - "List the available types on the work package.", + "List the available types on the work package", + ) + + inspectWorkPackageCmd.Flags().BoolVar( + &includeChildrenInJson, + "children", + false, + "Include direct child work packages in the output", + ) + + inspectWorkPackageCmd.Flags().BoolVar( + &printWorkPackageAsJSON, + "json", + false, + "Print machine-readable JSON output", ) RootCmd.AddCommand(inspectProjectCmd, inspectWorkPackageCmd) diff --git a/cmd/inspect/work_package.go b/cmd/inspect/work_package.go index a3e7349..affe696 100644 --- a/cmd/inspect/work_package.go +++ b/cmd/inspect/work_package.go @@ -7,13 +7,17 @@ import ( "github.com/spf13/cobra" "github.com/opf/openproject-cli/components/launch" + "github.com/opf/openproject-cli/components/presenter" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/work_packages" "github.com/opf/openproject-cli/components/routes" + "github.com/opf/openproject-cli/models" ) var shouldOpenWorkPackageInBrowser bool var listAvailableTypes bool +var includeChildrenInJson bool +var printWorkPackageAsJSON bool var inspectWorkPackageCmd = &cobra.Command{ Use: "workpackage [id]", @@ -25,13 +29,18 @@ var inspectWorkPackageCmd = &cobra.Command{ func inspectWorkPackage(_ *cobra.Command, args []string) { if len(args) != 1 { - printer.ErrorText(fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) + printInspectError("invalid_argument", fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) return } id, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - printer.ErrorText(fmt.Sprintf("'%s' is an invalid work package id. Must be a number.", args[0])) + printInspectError("invalid_argument", fmt.Sprintf("'%s' is an invalid work package id. Must be a number.", args[0])) + return + } + + if err := validateInspectWorkPackageFlags(); err != nil { + printInspectError("conflicting_arguments", err.Error()) return } @@ -43,6 +52,29 @@ func inspectWorkPackage(_ *cobra.Command, args []string) { return } + if printWorkPackageAsJSON { + var payload *models.WorkPackageInspectPayload + var err error + if includeChildrenInJson { + payload, err = work_packages.InspectWithChildren(id) + } else { + payload, err = work_packages.Inspect(id) + } + if err != nil { + printInspectError("api_error", err.Error()) + return + } + + data, err := presenter.MarshalJSON(payload) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) + return + } + workPackage, err := work_packages.Lookup(id) if err != nil { printer.Error(err) @@ -72,3 +104,34 @@ func listTypes(id uint64) { func hasListingFlag() bool { return listAvailableTypes } + +func validateInspectWorkPackageFlags() error { + if shouldOpenWorkPackageInBrowser && printWorkPackageAsJSON { + return fmt.Errorf("cannot use --open together with --json") + } + + if includeChildrenInJson && !printWorkPackageAsJSON { + return fmt.Errorf("cannot use --children without --json") + } + + if listAvailableTypes && (printWorkPackageAsJSON || includeChildrenInJson) { + return fmt.Errorf("cannot use --types together with --json or --children") + } + + return nil +} + +func printInspectError(code, message string) { + if !printWorkPackageAsJSON { + printer.ErrorText(message) + return + } + + data, err := presenter.MarshalError(code, message) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) +} diff --git a/cmd/inspect/work_package_test.go b/cmd/inspect/work_package_test.go new file mode 100644 index 0000000..1d81536 --- /dev/null +++ b/cmd/inspect/work_package_test.go @@ -0,0 +1,226 @@ +package inspect + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/routes" +) + +func TestInspectWorkPackagePrintsJSONWithChildren(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "description": {"raw": "Body"}, + "customField130": 3, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{"customField130": {"name": "Votes", "type": "Integer", "writable": true}}`) + case "/api/v3/work_packages": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 74413, + "subject": "Build a reusable SKILL.md based on OpenProject CLI", + "_links": { + "type": {"title": "Implementation"}, + "status": {"title": "new"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "parent": {"href": "/api/v3/work_packages/74316", "title": "Expand op CLI to support scripted work package workflows"} + } + } + ] + }, + "_type": "Collection", + "total": 1, + "count": 1, + "pageSize": -1, + "offset": 1 + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + shouldOpenWorkPackageInBrowser = false + listAvailableTypes = false + includeChildrenInJson = true + printWorkPackageAsJSON = true + + inspectWorkPackage(nil, []string{"74316"}) + + expected := "{\"work_package\":{\"id\":74316,\"subject\":\"Expand op CLI to support scripted work package workflows\",\"type\":\"Feature\",\"status\":\"new\",\"assignee\":\"\",\"description\":\"Body\",\"parent_id\":null,\"project\":{\"id\":1482,\"identifier\":\"cli\",\"name\":\"CLI\"},\"fields\":{\"customField130\":3},\"field_labels\":{\"Votes\":[\"customField130\"]}},\"children\":[{\"id\":74413,\"subject\":\"Build a reusable SKILL.md based on OpenProject CLI\",\"type\":\"Implementation\",\"status\":\"new\",\"parent_id\":74316}]}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestInspectWorkPackagePrintsJSONWithoutChildrenQuery(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "description": {"raw": "Body"}, + "customField130": 3, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{"customField130": {"name": "Votes", "type": "Integer", "writable": true}}`) + case "/api/v3/work_packages": + t.Fatalf("unexpected children query: %s", r.URL.Path) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + shouldOpenWorkPackageInBrowser = false + listAvailableTypes = false + includeChildrenInJson = false + printWorkPackageAsJSON = true + + inspectWorkPackage(nil, []string{"74316"}) + + expected := "{\"work_package\":{\"id\":74316,\"subject\":\"Expand op CLI to support scripted work package workflows\",\"type\":\"Feature\",\"status\":\"new\",\"assignee\":\"\",\"description\":\"Body\",\"parent_id\":null,\"project\":{\"id\":1482,\"identifier\":\"cli\",\"name\":\"CLI\"},\"fields\":{\"customField130\":3},\"field_labels\":{\"Votes\":[\"customField130\"]}},\"children\":[]}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestValidateInspectWorkPackageFlagsRejectsOpenAndJSON(t *testing.T) { + shouldOpenWorkPackageInBrowser = true + listAvailableTypes = false + includeChildrenInJson = false + printWorkPackageAsJSON = true + + err := validateInspectWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestValidateInspectWorkPackageFlagsRejectsTypesAndJSON(t *testing.T) { + shouldOpenWorkPackageInBrowser = false + listAvailableTypes = true + includeChildrenInJson = false + printWorkPackageAsJSON = true + + err := validateInspectWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestInspectWorkPackagePrintsJSONErrorForFlagConflict(t *testing.T) { + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + shouldOpenWorkPackageInBrowser = true + listAvailableTypes = false + includeChildrenInJson = false + printWorkPackageAsJSON = true + + inspectWorkPackage(nil, []string{"74316"}) + + expected := "{\"error\":{\"code\":\"conflicting_arguments\",\"message\":\"cannot use --open together with --json\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestInspectWorkPackagePrintsJSONErrorForChildrenWithoutJSON(t *testing.T) { + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + shouldOpenWorkPackageInBrowser = false + listAvailableTypes = false + includeChildrenInJson = true + printWorkPackageAsJSON = false + + inspectWorkPackage(nil, []string{"42"}) + + expected := "\u001b[31m[ERROR]\u001b[0m cannot use --children without --json\n" + if activePrinter.Result != expected { + t.Fatalf("expected %q, got %q", expected, activePrinter.Result) + } +} + +func TestInspectWorkPackagePrintsJSONErrorForTypesAndJSON(t *testing.T) { + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + shouldOpenWorkPackageInBrowser = false + listAvailableTypes = true + includeChildrenInJson = false + printWorkPackageAsJSON = true + + inspectWorkPackage(nil, []string{"74316"}) + + expected := "{\"error\":{\"code\":\"conflicting_arguments\",\"message\":\"cannot use --types together with --json or --children\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} diff --git a/components/presenter/json.go b/components/presenter/json.go new file mode 100644 index 0000000..6215618 --- /dev/null +++ b/components/presenter/json.go @@ -0,0 +1,16 @@ +package presenter + +import "encoding/json" + +func MarshalJSON(value any) ([]byte, error) { + return json.Marshal(value) +} + +func MarshalError(code, message string) ([]byte, error) { + return MarshalJSON(map[string]any{ + "error": map[string]string{ + "code": code, + "message": message, + }, + }) +} diff --git a/components/presenter/json_test.go b/components/presenter/json_test.go new file mode 100644 index 0000000..44010f2 --- /dev/null +++ b/components/presenter/json_test.go @@ -0,0 +1,66 @@ +package presenter_test + +import ( + "testing" + + "github.com/opf/openproject-cli/components/presenter" + "github.com/opf/openproject-cli/models" +) + +func TestInspectPayloadJSON(t *testing.T) { + parentID := uint64(74316) + + payload := models.WorkPackageInspectPayload{ + WorkPackage: models.WorkPackageDetails{ + ID: 74316, + Subject: "Expand op CLI to support scripted work package workflows", + Type: "Feature", + Status: "new", + Assignee: "", + Description: "Body", + Project: models.ProjectRef{ + ID: 1482, + Identifier: "cli", + Name: "CLI", + }, + Fields: map[string]any{ + "customField130": float64(3), + }, + FieldLabels: map[string][]string{ + "Votes": []string{"customField130"}, + }, + }, + Children: []models.WorkPackageSummary{ + { + ID: 74413, + Subject: "Build a reusable SKILL.md based on OpenProject CLI", + Type: "Implementation", + Status: "new", + ParentID: &parentID, + }, + }, + } + + got, err := presenter.MarshalJSON(payload) + if err != nil { + t.Fatal(err) + } + + expected := `{"work_package":{"id":74316,"subject":"Expand op CLI to support scripted work package workflows","type":"Feature","status":"new","assignee":"","description":"Body","parent_id":null,"project":{"id":1482,"identifier":"cli","name":"CLI"},"fields":{"customField130":3},"field_labels":{"Votes":["customField130"]}},"children":[{"id":74413,"subject":"Build a reusable SKILL.md based on OpenProject CLI","type":"Implementation","status":"new","parent_id":74316}]}` + + if string(got) != expected { + t.Fatalf("expected %s, got %s", expected, got) + } +} + +func TestErrorJSON(t *testing.T) { + got, err := presenter.MarshalError("conflicting_arguments", "cannot use --open together with --json") + if err != nil { + t.Fatal(err) + } + + expected := `{"error":{"code":"conflicting_arguments","message":"cannot use --open together with --json"}}` + if string(got) != expected { + t.Fatalf("expected %s, got %s", expected, got) + } +} diff --git a/components/resources/work_packages/details.go b/components/resources/work_packages/details.go new file mode 100644 index 0000000..82021de --- /dev/null +++ b/components/resources/work_packages/details.go @@ -0,0 +1,141 @@ +package work_packages + +import ( + "strconv" + + "github.com/opf/openproject-cli/components/parser" + "github.com/opf/openproject-cli/components/paths" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/dtos" + "github.com/opf/openproject-cli/models" +) + +func Inspect(id uint64) (*models.WorkPackageInspectPayload, error) { + workPackage, err := fetch(id) + if err != nil { + return nil, err + } + + schema, err := SchemaFor(workPackage) + if err != nil { + return nil, err + } + + return &models.WorkPackageInspectPayload{ + WorkPackage: workPackageDetails(workPackage, schema), + Children: []models.WorkPackageSummary{}, + }, nil +} + +func InspectWithChildren(id uint64) (*models.WorkPackageInspectPayload, error) { + payload, err := Inspect(id) + if err != nil { + return nil, err + } + + children, err := children(id) + if err != nil { + return nil, err + } + + payload.Children = children + return payload, nil +} + +func children(parentID uint64) ([]models.WorkPackageSummary, error) { + query := requests.NewUnpaginatedQuery(nil, []requests.Filter{ParentFilter(parentID)}) + response, err := requests.Get(paths.WorkPackages(), &query) + if err != nil { + return nil, err + } + + collection := parser.Parse[dtos.WorkPackageCollectionDto](response) + items := make([]models.WorkPackageSummary, 0, len(collection.Embedded.Elements)) + for _, element := range collection.Embedded.Elements { + items = append(items, workPackageSummary(element)) + } + + return items, nil +} + +func workPackageDetails(dto *dtos.WorkPackageDto, schema *Schema) models.WorkPackageDetails { + project := models.ProjectRef{} + if dto.Embedded != nil && dto.Embedded.Project != nil { + project = models.ProjectRef{ + ID: uint64(dto.Embedded.Project.Id), + Identifier: dto.Embedded.Project.Identifier, + Name: dto.Embedded.Project.Name, + } + } else if dto.Links != nil && dto.Links.Project != nil { + project = models.ProjectRef{ + ID: parser.IdFromLink(dto.Links.Project.Href), + Name: dto.Links.Project.Title, + } + } + + return models.WorkPackageDetails{ + ID: uint64(dto.Id), + Subject: dto.Subject, + Type: linkTitle(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Type }), + Status: linkTitle(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Status }), + Assignee: linkTitle(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Assignee }), + Description: longTextRaw(dto.Description), + ParentID: linkID(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Parent }), + Project: project, + Fields: dto.CustomFields, + FieldLabels: schema.fieldLabels(), + } +} + +func workPackageSummary(dto *dtos.WorkPackageDto) models.WorkPackageSummary { + return models.WorkPackageSummary{ + ID: uint64(dto.Id), + Subject: dto.Subject, + Type: linkTitle(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Type }), + Status: linkTitle(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Status }), + ParentID: linkID(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Parent }), + } +} + +func linkTitle(links *dtos.WorkPackageLinksDto, selector func(*dtos.WorkPackageLinksDto) *dtos.LinkDto) string { + if links == nil { + return "" + } + + link := selector(links) + if link == nil { + return "" + } + + return link.Title +} + +func longTextRaw(description *dtos.LongTextDto) string { + if description == nil { + return "" + } + + return description.Raw +} + +func linkID(links *dtos.WorkPackageLinksDto, selector func(*dtos.WorkPackageLinksDto) *dtos.LinkDto) *uint64 { + if links == nil { + return nil + } + + link := selector(links) + if link == nil || link.Href == "" { + return nil + } + + id := parser.IdFromLink(link.Href) + return &id +} + +func ParentFilter(parentID uint64) requests.Filter { + return requests.Filter{ + Operator: "=", + Name: "parent", + Values: []string{strconv.FormatUint(parentID, 10)}, + } +} diff --git a/components/resources/work_packages/details_test.go b/components/resources/work_packages/details_test.go new file mode 100644 index 0000000..f6cd1ae --- /dev/null +++ b/components/resources/work_packages/details_test.go @@ -0,0 +1,266 @@ +package work_packages_test + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "sort" + "strings" + "testing" + + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestInspectWithChildren(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "description": {"raw": "Body"}, + "customField130": 3, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case r.URL.Path == "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true} + }`) + case r.URL.Path == "/api/v3/work_packages": + if !strings.Contains(r.URL.RawQuery, "parent") { + t.Fatalf("expected parent filter in query: %s", r.URL.RawQuery) + } + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 74413, + "subject": "Build a reusable SKILL.md based on OpenProject CLI", + "_links": { + "type": {"title": "Implementation"}, + "status": {"title": "new"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "parent": {"href": "/api/v3/work_packages/74316", "title": "Expand op CLI to support scripted work package workflows"} + } + } + ] + }, + "_type": "Collection", + "total": 1, + "count": 1, + "pageSize": -1, + "offset": 1 + }`) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + payload, err := work_packages.InspectWithChildren(74316) + if err != nil { + t.Fatal(err) + } + + if payload.WorkPackage.Project.Identifier != "cli" { + t.Fatalf("expected project identifier cli, got %q", payload.WorkPackage.Project.Identifier) + } + + if payload.WorkPackage.Fields["customField130"] != float64(3) { + t.Fatalf("expected custom field value 3, got %#v", payload.WorkPackage.Fields["customField130"]) + } + + labels := payload.WorkPackage.FieldLabels["Votes"] + if len(labels) != 1 || labels[0] != "customField130" { + t.Fatalf("expected Votes label mapping, got %#v", payload.WorkPackage.FieldLabels) + } + + if len(payload.Children) != 1 { + t.Fatalf("expected one child, got %d", len(payload.Children)) + } + + if payload.Children[0].ID != 74413 { + t.Fatalf("expected child 74413, got %+v", payload.Children[0]) + } +} + +func TestInspectDoesNotQueryChildren(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "description": {"raw": "Body"}, + "customField130": 3, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true} + }`) + case "/api/v3/work_packages": + t.Fatalf("unexpected children query: %s", r.URL.Path) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + payload, err := work_packages.Inspect(74316) + if err != nil { + t.Fatal(err) + } + + if len(payload.Children) != 0 { + t.Fatalf("expected no children, got %+v", payload.Children) + } +} + +func TestInspectReturnsErrorForMissingWorkPackage(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"message":"not found"}`) + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + _, err = work_packages.Inspect(999999) + if err == nil { + t.Fatal("expected error for missing work package, got nil") + } +} + +func TestInspectReturnsErrorWhenSchemaFetchFails(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Any", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, `{"message":"boom"}`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + _, err = work_packages.Inspect(74316) + if err == nil { + t.Fatal("expected error when schema endpoint fails, got nil") + } +} + +func TestInspectPreservesDuplicateFieldLabels(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Any", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField17": {"name": "KPI", "type": "Integer", "writable": true}, + "customField22": {"name": "KPI", "type": "Integer", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + payload, err := work_packages.Inspect(74316) + if err != nil { + t.Fatal(err) + } + + labels := payload.WorkPackage.FieldLabels["KPI"] + sort.Strings(labels) + if len(labels) != 2 || labels[0] != "customField17" || labels[1] != "customField22" { + t.Fatalf("expected duplicate KPI mappings, got %#v", payload.WorkPackage.FieldLabels) + } +} diff --git a/components/resources/work_packages/schema.go b/components/resources/work_packages/schema.go new file mode 100644 index 0000000..884b59a --- /dev/null +++ b/components/resources/work_packages/schema.go @@ -0,0 +1,50 @@ +package work_packages + +import ( + "github.com/opf/openproject-cli/components/parser" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/dtos" +) + +type SchemaField struct { + APIName string + Label string + Type string + Writable bool +} + +type Schema struct { + Fields []SchemaField +} + +func SchemaFor(workPackage *dtos.WorkPackageDto) (*Schema, error) { + if workPackage == nil || workPackage.Links == nil || workPackage.Links.Schema == nil { + return &Schema{}, nil + } + + response, err := requests.Get(workPackage.Links.Schema.Href, nil) + if err != nil { + return nil, err + } + + dto := parser.Parse[dtos.WorkPackageSchemaDto](response) + fields := make([]SchemaField, 0, len(dto.Fields)) + for apiName, field := range dto.Fields { + fields = append(fields, SchemaField{ + APIName: apiName, + Label: field.Name, + Type: field.Type, + Writable: field.Writable, + }) + } + + return &Schema{Fields: fields}, nil +} + +func (schema *Schema) fieldLabels() map[string][]string { + labels := make(map[string][]string) + for _, field := range schema.Fields { + labels[field.Label] = append(labels[field.Label], field.APIName) + } + return labels +} diff --git a/dtos/project.go b/dtos/project.go index e660c34..e9d27bd 100644 --- a/dtos/project.go +++ b/dtos/project.go @@ -3,9 +3,10 @@ package dtos import "github.com/opf/openproject-cli/models" type ProjectDto struct { - Id int64 `json:"id"` - Name string `json:"name"` - Links *projectLinks `json:"_links"` + Id int64 `json:"id"` + Identifier string `json:"identifier"` + Name string `json:"name"` + Links *projectLinks `json:"_links"` } type projectLinks struct { @@ -36,7 +37,8 @@ func (dto *ProjectCollectionDto) Convert() []*models.Project { func (dto *ProjectDto) Convert() *models.Project { return &models.Project{ - Id: uint64(dto.Id), - Name: dto.Name, + Id: uint64(dto.Id), + Identifier: dto.Identifier, + Name: dto.Name, } } diff --git a/dtos/work_package.go b/dtos/work_package.go index 8110746..ebd68bb 100644 --- a/dtos/work_package.go +++ b/dtos/work_package.go @@ -1,6 +1,9 @@ package dtos import ( + "encoding/json" + "strings" + "github.com/opf/openproject-cli/models" ) @@ -9,6 +12,8 @@ type WorkPackageLinksDto struct { AddAttachment *LinkDto `json:"addAttachment,omitempty"` Status *LinkDto `json:"status,omitempty"` Project *LinkDto `json:"project,omitempty"` + Parent *LinkDto `json:"parent,omitempty"` + Schema *LinkDto `json:"schema,omitempty"` Assignee *LinkDto `json:"assignee,omitempty"` Type *LinkDto `json:"type,omitempty"` CustomActions []*LinkDto `json:"customActions,omitempty"` @@ -22,10 +27,12 @@ type WorkPackageDto struct { Description *LongTextDto `json:"description,omitempty"` Embedded *embeddedDto `json:"_embedded,omitempty"` LockVersion int `json:"lockVersion,omitempty"` + CustomFields map[string]any `json:"-"` } type embeddedDto struct { CustomActions []*CustomActionDto `json:"customActions"` + Project *ProjectDto `json:"project,omitempty"` } type workPackageElements struct { @@ -47,14 +54,38 @@ type CreateWorkPackageDto struct { /////////////// MODEL CONVERSION /////////////// +func (dto *WorkPackageDto) UnmarshalJSON(data []byte) error { + type alias WorkPackageDto + var base alias + if err := json.Unmarshal(data, &base); err != nil { + return err + } + + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + customFields := make(map[string]any) + for key, value := range raw { + if strings.HasPrefix(key, "customField") { + customFields[key] = value + } + } + + *dto = WorkPackageDto(base) + dto.CustomFields = customFields + return nil +} + func (dto *WorkPackageDto) Convert() *models.WorkPackage { return &models.WorkPackage{ Id: uint64(dto.Id), Subject: dto.Subject, - Type: dto.Links.Type.Title, - Assignee: dto.Links.Assignee.Title, - Status: dto.Links.Status.Title, - Description: dto.Description.Raw, + Type: linkTitle(dto.Links, func(links *WorkPackageLinksDto) *LinkDto { return links.Type }), + Assignee: linkTitle(dto.Links, func(links *WorkPackageLinksDto) *LinkDto { return links.Assignee }), + Status: linkTitle(dto.Links, func(links *WorkPackageLinksDto) *LinkDto { return links.Status }), + Description: longTextRaw(dto.Description), LockVersion: dto.LockVersion, } } @@ -74,3 +105,24 @@ func (dto *WorkPackageCollectionDto) Convert() *models.WorkPackageCollection { Items: workPackages, } } + +func linkTitle(links *WorkPackageLinksDto, selector func(*WorkPackageLinksDto) *LinkDto) string { + if links == nil { + return "" + } + + link := selector(links) + if link == nil { + return "" + } + + return link.Title +} + +func longTextRaw(description *LongTextDto) string { + if description == nil { + return "" + } + + return description.Raw +} diff --git a/dtos/work_package_schema.go b/dtos/work_package_schema.go new file mode 100644 index 0000000..6f6720b --- /dev/null +++ b/dtos/work_package_schema.go @@ -0,0 +1,39 @@ +package dtos + +import ( + "encoding/json" + "strings" +) + +type WorkPackageSchemaFieldDto struct { + Name string `json:"name"` + Type string `json:"type"` + Writable bool `json:"writable"` +} + +type WorkPackageSchemaDto struct { + Fields map[string]WorkPackageSchemaFieldDto `json:"-"` +} + +func (dto *WorkPackageSchemaDto) UnmarshalJSON(data []byte) error { + var raw map[string]json.RawMessage + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + fields := make(map[string]WorkPackageSchemaFieldDto) + for key, value := range raw { + if !strings.HasPrefix(key, "customField") { + continue + } + + var field WorkPackageSchemaFieldDto + if err := json.Unmarshal(value, &field); err != nil { + return err + } + fields[key] = field + } + + dto.Fields = fields + return nil +} diff --git a/dtos/work_package_test.go b/dtos/work_package_test.go new file mode 100644 index 0000000..fe6e2e4 --- /dev/null +++ b/dtos/work_package_test.go @@ -0,0 +1,61 @@ +package dtos + +import ( + "encoding/json" + "testing" +) + +func TestWorkPackageDtoUnmarshalCapturesInspectFields(t *testing.T) { + body := []byte(`{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "description": {"raw": "Body"}, + "customField130": 3, + "customField108": false, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "parent": {"href": "/api/v3/work_packages/70000", "title": "Umbrella"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + + var dto WorkPackageDto + if err := json.Unmarshal(body, &dto); err != nil { + t.Fatal(err) + } + + if dto.Embedded == nil || dto.Embedded.Project == nil { + t.Fatalf("expected embedded project, got %+v", dto.Embedded) + } + + if dto.Embedded.Project.Identifier != "cli" { + t.Fatalf("expected project identifier cli, got %q", dto.Embedded.Project.Identifier) + } + + if dto.Links == nil || dto.Links.Parent == nil || dto.Links.Parent.Href != "/api/v3/work_packages/70000" { + t.Fatalf("expected parent link to be captured, got %+v", dto.Links) + } + + if dto.Links == nil || dto.Links.Schema == nil || dto.Links.Schema.Href != "/api/v3/work_packages/schemas/1482-6" { + t.Fatalf("expected schema link to be captured, got %+v", dto.Links) + } + + if dto.CustomFields["customField130"] != float64(3) { + t.Fatalf("expected customField130 to be captured, got %#v", dto.CustomFields["customField130"]) + } + + if dto.CustomFields["customField108"] != false { + t.Fatalf("expected customField108 to be captured, got %#v", dto.CustomFields["customField108"]) + } +} diff --git a/models/project.go b/models/project.go index 964d214..9ae9a9d 100644 --- a/models/project.go +++ b/models/project.go @@ -1,6 +1,7 @@ package models type Project struct { - Id uint64 - Name string + Id uint64 + Identifier string + Name string } diff --git a/models/work_package_details.go b/models/work_package_details.go new file mode 100644 index 0000000..7f4db15 --- /dev/null +++ b/models/work_package_details.go @@ -0,0 +1,42 @@ +package models + +type ProjectRef struct { + ID uint64 `json:"id"` + Identifier string `json:"identifier"` + Name string `json:"name"` +} + +type WorkPackageSummary struct { + ID uint64 `json:"id"` + Subject string `json:"subject"` + Type string `json:"type"` + Status string `json:"status"` + ParentID *uint64 `json:"parent_id"` +} + +type WorkPackageDetails struct { + ID uint64 `json:"id"` + Subject string `json:"subject"` + Type string `json:"type"` + Status string `json:"status"` + Assignee string `json:"assignee"` + Description string `json:"description"` + ParentID *uint64 `json:"parent_id"` + Project ProjectRef `json:"project"` + Fields map[string]any `json:"fields"` + FieldLabels map[string][]string `json:"field_labels"` +} + +type WorkPackageInspectPayload struct { + WorkPackage WorkPackageDetails `json:"work_package"` + Children []WorkPackageSummary `json:"children"` +} + +type ErrorPayload struct { + Error ErrorDetails `json:"error"` +} + +type ErrorDetails struct { + Code string `json:"code"` + Message string `json:"message"` +} From 1dcf493cf6356917532b90cc81f7e6fca88071d6 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 24 Apr 2026 19:47:33 +0200 Subject: [PATCH 2/9] [#74316] add dry-run JSON WP mutations Add parent-aware create dry-runs and schema-driven update dry-runs for work packages, both with machine-readable JSON output. Keep the legacy typed update flags intact while adding --set, --parent, --dry-run, and --json as the agent workflow surface. https://community.openproject.org/projects/cli/work_packages/74316 --- README.md | 7 + cmd/create/create.go | 22 +- cmd/create/work_package.go | 91 +++- cmd/create/work_package_test.go | 328 ++++++++++++ cmd/update/update.go | 18 + cmd/update/work_package.go | 216 +++++++- cmd/update/work_package_test.go | 478 ++++++++++++++++++ components/errors/errors.go | 7 + components/resources/work_packages/create.go | 147 +++++- .../resources/work_packages/create_test.go | 136 +++++ components/resources/work_packages/details.go | 10 - .../work_packages/field_resolution.go | 126 +++++ .../work_packages/field_resolution_test.go | 67 +++ components/resources/work_packages/filters.go | 9 + components/resources/work_packages/schema.go | 6 + .../resources/work_packages/schema_test.go | 64 +++ components/resources/work_packages/update.go | 68 ++- .../resources/work_packages/update_fields.go | 68 +++ .../work_packages/update_fields_test.go | 56 ++ dtos/work_package_schema_test.go | 35 ++ models/work_package_mutation.go | 32 ++ 21 files changed, 1923 insertions(+), 68 deletions(-) create mode 100644 cmd/create/work_package_test.go create mode 100644 cmd/update/work_package_test.go create mode 100644 components/resources/work_packages/create_test.go create mode 100644 components/resources/work_packages/field_resolution.go create mode 100644 components/resources/work_packages/field_resolution_test.go create mode 100644 components/resources/work_packages/schema_test.go create mode 100644 components/resources/work_packages/update_fields.go create mode 100644 components/resources/work_packages/update_fields_test.go create mode 100644 dtos/work_package_schema_test.go create mode 100644 models/work_package_mutation.go diff --git a/README.md b/README.md index c6685be..6770ea8 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,10 @@ op update workpackage 42 --subject 'The new subject' --status 'In Progress' --ty # Uploading an attachment to a work package op update workpackage 42 --attach ./Downloads/Report.pdf + +# Resolving and validating field updates as JSON before applying them +# This first slice supports schema-resolved custom fields via --set. +op update workpackage 74316 --set 'Votes=3' --dry-run --json ``` #### Inspecting @@ -182,6 +186,9 @@ op update workpackage 42 --attach ./Downloads/Report.pdf # Inspecting a work package with more details, # then in the work package list command op inspect workpackage 42 + +# Inspecting a work package and its direct children as machine-readable JSON +op inspect workpackage 74316 --children --json ``` ## Creating a release diff --git a/cmd/create/create.go b/cmd/create/create.go index e03767c..c5bbece 100644 --- a/cmd/create/create.go +++ b/cmd/create/create.go @@ -16,7 +16,6 @@ func init() { 0, "Project ID to create the work package in", ) - _ = createWorkPackageCmd.MarkFlagRequired("project") createWorkPackageCmd.Flags().BoolVarP( &shouldOpenWorkPackageInBrowser, @@ -34,5 +33,26 @@ func init() { "Change the work package type", ) + createWorkPackageCmd.Flags().Uint64Var( + &parentWorkPackageID, + "parent", + 0, + "Create the work package as a child of an existing work package", + ) + + createWorkPackageCmd.Flags().BoolVar( + &printCreatedWorkPackageAsJSON, + "json", + false, + "Print machine-readable JSON output", + ) + + createWorkPackageCmd.Flags().BoolVar( + &dryRunCreateWorkPackage, + "dry-run", + false, + "Resolve and validate without persisting the work package", + ) + RootCmd.AddCommand(createWorkPackageCmd) } diff --git a/cmd/create/work_package.go b/cmd/create/work_package.go index 5e1bee4..a745fd2 100644 --- a/cmd/create/work_package.go +++ b/cmd/create/work_package.go @@ -5,14 +5,19 @@ import ( "github.com/spf13/cobra" + componentErrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/launch" + "github.com/opf/openproject-cli/components/presenter" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/work_packages" "github.com/opf/openproject-cli/components/routes" ) var projectId uint64 +var parentWorkPackageID uint64 var shouldOpenWorkPackageInBrowser bool +var printCreatedWorkPackageAsJSON bool +var dryRunCreateWorkPackage bool var typeFlag string var createWorkPackageCmd = &cobra.Command{ @@ -24,14 +29,55 @@ var createWorkPackageCmd = &cobra.Command{ func createWorkPackage(_ *cobra.Command, args []string) { if len(args) != 1 { - printer.ErrorText(fmt.Sprintf("Expected 1 argument [subject], but got %d", len(args))) + printCreateError("invalid_argument", fmt.Sprintf("Expected 1 argument [subject], but got %d", len(args))) + return + } + + if err := validateCreateWorkPackageFlags(); err != nil { + printCreateError("conflicting_arguments", err.Error()) return } subject := args[0] - workPackage, err := work_packages.Create(projectId, createOptions(subject)) + options := createOptions(subject) + + if dryRunCreateWorkPackage { + plan, err := work_packages.DryRunCreate(projectId, options) + if err != nil { + printCreateError(createErrorCode(err), err.Error()) + return + } + + data, err := presenter.MarshalJSON(plan) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) + return + } + + workPackage, err := work_packages.Create(projectId, options) if err != nil { - printer.Error(err) + printCreateError(createErrorCode(err), err.Error()) + return + } + + if printCreatedWorkPackageAsJSON { + payload, err := work_packages.Inspect(workPackage.Id) + if err != nil { + printCreateError("post_apply_inspect_failed", err.Error()) + return + } + + data, err := presenter.MarshalJSON(payload) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) return } @@ -54,5 +100,44 @@ func createOptions(subject string) map[work_packages.CreateOption]string { options[work_packages.CreateType] = typeFlag } + if parentWorkPackageID > 0 { + options[work_packages.CreateParent] = fmt.Sprintf("%d", parentWorkPackageID) + } + return options } + +func validateCreateWorkPackageFlags() error { + if shouldOpenWorkPackageInBrowser && printCreatedWorkPackageAsJSON { + return fmt.Errorf("cannot use --open together with --json") + } + + if dryRunCreateWorkPackage && !printCreatedWorkPackageAsJSON { + return fmt.Errorf("cannot use --dry-run without --json") + } + + return nil +} + +func printCreateError(code, message string) { + if !printCreatedWorkPackageAsJSON { + printer.ErrorText(message) + return + } + + data, err := presenter.MarshalError(code, message) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) +} + +func createErrorCode(err error) string { + if componentErrors.IsCustom(err) { + return "invalid_argument" + } + + return "api_error" +} diff --git a/cmd/create/work_package_test.go b/cmd/create/work_package_test.go new file mode 100644 index 0000000..c249081 --- /dev/null +++ b/cmd/create/work_package_test.go @@ -0,0 +1,328 @@ +package create + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/routes" +) + +func TestCreateWorkPackagePrintsDryRunJSONWithParent(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 0 + parentWorkPackageID = 74316 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = true + typeFlag = "" + + createWorkPackage(nil, []string{"Build reusable skill"}) + + expected := "{\"valid\":true,\"operation\":\"create\",\"project_id\":1482,\"parent_id\":74316,\"work_package\":{\"subject\":\"Build reusable skill\",\"type\":\"\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestCreateWorkPackagePrintsJSONErrorForFlagConflict(t *testing.T) { + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 1482 + parentWorkPackageID = 0 + shouldOpenWorkPackageInBrowser = true + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = false + typeFlag = "" + + createWorkPackage(nil, []string{"Build reusable skill"}) + + expected := "{\"error\":{\"code\":\"conflicting_arguments\",\"message\":\"cannot use --open together with --json\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestValidateCreateWorkPackageFlagsRejectsDryRunWithoutJSON(t *testing.T) { + projectId = 1482 + parentWorkPackageID = 0 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = false + dryRunCreateWorkPackage = true + typeFlag = "" + + err := validateCreateWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestCreateWorkPackagePrintsJSONWithoutChildrenQuery(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "identifier": "cli", + "name": "CLI", + "_links": { + "types": {"href": "/api/v3/projects/1482/types/available"} + } + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 7, + "name": "Implementation", + "_links": { + "self": {"href": "/api/v3/types/7"} + } + } + ] + } + }`) + case "/api/v3/projects/1482/work_packages": + _, _ = io.WriteString(w, `{ + "id": 74415, + "subject": "Build reusable skill", + "_links": { + "self": {"href": "/api/v3/work_packages/74415"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + }, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + } + }`) + case "/api/v3/work_packages/74415": + _, _ = io.WriteString(w, `{ + "id": 74415, + "subject": "Build reusable skill", + "_links": { + "self": {"href": "/api/v3/work_packages/74415"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + }, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + } + }`) + case "/api/v3/work_packages": + t.Fatalf("unexpected children query: %s", r.URL.Path) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 1482 + parentWorkPackageID = 0 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = false + typeFlag = "Implementation" + + createWorkPackage(nil, []string{"Build reusable skill"}) + + expected := "{\"work_package\":{\"id\":74415,\"subject\":\"Build reusable skill\",\"type\":\"Implementation\",\"status\":\"new\",\"assignee\":\"\",\"description\":\"\",\"parent_id\":null,\"project\":{\"id\":1482,\"identifier\":\"cli\",\"name\":\"CLI\"},\"fields\":{},\"field_labels\":{}},\"children\":[]}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestCreateWorkPackagePrintsInvalidArgumentForValidationError(t *testing.T) { + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 0 + parentWorkPackageID = 0 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = false + typeFlag = "" + + createWorkPackage(nil, []string{"Build reusable skill"}) + + expected := "{\"error\":{\"code\":\"invalid_argument\",\"message\":\"either --project or --parent is required\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestCreateWorkPackagePrintsAPIErrorForDryRunParentLookupFailure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, `{"message":"boom"}`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 0 + parentWorkPackageID = 74316 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = true + typeFlag = "" + + createWorkPackage(nil, []string{"Build reusable skill"}) + + expected := "{\"error\":{\"code\":\"api_error\",\"message\":\"{\\\"message\\\":\\\"boom\\\"}\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestCreateWorkPackagePrintsPostApplyInspectFailure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "identifier": "cli", + "name": "CLI", + "_links": { + "types": {"href": "/api/v3/projects/1482/types/available"} + } + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 7, + "name": "Implementation", + "_links": { + "self": {"href": "/api/v3/types/7"} + } + } + ] + } + }`) + case "/api/v3/projects/1482/work_packages": + _, _ = io.WriteString(w, `{ + "id": 74415, + "subject": "Build reusable skill", + "_links": { + "self": {"href": "/api/v3/work_packages/74415"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/74415": + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, `{"message":"inspect failed"}`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 1482 + parentWorkPackageID = 0 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = false + typeFlag = "Implementation" + + createWorkPackage(nil, []string{"Build reusable skill"}) + + expected := "{\"error\":{\"code\":\"post_apply_inspect_failed\",\"message\":\"{\\\"message\\\":\\\"inspect failed\\\"}\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} diff --git a/cmd/update/update.go b/cmd/update/update.go index d6e9fc1..801199a 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -50,4 +50,22 @@ func addWorkPackageFlags() { "", "Change the work package type", ) + workPackageCmd.Flags().StringArrayVar( + &setFlags, + "set", + nil, + "Set a schema-resolved custom field using label=value or apiField=value", + ) + workPackageCmd.Flags().BoolVar( + &printUpdatedWorkPackageAsJSON, + "json", + false, + "Print machine-readable JSON output", + ) + workPackageCmd.Flags().BoolVar( + &dryRunUpdateWorkPackage, + "dry-run", + false, + "Resolve and validate without persisting the update", + ) } diff --git a/cmd/update/work_package.go b/cmd/update/work_package.go index ad80ec7..d5ff8de 100644 --- a/cmd/update/work_package.go +++ b/cmd/update/work_package.go @@ -1,21 +1,27 @@ package update import ( + "errors" "fmt" "strconv" + "strings" "github.com/spf13/cobra" + "github.com/opf/openproject-cli/components/presenter" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/work_packages" ) var ( - actionFlag string - assigneeFlag uint64 - attachFlag string - subjectFlag string - typeFlag string + actionFlag string + assigneeFlag uint64 + attachFlag string + dryRunUpdateWorkPackage bool + printUpdatedWorkPackageAsJSON bool + setFlags []string + subjectFlag string + typeFlag string ) var workPackageCmd = &cobra.Command{ @@ -28,22 +34,140 @@ provided by a flag is executed on its own.`, func updateWorkPackage(_ *cobra.Command, args []string) { if len(args) != 1 { - printer.ErrorText(fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) + printUpdateError("invalid_argument", fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) return } id, err := strconv.ParseUint(args[0], 10, 64) if err != nil { - printer.ErrorText(fmt.Sprintf("'%s' is an invalid work package id. Must be a number.", args[0])) + printUpdateError("invalid_argument", fmt.Sprintf("'%s' is an invalid work package id. Must be a number.", args[0])) return } - if workPackage, err := work_packages.Update(id, updateOptions()); err == nil { + if err := validateUpdateWorkPackageFlags(); err != nil { + printUpdateError("conflicting_arguments", err.Error()) + return + } + + if len(setFlags) > 0 { + if dryRunUpdateWorkPackage { + plan, err := work_packages.DryRunUpdateFields(id, setFlags) + if err != nil { + printUpdateError(updateFieldErrorCode(err), err.Error()) + return + } + + data, err := presenter.MarshalJSON(plan) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) + return + } + + if err := work_packages.UpdateFields(id, setFlags); err != nil { + printUpdateError(updateFieldErrorCode(err), err.Error()) + return + } + + if printUpdatedWorkPackageAsJSON { + payload, err := work_packages.Inspect(id) + if err != nil { + printUpdateError("post_apply_inspect_failed", err.Error()) + return + } + + data, err := presenter.MarshalJSON(payload) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) + return + } + + workPackage, err := work_packages.Lookup(id) + if err != nil { + printer.Error(err) + return + } + + printer.Info("-- ") + printer.WorkPackage(workPackage) + return + } + + options := updateOptions() + if len(options) == 0 { + if printUpdatedWorkPackageAsJSON || dryRunUpdateWorkPackage { + printUpdateError("invalid_argument", "no updates specified: provide at least one update flag or --set") + return + } + + workPackage, err := work_packages.Lookup(id) + if err != nil { + printer.Error(err) + return + } + printer.Info("-- ") printer.WorkPackage(workPackage) - } else { + return + } + + if dryRunUpdateWorkPackage { + plan, err := work_packages.DryRunUpdate(id, options) + if err != nil { + printUpdateError("api_error", err.Error()) + return + } + + data, err := presenter.MarshalJSON(plan) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) + return + } + + if !printUpdatedWorkPackageAsJSON { + printer.Info("Updating work package ...") + } + + workPackage, err := work_packages.Update(id, options) + if err != nil { + if printUpdatedWorkPackageAsJSON { + printUpdateError("api_error", err.Error()) + return + } printer.Error(err) + return } + + if printUpdatedWorkPackageAsJSON { + payload, err := work_packages.Inspect(id) + if err != nil { + printUpdateError("post_apply_inspect_failed", err.Error()) + return + } + + data, err := presenter.MarshalJSON(payload) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) + return + } + + printer.Info("-- ") + printer.WorkPackage(workPackage) } func updateOptions() map[work_packages.UpdateOption]string { @@ -66,3 +190,77 @@ func updateOptions() map[work_packages.UpdateOption]string { return options } + +func validateUpdateWorkPackageFlags() error { + if dryRunUpdateWorkPackage && !printUpdatedWorkPackageAsJSON { + return fmt.Errorf("cannot use --dry-run without --json") + } + + if len(setFlags) > 0 && hasLegacyUpdateFlags() { + return fmt.Errorf("cannot combine --set with %s", strings.Join(activeLegacyUpdateFlags(), ", ")) + } + + return nil +} + +func hasLegacyUpdateFlags() bool { + return len(activeLegacyUpdateFlags()) > 0 +} + +func activeLegacyUpdateFlags() []string { + flags := []string{} + + if len(actionFlag) > 0 { + flags = append(flags, "--action") + } + if assigneeFlag > 0 { + flags = append(flags, "--assignee") + } + if len(attachFlag) > 0 { + flags = append(flags, "--attach") + } + if len(subjectFlag) > 0 { + flags = append(flags, "--subject") + } + if len(typeFlag) > 0 { + flags = append(flags, "--type") + } + + return flags +} + +func printUpdateError(code, message string) { + if !printUpdatedWorkPackageAsJSON { + printer.ErrorText(message) + return + } + + data, err := presenter.MarshalError(code, message) + if err != nil { + printer.Error(err) + return + } + + printer.Info(string(data)) +} + +func updateFieldErrorCode(err error) string { + switch { + case errors.Is(err, work_packages.ErrInvalidFieldAssignment): + return "invalid_argument" + case errors.Is(err, work_packages.ErrAmbiguousField): + return "ambiguous_field" + case errors.Is(err, work_packages.ErrDuplicateField): + return "duplicate_field" + case errors.Is(err, work_packages.ErrUnknownField): + return "unknown_field" + case errors.Is(err, work_packages.ErrUnsupportedFieldType): + return "unsupported_field_type" + case errors.Is(err, work_packages.ErrInvalidFieldValue): + return "invalid_field_value" + case errors.Is(err, work_packages.ErrNonWritableField): + return "non_writable_field" + default: + return "api_error" + } +} diff --git a/cmd/update/work_package_test.go b/cmd/update/work_package_test.go new file mode 100644 index 0000000..ed7311a --- /dev/null +++ b/cmd/update/work_package_test.go @@ -0,0 +1,478 @@ +package update + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/routes" +) + +func TestUpdateWorkPackagePrintsDryRunJSON(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = []string{"Votes=3"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = true + + updateWorkPackage(nil, []string{"74316"}) + + expected := "{\"valid\":true,\"operation\":\"update\",\"work_package_id\":74316,\"resolved_fields\":{\"Votes\":{\"api_field\":\"customField130\",\"value\":3}}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestValidateUpdateWorkPackageFlagsRejectsDryRunWithoutJSON(t *testing.T) { + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = []string{"Votes=3"} + printUpdatedWorkPackageAsJSON = false + dryRunUpdateWorkPackage = true + + err := validateUpdateWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestValidateUpdateWorkPackageFlagsRejectsMixedSetAndLegacyFlags(t *testing.T) { + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "new subject" + typeFlag = "" + setFlags = []string{"Votes=3"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = false + + err := validateUpdateWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestUpdateWorkPackagePrintsJSONWithoutChildrenQuery(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true} + }`) + case "/api/v3/work_packages": + t.Fatalf("unexpected children query: %s", r.URL.Path) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = []string{"Votes=3"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = false + + updateWorkPackage(nil, []string{"74316"}) + + expected := "{\"work_package\":{\"id\":74316,\"subject\":\"Expand op CLI to support scripted work package workflows\",\"type\":\"Feature\",\"status\":\"new\",\"assignee\":\"\",\"description\":\"\",\"parent_id\":null,\"project\":{\"id\":1482,\"identifier\":\"cli\",\"name\":\"CLI\"},\"fields\":{},\"field_labels\":{\"Votes\":[\"customField130\"]}},\"children\":[]}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestUpdateWorkPackagePrintsAmbiguousFieldJSONError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField17": {"name": "KPI", "type": "Integer", "writable": true}, + "customField22": {"name": "KPI", "type": "Integer", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = []string{"KPI=3"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = true + + updateWorkPackage(nil, []string{"74316"}) + + expected := "{\"error\":{\"code\":\"ambiguous_field\",\"message\":\"ambiguous field: \\\"KPI\\\"\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestUpdateWorkPackagePrintsDuplicateFieldJSONError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = []string{"Votes=3", "customField130=4"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = true + + updateWorkPackage(nil, []string{"74316"}) + + expected := "{\"error\":{\"code\":\"duplicate_field\",\"message\":\"duplicate field: \\\"customField130\\\"\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestUpdateWorkPackagePrintsAPIErrorForSetDryRunFetchFailure(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, `{"message":"boom"}`) + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = []string{"Votes=3"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = true + + updateWorkPackage(nil, []string{"74316"}) + + expected := "{\"error\":{\"code\":\"api_error\",\"message\":\"{\\\"message\\\":\\\"boom\\\"}\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestUpdateWorkPackagePrintsAPIErrorWhenPatchFails(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Old subject", + "lockVersion": 7, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + w.WriteHeader(http.StatusConflict) + _, _ = io.WriteString(w, `{"message":"conflict"}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "New subject" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = false + + updateWorkPackage(nil, []string{"74416"}) + + expected := "{\"error\":{\"code\":\"api_error\",\"message\":\"{\\\"message\\\":\\\"conflict\\\"}\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestUpdateWorkPackageWithoutFlagsPrintsCurrentWorkPackage(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = false + dryRunUpdateWorkPackage = false + + updateWorkPackage(nil, []string{"74316"}) + + if activePrinter.Result == "" { + t.Fatal("expected work package output, got empty result") + } +} + +func TestUpdateWorkPackagePrintsHumanProgressForNonJSONUpdate(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Old subject", + "lockVersion": 7, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + subjectFlag = "New subject" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = false + dryRunUpdateWorkPackage = false + + updateWorkPackage(nil, []string{"74416"}) + + if !strings.Contains(activePrinter.Result, "Updating work package ...") { + t.Fatalf("expected human progress output, got %q", activePrinter.Result) + } +} diff --git a/components/errors/errors.go b/components/errors/errors.go index 982013c..0aa9a6f 100644 --- a/components/errors/errors.go +++ b/components/errors/errors.go @@ -1,5 +1,7 @@ package errors +import stderrors "errors" + type customError struct { text string } @@ -12,6 +14,11 @@ func Custom(text string) error { return &customError{text: text} } +func IsCustom(err error) bool { + var target *customError + return stderrors.As(err, &target) +} + type ResponseError struct { status int response []byte diff --git a/components/resources/work_packages/create.go b/components/resources/work_packages/create.go index 65e5118..849722c 100644 --- a/components/resources/work_packages/create.go +++ b/components/resources/work_packages/create.go @@ -4,10 +4,11 @@ import ( "bytes" "encoding/json" "fmt" + "strconv" + componentErrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/paths" - "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/dtos" "github.com/opf/openproject-cli/models" @@ -17,12 +18,13 @@ type CreateOption int const ( CreateSubject CreateOption = iota + CreateParent CreateType ) var createMap = map[CreateOption]func(projectId uint64, workPackage *dtos.WorkPackageDto, input string) error{ CreateSubject: subjectCreate, - CreateType: typeCreate, + CreateParent: parentCreate, } func subjectCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) error { @@ -31,56 +33,83 @@ func subjectCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) err return nil } -func typeCreate(projectId uint64, workPackage *dtos.WorkPackageDto, input string) error { - types, err := availableTypes(&dtos.LinkDto{Href: paths.Project(projectId)}) +func parentCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) error { + parentID, err := strconv.ParseUint(input, 10, 64) if err != nil { - return err + return componentErrors.Custom(fmt.Sprintf("'%s' is an invalid work package id. Must be a number.", input)) } - foundType := findType(input, types) - if foundType == nil { - printer.ErrorText("Failed to create work package type.") - printer.Info(fmt.Sprintf( - "No unique available type from input %s found for project %s. Please use one of the types listed below.", - printer.Cyan(input), - printer.Red(fmt.Sprintf("#%d", projectId)), - )) - - printer.Types(types.Convert()) - - return nil + if workPackage.Links == nil { + workPackage.Links = &dtos.WorkPackageLinksDto{} } + workPackage.Links.Parent = &dtos.LinkDto{Href: paths.WorkPackage(parentID)} + return nil +} + +func setTypeLink(workPackage *dtos.WorkPackageDto, foundType *dtos.TypeDto) { if workPackage.Links == nil { workPackage.Links = &dtos.WorkPackageLinksDto{} } workPackage.Links.Type = foundType.Links.Self - - return nil } func Create(projectId uint64, options map[CreateOption]string) (*models.WorkPackage, error) { - return create(projectId, options) + resolved, err := resolveCreate(projectId, options) + if err != nil { + return nil, err + } + + return create(resolved) +} + +func DryRunCreate(projectId uint64, options map[CreateOption]string) (*models.WorkPackageCreatePlan, error) { + resolved, err := resolveCreate(projectId, options) + if err != nil { + return nil, err + } + + plan := &models.WorkPackageCreatePlan{ + Valid: true, + Operation: "create", + ProjectID: resolved.ProjectID, + ParentID: resolved.ParentID, + WorkPackage: models.WorkPackageDraft{ + Subject: resolved.Options[CreateSubject], + Type: resolved.TypeName, + }, + } + + return plan, nil } -func create(projectId uint64, options map[CreateOption]string) (*models.WorkPackage, error) { +func create(resolved *resolvedCreate) (*models.WorkPackage, error) { workPackage := dtos.WorkPackageDto{} - for option, value := range options { - err := createMap[option](projectId, &workPackage, value) + for _, option := range []CreateOption{CreateSubject, CreateParent} { + value, ok := resolved.Options[option] + if !ok { + continue + } + + err := createMap[option](resolved.ProjectID, &workPackage, value) if err != nil { return nil, err } } + if resolved.Type != nil { + setTypeLink(&workPackage, resolved.Type) + } + data, err := json.Marshal(workPackage) if err != nil { return nil, err } requestData := requests.RequestData{ContentType: "application/json", Body: bytes.NewReader(data)} - response, err := requests.Post(paths.ProjectWorkPackages(projectId), &requestData) + response, err := requests.Post(paths.ProjectWorkPackages(resolved.ProjectID), &requestData) if err != nil { return nil, err } @@ -88,3 +117,73 @@ func create(projectId uint64, options map[CreateOption]string) (*models.WorkPack resultingWorkPackage := parser.Parse[dtos.WorkPackageDto](response) return resultingWorkPackage.Convert(), nil } + +type resolvedCreate struct { + ProjectID uint64 + ParentID *uint64 + Type *dtos.TypeDto + TypeName string + Options map[CreateOption]string +} + +func resolveCreate(projectId uint64, options map[CreateOption]string) (*resolvedCreate, error) { + resolved := &resolvedCreate{ + ProjectID: projectId, + Options: map[CreateOption]string{}, + } + + for option, value := range options { + resolved.Options[option] = value + } + + if value, ok := options[CreateParent]; ok { + parentID, err := strconv.ParseUint(value, 10, 64) + if err != nil { + return nil, componentErrors.Custom(fmt.Sprintf("'%s' is an invalid work package id. Must be a number.", value)) + } + + parent, err := fetch(parentID) + if err != nil { + return nil, err + } + + inferredProjectID := linkID(parent.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Project }) + if inferredProjectID == nil { + return nil, componentErrors.Custom("unable to infer project from parent work package") + } + + if resolved.ProjectID > 0 && resolved.ProjectID != *inferredProjectID { + return nil, componentErrors.Custom(fmt.Sprintf("conflicting project ids: --project %d does not match parent project %d", resolved.ProjectID, *inferredProjectID)) + } + + resolved.ProjectID = *inferredProjectID + resolved.ParentID = &parentID + } + + if resolved.ProjectID == 0 { + return nil, componentErrors.Custom("either --project or --parent is required") + } + + if value, ok := options[CreateType]; ok { + foundType, err := resolveType(resolved.ProjectID, value) + if err != nil { + return nil, err + } + if foundType == nil { + return nil, componentErrors.Custom(fmt.Sprintf("No unique available type from input %s found for project #%d.", value, resolved.ProjectID)) + } + resolved.Type = foundType + resolved.TypeName = foundType.Name + } + + return resolved, nil +} + +func resolveType(projectId uint64, input string) (*dtos.TypeDto, error) { + types, err := availableTypes(&dtos.LinkDto{Href: paths.Project(projectId)}) + if err != nil { + return nil, err + } + + return findType(input, types), nil +} diff --git a/components/resources/work_packages/create_test.go b/components/resources/work_packages/create_test.go new file mode 100644 index 0000000..c02e787 --- /dev/null +++ b/components/resources/work_packages/create_test.go @@ -0,0 +1,136 @@ +package work_packages_test + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestDryRunCreateWithParentInfersProject(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + plan, err := work_packages.DryRunCreate(0, map[work_packages.CreateOption]string{ + work_packages.CreateSubject: "Build reusable skill", + work_packages.CreateParent: "74316", + }) + if err != nil { + t.Fatal(err) + } + + if !plan.Valid || plan.ProjectID != 1482 { + t.Fatalf("unexpected plan: %+v", plan) + } + + if plan.ParentID == nil || *plan.ParentID != 74316 { + t.Fatalf("expected parent id 74316, got %+v", plan.ParentID) + } +} + +func TestDryRunCreateWithTypeResolvesTypeName(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "identifier": "cli", + "name": "CLI", + "_links": { + "types": {"href": "/api/v3/projects/1482/types/available"} + } + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 7, + "name": "Implementation", + "_links": { + "self": {"href": "/api/v3/types/7"} + } + } + ] + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + plan, err := work_packages.DryRunCreate(0, map[work_packages.CreateOption]string{ + work_packages.CreateSubject: "Build reusable skill", + work_packages.CreateParent: "74316", + work_packages.CreateType: "Implementation", + }) + if err != nil { + t.Fatal(err) + } + + if plan.WorkPackage.Type != "Implementation" { + t.Fatalf("expected type Implementation, got %+v", plan.WorkPackage) + } +} diff --git a/components/resources/work_packages/details.go b/components/resources/work_packages/details.go index 82021de..816628b 100644 --- a/components/resources/work_packages/details.go +++ b/components/resources/work_packages/details.go @@ -1,8 +1,6 @@ package work_packages import ( - "strconv" - "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/paths" "github.com/opf/openproject-cli/components/requests" @@ -131,11 +129,3 @@ func linkID(links *dtos.WorkPackageLinksDto, selector func(*dtos.WorkPackageLink id := parser.IdFromLink(link.Href) return &id } - -func ParentFilter(parentID uint64) requests.Filter { - return requests.Filter{ - Operator: "=", - Name: "parent", - Values: []string{strconv.FormatUint(parentID, 10)}, - } -} diff --git a/components/resources/work_packages/field_resolution.go b/components/resources/work_packages/field_resolution.go new file mode 100644 index 0000000..6685f56 --- /dev/null +++ b/components/resources/work_packages/field_resolution.go @@ -0,0 +1,126 @@ +package work_packages + +import ( + "errors" + "fmt" + "strconv" + "strings" + "time" + + "github.com/opf/openproject-cli/models" +) + +var ( + ErrInvalidFieldAssignment = errors.New("invalid field assignment") + ErrAmbiguousField = errors.New("ambiguous field") + ErrDuplicateField = errors.New("duplicate field") + ErrInvalidFieldValue = errors.New("invalid field value") + ErrNonWritableField = errors.New("non-writable field") + ErrUnknownField = errors.New("unknown field") + ErrUnsupportedFieldType = errors.New("unsupported field type") +) + +func resolveFieldAssignments(schema *Schema, assignments []string) (map[string]models.ResolvedField, error) { + resolved := make(map[string]models.ResolvedField, len(assignments)) + seenAPIFields := make(map[string]struct{}, len(assignments)) + + for _, assignment := range assignments { + key, rawValue, ok := strings.Cut(assignment, "=") + if !ok { + return nil, fmt.Errorf("%w: %q, expected key=value", ErrInvalidFieldAssignment, assignment) + } + + if _, exists := resolved[key]; exists { + return nil, fmt.Errorf("%w: %q", ErrDuplicateField, key) + } + + field, err := resolveSchemaField(schema, key) + if err != nil { + return nil, err + } + + if !field.Writable { + return nil, fmt.Errorf("%w: %q", ErrNonWritableField, field.APIName) + } + + if _, exists := seenAPIFields[field.APIName]; exists { + return nil, fmt.Errorf("%w: %q", ErrDuplicateField, field.APIName) + } + + value, err := coerceFieldValue(field, rawValue) + if err != nil { + return nil, err + } + + resolved[key] = models.ResolvedField{ + APIField: field.APIName, + Value: value, + } + seenAPIFields[field.APIName] = struct{}{} + } + + return resolved, nil +} + +func resolveSchemaField(schema *Schema, key string) (*SchemaField, error) { + var exactAPIField *SchemaField + var labelMatches []SchemaField + + for _, field := range schema.Fields { + if field.APIName == key { + fieldCopy := field + exactAPIField = &fieldCopy + break + } + + if strings.EqualFold(field.Label, key) { + labelMatches = append(labelMatches, field) + } + } + + if exactAPIField != nil { + return exactAPIField, nil + } + + switch len(labelMatches) { + case 1: + fieldCopy := labelMatches[0] + return &fieldCopy, nil + case 0: + return nil, fmt.Errorf("%w: %q", ErrUnknownField, key) + default: + return nil, fmt.Errorf("%w: %q", ErrAmbiguousField, key) + } +} + +func coerceFieldValue(field *SchemaField, raw string) (any, error) { + switch field.Type { + case "String", "Formattable": + return raw, nil + case "Integer": + value, err := strconv.ParseInt(raw, 10, 64) + if err != nil { + return nil, fmt.Errorf("%w: invalid integer value %q for %s", ErrInvalidFieldValue, raw, field.Label) + } + return value, nil + case "Float": + value, err := strconv.ParseFloat(raw, 64) + if err != nil { + return nil, fmt.Errorf("%w: invalid float value %q for %s", ErrInvalidFieldValue, raw, field.Label) + } + return value, nil + case "Boolean": + value, err := strconv.ParseBool(raw) + if err != nil { + return nil, fmt.Errorf("%w: invalid boolean value %q for %s", ErrInvalidFieldValue, raw, field.Label) + } + return value, nil + case "Date": + if _, err := time.Parse("2006-01-02", raw); err != nil { + return nil, fmt.Errorf("%w: invalid date value %q for %s", ErrInvalidFieldValue, raw, field.Label) + } + return raw, nil + default: + return nil, fmt.Errorf("%w: %s for %s", ErrUnsupportedFieldType, field.Type, field.Label) + } +} diff --git a/components/resources/work_packages/field_resolution_test.go b/components/resources/work_packages/field_resolution_test.go new file mode 100644 index 0000000..443d02b --- /dev/null +++ b/components/resources/work_packages/field_resolution_test.go @@ -0,0 +1,67 @@ +package work_packages + +import ( + "errors" + "testing" +) + +func TestResolveFieldAssignmentsByLabel(t *testing.T) { + schema := &Schema{ + Fields: []SchemaField{ + {APIName: "customField130", Label: "Votes", Type: "Integer", Writable: true}, + }, + } + + resolved, err := resolveFieldAssignments(schema, []string{"Votes=3"}) + if err != nil { + t.Fatal(err) + } + + field := resolved["Votes"] + if field.APIField != "customField130" || field.Value != int64(3) { + t.Fatalf("unexpected resolution: %+v", field) + } +} + +func TestResolveFieldAssignmentsRejectsAmbiguousLabels(t *testing.T) { + schema := &Schema{ + Fields: []SchemaField{ + {APIName: "customField17", Label: "KPI", Type: "Integer", Writable: true}, + {APIName: "customField22", Label: "KPI", Type: "Integer", Writable: true}, + }, + } + + if _, err := resolveFieldAssignments(schema, []string{"KPI=42"}); err == nil { + t.Fatal("expected ambiguous field error") + } else if !errors.Is(err, ErrAmbiguousField) { + t.Fatalf("expected ErrAmbiguousField, got %v", err) + } +} + +func TestResolveFieldAssignmentsRejectsNonWritableFields(t *testing.T) { + schema := &Schema{ + Fields: []SchemaField{ + {APIName: "customField130", Label: "Votes", Type: "Integer", Writable: false}, + }, + } + + if _, err := resolveFieldAssignments(schema, []string{"Votes=3"}); err == nil { + t.Fatal("expected non-writable field error") + } else if !errors.Is(err, ErrNonWritableField) { + t.Fatalf("expected ErrNonWritableField, got %v", err) + } +} + +func TestResolveFieldAssignmentsRejectsDuplicateAPIFields(t *testing.T) { + schema := &Schema{ + Fields: []SchemaField{ + {APIName: "customField130", Label: "Votes", Type: "Integer", Writable: true}, + }, + } + + if _, err := resolveFieldAssignments(schema, []string{"Votes=3", "customField130=4"}); err == nil { + t.Fatal("expected duplicate field error") + } else if !errors.Is(err, ErrDuplicateField) { + t.Fatalf("expected ErrDuplicateField, got %v", err) + } +} diff --git a/components/resources/work_packages/filters.go b/components/resources/work_packages/filters.go index 19a5cc3..ba1b111 100644 --- a/components/resources/work_packages/filters.go +++ b/components/resources/work_packages/filters.go @@ -1,6 +1,7 @@ package work_packages import ( + "strconv" "strings" "github.com/opf/openproject-cli/components/requests" @@ -91,3 +92,11 @@ func StatusFilter(status string) requests.Filter { Values: values, } } + +func ParentFilter(parentID uint64) requests.Filter { + return requests.Filter{ + Operator: "=", + Name: "parent", + Values: []string{strconv.FormatUint(parentID, 10)}, + } +} diff --git a/components/resources/work_packages/schema.go b/components/resources/work_packages/schema.go index 884b59a..549cded 100644 --- a/components/resources/work_packages/schema.go +++ b/components/resources/work_packages/schema.go @@ -1,6 +1,8 @@ package work_packages import ( + "sort" + "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/dtos" @@ -38,6 +40,10 @@ func SchemaFor(workPackage *dtos.WorkPackageDto) (*Schema, error) { }) } + sort.Slice(fields, func(i, j int) bool { + return fields[i].APIName < fields[j].APIName + }) + return &Schema{Fields: fields}, nil } diff --git a/components/resources/work_packages/schema_test.go b/components/resources/work_packages/schema_test.go new file mode 100644 index 0000000..7d766a6 --- /dev/null +++ b/components/resources/work_packages/schema_test.go @@ -0,0 +1,64 @@ +package work_packages_test + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" + "github.com/opf/openproject-cli/dtos" +) + +func TestSchemaForWithoutSchemaLinkReturnsEmptySchema(t *testing.T) { + schema, err := work_packages.SchemaFor(&dtos.WorkPackageDto{}) + if err != nil { + t.Fatal(err) + } + + if len(schema.Fields) != 0 { + t.Fatalf("expected empty schema, got %+v", schema.Fields) + } +} + +func TestSchemaForSortsCustomFields(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true}, + "customField108": {"name": "Requires doc change", "type": "Boolean", "writable": true}, + "subject": {"name": "Subject", "type": "String", "writable": true} + }`) + default: + t.Fatalf("unexpected path: %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + schema, err := work_packages.SchemaFor(&dtos.WorkPackageDto{ + Links: &dtos.WorkPackageLinksDto{ + Schema: &dtos.LinkDto{Href: "/api/v3/work_packages/schemas/1482-6"}, + }, + }) + if err != nil { + t.Fatal(err) + } + + if len(schema.Fields) != 2 { + t.Fatalf("expected two custom fields, got %+v", schema.Fields) + } + + if schema.Fields[0].APIName != "customField108" || schema.Fields[1].APIName != "customField130" { + t.Fatalf("expected sorted fields, got %+v", schema.Fields) + } +} diff --git a/components/resources/work_packages/update.go b/components/resources/work_packages/update.go index 7cc751d..2cb28c6 100644 --- a/components/resources/work_packages/update.go +++ b/components/resources/work_packages/update.go @@ -33,6 +33,43 @@ var patchMap = map[UpdateOption]func(patch, workPackage *dtos.WorkPackageDto, in UpdateSubject: subjectPatch, } +func DryRunUpdate(id uint64, options map[UpdateOption]string) (*models.WorkPackageUpdatePlan, error) { + workPackage, err := fetch(id) + if err != nil { + return nil, err + } + + plan := &models.WorkPackageUpdatePlan{ + Valid: true, + Operation: "update", + WorkPackageID: id, + Subject: options[UpdateSubject], + Action: options[UpdateCustomAction], + Attach: options[UpdateAttachment], + ResolvedFields: map[string]models.ResolvedField{}, + } + + if assignee, ok := options[UpdateAssignee]; ok { + plan.Assignee = assignee + } + + if value, ok := options[UpdateType]; ok { + types, err := availableTypes(workPackage.Links.Project) + if err != nil { + return nil, err + } + + foundType := findType(value, types) + if foundType == nil { + return nil, fmt.Errorf("no unique available type from input %q found for work package #%d", value, id) + } + + plan.Type = foundType.Name + } + + return plan, nil +} + func Update(id uint64, options map[UpdateOption]string) (*models.WorkPackage, error) { workPackage, err := fetch(id) if err != nil { @@ -42,25 +79,25 @@ func Update(id uint64, options map[UpdateOption]string) (*models.WorkPackage, er if customAction, ok := options[UpdateCustomAction]; ok { err = action(workPackage, customAction) if err != nil { - printer.Error(err) - } else { - // reload work package to get new lock version - workPackage, err = fetch(id) - if err != nil { - return nil, err - } + return nil, err + } + + // reload work package to get new lock version + workPackage, err = fetch(id) + if err != nil { + return nil, err } } err = patch(workPackage, options) if err != nil { - printer.Error(err) + return nil, err } if file, ok := options[UpdateAttachment]; ok { err = upload(workPackage, file) if err != nil { - printer.Error(err) + return nil, err } } @@ -75,7 +112,6 @@ func Update(id uint64, options map[UpdateOption]string) (*models.WorkPackage, er func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) error { var patchNeeded = false patchDto := dtos.WorkPackageDto{LockVersion: workPackage.LockVersion} - var updateString string for option, value := range options { if !common.Contains(patchableUpdates, option) { @@ -87,22 +123,13 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er if err != nil { return err } - - if len(updateStringLine) > 0 { - if len(updateString) > 0 { - updateString += "\n" - } - updateString += fmt.Sprintf("\t%s", updateStringLine) - } + _ = updateStringLine } if !patchNeeded { return nil } - printer.Info(fmt.Sprintf("Updating work package with patch ...")) - printer.Info(updateString) - marshal, err := json.Marshal(patchDto) if err != nil { return err @@ -113,7 +140,6 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er return err } - printer.Done() return nil } diff --git a/components/resources/work_packages/update_fields.go b/components/resources/work_packages/update_fields.go new file mode 100644 index 0000000..39bf2a4 --- /dev/null +++ b/components/resources/work_packages/update_fields.go @@ -0,0 +1,68 @@ +package work_packages + +import ( + "bytes" + "encoding/json" + + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/models" +) + +func DryRunUpdateFields(id uint64, assignments []string) (*models.WorkPackageUpdatePlan, error) { + workPackage, err := fetch(id) + if err != nil { + return nil, err + } + + schema, err := SchemaFor(workPackage) + if err != nil { + return nil, err + } + + resolved, err := resolveFieldAssignments(schema, assignments) + if err != nil { + return nil, err + } + + return &models.WorkPackageUpdatePlan{ + Valid: true, + Operation: "update", + WorkPackageID: id, + ResolvedFields: resolved, + }, nil +} + +func UpdateFields(id uint64, assignments []string) error { + workPackage, err := fetch(id) + if err != nil { + return err + } + + schema, err := SchemaFor(workPackage) + if err != nil { + return err + } + + resolved, err := resolveFieldAssignments(schema, assignments) + if err != nil { + return err + } + + patch := map[string]any{ + "lockVersion": workPackage.LockVersion, + } + for _, field := range resolved { + patch[field.APIField] = field.Value + } + + body, err := json.Marshal(patch) + if err != nil { + return err + } + + _, err = requests.Patch(workPackage.Links.Self.Href, &requests.RequestData{ + ContentType: "application/json", + Body: bytes.NewReader(body), + }) + return err +} diff --git a/components/resources/work_packages/update_fields_test.go b/components/resources/work_packages/update_fields_test.go new file mode 100644 index 0000000..90b4568 --- /dev/null +++ b/components/resources/work_packages/update_fields_test.go @@ -0,0 +1,56 @@ +package work_packages_test + +import ( + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestDryRunUpdateFieldsResolvesLabels(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField130": {"name": "Votes", "type": "Integer", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + plan, err := work_packages.DryRunUpdateFields(74316, []string{"Votes=3"}) + if err != nil { + t.Fatal(err) + } + + field := plan.ResolvedFields["Votes"] + if field.APIField != "customField130" || field.Value != int64(3) { + t.Fatalf("unexpected plan: %+v", plan) + } +} diff --git a/dtos/work_package_schema_test.go b/dtos/work_package_schema_test.go new file mode 100644 index 0000000..a1ff2ec --- /dev/null +++ b/dtos/work_package_schema_test.go @@ -0,0 +1,35 @@ +package dtos + +import ( + "encoding/json" + "testing" +) + +func TestWorkPackageSchemaDtoUnmarshalIgnoresNonCustomFields(t *testing.T) { + body := []byte(`{ + "subject": {"name": "Subject", "type": "String", "writable": true}, + "customField108": {"name": "Requires doc change", "type": "Boolean", "writable": true}, + "customField130": {"name": "Votes", "type": "Integer", "writable": false} + }`) + + var dto WorkPackageSchemaDto + if err := json.Unmarshal(body, &dto); err != nil { + t.Fatal(err) + } + + if len(dto.Fields) != 2 { + t.Fatalf("expected two custom fields, got %+v", dto.Fields) + } + + if _, ok := dto.Fields["subject"]; ok { + t.Fatalf("expected non-custom fields to be ignored, got %+v", dto.Fields) + } + + if !dto.Fields["customField108"].Writable { + t.Fatalf("expected customField108 to be writable, got %+v", dto.Fields["customField108"]) + } + + if dto.Fields["customField130"].Type != "Integer" { + t.Fatalf("expected customField130 type to be Integer, got %+v", dto.Fields["customField130"]) + } +} diff --git a/models/work_package_mutation.go b/models/work_package_mutation.go new file mode 100644 index 0000000..8af9082 --- /dev/null +++ b/models/work_package_mutation.go @@ -0,0 +1,32 @@ +package models + +type WorkPackageDraft struct { + Subject string `json:"subject"` + Type string `json:"type"` +} + +type WorkPackageCreatePlan struct { + Valid bool `json:"valid"` + Operation string `json:"operation"` + ProjectID uint64 `json:"project_id"` + ParentID *uint64 `json:"parent_id"` + WorkPackage WorkPackageDraft `json:"work_package"` +} + +type ResolvedField struct { + APIField string `json:"api_field"` + Value any `json:"value"` +} + +type WorkPackageUpdatePlan struct { + Valid bool `json:"valid"` + Operation string `json:"operation"` + WorkPackageID uint64 `json:"work_package_id"` + Subject string `json:"subject,omitempty"` + Type string `json:"type,omitempty"` + Assignee string `json:"assignee,omitempty"` + Description *string `json:"description,omitempty"` + Action string `json:"action,omitempty"` + Attach string `json:"attach,omitempty"` + ResolvedFields map[string]ResolvedField `json:"resolved_fields"` +} From 149a355a3a86a43429384d55029287d189dc7b2b Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 24 Apr 2026 19:56:20 +0200 Subject: [PATCH 3/9] [#74316] document JSON WP workflows Add README examples for the new machine-readable Work Package flows: inspect with children, parent-aware create dry-runs, and schema-driven update dry-runs. https://community.openproject.org/projects/cli/work_packages/74316 --- README.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/README.md b/README.md index 6770ea8..82a8f02 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,10 @@ op create workpackge --project 11 'Document new CLI tool' # Same command with shorthands and directly open it in a browser to continue working on it. op create workpackge -p11 'Document new CLI tool' -o + +# Validating the creation of a child work package without persisting it. +# The parent determines the project automatically. +op create workpackage --parent 74316 --type Implementation 'Build reusable skill' --dry-run --json ``` #### Listing @@ -191,6 +195,27 @@ op inspect workpackage 42 op inspect workpackage 74316 --children --json ``` +#### JSON error codes + +The `--json` variants of `inspect`, `create`, and `update` return a stable error envelope: + +```json +{"error": {"code": "", "message": ""}} +``` + +| Code | Trigger | Mutation persisted? | +|---|---|---| +| `invalid_argument` | Missing positional arg, invalid id, local validation failure, malformed `--set`, or no update options under `--json`/`--dry-run` | No | +| `conflicting_arguments` | Incompatible flag combinations, e.g. `--description` + `--set`, `--open` + `--json`, `--dry-run` without `--json` | No | +| `api_error` | Underlying OpenProject API call failed while resolving, creating, updating, or dry-running a work package | No | +| `post_apply_inspect_failed` | CREATE or PATCH succeeded but the follow-up inspect to build the JSON response failed — the mutation **has persisted**, only the response payload could not be assembled | **Yes** | +| `ambiguous_field` | A `--set` label matched more than one schema field | No | +| `duplicate_field` | A `--set` assignment names the same API field twice | No | +| `unknown_field` | A `--set` label or API field is not present in the resolved schema | No | +| `unsupported_field_type` | A `--set` field's schema type is not yet coercible | No | +| `invalid_field_value` | A `--set` value fails type coercion (e.g. non-integer for an `Integer` field) | No | +| `non_writable_field` | A `--set` field exists in the schema but is not writable | No | + ## Creating a release Releases are triggered by pushing a git tag. The tag name becomes the version string embedded in the binaries. From 5f0bd49a717085d7c6614e65f1324363aec0dc6d Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 24 Apr 2026 21:30:05 +0200 Subject: [PATCH 4/9] [#74416] add work package description support Add explicit `--description` support to create and update work package commands. This extends create and the core update patch path, includes JSON dry-run coverage for descriptions, documents the new flag, and keeps validation strict around unsupported flag combinations. https://community.openproject.org/wp/74416 --- README.md | 10 + cmd/create/create.go | 7 + cmd/create/work_package.go | 12 +- cmd/create/work_package_test.go | 93 +++++- cmd/update/update.go | 6 + cmd/update/work_package.go | 35 ++- cmd/update/work_package_test.go | 214 +++++++++++++ components/resources/work_packages/create.go | 19 +- .../resources/work_packages/create_test.go | 121 +++++++ components/resources/work_packages/update.go | 26 +- .../resources/work_packages/update_test.go | 296 ++++++++++++++++++ models/work_package_mutation.go | 5 +- 12 files changed, 826 insertions(+), 18 deletions(-) create mode 100644 components/resources/work_packages/update_test.go diff --git a/README.md b/README.md index 82a8f02..0abbb5d 100644 --- a/README.md +++ b/README.md @@ -130,6 +130,7 @@ op update workpackge 42 - --action -a -- Executes a custom action on a work package --assignee -- Assign a user to the work package --attach -- Attach a file to the work package +--description -- Change the raw work package description --help -h -- help for workpackage --subject -- Change the subject of the work package --type -t -- Change the work package type @@ -154,6 +155,11 @@ op create workpackge -p11 'Document new CLI tool' -o # Validating the creation of a child work package without persisting it. # The parent determines the project automatically. op create workpackage --parent 74316 --type Implementation 'Build reusable skill' --dry-run --json + +# Creating a child work package with an explicit raw description. +op create workpackage --parent 74316 --type Implementation \ + --description 'Use openproject-cli JSON workflows from a reusable agent skill.' \ + 'Build reusable skill' ``` #### Listing @@ -182,6 +188,10 @@ op update workpackage 42 --attach ./Downloads/Report.pdf # Resolving and validating field updates as JSON before applying them # This first slice supports schema-resolved custom fields via --set. op update workpackage 74316 --set 'Votes=3' --dry-run --json + +# Updating core fields, including the raw description, in one PATCH request. +op update workpackage 74416 --subject 'Add explicit description support' \ + --description 'Allow create and update to write the raw ticket body.' ``` #### Inspecting diff --git a/cmd/create/create.go b/cmd/create/create.go index c5bbece..7f072f7 100644 --- a/cmd/create/create.go +++ b/cmd/create/create.go @@ -40,6 +40,13 @@ func init() { "Create the work package as a child of an existing work package", ) + createWorkPackageCmd.Flags().StringVar( + &descriptionFlag, + "description", + "", + "Set the raw work package description", + ) + createWorkPackageCmd.Flags().BoolVar( &printCreatedWorkPackageAsJSON, "json", diff --git a/cmd/create/work_package.go b/cmd/create/work_package.go index a745fd2..c3b4329 100644 --- a/cmd/create/work_package.go +++ b/cmd/create/work_package.go @@ -19,6 +19,8 @@ var shouldOpenWorkPackageInBrowser bool var printCreatedWorkPackageAsJSON bool var dryRunCreateWorkPackage bool var typeFlag string +var descriptionFlag string +var descriptionFlagChanged bool var createWorkPackageCmd = &cobra.Command{ Use: "workpackage [subject]", @@ -27,7 +29,11 @@ var createWorkPackageCmd = &cobra.Command{ Run: createWorkPackage, } -func createWorkPackage(_ *cobra.Command, args []string) { +func createWorkPackage(cmd *cobra.Command, args []string) { + if cmd != nil { + descriptionFlagChanged = cmd.Flags().Changed("description") + } + if len(args) != 1 { printCreateError("invalid_argument", fmt.Sprintf("Expected 1 argument [subject], but got %d", len(args))) return @@ -104,6 +110,10 @@ func createOptions(subject string) map[work_packages.CreateOption]string { options[work_packages.CreateParent] = fmt.Sprintf("%d", parentWorkPackageID) } + if descriptionFlagChanged { + options[work_packages.CreateDescription] = descriptionFlag + } + return options } diff --git a/cmd/create/work_package_test.go b/cmd/create/work_package_test.go index c249081..6ca3f94 100644 --- a/cmd/create/work_package_test.go +++ b/cmd/create/work_package_test.go @@ -57,10 +57,91 @@ func TestCreateWorkPackagePrintsDryRunJSONWithParent(t *testing.T) { printCreatedWorkPackageAsJSON = true dryRunCreateWorkPackage = true typeFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false createWorkPackage(nil, []string{"Build reusable skill"}) - expected := "{\"valid\":true,\"operation\":\"create\",\"project_id\":1482,\"parent_id\":74316,\"work_package\":{\"subject\":\"Build reusable skill\",\"type\":\"\"}}\n" + expected := "{\"valid\":true,\"operation\":\"create\",\"project_id\":1482,\"parent_id\":74316,\"work_package\":{\"subject\":\"Build reusable skill\",\"type\":\"\",\"description\":\"\"}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + +func TestCreateWorkPackagePrintsDryRunJSONWithDescription(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "identifier": "cli", + "name": "CLI", + "_links": { + "types": {"href": "/api/v3/projects/1482/types/available"} + } + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 7, + "name": "Implementation", + "_links": { + "self": {"href": "/api/v3/types/7"} + } + } + ] + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + projectId = 0 + parentWorkPackageID = 74316 + shouldOpenWorkPackageInBrowser = false + printCreatedWorkPackageAsJSON = true + dryRunCreateWorkPackage = true + typeFlag = "Implementation" + descriptionFlag = "Body" + descriptionFlagChanged = true + + createWorkPackage(nil, []string{"Add explicit work package description support"}) + + expected := "{\"valid\":true,\"operation\":\"create\",\"project_id\":1482,\"parent_id\":74316,\"work_package\":{\"subject\":\"Add explicit work package description support\",\"type\":\"Implementation\",\"description\":\"Body\"}}\n" if activePrinter.Result != expected { t.Fatalf("expected %s, got %s", expected, activePrinter.Result) } @@ -76,6 +157,8 @@ func TestCreateWorkPackagePrintsJSONErrorForFlagConflict(t *testing.T) { printCreatedWorkPackageAsJSON = true dryRunCreateWorkPackage = false typeFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false createWorkPackage(nil, []string{"Build reusable skill"}) @@ -92,6 +175,8 @@ func TestValidateCreateWorkPackageFlagsRejectsDryRunWithoutJSON(t *testing.T) { printCreatedWorkPackageAsJSON = false dryRunCreateWorkPackage = true typeFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false err := validateCreateWorkPackageFlags() if err == nil { @@ -207,6 +292,8 @@ func TestCreateWorkPackagePrintsInvalidArgumentForValidationError(t *testing.T) printCreatedWorkPackageAsJSON = true dryRunCreateWorkPackage = false typeFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false createWorkPackage(nil, []string{"Build reusable skill"}) @@ -245,6 +332,8 @@ func TestCreateWorkPackagePrintsAPIErrorForDryRunParentLookupFailure(t *testing. printCreatedWorkPackageAsJSON = true dryRunCreateWorkPackage = true typeFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false createWorkPackage(nil, []string{"Build reusable skill"}) @@ -318,6 +407,8 @@ func TestCreateWorkPackagePrintsPostApplyInspectFailure(t *testing.T) { printCreatedWorkPackageAsJSON = true dryRunCreateWorkPackage = false typeFlag = "Implementation" + descriptionFlag = "" + descriptionFlagChanged = false createWorkPackage(nil, []string{"Build reusable skill"}) diff --git a/cmd/update/update.go b/cmd/update/update.go index 801199a..b581004 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -43,6 +43,12 @@ func addWorkPackageFlags() { "", "Change the subject of the work package", ) + workPackageCmd.Flags().StringVar( + &descriptionFlag, + "description", + "", + "Change the raw work package description", + ) workPackageCmd.Flags().StringVarP( &typeFlag, "type", diff --git a/cmd/update/work_package.go b/cmd/update/work_package.go index d5ff8de..478d71e 100644 --- a/cmd/update/work_package.go +++ b/cmd/update/work_package.go @@ -17,6 +17,8 @@ var ( actionFlag string assigneeFlag uint64 attachFlag string + descriptionFlag string + descriptionFlagChanged bool dryRunUpdateWorkPackage bool printUpdatedWorkPackageAsJSON bool setFlags []string @@ -32,7 +34,11 @@ provided by a flag is executed on its own.`, Run: updateWorkPackage, } -func updateWorkPackage(_ *cobra.Command, args []string) { +func updateWorkPackage(cmd *cobra.Command, args []string) { + if cmd != nil { + descriptionFlagChanged = cmd.Flags().Changed("description") + } + if len(args) != 1 { printUpdateError("invalid_argument", fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) return @@ -145,6 +151,7 @@ func updateWorkPackage(_ *cobra.Command, args []string) { printUpdateError("api_error", err.Error()) return } + printer.Error(err) return } @@ -184,6 +191,9 @@ func updateOptions() map[work_packages.UpdateOption]string { if len(subjectFlag) > 0 { options[work_packages.UpdateSubject] = subjectFlag } + if descriptionFlagChanged { + options[work_packages.UpdateDescription] = descriptionFlag + } if len(typeFlag) > 0 { options[work_packages.UpdateType] = typeFlag } @@ -196,6 +206,13 @@ func validateUpdateWorkPackageFlags() error { return fmt.Errorf("cannot use --dry-run without --json") } + if descriptionFlagChanged { + conflicts := activeDescriptionConflicts() + if len(conflicts) > 0 { + return fmt.Errorf("cannot combine --description with %s", strings.Join(conflicts, ", ")) + } + } + if len(setFlags) > 0 && hasLegacyUpdateFlags() { return fmt.Errorf("cannot combine --set with %s", strings.Join(activeLegacyUpdateFlags(), ", ")) } @@ -222,6 +239,9 @@ func activeLegacyUpdateFlags() []string { if len(subjectFlag) > 0 { flags = append(flags, "--subject") } + if descriptionFlagChanged { + flags = append(flags, "--description") + } if len(typeFlag) > 0 { flags = append(flags, "--type") } @@ -229,6 +249,19 @@ func activeLegacyUpdateFlags() []string { return flags } +func activeDescriptionConflicts() []string { + flags := []string{} + + if len(actionFlag) > 0 { + flags = append(flags, "--action") + } + if len(attachFlag) > 0 { + flags = append(flags, "--attach") + } + + return flags +} + func printUpdateError(code, message string) { if !printUpdatedWorkPackageAsJSON { printer.ErrorText(message) diff --git a/cmd/update/work_package_test.go b/cmd/update/work_package_test.go index ed7311a..1144fa1 100644 --- a/cmd/update/work_package_test.go +++ b/cmd/update/work_package_test.go @@ -1,6 +1,7 @@ package update import ( + "encoding/json" "io" "net/http" "net/http/httptest" @@ -8,6 +9,8 @@ import ( "strings" "testing" + "github.com/spf13/cobra" + "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/components/routes" @@ -53,6 +56,8 @@ func TestUpdateWorkPackagePrintsDryRunJSON(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "" typeFlag = "" setFlags = []string{"Votes=3"} @@ -67,10 +72,63 @@ func TestUpdateWorkPackagePrintsDryRunJSON(t *testing.T) { } } +func TestUpdateWorkPackagePrintsDryRunJSONWithSubjectAndDescription(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Old subject", + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + descriptionFlag = "Body" + descriptionFlagChanged = true + subjectFlag = "New subject" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = true + + updateWorkPackage(nil, []string{"74416"}) + + expected := "{\"valid\":true,\"operation\":\"update\",\"work_package_id\":74416,\"subject\":\"New subject\",\"description\":\"Body\",\"resolved_fields\":{}}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + func TestValidateUpdateWorkPackageFlagsRejectsDryRunWithoutJSON(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "" typeFlag = "" setFlags = []string{"Votes=3"} @@ -87,6 +145,8 @@ func TestValidateUpdateWorkPackageFlagsRejectsMixedSetAndLegacyFlags(t *testing. actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "new subject" typeFlag = "" setFlags = []string{"Votes=3"} @@ -99,6 +159,78 @@ func TestValidateUpdateWorkPackageFlagsRejectsMixedSetAndLegacyFlags(t *testing. } } +func TestValidateUpdateWorkPackageFlagsRejectsDescriptionAndSet(t *testing.T) { + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + descriptionFlag = "Body" + descriptionFlagChanged = true + subjectFlag = "" + typeFlag = "" + setFlags = []string{"Votes=3"} + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = false + + err := validateUpdateWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestValidateUpdateWorkPackageFlagsRejectsDescriptionAndAttach(t *testing.T) { + actionFlag = "" + assigneeFlag = 0 + attachFlag = "/tmp/file.txt" + descriptionFlag = "Body" + descriptionFlagChanged = true + subjectFlag = "" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = false + dryRunUpdateWorkPackage = false + + err := validateUpdateWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestValidateUpdateWorkPackageFlagsRejectsDescriptionAndAction(t *testing.T) { + actionFlag = "Claim" + assigneeFlag = 0 + attachFlag = "" + descriptionFlag = "Body" + descriptionFlagChanged = true + subjectFlag = "" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = false + dryRunUpdateWorkPackage = false + + err := validateUpdateWorkPackageFlags() + if err == nil { + t.Fatal("expected validation error") + } +} + +func TestValidateUpdateWorkPackageFlagsAllowsDescriptionAndSubject(t *testing.T) { + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + descriptionFlag = "Body" + descriptionFlagChanged = true + subjectFlag = "New subject" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = true + dryRunUpdateWorkPackage = true + + err := validateUpdateWorkPackageFlags() + if err != nil { + t.Fatalf("expected validation success, got %v", err) + } +} + func TestUpdateWorkPackagePrintsJSONWithoutChildrenQuery(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -156,6 +288,8 @@ func TestUpdateWorkPackagePrintsJSONWithoutChildrenQuery(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "" typeFlag = "" setFlags = []string{"Votes=3"} @@ -211,6 +345,8 @@ func TestUpdateWorkPackagePrintsAmbiguousFieldJSONError(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "" typeFlag = "" setFlags = []string{"KPI=3"} @@ -265,6 +401,8 @@ func TestUpdateWorkPackagePrintsDuplicateFieldJSONError(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "" typeFlag = "" setFlags = []string{"Votes=3", "customField130=4"} @@ -358,6 +496,8 @@ func TestUpdateWorkPackagePrintsAPIErrorWhenPatchFails(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "New subject" typeFlag = "" setFlags = nil @@ -407,6 +547,8 @@ func TestUpdateWorkPackageWithoutFlagsPrintsCurrentWorkPackage(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "" typeFlag = "" setFlags = nil @@ -464,6 +606,8 @@ func TestUpdateWorkPackagePrintsHumanProgressForNonJSONUpdate(t *testing.T) { actionFlag = "" assigneeFlag = 0 attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false subjectFlag = "New subject" typeFlag = "" setFlags = nil @@ -476,3 +620,73 @@ func TestUpdateWorkPackagePrintsHumanProgressForNonJSONUpdate(t *testing.T) { t.Fatalf("expected human progress output, got %q", activePrinter.Result) } } + +func TestUpdateWorkPackageClearsDescriptionViaEmptyFlag(t *testing.T) { + var patchBody map[string]any + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Subject", + "description": {"raw": "Existing body"}, + "lockVersion": 3, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + _ = json.NewDecoder(r.Body).Decode(&patchBody) + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + printer.Init(&printer.TestingPrinter{}) + + actionFlag = "" + assigneeFlag = 0 + attachFlag = "" + descriptionFlag = "" + descriptionFlagChanged = false + subjectFlag = "" + typeFlag = "" + setFlags = nil + printUpdatedWorkPackageAsJSON = false + dryRunUpdateWorkPackage = false + + cmd := &cobra.Command{Use: "workpackage"} + cmd.Flags().StringVar(&descriptionFlag, "description", "", "") + if err := cmd.ParseFlags([]string{"--description", ""}); err != nil { + t.Fatal(err) + } + + updateWorkPackage(cmd, []string{"74416"}) + + description, ok := patchBody["description"].(map[string]any) + if !ok { + t.Fatalf("expected description object in PATCH body, got %#v", patchBody["description"]) + } + if description["raw"] != "" { + t.Fatalf("expected empty description raw, got %#v", description["raw"]) + } +} diff --git a/components/resources/work_packages/create.go b/components/resources/work_packages/create.go index 849722c..86a7e05 100644 --- a/components/resources/work_packages/create.go +++ b/components/resources/work_packages/create.go @@ -20,11 +20,13 @@ const ( CreateSubject CreateOption = iota CreateParent CreateType + CreateDescription ) var createMap = map[CreateOption]func(projectId uint64, workPackage *dtos.WorkPackageDto, input string) error{ - CreateSubject: subjectCreate, - CreateParent: parentCreate, + CreateSubject: subjectCreate, + CreateParent: parentCreate, + CreateDescription: descriptionCreate, } func subjectCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) error { @@ -47,6 +49,12 @@ func parentCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) erro return nil } +func descriptionCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) error { + workPackage.Description = &dtos.LongTextDto{Raw: input} + + return nil +} + func setTypeLink(workPackage *dtos.WorkPackageDto, foundType *dtos.TypeDto) { if workPackage.Links == nil { workPackage.Links = &dtos.WorkPackageLinksDto{} @@ -76,8 +84,9 @@ func DryRunCreate(projectId uint64, options map[CreateOption]string) (*models.Wo ProjectID: resolved.ProjectID, ParentID: resolved.ParentID, WorkPackage: models.WorkPackageDraft{ - Subject: resolved.Options[CreateSubject], - Type: resolved.TypeName, + Subject: resolved.Options[CreateSubject], + Type: resolved.TypeName, + Description: resolved.Options[CreateDescription], }, } @@ -87,7 +96,7 @@ func DryRunCreate(projectId uint64, options map[CreateOption]string) (*models.Wo func create(resolved *resolvedCreate) (*models.WorkPackage, error) { workPackage := dtos.WorkPackageDto{} - for _, option := range []CreateOption{CreateSubject, CreateParent} { + for _, option := range []CreateOption{CreateSubject, CreateParent, CreateDescription} { value, ok := resolved.Options[option] if !ok { continue diff --git a/components/resources/work_packages/create_test.go b/components/resources/work_packages/create_test.go index c02e787..fe7da46 100644 --- a/components/resources/work_packages/create_test.go +++ b/components/resources/work_packages/create_test.go @@ -1,6 +1,7 @@ package work_packages_test import ( + "encoding/json" "io" "net/http" "net/http/httptest" @@ -134,3 +135,123 @@ func TestDryRunCreateWithTypeResolvesTypeName(t *testing.T) { t.Fatalf("expected type Implementation, got %+v", plan.WorkPackage) } } + +func TestDryRunCreateWithDescriptionIncludesDescription(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74316": + _, _ = io.WriteString(w, `{ + "id": 74316, + "subject": "Expand op CLI to support scripted work package workflows", + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74316"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "identifier": "cli", + "name": "CLI", + "_links": { + "types": {"href": "/api/v3/projects/1482/types/available"} + } + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 7, + "name": "Implementation", + "_links": { + "self": {"href": "/api/v3/types/7"} + } + } + ] + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + plan, err := work_packages.DryRunCreate(0, map[work_packages.CreateOption]string{ + work_packages.CreateSubject: "Add explicit work package description support", + work_packages.CreateParent: "74316", + work_packages.CreateType: "Implementation", + work_packages.CreateDescription: "Body", + }) + if err != nil { + t.Fatal(err) + } + + if plan.WorkPackage.Description != "Body" { + t.Fatalf("expected description Body, got %+v", plan.WorkPackage) + } +} + +func TestCreatePatchIncludesDescriptionAlone(t *testing.T) { + var postBody map[string]any + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/projects/1482/work_packages": + if r.Method != http.MethodPost { + t.Fatalf("unexpected method %s", r.Method) + } + if err := json.NewDecoder(r.Body).Decode(&postBody); err != nil { + t.Fatal(err) + } + + description, ok := postBody["description"].(map[string]any) + if !ok { + t.Fatalf("expected description object in POST body, got %#v", postBody["description"]) + } + if description["raw"] != "Only a description" { + t.Fatalf("expected description raw %q, got %#v", "Only a description", description["raw"]) + } + + w.WriteHeader(http.StatusCreated) + _, _ = io.WriteString(w, `{ + "id": 99999, + "subject": "", + "_links": {"self": {"href": "/api/v3/work_packages/99999"}} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + _, err = work_packages.Create(1482, map[work_packages.CreateOption]string{ + work_packages.CreateDescription: "Only a description", + }) + if err != nil { + t.Fatalf("Create returned error: %v", err) + } +} diff --git a/components/resources/work_packages/update.go b/components/resources/work_packages/update.go index 2cb28c6..55593e9 100644 --- a/components/resources/work_packages/update.go +++ b/components/resources/work_packages/update.go @@ -6,7 +6,6 @@ import ( "fmt" "strconv" - "github.com/opf/openproject-cli/components/common" "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/paths" "github.com/opf/openproject-cli/components/printer" @@ -22,15 +21,17 @@ const ( UpdateAssignee UpdateAttachment UpdateSubject + UpdateDescription UpdateType ) -var patchableUpdates = []UpdateOption{UpdateSubject, UpdateType, UpdateAssignee} +var patchableUpdates = []UpdateOption{UpdateSubject, UpdateType, UpdateAssignee, UpdateDescription} var patchMap = map[UpdateOption]func(patch, workPackage *dtos.WorkPackageDto, input string) (string, error){ - UpdateAssignee: assigneePatch, - UpdateType: typePatch, - UpdateSubject: subjectPatch, + UpdateAssignee: assigneePatch, + UpdateType: typePatch, + UpdateSubject: subjectPatch, + UpdateDescription: descriptionPatch, } func DryRunUpdate(id uint64, options map[UpdateOption]string) (*models.WorkPackageUpdatePlan, error) { @@ -49,6 +50,10 @@ func DryRunUpdate(id uint64, options map[UpdateOption]string) (*models.WorkPacka ResolvedFields: map[string]models.ResolvedField{}, } + if description, ok := options[UpdateDescription]; ok { + plan.Description = &description + } + if assignee, ok := options[UpdateAssignee]; ok { plan.Assignee = assignee } @@ -113,8 +118,9 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er var patchNeeded = false patchDto := dtos.WorkPackageDto{LockVersion: workPackage.LockVersion} - for option, value := range options { - if !common.Contains(patchableUpdates, option) { + for _, option := range patchableUpdates { + value, ok := options[option] + if !ok { continue } @@ -139,7 +145,6 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er if err != nil { return err } - return nil } @@ -186,3 +191,8 @@ func assigneePatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) patch.Links.Assignee = &dtos.LinkDto{Href: paths.User(userId)} return fmt.Sprintf("Assignee -> %s", input), nil } + +func descriptionPatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { + patch.Description = &dtos.LongTextDto{Raw: input} + return "Description updated", nil +} diff --git a/components/resources/work_packages/update_test.go b/components/resources/work_packages/update_test.go new file mode 100644 index 0000000..3bc669a --- /dev/null +++ b/components/resources/work_packages/update_test.go @@ -0,0 +1,296 @@ +package work_packages_test + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestUpdatePatchIncludesSubjectAndDescription(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Old subject", + "lockVersion": 7, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + var body map[string]any + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatal(err) + } + + description, ok := body["description"].(map[string]any) + if !ok { + t.Fatalf("expected description object, got %#v", body["description"]) + } + + if body["subject"] != "New subject" { + t.Fatalf("expected subject New subject, got %#v", body["subject"]) + } + + if description["raw"] != "Body" { + t.Fatalf("expected description raw Body, got %#v", description["raw"]) + } + + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update(74416, map[work_packages.UpdateOption]string{ + work_packages.UpdateSubject: "New subject", + work_packages.UpdateDescription: "Body", + }) + if err != nil { + t.Fatal(err) + } +} + +func TestDryRunUpdateIncludesAllLegacyFields(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Old subject", + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "identifier": "cli", + "name": "CLI", + "_links": { + "types": {"href": "/api/v3/projects/1482/types/available"} + } + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + { + "id": 7, + "name": "Implementation", + "_links": {"self": {"href": "/api/v3/types/7"}} + }, + { + "id": 6, + "name": "Feature", + "_links": {"self": {"href": "/api/v3/types/6"}} + } + ] + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + printer.Init(&printer.TestingPrinter{}) + + cases := []struct { + name string + options map[work_packages.UpdateOption]string + check func(t *testing.T, plan any) + }{ + { + name: "subject only", + options: map[work_packages.UpdateOption]string{work_packages.UpdateSubject: "Renamed"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "subject", "Renamed") + }, + }, + { + name: "type by name resolves against project types", + options: map[work_packages.UpdateOption]string{work_packages.UpdateType: "Feature"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "type", "Feature") + }, + }, + { + name: "assignee is echoed back", + options: map[work_packages.UpdateOption]string{work_packages.UpdateAssignee: "42"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "assignee", "42") + }, + }, + { + name: "description round-trips", + options: map[work_packages.UpdateOption]string{work_packages.UpdateDescription: "New body"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "description", "New body") + }, + }, + { + name: "action surfaces as preview", + options: map[work_packages.UpdateOption]string{work_packages.UpdateCustomAction: "Claim"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "action", "Claim") + }, + }, + { + name: "attach surfaces as preview", + options: map[work_packages.UpdateOption]string{work_packages.UpdateAttachment: "/tmp/f.txt"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "attach", "/tmp/f.txt") + }, + }, + { + name: "combined legacy fields all appear", + options: map[work_packages.UpdateOption]string{ + work_packages.UpdateSubject: "S", + work_packages.UpdateDescription: "D", + work_packages.UpdateAssignee: "7", + }, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "subject", "S") + assertPlanField(t, plan, "description", "D") + assertPlanField(t, plan, "assignee", "7") + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + plan, err := work_packages.DryRunUpdate(74416, tc.options) + if err != nil { + t.Fatalf("DryRunUpdate returned error: %v", err) + } + + marshalled, err := json.Marshal(plan) + if err != nil { + t.Fatal(err) + } + + var unmarshalled map[string]any + if err := json.Unmarshal(marshalled, &unmarshalled); err != nil { + t.Fatal(err) + } + + if unmarshalled["valid"] != true { + t.Fatalf("expected valid:true, got %#v", unmarshalled["valid"]) + } + + tc.check(t, unmarshalled) + }) + } +} + +func TestDryRunUpdateReturnsErrorForMissingWorkPackage(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, `{"message":"not found"}`) + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.DryRunUpdate(999999, map[work_packages.UpdateOption]string{ + work_packages.UpdateSubject: "x", + }) + if err == nil { + t.Fatal("expected error for missing work package, got nil") + } +} + +func TestDryRunUpdateReturnsErrorForUnresolvedType(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + _, _ = io.WriteString(w, `{ + "id": 74416, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"} + } + }`) + case "/api/v3/projects/1482": + _, _ = io.WriteString(w, `{ + "id": 1482, + "_links": {"types": {"href": "/api/v3/projects/1482/types/available"}} + }`) + case "/api/v3/projects/1482/types/available": + _, _ = io.WriteString(w, `{"_embedded":{"elements":[]}}`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.DryRunUpdate(74416, map[work_packages.UpdateOption]string{ + work_packages.UpdateType: "Nonsense", + }) + if err == nil { + t.Fatal("expected error for unresolved type, got nil") + } +} + +func assertPlanField(t *testing.T, plan any, field string, expected string) { + t.Helper() + m, ok := plan.(map[string]any) + if !ok { + t.Fatalf("expected map, got %T", plan) + } + if m[field] != expected { + t.Fatalf("expected %s=%q, got %#v", field, expected, m[field]) + } +} diff --git a/models/work_package_mutation.go b/models/work_package_mutation.go index 8af9082..f1391ec 100644 --- a/models/work_package_mutation.go +++ b/models/work_package_mutation.go @@ -1,8 +1,9 @@ package models type WorkPackageDraft struct { - Subject string `json:"subject"` - Type string `json:"type"` + Subject string `json:"subject"` + Type string `json:"type"` + Description string `json:"description"` } type WorkPackageCreatePlan struct { From 87900d70e4f818f9642e0b280fc7eb09cdae6021 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 24 Apr 2026 22:21:25 +0200 Subject: [PATCH 5/9] fix typo: workpackge -> workpackage in README examples --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0abbb5d..04195eb 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ projects -- Lists projects workpackages -- Lists work packages # Discover flags: hitting completion key after -op update workpackge 42 - +op update workpackage 42 - # returns --action -a -- Executes a custom action on a work package --assignee -- Assign a user to the work package @@ -147,10 +147,10 @@ of examples, that might be useful for a great number of people. # Creating a work package in a project only by subject. # Work package is created with many default values (as for type and status), # very similar to how a work package is created inline in a work package table. -op create workpackge --project 11 'Document new CLI tool' +op create workpackage --project 11 'Document new CLI tool' # Same command with shorthands and directly open it in a browser to continue working on it. -op create workpackge -p11 'Document new CLI tool' -o +op create workpackage -p11 'Document new CLI tool' -o # Validating the creation of a child work package without persisting it. # The parent determines the project automatically. From e448b8c3b5676867a21678041572ad00cd7a4185 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Fri, 24 Apr 2026 23:34:41 +0200 Subject: [PATCH 6/9] Restore update patch summaries Move the human-readable update summary into cmd/update so non-JSON users still see which fields are about to change while the resource layer stays silent for machine-oriented paths. Simplify patch helpers to return only errors now that the resource layer no longer builds its own summary. --- cmd/update/work_package.go | 23 ++++++++++++- cmd/update/work_package_test.go | 10 ++++-- components/resources/work_packages/update.go | 36 +++++++------------- 3 files changed, 42 insertions(+), 27 deletions(-) diff --git a/cmd/update/work_package.go b/cmd/update/work_package.go index 478d71e..282a95f 100644 --- a/cmd/update/work_package.go +++ b/cmd/update/work_package.go @@ -142,7 +142,10 @@ func updateWorkPackage(cmd *cobra.Command, args []string) { } if !printUpdatedWorkPackageAsJSON { - printer.Info("Updating work package ...") + printer.Info("Updating work package with patch ...") + if summary := updateSummaryLines(options); len(summary) > 0 { + printer.Info("\t" + strings.Join(summary, "\n\t")) + } } workPackage, err := work_packages.Update(id, options) @@ -173,10 +176,28 @@ func updateWorkPackage(cmd *cobra.Command, args []string) { return } + printer.Done() printer.Info("-- ") printer.WorkPackage(workPackage) } +func updateSummaryLines(options map[work_packages.UpdateOption]string) []string { + var lines []string + if v, ok := options[work_packages.UpdateSubject]; ok { + lines = append(lines, "Subject -> "+v) + } + if v, ok := options[work_packages.UpdateType]; ok { + lines = append(lines, "Type -> "+v) + } + if v, ok := options[work_packages.UpdateAssignee]; ok { + lines = append(lines, "Assignee -> "+v) + } + if v, ok := options[work_packages.UpdateDescription]; ok { + lines = append(lines, "Description -> "+v) + } + return lines +} + func updateOptions() map[work_packages.UpdateOption]string { var options = make(map[work_packages.UpdateOption]string) if len(actionFlag) > 0 { diff --git a/cmd/update/work_package_test.go b/cmd/update/work_package_test.go index 1144fa1..8970bf9 100644 --- a/cmd/update/work_package_test.go +++ b/cmd/update/work_package_test.go @@ -616,8 +616,14 @@ func TestUpdateWorkPackagePrintsHumanProgressForNonJSONUpdate(t *testing.T) { updateWorkPackage(nil, []string{"74416"}) - if !strings.Contains(activePrinter.Result, "Updating work package ...") { - t.Fatalf("expected human progress output, got %q", activePrinter.Result) + if !strings.Contains(activePrinter.Result, "Updating work package with patch ...") { + t.Fatalf("expected human progress header, got %q", activePrinter.Result) + } + if !strings.Contains(activePrinter.Result, "Subject -> New subject") { + t.Fatalf("expected per-field summary line, got %q", activePrinter.Result) + } + if !strings.Contains(activePrinter.Result, "DONE") { + t.Fatalf("expected DONE marker, got %q", activePrinter.Result) } } diff --git a/components/resources/work_packages/update.go b/components/resources/work_packages/update.go index 55593e9..9519ae6 100644 --- a/components/resources/work_packages/update.go +++ b/components/resources/work_packages/update.go @@ -8,7 +8,6 @@ import ( "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/paths" - "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/dtos" "github.com/opf/openproject-cli/models" @@ -27,7 +26,7 @@ const ( var patchableUpdates = []UpdateOption{UpdateSubject, UpdateType, UpdateAssignee, UpdateDescription} -var patchMap = map[UpdateOption]func(patch, workPackage *dtos.WorkPackageDto, input string) (string, error){ +var patchMap = map[UpdateOption]func(patch, workPackage *dtos.WorkPackageDto, input string) error{ UpdateAssignee: assigneePatch, UpdateType: typePatch, UpdateSubject: subjectPatch, @@ -125,11 +124,9 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er } patchNeeded = true - updateStringLine, err := patchMap[option](&patchDto, workPackage, value) - if err != nil { + if err := patchMap[option](&patchDto, workPackage, value); err != nil { return err } - _ = updateStringLine } if !patchNeeded { @@ -148,24 +145,15 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er return nil } -func typePatch(patch, workPackage *dtos.WorkPackageDto, input string) (string, error) { +func typePatch(patch, workPackage *dtos.WorkPackageDto, input string) error { types, err := availableTypes(workPackage.Links.Project) if err != nil { - return "", err + return err } foundType := findType(input, types) if foundType == nil { - printer.ErrorText("Failed to update work package type.") - printer.Info(fmt.Sprintf( - "No unique available type from input %s found for project %s. Please use one of the types listed below.", - printer.Cyan(input), - printer.Red(fmt.Sprintf("#%d", parser.IdFromLink(workPackage.Links.Project.Href))), - )) - - printer.Types(types.Convert()) - - return "", nil + return fmt.Errorf("no unique available type from input %q found for project #%d", input, parser.IdFromLink(workPackage.Links.Project.Href)) } if patch.Links == nil { @@ -173,15 +161,15 @@ func typePatch(patch, workPackage *dtos.WorkPackageDto, input string) (string, e } patch.Links.Type = foundType.Links.Self - return fmt.Sprintf("Type -> %s", foundType.Name), nil + return nil } -func subjectPatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { +func subjectPatch(patch, _ *dtos.WorkPackageDto, input string) error { patch.Subject = input - return fmt.Sprintf("Subject -> %s", input), nil + return nil } -func assigneePatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { +func assigneePatch(patch, _ *dtos.WorkPackageDto, input string) error { userId, _ := strconv.ParseUint(input, 10, 64) if patch.Links == nil { @@ -189,10 +177,10 @@ func assigneePatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) } patch.Links.Assignee = &dtos.LinkDto{Href: paths.User(userId)} - return fmt.Sprintf("Assignee -> %s", input), nil + return nil } -func descriptionPatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { +func descriptionPatch(patch, _ *dtos.WorkPackageDto, input string) error { patch.Description = &dtos.LongTextDto{Raw: input} - return "Description updated", nil + return nil } From f065aee1acb9458bc8db0bf8197644c5ef8cee48 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 25 Apr 2026 00:26:27 +0200 Subject: [PATCH 7/9] [#74415] fix formattable custom field updates Wrap Formattable custom-field values as raw long-text objects before PATCHing so OpenProject preserves the content instead of clearing it. Add regression coverage for both value coercion and the resulting HTTP patch body. https://community.openproject.org/wp/74415 --- .../work_packages/field_resolution.go | 4 +- .../work_packages/field_resolution_test.go | 22 +++++++ .../work_packages/update_fields_test.go | 61 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/components/resources/work_packages/field_resolution.go b/components/resources/work_packages/field_resolution.go index 6685f56..1c36e2a 100644 --- a/components/resources/work_packages/field_resolution.go +++ b/components/resources/work_packages/field_resolution.go @@ -95,8 +95,10 @@ func resolveSchemaField(schema *Schema, key string) (*SchemaField, error) { func coerceFieldValue(field *SchemaField, raw string) (any, error) { switch field.Type { - case "String", "Formattable": + case "String": return raw, nil + case "Formattable": + return map[string]any{"raw": raw}, nil case "Integer": value, err := strconv.ParseInt(raw, 10, 64) if err != nil { diff --git a/components/resources/work_packages/field_resolution_test.go b/components/resources/work_packages/field_resolution_test.go index 443d02b..ccebf4a 100644 --- a/components/resources/work_packages/field_resolution_test.go +++ b/components/resources/work_packages/field_resolution_test.go @@ -65,3 +65,25 @@ func TestResolveFieldAssignmentsRejectsDuplicateAPIFields(t *testing.T) { t.Fatalf("expected ErrDuplicateField, got %v", err) } } + +func TestResolveFieldAssignmentsCoercesFormattableFields(t *testing.T) { + schema := &Schema{ + Fields: []SchemaField{ + {APIName: "customField401", Label: "Acceptance criteria", Type: "Formattable", Writable: true}, + }, + } + + resolved, err := resolveFieldAssignments(schema, []string{"Acceptance criteria=Line one\nLine two"}) + if err != nil { + t.Fatal(err) + } + + field := resolved["Acceptance criteria"] + value, ok := field.Value.(map[string]any) + if !ok { + t.Fatalf("expected formattable value map, got %#v", field.Value) + } + if value["raw"] != "Line one\nLine two" { + t.Fatalf("expected raw long text, got %#v", value["raw"]) + } +} diff --git a/components/resources/work_packages/update_fields_test.go b/components/resources/work_packages/update_fields_test.go index 90b4568..9f8deff 100644 --- a/components/resources/work_packages/update_fields_test.go +++ b/components/resources/work_packages/update_fields_test.go @@ -1,6 +1,7 @@ package work_packages_test import ( + "encoding/json" "io" "net/http" "net/http/httptest" @@ -54,3 +55,63 @@ func TestDryRunUpdateFieldsResolvesLabels(t *testing.T) { t.Fatalf("unexpected plan: %+v", plan) } } + +func TestUpdateFieldsPatchesFormattableCustomFieldsAsLongText(t *testing.T) { + var patchBody map[string]any + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74172": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74172, + "subject": "Epic", + "lockVersion": 5, + "_links": { + "self": {"href": "/api/v3/work_packages/74172"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Feature"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + if err := json.NewDecoder(r.Body).Decode(&patchBody); err != nil { + t.Fatal(err) + } + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField401": {"name": "Acceptance criteria", "type": "Formattable", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + if err := work_packages.UpdateFields(74172, []string{"Acceptance criteria=Native HTML5 DnD was rejected.\nDragula was rejected."}); err != nil { + t.Fatal(err) + } + + value, ok := patchBody["customField401"].(map[string]any) + if !ok { + t.Fatalf("expected long-text patch object, got %#v", patchBody["customField401"]) + } + if value["raw"] != "Native HTML5 DnD was rejected.\nDragula was rejected." { + t.Fatalf("expected raw long text, got %#v", value["raw"]) + } +} From 6a6827606777c9d9cabc31551d4c1ab628a9cf56 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 25 Apr 2026 00:26:37 +0200 Subject: [PATCH 8/9] [#74316] add work package status updates Add a status flag on work package updates and validate it through the same status lookup in both dry-run and live PATCH flows. Extend the mutation plan and add regression coverage for status resolution and the resulting PATCH link payload. https://community.openproject.org/wp/74316 --- cmd/update/update.go | 6 + cmd/update/work_package.go | 10 ++ components/paths/paths.go | 4 + components/resources/work_packages/update.go | 45 ++++++- .../resources/work_packages/update_test.go | 123 ++++++++++++++++++ models/work_package_mutation.go | 1 + 6 files changed, 188 insertions(+), 1 deletion(-) diff --git a/cmd/update/update.go b/cmd/update/update.go index b581004..68a3ffe 100644 --- a/cmd/update/update.go +++ b/cmd/update/update.go @@ -49,6 +49,12 @@ func addWorkPackageFlags() { "", "Change the raw work package description", ) + workPackageCmd.Flags().StringVar( + &statusFlag, + "status", + "", + "Change the status of the work package (by name)", + ) workPackageCmd.Flags().StringVarP( &typeFlag, "type", diff --git a/cmd/update/work_package.go b/cmd/update/work_package.go index 282a95f..2b3b81d 100644 --- a/cmd/update/work_package.go +++ b/cmd/update/work_package.go @@ -22,6 +22,7 @@ var ( dryRunUpdateWorkPackage bool printUpdatedWorkPackageAsJSON bool setFlags []string + statusFlag string subjectFlag string typeFlag string ) @@ -195,6 +196,9 @@ func updateSummaryLines(options map[work_packages.UpdateOption]string) []string if v, ok := options[work_packages.UpdateDescription]; ok { lines = append(lines, "Description -> "+v) } + if v, ok := options[work_packages.UpdateStatus]; ok { + lines = append(lines, "Status -> "+v) + } return lines } @@ -215,6 +219,9 @@ func updateOptions() map[work_packages.UpdateOption]string { if descriptionFlagChanged { options[work_packages.UpdateDescription] = descriptionFlag } + if len(statusFlag) > 0 { + options[work_packages.UpdateStatus] = statusFlag + } if len(typeFlag) > 0 { options[work_packages.UpdateType] = typeFlag } @@ -263,6 +270,9 @@ func activeLegacyUpdateFlags() []string { if descriptionFlagChanged { flags = append(flags, "--description") } + if len(statusFlag) > 0 { + flags = append(flags, "--status") + } if len(typeFlag) > 0 { flags = append(flags, "--type") } diff --git a/components/paths/paths.go b/components/paths/paths.go index 7561e84..c6df6a0 100644 --- a/components/paths/paths.go +++ b/components/paths/paths.go @@ -34,6 +34,10 @@ func Status() string { return Root() + "/statuses" } +func StatusById(id uint64) string { + return fmt.Sprintf("%s/%d", Status(), id) +} + func TimeEntries() string { return Root() + "/time_entries" } diff --git a/components/resources/work_packages/update.go b/components/resources/work_packages/update.go index 9519ae6..79ef8f5 100644 --- a/components/resources/work_packages/update.go +++ b/components/resources/work_packages/update.go @@ -5,10 +5,12 @@ import ( "encoding/json" "fmt" "strconv" + "strings" "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/paths" "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/status" "github.com/opf/openproject-cli/dtos" "github.com/opf/openproject-cli/models" ) @@ -22,15 +24,17 @@ const ( UpdateSubject UpdateDescription UpdateType + UpdateStatus ) -var patchableUpdates = []UpdateOption{UpdateSubject, UpdateType, UpdateAssignee, UpdateDescription} +var patchableUpdates = []UpdateOption{UpdateSubject, UpdateType, UpdateAssignee, UpdateDescription, UpdateStatus} var patchMap = map[UpdateOption]func(patch, workPackage *dtos.WorkPackageDto, input string) error{ UpdateAssignee: assigneePatch, UpdateType: typePatch, UpdateSubject: subjectPatch, UpdateDescription: descriptionPatch, + UpdateStatus: statusPatch, } func DryRunUpdate(id uint64, options map[UpdateOption]string) (*models.WorkPackageUpdatePlan, error) { @@ -44,6 +48,7 @@ func DryRunUpdate(id uint64, options map[UpdateOption]string) (*models.WorkPacka Operation: "update", WorkPackageID: id, Subject: options[UpdateSubject], + Status: options[UpdateStatus], Action: options[UpdateCustomAction], Attach: options[UpdateAttachment], ResolvedFields: map[string]models.ResolvedField{}, @@ -57,6 +62,15 @@ func DryRunUpdate(id uint64, options map[UpdateOption]string) (*models.WorkPacka plan.Assignee = assignee } + if value, ok := options[UpdateStatus]; ok { + resolvedStatus, err := resolveStatus(value) + if err != nil { + return nil, err + } + + plan.Status = resolvedStatus.Name + } + if value, ok := options[UpdateType]; ok { types, err := availableTypes(workPackage.Links.Project) if err != nil { @@ -184,3 +198,32 @@ func descriptionPatch(patch, _ *dtos.WorkPackageDto, input string) error { patch.Description = &dtos.LongTextDto{Raw: input} return nil } + +func statusPatch(patch, _ *dtos.WorkPackageDto, input string) error { + resolvedStatus, err := resolveStatus(input) + if err != nil { + return err + } + + if patch.Links == nil { + patch.Links = &dtos.WorkPackageLinksDto{} + } + + patch.Links.Status = &dtos.LinkDto{Href: paths.StatusById(resolvedStatus.Id)} + return nil +} + +func resolveStatus(input string) (*models.Status, error) { + statuses, err := status.All() + if err != nil { + return nil, err + } + + for _, candidate := range statuses { + if strings.EqualFold(candidate.Name, input) { + return candidate, nil + } + } + + return nil, fmt.Errorf("no status named %q found", input) +} diff --git a/components/resources/work_packages/update_test.go b/components/resources/work_packages/update_test.go index 3bc669a..6698d67 100644 --- a/components/resources/work_packages/update_test.go +++ b/components/resources/work_packages/update_test.go @@ -119,6 +119,15 @@ func TestDryRunUpdateIncludesAllLegacyFields(t *testing.T) { ] } }`) + case "/api/v3/statuses": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + {"id": 1, "name": "New"}, + {"id": 2, "name": "In development"} + ] + } + }`) default: t.Fatalf("unexpected path %s", r.URL.Path) } @@ -166,6 +175,13 @@ func TestDryRunUpdateIncludesAllLegacyFields(t *testing.T) { assertPlanField(t, plan, "description", "New body") }, }, + { + name: "status resolves against known statuses", + options: map[work_packages.UpdateOption]string{work_packages.UpdateStatus: "in development"}, + check: func(t *testing.T, plan any) { + assertPlanField(t, plan, "status", "In development") + }, + }, { name: "action surfaces as preview", options: map[work_packages.UpdateOption]string{work_packages.UpdateCustomAction: "Claim"}, @@ -284,6 +300,113 @@ func TestDryRunUpdateReturnsErrorForUnresolvedType(t *testing.T) { } } +func TestDryRunUpdateReturnsErrorForUnresolvedStatus(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + _, _ = io.WriteString(w, `{ + "id": 74416, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"} + } + }`) + case "/api/v3/statuses": + _, _ = io.WriteString(w, `{"_embedded":{"elements":[]}}`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.DryRunUpdate(74416, map[work_packages.UpdateOption]string{ + work_packages.UpdateStatus: "In progress", + }) + if err == nil { + t.Fatal("expected error for unresolved status, got nil") + } +} + +func TestUpdatePatchIncludesStatus(t *testing.T) { + var patchBody map[string]any + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74416": + switch r.Method { + case http.MethodGet: + _, _ = io.WriteString(w, `{ + "id": 74416, + "subject": "Old subject", + "lockVersion": 7, + "_links": { + "self": {"href": "/api/v3/work_packages/74416"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "status": {"href": "/api/v3/statuses/1", "title": "New"}, + "type": {"href": "/api/v3/types/7", "title": "Implementation"}, + "assignee": {"href": null, "title": ""} + } + }`) + case http.MethodPatch: + if err := json.NewDecoder(r.Body).Decode(&patchBody); err != nil { + t.Fatal(err) + } + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{}`) + default: + t.Fatalf("unexpected method %s", r.Method) + } + case "/api/v3/statuses": + _, _ = io.WriteString(w, `{ + "_embedded": { + "elements": [ + {"id": 1, "name": "New"}, + {"id": 2, "name": "In development"} + ] + } + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update(74416, map[work_packages.UpdateOption]string{ + work_packages.UpdateStatus: "In development", + }) + if err != nil { + t.Fatal(err) + } + + links, ok := patchBody["_links"].(map[string]any) + if !ok { + t.Fatalf("expected links object, got %#v", patchBody["_links"]) + } + status, ok := links["status"].(map[string]any) + if !ok { + t.Fatalf("expected status link object, got %#v", links["status"]) + } + if status["href"] != "/api/v3/statuses/2" { + t.Fatalf("expected status href /api/v3/statuses/2, got %#v", status["href"]) + } +} + func assertPlanField(t *testing.T, plan any, field string, expected string) { t.Helper() m, ok := plan.(map[string]any) diff --git a/models/work_package_mutation.go b/models/work_package_mutation.go index f1391ec..dbd025d 100644 --- a/models/work_package_mutation.go +++ b/models/work_package_mutation.go @@ -26,6 +26,7 @@ type WorkPackageUpdatePlan struct { Subject string `json:"subject,omitempty"` Type string `json:"type,omitempty"` Assignee string `json:"assignee,omitempty"` + Status string `json:"status,omitempty"` Description *string `json:"description,omitempty"` Action string `json:"action,omitempty"` Attach string `json:"attach,omitempty"` From e8afaa9ff18a01b7838b9e56bc4156c4f9930123 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sat, 25 Apr 2026 00:55:14 +0200 Subject: [PATCH 9/9] [#74418] normalize Formattable fields on read Normalize Formattable custom fields to their raw string values in Work Package JSON reads so they mirror description and can be round-tripped without parsing rendered objects. https://community.openproject.org/wp/74418 --- cmd/inspect/work_package_test.go | 63 +++++++++++++++++++ components/resources/work_packages/details.go | 41 +++++++++++- .../resources/work_packages/details_test.go | 48 ++++++++++++++ 3 files changed, 151 insertions(+), 1 deletion(-) diff --git a/cmd/inspect/work_package_test.go b/cmd/inspect/work_package_test.go index 1d81536..54699cb 100644 --- a/cmd/inspect/work_package_test.go +++ b/cmd/inspect/work_package_test.go @@ -150,6 +150,69 @@ func TestInspectWorkPackagePrintsJSONWithoutChildrenQuery(t *testing.T) { } } +func TestInspectWorkPackagePrintsFormattableFieldsAsRawStrings(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74172": + _, _ = io.WriteString(w, `{ + "id": 74172, + "subject": "Epic", + "description": {"raw": "Body"}, + "customField401": { + "format": "markdown", + "html": "

Accepted

", + "raw": "Accepted" + }, + "_embedded": { + "project": { + "id": 1482, + "identifier": "cli", + "name": "CLI" + } + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74172"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Epic"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{"customField401":{"name":"Acceptance criteria","type":"Formattable","writable":true}}`) + case "/api/v3/work_packages": + t.Fatalf("unexpected children query: %s", r.URL.Path) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + routes.Init(host) + + activePrinter := &printer.TestingPrinter{} + printer.Init(activePrinter) + + shouldOpenWorkPackageInBrowser = false + listAvailableTypes = false + includeChildrenInJson = false + printWorkPackageAsJSON = true + + inspectWorkPackage(nil, []string{"74172"}) + + expected := "{\"work_package\":{\"id\":74172,\"subject\":\"Epic\",\"type\":\"Epic\",\"status\":\"new\",\"assignee\":\"\",\"description\":\"Body\",\"parent_id\":null,\"project\":{\"id\":1482,\"identifier\":\"cli\",\"name\":\"CLI\"},\"fields\":{\"customField401\":\"Accepted\"},\"field_labels\":{\"Acceptance criteria\":[\"customField401\"]}},\"children\":[]}\n" + if activePrinter.Result != expected { + t.Fatalf("expected %s, got %s", expected, activePrinter.Result) + } +} + func TestValidateInspectWorkPackageFlagsRejectsOpenAndJSON(t *testing.T) { shouldOpenWorkPackageInBrowser = true listAvailableTypes = false diff --git a/components/resources/work_packages/details.go b/components/resources/work_packages/details.go index 816628b..62bcda9 100644 --- a/components/resources/work_packages/details.go +++ b/components/resources/work_packages/details.go @@ -80,7 +80,7 @@ func workPackageDetails(dto *dtos.WorkPackageDto, schema *Schema) models.WorkPac Description: longTextRaw(dto.Description), ParentID: linkID(dto.Links, func(links *dtos.WorkPackageLinksDto) *dtos.LinkDto { return links.Parent }), Project: project, - Fields: dto.CustomFields, + Fields: normalizeCustomFields(dto.CustomFields, schema), FieldLabels: schema.fieldLabels(), } } @@ -129,3 +129,42 @@ func linkID(links *dtos.WorkPackageLinksDto, selector func(*dtos.WorkPackageLink id := parser.IdFromLink(link.Href) return &id } + +func normalizeCustomFields(fields map[string]any, schema *Schema) map[string]any { + if len(fields) == 0 { + return fields + } + + typesByAPIName := make(map[string]string, len(schema.Fields)) + for _, field := range schema.Fields { + typesByAPIName[field.APIName] = field.Type + } + + normalized := make(map[string]any, len(fields)) + for apiName, value := range fields { + if typesByAPIName[apiName] == "Formattable" { + if rawValue, ok := rawLongTextValue(value); ok { + normalized[apiName] = rawValue + continue + } + } + + normalized[apiName] = value + } + + return normalized +} + +func rawLongTextValue(value any) (string, bool) { + object, ok := value.(map[string]any) + if !ok { + return "", false + } + + raw, ok := object["raw"].(string) + if !ok { + return "", false + } + + return raw, true +} diff --git a/components/resources/work_packages/details_test.go b/components/resources/work_packages/details_test.go index f6cd1ae..a920049 100644 --- a/components/resources/work_packages/details_test.go +++ b/components/resources/work_packages/details_test.go @@ -264,3 +264,51 @@ func TestInspectPreservesDuplicateFieldLabels(t *testing.T) { t.Fatalf("expected duplicate KPI mappings, got %#v", payload.WorkPackage.FieldLabels) } } + +func TestInspectNormalizesFormattableCustomFieldsToRawStrings(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v3/work_packages/74172": + _, _ = io.WriteString(w, `{ + "id": 74172, + "subject": "Epic", + "customField401": { + "format": "markdown", + "html": "

Body

", + "raw": "Body" + }, + "_links": { + "self": {"href": "/api/v3/work_packages/74172"}, + "project": {"href": "/api/v3/projects/1482", "title": "CLI"}, + "schema": {"href": "/api/v3/work_packages/schemas/1482-6"}, + "status": {"href": "/api/v3/statuses/1", "title": "new"}, + "type": {"href": "/api/v3/types/6", "title": "Epic"}, + "assignee": {"href": null, "title": ""} + } + }`) + case "/api/v3/work_packages/schemas/1482-6": + _, _ = io.WriteString(w, `{ + "customField401": {"name": "Acceptance criteria", "type": "Formattable", "writable": true} + }`) + default: + t.Fatalf("unexpected path %s", r.URL.Path) + } + })) + defer server.Close() + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + requests.Init(host, "token", false) + + payload, err := work_packages.Inspect(74172) + if err != nil { + t.Fatal(err) + } + + if payload.WorkPackage.Fields["customField401"] != "Body" { + t.Fatalf("expected Formattable field raw string, got %#v", payload.WorkPackage.Fields["customField401"]) + } +}