diff --git a/.claude/op.md b/.claude/op.md new file mode 100644 index 0000000..45d0977 --- /dev/null +++ b/.claude/op.md @@ -0,0 +1,186 @@ +# OpenProject CLI (`op`) + +`op` is a CLI for interacting with OpenProject project management instances. Use it proactively whenever the user discusses work packages, projects, time entries, notifications, or anything related to OpenProject — just as naturally as you'd use `git` for version control or `ls` to explore the filesystem. + +## Global flags + +These flags apply to every command: + +``` +--format string Output format: text (default) or json +--profile string Profile name (overrides OP_CLI_PROFILE env var, default "default") +--verbose Print verbose output +``` + +## Orientation + +```bash +op whoami # Show current user and server (verify connection) +op whoami --profile # Same for a specific profile +op project list # Discover available projects (get IDs for other commands) +``` + +## Identifiers + +OpenProject has different identifiers for project and work packages. Mixing them up causes silent wrong-WP bugs. + +Since 17.5, semantic identifiers can be enabled. Once enabled, work packages have project-based identifiers like Jira. + +### Project id + +The internal integer primary key of the database. Examples: `141` or `239`. In JSON output, the **`id`** field on a *project* object. + +### Project identifier + +The human-readable short string identifying the project uniquely. Unique across all projects of the instance. When semantic identifiers are enabled on the instance, they are alphanumeric characters (e.g., `OP`, `PROJ`, `JIM`). When not, they are slugs (e.g., `openproject`, `stream-jira-exit`, `jira-migrator`). In JSON output, the **`identifier`** field on a *project* object. + +### Work package id + +The internal integer primary key (e.g., `71305`). Unique across all projects on the instance. Always present regardless of instance configuration. This is the number in `/wp/71305` URLs. In JSON output: the **`id`** field of a *work-package* object. + +```bash +op work-package inspect 71305 +op work-package inspect --format json 71305 | jq .id # → 71305 +``` + +### Work package semantic identifier (aka display ID, aka project-based identifier) + +When enabled by an administrator, each work package gets a project-scoped human label of the form `{PREFIX}-{N}` (e.g., `SJF-6`, `AGILE-54`, `OP-18917`, `JIM-43`). The prefix is the project identifier. Designed to support Jira migrations (existing Jira issue keys can be preserved). Historical numerical IDs remain valid and continue resolving to the same work packages. + +When enabled, the CLI and list output show these instead of bare numbers. In JSON output: the **`display_id`** field of a *work-package* object (same as `id` on instances where the feature is off). + +```bash +op work-package inspect --format json 71305 | jq .display_id # → "AGILE-54" (if enabled) + # → "71305" (if not enabled) +``` + +> **Gotcha — never strip the display ID prefix:** when project-based identifiers are enabled, list output shows `OP-7756` or `AGILE-32`. Pass these as-is to `inspect` — stripping the prefix gives a bare integer that resolves to a completely different WP: +> ```bash +> op work-package inspect --format json OP-7756 # correct +> op work-package inspect --format json 7756 # WRONG — different unrelated WP +> ``` + +### Choosing between id and identifier + +Prefer the numeric id in scripts, prefer the semantic identifier in human-facing output like changelogs. + +--- + +## Work packages + +IDs accept either an ID (`42`) or an identifier (`PROJ-123`, when enabled) everywhere. + +```bash +op work-package list # All visible work packages +op work-package list -p # Filter by project (numeric ID or identifier/slug) +op work-package list -s open # Filter: open / closed / / comma-separated IDs +op work-package list -s '!' # Exclude a status (prefix with !) +op work-package list -a me # Filter by assignee (me or user ID) +op work-package list -t # Filter by type (comma-separated IDs, ! prefix to exclude) +op work-package list -v # Filter by version +op work-package list --not-version # Exclude a version +op work-package list --parent-id # Direct children of a work package +op work-package list --include-sub-projects # Include sub-project work packages +op work-package list --sub-project # Limit sub-projects to include (with --include-sub-projects) +op work-package list --not-sub-project # Exclude sub-projects (with --include-sub-projects) +op work-package list --timestamp 2025-01-01 # Work packages as of a date (YYYY-MM-DD or YYYY-MM-DDTHH:MM:SSZ) +op work-package list --total # Show only the total count + +op work-package search ... # Search by subject, type, status, project name, or identifier +op work-package search ... -p # Limit search to a project; multiple words are ANDed; up to 100 results + +op work-package inspect # Full details of one work package +op work-package inspect --types # List available types instead of showing the work package +op work-package inspect --open # Open in default browser + +op work-package create "Subject" -p # Create (project accepts numeric ID or slug) +op work-package create "Subject" -p \ + --type --assignee \ + --description "markdown text" --open # Full create with all options + +op work-package update --subject "…" # Update subject +op work-package update --type # Change type +op work-package update --assignee # Change assignee +op work-package update --description "…" # Update description (markdown) +op work-package update --action # Execute a custom action (e.g. status transition) +op work-package update --attach # Attach a file +``` + +## Time entries + +```bash +op time-entry list # Time entries for current user +op time-entry list -u # Time entries for a specific user + +op time-entry create \ + -w \ + --hours 1.5 \ + --activity "Development" \ + --spent-on 2025-01-15 \ + --comment "Fixed the bug" \ + -u # All flags; --spent-on defaults to today +``` + +## Budgets + +```bash +op budget list -p # Budgets for a project +op budget inspect # Full details of a budget +``` + +## Projects + +```bash +op project list # All visible projects +op project inspect # Project details (numeric ID or identifier/slug) +op project inspect --open # Open in default browser +``` + +## Other resources + +```bash +op notification list # Unread notifications +op notification list -r # Filter by reason + +op user search # Find a user + +op status list # List work package statuses +op type list # List work package types + +op activity list --work-package # Activities for a work package (flag is required) +``` + +## Git integration + +```bash +op git start workpackage # Create a branch from WP id/subject/type and switch to it +``` + +## Profiles (multiple OpenProject instances) + +The `--profile` flag selects which instance to talk to (stored in `~/.config/openproject/config`). The default profile is used when omitted. The env var `OP_CLI_PROFILE` also selects a profile. + +```bash +op login # Authenticate (prompts for host, token, profile name) +op login --profile work # Authenticate a named profile +op logout # Remove the default profile +op logout --profile work # Remove a named profile +op whoami # Show all profiles with their server and user +``` + +## JSON output + +Use `--format json` to get machine-readable output or raw markdown descriptions: + +```bash +op work-package inspect --format json +op work-package list --format json -p +``` + +## Workflow for use-case analysis + +1. `op whoami` — confirm which instance you're on +2. `op project list` — find relevant project IDs +3. `op work-package list -p -s open` — get the work items +4. `op work-package inspect --format json ` — drill into specifics (raw markdown) +5. `op work-package search -p ` — find by subject or identifier fragment diff --git a/.gitignore b/.gitignore index f9b658d..fe38f9e 100644 --- a/.gitignore +++ b/.gitignore @@ -2,8 +2,9 @@ .fleet .idea -# Compile output, if running `go build` on a linux/x64 +# Compile output openproject-cli +op # If you prefer the allow list template instead of the deny list, see community template: # https://github.com/github/gitignore/blob/main/community/Golang/Go.AllowList.gitignore @@ -26,3 +27,6 @@ openproject-cli # Go workspace file go.work + +# Claude local settings file +.claude/settings.local.json diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..d2e458b --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,114 @@ +# OpenProject CLI — Developer Guide + +## Build & Run + +```bash +# Build +go build -o op . + +# Run tests +go test ./... + +# Run a specific test package +go test ./components/printer/... + +# Install locally +go install . +``` + +## Architecture + +The codebase is organized in strict layers. Each layer only depends on layers below it. + +``` +cmd/ # Cobra commands — CLI entry points only, no business logic +components/ + resources/ # Business logic — API calls, option handling + paths/ # API path constants (paths.go — single file) + requests/ # HTTP client (GET, POST, PATCH) + parser/ # JSON response parsing + printer/ # Terminal output formatting + routes/ # Browser URL generation + common/ # Shared utilities (string, slice, math) + configuration/ # Multi-profile config (INI), CLI version + launch/ # Browser launcher +models/ # Domain models (plain structs, no logic) +dtos/ # JSON DTOs with Convert() to models +``` + +### Data flow + +API response → `parser.Parse[SomethingDto]()` → `dto.Convert()` → `models.Something` → `printer.Something()` + +### DTO conventions + +- DTOs live in `dtos/`, named `Dto` +- DTOs mirror the OpenProject API v3 HAL JSON structure +- Links use `*LinkDto` with `Href` and `Title` fields, serialized as `_links` +- Every DTO implements `Convert() *models.Something` to produce a domain model +- Collections follow the pattern: `CollectionDto` with `Embedded.Elements` +- `omitempty` on all JSON fields in DTOs used for POST/PATCH bodies +- `WorkPackageDto` includes a `displayId` field (camelCase, from the API) mapped to `DisplayId` on the model; always present — holds the semantic identifier (e.g. `PROJ-123`) when project-based identifiers are enabled, or the numeric id as a string otherwise + +### Command conventions + +- Commands follow `op ` (noun-first), e.g. `op work-package list`, `op time-entry create` +- Each noun has its own package under `cmd/` (`workpackage`, `timeentry`, `project`, `user`, …) — package names use no hyphens even when the command name does (e.g. `work-package` → `package workpackage`) +- Each noun package exposes a `RootCmd` registered in `cmd/root.go` +- One file per verb within each noun package (e.g. `cmd/workpackage/list.go`, `cmd/workpackage/create.go`) +- Flags: always define long flag names; add short flags (`-p`, `-o`) for frequently used ones +- Flags that resolve a resource (e.g. `--type`, `--assignee`) perform an API lookup and store the resolved link in the DTO + +### Resource conventions + +- Each resource has its own package under `components/resources/` +- Operation types use `iota` enums: `CreateOption`, `UpdateOption`, `FilterOption` +- Operations are dispatched via a `map[Option]func(...)` — add new options by extending the map +- Public API: `Create(...)`, `Lookup(id)`, `All(filters, query, ...)`, `Update(id, options)` + +### Configuration conventions + +- Config stored as INI at `~/.config/openproject/config` (or `$XDG_CONFIG_HOME/openproject/config`) +- Each profile is an INI section: `[name]` with `host` and `token` keys +- Profile names: letters, digits, `-`, `_` only; no leading/trailing hyphens; validated by `ValidateProfileName`, sanitized by `SanitizeProfileName` +- Key constants: `DefaultProfile = "default"`, `EnvProfile = "OP_CLI_PROFILE"` +- Key functions: `ReadConfig(profile)`, `WriteConfigForProfile(profile, host, token)`, `DeleteProfile(profile)`, `AllProfiles()` +- `OP_CLI_HOST` / `OP_CLI_TOKEN` env vars override all profiles; `OP_CLI_PROFILE` selects a profile (overridden by `--profile` flag) +- Old single-line format (`host token`) is auto-migrated to `[default]` on first read + +### Paths conventions + +- All API paths are defined in `components/paths/paths.go` +- Functions are named after the resource: `WorkPackage(id)`, `WorkPackages()`, `ProjectWorkPackages(projectId)` +- All paths are relative (no host), starting with `/api/v3` +- Project path functions (`Project`, `ProjectWorkPackages`, `ProjectVersions`, `ProjectBudgets`) take a `string` that may be either a numeric ID (`"42"`) or a human-readable identifier (`"my-project"`); the OpenProject API accepts both forms at the same endpoints +- `WorkPackage(id)` and `WorkPackageActivities(id)` take a `string` that may be either a numeric ID (`"12345"`) or a project-based semantic identifier (`"PROJ-123"`); validated by `work_packages.ValidateIdentifier` + +### Printer conventions + +- All terminal output goes through `printer/` — never `fmt.Println` directly in commands +- Color scheme: Red = ID, Green = type, Cyan = subject/name, Yellow = status +- `printer.Error(err)` for errors, `printer.ErrorText(msg)` for plain error strings +- `printer.Info(msg)` for progress messages, `printer.Done()` after successful mutations +- Work packages display `DisplayId` from the API: semantic form (e.g. `PROJ-123`) when project-based identifiers are enabled, `#N` for numeric-only systems (where `displayId` equals the numeric id) +- Work package browser URLs use the short `wp/` form (e.g. `wp/PROJ-123` or `wp/42`) + +## Testing conventions + +- Component tests use external test packages (`package printer_test`, `package requests_test`, `package work_packages_test`); cmd-layer tests use internal packages (`package workpackage`) because they exercise unexported command handlers and flag variables +- `TestMain` in `printer_test` initializes shared state (routes, printer) for the package +- Tests use plain `t.Errorf` — no test framework, no assertions library +- Tests exist for `printer`, `requests`, `common`, `configuration`, `components/resources/work_packages`, and the `cmd/` packages (root, `activity`, `project`, `user`, `workpackage`) +- Regression tests for command/resource behaviour use `httptest` localhost servers and count mutating requests to assert no-mutation guarantees +- When adding a new printer function, add a corresponding test in `components/printer/` +- When adding a new configuration function, add a corresponding test in `components/configuration/` + +## Dependencies + +| Package | Purpose | +|---------|---------| +| `github.com/spf13/cobra` | CLI framework | +| `github.com/fatih/color` | Terminal colors (via printer) | +| `github.com/briandowns/spinner` | Progress spinner | +| `github.com/go-git/go-git/v5` | Git integration (`op git` commands) | +| `github.com/sosodev/duration` | ISO 8601 duration parsing | diff --git a/README.md b/README.md index c6685be..40d3d49 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,28 @@ Open a new shell and completion should work by using the `TAB` key as usual. ## Usage The OpenProject CLI commands are structured in a common, human-readable pattern. Every command is built -as `op VERB RESOURCE [additional information]`. You will see plenty of examples within this section. +as `op NOUN VERB [additional information]`. You will see plenty of examples within this section. + +### Breaking change: noun-first commands + +Earlier releases (up to 0.5.5) used verb-first commands. These have been replaced without aliases; +scripts need to migrate: + +| Old (verb-first) | New (noun-first) | +|---------------------------|--------------------------------------| +| `op list workpackages` | `op work-package list` | +| `op create workpackage` | `op work-package create` | +| `op update workpackage` | `op work-package update` | +| `op inspect workpackage` | `op work-package inspect` | +| `op search workpackages` | `op work-package search` | +| `op list projects` | `op project list` | +| `op inspect project 42` | `op project inspect 42` | +| `op list notifications` | `op notification list` | +| `op list activities 42` | `op activity list --work-package 42` | +| `op list status` | `op status list` | +| `op list types` | `op type list` | +| `op list timeentries` | `op time-entry list` | +| `op search user X` | `op user search X` | ### Discoverability @@ -102,12 +123,11 @@ possibilities he can use from here. # General help op -h -# Help about what things I can list -op list -h +# Help about work package commands +op work-package -h -# Help about how can I narrow down my list of work -# packages to get exactly what I was looking for -op list workpackages -h +# Help about how can I narrow down my list of work packages +op work-package list -h ``` The second basement is the autocompletion. To set it up correctly, follow the steps explained in the `Completion` @@ -117,24 +137,104 @@ Once it is working, you can use it to discover possible commands and flags while ```shell # Chaining commands: hitting completion key after -op list +op work-package # returns -activities -- Lists activities for work package -notifications -- Lists notificationswork package -projects -- Lists projects -workpackages -- Lists work packages +create -- Create work package in project +inspect -- Inspect a work package +list -- Lists work packages +search -- Searches for work packages +update -- Update a work package # Discover flags: hitting completion key after -op update workpackge 42 - +op work-package update 42 - # returns --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 ---help -h -- help for workpackage +--help -h -- help for work-package --subject -- Change the subject of the work package --type -t -- Change the work package type ``` +### Authentication + +#### Single instance (default) + +Run `op login` to authenticate. You will be prompted for the host URL, an API +token, and a profile name (defaults to `default`): + +```shell +op login +# Profile name? [default] +# OpenProject host URL: https://community.openproject.org +# OpenProject API Token: ... +``` + +#### Multiple instances + +Use named profiles to work with more than one OpenProject instance: + +```shell +# Create a profile named "work" +op login --profile work + +# Create a profile named "staging" +op login --profile staging +``` + +All commands accept a `--profile` flag to select which instance to use: + +```shell +op work-package list --profile work +op project list --profile staging +``` + +The `OP_CLI_PROFILE` environment variable is an alternative to `--profile`, +useful in scripts or shell sessions where you always want the same instance: + +```shell +export OP_CLI_PROFILE=work +op work-package list # uses the "work" profile +``` + +When both are set, `--profile` takes precedence. The explicit credential +variables `OP_CLI_HOST` and `OP_CLI_TOKEN` override everything. + +#### Inspecting and removing profiles + +`op whoami` shows all configured profiles together with their server URL and +authenticated user: + +```shell +op whoami +# Profile: default +# Server: https://community.openproject.org +# User: #42 Jane Doe +# +# Profile: work +# Server: https://work.example.com +# User: #7 John Smith +``` + +Pass `--profile` to inspect a single profile: + +```shell +op whoami --profile work +``` + +`op logout` removes a profile (defaults to `default`): + +```shell +op logout # removes the "default" profile +op logout --profile work +``` + +#### Profile names + +Profile names may only contain letters, digits, `-` and `_`; hyphens must not +be consecutive, leading, or trailing. When prompted interactively the CLI +suggests a sanitized version of any invalid input. + ### Prominent examples There are a couple of use cases, you might want to execute from the command line. In this section we provide a handful @@ -146,34 +246,59 @@ 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' +# --project accepts either a numeric ID or the project identifier from the URL. +op work-package create --project 11 'Document new CLI tool' +op work-package create --project my-project '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 work-package create -p11 'Document new CLI tool' -o ``` #### Listing ```shell # Get a list of unread notifications and filter them by reason -op list notifications --reason mentioned +op notification list --reason mentioned # Get a list of all work packages assigned to me -op list workpackages --assignee me +op work-package list --assignee me + +# Get a list of direct children of a work package +# --parent-id accepts either a numeric ID or a project-based identifier (e.g. PROJ-123) +op work-package list --parent-id 42 +op work-package list --parent-id PROJ-123 ``` #### Updating ```shell # Executing a custom action on a work package -op update workpackage 42 --action Claim +# The ID argument accepts either a numeric ID or a project-based identifier (e.g. PROJ-123) +op work-package update 42 --action Claim +op work-package update PROJ-123 --action Claim # Batch updating some properties of a work package -# Valid input will get processed, while invalid (e.g. wrongly typed) input will get omitted -op update workpackage 42 --subject 'The new subject' --status 'In Progress' --type Implementation +op work-package update 42 --subject 'The new subject' --type Implementation + +# Local inputs are validated before the first change is sent. Each API action +# remains a separate request, so an earlier change may remain if a later +# request fails. # Uploading an attachment to a work package -op update workpackage 42 --attach ./Downloads/Report.pdf +op work-package update 42 --attach ./Downloads/Report.pdf +``` + +#### Searching + +```shell +# Search work packages by subject, type, status, project name, or identifier. +# Multiple words are ANDed: all terms must match. Returns up to 100 results. +op work-package search cascade +op work-package search fix login + +# Limit search to a specific project (numeric ID or identifier) +op work-package search cascade --project my-project +op work-package search cascade -p 11 ``` #### Inspecting @@ -181,9 +306,43 @@ op update workpackage 42 --attach ./Downloads/Report.pdf ```shell # Inspecting a work package with more details, # then in the work package list command -op inspect workpackage 42 +# Accepts either a numeric ID or a project-based identifier (e.g. PROJ-123) +op work-package inspect 42 +op work-package inspect PROJ-123 ``` +## AI agent integration (Claude Code) + +The repository ships a [`.claude/op.md`](.claude/op.md) reference file that teaches Claude Code how to use `op`. Once set up, the agent will use `op` proactively whenever you discuss work packages, projects, or anything OpenProject-related. + +`~/.claude/CLAUDE.md` is Claude Code's global instruction file — create it if it doesn't exist yet. + +### Option A — available across all your projects (recommended) + +Symlink the file into your Claude configuration directory from the repo root: + +```shell +ln -s $(pwd)/.claude/op.md ~/.claude/op.md +``` + +Then add a line to `~/.claude/CLAUDE.md`: + +```markdown +Use `op.md` as a reference for all `op` CLI commands (work packages, projects, time entries, search, profiles…). Read it whenever the user asks about OpenProject or wants to use `op`. +``` + +The symlink keeps the file in sync automatically as you pull new versions. + +### Option B — available in this project only + +Add a line to the project's `CLAUDE.md` at the repo root: + +```markdown +Use `.claude/op.md` as a reference for all `op` CLI commands (work packages, projects, time entries, search, profiles…). Read it whenever the user asks about OpenProject or wants to use `op`. +``` + +No copy or symlink needed — the path is relative to the project root. + ## Creating a release Releases are triggered by pushing a git tag. The tag name becomes the version string embedded in the binaries. diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..8f5eb49 --- /dev/null +++ b/TODO.md @@ -0,0 +1,11 @@ +# TODO + +Issues identified during code review — non-blocking, to address in future iterations. + +## Known limitations + +- [ ] **`op time-entry create --activity` cannot list available activities** — `GET /api/v3/time_entries/activities` returns 404 in some OpenProject instances (version or permission issue). Investigate using the form endpoint `GET /api/v3/time_entries/form` which may include allowed activities in its schema. + +## CLI UX + +- [ ] **`--format` flag has no short option** — inconsistent with the convention of adding short flags for frequently used ones. Consider `-f` if it doesn't conflict. diff --git a/cmd/activity/activity.go b/cmd/activity/activity.go new file mode 100644 index 0000000..12b147c --- /dev/null +++ b/cmd/activity/activity.go @@ -0,0 +1,22 @@ +package activity + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "activity [verb]", + Short: "Manage activities", + Long: "List activities of a work package (--work-package).", +} + +func init() { + listCmd.Flags().StringVarP( + &listWpId, + "work-package", + "w", + "", + "Work package ID or identifier to list activities for", + ) + _ = listCmd.MarkFlagRequired("work-package") + + RootCmd.AddCommand(listCmd) +} diff --git a/cmd/activity/activity_test.go b/cmd/activity/activity_test.go new file mode 100644 index 0000000..af98c2f --- /dev/null +++ b/cmd/activity/activity_test.go @@ -0,0 +1,15 @@ +package activity_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/cmd/activity" +) + +func TestRootCmd_UsesSingularNoun(t *testing.T) { + use := activity.RootCmd.Use + if !strings.HasPrefix(use, "activity") { + t.Errorf("expected command name 'activity', got %q", use) + } +} diff --git a/cmd/activity/list.go b/cmd/activity/list.go new file mode 100644 index 0000000..2130fa5 --- /dev/null +++ b/cmd/activity/list.go @@ -0,0 +1,56 @@ +package activity + +import ( + stderrors "errors" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/users" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +var listWpId string + +var listCmd = &cobra.Command{ + Use: "list", + Short: "Lists activities", + Long: "Get a list of activities, scoped by the provided flag (e.g. --work-package).", + RunE: listActivities, +} + +func listActivities(_ *cobra.Command, _ []string) error { + if err := work_packages.ValidateIdentifier(listWpId); err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + return listWorkPackageActivities(listWpId) +} + +func listWorkPackageActivities(wpId string) error { + acts, err := work_packages.Activities(wpId) + if err != nil { + if !stderrors.Is(err, openerrors.ErrHandled) { + printer.Error(err) + } + return openerrors.ErrHandled + } + + var userIds []uint64 + for _, a := range acts { + if a.UserId > 0 { + userIds = append(userIds, a.UserId) + } + } + + userList, err := users.ByIds(userIds) + if err != nil { + if !stderrors.Is(err, openerrors.ErrHandled) { + printer.Error(err) + } + return openerrors.ErrHandled + } + printer.Activities(acts, userList) + return nil +} diff --git a/cmd/activity/list_test.go b/cmd/activity/list_test.go new file mode 100644 index 0000000..6cd4ebd --- /dev/null +++ b/cmd/activity/list_test.go @@ -0,0 +1,88 @@ +package activity + +import ( + "errors" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" +) + +func TestListWorkPackageActivitiesReportsFetchErrorsOnce(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, _ *http.Request) { + http.Error(response, "activity failure", http.StatusInternalServerError) + })) + t.Cleanup(server.Close) + + testingPrinter := initActivityTest(t, server.URL) + err := listWorkPackageActivities("42") + + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("listWorkPackageActivities error = %v, want ErrHandled", err) + } + if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 { + t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult) + } + if testingPrinter.Result != "" { + t.Errorf("stdout = %q, want empty", testingPrinter.Result) + } +} + +func TestListWorkPackageActivitiesReportsUserFetchErrorsOnce(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + if request.URL.Path == "/api/v3/work_packages/42/activities" { + _, _ = response.Write([]byte(`{ + "_embedded": { + "elements": [{ + "id": 1, + "comment": {"raw": ""}, + "details": [], + "_links": {"user": {"href": "/api/v3/users/7"}} + }] + } + }`)) + return + } + http.Error(response, "user failure", http.StatusInternalServerError) + })) + t.Cleanup(server.Close) + + testingPrinter := initActivityTest(t, server.URL) + err := listWorkPackageActivities("42") + + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("listWorkPackageActivities error = %v, want ErrHandled", err) + } + if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 { + t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult) + } + if testingPrinter.Result != "" { + t.Errorf("stdout = %q, want empty", testingPrinter.Result) + } +} + +func initActivityTest(t *testing.T, serverURL string) *printer.TestingPrinter { + t.Helper() + + host, err := url.Parse(serverURL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + if err := printer.InitRenderer("json"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = printer.InitRenderer("text") + }) + return testingPrinter +} diff --git a/cmd/budget/budget.go b/cmd/budget/budget.go new file mode 100644 index 0000000..d3f4fb2 --- /dev/null +++ b/cmd/budget/budget.go @@ -0,0 +1,23 @@ +package budget + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "budget [verb]", + Short: "Manage budgets", + Long: "List and inspect budgets in OpenProject.", +} + +func init() { + listCmd.Flags().StringVarP( + &listProjectId, + "project", + "p", + "", + "Project numeric ID or identifier", + ) + + _ = listCmd.MarkFlagRequired("project") + + RootCmd.AddCommand(listCmd, inspectCmd) +} diff --git a/cmd/budget/inspect.go b/cmd/budget/inspect.go new file mode 100644 index 0000000..a3ec0a7 --- /dev/null +++ b/cmd/budget/inspect.go @@ -0,0 +1,41 @@ +package budget + +import ( + "fmt" + "strconv" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/budgets" +) + +var inspectCmd = &cobra.Command{ + Use: "inspect [id]", + Short: "Show details about a budget", + Long: "Show detailed information of a budget referenced by its ID.", + RunE: inspectBudget, +} + +func inspectBudget(_ *cobra.Command, args []string) error { + if len(args) != 1 { + printer.ErrorText(fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) + return openerrors.ErrHandled + } + + id, err := strconv.ParseUint(args[0], 10, 64) + if err != nil { + printer.ErrorText(fmt.Sprintf("'%s' is an invalid budget id. Must be a number.", args[0])) + return openerrors.ErrHandled + } + + budget, err := budgets.Lookup(id) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + printer.Budget(budget) + return nil +} diff --git a/cmd/budget/list.go b/cmd/budget/list.go new file mode 100644 index 0000000..ec88b9f --- /dev/null +++ b/cmd/budget/list.go @@ -0,0 +1,37 @@ +package budget + +import ( + "fmt" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/budgets" + "github.com/opf/openproject-cli/components/resources/projects" +) + +var listProjectId string + +var listCmd = &cobra.Command{ + Use: "list", + Short: "Lists budgets of a project", + Long: "Get a list of all budgets for a given project.", + RunE: listBudgets, +} + +func listBudgets(_ *cobra.Command, _ []string) error { + if err := projects.ValidateIdentifier(listProjectId); err != nil { + printer.ErrorText(fmt.Sprintf("--project: %s", err.Error())) + return openerrors.ErrHandled + } + + all, err := budgets.AllForProject(listProjectId) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + printer.Budgets(all) + return nil +} diff --git a/cmd/create/create.go b/cmd/create/create.go deleted file mode 100644 index e03767c..0000000 --- a/cmd/create/create.go +++ /dev/null @@ -1,38 +0,0 @@ -package create - -import "github.com/spf13/cobra" - -var RootCmd = &cobra.Command{ - Use: "create [resource]", - Short: "Creates a specific resource", - Long: "Create a specific resource in OpenProject", -} - -func init() { - createWorkPackageCmd.Flags().Uint64VarP( - &projectId, - "project", - "p", - 0, - "Project ID to create the work package in", - ) - _ = createWorkPackageCmd.MarkFlagRequired("project") - - createWorkPackageCmd.Flags().BoolVarP( - &shouldOpenWorkPackageInBrowser, - "open", - "o", - false, - "Open the created work package in the default browser", - ) - - createWorkPackageCmd.Flags().StringVarP( - &typeFlag, - "type", - "t", - "", - "Change the work package type", - ) - - RootCmd.AddCommand(createWorkPackageCmd) -} diff --git a/cmd/create/work_package.go b/cmd/create/work_package.go deleted file mode 100644 index 5e1bee4..0000000 --- a/cmd/create/work_package.go +++ /dev/null @@ -1,58 +0,0 @@ -package create - -import ( - "fmt" - - "github.com/spf13/cobra" - - "github.com/opf/openproject-cli/components/launch" - "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 shouldOpenWorkPackageInBrowser bool -var typeFlag string - -var createWorkPackageCmd = &cobra.Command{ - Use: "workpackage [subject]", - Short: "Create work package in project", - Long: "Create a new work package with the given subject in a project", - Run: createWorkPackage, -} - -func createWorkPackage(_ *cobra.Command, args []string) { - if len(args) != 1 { - printer.ErrorText(fmt.Sprintf("Expected 1 argument [subject], but got %d", len(args))) - return - } - - subject := args[0] - workPackage, err := work_packages.Create(projectId, createOptions(subject)) - if err != nil { - printer.Error(err) - return - } - - if shouldOpenWorkPackageInBrowser { - err = launch.Browser(routes.WorkPackageUrl(workPackage)) - if err != nil { - printer.ErrorText(fmt.Sprintf("Error opening browser: %+v", err)) - } - } else { - printer.WorkPackage(workPackage) - } -} - -func createOptions(subject string) map[work_packages.CreateOption]string { - var options = make(map[work_packages.CreateOption]string) - - options[work_packages.CreateSubject] = subject - - if len(typeFlag) > 0 { - options[work_packages.CreateType] = typeFlag - } - - return options -} diff --git a/cmd/git/start/work_package.go b/cmd/git/start/work_package.go index 50baa1e..7281cad 100644 --- a/cmd/git/start/work_package.go +++ b/cmd/git/start/work_package.go @@ -2,15 +2,14 @@ package start import ( "fmt" - "os" "regexp" - "strconv" "strings" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/spf13/cobra" + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/work_packages" ) @@ -18,29 +17,36 @@ import ( var startWorkPackageCmd = &cobra.Command{ Use: "workpackage [id]", Short: "Starts the work on a work package", - Long: "Creates a branch based on the workpackage id, subject and type. Switches to the branch after creation.", - Run: startWorkPackage, + Long: "Creates a branch based on the work package identifier (numeric ID or project-based, e.g. PROJ-123), subject and type. Switches to the branch after creation.", + RunE: startWorkPackage, } -func startWorkPackage(_ *cobra.Command, args []string) { - id := checkArgumentsForId(args) +func startWorkPackage(_ *cobra.Command, args []string) error { + id, err := checkArgumentsForId(args) + if err != nil { + return err + } repo, err := git.PlainOpen(".") if err != nil { printer.ErrorText("No git repository found in the current location.") - return + return openerrors.ErrHandled } head, err := repo.Head() - handleGitError(err) + if err := handleGitError(err); err != nil { + return err + } worktree, err := repo.Worktree() - handleGitError(err) + if err := handleGitError(err); err != nil { + return err + } branchName, err := deriveBranchName(id) if err != nil { printer.Error(err) - return + return openerrors.ErrHandled } err = worktree.Checkout(&git.CheckoutOptions{ @@ -49,34 +55,37 @@ func startWorkPackage(_ *cobra.Command, args []string) { Create: true, Keep: true, }) - handleGitError(err) + if err := handleGitError(err); err != nil { + return err + } printer.Info(fmt.Sprintf("Switched to a new branch: %s", branchName)) + return nil } -func checkArgumentsForId(args []string) uint64 { +func checkArgumentsForId(args []string) (string, error) { if len(args) != 1 { printer.ErrorText(fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) - os.Exit(1) + return "", openerrors.ErrHandled } - 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])) - os.Exit(1) + if err := work_packages.ValidateIdentifier(args[0]); err != nil { + printer.ErrorText(err.Error()) + return "", openerrors.ErrHandled } - return id + return args[0], nil } -func handleGitError(err error) { +func handleGitError(err error) error { if err != nil { printer.ErrorText(fmt.Sprintf("Fatal error on executing git commands: %s", err.Error())) - os.Exit(1) + return openerrors.ErrHandled } + return nil } -func deriveBranchName(id uint64) (string, error) { +func deriveBranchName(id string) (string, error) { workPackage, err := work_packages.Lookup(id) if err != nil { return "", err diff --git a/cmd/inspect/inspect.go b/cmd/inspect/inspect.go deleted file mode 100644 index b956ab4..0000000 --- a/cmd/inspect/inspect.go +++ /dev/null @@ -1,36 +0,0 @@ -package inspect - -import "github.com/spf13/cobra" - -var RootCmd = &cobra.Command{ - Use: "inspect [type] [id]", - Short: "Show details about an object", - Long: "Show detailed information of an object of a specific type referenced by it's ID.", -} - -func init() { - inspectProjectCmd.Flags().BoolVarP( - &shouldOpenProjectInBrowser, - "open", - "o", - false, - "Open the project in the default browser", - ) - - inspectWorkPackageCmd.Flags().BoolVarP( - &shouldOpenWorkPackageInBrowser, - "open", - "o", - false, - "Open the work package in the default browser", - ) - - inspectWorkPackageCmd.Flags().BoolVar( - &listAvailableTypes, - "types", - false, - "List the available types on the work package.", - ) - - RootCmd.AddCommand(inspectProjectCmd, inspectWorkPackageCmd) -} diff --git a/cmd/inspect/project.go b/cmd/inspect/project.go deleted file mode 100644 index f351127..0000000 --- a/cmd/inspect/project.go +++ /dev/null @@ -1,50 +0,0 @@ -package inspect - -import ( - "fmt" - "strconv" - - "github.com/spf13/cobra" - - "github.com/opf/openproject-cli/components/launch" - "github.com/opf/openproject-cli/components/printer" - "github.com/opf/openproject-cli/components/resources/projects" - "github.com/opf/openproject-cli/components/routes" -) - -var shouldOpenProjectInBrowser bool - -var inspectProjectCmd = &cobra.Command{ - Use: "project [id]", - Short: "Show details about a project", - Long: "Show detailed information of a project refereced by it's ID.", - Run: inspectProject, -} - -func inspectProject(_ *cobra.Command, args []string) { - if len(args) != 1 { - printer.ErrorText(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 project id. Must be a number.", args[0])) - return - } - - project, err := projects.Lookup(id) - if err != nil { - printer.Error(err) - return - } - - if shouldOpenProjectInBrowser { - err = launch.Browser(routes.ProjectUrl(project)) - if err != nil { - printer.ErrorText(fmt.Sprintf("Error opening browser: %+v", err)) - } - } else { - printer.Project(project) - } -} diff --git a/cmd/inspect/work_package.go b/cmd/inspect/work_package.go deleted file mode 100644 index a3e7349..0000000 --- a/cmd/inspect/work_package.go +++ /dev/null @@ -1,74 +0,0 @@ -package inspect - -import ( - "fmt" - "strconv" - - "github.com/spf13/cobra" - - "github.com/opf/openproject-cli/components/launch" - "github.com/opf/openproject-cli/components/printer" - "github.com/opf/openproject-cli/components/resources/work_packages" - "github.com/opf/openproject-cli/components/routes" -) - -var shouldOpenWorkPackageInBrowser bool -var listAvailableTypes bool - -var inspectWorkPackageCmd = &cobra.Command{ - Use: "workpackage [id]", - Aliases: []string{"wp"}, - Short: "Show details about a work package", - Long: "Show detailed information of a work package referenced by it's ID.", - Run: inspectWorkPackage, -} - -func inspectWorkPackage(_ *cobra.Command, args []string) { - if len(args) != 1 { - printer.ErrorText(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])) - return - } - - if hasListingFlag() { - switch { - case listAvailableTypes: - listTypes(id) - } - return - } - - workPackage, err := work_packages.Lookup(id) - if err != nil { - printer.Error(err) - return - } - - if shouldOpenWorkPackageInBrowser { - err = launch.Browser(routes.WorkPackageUrl(workPackage)) - if err != nil { - printer.ErrorText(fmt.Sprintf("Error opening browser: %+v", err)) - } - } else { - printer.WorkPackage(workPackage) - } -} - -func listTypes(id uint64) { - types, err := work_packages.AvailableTypes(id) - if err != nil { - printer.Error(err) - return - } - - printer.Types(types) -} - -func hasListingFlag() bool { - return listAvailableTypes -} diff --git a/cmd/list/activities.go b/cmd/list/activities.go deleted file mode 100644 index 7137371..0000000 --- a/cmd/list/activities.go +++ /dev/null @@ -1,47 +0,0 @@ -package list - -import ( - "fmt" - "strconv" - - "github.com/spf13/cobra" - - "github.com/opf/openproject-cli/components/printer" - "github.com/opf/openproject-cli/components/resources/users" - "github.com/opf/openproject-cli/components/resources/work_packages" -) - -var activitiesCmd = &cobra.Command{ - Use: "activities [workPackageId]", - Aliases: []string{"ac"}, - Short: "Lists activities for work package", - Long: `Get a list of activities for a work package.`, - Run: listActivities, -} - -func listActivities(_ *cobra.Command, args []string) { - if len(args) != 1 { - printer.ErrorText(fmt.Sprintf("Expected 1 argument [workPackageId], but got %d", len(args))) - return - } - - wpId, err := strconv.ParseUint(args[0], 10, 64) - if err != nil { - printer.ErrorText(err.Error()) - } - activities, err := work_packages.Activities(wpId) - if err != nil { - printer.ErrorText(err.Error()) - } - - var userIds []uint64 - for _, a := range activities { - if a.UserId > 0 { - userIds = append(userIds, a.UserId) - continue - } - } - - userList := users.ByIds(userIds) - printer.Activities(activities, userList) -} diff --git a/cmd/list/list.go b/cmd/list/list.go deleted file mode 100644 index b230188..0000000 --- a/cmd/list/list.go +++ /dev/null @@ -1,33 +0,0 @@ -package list - -import "github.com/spf13/cobra" - -var RootCmd = &cobra.Command{ - Use: "list [resource]", - Short: "Lists the specific resource", - Long: `Get a list of the ordered resource. -The list can get filtered further.`, -} - -func init() { - initWorkPackagesFlags() - initTimeEntriesFlags() - - notificationsCmd.Flags().StringVarP( - ¬ificationReason, - "reason", - "r", - "", - "The reason for the notification", - ) - - RootCmd.AddCommand( - projectsCmd, - notificationsCmd, - workPackagesCmd, - activitiesCmd, - statusCmd, - timeEntriesCmd, - typesCmd, - ) -} diff --git a/cmd/list/status.go b/cmd/list/status.go deleted file mode 100644 index cf9df8f..0000000 --- a/cmd/list/status.go +++ /dev/null @@ -1,22 +0,0 @@ -package list - -import ( - "github.com/opf/openproject-cli/components/printer" - "github.com/opf/openproject-cli/components/resources/status" - "github.com/spf13/cobra" -) - -var statusCmd = &cobra.Command{ - Use: "status", - Short: "Lists status", - Long: "Get a list of all status of the instance.", - Run: listStatus, -} - -func listStatus(_ *cobra.Command, _ []string) { - if all, err := status.All(); err == nil { - printer.StatusList(all) - } else { - printer.Error(err) - } -} diff --git a/cmd/list/work_packages.go b/cmd/list/work_packages.go deleted file mode 100644 index 509a4e5..0000000 --- a/cmd/list/work_packages.go +++ /dev/null @@ -1,169 +0,0 @@ -package list - -import ( - "fmt" - "os" - "regexp" - "strconv" - - "github.com/opf/openproject-cli/components/common" - "github.com/opf/openproject-cli/components/printer" - "github.com/opf/openproject-cli/components/requests" - "github.com/opf/openproject-cli/components/resources" - "github.com/opf/openproject-cli/components/resources/projects" - "github.com/opf/openproject-cli/components/resources/work_packages" - "github.com/opf/openproject-cli/components/resources/work_packages/filters" - "github.com/opf/openproject-cli/models" - "github.com/spf13/cobra" -) - -var assignee string -var projectId uint64 -var showTotal bool -var statusFilter string -var typeFilter string -var includeSubProjects bool - -var activeFilters = map[string]resources.Filter{ - "notSubProject": filters.NewNotSubProjectFilter(), - "notVersion": filters.NewNotVersionFilter(), - "subProject": filters.NewSubProjectFilter(), - "timestamp": filters.NewTimestampFilter(), - "version": filters.NewVersionFilter(), -} - -var workPackagesCmd = &cobra.Command{ - Use: "workpackages", - Aliases: []string{"wps"}, - Short: "Lists work packages", - Long: "Get a list of visible work packages. Filter flags can be applied.", - Run: listWorkPackages, -} - -func listWorkPackages(_ *cobra.Command, _ []string) { - // This needs to be removed, once all filters are built the "new" way - if errorText := validateCommandFlagComposition(); len(errorText) > 0 { - printer.ErrorText(errorText) - return - } - - query, err := buildQuery() - if err != nil { - printer.ErrorText(err.Error()) - return - } - - collection, err := work_packages.All(filterOptions(), query, showTotal) - switch { - case err == nil && showTotal: - printer.Number(collection.Total) - case err == nil: - printer.WorkPackages(collection.Items) - default: - printer.Error(err) - } -} - -func validateCommandFlagComposition() (errorText string) { - switch { - case len(activeFilters["version"].Value()) != 0 && projectId == 0: - return "Version flag (--version) can only be used in conjunction with projectId flag (-p or --project-id)." - case len(activeFilters["notVersion"].Value()) != 0 && projectId == 0: - return "Not version filter flag (--not-version) can only be used in conjunction with projectId flag (-p or --project-id)." - case len(activeFilters["subProject"].Value()) > 0 || len(activeFilters["notSubProject"].Value()) > 0: - if !includeSubProjects || projectId == 0 { - return `Sub project filter flags (--sub-project or --not-sub-project) can only be used -in conjunction with setting the flag --include-sub-projects and setting a -project with the projectId flag (-p or --project-id).` - } - } - - return "" -} - -func buildQuery() (requests.Query, error) { - var q requests.Query - - for _, filter := range activeFilters { - if filter.Value() == filter.DefaultValue() { - continue - } - - err := filter.ValidateInput() - if err != nil { - return requests.NewEmptyQuery(), err - } - - q = q.Merge(filter.Query()) - } - - return q, nil -} - -func filterOptions() *map[work_packages.FilterOption]string { - options := make(map[work_packages.FilterOption]string) - - options[work_packages.IncludeSubProjects] = strconv.FormatBool(includeSubProjects) - - if projectId > 0 { - options[work_packages.Project] = strconv.FormatUint(projectId, 10) - } - - if len(assignee) > 0 { - options[work_packages.Assignee] = assignee - } - - if len(statusFilter) > 0 { - options[work_packages.Status] = validateFilterValue(work_packages.Status, statusFilter) - } - - if len(typeFilter) > 0 { - options[work_packages.Type] = validateFilterValue(work_packages.Type, typeFilter) - } - - return &options -} - -func validatedVersionId(version string) string { - project, err := projects.Lookup(projectId) - if err != nil { - printer.Error(err) - } - - versions, err := projects.AvailableVersions(project.Id) - if err != nil { - printer.Error(err) - } - - filteredVersions := common.Filter(versions, func(v *models.Version) bool { - return v.Name == version - }) - - if len(filteredVersions) != 1 { - printer.Info(fmt.Sprintf( - "No unique available version from input %s found for projectId %s. Please use one of the versions listed below.", - printer.Cyan(version), - printer.Red(fmt.Sprintf("#%d", project.Id)), - )) - - printer.Versions(versions) - - os.Exit(-1) - } - - return strconv.FormatUint(filteredVersions[0].Id, 10) -} - -func validateFilterValue(filter work_packages.FilterOption, value string) string { - matched, err := regexp.Match(work_packages.InputValidationExpression[filter], []byte(value)) - if err != nil { - printer.Error(err) - } - - if !matched { - printer.ErrorText(fmt.Sprintf("Invalid %s value %s.", filter, printer.Yellow(value))) - os.Exit(-1) - } - - return value -} diff --git a/cmd/login.go b/cmd/login.go index 7e74747..7c5da86 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -12,6 +12,7 @@ import ( "github.com/opf/openproject-cli/components/common" "github.com/opf/openproject-cli/components/configuration" + openerrors "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" @@ -27,7 +28,7 @@ var loginCmd = &cobra.Command{ this tool for a specific OpenProject instance. The login needs the host URL of the OpenProject instance and a generated API token.`, - Run: login, + RunE: login, } const ( @@ -37,7 +38,13 @@ const ( tokenInputError = "There was a problem parsing the token input. Please try again." ) -func login(_ *cobra.Command, _ []string) { +func login(cmd *cobra.Command, _ []string) error { + profile, err := resolveLoginProfile(cmd) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + var hostUrl *url.URL var token string @@ -64,10 +71,10 @@ func login(_ *cobra.Command, _ []string) { } for { - fmt.Printf("OpenProject API Token (Visit %s/my/access_tokens to generate one): ", hostUrl) + printer.Input(fmt.Sprintf("OpenProject API Token (Visit %s/my/access_tokens to generate one): ", hostUrl)) ok, t := requestApiToken() if !ok { - fmt.Println(tokenInputError) + printer.ErrorText(tokenInputError) continue } @@ -88,7 +95,64 @@ func login(_ *cobra.Command, _ []string) { break } - storeLoginData(hostUrl, token) + return storeLoginData(profile, hostUrl, token) +} + +// resolveLoginProfile determines the profile name for the login command. +// +// - If --profile was explicitly passed: validate the value immediately, +// display it, and use it without prompting. +// - If OP_CLI_PROFILE is set (but --profile was not): display the value +// and use it without prompting. +// - Otherwise: prompt the user interactively. +func resolveLoginProfile(cmd *cobra.Command) (string, error) { + if cmd.Root().PersistentFlags().Changed("profile") { + if err := configuration.ValidateProfileName(profileName); err != nil { + return "", err + } + printer.Info(fmt.Sprintf("Profile: %s", profileName)) + return profileName, nil + } + + if env := os.Getenv(configuration.EnvProfile); env != "" { + if err := configuration.ValidateProfileName(env); err != nil { + return "", err + } + printer.Info(fmt.Sprintf("Profile: %s", env)) + return env, nil + } + + return promptProfileName(configuration.DefaultProfile) +} + +// promptProfileName shows an interactive prompt, re-prompting on invalid input +// and offering the sanitized form as the next default. +func promptProfileName(defaultName string) (string, error) { + reader := bufio.NewReader(os.Stdin) + + for { + printer.Input(fmt.Sprintf("Profile name? [%s] ", defaultName)) + input, err := reader.ReadString('\n') + if err != nil { + return "", err + } + + input = common.SanitizeLineBreaks(strings.TrimSpace(input)) + if input == "" { + return defaultName, nil + } + + if err := configuration.ValidateProfileName(input); err != nil { + sanitized := configuration.SanitizeProfileName(input) + printer.ErrorText( + "Invalid profile name. Only letters, numbers, - and _ are allowed (no leading/trailing hyphens).", + ) + defaultName = sanitized + continue + } + + return input, nil + } } func parseHostUrl() (ok bool, errMessage string, host *url.URL) { @@ -165,9 +229,43 @@ func requestApiToken() (ok bool, token string) { return true, input } -func storeLoginData(host *url.URL, token string) { - err := configuration.WriteConfigFile(host.String(), token) +func confirmOverwrite(profile string) (bool, error) { + reader := bufio.NewReader(os.Stdin) + printer.Input(fmt.Sprintf("Profile %q already exists, overwrite? [y/N] ", profile)) + input, err := reader.ReadString('\n') if err != nil { + return false, err + } + answer := strings.ToLower(strings.TrimSpace(common.SanitizeLineBreaks(input))) + return answer == "y" || answer == "yes", nil +} + +func storeLoginData(profile string, host *url.URL, token string) error { + profiles, err := configuration.AllProfiles() + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + for _, p := range profiles { + if p.Name == profile { + ok, err := confirmOverwrite(profile) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + if !ok { + printer.Info("Login cancelled.") + return nil + } + break + } + } + + if err := configuration.WriteConfigForProfile(profile, host.String(), token); err != nil { printer.Error(err) + return openerrors.ErrHandled } + printer.Done() + return nil } diff --git a/cmd/logout.go b/cmd/logout.go new file mode 100644 index 0000000..3cdbd0b --- /dev/null +++ b/cmd/logout.go @@ -0,0 +1,59 @@ +package cmd + +import ( + "bufio" + "fmt" + "os" + "strings" + + "github.com/spf13/cobra" + + "github.com/opf/openproject-cli/components/common" + "github.com/opf/openproject-cli/components/configuration" + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" +) + +var logoutCmd = &cobra.Command{ + Use: "logout", + Short: "Removes a stored profile", + Long: "Removes the credentials for the given profile (defaults to 'default').", + RunE: logout, +} + +func logout(cmd *cobra.Command, _ []string) error { + profile, _ := resolvedProfile(cmd) + + if err := configuration.ValidateProfileName(profile); err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + ok, err := confirmRemove(profile) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + if !ok { + return nil + } + + if err := configuration.DeleteProfile(profile); err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + printer.Done() + return nil +} + +func confirmRemove(profile string) (bool, error) { + reader := bufio.NewReader(os.Stdin) + printer.Input(fmt.Sprintf("Remove profile %q? [y/N] ", profile)) + input, err := reader.ReadString('\n') + if err != nil { + return false, err + } + answer := strings.ToLower(strings.TrimSpace(common.SanitizeLineBreaks(input))) + return answer == "y" || answer == "yes", nil +} diff --git a/cmd/list/notifications.go b/cmd/notification/list.go similarity index 50% rename from cmd/list/notifications.go rename to cmd/notification/list.go index 7665fce..184dddd 100644 --- a/cmd/list/notifications.go +++ b/cmd/notification/list.go @@ -1,4 +1,4 @@ -package list +package notification import ( "fmt" @@ -6,30 +6,34 @@ import ( "github.com/spf13/cobra" "github.com/opf/openproject-cli/components/common" + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/notifications" ) -var notificationReason string +var listReason string var validReasons = []string{"", "assigned", "mentioned", "responsible", "watched", "dateAlert"} -var notificationsCmd = &cobra.Command{ - Use: "notifications", + +var listCmd = &cobra.Command{ + Use: "list", Short: "Lists notifications", Long: `Get a list of unread notifications. The list can get filtered further.`, - Run: listNotifications, + RunE: listNotifications, } -func listNotifications(_ *cobra.Command, _ []string) { - if !common.Contains(validReasons, notificationReason) { - printer.ErrorText(fmt.Sprintf("Reason '%s' is invalid.", notificationReason)) - return +func listNotifications(_ *cobra.Command, _ []string) error { + if !common.Contains(validReasons, listReason) { + printer.ErrorText(fmt.Sprintf("Reason '%s' is invalid.", listReason)) + return openerrors.ErrHandled } - if all, err := notifications.All(notificationReason); err == nil { + if all, err := notifications.All(listReason); err == nil { printer.Notifications(all) } else { printer.Error(err) + return openerrors.ErrHandled } + return nil } diff --git a/cmd/notification/notification.go b/cmd/notification/notification.go new file mode 100644 index 0000000..4fad4b2 --- /dev/null +++ b/cmd/notification/notification.go @@ -0,0 +1,21 @@ +package notification + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "notification [verb]", + Short: "Manage notifications", + Long: "List notifications in OpenProject.", +} + +func init() { + listCmd.Flags().StringVarP( + &listReason, + "reason", + "r", + "", + "The reason for the notification", + ) + + RootCmd.AddCommand(listCmd) +} diff --git a/cmd/project/inspect.go b/cmd/project/inspect.go new file mode 100644 index 0000000..c340ec3 --- /dev/null +++ b/cmd/project/inspect.go @@ -0,0 +1,51 @@ +package project + +import ( + "fmt" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/launch" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/projects" + "github.com/opf/openproject-cli/components/routes" +) + +var openInBrowser bool + +var inspectCmd = &cobra.Command{ + Use: "inspect [id|identifier]", + Short: "Show details about a project", + Long: "Show detailed information of a project referenced by its numeric ID or identifier.", + RunE: inspectProject, +} + +func inspectProject(_ *cobra.Command, args []string) error { + if len(args) != 1 { + printer.ErrorText(fmt.Sprintf("Expected 1 argument [id|identifier], but got %d", len(args))) + return openerrors.ErrHandled + } + + if err := projects.ValidateIdentifier(args[0]); err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + + project, err := projects.Lookup(args[0]) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + if openInBrowser { + err = launch.Browser(routes.ProjectUrl(project)) + if err != nil { + printer.ErrorText(fmt.Sprintf("Error opening browser: %+v", err)) + return openerrors.ErrHandled + } + } else { + printer.Project(project) + } + return nil +} diff --git a/cmd/project/inspect_test.go b/cmd/project/inspect_test.go new file mode 100644 index 0000000..7a7b977 --- /dev/null +++ b/cmd/project/inspect_test.go @@ -0,0 +1,49 @@ +package project + +import ( + "errors" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/routes" +) + +func TestInspectBrowserFailureReturnsError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + if request.Method != http.MethodGet { + t.Errorf("request method = %s, want GET", request.Method) + } + response.Header().Set("Content-Type", "application/json") + _, _ = response.Write([]byte(`{"id":1,"identifier":"example","name":"Example"}`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + routes.Init(host) + + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + t.Setenv("PATH", t.TempDir()) + openInBrowser = true + t.Cleanup(func() { + openInBrowser = false + }) + + err = inspectProject(nil, []string{"1"}) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("inspectProject error = %v, want ErrHandled", err) + } + if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 { + t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult) + } +} diff --git a/cmd/list/projects.go b/cmd/project/list.go similarity index 60% rename from cmd/list/projects.go rename to cmd/project/list.go index d3c117f..f21e723 100644 --- a/cmd/list/projects.go +++ b/cmd/project/list.go @@ -1,24 +1,27 @@ -package list +package project import ( "github.com/spf13/cobra" + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/projects" ) -var projectsCmd = &cobra.Command{ - Use: "projects", +var listCmd = &cobra.Command{ + Use: "list", Short: "Lists projects", Long: `Get a list of visible projects. The list can get filtered further.`, - Run: listProjects, + RunE: listProjects, } -func listProjects(_ *cobra.Command, _ []string) { +func listProjects(_ *cobra.Command, _ []string) error { if all, err := projects.All(); err == nil { printer.Projects(all) } else { printer.Error(err) + return openerrors.ErrHandled } + return nil } diff --git a/cmd/project/project.go b/cmd/project/project.go new file mode 100644 index 0000000..34b958d --- /dev/null +++ b/cmd/project/project.go @@ -0,0 +1,21 @@ +package project + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "project [verb]", + Short: "Manage projects", + Long: "List and inspect projects in OpenProject.", +} + +func init() { + inspectCmd.Flags().BoolVarP( + &openInBrowser, + "open", + "o", + false, + "Open the project in the default browser", + ) + + RootCmd.AddCommand(listCmd, inspectCmd) +} diff --git a/cmd/root.go b/cmd/root.go index ce21db8..e567023 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -9,13 +9,18 @@ import ( "github.com/spf13/cobra" - "github.com/opf/openproject-cli/cmd/create" + "github.com/opf/openproject-cli/cmd/activity" + "github.com/opf/openproject-cli/cmd/budget" "github.com/opf/openproject-cli/cmd/git" - "github.com/opf/openproject-cli/cmd/inspect" - "github.com/opf/openproject-cli/cmd/list" - "github.com/opf/openproject-cli/cmd/search" - "github.com/opf/openproject-cli/cmd/update" + "github.com/opf/openproject-cli/cmd/notification" + "github.com/opf/openproject-cli/cmd/project" + "github.com/opf/openproject-cli/cmd/status" + "github.com/opf/openproject-cli/cmd/timeentry" + "github.com/opf/openproject-cli/cmd/user" + "github.com/opf/openproject-cli/cmd/workpackage" + "github.com/opf/openproject-cli/cmd/wptype" "github.com/opf/openproject-cli/components/configuration" + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/components/routes" @@ -23,6 +28,8 @@ import ( var Verbose bool var showVersionFlag bool +var outputFormat string +var profileName string var rootCmd = &cobra.Command{ Use: os.Args[0], @@ -30,6 +37,61 @@ var rootCmd = &cobra.Command{ Long: `OpenProject CLI is a fast, reliable and easy-to-use tool to manage your work packages, notifications and projects of your OpenProject instance.`, + SilenceErrors: true, + SilenceUsage: true, + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + if err := printer.InitRenderer(outputFormat); err != nil { + return openerrors.ErrHandled + } + + // login and logout manage their own profile and requests setup + if cmd.Name() == "login" || cmd.Name() == "logout" { + return nil + } + + // Offline commands must work without a valid configuration. + if isOfflineCommand(cmd) { + return nil + } + + profile, explicit := resolvedProfile(cmd) + + if err := configuration.ValidateProfileName(profile); err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + host, token, err := configuration.ReadConfig(profile) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + if insecure, mode := configuration.InsecureConfigPermissions(); insecure { + path := configuration.ConfigFilePath() + printer.Warning(fmt.Sprintf( + "config file %s is accessible by other users (mode %#o); it stores your API token. Run 'chmod 600 \"%s\"' to restrict access.", + path, mode, path, + )) + } + + if host == "" && explicit { + printer.Error(openerrors.Custom(fmt.Sprintf( + "Profile %q not found. Run 'op login --profile %s' to create it.", + profile, profile, + ))) + return openerrors.ErrHandled + } + + parse, err := url.Parse(host) + if err != nil { + printer.ErrorText(fmt.Sprintf("invalid host URL %q: %s", host, err)) + return openerrors.ErrHandled + } + requests.Init(parse, token, Verbose) + routes.Init(parse) + return nil + }, Run: func(cmd *cobra.Command, args []string) { if showVersionFlag { versionText := fmt.Sprintf( @@ -41,7 +103,7 @@ projects of your OpenProject instance.`, runtime.Version(), ) - fmt.Println(printer.Yellow(versionText)) + printer.Output(printer.Yellow(versionText)) return } @@ -50,6 +112,33 @@ projects of your OpenProject instance.`, }, } +// isOfflineCommand reports whether cmd needs no API access: shell completion +// generation (including cobra's hidden __complete machinery) and `op +// --version`. These must not fail on a missing or corrupt configuration. +func isOfflineCommand(cmd *cobra.Command) bool { + switch cmd.Name() { + case "completion", cobra.ShellCompRequestCmd, cobra.ShellCompNoDescRequestCmd: + return true + } + if parent := cmd.Parent(); parent != nil && parent.Name() == "completion" { + return true + } + return cmd == cmd.Root() && showVersionFlag +} + +// resolvedProfile returns the effective profile name for the current +// invocation, and whether it was explicitly specified by the user (flag or +// OP_CLI_PROFILE env var) vs falling through to the "default" fallback. +func resolvedProfile(cmd *cobra.Command) (profile string, explicit bool) { + if cmd.Root().PersistentFlags().Changed("profile") { + return profileName, true + } + if env := os.Getenv(configuration.EnvProfile); env != "" { + return env, true + } + return configuration.DefaultProfile, false +} + func Execute(version *configuration.Version) error { configuration.Init(version) @@ -60,20 +149,6 @@ func init() { activePrinter := &printer.ConsolePrinter{} printer.Init(activePrinter) - host, token, err := configuration.ReadConfig() - if err != nil { - printer.Error(err) - return - } - - parse, err := url.Parse(host) - if err != nil { - printer.Error(err) - } - - requests.Init(parse, token, Verbose) - routes.Init(parse) - rootCmd.Flags().BoolVarP( &showVersionFlag, "version", @@ -90,13 +165,38 @@ func init() { "Print verbose information of any process that supports this output.", ) + rootCmd.PersistentFlags().StringVarP( + &outputFormat, + "format", + "", + "text", + `Output format. Accepted values: text, json`, + ) + + rootCmd.PersistentFlags().StringVarP( + &profileName, + "profile", + "", + configuration.DefaultProfile, + "Profile name to use (overrides OP_CLI_PROFILE env var)", + ) + rootCmd.AddCommand( loginCmd, - list.RootCmd, - update.RootCmd, - inspect.RootCmd, - create.RootCmd, - search.RootCmd, + logoutCmd, + whoamiCmd, + // noun-first (new) + activity.RootCmd, + budget.RootCmd, + workpackage.RootCmd, + project.RootCmd, + user.RootCmd, + timeentry.RootCmd, + wptype.RootCmd, + status.RootCmd, + notification.RootCmd, git.RootCmd, ) + + rootCmd.InitDefaultCompletionCmd() } diff --git a/cmd/root_test.go b/cmd/root_test.go new file mode 100644 index 0000000..3852aee --- /dev/null +++ b/cmd/root_test.go @@ -0,0 +1,31 @@ +package cmd + +import ( + "errors" + "strings" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" +) + +func TestInvalidOutputFormatStopsCommand(t *testing.T) { + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + outputFormat = "yaml" + t.Cleanup(func() { + outputFormat = "text" + _ = printer.InitRenderer("text") + }) + + err := rootCmd.PersistentPreRunE(rootCmd, nil) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("PersistentPreRunE error = %v, want ErrHandled", err) + } + if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 { + t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult) + } + if testingPrinter.Result != "" { + t.Errorf("stdout = %q, want empty", testingPrinter.Result) + } +} diff --git a/cmd/search/search.go b/cmd/search/search.go deleted file mode 100644 index 389c1cf..0000000 --- a/cmd/search/search.go +++ /dev/null @@ -1,15 +0,0 @@ -package search - -import "github.com/spf13/cobra" - -var RootCmd = &cobra.Command{ - Use: "search [resource]", - Short: "Searches for the specific resource", - Long: `Execute a search on the given resource, with an search -input text. This input can be an id, a key word -like 'me', or a name.`, -} - -func init() { - RootCmd.AddCommand(userCmd) -} diff --git a/cmd/status/list.go b/cmd/status/list.go new file mode 100644 index 0000000..bda1ec3 --- /dev/null +++ b/cmd/status/list.go @@ -0,0 +1,26 @@ +package status + +import ( + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + wpstatus "github.com/opf/openproject-cli/components/resources/status" +) + +var listCmd = &cobra.Command{ + Use: "list", + Short: "Lists statuses", + Long: "Get a list of all statuses of the instance.", + RunE: listStatuses, +} + +func listStatuses(_ *cobra.Command, _ []string) error { + if all, err := wpstatus.All(); err == nil { + printer.StatusList(all) + } else { + printer.Error(err) + return openerrors.ErrHandled + } + return nil +} diff --git a/cmd/status/status.go b/cmd/status/status.go new file mode 100644 index 0000000..7397c1d --- /dev/null +++ b/cmd/status/status.go @@ -0,0 +1,13 @@ +package status + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "status [verb]", + Short: "Manage statuses", + Long: "List work package statuses in OpenProject.", +} + +func init() { + RootCmd.AddCommand(listCmd) +} diff --git a/cmd/timeentry/create.go b/cmd/timeentry/create.go new file mode 100644 index 0000000..3d335d3 --- /dev/null +++ b/cmd/timeentry/create.go @@ -0,0 +1,70 @@ +package timeentry + +import ( + stderrors "errors" + "strconv" + "time" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/time_entries" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +var createWorkPackageId string +var createHours float64 +var createActivity string +var createSpentOn string +var createUserId uint64 +var createComment string + +var createCmd = &cobra.Command{ + Use: "create", + Short: "Create a time entry", + Long: "Log time spent on a work package.", + RunE: createTimeEntry, +} + +func createTimeEntry(cmd *cobra.Command, _ []string) error { + if err := work_packages.ValidateIdentifier(createWorkPackageId); err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + + options := map[time_entries.CreateOption]string{ + time_entries.CreateWorkPackage: createWorkPackageId, + time_entries.CreateHours: strconv.FormatFloat(createHours, 'f', -1, 64), + } + + if cmd.Flags().Changed("activity") { + options[time_entries.CreateActivity] = createActivity + } + + if cmd.Flags().Changed("spent-on") { + options[time_entries.CreateSpentOn] = createSpentOn + } else { + options[time_entries.CreateSpentOn] = time.Now().Format(time.DateOnly) + } + + if createUserId > 0 { + options[time_entries.CreateUser] = strconv.FormatUint(createUserId, 10) + } + + if cmd.Flags().Changed("comment") { + options[time_entries.CreateComment] = createComment + } + + entry, err := time_entries.Create(options) + if err != nil { + if !stderrors.Is(err, openerrors.ErrHandled) { + printer.Error(err) + } + return openerrors.ErrHandled + } + + printer.TimeEntry(entry) + printer.Done() + return nil +} diff --git a/cmd/timeentry/create_flags.go b/cmd/timeentry/create_flags.go new file mode 100644 index 0000000..78df5b8 --- /dev/null +++ b/cmd/timeentry/create_flags.go @@ -0,0 +1,13 @@ +package timeentry + +func initCreateFlags() { + createCmd.Flags().StringVarP(&createWorkPackageId, "work-package", "w", "", "Work package ID or identifier to log time on") + createCmd.Flags().Float64VarP(&createHours, "hours", "", 0, "Number of hours spent (e.g. 1.5 for 1h30m)") + createCmd.Flags().StringVarP(&createActivity, "activity", "", "", "Activity name (e.g. Development, Design)") + createCmd.Flags().StringVarP(&createSpentOn, "spent-on", "", "", "Date of the time entry (YYYY-MM-DD, default: today)") + createCmd.Flags().Uint64VarP(&createUserId, "user", "u", 0, "User ID (default: current user)") + createCmd.Flags().StringVarP(&createComment, "comment", "", "", "Comment for the time entry") + + _ = createCmd.MarkFlagRequired("work-package") + _ = createCmd.MarkFlagRequired("hours") +} diff --git a/cmd/list/time_entries.go b/cmd/timeentry/list.go similarity index 77% rename from cmd/list/time_entries.go rename to cmd/timeentry/list.go index 0bd04f8..1049e9b 100644 --- a/cmd/list/time_entries.go +++ b/cmd/timeentry/list.go @@ -1,37 +1,41 @@ -package list +package timeentry import ( + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/components/resources" "github.com/opf/openproject-cli/components/resources/time_entries" "github.com/opf/openproject-cli/components/resources/time_entries/filters" - "github.com/spf13/cobra" ) var activeTimeEntryFilters = map[string]resources.Filter{ "user": filters.NewUserFilter(), } -var timeEntriesCmd = &cobra.Command{ - Use: "timeentries", +var listCmd = &cobra.Command{ + Use: "list", Short: "Lists time entries", Long: "Get a list of all time entries.", - Run: listTimeEntries, + RunE: listTimeEntries, } -func listTimeEntries(_ *cobra.Command, _ []string) { +func listTimeEntries(_ *cobra.Command, _ []string) error { query, err := buildTimeEntriesQuery() if err != nil { printer.ErrorText(err.Error()) - return + return openerrors.ErrHandled } if all, err := time_entries.All(query); err == nil { printer.TimeEntryList(all) } else { printer.Error(err) + return openerrors.ErrHandled } + return nil } func buildTimeEntriesQuery() (requests.Query, error) { diff --git a/cmd/list/time_entries_flags.go b/cmd/timeentry/list_flags.go similarity index 68% rename from cmd/list/time_entries_flags.go rename to cmd/timeentry/list_flags.go index 873a587..cd8b226 100644 --- a/cmd/list/time_entries_flags.go +++ b/cmd/timeentry/list_flags.go @@ -1,8 +1,8 @@ -package list +package timeentry -func initTimeEntriesFlags() { +func initListFlags() { for _, filter := range activeTimeEntryFilters { - timeEntriesCmd.Flags().StringVarP( + listCmd.Flags().StringVarP( filter.ValuePointer(), filter.Name(), filter.ShortHand(), diff --git a/cmd/timeentry/timeentry.go b/cmd/timeentry/timeentry.go new file mode 100644 index 0000000..c6bc42f --- /dev/null +++ b/cmd/timeentry/timeentry.go @@ -0,0 +1,16 @@ +package timeentry + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "time-entry [verb]", + Short: "Manage time entries", + Long: "List and create time entries in OpenProject.", +} + +func init() { + initListFlags() + initCreateFlags() + + RootCmd.AddCommand(listCmd, createCmd) +} diff --git a/cmd/timeentry/timeentry_test.go b/cmd/timeentry/timeentry_test.go new file mode 100644 index 0000000..c150e57 --- /dev/null +++ b/cmd/timeentry/timeentry_test.go @@ -0,0 +1,15 @@ +package timeentry_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/cmd/timeentry" +) + +func TestRootCmd_UsesHyphenatedName(t *testing.T) { + use := timeentry.RootCmd.Use + if !strings.HasPrefix(use, "time-entry") { + t.Errorf("expected command name 'time-entry', got %q", use) + } +} diff --git a/cmd/update/update.go b/cmd/update/update.go deleted file mode 100644 index d6e9fc1..0000000 --- a/cmd/update/update.go +++ /dev/null @@ -1,53 +0,0 @@ -package update - -import "github.com/spf13/cobra" - -var RootCmd = &cobra.Command{ - Use: "update [resource] [id]", - Short: "Updates the specific resource", - Long: `Sends an update to the given resource, -which is identified by its id. The data -to update is determined by the provided -flags.`, -} - -func init() { - addWorkPackageFlags() - - RootCmd.AddCommand(workPackageCmd) -} - -func addWorkPackageFlags() { - workPackageCmd.Flags().StringVarP( - &actionFlag, - "action", - "a", - "", - "Executes a custom action on a work package", - ) - workPackageCmd.Flags().Uint64Var( - &assigneeFlag, - "assignee", - 0, - "Assign a user to the work package", - ) - workPackageCmd.Flags().StringVar( - &attachFlag, - "attach", - "", - "Attach a file to the work package", - ) - workPackageCmd.Flags().StringVar( - &subjectFlag, - "subject", - "", - "Change the subject of the work package", - ) - workPackageCmd.Flags().StringVarP( - &typeFlag, - "type", - "t", - "", - "Change the work package type", - ) -} diff --git a/cmd/update/work_package.go b/cmd/update/work_package.go deleted file mode 100644 index ad80ec7..0000000 --- a/cmd/update/work_package.go +++ /dev/null @@ -1,68 +0,0 @@ -package update - -import ( - "fmt" - "strconv" - - "github.com/spf13/cobra" - - "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 -) - -var workPackageCmd = &cobra.Command{ - Use: "workpackage [id]", - Short: "Updates the work package", - Long: `Update a work package. Each update -provided by a flag is executed on its own.`, - Run: updateWorkPackage, -} - -func updateWorkPackage(_ *cobra.Command, args []string) { - if len(args) != 1 { - printer.ErrorText(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])) - return - } - - if workPackage, err := work_packages.Update(id, updateOptions()); err == nil { - printer.Info("-- ") - printer.WorkPackage(workPackage) - } else { - printer.Error(err) - } -} - -func updateOptions() map[work_packages.UpdateOption]string { - var options = make(map[work_packages.UpdateOption]string) - if len(actionFlag) > 0 { - options[work_packages.UpdateCustomAction] = actionFlag - } - if assigneeFlag > 0 { - options[work_packages.UpdateAssignee] = strconv.FormatUint(assigneeFlag, 10) - } - if len(attachFlag) > 0 { - options[work_packages.UpdateAttachment] = attachFlag - } - if len(subjectFlag) > 0 { - options[work_packages.UpdateSubject] = subjectFlag - } - if len(typeFlag) > 0 { - options[work_packages.UpdateType] = typeFlag - } - - return options -} diff --git a/cmd/search/user.go b/cmd/user/search.go similarity index 58% rename from cmd/search/user.go rename to cmd/user/search.go index f99f27c..9e36ff2 100644 --- a/cmd/search/user.go +++ b/cmd/user/search.go @@ -1,48 +1,51 @@ -package search +package user import ( "fmt" + + "github.com/spf13/cobra" + "github.com/opf/openproject-cli/components/common" + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/users" - "github.com/spf13/cobra" ) -var userCmd = &cobra.Command{ - Use: "user [searchInput]", +var keywords = []string{"me"} + +var searchCmd = &cobra.Command{ + Use: "search [searchInput]", Short: "Searches for a user", Long: "Searches for a user by id, keyword, or name. Returns a list of possible matches.", - Run: searchUser, + RunE: searchUser, } -var keywords = []string{"me"} - -func searchUser(_ *cobra.Command, args []string) { +func searchUser(_ *cobra.Command, args []string) error { if len(args) != 1 { printer.ErrorText(fmt.Sprintf("Expected 1 argument [searchInput], but got %d", len(args))) - return + return openerrors.ErrHandled } + query := args[0] - if common.Contains(keywords, args[0]) { + if common.Contains(keywords, query) { me, err := users.Me() if err != nil { printer.Error(err) - } else { - printer.User(me) + return openerrors.ErrHandled } - - return + printer.User(me) + return nil } - collection, err := users.Search(args[0]) + collection, err := users.Search(query) if err != nil { printer.Error(err) - return + return openerrors.ErrHandled } if len(collection) == 0 { - printer.Info(fmt.Sprintf("No user found for search input %s.", printer.Cyan(args[0]))) - } else { - printer.Users(collection) + printer.Info(fmt.Sprintf("No user found for search input %s.", printer.Cyan(query))) } + printer.Users(collection) + return nil } diff --git a/cmd/user/search_test.go b/cmd/user/search_test.go new file mode 100644 index 0000000..36e3ce0 --- /dev/null +++ b/cmd/user/search_test.go @@ -0,0 +1,71 @@ +package user + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" +) + +func TestSearchUsersRendersEmptyJSONArray(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, _ *http.Request) { + response.Header().Set("Content-Type", "application/json") + _, _ = response.Write([]byte(`{"_embedded":{"elements":[]},"total":0,"count":0}`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + if err := printer.InitRenderer("json"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = printer.InitRenderer("text") + }) + + if err := searchUser(nil, []string{"missing person"}); err != nil { + t.Fatalf("searchUser returned an error: %v", err) + } + + if testingPrinter.Result != "[]\n" { + t.Errorf("stdout = %q, want %q", testingPrinter.Result, "[]\n") + } + if !strings.Contains(testingPrinter.ErrResult, "missing person") { + t.Errorf("stderr should contain the complete query, got %q", testingPrinter.ErrResult) + } +} + +func TestSearchUserRequiresOneArgument(t *testing.T) { + requestCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, _ *http.Request) { + requestCount++ + _, _ = response.Write([]byte(`{"_embedded":{"elements":[]}}`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + + if err := searchUser(nil, []string{"missing", "person"}); err == nil { + t.Fatal("searchUser returned nil for two arguments") + } + if requestCount != 0 { + t.Errorf("request count = %d, want 0", requestCount) + } +} diff --git a/cmd/user/user.go b/cmd/user/user.go new file mode 100644 index 0000000..52bf1e3 --- /dev/null +++ b/cmd/user/user.go @@ -0,0 +1,13 @@ +package user + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "user [verb]", + Short: "Manage users", + Long: "Search and inspect users in OpenProject.", +} + +func init() { + RootCmd.AddCommand(searchCmd) +} diff --git a/cmd/whoami.go b/cmd/whoami.go new file mode 100644 index 0000000..5a342a5 --- /dev/null +++ b/cmd/whoami.go @@ -0,0 +1,116 @@ +package cmd + +import ( + stderrors "errors" + "net/http" + "net/url" + + "github.com/spf13/cobra" + + "github.com/opf/openproject-cli/components/configuration" + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/users" + "github.com/opf/openproject-cli/components/routes" +) + +var whoamiCmd = &cobra.Command{ + Use: "whoami", + Short: "Show current user and server", + Long: "Display the configured OpenProject server and the currently authenticated user.", + RunE: whoami, +} + +func whoami(cmd *cobra.Command, _ []string) error { + // OP_CLI_HOST/OP_CLI_TOKEN override every stored profile, so listing file + // profiles would just query the same server under different labels. + if configuration.HasEnvironmentConfig() { + return whoamiSingle("environment") + } + + profile, explicit := resolvedProfile(cmd) + + if explicit { + return whoamiSingle(profile) + } + + // No profile specified: show all profiles + profiles, err := configuration.AllProfiles() + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + if len(profiles) == 0 { + printer.ErrorText("No profiles configured. Run `op login` to authenticate.") + return openerrors.ErrHandled + } + + var entries []printer.WhoamiEntry + var failed bool + for _, p := range profiles { + entry, err := whoamiEntry(p.Name) + if err != nil { + failed = true + continue + } + entries = append(entries, entry) + } + + if len(entries) > 0 { + printer.WhoamiList(entries) + } + if failed { + return openerrors.ErrHandled + } + return nil +} + +// whoamiSingle renders exactly one profile as a list, so JSON output stays a +// single-element array consistent with the all-profiles path. +func whoamiSingle(profile string) error { + entry, err := whoamiEntry(profile) + if err != nil { + return err + } + printer.WhoamiList([]printer.WhoamiEntry{entry}) + return nil +} + +func whoamiEntry(profile string) (printer.WhoamiEntry, error) { + var entry printer.WhoamiEntry + + host, token, err := configuration.ReadConfig(profile) + if err != nil { + printer.Error(err) + return entry, openerrors.ErrHandled + } + + if host == "" { + printer.ErrorText("Profile \"" + profile + "\" is not configured. Run `op login --profile " + profile + "` to authenticate.") + return entry, openerrors.ErrHandled + } + + parse, err := url.Parse(host) + if err != nil { + printer.Error(err) + return entry, openerrors.ErrHandled + } + requests.Init(parse, token, Verbose) + routes.Init(parse) + + user, err := users.Me() + if err != nil { + printer.Info("Server: " + host) + var responseErr *openerrors.ResponseError + if stderrors.As(err, &responseErr) && responseErr.Status() == http.StatusUnauthorized { + printer.ErrorText("Invalid or expired token. Run `op login --profile " + profile + "` to re-authenticate.") + } else { + printer.Error(err) + } + return entry, openerrors.ErrHandled + } + + return printer.WhoamiEntry{Profile: profile, Host: host, User: user}, nil +} diff --git a/cmd/workpackage/create.go b/cmd/workpackage/create.go new file mode 100644 index 0000000..61f1d29 --- /dev/null +++ b/cmd/workpackage/create.go @@ -0,0 +1,75 @@ +package workpackage + +import ( + stderrors "errors" + "fmt" + "strconv" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/launch" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/projects" + "github.com/opf/openproject-cli/components/resources/work_packages" + "github.com/opf/openproject-cli/components/routes" +) + +var createProjectId string +var createOpenInBrowser bool +var createTypeFlag string +var createAssigneeFlag uint64 +var createDescriptionFlag string + +var createCmd = &cobra.Command{ + Use: "create [subject]", + Short: "Create work package in project", + Long: "Create a new work package with the given subject in a project", + RunE: createWorkPackage, +} + +func createWorkPackage(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + printer.ErrorText(fmt.Sprintf("Expected 1 argument [subject], but got %d", len(args))) + return openerrors.ErrHandled + } + + subject := args[0] + if err := projects.ValidateIdentifier(createProjectId); err != nil { + printer.ErrorText(fmt.Sprintf("--project: %s", err.Error())) + return openerrors.ErrHandled + } + + workPackage, err := work_packages.Create(createProjectId, createOptions(cmd, subject)) + if err != nil { + if !stderrors.Is(err, openerrors.ErrHandled) { + printer.Error(err) + } + return openerrors.ErrHandled + } + + if createOpenInBrowser { + err = launch.Browser(routes.WorkPackageUrl(workPackage)) + if err != nil { + printer.Warning(fmt.Sprintf("Error opening browser: %+v", err)) + } + } else { + printer.WorkPackage(workPackage) + } + return nil +} + +func createOptions(cmd *cobra.Command, subject string) map[work_packages.CreateOption]string { + options := make(map[work_packages.CreateOption]string) + options[work_packages.CreateSubject] = subject + if len(createTypeFlag) > 0 { + options[work_packages.CreateType] = createTypeFlag + } + if createAssigneeFlag > 0 { + options[work_packages.CreateAssignee] = strconv.FormatUint(createAssigneeFlag, 10) + } + if cmd.Flags().Changed("description") { + options[work_packages.CreateDescription] = createDescriptionFlag + } + return options +} diff --git a/cmd/workpackage/create_test.go b/cmd/workpackage/create_test.go new file mode 100644 index 0000000..8868fb8 --- /dev/null +++ b/cmd/workpackage/create_test.go @@ -0,0 +1,43 @@ +package workpackage + +import ( + "net/http" + "strings" + "testing" +) + +func TestCreateBrowserFailureKeepsSuccessfulExit(t *testing.T) { + testingPrinter, requestCount := initWorkPackageTestServer(t, func(response http.ResponseWriter, request *http.Request) { + if request.Method != http.MethodPost { + t.Errorf("request method = %s, want POST", request.Method) + } + response.Header().Set("Content-Type", "application/json") + _, _ = response.Write([]byte(`{"id":42,"displayId":"42","subject":"Created"}`)) + }) + + t.Setenv("PATH", t.TempDir()) + createProjectId = "1" + createOpenInBrowser = true + createTypeFlag = "" + createAssigneeFlag = 0 + createDescriptionFlag = "" + t.Cleanup(func() { + createProjectId = "" + createOpenInBrowser = false + createTypeFlag = "" + createAssigneeFlag = 0 + createDescriptionFlag = "" + }) + + if err := createWorkPackage(createCmd, []string{"Created"}); err != nil { + t.Fatalf("createWorkPackage returned an error after the API create succeeded: %v", err) + } + + if *requestCount != 1 { + t.Errorf("request count = %d, want 1", *requestCount) + } + if !strings.Contains(testingPrinter.ErrResult, "[WARNING]") || + !strings.Contains(testingPrinter.ErrResult, "Error opening browser") { + t.Errorf("stderr should warn about the browser failure, got %q", testingPrinter.ErrResult) + } +} diff --git a/cmd/workpackage/helpers.go b/cmd/workpackage/helpers.go new file mode 100644 index 0000000..6869e80 --- /dev/null +++ b/cmd/workpackage/helpers.go @@ -0,0 +1,10 @@ +package workpackage + +import opErrors "github.com/opf/openproject-cli/components/errors" + +func isNotFound(err error) bool { + if respErr, ok := err.(*opErrors.ResponseError); ok { + return respErr.Status() == 404 + } + return false +} diff --git a/cmd/workpackage/inspect.go b/cmd/workpackage/inspect.go new file mode 100644 index 0000000..6af6e94 --- /dev/null +++ b/cmd/workpackage/inspect.go @@ -0,0 +1,75 @@ +package workpackage + +import ( + "fmt" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/launch" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/work_packages" + "github.com/opf/openproject-cli/components/routes" +) + +var inspectOpenInBrowser bool +var inspectListAvailableTypes bool + +var inspectCmd = &cobra.Command{ + Use: "inspect [id]", + Short: "Show details about a work package", + Long: "Show detailed information of a work package referenced by its numeric ID (e.g. 12345) or project-based identifier (e.g. PROJ-123).", + RunE: inspectWorkPackage, +} + +func inspectWorkPackage(_ *cobra.Command, args []string) error { + if len(args) != 1 { + printer.ErrorText(fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) + return openerrors.ErrHandled + } + + id := args[0] + if err := work_packages.ValidateIdentifier(id); err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + + if inspectHasListingFlag() { + switch { + case inspectListAvailableTypes: + return inspectAvailableTypes(id) + } + return nil + } + + workPackage, err := work_packages.Lookup(id) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + + if inspectOpenInBrowser { + err = launch.Browser(routes.WorkPackageUrl(workPackage)) + if err != nil { + printer.ErrorText(fmt.Sprintf("Error opening browser: %+v", err)) + return openerrors.ErrHandled + } + } else { + printer.WorkPackage(workPackage) + } + return nil +} + +func inspectAvailableTypes(id string) error { + types, err := work_packages.AvailableTypes(id) + if err != nil { + printer.Error(err) + return openerrors.ErrHandled + } + printer.Types(types) + return nil +} + +func inspectHasListingFlag() bool { + return inspectListAvailableTypes +} diff --git a/cmd/workpackage/inspect_test.go b/cmd/workpackage/inspect_test.go new file mode 100644 index 0000000..8a47b37 --- /dev/null +++ b/cmd/workpackage/inspect_test.go @@ -0,0 +1,37 @@ +package workpackage + +import ( + "errors" + "net/http" + "strings" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" +) + +func TestInspectBrowserFailureReturnsError(t *testing.T) { + testingPrinter, requestCount := initWorkPackageTestServer(t, func(response http.ResponseWriter, request *http.Request) { + if request.Method != http.MethodGet { + t.Errorf("request method = %s, want GET", request.Method) + } + response.Header().Set("Content-Type", "application/json") + _, _ = response.Write([]byte(`{"id":42,"displayId":"42","subject":"Example"}`)) + }) + + t.Setenv("PATH", t.TempDir()) + inspectOpenInBrowser = true + t.Cleanup(func() { + inspectOpenInBrowser = false + }) + + err := inspectWorkPackage(nil, []string{"42"}) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("inspectWorkPackage error = %v, want ErrHandled", err) + } + if *requestCount != 1 { + t.Errorf("request count = %d, want 1", *requestCount) + } + if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 { + t.Errorf("error diagnostic count = %d, want 1; stderr: %q", count, testingPrinter.ErrResult) + } +} diff --git a/cmd/workpackage/list.go b/cmd/workpackage/list.go new file mode 100644 index 0000000..b431e33 --- /dev/null +++ b/cmd/workpackage/list.go @@ -0,0 +1,183 @@ +package workpackage + +import ( + "fmt" + "regexp" + "strconv" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources" + "github.com/opf/openproject-cli/components/resources/projects" + "github.com/opf/openproject-cli/components/resources/work_packages" + "github.com/opf/openproject-cli/components/resources/work_packages/filters" +) + +var listAssignee string +var listParentId string +var listProjectId string +var listShowTotal bool +var listStatusFilter string +var listTypeFilter string +var listIncludeSubProjects bool + +var activeFilters = map[string]resources.Filter{ + "notSubProject": filters.NewNotSubProjectFilter(), + "notVersion": filters.NewNotVersionFilter(), + "subProject": filters.NewSubProjectFilter(), + "timestamp": filters.NewTimestampFilter(), + "version": filters.NewVersionFilter(), +} + +var listCmd = &cobra.Command{ + Use: "list", + Short: "Lists work packages", + Long: "Get a list of visible work packages. Filter flags can be applied.", + RunE: listWorkPackages, +} + +func listWorkPackages(_ *cobra.Command, _ []string) error { + if errorText := validateCommandFlagComposition(); len(errorText) > 0 { + printer.ErrorText(errorText) + return openerrors.ErrHandled + } + + if len(listProjectId) > 0 { + if err := projects.ValidateIdentifier(listProjectId); err != nil { + printer.ErrorText(fmt.Sprintf("--project: %s", err.Error())) + return openerrors.ErrHandled + } + } + + if len(listParentId) > 0 { + if err := work_packages.ValidateIdentifier(listParentId); err != nil { + printer.ErrorText(fmt.Sprintf("--parent-id: %s", err.Error())) + return openerrors.ErrHandled + } + parentWp, err := work_packages.Lookup(listParentId) + if err != nil { + if isNotFound(err) { + printer.ErrorText(fmt.Sprintf("--parent-id: work package %s not found.", listParentId)) + } else { + printer.Error(err) + } + return openerrors.ErrHandled + } + listParentId = fmt.Sprintf("%d", parentWp.Id) + } + + query, err := buildQuery() + if err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + + options, err := filterOptions() + if err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + + collection, err := work_packages.All(options, query, listShowTotal) + switch { + case err == nil && listShowTotal: + printer.Number(collection.Total) + case err == nil: + printer.WorkPackages(collection.Items) + case isNotFound(err) && len(listProjectId) > 0: + printer.ErrorText(fmt.Sprintf("--project: no project found with identifier or ID '%s'", listProjectId)) + return openerrors.ErrHandled + default: + printer.Error(err) + return openerrors.ErrHandled + } + return nil +} + +func validateCommandFlagComposition() (errorText string) { + switch { + case len(activeFilters["version"].Value()) != 0 && len(listProjectId) == 0: + return "Version flag (--version) can only be used in conjunction with project flag (-p or --project)." + case len(activeFilters["notVersion"].Value()) != 0 && len(listProjectId) == 0: + return "Not version filter flag (--not-version) can only be used in conjunction with project flag (-p or --project)." + case len(activeFilters["subProject"].Value()) > 0 || len(activeFilters["notSubProject"].Value()) > 0: + if !listIncludeSubProjects || len(listProjectId) == 0 { + return `Sub project filter flags (--sub-project or --not-sub-project) can only be used +in conjunction with setting the flag --include-sub-projects and setting a +project with the project flag (-p or --project).` + } + } + + return "" +} + +func buildQuery() (requests.Query, error) { + var q requests.Query + + for _, filter := range activeFilters { + if filter.Value() == filter.DefaultValue() { + continue + } + + err := filter.ValidateInput() + if err != nil { + return requests.NewEmptyQuery(), err + } + + q = q.Merge(filter.Query()) + } + + return q, nil +} + +func filterOptions() (*map[work_packages.FilterOption]string, error) { + options := make(map[work_packages.FilterOption]string) + + options[work_packages.IncludeSubProjects] = strconv.FormatBool(listIncludeSubProjects) + + if len(listParentId) > 0 { + options[work_packages.Parent] = listParentId + } + + if len(listProjectId) > 0 { + options[work_packages.Project] = listProjectId + } + + if len(listAssignee) > 0 { + options[work_packages.Assignee] = listAssignee + } + + if len(listStatusFilter) > 0 { + value, err := validateFilterValue(work_packages.Status, listStatusFilter) + if err != nil { + return nil, err + } + options[work_packages.Status] = value + } + + if len(listTypeFilter) > 0 { + value, err := validateFilterValue(work_packages.Type, listTypeFilter) + if err != nil { + return nil, err + } + options[work_packages.Type] = value + } + + return &options, nil +} + +func validateFilterValue(filter work_packages.FilterOption, value string) (string, error) { + matched, err := regexp.Match(work_packages.InputValidationExpression[filter], []byte(value)) + if err != nil { + return "", err + } + + if !matched { + return "", fmt.Errorf("invalid %s value %s", filter, printer.Yellow(value)) + } + + return value, nil +} diff --git a/cmd/list/work_packages_flags.go b/cmd/workpackage/list_flags.go similarity index 66% rename from cmd/list/work_packages_flags.go rename to cmd/workpackage/list_flags.go index a08b9f9..d7510b1 100644 --- a/cmd/list/work_packages_flags.go +++ b/cmd/workpackage/list_flags.go @@ -1,23 +1,30 @@ -package list +package workpackage -func initWorkPackagesFlags() { - workPackagesCmd.Flags().StringVarP( - &assignee, +func initListFlags() { + listCmd.Flags().StringVarP( + &listAssignee, "assignee", "a", "", "Assignee of the work package (can be ID or 'me')", ) - workPackagesCmd.Flags().Uint64VarP( - &projectId, - "project-id", + listCmd.Flags().StringVarP( + &listParentId, + "parent-id", + "", + "", + "Show only direct children of the specified work package ID or identifier") + + listCmd.Flags().StringVarP( + &listProjectId, + "project", "p", - 0, - "Show only work packages within the specified projectId") + "", + "Show only work packages within the specified project (numeric ID or identifier)") - workPackagesCmd.Flags().StringVarP( - &statusFilter, + listCmd.Flags().StringVarP( + &listStatusFilter, "status", "s", "", @@ -27,8 +34,8 @@ keywords 'open', 'closed', a single ID or a comma separated array of IDs, i.e. prefixed with an '!' the list is instead filtered to not have the specified status.`) - workPackagesCmd.Flags().StringVarP( - &typeFilter, + listCmd.Flags().StringVarP( + &listTypeFilter, "type", "t", "", @@ -37,8 +44,8 @@ ID or a comma separated array of IDs, i.e. '7,13'. Multiple values are concatenated with a logical 'OR'. If the IDs are prefixed with an '!' the list is instead filtered to not have the specified status.`) - workPackagesCmd.Flags().BoolVarP( - &includeSubProjects, + listCmd.Flags().BoolVarP( + &listIncludeSubProjects, "include-sub-projects", "", false, @@ -46,15 +53,15 @@ is instead filtered to not have the specified status.`) packages of sub projects should be included in the list. If omitting the flag, the default is false.`) - workPackagesCmd.Flags().BoolVarP( - &showTotal, + listCmd.Flags().BoolVarP( + &listShowTotal, "total", "", false, "Show only the total number of work packages matching the filter options.") for _, filter := range activeFilters { - workPackagesCmd.Flags().StringVarP( + listCmd.Flags().StringVarP( filter.ValuePointer(), filter.Name(), filter.ShortHand(), diff --git a/cmd/workpackage/options_test.go b/cmd/workpackage/options_test.go new file mode 100644 index 0000000..d66d8dd --- /dev/null +++ b/cmd/workpackage/options_test.go @@ -0,0 +1,99 @@ +package workpackage + +import ( + "errors" + "strings" + "testing" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func newCmdWithDescriptionFlag(flag *string) *cobra.Command { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().StringVar(flag, "description", "", "") + return cmd +} + +func TestCreateOptions_DescriptionOmitted(t *testing.T) { + createDescriptionFlag = "" + cmd := newCmdWithDescriptionFlag(&createDescriptionFlag) + _ = cmd.Flags().Parse([]string{}) + + options := createOptions(cmd, "subject") + + if _, ok := options[work_packages.CreateDescription]; ok { + t.Error("expected CreateDescription to be absent when flag not provided") + } +} + +func TestCreateOptions_DescriptionProvided(t *testing.T) { + createDescriptionFlag = "" + cmd := newCmdWithDescriptionFlag(&createDescriptionFlag) + _ = cmd.Flags().Parse([]string{"--description", "## Hello"}) + + options := createOptions(cmd, "subject") + + val, ok := options[work_packages.CreateDescription] + if !ok { + t.Error("expected CreateDescription to be present when flag provided") + } + if val != "## Hello" { + t.Errorf("expected %q, got %q", "## Hello", val) + } +} + +func TestUpdateOptions_DescriptionOmitted(t *testing.T) { + updateDescriptionFlag = "" + cmd := newCmdWithDescriptionFlag(&updateDescriptionFlag) + _ = cmd.Flags().Parse([]string{}) + + options := updateOptions(cmd) + + if _, ok := options[work_packages.UpdateDescription]; ok { + t.Error("expected UpdateDescription to be absent when flag not provided") + } +} + +func TestUpdateOptions_DescriptionProvided(t *testing.T) { + updateDescriptionFlag = "" + cmd := newCmdWithDescriptionFlag(&updateDescriptionFlag) + _ = cmd.Flags().Parse([]string{"--description", ""}) + + options := updateOptions(cmd) + + if _, ok := options[work_packages.UpdateDescription]; !ok { + t.Error("expected UpdateDescription to be present when flag explicitly provided with empty string") + } +} + +func TestUpdateWithoutOptionsStopsBeforeRequest(t *testing.T) { + updateActionFlag = "" + updateAssigneeFlag = 0 + updateAttachFlag = "" + updateDescriptionFlag = "" + updateSubjectFlag = "" + updateTypeFlag = "" + + cmd := newCmdWithDescriptionFlag(&updateDescriptionFlag) + if err := cmd.Flags().Parse(nil); err != nil { + t.Fatal(err) + } + + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + + err := updateWorkPackage(cmd, []string{"42"}) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("updateWorkPackage error = %v, want ErrHandled", err) + } + if !strings.Contains(testingPrinter.ErrResult, "No update options provided") { + t.Errorf("stderr = %q, want no-options diagnostic", testingPrinter.ErrResult) + } + if strings.Contains(testingPrinter.ErrResult, "Cannot execute requests") { + t.Errorf("command attempted a request: %q", testingPrinter.ErrResult) + } +} diff --git a/cmd/workpackage/search.go b/cmd/workpackage/search.go new file mode 100644 index 0000000..a3f68c1 --- /dev/null +++ b/cmd/workpackage/search.go @@ -0,0 +1,55 @@ +package workpackage + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/projects" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +var searchProjectId string + +var searchCmd = &cobra.Command{ + Use: "search ...", + Short: "Searches for work packages", + Long: `Searches for work packages by subject, type, status, project name, or identifier. +Multiple words are ANDed: all terms must match. Returns up to 100 results.`, + RunE: searchWorkPackages, +} + +func searchWorkPackages(_ *cobra.Command, args []string) error { + query := strings.Join(args, " ") + if strings.TrimSpace(query) == "" { + printer.ErrorText("Search query cannot be blank") + return openerrors.ErrHandled + } + + isProjectScoped := len(searchProjectId) > 0 + if isProjectScoped { + if err := projects.ValidateIdentifier(searchProjectId); err != nil { + printer.ErrorText(fmt.Sprintf("--project: %s", err.Error())) + return openerrors.ErrHandled + } + } + + collection, err := work_packages.Search(query, searchProjectId) + if err != nil { + if isNotFound(err) && isProjectScoped { + printer.ErrorText(fmt.Sprintf("--project: no project found with identifier or ID '%s'", searchProjectId)) + } else { + printer.Error(err) + } + return openerrors.ErrHandled + } + + if len(collection) == 0 { + printer.Info(fmt.Sprintf("No work package found for search input %s.", printer.Cyan(query))) + } + printer.WorkPackages(collection) + return nil +} diff --git a/cmd/workpackage/search_test.go b/cmd/workpackage/search_test.go new file mode 100644 index 0000000..972a001 --- /dev/null +++ b/cmd/workpackage/search_test.go @@ -0,0 +1,25 @@ +package workpackage + +import ( + "net/http" + "strings" + "testing" +) + +func TestSearchWorkPackagesRendersEmptyJSONArray(t *testing.T) { + testingPrinter, _ := initWorkPackageTestServer(t, func(response http.ResponseWriter, _ *http.Request) { + response.Header().Set("Content-Type", "application/json") + _, _ = response.Write([]byte(`{"_embedded":{"elements":[]},"total":0,"count":0}`)) + }) + + if err := searchWorkPackages(nil, []string{"missing"}); err != nil { + t.Fatalf("searchWorkPackages returned an error: %v", err) + } + + if testingPrinter.Result != "[]\n" { + t.Errorf("stdout = %q, want %q", testingPrinter.Result, "[]\n") + } + if !strings.Contains(testingPrinter.ErrResult, "No work package found") { + t.Errorf("stderr should describe the empty result, got %q", testingPrinter.ErrResult) + } +} diff --git a/cmd/workpackage/test_helpers_test.go b/cmd/workpackage/test_helpers_test.go new file mode 100644 index 0000000..73c7c0d --- /dev/null +++ b/cmd/workpackage/test_helpers_test.go @@ -0,0 +1,41 @@ +package workpackage + +import ( + "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 initWorkPackageTestServer(t *testing.T, handler http.HandlerFunc) (*printer.TestingPrinter, *int) { + t.Helper() + + requestCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + requestCount++ + handler(response, request) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + routes.Init(host) + + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + if err := printer.InitRenderer("json"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + _ = printer.InitRenderer("text") + }) + + return testingPrinter, &requestCount +} diff --git a/cmd/workpackage/update.go b/cmd/workpackage/update.go new file mode 100644 index 0000000..1627fcc --- /dev/null +++ b/cmd/workpackage/update.go @@ -0,0 +1,82 @@ +package workpackage + +import ( + stderrors "errors" + "fmt" + "strconv" + + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +var updateActionFlag string +var updateAssigneeFlag uint64 +var updateAttachFlag string +var updateDescriptionFlag string +var updateSubjectFlag string +var updateTypeFlag string + +var updateCmd = &cobra.Command{ + Use: "update [id]", + Short: "Updates the work package", + Long: `Update a work package referenced by its numeric ID (e.g. 12345) or project-based identifier (e.g. PROJ-123). Each update +provided by a flag is executed on its own.`, + RunE: updateWorkPackage, +} + +func updateWorkPackage(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + printer.ErrorText(fmt.Sprintf("Expected 1 argument [id], but got %d", len(args))) + return openerrors.ErrHandled + } + + id := args[0] + if err := work_packages.ValidateIdentifier(id); err != nil { + printer.ErrorText(err.Error()) + return openerrors.ErrHandled + } + + options := updateOptions(cmd) + if len(options) == 0 { + printer.ErrorText("No update options provided. Use --help to see available flags.") + return openerrors.ErrHandled + } + + workPackage, err := work_packages.Update(id, options) + if err != nil { + if !stderrors.Is(err, openerrors.ErrHandled) { + printer.Error(err) + } + return openerrors.ErrHandled + } + + printer.Info("-- ") + printer.WorkPackage(workPackage) + return nil +} + +func updateOptions(cmd *cobra.Command) map[work_packages.UpdateOption]string { + options := make(map[work_packages.UpdateOption]string) + if len(updateActionFlag) > 0 { + options[work_packages.UpdateCustomAction] = updateActionFlag + } + if updateAssigneeFlag > 0 { + options[work_packages.UpdateAssignee] = strconv.FormatUint(updateAssigneeFlag, 10) + } + if len(updateAttachFlag) > 0 { + options[work_packages.UpdateAttachment] = updateAttachFlag + } + if cmd.Flags().Changed("description") { + options[work_packages.UpdateDescription] = updateDescriptionFlag + } + if len(updateSubjectFlag) > 0 { + options[work_packages.UpdateSubject] = updateSubjectFlag + } + if len(updateTypeFlag) > 0 { + options[work_packages.UpdateType] = updateTypeFlag + } + return options +} diff --git a/cmd/workpackage/workpackage.go b/cmd/workpackage/workpackage.go new file mode 100644 index 0000000..2f39122 --- /dev/null +++ b/cmd/workpackage/workpackage.go @@ -0,0 +1,111 @@ +package workpackage + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "work-package [verb]", + Short: "Manage work packages", + Long: "Create, list, update, inspect, and manage work packages in OpenProject.", +} + +func init() { + initListFlags() + + createCmd.Flags().StringVarP( + &createProjectId, + "project", + "p", + "", + "Project numeric ID or identifier to create the work package in", + ) + _ = createCmd.MarkFlagRequired("project") + createCmd.Flags().BoolVarP( + &createOpenInBrowser, + "open", + "o", + false, + "Open the created work package in the default browser", + ) + createCmd.Flags().StringVarP( + &createTypeFlag, + "type", + "t", + "", + "Change the work package type", + ) + createCmd.Flags().Uint64Var( + &createAssigneeFlag, + "assignee", + 0, + "Assign a user to the work package", + ) + createCmd.Flags().StringVar( + &createDescriptionFlag, + "description", + "", + "Description of the work package (markdown)", + ) + + updateCmd.Flags().StringVarP( + &updateActionFlag, + "action", + "a", + "", + "Executes a custom action on a work package", + ) + updateCmd.Flags().Uint64Var( + &updateAssigneeFlag, + "assignee", + 0, + "Assign a user to the work package", + ) + updateCmd.Flags().StringVar( + &updateAttachFlag, + "attach", + "", + "Attach a file to the work package", + ) + updateCmd.Flags().StringVar( + &updateDescriptionFlag, + "description", + "", + "Description of the work package (markdown)", + ) + updateCmd.Flags().StringVar( + &updateSubjectFlag, + "subject", + "", + "Change the subject of the work package", + ) + updateCmd.Flags().StringVarP( + &updateTypeFlag, + "type", + "t", + "", + "Change the work package type", + ) + + inspectCmd.Flags().BoolVarP( + &inspectOpenInBrowser, + "open", + "o", + false, + "Open the work package in the default browser", + ) + inspectCmd.Flags().BoolVar( + &inspectListAvailableTypes, + "types", + false, + "List the available types on the work package.", + ) + + searchCmd.Flags().StringVarP( + &searchProjectId, + "project", + "p", + "", + "Limit search to a project (numeric ID or identifier)", + ) + + RootCmd.AddCommand(listCmd, createCmd, updateCmd, inspectCmd, searchCmd) +} diff --git a/cmd/workpackage/workpackage_test.go b/cmd/workpackage/workpackage_test.go new file mode 100644 index 0000000..71ad06e --- /dev/null +++ b/cmd/workpackage/workpackage_test.go @@ -0,0 +1,15 @@ +package workpackage_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/cmd/workpackage" +) + +func TestRootCmd_UsesHyphenatedName(t *testing.T) { + use := workpackage.RootCmd.Use + if !strings.HasPrefix(use, "work-package") { + t.Errorf("expected command name 'work-package', got %q", use) + } +} diff --git a/cmd/list/types.go b/cmd/wptype/list.go similarity index 59% rename from cmd/list/types.go rename to cmd/wptype/list.go index e917354..ef4252b 100644 --- a/cmd/list/types.go +++ b/cmd/wptype/list.go @@ -1,22 +1,26 @@ -package list +package wptype import ( + "github.com/spf13/cobra" + + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/resources/types" - "github.com/spf13/cobra" ) -var typesCmd = &cobra.Command{ - Use: "types", +var listCmd = &cobra.Command{ + Use: "list", Short: "Lists work package types", Long: "Get a list of all work package types of the instance.", - Run: listTypes, + RunE: listTypes, } -func listTypes(_ *cobra.Command, _ []string) { +func listTypes(_ *cobra.Command, _ []string) error { if all, err := types.All(); err == nil { printer.Types(all) } else { printer.Error(err) + return openerrors.ErrHandled } + return nil } diff --git a/cmd/wptype/wptype.go b/cmd/wptype/wptype.go new file mode 100644 index 0000000..c643cbd --- /dev/null +++ b/cmd/wptype/wptype.go @@ -0,0 +1,13 @@ +package wptype + +import "github.com/spf13/cobra" + +var RootCmd = &cobra.Command{ + Use: "type [verb]", + Short: "Manage work package types", + Long: "List work package types in OpenProject.", +} + +func init() { + RootCmd.AddCommand(listCmd) +} diff --git a/components/configuration/filemode_test.go b/components/configuration/filemode_test.go new file mode 100644 index 0000000..7ebfa47 --- /dev/null +++ b/components/configuration/filemode_test.go @@ -0,0 +1,76 @@ +package configuration_test + +// Copilot review (PR #15): the config file stores API tokens, so it must not be +// world-readable. WriteConfigForProfile must create it with mode 0600. + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/opf/openproject-cli/components/configuration" +) + +func TestWriteConfigForProfile_FileModeIs0600(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://example.com", "secret-token"); err != nil { + t.Fatal(err) + } + + path := filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "openproject", "config") + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if mode := info.Mode().Perm(); mode != 0600 { + t.Errorf("config file mode = %04o, want 0600 (token must not be world-readable)", mode) + } +} + +// A config written by the CLI (mode 0600) must not be reported as insecure. +func TestInsecureConfigPermissions_SecureFile(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://example.com", "secret-token"); err != nil { + t.Fatal(err) + } + + if insecure, mode := configuration.InsecureConfigPermissions(); insecure { + t.Errorf("0600 config reported insecure (mode %#o)", mode) + } +} + +// A config readable by group or other users must be reported as insecure so the +// CLI can warn that the API token may leak. +func TestInsecureConfigPermissions_WorldReadableFile(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix permission bits are not meaningful on Windows") + } + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://example.com", "secret-token"); err != nil { + t.Fatal(err) + } + if err := os.Chmod(configuration.ConfigFilePath(), 0644); err != nil { + t.Fatal(err) + } + + insecure, mode := configuration.InsecureConfigPermissions() + if !insecure { + t.Errorf("0644 config not reported insecure (mode %#o)", mode) + } + if mode.Perm() != 0644 { + t.Errorf("reported mode = %#o, want 0644", mode.Perm()) + } +} + +// A missing config file is not insecure. +func TestInsecureConfigPermissions_MissingFile(t *testing.T) { + setupTempConfig(t) + + if insecure, _ := configuration.InsecureConfigPermissions(); insecure { + t.Error("missing config file reported insecure") + } +} diff --git a/components/configuration/findings_marshal_test.go b/components/configuration/findings_marshal_test.go new file mode 100644 index 0000000..b5e4a88 --- /dev/null +++ b/components/configuration/findings_marshal_test.go @@ -0,0 +1,41 @@ +package configuration_test + +// Regression test for review finding #3 on PR #15: rewriting the config must +// preserve keys other than host/token. Reuses setupTempConfig / writeRaw from +// profiles_test.go (same package). + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/opf/openproject-cli/components/configuration" +) + +// readRawConfig returns the raw bytes of the config file the package writes to. +func readRawConfig(t *testing.T) string { + t.Helper() + path := filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "openproject", "config") + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading raw config: %v", err) + } + return string(data) +} + +func TestFinding_MarshalPreservesUnknownKeys(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "[default]\nhost = https://example.com\ntoken = tok\nextra = keepme\n") + + // Trigger a read-modify-write cycle on a different profile so the default + // section is preserved-and-rewritten rather than edited. + if err := configuration.WriteConfigForProfile("work", "https://work.example.com", "tok-work"); err != nil { + t.Fatal(err) + } + + raw := readRawConfig(t) + if !strings.Contains(raw, "extra = keepme") { + t.Errorf("expected unknown key 'extra = keepme' to survive rewrite, file was:\n%s", raw) + } +} diff --git a/components/configuration/findings_test.go b/components/configuration/findings_test.go new file mode 100644 index 0000000..f8ae0cf --- /dev/null +++ b/components/configuration/findings_test.go @@ -0,0 +1,104 @@ +package configuration_test + +// Regression tests for review findings #1 and #2 on PR #15: a corrupt or +// malformed config file must be reported as an error rather than silently +// yielding empty (or bogus migrated) credentials. They reuse setupTempConfig / +// writeRaw from profiles_test.go (same package). + +import ( + "testing" + + "github.com/opf/openproject-cli/components/configuration" +) + +// Finding #1: a corrupt config file must now be reported as an error rather than +// silently yielding empty (or, for spaced garbage, bogus migrated) credentials. +func TestFinding_CorruptConfigReportsError(t *testing.T) { + corrupt := []struct { + name string + content string + }{ + {"garbage with spaces", "%%% not ini and not host token %%%"}, + {"garbage no spaces", "@@@garbage@@@"}, + {"malformed ini section", "[default\nhost broken"}, + {"host only, no token", "https://example.com"}, + } + + for _, c := range corrupt { + t.Run(c.name, func(t *testing.T) { + setupTempConfig(t) + writeRaw(t, c.content) + + host, token, err := configuration.ReadConfig("default") + if err == nil { + t.Errorf("expected an error for corrupt config %q, got host=%q token=%q", c.content, host, token) + } + }) + } +} + +// Finding #2: a malformed old-format file (single field, no usable host) is now +// surfaced as an error instead of a silent logout. +func TestFinding_MalformedOldFormatReportsError(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "justatokenwithnospace") + + if _, _, err := configuration.ReadConfig("default"); err == nil { + t.Error("expected an error for malformed old-format config, got nil") + } +} + +// Finding #1 (follow-up): an INI section that parses but is missing host or +// token — e.g. because a malformed line was silently dropped — must be reported +// as invalid rather than handed back as empty credentials. +func TestFinding_SectionMissingCredentialReportsError(t *testing.T) { + cases := []struct { + name string + content string + }{ + {"malformed host line dropped", "[default]\nhost broken\ntoken = tok"}, + {"missing token", "[default]\nhost = https://example.com"}, + {"missing host", "[default]\ntoken = tok"}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + setupTempConfig(t) + writeRaw(t, c.content) + + if _, _, err := configuration.ReadConfig("default"); err == nil { + t.Errorf("expected an error for incomplete profile section %q, got nil", c.content) + } + }) + } +} + +// A section with usable host and token must still succeed even if it carries an +// extra unrecognised line — completeness of credentials is what matters, not +// line-by-line syntax pedantry. +func TestFinding_CompleteSectionWithStrayLineStillWorks(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "[default]\nhost = https://example.com\ntoken = tok\nstrayline\n") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatalf("complete section should read cleanly, got error: %v", err) + } + if host != "https://example.com" || token != "tok" { + t.Errorf("got host=%q token=%q", host, token) + } +} + +// A well-formed old file ("host token") must still migrate cleanly. +func TestFinding_WellFormedOldFormatStillMigrates(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "https://legacy.example.com legacytoken") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "https://legacy.example.com" || token != "legacytoken" { + t.Errorf("well-formed old file should migrate, got host=%q token=%q", host, token) + } +} diff --git a/components/configuration/hardening_test.go b/components/configuration/hardening_test.go new file mode 100644 index 0000000..0b11dc5 --- /dev/null +++ b/components/configuration/hardening_test.go @@ -0,0 +1,164 @@ +package configuration_test + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/opf/openproject-cli/components/configuration" + "github.com/opf/openproject-cli/components/printer" +) + +func configPath() string { + return filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "openproject", "config") +} + +// Migration used to rewrite the config in place, which kept a permissive +// pre-existing mode (e.g. 0644) on a file that stores API tokens. The +// atomic rewrite must leave the migrated file with mode 0600. +func TestMigration_resetsInsecureFileMode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix permission bits are not meaningful on windows") + } + setupTempConfig(t) + writeRaw(t, "https://legacy.example.com legacytoken") + + if _, _, err := configuration.ReadConfig("default"); err != nil { + t.Fatal(err) + } + + info, err := os.Stat(configPath()) + if err != nil { + t.Fatal(err) + } + if mode := info.Mode().Perm(); mode != 0600 { + t.Errorf("migrated config mode = %#o, want 0600", mode) + } +} + +// A readable but non-writable legacy config must still yield its credentials; +// persisting the migrated format is best effort. +func TestMigration_readOnlyConfigStillReadable(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("directory write permissions are not meaningful on windows") + } + if os.Geteuid() == 0 { + t.Skip("running as root bypasses permission checks") + } + printer.Init(&printer.TestingPrinter{}) + setupTempConfig(t) + writeRaw(t, "https://legacy.example.com legacytoken") + + dir := filepath.Dir(configPath()) + if err := os.Chmod(dir, 0500); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(dir, 0700) }) + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatalf("ReadConfig on read-only legacy config: %v", err) + } + if host != "https://legacy.example.com" || token != "legacytoken" { + t.Errorf("got host %q token %q", host, token) + } +} + +// Hand-edited files can contain duplicate sections; logout must remove all of +// them, or an earlier section's token silently stays active. +func TestDeleteProfile_removesDuplicateSections(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "[work]\nhost = https://a.example.com\ntoken = tok-a\n\n[work]\nhost = https://b.example.com\ntoken = tok-b\n") + + if err := configuration.DeleteProfile("work"); err != nil { + t.Fatal(err) + } + + data, err := os.ReadFile(configPath()) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "[work]") { + t.Errorf("config still contains a [work] section after delete:\n%s", data) + } +} + +// A host that is not an absolute URL must be rejected on read with a clear +// message instead of surfacing later as a confusing request failure. +func TestReadConfig_rejectsInvalidHost(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "[default]\nhost = garbage\ntoken = tok\n") + + _, _, err := configuration.ReadConfig("default") + if err == nil { + t.Fatal("expected error for non-URL host") + } + if !strings.Contains(err.Error(), "invalid host") { + t.Errorf("error should mention invalid host, got: %v", err) + } +} + +func TestReadConfig_EnvironmentDoesNotRequireConfigDirectory(t *testing.T) { + configRoot := filepath.Join(t.TempDir(), "not-a-directory") + if err := os.WriteFile(configRoot, []byte("occupied"), 0600); err != nil { + t.Fatal(err) + } + t.Setenv("XDG_CONFIG_HOME", configRoot) + t.Setenv("OP_CLI_HOST", "https://env.example.com") + t.Setenv("OP_CLI_TOKEN", "env-token") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatalf("ReadConfig with environment credentials: %v", err) + } + if host != "https://env.example.com" || token != "env-token" { + t.Errorf("got host %q token %q", host, token) + } +} + +func TestReadConfig_rejectsUnsupportedEnvironmentScheme(t *testing.T) { + setupTempConfig(t) + t.Setenv("OP_CLI_HOST", "ftp://env.example.com") + t.Setenv("OP_CLI_TOKEN", "env-token") + + _, _, err := configuration.ReadConfig("default") + if err == nil { + t.Fatal("expected error for unsupported environment host scheme") + } + if !strings.Contains(err.Error(), "only http and https") { + t.Errorf("error should describe supported schemes, got: %v", err) + } + if !strings.Contains(err.Error(), "OP_CLI_HOST") { + t.Errorf("error should name OP_CLI_HOST as the source, got: %v", err) + } +} + +// Scheme comparison must be case-insensitive: URL schemes are defined as +// case-insensitive, and hosts pasted from other tools may be upper-cased. +func TestReadConfig_acceptsMixedCaseScheme(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "[default]\nhost = HTTPS://file.example.com\ntoken = file-token\n") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatalf("ReadConfig with mixed-case scheme: %v", err) + } + if host != "HTTPS://file.example.com" || token != "file-token" { + t.Errorf("got host %q token %q", host, token) + } +} + +func TestReadConfig_rejectsUnsupportedProfileScheme(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "[default]\nhost = ftp://file.example.com\ntoken = file-token\n") + + _, _, err := configuration.ReadConfig("default") + if err == nil { + t.Fatal("expected error for unsupported profile host scheme") + } + if !strings.Contains(err.Error(), "only http and https") { + t.Errorf("error should describe supported schemes, got: %v", err) + } +} diff --git a/components/configuration/profiles.go b/components/configuration/profiles.go new file mode 100644 index 0000000..4e3b968 --- /dev/null +++ b/components/configuration/profiles.go @@ -0,0 +1,439 @@ +package configuration + +import ( + "fmt" + "net/url" + "os" + "path/filepath" + "regexp" + "runtime" + "sort" + "strings" + + "github.com/opf/openproject-cli/components/common" + "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" +) + +const ( + EnvProfile = "OP_CLI_PROFILE" + DefaultProfile = "default" +) + +// Profile holds credentials for a named OpenProject instance. +type Profile struct { + Name string + Host string + Token string +} + +var invalidChars = regexp.MustCompile(`[^a-zA-Z0-9_]`) +var multiHyphen = regexp.MustCompile(`-{2,}`) + +// SanitizeProfileName replaces invalid characters with hyphens, collapses +// consecutive hyphens, strips leading/trailing hyphens, and falls back to +// "default" when the result is empty. +func SanitizeProfileName(name string) string { + result := invalidChars.ReplaceAllString(name, "-") + result = multiHyphen.ReplaceAllString(result, "-") + result = strings.Trim(result, "-") + if result == "" { + return DefaultProfile + } + return result +} + +// ValidateProfileName returns an error when name is not already in sanitized +// form (only letters, digits, - and _, no leading/trailing hyphens, non-empty). +func ValidateProfileName(name string) error { + if name == "" { + return errors.Custom("profile name cannot be empty") + } + if SanitizeProfileName(name) != name { + return errors.Custom(fmt.Sprintf( + "invalid profile name %q: only letters, numbers, - and _ are allowed (no leading/trailing hyphens)", + name, + )) + } + return nil +} + +// ReadConfig returns the host and token for profile. Environment credentials +// take precedence and do not require the config directory to be accessible. +func ReadConfig(profile string) (host, token string, err error) { + if ok, h, t := readEnvironment(); ok { + if err := validateHost(h); err != nil { + return "", "", errors.Custom(fmt.Sprintf( + "environment variable OP_CLI_HOST has an invalid host %q: %v", + h, err, + )) + } + return h, t, nil + } + if err = ensureConfigDir(); err != nil { + return "", "", err + } + return readConfigForProfile(profile) +} + +// WriteConfigForProfile writes or updates profile in the config file. +func WriteConfigForProfile(profile, host, token string) error { + if err := ensureConfigDir(); err != nil { + return err + } + return writeProfile(profile, host, token) +} + +// DeleteProfile removes profile from the config file, including any +// duplicate sections with the same name. It returns an error when the +// profile does not exist, so callers do not report success for a no-op. +func DeleteProfile(profile string) error { + if err := ensureConfigDir(); err != nil { + return err + } + return deleteProfile(profile) +} + +// ConfigFilePath returns the path of the config file. It honours +// $XDG_CONFIG_HOME/$HOME, so it is absolute only when those are. Exposed so +// callers can name the file in user-facing messages. +func ConfigFilePath() string { + return configFile() +} + +// InsecureConfigPermissions reports whether the config file exists with +// permissions that let group or other users access it. The file stores API +// tokens (and is written with mode 0600), so callers should warn the user when +// this returns true. The second value is the file's permission bits. Unix +// permission bits are not meaningful on Windows, so it always reports false +// there; a missing or unreadable file is likewise not reported as insecure. +func InsecureConfigPermissions() (insecure bool, mode os.FileMode) { + if runtime.GOOS == "windows" { + return false, 0 + } + info, err := os.Stat(configFile()) + if err != nil { + return false, 0 + } + mode = info.Mode().Perm() + return mode&0077 != 0, mode +} + +// AllProfiles returns every profile stored in the config file. +func AllProfiles() ([]*Profile, error) { + if err := ensureConfigDir(); err != nil { + return nil, err + } + return readAllProfiles() +} + +// --- internal helpers -------------------------------------------------------- + +type iniSection struct { + name string + kv map[string]string +} + +type iniFile struct { + sections []*iniSection + index map[string]int +} + +func newIniFile() *iniFile { + return &iniFile{index: make(map[string]int)} +} + +func parseIni(data []byte) *iniFile { + f := newIniFile() + var current *iniSection + + for _, raw := range strings.Split(string(data), "\n") { + line := strings.TrimSpace(raw) + if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, ";") { + continue + } + if strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") { + name := line[1 : len(line)-1] + s := &iniSection{name: name, kv: make(map[string]string)} + f.index[name] = len(f.sections) + f.sections = append(f.sections, s) + current = s + continue + } + if current != nil { + if idx := strings.Index(line, "="); idx > 0 { + key := strings.TrimSpace(line[:idx]) + val := strings.TrimSpace(line[idx+1:]) + current.kv[key] = val + } + } + } + return f +} + +func (f *iniFile) get(section, key string) (string, bool) { + idx, ok := f.index[section] + if !ok { + return "", false + } + v, ok := f.sections[idx].kv[key] + return v, ok +} + +func (f *iniFile) hasSection(section string) bool { + _, ok := f.index[section] + return ok +} + +func (f *iniFile) set(section, key, val string) { + idx, ok := f.index[section] + if !ok { + s := &iniSection{name: section, kv: make(map[string]string)} + f.index[section] = len(f.sections) + f.sections = append(f.sections, s) + idx = len(f.sections) - 1 + } + f.sections[idx].kv[key] = val +} + +// delete removes every section named section (duplicate sections can exist in +// hand-edited files; leaving one behind would silently keep its credentials +// active). Reports whether anything was removed. +func (f *iniFile) delete(section string) bool { + kept := f.sections[:0] + removed := false + for _, s := range f.sections { + if s.name == section { + removed = true + continue + } + kept = append(kept, s) + } + if !removed { + return false + } + f.sections = kept + f.index = make(map[string]int, len(f.sections)) + for i, s := range f.sections { + f.index[s.name] = i + } + return true +} + +func (f *iniFile) marshal() []byte { + var sb strings.Builder + for i, s := range f.sections { + if i > 0 { + sb.WriteString("\n") + } + sb.WriteString(fmt.Sprintf("[%s]\n", s.name)) + // host and token first for stable, readable ordering. + written := make(map[string]bool) + for _, key := range []string{"host", "token"} { + if v, ok := s.kv[key]; ok { + sb.WriteString(fmt.Sprintf("%s = %s\n", key, v)) + written[key] = true + } + } + // Preserve any other keys (sorted for deterministic output) so data the + // CLI does not recognise is not silently dropped on rewrite. + rest := make([]string, 0, len(s.kv)) + for k := range s.kv { + if !written[k] { + rest = append(rest, k) + } + } + sort.Strings(rest) + for _, k := range rest { + sb.WriteString(fmt.Sprintf("%s = %s\n", k, s.kv[k])) + } + } + return []byte(sb.String()) +} + +// looksLikeHost reports whether s is an absolute HTTP(S) URL. It distinguishes +// the legacy "host token" format from corrupt configuration content. +func looksLikeHost(s string) bool { + return validateHost(s) == nil +} + +// validateHost rejects relative URLs and schemes the HTTP client cannot use. +func validateHost(s string) error { + u, err := url.Parse(s) + if err != nil { + return fmt.Errorf("invalid host URL: %w", err) + } + if u.Scheme == "" || u.Host == "" { + return fmt.Errorf("expected an absolute URL like https://example.openproject.com") + } + scheme := strings.ToLower(u.Scheme) + if scheme != "http" && scheme != "https" { + return fmt.Errorf("unsupported protocol scheme %q: only http and https are allowed", u.Scheme) + } + return nil +} + +// readOrMigrate reads the config file and migrates it from the old +// single-line "host token" format when needed. Returns (ini, migrated, error). +// A non-empty file that is neither valid INI nor a well-formed old-format line +// is reported as corrupt rather than silently yielding empty/bogus credentials. +func readOrMigrate(data []byte) (*iniFile, bool, error) { + content := strings.TrimSpace(string(data)) + if content == "" { + return newIniFile(), false, nil + } + + // Detect INI by parsing rather than a raw prefix check: parseIni skips + // comment and blank lines, so a valid INI file that opens with a comment + // still yields section headers. Any section present means INI format. + if f := parseIni(data); len(f.sections) > 0 { + return f, false, nil + } + + // Old format: no section headers, so treat the content as a single + // "host token" line. Require the first field to look like a real host URL, + // so a corrupt file is not migrated into bogus credentials just because it + // happens to contain a space. looksLikeHost accepts IPv6 host URLs + // (e.g. http://[::1]). + clean := common.SanitizeLineBreaks(content) + parts := strings.SplitN(clean, " ", 2) + if len(parts) == 2 && looksLikeHost(parts[0]) && parts[1] != "" { + f := newIniFile() + f.set(DefaultProfile, "host", parts[0]) + f.set(DefaultProfile, "token", parts[1]) + return f, true, nil + } + return nil, false, invalidConfigError() +} + +func invalidConfigError() error { + return errors.Custom(fmt.Sprintf( + "invalid config file at %s. Please remove the file and run `op login` again.", + configFile(), + )) +} + +// writeConfigFile atomically replaces the config file: it writes a temporary +// file in the same directory (created with mode 0600) and renames it over the +// target. An interrupted write cannot leave a truncated config, and a +// permissive mode on the old file (e.g. 0644 from earlier CLI versions) is +// not carried over. +func writeConfigFile(data []byte) error { + target := configFile() + tmp, err := os.CreateTemp(filepath.Dir(target), "config-*.tmp") + if err != nil { + return err + } + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + _ = os.Remove(tmp.Name()) + return err + } + if err := tmp.Close(); err != nil { + _ = os.Remove(tmp.Name()) + return err + } + if err := os.Rename(tmp.Name(), target); err != nil { + _ = os.Remove(tmp.Name()) + return err + } + return nil +} + +func readOrMigrateFile() (*iniFile, error) { + data, err := os.ReadFile(configFile()) + if os.IsNotExist(err) { + return newIniFile(), nil + } + if err != nil { + return nil, err + } + + f, migrated, err := readOrMigrate(data) + if err != nil { + return nil, err + } + if migrated { + // Best effort: a config that cannot be rewritten (e.g. read-only + // location) must not block reading the credentials it holds. + if err := writeConfigFile(f.marshal()); err != nil { + printer.Warning(fmt.Sprintf( + "could not rewrite config file %s in the new format: %s", configFile(), err, + )) + } + } + return f, nil +} + +func readConfigForProfile(profile string) (host, token string, err error) { + f, err := readOrMigrateFile() + if err != nil { + return "", "", err + } + host, _ = f.get(profile, "host") + token, _ = f.get(profile, "token") + // An absent profile is a normal "not logged in" state. But a section that + // exists yet is missing host or token is corrupt (e.g. a malformed line was + // dropped during parsing) and must be reported rather than handed back as + // empty credentials. + if f.hasSection(profile) && (host == "" || token == "") { + return "", "", invalidConfigError() + } + if host != "" { + if err := validateHost(host); err != nil { + return "", "", errors.Custom(fmt.Sprintf( + "profile %q has an invalid host %q: %v", + profile, host, err, + )) + } + } + return host, token, nil +} + +func readAllProfiles() ([]*Profile, error) { + f, err := readOrMigrateFile() + if err != nil { + return nil, err + } + profiles := make([]*Profile, 0, len(f.sections)) + for _, s := range f.sections { + profiles = append(profiles, &Profile{ + Name: s.name, + Host: s.kv["host"], + Token: s.kv["token"], + }) + } + return profiles, nil +} + +func writeProfile(profile, host, token string) error { + f, err := readOrMigrateFile() + if err != nil { + return err + } + f.set(profile, "host", host) + f.set(profile, "token", token) + return writeConfigFile(f.marshal()) +} + +func profileNotFoundError(profile string) error { + return errors.Custom(fmt.Sprintf("profile %q not found", profile)) +} + +func deleteProfile(profile string) error { + data, err := os.ReadFile(configFile()) + if os.IsNotExist(err) { + return profileNotFoundError(profile) + } + if err != nil { + return err + } + f, _, err := readOrMigrate(data) + if err != nil { + return err + } + if !f.delete(profile) { + return profileNotFoundError(profile) + } + return writeConfigFile(f.marshal()) +} diff --git a/components/configuration/profiles_test.go b/components/configuration/profiles_test.go new file mode 100644 index 0000000..ed151f9 --- /dev/null +++ b/components/configuration/profiles_test.go @@ -0,0 +1,403 @@ +package configuration_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/opf/openproject-cli/components/configuration" +) + +// setupTempConfig points XDG_CONFIG_HOME at a temp dir and unsets credential +// env vars so every test starts from a clean slate. +func setupTempConfig(t *testing.T) { + t.Helper() + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + unsetEnv(t, "OP_CLI_HOST") + unsetEnv(t, "OP_CLI_TOKEN") +} + +// unsetEnv removes an env var for the duration of the test and restores the +// original value (or removes it again) in cleanup. +func unsetEnv(t *testing.T, key string) { + t.Helper() + old, existed := os.LookupEnv(key) + os.Unsetenv(key) + t.Cleanup(func() { + if existed { + os.Setenv(key, old) + } else { + os.Unsetenv(key) + } + }) +} + +// writeRaw writes arbitrary bytes directly to the config file, bypassing the +// profile API – used to seed old-format files for migration tests. +func writeRaw(t *testing.T, content string) { + t.Helper() + dir := filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "openproject") + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "config"), []byte(content), 0644); err != nil { + t.Fatal(err) + } +} + +// ---------- SanitizeProfileName ---------- + +func TestSanitizeProfileName(t *testing.T) { + cases := []struct { + input string + want string + }{ + {"default", "default"}, + {"work", "work"}, + {"my-profile", "my-profile"}, + {"my_profile", "my_profile"}, + {"MyProfile123", "MyProfile123"}, + {"my profile", "my-profile"}, + {"my@work", "my-work"}, + {"my--work", "my-work"}, + {"-mywork-", "mywork"}, + {"--", "default"}, + {"!@#$%", "default"}, + {"", "default"}, + } + for _, c := range cases { + got := configuration.SanitizeProfileName(c.input) + if got != c.want { + t.Errorf("SanitizeProfileName(%q) = %q, want %q", c.input, got, c.want) + } + } +} + +// ---------- ValidateProfileName ---------- + +func TestValidateProfileName_valid(t *testing.T) { + valid := []string{ + "default", "work", "my-profile", "my_profile", + "MyProfile123", "a", "1", "a1", "a-b", "a_b", + } + for _, name := range valid { + if err := configuration.ValidateProfileName(name); err != nil { + t.Errorf("ValidateProfileName(%q) should be valid, got: %v", name, err) + } + } +} + +func TestValidateProfileName_invalid(t *testing.T) { + invalid := []string{ + "", + "my profile", + "my@work", + "!@#", + "-leading", + "trailing-", + "my--work", + "--", + } + for _, name := range invalid { + if err := configuration.ValidateProfileName(name); err == nil { + t.Errorf("ValidateProfileName(%q) should be invalid, got no error", name) + } + } +} + +// ---------- WriteConfigForProfile / ReadConfig ---------- + +func TestWriteAndReadConfigForProfile(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://example.com", "token123"); err != nil { + t.Fatal(err) + } + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "https://example.com" { + t.Errorf("host = %q, want %q", host, "https://example.com") + } + if token != "token123" { + t.Errorf("token = %q, want %q", token, "token123") + } +} + +func TestWriteMultipleProfilesAndReadBack(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://default.example.com", "tok-default"); err != nil { + t.Fatal(err) + } + if err := configuration.WriteConfigForProfile("work", "https://work.example.com", "tok-work"); err != nil { + t.Fatal(err) + } + + host, token, err := configuration.ReadConfig("work") + if err != nil { + t.Fatal(err) + } + if host != "https://work.example.com" { + t.Errorf("host = %q, want %q", host, "https://work.example.com") + } + if token != "tok-work" { + t.Errorf("token = %q, want %q", token, "tok-work") + } + + host, token, err = configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "https://default.example.com" { + t.Errorf("host = %q, want %q", host, "https://default.example.com") + } + if token != "tok-default" { + t.Errorf("token = %q, want %q", token, "tok-default") + } +} + +func TestReadConfig_missingProfile_returnsEmpty(t *testing.T) { + setupTempConfig(t) + + // Write one profile but read a different one + if err := configuration.WriteConfigForProfile("default", "https://example.com", "tok"); err != nil { + t.Fatal(err) + } + + host, token, err := configuration.ReadConfig("nonexistent") + if err != nil { + t.Fatal(err) + } + if host != "" || token != "" { + t.Errorf("expected empty credentials for missing profile, got host=%q token=%q", host, token) + } +} + +func TestReadConfig_noFile_returnsEmpty(t *testing.T) { + setupTempConfig(t) + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "" || token != "" { + t.Errorf("expected empty credentials when no config file, got host=%q token=%q", host, token) + } +} + +func TestReadConfig_envVarsOverrideProfile(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://file.example.com", "file-token"); err != nil { + t.Fatal(err) + } + + t.Setenv("OP_CLI_HOST", "https://env.example.com") + t.Setenv("OP_CLI_TOKEN", "env-token") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "https://env.example.com" { + t.Errorf("host = %q, want env var value", host) + } + if token != "env-token" { + t.Errorf("token = %q, want env var value", token) + } +} + +func TestWriteConfigForProfile_overwritesExisting(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("work", "https://old.example.com", "old-token"); err != nil { + t.Fatal(err) + } + if err := configuration.WriteConfigForProfile("work", "https://new.example.com", "new-token"); err != nil { + t.Fatal(err) + } + + host, token, err := configuration.ReadConfig("work") + if err != nil { + t.Fatal(err) + } + if host != "https://new.example.com" { + t.Errorf("host = %q, want %q", host, "https://new.example.com") + } + if token != "new-token" { + t.Errorf("token = %q, want %q", token, "new-token") + } +} + +// ---------- AllProfiles ---------- + +func TestAllProfiles(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://a.example.com", "tok-a"); err != nil { + t.Fatal(err) + } + if err := configuration.WriteConfigForProfile("work", "https://b.example.com", "tok-b"); err != nil { + t.Fatal(err) + } + + profiles, err := configuration.AllProfiles() + if err != nil { + t.Fatal(err) + } + if len(profiles) != 2 { + t.Fatalf("expected 2 profiles, got %d", len(profiles)) + } + + byName := make(map[string]*configuration.Profile) + for _, p := range profiles { + byName[p.Name] = p + } + + if p, ok := byName["default"]; !ok { + t.Error("missing 'default' profile") + } else { + if p.Host != "https://a.example.com" { + t.Errorf("default host = %q", p.Host) + } + } + if p, ok := byName["work"]; !ok { + t.Error("missing 'work' profile") + } else { + if p.Host != "https://b.example.com" { + t.Errorf("work host = %q", p.Host) + } + } +} + +func TestAllProfiles_noFile_returnsEmpty(t *testing.T) { + setupTempConfig(t) + + profiles, err := configuration.AllProfiles() + if err != nil { + t.Fatal(err) + } + if len(profiles) != 0 { + t.Errorf("expected 0 profiles, got %d", len(profiles)) + } +} + +// ---------- DeleteProfile ---------- + +func TestDeleteProfile_removesProfile(t *testing.T) { + setupTempConfig(t) + + if err := configuration.WriteConfigForProfile("default", "https://a.example.com", "tok-a"); err != nil { + t.Fatal(err) + } + if err := configuration.WriteConfigForProfile("work", "https://b.example.com", "tok-b"); err != nil { + t.Fatal(err) + } + + if err := configuration.DeleteProfile("default"); err != nil { + t.Fatal(err) + } + + profiles, err := configuration.AllProfiles() + if err != nil { + t.Fatal(err) + } + if len(profiles) != 1 { + t.Fatalf("expected 1 profile after delete, got %d", len(profiles)) + } + if profiles[0].Name != "work" { + t.Errorf("remaining profile name = %q, want %q", profiles[0].Name, "work") + } +} + +func TestDeleteProfile_missingProfileErrors(t *testing.T) { + setupTempConfig(t) + + // Delete with no config file at all must report the profile as missing, + // so logout cannot claim success for a no-op. + if err := configuration.DeleteProfile("default"); err == nil { + t.Error("DeleteProfile with no config file should error") + } + + if err := configuration.WriteConfigForProfile("default", "https://a.example.com", "tok-a"); err != nil { + t.Fatal(err) + } + if err := configuration.DeleteProfile("nonexistent"); err == nil { + t.Error("DeleteProfile on missing profile should error") + } +} + +// ---------- Migration ---------- + +func TestMigration_oldFormatMigratedToDefault(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "https://legacy.example.com legacytoken") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "https://legacy.example.com" { + t.Errorf("host = %q, want %q", host, "https://legacy.example.com") + } + if token != "legacytoken" { + t.Errorf("token = %q, want %q", token, "legacytoken") + } +} + +func TestMigration_IPv6HostMigratedCorrectly(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "http://[::1]:8080 mytoken") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatal(err) + } + if host != "http://[::1]:8080" { + t.Errorf("host = %q, want %q", host, "http://[::1]:8080") + } + if token != "mytoken" { + t.Errorf("token = %q, want %q", token, "mytoken") + } +} + +func TestMigration_oldFormatRewrittenAsIni(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "https://legacy.example.com legacytoken") + + // Trigger migration by reading + if _, _, err := configuration.ReadConfig("default"); err != nil { + t.Fatal(err) + } + + // Now AllProfiles should work correctly + profiles, err := configuration.AllProfiles() + if err != nil { + t.Fatal(err) + } + if len(profiles) != 1 || profiles[0].Name != "default" { + t.Errorf("after migration: expected [default], got %v", profiles) + } +} + +// A valid INI file may open with a comment line. parseIni skips comments, so +// the reader must treat it as INI rather than mistaking it for the legacy +// "host token" format and rejecting it as corrupt. +func TestReadConfig_IniWithLeadingComment(t *testing.T) { + setupTempConfig(t) + writeRaw(t, "# my openproject config\n[default]\nhost = https://commented.example.com\ntoken = commenttoken\n") + + host, token, err := configuration.ReadConfig("default") + if err != nil { + t.Fatalf("ReadConfig returned error for commented INI: %v", err) + } + if host != "https://commented.example.com" { + t.Errorf("host = %q, want %q", host, "https://commented.example.com") + } + if token != "commenttoken" { + t.Errorf("token = %q, want %q", token, "commenttoken") + } +} diff --git a/components/configuration/util.go b/components/configuration/util.go index ea4f695..b68f815 100644 --- a/components/configuration/util.go +++ b/components/configuration/util.go @@ -1,13 +1,8 @@ package configuration import ( - "fmt" "os" "path/filepath" - "strings" - - "github.com/opf/openproject-cli/components/common" - "github.com/opf/openproject-cli/components/errors" ) const ( @@ -17,50 +12,20 @@ const ( configFileName = "config" ) -func WriteConfigFile(host, token string) error { - err := ensureConfigDir() - if err != nil { - return err - } - - bytes := []byte(fmt.Sprintf("%s %s", host, token)) - return os.WriteFile(configFile(), bytes, 0644) -} - -func ReadConfig() (host, token string, err error) { - err = ensureConfigDir() - if err != nil { - return "", "", err - } - - ok, h, t := readEnvironment() - if ok { - return h, t, nil - } - - file, err := os.ReadFile(configFile()) - if os.IsNotExist(err) { - // Empty config file is no error, - // user just has to run login command first - return "", "", nil - } - - parts := strings.Split(common.SanitizeLineBreaks(string(file)), " ") - if len(parts) != 2 { - return "", "", errors.Custom(fmt.Sprintf("Invalid config file at %s. Please remove the file and run `op login` again.", configFile())) - } - - return parts[0], parts[1], nil -} - func readEnvironment() (ok bool, host, token string) { host, hasHost := os.LookupEnv(envHost) token, hasToken := os.LookupEnv(envToken) ok = hasHost && hasToken - return } +// HasEnvironmentConfig reports whether OP_CLI_HOST and OP_CLI_TOKEN are both +// set, in which case they override every stored profile. +func HasEnvironmentConfig() bool { + ok, _, _ := readEnvironment() + return ok +} + func ensureConfigDir() error { if _, err := os.Stat(configFileDir()); os.IsNotExist(err) { err = os.MkdirAll(configFileDir(), 0700) @@ -68,7 +33,6 @@ func ensureConfigDir() error { return err } } - return nil } @@ -81,7 +45,6 @@ func configFileDir() string { if present { return filepath.Join(xdgConfigDir, configDirName) } - return filepath.Join(homeDir(), ".config", configDirName) } @@ -89,8 +52,5 @@ func homeDir() string { if home, ok := os.LookupEnv("HOME"); ok { return home } - - // On Windows `$HOME` is not set per default, but it is - // constructed from `$HOMEDRIVE` and `$HOMEPATH`. return filepath.Join(os.Getenv("HOMEDRIVE"), os.Getenv("HOMEPATH")) } diff --git a/components/errors/errors.go b/components/errors/errors.go index 982013c..fb3b5b6 100644 --- a/components/errors/errors.go +++ b/components/errors/errors.go @@ -1,5 +1,12 @@ package errors +import "errors" + +// ErrHandled signals that an error was already reported to the user via the +// printer. Commands return it from RunE so the process exits nonzero without +// printing the error a second time in main. +var ErrHandled = errors.New("error already reported") + type customError struct { text string } diff --git a/components/paths/paths.go b/components/paths/paths.go index 7561e84..70be27a 100644 --- a/components/paths/paths.go +++ b/components/paths/paths.go @@ -10,22 +10,34 @@ func Principals() string { return Root() + "/principals" } -func Project(id uint64) string { - return Projects() + fmt.Sprintf("/%d", id) +func Project(id string) string { + return Projects() + "/" + id } func Projects() string { return Root() + "/projects" } -func ProjectVersions(projectId uint64) string { +func ProjectVersions(projectId string) string { return Project(projectId) + "/versions" } -func ProjectWorkPackages(projectId uint64) string { +func ProjectWorkPackages(projectId string) string { return Project(projectId) + "/work_packages" } +func Budget(id uint64) string { + return Budgets() + fmt.Sprintf("/%d", id) +} + +func Budgets() string { + return Root() + "/budgets" +} + +func ProjectBudgets(projectId string) string { + return Project(projectId) + "/budgets" +} + func Root() string { return "/api/v3" } @@ -38,6 +50,14 @@ func TimeEntries() string { return Root() + "/time_entries" } +func TimeEntry(id uint64) string { + return TimeEntries() + fmt.Sprintf("/%d", id) +} + +func TimeEntryActivities() string { + return TimeEntries() + "/activities" +} + func Types() string { return Root() + "/types" } @@ -54,14 +74,14 @@ func Users() string { return Root() + "/users" } -func WorkPackage(id uint64) string { - return WorkPackages() + fmt.Sprintf("/%d", id) +func WorkPackage(id string) string { + return WorkPackages() + "/" + id } func WorkPackages() string { return Root() + "/work_packages" } -func WorkPackageActivities(id uint64) string { +func WorkPackageActivities(id string) string { return WorkPackage(id) + "/activities" } diff --git a/components/printer/activities.go b/components/printer/activities.go index e3f0320..b10f944 100644 --- a/components/printer/activities.go +++ b/components/printer/activities.go @@ -1,59 +1,44 @@ package printer import ( - "sort" + "cmp" + "slices" "strings" "github.com/opf/openproject-cli/models" ) func Activities(activities []*models.Activity, users []*models.User) { - for _, activity := range activities { - user := &models.User{ - Id: 0, - Name: "", - FirstName: "", - LastName: "", - } - if activity.UserId > 0 { - userIndex := sort.Search(len(users)-1, func(i int) bool { return users[i].Id == activity.UserId }) - user = users[userIndex] - } - printActivityHeadline(activity, user) - printActivityBody(activity) - println("") - } + // The renderers resolve actors with a binary search over user IDs; the + // API guarantees no response order, so sort a copy here. + sorted := slices.Clone(users) + slices.SortFunc(sorted, func(a, b *models.User) int { + return cmp.Compare(a.Id, b.Id) + }) + activeRenderer.Activities(activities, sorted) } func printActivityHeadline(activity *models.Activity, user *models.User) { var parts []string - if len(user.Name) > 0 { parts = append(parts, Green(user.Name)) } - parts = append(parts, Yellow(activity.UpdatedAt)) - activePrinter.Println(strings.Join(parts, " ")) } func printActivityBody(activity *models.Activity) { var parts []string - if len(activity.Comment) > 0 { parts = append(parts, Yellow(activity.Comment)) - if len(activity.Details) > 0 { parts = append(parts, "---") } } - var detailsParts []string for _, detail := range activity.Details { detailsParts = append(detailsParts, *detail) } - parts = append(parts, strings.Join(detailsParts, "\n")) - activePrinter.Println(strings.Join(parts, "\n \n")) } diff --git a/components/printer/activities_test.go b/components/printer/activities_test.go new file mode 100644 index 0000000..a7a6b7d --- /dev/null +++ b/components/printer/activities_test.go @@ -0,0 +1,35 @@ +package printer_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/models" +) + +// TestActivities_UserLookup_FirstUser reproduces the sort.Search bug: +// the old predicate (Id == target) is not monotone, and len(users)-1 as the +// upper bound means the last user is never searched. Together they cause +// sort.Search to return the wrong index when the target is the first user. +func TestActivities_UserLookup_FirstUser(t *testing.T) { + testingPrinter.Reset() + printer.InitRenderer("text") + + users := []*models.User{ + {Id: 1, Name: "Alice"}, + {Id: 3, Name: "Bob"}, + {Id: 5, Name: "Charlie"}, + } + activity := &models.Activity{ + Id: 42, + UserId: 1, // Alice — first in the slice + UpdatedAt: "2024-01-01", + } + + printer.Activities([]*models.Activity{activity}, users) + + if !strings.Contains(testingPrinter.Result, printer.Green("Alice")) { + t.Errorf("expected user 'Alice' in output, got: %q", testingPrinter.Result) + } +} diff --git a/components/printer/budgets.go b/components/printer/budgets.go new file mode 100644 index 0000000..83bd8b7 --- /dev/null +++ b/components/printer/budgets.go @@ -0,0 +1,11 @@ +package printer + +import "github.com/opf/openproject-cli/models" + +func Budget(budget *models.Budget) { + activeRenderer.Budget(budget) +} + +func Budgets(budgets []*models.Budget) { + activeRenderer.Budgets(budgets) +} diff --git a/components/printer/budgets_test.go b/components/printer/budgets_test.go new file mode 100644 index 0000000..00df42e --- /dev/null +++ b/components/printer/budgets_test.go @@ -0,0 +1,47 @@ +package printer_test + +import ( + "fmt" + "strconv" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/models" +) + +func TestBudget(t *testing.T) { + testingPrinter.Reset() + + budget := models.Budget{Id: 7, Subject: "Conception"} + + idString := "#" + strconv.FormatUint(budget.Id, 10) + expected := fmt.Sprintf("%s %s\n", printer.Red(idString), printer.Cyan(budget.Subject)) + + printer.Budget(&budget) + + if testingPrinter.Result != expected { + t.Errorf("Expected %s, but got %s", expected, testingPrinter.Result) + } +} + +func TestBudgets_AlignsIds(t *testing.T) { + testingPrinter.Reset() + + budgets := []*models.Budget{ + {Id: 7, Subject: "Conception"}, + {Id: 42, Subject: "Développement"}, + {Id: 100, Subject: "Recette & déploiement"}, + } + + // IDs are right-aligned to the width of the longest ID (3 digits) + expected := "" + + fmt.Sprintf("%s %s\n", printer.Red(" #7"), printer.Cyan("Conception")) + + fmt.Sprintf("%s %s\n", printer.Red(" #42"), printer.Cyan("Développement")) + + fmt.Sprintf("%s %s\n", printer.Red("#100"), printer.Cyan("Recette & déploiement")) + + printer.Budgets(budgets) + + if testingPrinter.Result != expected { + t.Errorf("Expected %s, but got %s", expected, testingPrinter.Result) + } +} diff --git a/components/printer/common.go b/components/printer/common.go index 7b3b33f..2b66b94 100644 --- a/components/printer/common.go +++ b/components/printer/common.go @@ -2,6 +2,7 @@ package printer import ( "encoding/json" + "fmt" "github.com/opf/openproject-cli/components/errors" ) @@ -12,16 +13,29 @@ type apiErrorModel struct { Messsage string `json:"message,omitempty"` } -func Info(msg string) { +// Output writes requested data to standard out. Use it (or a renderer) for +// command results only; diagnostics belong on standard error. +func Output(msg string) { activePrinter.Println(msg) } +// Info writes a progress or context message to standard error. +func Info(msg string) { + activePrinter.Eprintln(msg) +} + func Input(prompt string) { - activePrinter.Printf(prompt) + activePrinter.Eprintf(prompt) +} + +// Warning writes a non-fatal diagnostic to standard error so it never corrupts +// machine-readable output (e.g. JSON) written to standard out. +func Warning(msg string) { + activePrinter.Eprintln(fmt.Sprintf("%s %s", Yellow("[WARNING]"), msg)) } func Done() { - activePrinter.Println(Green("DONE")) + activePrinter.Eprintln(Green("DONE")) } func Error(err error) { @@ -30,18 +44,18 @@ func Error(err error) { err := err.(*errors.ResponseError) responseError(err.Status(), err.Response()) default: - activePrinter.Printf("%s Program exited with error: %+v\n", Red("[ERROR]"), err) + activePrinter.Eprintf("%s Program exited with error: %+v\n", Red("[ERROR]"), err) } } func Debug(verboseFlag bool, msg string) { if verboseFlag { - activePrinter.Printf("%s %s\n", Magenta("[DEBUG]"), msg) + activePrinter.Eprintf("%s %s\n", Magenta("[DEBUG]"), msg) } } func ErrorText(msg string) { - activePrinter.Printf("%s %s\n", Red("[ERROR]"), msg) + activePrinter.Eprintf("%s %s\n", Red("[ERROR]"), msg) } func responseError(status int, body []byte) { @@ -58,7 +72,7 @@ func responseError(status int, body []byte) { bodyRepresentation = string(body) } - activePrinter.Printf( + activePrinter.Eprintf( "%s Bad response from server: (%d)\n\n%s\n", Red("[ERROR]"), status, @@ -67,7 +81,7 @@ func responseError(status int, body []byte) { } func apiError(status int, err apiErrorModel) { - activePrinter.Printf( + activePrinter.Eprintf( "%s API request failure (%d): %s\n%s\n", Red("[ERROR]"), status, diff --git a/components/printer/console_printer.go b/components/printer/console_printer.go index a60002d..cafe1d8 100644 --- a/components/printer/console_printer.go +++ b/components/printer/console_printer.go @@ -1,6 +1,9 @@ package printer -import "fmt" +import ( + "fmt" + "os" +) type ConsolePrinter struct{} @@ -11,3 +14,11 @@ func (printer *ConsolePrinter) Printf(format string, a ...any) (n int, err error func (printer *ConsolePrinter) Println(a ...any) (n int, err error) { return fmt.Println(a...) } + +func (printer *ConsolePrinter) Eprintln(a ...any) (n int, err error) { + return fmt.Fprintln(os.Stderr, a...) +} + +func (printer *ConsolePrinter) Eprintf(format string, a ...any) (n int, err error) { + return fmt.Fprintf(os.Stderr, format, a...) +} diff --git a/components/printer/custom_actions.go b/components/printer/custom_actions.go index 4bb1844..6b02188 100644 --- a/components/printer/custom_actions.go +++ b/components/printer/custom_actions.go @@ -1,18 +1,7 @@ package printer -import ( - "fmt" - - "github.com/opf/openproject-cli/models" -) +import "github.com/opf/openproject-cli/models" func CustomActions(actions []*models.CustomAction) { - for _, a := range actions { - printCustomAction(a) - } -} - -func printCustomAction(action *models.CustomAction) { - id := fmt.Sprintf("#%d", action.Id) - activePrinter.Printf("%s %s\n", Red(id), Cyan(action.Name)) + activeRenderer.CustomActions(actions) } diff --git a/components/printer/json_renderer.go b/components/printer/json_renderer.go new file mode 100644 index 0000000..a933a68 --- /dev/null +++ b/components/printer/json_renderer.go @@ -0,0 +1,275 @@ +package printer + +import ( + "encoding/json" + "fmt" + "sort" + + "github.com/opf/openproject-cli/models" +) + +type JsonRenderer struct{} + +func (r *JsonRenderer) Budget(b *models.Budget) { + printJson(struct { + Id uint64 `json:"id"` + Subject string `json:"subject"` + }{b.Id, b.Subject}) +} + +func (r *JsonRenderer) Budgets(bs []*models.Budget) { + type item struct { + Id uint64 `json:"id"` + Subject string `json:"subject"` + } + out := make([]item, len(bs)) + for i, b := range bs { + out[i] = item{b.Id, b.Subject} + } + printJson(out) +} + +func (r *JsonRenderer) WorkPackage(wp *models.WorkPackage) { + printJson(struct { + Id uint64 `json:"id"` + DisplayId string `json:"display_id,omitempty"` + Subject string `json:"subject"` + Type string `json:"type"` + Status string `json:"status"` + Assignee string `json:"assignee"` + Description string `json:"description"` + }{wp.Id, wp.DisplayId, wp.Subject, wp.Type, wp.Status, wp.Assignee, wp.Description}) +} + +func (r *JsonRenderer) WorkPackages(wps []*models.WorkPackage) { + type item struct { + Id uint64 `json:"id"` + DisplayId string `json:"display_id,omitempty"` + Subject string `json:"subject"` + Type string `json:"type"` + Status string `json:"status"` + Assignee string `json:"assignee"` + } + out := make([]item, len(wps)) + for i, wp := range wps { + out[i] = item{wp.Id, wp.DisplayId, wp.Subject, wp.Type, wp.Status, wp.Assignee} + } + printJson(out) +} + +func (r *JsonRenderer) Project(p *models.Project) { + printJson(struct { + Id uint64 `json:"id"` + Identifier string `json:"identifier"` + Name string `json:"name"` + }{p.Id, p.Identifier, p.Name}) +} + +func (r *JsonRenderer) Projects(ps []*models.Project) { + type item struct { + Id uint64 `json:"id"` + Identifier string `json:"identifier"` + Name string `json:"name"` + } + out := make([]item, len(ps)) + for i, p := range ps { + out[i] = item{p.Id, p.Identifier, p.Name} + } + printJson(out) +} + +func (r *JsonRenderer) User(u *models.User) { + printJson(struct { + Id uint64 `json:"id"` + Name string `json:"name"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` + }{u.Id, u.Name, u.FirstName, u.LastName}) +} + +func (r *JsonRenderer) Users(us []*models.User) { + type item struct { + Id uint64 `json:"id"` + Name string `json:"name"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` + } + out := make([]item, len(us)) + for i, u := range us { + out[i] = item{u.Id, u.Name, u.FirstName, u.LastName} + } + printJson(out) +} + +func (r *JsonRenderer) Types(types []*models.Type) { + type item struct { + Id uint64 `json:"id"` + Name string `json:"name"` + } + out := make([]item, len(types)) + for i, t := range types { + out[i] = item{t.Id, t.Name} + } + printJson(out) +} + +func (r *JsonRenderer) Status(s *models.Status) { + printJson(struct { + Id uint64 `json:"id"` + Name string `json:"name"` + IsDefault bool `json:"is_default"` + IsClosed bool `json:"is_closed"` + }{s.Id, s.Name, s.IsDefault, s.IsClosed}) +} + +func (r *JsonRenderer) StatusList(statuses []*models.Status) { + type item struct { + Id uint64 `json:"id"` + Name string `json:"name"` + IsDefault bool `json:"is_default"` + IsClosed bool `json:"is_closed"` + } + out := make([]item, len(statuses)) + for i, s := range statuses { + out[i] = item{s.Id, s.Name, s.IsDefault, s.IsClosed} + } + printJson(out) +} + +func (r *JsonRenderer) TimeEntry(t *models.TimeEntry) { + printJson(struct { + Id uint64 `json:"id"` + Comment string `json:"comment"` + Project string `json:"project"` + WorkPackage string `json:"work_package"` + SpentOn string `json:"spent_on"` + Hours float64 `json:"hours"` + Activity string `json:"activity"` + User string `json:"user"` + }{t.Id, t.Comment, t.Project, t.WorkPackage, t.SpentOn.Format("2006-01-02"), t.Hours.Hours(), t.Activity, t.User}) +} + +func (r *JsonRenderer) TimeEntryList(entries []*models.TimeEntry) { + type item struct { + Id uint64 `json:"id"` + Comment string `json:"comment"` + Project string `json:"project"` + WorkPackage string `json:"work_package"` + SpentOn string `json:"spent_on"` + Hours float64 `json:"hours"` + Activity string `json:"activity"` + User string `json:"user"` + } + out := make([]item, len(entries)) + for i, t := range entries { + out[i] = item{ + t.Id, + t.Comment, + t.Project, + t.WorkPackage, + t.SpentOn.Format("2006-01-02"), + t.Hours.Hours(), + t.Activity, + t.User, + } + } + printJson(out) +} + +func (r *JsonRenderer) Notification(n *models.Notification) { + printJson(struct { + Id uint64 `json:"id"` + ResourceId uint64 `json:"resource_id"` + ResourceSubject string `json:"resource_subject"` + Reason string `json:"reason"` + Read bool `json:"read"` + }{n.Id, n.ResourceId, n.ResourceSubject, n.Reason, n.Read}) +} + +func (r *JsonRenderer) Notifications(ns []*models.Notification) { + type item struct { + Id uint64 `json:"id"` + ResourceId uint64 `json:"resource_id"` + ResourceSubject string `json:"resource_subject"` + Reason string `json:"reason"` + Read bool `json:"read"` + } + out := make([]item, len(ns)) + for i, n := range ns { + out[i] = item{n.Id, n.ResourceId, n.ResourceSubject, n.Reason, n.Read} + } + printJson(out) +} + +func (r *JsonRenderer) Activities(activities []*models.Activity, users []*models.User) { + type item struct { + Id uint64 `json:"id"` + Comment string `json:"comment"` + Details []*string `json:"details"` + UpdatedAt string `json:"updated_at"` + User string `json:"user"` + } + out := make([]item, len(activities)) + for i, a := range activities { + userName := "" + if a.UserId > 0 { + idx := sort.Search(len(users), func(j int) bool { return users[j].Id >= a.UserId }) + if idx < len(users) && users[idx].Id == a.UserId { + userName = users[idx].Name + } + } + out[i] = item{a.Id, a.Comment, a.Details, a.UpdatedAt, userName} + } + printJson(out) +} + +func (r *JsonRenderer) CustomActions(actions []*models.CustomAction) { + type item struct { + Id uint64 `json:"id"` + Name string `json:"name"` + } + out := make([]item, len(actions)) + for i, a := range actions { + out[i] = item{a.Id, a.Name} + } + printJson(out) +} + +type whoamiJson struct { + Profile string `json:"profile"` + Server string `json:"server"` + Id uint64 `json:"id"` + Name string `json:"name"` +} + +func (r *JsonRenderer) Whoami(profile, host string, user *models.User) { + printJson(whoamiJson{profile, host, user.Id, user.Name}) +} + +func (r *JsonRenderer) WhoamiList(entries []WhoamiEntry) { + out := make([]whoamiJson, len(entries)) + for i, entry := range entries { + out[i] = whoamiJson{entry.Profile, entry.Host, entry.User.Id, entry.User.Name} + } + printJson(out) +} + +func (r *JsonRenderer) Number(n int64) { + printJson(struct { + Total int64 `json:"total"` + }{n}) +} + +func printJson(v any) { + b, err := json.MarshalIndent(v, "", " ") + if err != nil { + // Marshal the error payload too, so the failure output is still valid + // JSON even when the error text contains quotes or newlines. + errJson, _ := json.Marshal(struct { + Error string `json:"error"` + }{fmt.Sprintf("failed to serialize output: %s", err)}) + activePrinter.Println(string(errJson)) + return + } + activePrinter.Println(string(b)) +} diff --git a/components/printer/loading.go b/components/printer/loading.go index ab7d9ca..96d09b2 100644 --- a/components/printer/loading.go +++ b/components/printer/loading.go @@ -1,6 +1,7 @@ package printer import ( + "os" "time" "github.com/briandowns/spinner" @@ -11,7 +12,7 @@ type function[T any] func() (T, error) var loadingSpinner *spinner.Spinner func init() { - loadingSpinner = spinner.New(spinner.CharSets[14], 100*time.Millisecond) + loadingSpinner = spinner.New(spinner.CharSets[14], 100*time.Millisecond, spinner.WithWriter(os.Stderr)) loadingSpinner.Suffix = " fetching data" _ = loadingSpinner.Color("yellow") } diff --git a/components/printer/notifications.go b/components/printer/notifications.go index bfea26b..36f4fb3 100644 --- a/components/printer/notifications.go +++ b/components/printer/notifications.go @@ -4,32 +4,20 @@ import ( "fmt" "strings" - "github.com/opf/openproject-cli/components/common" "github.com/opf/openproject-cli/models" ) -type groupedNotification struct { - notification *models.Notification - count int +func Notification(n *models.Notification) { + activeRenderer.Notification(n) } func Notifications(notifications []*models.Notification) { - grouped := group(notifications) - - var maxIdLength int - var maxReasonLength int - for _, element := range grouped { - maxIdLength = common.Max(maxIdLength, idLength(element.notification.ResourceId)) - maxReasonLength = common.Max(maxReasonLength, len(element.notification.Reason)) - } - - for _, notification := range grouped { - printNotification(notification, maxIdLength, maxReasonLength) - } + activeRenderer.Notifications(notifications) } -func Notification(notification *models.Notification) { - printNotification(&groupedNotification{notification: notification, count: 1}, idLength(notification.ResourceId), len(notification.Reason)) +type groupedNotification struct { + notification *models.Notification + count int } func printNotification(line *groupedNotification, maxIdLength, maxReasonLength int) { @@ -63,12 +51,10 @@ func group(notifications []*models.Notification) []*groupedNotification { break } } - if !alreadyAdded { list = append(list, &groupedNotification{notification: notification, count: 1}) } } - return list } diff --git a/components/printer/numbers.go b/components/printer/numbers.go index e770436..b70535f 100644 --- a/components/printer/numbers.go +++ b/components/printer/numbers.go @@ -1,7 +1,5 @@ package printer -import "strconv" - -func Number(number int64) { - activePrinter.Printf("%s\n", Cyan(strconv.FormatInt(number, 10))) +func Number(n int64) { + activeRenderer.Number(n) } diff --git a/components/printer/printer.go b/components/printer/printer.go index 00f0c24..e67d862 100644 --- a/components/printer/printer.go +++ b/components/printer/printer.go @@ -3,6 +3,10 @@ package printer type Printer interface { Printf(format string, a ...any) (n int, err error) Println(a ...any) (n int, err error) + // Eprintln and Eprintf write to standard error, keeping diagnostics out + // of the machine-readable output written to standard out. + Eprintln(a ...any) (n int, err error) + Eprintf(format string, a ...any) (n int, err error) } var activePrinter Printer diff --git a/components/printer/projects.go b/components/printer/projects.go index e3cda1c..a085630 100644 --- a/components/printer/projects.go +++ b/components/printer/projects.go @@ -1,22 +1,11 @@ package printer -import ( - "fmt" - - "github.com/opf/openproject-cli/models" -) +import "github.com/opf/openproject-cli/models" func Projects(projects []*models.Project) { - for _, p := range projects { - printProject(p) - } + activeRenderer.Projects(projects) } func Project(project *models.Project) { - printProject(project) -} - -func printProject(project *models.Project) { - id := fmt.Sprintf("#%d", project.Id) - activePrinter.Printf("%s %s\n", Red(id), Cyan(project.Name)) + activeRenderer.Project(project) } diff --git a/components/printer/projects_test.go b/components/printer/projects_test.go index a5ac51a..8d17875 100644 --- a/components/printer/projects_test.go +++ b/components/printer/projects_test.go @@ -13,10 +13,10 @@ import ( func TestProject(t *testing.T) { testingPrinter.Reset() - project := models.Project{Id: 42, Name: "Example"} + project := models.Project{Id: 42, Identifier: "example", Name: "Example"} idString := "#" + strconv.FormatUint(project.Id, 10) - expected := fmt.Sprintf("%s %s\n", printer.Red(idString), printer.Cyan(project.Name)) + expected := fmt.Sprintf("%s %s (%s)\n", printer.Red(idString), printer.Cyan(project.Name), project.Identifier) printer.Project(&project) @@ -29,9 +29,9 @@ func TestProjects(t *testing.T) { testingPrinter.Reset() projects := []*models.Project{ - {Id: 42, Name: "Foo"}, - {Id: 45, Name: "Bar"}, - {Id: 123, Name: "Baz"}, + {Id: 42, Identifier: "foo", Name: "Foo"}, + {Id: 45, Identifier: "bar", Name: "Bar"}, + {Id: 123, Identifier: "baz", Name: "Baz"}, } expected := common.Reduce[*models.Project, string]( @@ -39,7 +39,7 @@ func TestProjects(t *testing.T) { func(state string, project *models.Project) string { idString := "#" + strconv.FormatUint(project.Id, 10) - return state + fmt.Sprintf("%s %s\n", printer.Red(idString), printer.Cyan(project.Name)) + return state + fmt.Sprintf("%s %s (%s)\n", printer.Red(idString), printer.Cyan(project.Name), project.Identifier) }, "") diff --git a/components/printer/renderer.go b/components/printer/renderer.go new file mode 100644 index 0000000..522870a --- /dev/null +++ b/components/printer/renderer.go @@ -0,0 +1,46 @@ +package printer + +import ( + "fmt" + + "github.com/opf/openproject-cli/models" +) + +type Renderer interface { + Budget(*models.Budget) + Budgets([]*models.Budget) + WorkPackage(*models.WorkPackage) + WorkPackages([]*models.WorkPackage) + Project(*models.Project) + Projects([]*models.Project) + User(*models.User) + Users([]*models.User) + Types([]*models.Type) + Status(*models.Status) + StatusList([]*models.Status) + TimeEntryList([]*models.TimeEntry) + TimeEntry(*models.TimeEntry) + Notification(*models.Notification) + Notifications([]*models.Notification) + Activities([]*models.Activity, []*models.User) + CustomActions([]*models.CustomAction) + Number(int64) + Whoami(profile, host string, user *models.User) + WhoamiList(entries []WhoamiEntry) +} + +var activeRenderer Renderer = &TextRenderer{} + +func InitRenderer(format string) error { + switch format { + case "json": + activeRenderer = &JsonRenderer{} + case "text", "": + activeRenderer = &TextRenderer{} + default: + activeRenderer = &TextRenderer{} + ErrorText(fmt.Sprintf("unknown output format %q", format)) + return fmt.Errorf("unknown output format %q", format) + } + return nil +} diff --git a/components/printer/renderer_test.go b/components/printer/renderer_test.go new file mode 100644 index 0000000..75687d1 --- /dev/null +++ b/components/printer/renderer_test.go @@ -0,0 +1,38 @@ +package printer_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" +) + +func TestInitRenderer_UnknownFormat_PrintsError(t *testing.T) { + testingPrinter.Reset() + err := printer.InitRenderer("xml") + if err == nil { + t.Fatal("expected unknown format to return an error") + } + if !strings.Contains(testingPrinter.ErrResult, "xml") { + t.Errorf("expected error mentioning unknown format 'xml', got: %q", testingPrinter.ErrResult) + } + if count := strings.Count(testingPrinter.ErrResult, "[ERROR]"); count != 1 { + t.Errorf("error diagnostic count = %d, want 1", count) + } + if testingPrinter.Result != "" { + t.Errorf("stdout = %q, want empty", testingPrinter.Result) + } +} + +func TestInitRenderer_KnownFormats_NoError(t *testing.T) { + defer printer.InitRenderer("text") + for _, format := range []string{"text", "json"} { + testingPrinter.Reset() + if err := printer.InitRenderer(format); err != nil { + t.Errorf("InitRenderer(%q) returned an error: %v", format, err) + } + if strings.Contains(testingPrinter.ErrResult, "[ERROR]") { + t.Errorf("unexpected error output for known format %q: %s", format, testingPrinter.ErrResult) + } + } +} diff --git a/components/printer/status.go b/components/printer/status.go index fec62e5..079c92c 100644 --- a/components/printer/status.go +++ b/components/printer/status.go @@ -1,39 +1,11 @@ package printer -import ( - "fmt" - "github.com/opf/openproject-cli/components/common" - "github.com/opf/openproject-cli/models" - "strings" -) +import "github.com/opf/openproject-cli/models" -func StatusList(status []*models.Status) { - var maxIdLength = 0 - for _, s := range status { - maxIdLength = common.Max(maxIdLength, idLength(s.Id)) - } - - for _, s := range status { - printStatus(s, maxIdLength) - } -} - -func Status(status *models.Status) { - printStatus(status, idLength(status.Id)) +func Status(s *models.Status) { + activeRenderer.Status(s) } -func printStatus(status *models.Status, maxIdLength int) { - var parts []string - - diff := maxIdLength - idLength(status.Id) - idStr := fmt.Sprintf("%s#%d", indent(diff), status.Id) - parts = append(parts, Red(idStr)) - - parts = append(parts, Cyan(status.Name)) - - if status.IsDefault { - parts = append(parts, fmt.Sprintf("(%s)", Yellow("default"))) - } - - activePrinter.Println(strings.Join(parts, " ")) +func StatusList(statuses []*models.Status) { + activeRenderer.StatusList(statuses) } diff --git a/components/printer/streams_test.go b/components/printer/streams_test.go new file mode 100644 index 0000000..f3d5ef4 --- /dev/null +++ b/components/printer/streams_test.go @@ -0,0 +1,47 @@ +package printer_test + +import ( + "errors" + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" +) + +// Diagnostics must go to standard error so that machine-readable output +// (e.g. --format json) on standard out stays parseable. +func TestDiagnosticsGoToStderr(t *testing.T) { + diagnostics := map[string]func(){ + "Info": func() { printer.Info("progress") }, + "Done": func() { printer.Done() }, + "Error": func() { printer.Error(errors.New("boom")) }, + "ErrorText": func() { printer.ErrorText("boom") }, + "Warning": func() { printer.Warning("boom") }, + "Debug": func() { printer.Debug(true, "boom") }, + } + + for name, call := range diagnostics { + testingPrinter.Reset() + call() + + if len(testingPrinter.ErrResult) == 0 { + t.Errorf("%s: expected output on stderr, got none", name) + } + if len(testingPrinter.Result) > 0 { + t.Errorf("%s: unexpected output on stdout: %q", name, testingPrinter.Result) + } + } +} + +func TestOutputGoesToStdout(t *testing.T) { + testingPrinter.Reset() + + printer.Output("data") + + if !strings.Contains(testingPrinter.Result, "data") { + t.Errorf("expected data on stdout, got: %q", testingPrinter.Result) + } + if len(testingPrinter.ErrResult) > 0 { + t.Errorf("unexpected output on stderr: %q", testingPrinter.ErrResult) + } +} diff --git a/components/printer/testing_printer.go b/components/printer/testing_printer.go index faea265..e3a9a73 100644 --- a/components/printer/testing_printer.go +++ b/components/printer/testing_printer.go @@ -2,8 +2,11 @@ package printer import "fmt" +// TestingPrinter captures output for assertions. Result collects what would +// go to standard out, ErrResult what would go to standard error. type TestingPrinter struct { - Result string + Result string + ErrResult string } func (printer *TestingPrinter) Printf(format string, a ...any) (n int, err error) { @@ -18,6 +21,19 @@ func (printer *TestingPrinter) Println(a ...any) (n int, err error) { return len(printer.Result), nil } +func (printer *TestingPrinter) Eprintln(a ...any) (n int, err error) { + printer.ErrResult += fmt.Sprintln(a...) + + return len(printer.ErrResult), nil +} + +func (printer *TestingPrinter) Eprintf(format string, a ...any) (n int, err error) { + printer.ErrResult += fmt.Sprintf(format, a...) + + return len(printer.ErrResult), nil +} + func (printer *TestingPrinter) Reset() { printer.Result = "" + printer.ErrResult = "" } diff --git a/components/printer/text_renderer.go b/components/printer/text_renderer.go new file mode 100644 index 0000000..e3ba21d --- /dev/null +++ b/components/printer/text_renderer.go @@ -0,0 +1,211 @@ +package printer + +import ( + "fmt" + "sort" + "strconv" + "strings" + "unicode/utf8" + + "github.com/opf/openproject-cli/components/common" + "github.com/opf/openproject-cli/models" +) + +type TextRenderer struct{} + +func (r *TextRenderer) Budget(b *models.Budget) { + printBudget(b, idLength(b.Id)) +} + +func (r *TextRenderer) Budgets(bs []*models.Budget) { + var maxIdLength = 0 + for _, b := range bs { + maxIdLength = common.Max(maxIdLength, idLength(b.Id)) + } + for _, b := range bs { + printBudget(b, maxIdLength) + } +} + +func (r *TextRenderer) WorkPackage(wp *models.WorkPackage) { + printHeadline(wp, displayIdLength(wp), 0, utf8.RuneCountInString(wp.Type)) + printAttributes(wp) + activePrinter.Println() + printOpenLink(wp) + activePrinter.Println() + printDescription(wp) +} + +func (r *TextRenderer) WorkPackages(wps []*models.WorkPackage) { + var maxIdLength = 0 + var maxTypeLength = 0 + var maxStatusLength = 0 + for _, w := range wps { + maxIdLength = common.Max(maxIdLength, displayIdLength(w)) + maxTypeLength = common.Max(maxTypeLength, utf8.RuneCountInString(w.Type)) + maxStatusLength = common.Max(maxStatusLength, utf8.RuneCountInString(w.Status)) + } + for _, wp := range wps { + printHeadline(wp, maxIdLength, maxStatusLength, maxTypeLength) + } +} + +func (r *TextRenderer) Project(p *models.Project) { + printProject(p) +} + +func (r *TextRenderer) Projects(ps []*models.Project) { + for _, p := range ps { + printProject(p) + } +} + +func (r *TextRenderer) User(u *models.User) { + printUser(u, idLength(u.Id)) +} + +func (r *TextRenderer) Users(us []*models.User) { + var maxIdLength = 0 + for _, u := range us { + maxIdLength = common.Max(maxIdLength, idLength(u.Id)) + } + for _, u := range us { + printUser(u, maxIdLength) + } +} + +func (r *TextRenderer) Types(types []*models.Type) { + var maxIdLength = 0 + for _, t := range types { + maxIdLength = common.Max(maxIdLength, idLength(t.Id)) + } + for _, t := range types { + printType(t, maxIdLength) + } +} + +func (r *TextRenderer) Status(s *models.Status) { + printStatus(s, idLength(s.Id)) +} + +func (r *TextRenderer) StatusList(statuses []*models.Status) { + var maxIdLength = 0 + for _, s := range statuses { + maxIdLength = common.Max(maxIdLength, idLength(s.Id)) + } + for _, s := range statuses { + printStatus(s, maxIdLength) + } +} + +func (r *TextRenderer) TimeEntry(t *models.TimeEntry) { + printTimeEntry(t, idLength(t.Id), len(t.Activity), len(t.Project)) +} + +func (r *TextRenderer) TimeEntryList(entries []*models.TimeEntry) { + var maxIdLength = 0 + var maxActivityLength = 0 + var maxProjectLength = 0 + for _, t := range entries { + maxIdLength = common.Max(maxIdLength, idLength(t.Id)) + maxActivityLength = common.Max(maxActivityLength, len(t.Activity)) + maxProjectLength = common.Max(maxProjectLength, len(t.Project)) + } + for _, t := range entries { + printTimeEntry(t, maxIdLength, maxActivityLength, maxProjectLength) + } +} + +func (r *TextRenderer) Notification(n *models.Notification) { + printNotification(&groupedNotification{notification: n, count: 1}, idLength(n.ResourceId), len(n.Reason)) +} + +func (r *TextRenderer) Notifications(ns []*models.Notification) { + grouped := group(ns) + var maxIdLength int + var maxReasonLength int + for _, element := range grouped { + maxIdLength = common.Max(maxIdLength, idLength(element.notification.ResourceId)) + maxReasonLength = common.Max(maxReasonLength, len(element.notification.Reason)) + } + for _, n := range grouped { + printNotification(n, maxIdLength, maxReasonLength) + } +} + +func (r *TextRenderer) Activities(activities []*models.Activity, users []*models.User) { + for _, activity := range activities { + user := &models.User{} + if activity.UserId > 0 { + idx := sort.Search(len(users), func(i int) bool { return users[i].Id >= activity.UserId }) + if idx < len(users) && users[idx].Id == activity.UserId { + user = users[idx] + } + } + printActivityHeadline(activity, user) + printActivityBody(activity) + activePrinter.Println("") + } +} + +func (r *TextRenderer) CustomActions(actions []*models.CustomAction) { + for _, a := range actions { + printCustomAction(a) + } +} + +func (r *TextRenderer) Whoami(profile, host string, user *models.User) { + activePrinter.Printf("Profile: %s\n", Yellow(profile)) + activePrinter.Printf("Server: %s\n", Cyan(host)) + activePrinter.Printf("User: %s %s\n", Red(fmt.Sprintf("#%d", user.Id)), Cyan(user.Name)) +} + +func (r *TextRenderer) WhoamiList(entries []WhoamiEntry) { + for i, entry := range entries { + if i > 0 { + activePrinter.Println() + } + r.Whoami(entry.Profile, entry.Host, entry.User) + } +} + +func (r *TextRenderer) Number(n int64) { + activePrinter.Printf("%s\n", Cyan(strconv.FormatInt(n, 10))) +} + +func printBudget(b *models.Budget, maxIdLength int) { + diff := maxIdLength - idLength(b.Id) + idStr := fmt.Sprintf("%s#%d", indent(diff), b.Id) + activePrinter.Printf("%s %s\n", Red(idStr), Cyan(b.Subject)) +} + +func printProject(p *models.Project) { + id := fmt.Sprintf("#%d", p.Id) + activePrinter.Printf("%s %s (%s)\n", Red(id), Cyan(p.Name), p.Identifier) +} + +func printUser(u *models.User, maxIdLength int) { + diff := maxIdLength - idLength(u.Id) + idStr := fmt.Sprintf("%s#%d", indent(diff), u.Id) + activePrinter.Println(strings.Join([]string{Red(idStr), Cyan(u.Name)}, " ")) +} + +func printType(t *models.Type, maxIdLength int) { + diff := maxIdLength - idLength(t.Id) + idStr := fmt.Sprintf("%s#%d", indent(diff), t.Id) + activePrinter.Println(strings.Join([]string{Red(idStr), Cyan(t.Name)}, " ")) +} + +func printStatus(s *models.Status, maxIdLength int) { + diff := maxIdLength - idLength(s.Id) + idStr := fmt.Sprintf("%s#%d", indent(diff), s.Id) + parts := []string{Red(idStr), Cyan(s.Name)} + if s.IsDefault { + parts = append(parts, fmt.Sprintf("(%s)", Yellow("default"))) + } + activePrinter.Println(strings.Join(parts, " ")) +} + +func printCustomAction(a *models.CustomAction) { + activePrinter.Printf("%s %s\n", Red(fmt.Sprintf("#%d", a.Id)), Cyan(a.Name)) +} diff --git a/components/printer/time_entries.go b/components/printer/time_entries.go index 67a7335..ab19405 100644 --- a/components/printer/time_entries.go +++ b/components/printer/time_entries.go @@ -4,61 +4,45 @@ import ( "fmt" "strings" - "github.com/opf/openproject-cli/components/common" "github.com/opf/openproject-cli/models" ) -func TimeEntryList(timeEntry []*models.TimeEntry) { - var maxIdLength = 0 - var maxActivityLength = 0 - var maxProjectLength = 0 - for _, t := range timeEntry { - maxIdLength = common.Max(maxIdLength, idLength(t.Id)) - maxActivityLength = common.Max(maxActivityLength, len(t.Activity)) - maxProjectLength = common.Max(maxProjectLength, len(t.Project)) - } - - for _, t := range timeEntry { - printTimeEntry(t, maxIdLength, maxActivityLength, maxProjectLength) - } +func TimeEntry(t *models.TimeEntry) { + activeRenderer.TimeEntry(t) } -func TimeEntry(timeEntry *models.TimeEntry) { - printTimeEntry(timeEntry, idLength(timeEntry.Id), len(timeEntry.Activity), len(timeEntry.Project)) +func TimeEntryList(entries []*models.TimeEntry) { + activeRenderer.TimeEntryList(entries) } -func printTimeEntry(timeEntry *models.TimeEntry, maxIdLength int, maxActivityLength int, maxProjectLength int) { +func printTimeEntry(t *models.TimeEntry, maxIdLength, maxActivityLength, maxProjectLength int) { var parts []string - diff := maxIdLength - idLength(timeEntry.Id) - idStr := fmt.Sprintf("%s#%d", indent(diff), timeEntry.Id) - + diff := maxIdLength - idLength(t.Id) + idStr := fmt.Sprintf("%s#%d", indent(diff), t.Id) parts = append(parts, Red(idStr)) if maxActivityLength > 0 { - diff = maxActivityLength - len(timeEntry.Activity) - activityStr := Green(strings.ToUpper(timeEntry.Activity)) + indent(diff) + diff = maxActivityLength - len(t.Activity) + activityStr := Green(strings.ToUpper(t.Activity)) + indent(diff) parts = append(parts, activityStr) } - parts = append(parts, Cyan(timeEntry.SpentOn.Format("Mon Jan _2"))) - - hoursStr := fmt.Sprintf("%.2fh", timeEntry.Hours.Hours()) - parts = append(parts, hoursStr) + parts = append(parts, Cyan(t.SpentOn.Format("Mon Jan _2"))) + parts = append(parts, fmt.Sprintf("%.2fh", t.Hours.Hours())) if maxProjectLength > 0 { - diff = maxProjectLength - len(timeEntry.Project) - projectStr := Yellow(timeEntry.Project) + indent(diff) + diff = maxProjectLength - len(t.Project) + projectStr := Yellow(t.Project) + indent(diff) parts = append(parts, projectStr) } - parts = append(parts, Cyan(timeEntry.WorkPackage)) + parts = append(parts, Cyan(t.WorkPackage)) - if len(timeEntry.Comment) > 0 { - parts = append(parts, timeEntry.Comment) + if len(t.Comment) > 0 { + parts = append(parts, t.Comment) } - - if timeEntry.Ongoing { + if t.Ongoing { parts = append(parts, fmt.Sprintf("(%s)", Yellow("ongoing"))) } diff --git a/components/printer/types.go b/components/printer/types.go index a62bc1e..2f8fe45 100644 --- a/components/printer/types.go +++ b/components/printer/types.go @@ -2,30 +2,19 @@ package printer import ( "fmt" - "strings" - "github.com/opf/openproject-cli/components/common" "github.com/opf/openproject-cli/models" ) func Types(types []*models.Type) { - var maxIdLength = 0 - for _, t := range types { - maxIdLength = common.Max(maxIdLength, idLength(t.Id)) - } + activeRenderer.Types(types) +} +// AvailableTypes lists type names on standard error as a guidance hint, so it +// never pollutes machine-readable output (e.g. JSON) on a failed command. +func AvailableTypes(types []*models.Type) { + Info("Available types:") for _, t := range types { - printType(t, maxIdLength) + Info(fmt.Sprintf(" - %s", t.Name)) } } - -func printType(workPackageType *models.Type, maxIdLength int) { - var parts []string - - diff := maxIdLength - idLength(workPackageType.Id) - idStr := fmt.Sprintf("%s#%d", indent(diff), workPackageType.Id) - parts = append(parts, Red(idStr)) - - parts = append(parts, Cyan(workPackageType.Name)) - activePrinter.Println(strings.Join(parts, " ")) -} diff --git a/components/printer/users.go b/components/printer/users.go index 8cde6d5..0d41dd2 100644 --- a/components/printer/users.go +++ b/components/printer/users.go @@ -1,35 +1,11 @@ package printer -import ( - "fmt" - "strings" - - "github.com/opf/openproject-cli/components/common" - "github.com/opf/openproject-cli/models" -) +import "github.com/opf/openproject-cli/models" func Users(users []*models.User) { - var maxIdLength = 0 - for _, u := range users { - maxIdLength = common.Max(maxIdLength, idLength(u.Id)) - } - - for _, u := range users { - printUser(u, maxIdLength) - } + activeRenderer.Users(users) } func User(user *models.User) { - printUser(user, idLength(user.Id)) -} - -func printUser(user *models.User, maxIdLength int) { - var parts []string - - diff := maxIdLength - idLength(user.Id) - idStr := fmt.Sprintf("%s#%d", indent(diff), user.Id) - parts = append(parts, Red(idStr)) - - parts = append(parts, Cyan(user.Name)) - activePrinter.Println(strings.Join(parts, " ")) + activeRenderer.User(user) } diff --git a/components/printer/warning_test.go b/components/printer/warning_test.go new file mode 100644 index 0000000..457ff87 --- /dev/null +++ b/components/printer/warning_test.go @@ -0,0 +1,21 @@ +package printer_test + +import ( + "strings" + "testing" + + "github.com/opf/openproject-cli/components/printer" +) + +func TestWarning(t *testing.T) { + testingPrinter.Reset() + + printer.Warning("watch out") + + if !strings.Contains(testingPrinter.ErrResult, "[WARNING]") { + t.Errorf("warning missing [WARNING] tag: %q", testingPrinter.ErrResult) + } + if !strings.Contains(testingPrinter.ErrResult, "watch out") { + t.Errorf("warning missing message: %q", testingPrinter.ErrResult) + } +} diff --git a/components/printer/whoami.go b/components/printer/whoami.go new file mode 100644 index 0000000..9a0ebd0 --- /dev/null +++ b/components/printer/whoami.go @@ -0,0 +1,20 @@ +package printer + +import "github.com/opf/openproject-cli/models" + +// WhoamiEntry is one resolved identity (profile, server, user) for display. +type WhoamiEntry struct { + Profile string + Host string + User *models.User +} + +func Whoami(profile, host string, user *models.User) { + activeRenderer.Whoami(profile, host, user) +} + +// WhoamiList renders several identities as one output value, so JSON mode +// emits a single array instead of multiple top-level objects. +func WhoamiList(entries []WhoamiEntry) { + activeRenderer.WhoamiList(entries) +} diff --git a/components/printer/whoami_test.go b/components/printer/whoami_test.go new file mode 100644 index 0000000..b092eb3 --- /dev/null +++ b/components/printer/whoami_test.go @@ -0,0 +1,76 @@ +package printer_test + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/models" +) + +func TestWhoami(t *testing.T) { + testingPrinter.Reset() + + user := &models.User{Id: 42, Name: "Jane Doe"} + printer.Whoami("default", "https://example.com", user) + + expected := fmt.Sprintf( + "Profile: %s\nServer: %s\nUser: %s %s\n", + printer.Yellow("default"), + printer.Cyan("https://example.com"), + printer.Red("#42"), + printer.Cyan("Jane Doe"), + ) + + if testingPrinter.Result != expected { + t.Errorf("Whoami output mismatch\ngot: %q\nwant: %q", testingPrinter.Result, expected) + } +} + +func TestWhoamiList_Text_SeparatesEntries(t *testing.T) { + testingPrinter.Reset() + + printer.WhoamiList([]printer.WhoamiEntry{ + {Profile: "default", Host: "https://a.example.com", User: &models.User{Id: 1, Name: "A"}}, + {Profile: "work", Host: "https://b.example.com", User: &models.User{Id: 2, Name: "B"}}, + }) + + expected := fmt.Sprintf( + "Profile: %s\nServer: %s\nUser: %s %s\n\nProfile: %s\nServer: %s\nUser: %s %s\n", + printer.Yellow("default"), + printer.Cyan("https://a.example.com"), + printer.Red("#1"), + printer.Cyan("A"), + printer.Yellow("work"), + printer.Cyan("https://b.example.com"), + printer.Red("#2"), + printer.Cyan("B"), + ) + + if testingPrinter.Result != expected { + t.Errorf("WhoamiList output mismatch\ngot: %q\nwant: %q", testingPrinter.Result, expected) + } +} + +func TestWhoamiList_Json_EmitsSingleArray(t *testing.T) { + printer.InitRenderer("json") + defer printer.InitRenderer("text") + testingPrinter.Reset() + + printer.WhoamiList([]printer.WhoamiEntry{ + {Profile: "default", Host: "https://a.example.com", User: &models.User{Id: 1, Name: "A"}}, + {Profile: "work", Host: "https://b.example.com", User: &models.User{Id: 2, Name: "B"}}, + }) + + var parsed []map[string]any + if err := json.Unmarshal([]byte(testingPrinter.Result), &parsed); err != nil { + t.Fatalf("output is not a single JSON array: %v\noutput: %q", err, testingPrinter.Result) + } + if len(parsed) != 2 { + t.Errorf("expected 2 entries, got %d", len(parsed)) + } + if parsed[1]["profile"] != "work" { + t.Errorf("second entry profile = %v, want work", parsed[1]["profile"]) + } +} diff --git a/components/printer/work_packages.go b/components/printer/work_packages.go index 9695df8..778a6fd 100644 --- a/components/printer/work_packages.go +++ b/components/printer/work_packages.go @@ -6,44 +6,41 @@ import ( "strings" "unicode/utf8" - "github.com/opf/openproject-cli/components/common" "github.com/opf/openproject-cli/components/routes" "github.com/opf/openproject-cli/models" ) -func WorkPackages(workPackages []*models.WorkPackage) { - var maxIdLength = 0 - var maxTypeLength = 0 - var maxStatusLength = 0 - for _, w := range workPackages { - maxIdLength = common.Max(maxIdLength, idLength(w.Id)) - maxTypeLength = common.Max(maxTypeLength, utf8.RuneCountInString(w.Type)) - maxStatusLength = common.Max(maxStatusLength, utf8.RuneCountInString(w.Status)) - } - - for _, workPackage := range workPackages { - printHeadline(workPackage, maxIdLength, maxStatusLength, maxTypeLength) - } +func WorkPackage(wp *models.WorkPackage) { + activeRenderer.WorkPackage(wp) } -func WorkPackage(workPackage *models.WorkPackage) { - printHeadline(workPackage, idLength(workPackage.Id), 0, utf8.RuneCountInString(workPackage.Type)) - printAttributes(workPackage) - activePrinter.Println() - printOpenLink(workPackage) - activePrinter.Println() - printDescription(workPackage) +func WorkPackages(wps []*models.WorkPackage) { + activeRenderer.WorkPackages(wps) } func idLength(id uint64) int { return len(strconv.FormatUint(id, 10)) + 1 } +// formatDisplayId returns the display identifier: "SJF-13" for semantic IDs, +// "#42" for numeric-only systems (where displayId equals the numeric id) and +// for servers that omit displayId entirely. +func formatDisplayId(wp *models.WorkPackage) string { + if wp.DisplayId == "" || wp.DisplayId == strconv.FormatUint(wp.Id, 10) { + return fmt.Sprintf("#%d", wp.Id) + } + return wp.DisplayId +} + +func displayIdLength(wp *models.WorkPackage) int { + return utf8.RuneCountInString(formatDisplayId(wp)) +} + func printHeadline(workPackage *models.WorkPackage, maxIdLength, maxStatusLength, maxTypeLength int) { var parts []string - diff := maxIdLength - idLength(workPackage.Id) - idStr := fmt.Sprintf("%s#%d", indent(diff), workPackage.Id) + diff := maxIdLength - displayIdLength(workPackage) + idStr := fmt.Sprintf("%s%s", indent(diff), formatDisplayId(workPackage)) parts = append(parts, Red(idStr)) diff = maxTypeLength - utf8.RuneCountInString(workPackage.Type) @@ -75,52 +72,40 @@ func printOpenLink(workPackage *models.WorkPackage) { } func printDescription(workPackage *models.WorkPackage) { - lines := splitIntoLines(workPackage.Description, 80) - for _, line := range lines { + for _, line := range splitIntoLines(workPackage.Description, 80) { activePrinter.Printf("%s\n", line) } } func splitWords(text string, lineLength int) []string { words := strings.Fields(text) - var lines []string var line string - for _, word := range words { if len(line)+len(word)+1 > lineLength { lines = append(lines, line) line = "" } - if len(line) > 0 { line += " " } - line += word } - if len(line) > 0 { lines = append(lines, line) } - return lines } func splitIntoLines(text string, lineLength int) []string { - paragraphs := strings.Split(text, "\n") - var lines []string - - for _, paragraph := range paragraphs { - splitParagraph := splitWords(paragraph, lineLength) - - if len(splitParagraph) == 0 { - lines = append(lines, "") // Append empty line + for _, paragraph := range strings.Split(text, "\n") { + split := splitWords(paragraph, lineLength) + if len(split) == 0 { + lines = append(lines, "") } else { - lines = append(lines, splitParagraph...) + lines = append(lines, split...) } } - return lines } diff --git a/components/printer/work_packages_test.go b/components/printer/work_packages_test.go index da09dab..cbb13bc 100644 --- a/components/printer/work_packages_test.go +++ b/components/printer/work_packages_test.go @@ -3,6 +3,7 @@ package printer_test import ( "fmt" "strconv" + "strings" "testing" "github.com/opf/openproject-cli/components/printer" @@ -14,11 +15,12 @@ func TestWorkPackage(t *testing.T) { testingPrinter.Reset() workPackage := models.WorkPackage{ - Id: 42, - Subject: "Test", - Type: "TASK", - Assignee: "Aaron", - Status: "New", + Id: 42, + DisplayId: "42", + Subject: "Test", + Type: "TASK", + Assignee: "Aaron", + Status: "New", Description: `Lorem ipsum dolor sit amet, consectetur adipiscing elit. Etiam non est sed nunc euismod luctus. Donec vehicula scelerisque efficitur. Nunc arcu ligula, dictum maximus consequat id, tincidunt vitae augue. Vestibulum ut tellus id nisi faucibus efficitur id eu tortor. Vestibulum sed vehicula turpis, sit amet eleifend massa. Proin eu quam justo. Nulla id libero sit amet turpis venenatis mollis. Quisque iaculis lectus non ligula faucibus, ut pellentesque velit sodales. Vivamus nibh est, molestie at laoreet nec, lacinia porttitor nisl. Nulla eget urna in enim porttitor tempus. Nullam velit nunc, ultrices eget molestie vitae, tincidunt vitae felis. Integer augue purus, mollis a vestibulum quis, sagittis ac lacus. Ut vitae tempor tellus. Cras neque turpis, malesuada nec tincidunt vel, mattis vel dolor. Ut hendrerit magna ac suscipit convallis. Ut quis nisi vel metus facilisis sagittis eu eget orci. Pellentesque laoreet metus vitae nulla fringilla, sed lacinia sem laoreet. Maecenas velit erat, luctus ac metus eget, hendrerit tincidunt dolor. Cras mattis orci sem, sed convallis arcu venenatis nec. Donec imperdiet mattis ante, quis euismod lorem viverra ac. Pellentesque in efficitur magna, at ullamcorper ipsum. Vivamus vulputate, tellus et blandit mollis, elit nisl posuere dui, nec molestie metus arcu in lectus. Vivamus eget congue libero, ut congue dolor. Interdum et malesuada fames ac ante ipsum primis in faucibus. Suspendisse blandit, velit quis euismod tincidunt, nunc lectus rutrum nisi, et commodo enim ligula nec sem. Pellentesque nec tincidunt sapien.`, @@ -69,6 +71,7 @@ func TestWorkPackage_Assignee_With_Empty_String(t *testing.T) { workPackage := models.WorkPackage{ Id: 42, + DisplayId: "42", Subject: "Test", Type: "TASK", Assignee: "", @@ -103,16 +106,18 @@ func TestWorkPackages_WithAccentedCharacters(t *testing.T) { // gets one space too few with the broken code (diff=10-6=4 instead of correct 10-5=5). workPackages := []*models.WorkPackage{ { - Id: 2, - Subject: "Subject A", - Type: "Tâche", - Status: "En cours", + Id: 2, + DisplayId: "2", + Subject: "Subject A", + Type: "Tâche", + Status: "En cours", }, { - Id: 10, - Subject: "Subject B", - Type: "User Story", - Status: "En cours de spécification", + Id: 10, + DisplayId: "10", + Subject: "Subject B", + Type: "User Story", + Status: "En cours de spécification", }, } @@ -137,6 +142,7 @@ func TestWorkPackages(t *testing.T) { workPackages := []*models.WorkPackage{ { Id: 42, + DisplayId: "42", Subject: "Test 1", Type: "PHASE", Assignee: "Obi-Wan", @@ -146,6 +152,7 @@ func TestWorkPackages(t *testing.T) { }, { Id: 43, + DisplayId: "43", Subject: "Test 2", Type: "TASK", Assignee: "Anakin", @@ -168,3 +175,41 @@ func TestWorkPackages(t *testing.T) { t.Errorf("\nExpected:\n%sbut got:\n%s", expected, testingPrinter.Result) } } + +func TestWorkPackages_WithSemanticDisplayId(t *testing.T) { + testingPrinter.Reset() + + // Mix of semantic (SJF-13, 6 chars) and numeric (#10, 3 chars) display IDs. + // Numeric WP has DisplayId == strconv(Id) so it falls back to #10. + workPackages := []*models.WorkPackage{ + {Id: 41855, DisplayId: "SJF-13", Subject: "Semantic WP", Type: "EPIC", Status: "Open"}, + {Id: 10, DisplayId: "10", Subject: "Numeric WP", Type: "TASK", Status: "New"}, + } + + var expected string + // maxIdLength = max(6, 3) = 6 → "#10" gets 3 leading spaces + // maxTypeLength = max(4, 4) = 4 + // maxStatusLength = max(4, 3) = 4 + expected += fmt.Sprintf("%s %s [%s] %s\n", printer.Red("SJF-13"), printer.Green("EPIC"), printer.Yellow("Open"), printer.Cyan("Semantic WP")) + expected += fmt.Sprintf("%s %s [%s] %s\n", printer.Red(" #10"), printer.Green("TASK"), printer.Yellow("New"), printer.Cyan("Numeric WP")) + + printer.WorkPackages(workPackages) + + if testingPrinter.Result != expected { + t.Errorf("\nExpected:\n%sbut got:\n%s", expected, testingPrinter.Result) + } +} + +// Older servers omit displayId entirely; output must fall back to the +// numeric id instead of printing an empty identifier. +func TestWorkPackages_EmptyDisplayIdFallsBackToNumericId(t *testing.T) { + testingPrinter.Reset() + + printer.WorkPackages([]*models.WorkPackage{ + {Id: 42, DisplayId: "", Subject: "Legacy", Type: "TASK", Status: "New"}, + }) + + if !strings.Contains(testingPrinter.Result, "#42") { + t.Errorf("expected fallback to #42, got: %q", testingPrinter.Result) + } +} diff --git a/components/requests/query.go b/components/requests/query.go index 1657618..0377551 100644 --- a/components/requests/query.go +++ b/components/requests/query.go @@ -1,11 +1,13 @@ package requests import ( + "encoding/json" "fmt" - "github.com/opf/openproject-cli/components/common" "net/url" "slices" "strings" + + "github.com/opf/openproject-cli/components/common" ) type Query struct { @@ -20,12 +22,23 @@ type Filter struct { } func (filter Filter) String() string { - return fmt.Sprintf( - "{\"%s\":{\"operator\":\"%s\",\"values\":[\"%s\"]}}", - filter.Name, - filter.Operator, - strings.Join(filter.Values, "\",\""), - ) + values := filter.Values + if values == nil { + values = []string{} + } + // Marshal instead of string interpolation so quotes, backslashes and + // control characters in values cannot produce malformed filter JSON. + payload := map[string]any{ + filter.Name: map[string]any{ + "operator": filter.Operator, + "values": values, + }, + } + b, err := json.Marshal(payload) + if err != nil { + return "{}" + } + return string(b) } func (filter Filter) Equals(other Filter) bool { diff --git a/components/requests/query_test.go b/components/requests/query_test.go index be58969..899fb75 100644 --- a/components/requests/query_test.go +++ b/components/requests/query_test.go @@ -1,6 +1,7 @@ package requests_test import ( + "encoding/json" "strings" "testing" @@ -186,3 +187,22 @@ func TestQuery_Merge(t *testing.T) { t.Errorf("Expected %+v, but got %+v", query3, result) } } + +func TestFilter_String_EscapesSpecialCharacters(t *testing.T) { + filter := requests.Filter{ + Name: "subject", + Operator: "**", + Values: []string{`foo"bar\baz`}, + } + + var parsed map[string]struct { + Operator string `json:"operator"` + Values []string `json:"values"` + } + if err := json.Unmarshal([]byte(filter.String()), &parsed); err != nil { + t.Fatalf("Filter.String() is not valid JSON: %v\ngot: %s", err, filter.String()) + } + if parsed["subject"].Values[0] != `foo"bar\baz` { + t.Errorf("value round-trip mismatch: got %q", parsed["subject"].Values[0]) + } +} diff --git a/components/resources/budgets/functions.go b/components/resources/budgets/functions.go new file mode 100644 index 0000000..b55358d --- /dev/null +++ b/components/resources/budgets/functions.go @@ -0,0 +1,30 @@ +package budgets + +import ( + "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 Lookup(id uint64) (*models.Budget, error) { + response, err := requests.Get(paths.Budget(id), nil) + if err != nil { + return nil, err + } + + element := parser.Parse[dtos.BudgetDto](response) + return element.Convert(), nil +} + +func AllForProject(projectId string) ([]*models.Budget, error) { + query := requests.NewPaginatedQuery(-1, nil) + response, err := requests.Get(paths.ProjectBudgets(projectId), &query) + if err != nil { + return nil, err + } + + element := parser.Parse[dtos.BudgetCollectionDto](response) + return element.Convert(), nil +} diff --git a/components/resources/projects/functions.go b/components/resources/projects/functions.go index fbe631b..6208717 100644 --- a/components/resources/projects/functions.go +++ b/components/resources/projects/functions.go @@ -19,8 +19,11 @@ func All() ([]*models.Project, error) { return element.Convert(), nil } -func Lookup(id uint64) (*models.Project, error) { - response, err := requests.Get(paths.Project(id), nil) +func Lookup(identifier string) (*models.Project, error) { + if err := ValidateIdentifier(identifier); err != nil { + return nil, err + } + response, err := requests.Get(paths.Project(identifier), nil) if err != nil { return nil, err } diff --git a/components/resources/projects/validate.go b/components/resources/projects/validate.go new file mode 100644 index 0000000..85fc343 --- /dev/null +++ b/components/resources/projects/validate.go @@ -0,0 +1,24 @@ +package projects + +import ( + "fmt" + "regexp" + + "github.com/opf/openproject-cli/components/errors" +) + +// Dashes are allowed for old project identifiers; semantic identifiers only allow letters, numbers and underscores. +var invalidIdentifierChars = regexp.MustCompile(`[^a-zA-Z0-9\-_]`) + +func ValidateIdentifier(identifier string) error { + if identifier == "" { + return errors.Custom("project identifier cannot be empty") + } + if invalidIdentifierChars.MatchString(identifier) { + return errors.Custom(fmt.Sprintf( + "invalid project %q: only letters, numbers, - and _ are allowed", + identifier, + )) + } + return nil +} diff --git a/components/resources/projects/validate_test.go b/components/resources/projects/validate_test.go new file mode 100644 index 0000000..6e97703 --- /dev/null +++ b/components/resources/projects/validate_test.go @@ -0,0 +1,42 @@ +package projects_test + +import ( + "testing" + + "github.com/opf/openproject-cli/components/resources/projects" +) + +func TestValidateIdentifier(t *testing.T) { + valid := []string{ + "devops", + "my-project", + "project_name", + "42", + "ABC123", + } + + for _, id := range valid { + if err := projects.ValidateIdentifier(id); err != nil { + t.Errorf("ValidateIdentifier(%q) returned unexpected error: %v", id, err) + } + } + + invalid := []struct { + input string + desc string + }{ + {"", "empty string"}, + {"my project", "space"}, + {"project/path", "slash"}, + {"proj@name", "at sign"}, + {"proj.name", "dot"}, + {"proj!name", "exclamation mark"}, + {"project+extra", "plus sign"}, + } + + for _, tc := range invalid { + if err := projects.ValidateIdentifier(tc.input); err == nil { + t.Errorf("ValidateIdentifier(%q) (%s) expected error but got nil", tc.input, tc.desc) + } + } +} diff --git a/components/resources/projects/versions.go b/components/resources/projects/versions.go index 875d991..ef84a57 100644 --- a/components/resources/projects/versions.go +++ b/components/resources/projects/versions.go @@ -8,7 +8,7 @@ import ( "github.com/opf/openproject-cli/models" ) -func AvailableVersions(projectId uint64) ([]*models.Version, error) { +func AvailableVersions(projectId string) ([]*models.Version, error) { response, err := requests.Get(paths.ProjectVersions(projectId), nil) if err != nil { return nil, err diff --git a/components/resources/time_entries/activity_error_test.go b/components/resources/time_entries/activity_error_test.go new file mode 100644 index 0000000..79bd0fb --- /dev/null +++ b/components/resources/time_entries/activity_error_test.go @@ -0,0 +1,50 @@ +package time_entries + +import ( + stderrors "errors" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" +) + +// An unresolved --activity is already reported inside the resource layer, so +// Create must return ErrHandled (not a fresh error the caller re-prints) and +// must not POST the time entry. +func TestCreateUnknownActivityReturnsErrHandled(t *testing.T) { + postCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + switch { + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/time_entries/activities": + _, _ = response.Write([]byte(`{"_embedded":{"elements":[{"id":1,"name":"Development"}]}}`)) + case request.Method == http.MethodPost: + postCount++ + _, _ = response.Write([]byte(`{"id":1}`)) + default: + http.Error(response, "unexpected request", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = Create(map[CreateOption]string{ + CreateActivity: "no-such-activity", + }) + if !stderrors.Is(err, openerrors.ErrHandled) { + t.Fatalf("Create error = %v, want ErrHandled", err) + } + if postCount != 0 { + t.Errorf("POST count = %d, want 0", postCount) + } +} diff --git a/components/resources/time_entries/create.go b/components/resources/time_entries/create.go new file mode 100644 index 0000000..8095127 --- /dev/null +++ b/components/resources/time_entries/create.go @@ -0,0 +1,214 @@ +package time_entries + +import ( + "bytes" + "encoding/json" + "fmt" + "math" + "strconv" + "strings" + "time" + + openerrors "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" +) + +type CreateOption int + +const ( + CreateWorkPackage CreateOption = iota + CreateHours + CreateActivity + CreateSpentOn + CreateUser + CreateComment +) + +var createMap = map[CreateOption]func(entry *dtos.TimeEntryDto, input string) error{ + CreateWorkPackage: workPackageCreate, + CreateHours: hoursCreate, + CreateActivity: activityCreate, + CreateSpentOn: spentOnCreate, + CreateUser: userCreate, + CreateComment: commentCreate, +} + +// input is validated by the caller via work_packages.ValidateIdentifier before reaching here +func workPackageCreate(entry *dtos.TimeEntryDto, input string) error { + if entry.Links == nil { + entry.Links = &dtos.TimeEntryLinksDto{} + } + entry.Links.WorkPackage = &dtos.LinkDto{Href: paths.WorkPackage(input)} + return nil +} + +func hoursCreate(entry *dtos.TimeEntryDto, input string) error { + hours, err := strconv.ParseFloat(input, 64) + if err != nil { + return fmt.Errorf("invalid hours %q: must be a number", input) + } + if math.IsNaN(hours) || math.IsInf(hours, 0) { + return fmt.Errorf("invalid hours %q: must be a finite number", input) + } + if hours <= 0 { + return fmt.Errorf("hours must be greater than 0") + } + duration := hoursToISO8601(hours) + if duration == "PT0S" { + return fmt.Errorf("hours %q is too small: the minimum bookable duration is one second", input) + } + entry.Hours = duration + return nil +} + +// hoursToISO8601 formats a number of hours as an ISO 8601 time-only duration +// (e.g. PT1H30M, PT45M, PT8H). Seconds are included so sub-minute entries are +// not silently rounded away to a zero-length duration. Hours are kept as a flat +// hour count (no day/week roll-up) because OpenProject time entries are +// expressed in hours. +func hoursToISO8601(hours float64) string { + totalSeconds := int64(math.Round(hours * 3600)) + h := totalSeconds / 3600 + m := (totalSeconds % 3600) / 60 + s := totalSeconds % 60 + + out := "PT" + if h > 0 { + out += fmt.Sprintf("%dH", h) + } + if m > 0 { + out += fmt.Sprintf("%dM", m) + } + if s > 0 { + out += fmt.Sprintf("%dS", s) + } + if out == "PT" { + return "PT0S" + } + return out +} + +func activityCreate(entry *dtos.TimeEntryDto, input string) error { + activities, err := AllActivities() + if err != nil { + printer.ErrorText(fmt.Sprintf("Could not fetch available activities: %s", err)) + printer.Info("Use `op time-entry list` to see existing entries and their activities.") + return openerrors.ErrHandled + } + + if strings.TrimSpace(input) == "" { + return fmt.Errorf("activity cannot be empty") + } + + found, candidates := findActivity(input, activities) + if found == nil && len(candidates) > 1 { + printer.ErrorText(fmt.Sprintf("Activity %q is ambiguous.", input)) + printer.Info("Matching activities:") + for _, a := range candidates { + printer.Info(fmt.Sprintf(" - %s", a.Name)) + } + return openerrors.ErrHandled + } + if found == nil { + printer.ErrorText(fmt.Sprintf("No activity matching %q found.", input)) + if len(activities) > 0 { + printer.Info("Available activities:") + for _, a := range activities { + printer.Info(fmt.Sprintf(" - %s", a.Name)) + } + } + return openerrors.ErrHandled + } + + if entry.Links == nil { + entry.Links = &dtos.TimeEntryLinksDto{} + } + entry.Links.Activity = &dtos.LinkDto{Href: found.Href} + return nil +} + +// findActivity resolves input to an activity: an exact (case-insensitive) +// name match wins; otherwise a prefix match is accepted only when unique. +// When the prefix is ambiguous it returns nil plus all candidates. +func findActivity(input string, activities []*models.TimeEntryActivity) (*models.TimeEntryActivity, []*models.TimeEntryActivity) { + lower := strings.ToLower(input) + for _, a := range activities { + if strings.ToLower(a.Name) == lower { + return a, nil + } + } + + var candidates []*models.TimeEntryActivity + for _, a := range activities { + if strings.HasPrefix(strings.ToLower(a.Name), lower) { + candidates = append(candidates, a) + } + } + if len(candidates) == 1 { + return candidates[0], nil + } + return nil, candidates +} + +func spentOnCreate(entry *dtos.TimeEntryDto, input string) error { + if _, err := time.Parse(time.DateOnly, input); err != nil { + return fmt.Errorf("invalid date %q: expected format YYYY-MM-DD", input) + } + entry.SpentOn = input + return nil +} + +func userCreate(entry *dtos.TimeEntryDto, input string) error { + id, err := strconv.ParseUint(input, 10, 64) + if err != nil { + return fmt.Errorf("invalid user id %q: must be a number", input) + } + if entry.Links == nil { + entry.Links = &dtos.TimeEntryLinksDto{} + } + entry.Links.User = &dtos.LinkDto{Href: paths.User(id)} + return nil +} + +func commentCreate(entry *dtos.TimeEntryDto, input string) error { + entry.Comment = &dtos.LongTextDto{Format: "plain", Raw: input} + return nil +} + +func AllActivities() ([]*models.TimeEntryActivity, error) { + response, err := requests.Get(paths.TimeEntryActivities(), nil) + if err != nil { + return nil, err + } + element := parser.Parse[dtos.TimeEntryActivityCollectionDto](response) + return element.Convert(), nil +} + +func Create(options map[CreateOption]string) (*models.TimeEntry, error) { + entry := &dtos.TimeEntryDto{} + + for option, value := range options { + if err := createMap[option](entry, value); err != nil { + return nil, err + } + } + + data, err := json.Marshal(entry) + if err != nil { + return nil, err + } + + requestData := requests.RequestData{ContentType: "application/json", Body: bytes.NewReader(data)} + response, err := requests.Post(paths.TimeEntries(), &requestData) + if err != nil { + return nil, err + } + + result := parser.Parse[dtos.TimeEntryDto](response) + return result.Convert(), nil +} diff --git a/components/resources/time_entries/create_test.go b/components/resources/time_entries/create_test.go new file mode 100644 index 0000000..aeb242f --- /dev/null +++ b/components/resources/time_entries/create_test.go @@ -0,0 +1,57 @@ +package time_entries + +// Regression tests for review finding #4 on PR #15, asserting the FIXED +// behaviour of hoursToISO8601: correct ISO 8601 output that round-trips through +// github.com/sosodev/duration, including sub-minute precision (no silent loss). + +import ( + "math" + "testing" + + "github.com/sosodev/duration" +) + +func TestHoursToISO8601_FormatAndRoundTrip(t *testing.T) { + cases := []struct { + hours float64 + want string + }{ + {1.0, "PT1H"}, + {1.5, "PT1H30M"}, + {0.25, "PT15M"}, + {0.5, "PT30M"}, + {8.0, "PT8H"}, + {2.75, "PT2H45M"}, + {25.0, "PT25H"}, // flat hours, no day roll-up + } + + for _, c := range cases { + got := hoursToISO8601(c.hours) + if got != c.want { + t.Errorf("hoursToISO8601(%g) = %q, want %q", c.hours, got, c.want) + } + + parsed, err := duration.Parse(got) + if err != nil { + t.Errorf("library cannot parse output %q: %v", got, err) + continue + } + if rt := parsed.ToTimeDuration().Hours(); math.Abs(rt-c.hours) > 1e-9 { + t.Errorf("round-trip of %q = %g hours, want %g", got, rt, c.hours) + } + } +} + +// A positive sub-minute entry must no longer be rounded away to zero. +func TestHoursToISO8601_SubMinutePreserved(t *testing.T) { + const tiny = 0.001 // 3.6 seconds + got := hoursToISO8601(tiny) + + parsed, err := duration.Parse(got) + if err != nil { + t.Fatalf("library cannot parse %q: %v", got, err) + } + if rt := parsed.ToTimeDuration().Hours(); rt <= 0 { + t.Errorf("sub-minute input %g formatted as %q -> %g hours; expected > 0", tiny, got, rt) + } +} diff --git a/components/resources/users/functions.go b/components/resources/users/functions.go index 65841d2..8a517ab 100644 --- a/components/resources/users/functions.go +++ b/components/resources/users/functions.go @@ -4,16 +4,15 @@ import ( "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" "github.com/opf/openproject-cli/components/requests" "github.com/opf/openproject-cli/components/resources" "github.com/opf/openproject-cli/dtos" "github.com/opf/openproject-cli/models" ) -func ByIds(ids []uint64) []*models.User { +func ByIds(ids []uint64) ([]*models.User, error) { if len(ids) == 0 { - return []*models.User{} + return []*models.User{}, nil } var filters []requests.Filter filters = append(filters, IdFilter(ids)) @@ -24,11 +23,11 @@ func ByIds(ids []uint64) []*models.User { response, err := requests.Get(requestUrl, &query) if err != nil { - printer.Error(err) + return nil, err } userCollection := parser.Parse[dtos.UserCollectionDto](response) - return userCollection.Convert() + return userCollection.Convert(), nil } func Me() (*models.User, error) { diff --git a/components/resources/work_packages/activities.go b/components/resources/work_packages/activities.go index a273982..7aacbda 100644 --- a/components/resources/work_packages/activities.go +++ b/components/resources/work_packages/activities.go @@ -3,16 +3,15 @@ package work_packages 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" ) -func Activities(id uint64) (activites []*models.Activity, err error) { +func Activities(id string) (activites []*models.Activity, err error) { response, err := requests.Get(paths.WorkPackageActivities(id), nil) if err != nil { - printer.Error(err) + return nil, err } activitiesDto := parser.Parse[dtos.ActivityCollectionDto](response) diff --git a/components/resources/work_packages/attachment_guard_test.go b/components/resources/work_packages/attachment_guard_test.go new file mode 100644 index 0000000..af86a9c --- /dev/null +++ b/components/resources/work_packages/attachment_guard_test.go @@ -0,0 +1,32 @@ +package work_packages + +import ( + "testing" + + "github.com/opf/openproject-cli/dtos" +) + +// A work package whose payload has no _links object must not panic the +// attachment validation; it should be reported as not accepting attachments. +func TestValidateAttachmentNilLinks(t *testing.T) { + err := validateAttachment(&dtos.WorkPackageDto{Links: nil}, "irrelevant") + if err == nil { + t.Fatal("validateAttachment(nil Links) = nil, want error") + } +} + +// upload must self-guard: the work package may be re-fetched after a custom +// action, dropping the addAttachment link that validateAttachment saw earlier. +func TestUploadNilAddAttachment(t *testing.T) { + cases := map[string]*dtos.WorkPackageDto{ + "nil links": {Links: nil}, + "nil add attachment": {Links: &dtos.WorkPackageLinksDto{}}, + } + for name, dto := range cases { + t.Run(name, func(t *testing.T) { + if err := upload(dto, "irrelevant"); err == nil { + t.Errorf("upload(%s) = nil, want error", name) + } + }) + } +} diff --git a/components/resources/work_packages/create.go b/components/resources/work_packages/create.go index 65e5118..da313eb 100644 --- a/components/resources/work_packages/create.go +++ b/components/resources/work_packages/create.go @@ -4,7 +4,9 @@ import ( "bytes" "encoding/json" "fmt" + "strconv" + openerrors "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" @@ -18,20 +20,24 @@ type CreateOption int const ( CreateSubject CreateOption = iota CreateType + CreateAssignee + CreateDescription ) -var createMap = map[CreateOption]func(projectId uint64, workPackage *dtos.WorkPackageDto, input string) error{ - CreateSubject: subjectCreate, - CreateType: typeCreate, +var createMap = map[CreateOption]func(projectId string, workPackage *dtos.WorkPackageDto, input string) error{ + CreateSubject: subjectCreate, + CreateType: typeCreate, + CreateAssignee: assigneeCreate, + CreateDescription: descriptionCreate, } -func subjectCreate(_ uint64, workPackage *dtos.WorkPackageDto, input string) error { +func subjectCreate(_ string, workPackage *dtos.WorkPackageDto, input string) error { workPackage.Subject = input return nil } -func typeCreate(projectId uint64, workPackage *dtos.WorkPackageDto, input string) error { +func typeCreate(projectId string, workPackage *dtos.WorkPackageDto, input string) error { types, err := availableTypes(&dtos.LinkDto{Href: paths.Project(projectId)}) if err != nil { return err @@ -43,28 +49,45 @@ func typeCreate(projectId uint64, workPackage *dtos.WorkPackageDto, input string 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.Red(projectId), )) + printer.AvailableTypes(types.Convert()) + return openerrors.ErrHandled + } - printer.Types(types.Convert()) + if workPackage.Links == nil { + workPackage.Links = &dtos.WorkPackageLinksDto{} + } - return nil + workPackage.Links.Type = foundType.Links.Self + + return nil +} + +func assigneeCreate(_ string, workPackage *dtos.WorkPackageDto, input string) error { + userId, err := strconv.ParseUint(input, 10, 64) + if err != nil { + return fmt.Errorf("invalid user id %q: must be a number", input) } if workPackage.Links == nil { workPackage.Links = &dtos.WorkPackageLinksDto{} } - workPackage.Links.Type = foundType.Links.Self + workPackage.Links.Assignee = &dtos.LinkDto{Href: paths.User(userId)} + return nil +} +func descriptionCreate(_ string, workPackage *dtos.WorkPackageDto, input string) error { + workPackage.Description = &dtos.LongTextDto{Format: "markdown", Raw: input} return nil } -func Create(projectId uint64, options map[CreateOption]string) (*models.WorkPackage, error) { +func Create(projectId string, options map[CreateOption]string) (*models.WorkPackage, error) { return create(projectId, options) } -func create(projectId uint64, options map[CreateOption]string) (*models.WorkPackage, error) { +func create(projectId string, options map[CreateOption]string) (*models.WorkPackage, error) { workPackage := dtos.WorkPackageDto{} for option, value := range options { diff --git a/components/resources/work_packages/create_test.go b/components/resources/work_packages/create_test.go new file mode 100644 index 0000000..d447e35 --- /dev/null +++ b/components/resources/work_packages/create_test.go @@ -0,0 +1,91 @@ +package work_packages_test + +import ( + "errors" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestCreateRejectsInvalidTypeBeforePosting(t *testing.T) { + postCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + switch { + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1": + _, _ = response.Write([]byte(`{"_links":{"types":{"href":"/api/v3/projects/1/types"}}}`)) + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1/types": + _, _ = response.Write([]byte(`{"_embedded":{"elements":[]}}`)) + case request.Method == http.MethodPost: + postCount++ + _, _ = response.Write([]byte(`{"id":42}`)) + default: + http.Error(response, "unexpected request", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Create("1", map[work_packages.CreateOption]string{ + work_packages.CreateSubject: "Subject", + work_packages.CreateType: "Missing", + }) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("Create error = %v, want ErrHandled", err) + } + if postCount != 0 { + t.Errorf("POST count = %d, want 0", postCount) + } +} + +// On an unresolved --type, the "available types" hint is guidance and must go +// to stderr, never to stdout, so --format json output stays clean. +func TestCreateInvalidTypeHintGoesToStderr(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + switch { + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1": + _, _ = response.Write([]byte(`{"_links":{"types":{"href":"/api/v3/projects/1/types"}}}`)) + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1/types": + _, _ = response.Write([]byte(`{"_embedded":{"elements":[{"id":7,"name":"Task"}]}}`)) + default: + http.Error(response, "unexpected request", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + testingPrinter := &printer.TestingPrinter{} + printer.Init(testingPrinter) + + _, err = work_packages.Create("1", map[work_packages.CreateOption]string{ + work_packages.CreateSubject: "Subject", + work_packages.CreateType: "Missing", + }) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("Create error = %v, want ErrHandled", err) + } + if testingPrinter.Result != "" { + t.Errorf("stdout = %q, want empty on failed create", testingPrinter.Result) + } + if !strings.Contains(testingPrinter.ErrResult, "Task") { + t.Errorf("stderr = %q, want it to list available type %q", testingPrinter.ErrResult, "Task") + } +} diff --git a/components/resources/work_packages/custom_actions.go b/components/resources/work_packages/custom_actions.go index 0203882..1732cc8 100644 --- a/components/resources/work_packages/custom_actions.go +++ b/components/resources/work_packages/custom_actions.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/opf/openproject-cli/components/common" + openerrors "github.com/opf/openproject-cli/components/errors" "github.com/opf/openproject-cli/components/parser" "github.com/opf/openproject-cli/components/printer" "github.com/opf/openproject-cli/components/requests" @@ -14,7 +15,7 @@ import ( "github.com/opf/openproject-cli/models" ) -func action(workPackage *dtos.WorkPackageDto, action string) error { +func resolveAction(workPackage *dtos.WorkPackageDto, action string) (*dtos.CustomActionDto, error) { foundAction := findAction(action, workPackage.Embedded.CustomActions) if foundAction == nil { printer.ErrorText("Failed to execute work package custom action.") @@ -31,10 +32,10 @@ func action(workPackage *dtos.WorkPackageDto, action string) error { []*models.CustomAction{}, ) printer.CustomActions(availableActions) - return nil + return nil, openerrors.ErrHandled } - return executeAction(workPackage, foundAction) + return foundAction, nil } func findAction(actionInput string, availableActions []*dtos.CustomActionDto) *dtos.CustomActionDto { diff --git a/components/resources/work_packages/filters.go b/components/resources/work_packages/filters.go index 19a5cc3..dec6189 100644 --- a/components/resources/work_packages/filters.go +++ b/components/resources/work_packages/filters.go @@ -14,6 +14,7 @@ const ( Status Type IncludeSubProjects + Parent ) var InputValidationExpression = map[FilterOption]string{ @@ -33,11 +34,21 @@ func (f FilterOption) String() string { return "type" case IncludeSubProjects: return "include-sub-projects" + case Parent: + return "parent-id" default: return "filter" } } +func ParentFilter(id string) requests.Filter { + return requests.Filter{ + Operator: "=", + Name: "parent", + Values: []string{id}, + } +} + func AssigneeFilter(name string) requests.Filter { return requests.Filter{ Operator: "=", diff --git a/components/resources/work_packages/read.go b/components/resources/work_packages/read.go index 03b9357..cb90400 100644 --- a/components/resources/work_packages/read.go +++ b/components/resources/work_packages/read.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" @@ -10,7 +8,7 @@ import ( "github.com/opf/openproject-cli/models" ) -func Lookup(id uint64) (*models.WorkPackage, error) { +func Lookup(id string) (*models.WorkPackage, error) { workPackage, err := fetch(id) if err != nil { return nil, err @@ -21,7 +19,7 @@ func Lookup(id uint64) (*models.WorkPackage, error) { func All(filterOptions *map[FilterOption]string, query requests.Query, showOnlyTotal bool) (*models.WorkPackageCollection, error) { var filters []requests.Filter - var projectId *uint64 + var projectId *string var queryAttributes = make(map[string]string) for updateOpt, value := range *filterOptions { @@ -35,8 +33,9 @@ func All(filterOptions *map[FilterOption]string, query requests.Query, showOnlyT case Type: filters = append(filters, TypeFilter(value)) case Project: - n, _ := strconv.ParseUint(value, 10, 64) - projectId = &n + projectId = &value + case Parent: + filters = append(filters, ParentFilter(value)) } } @@ -64,7 +63,7 @@ func All(filterOptions *map[FilterOption]string, query requests.Query, showOnlyT return workPackageCollection.Convert(), nil } -func AvailableTypes(id uint64) ([]*models.Type, error) { +func AvailableTypes(id string) ([]*models.Type, error) { workPackageDto, err := fetch(id) if err != nil { return nil, err @@ -78,7 +77,7 @@ func AvailableTypes(id uint64) ([]*models.Type, error) { return types.Convert(), nil } -func fetch(id uint64) (*dtos.WorkPackageDto, error) { +func fetch(id string) (*dtos.WorkPackageDto, error) { response, err := requests.Get(paths.WorkPackage(id), nil) if err != nil { return nil, err diff --git a/components/resources/work_packages/search.go b/components/resources/work_packages/search.go new file mode 100644 index 0000000..21a57e3 --- /dev/null +++ b/components/resources/work_packages/search.go @@ -0,0 +1,28 @@ +package work_packages + +import ( + "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" + "github.com/opf/openproject-cli/dtos" + "github.com/opf/openproject-cli/models" +) + +func Search(input string, projectId string) ([]*models.WorkPackage, error) { + filters := []requests.Filter{resources.TypeAheadFilter(input)} + query := requests.NewFilterQuery(filters) + + requestUrl := paths.WorkPackages() + if projectId != "" { + requestUrl = paths.ProjectWorkPackages(projectId) + } + + response, err := requests.Get(requestUrl, &query) + if err != nil { + return nil, err + } + + collection := parser.Parse[dtos.WorkPackageCollectionDto](response) + return collection.Convert().Items, nil +} diff --git a/components/resources/work_packages/update.go b/components/resources/work_packages/update.go index 7cc751d..3cf5568 100644 --- a/components/resources/work_packages/update.go +++ b/components/resources/work_packages/update.go @@ -4,9 +4,11 @@ import ( "bytes" "encoding/json" "fmt" + "os" "strconv" "github.com/opf/openproject-cli/components/common" + openerrors "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" @@ -21,46 +23,69 @@ const ( UpdateCustomAction UpdateOption = iota UpdateAssignee UpdateAttachment + UpdateDescription UpdateSubject 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, + UpdateDescription: descriptionPatch, + UpdateType: typePatch, + UpdateSubject: subjectPatch, } -func Update(id uint64, options map[UpdateOption]string) (*models.WorkPackage, error) { +func Update(id string, options map[UpdateOption]string) (*models.WorkPackage, error) { workPackage, err := fetch(id) if err != nil { return nil, err } - if customAction, ok := options[UpdateCustomAction]; ok { - err = action(workPackage, customAction) + var customAction *dtos.CustomActionDto + if input, ok := options[UpdateCustomAction]; ok { + customAction, err = resolveAction(workPackage, input) 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 } } - err = patch(workPackage, options) + patchDto, updateString, patchNeeded, err := preparePatch(workPackage, options) if err != nil { - printer.Error(err) + return nil, err + } + + if file, ok := options[UpdateAttachment]; ok { + if err := validateAttachment(workPackage, file); err != nil { + return nil, err + } + } + + if customAction != nil { + err = executeAction(workPackage, customAction) + if err != nil { + return nil, err + } + + // reload work package to get new lock version + workPackage, err = fetch(id) + if err != nil { + return nil, err + } + } + + if patchNeeded { + patchDto.LockVersion = workPackage.LockVersion + if err := executePatch(workPackage, patchDto, updateString); err != nil { + return nil, err + } } if file, ok := options[UpdateAttachment]; ok { err = upload(workPackage, file) if err != nil { - printer.Error(err) + return nil, err } } @@ -72,9 +97,9 @@ func Update(id uint64, options map[UpdateOption]string) (*models.WorkPackage, er return workPackage.Convert(), nil } -func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) error { +func preparePatch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) (*dtos.WorkPackageDto, string, bool, error) { var patchNeeded = false - patchDto := dtos.WorkPackageDto{LockVersion: workPackage.LockVersion} + patchDto := &dtos.WorkPackageDto{} var updateString string for option, value := range options { @@ -83,9 +108,9 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er } patchNeeded = true - updateStringLine, err := patchMap[option](&patchDto, workPackage, value) + updateStringLine, err := patchMap[option](patchDto, workPackage, value) if err != nil { - return err + return nil, "", false, err } if len(updateStringLine) > 0 { @@ -96,12 +121,11 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er } } - if !patchNeeded { - return nil - } + return patchDto, updateString, patchNeeded, nil +} - printer.Info(fmt.Sprintf("Updating work package with patch ...")) - printer.Info(updateString) +func executePatch(workPackage, patchDto *dtos.WorkPackageDto, updateString string) error { + printer.Info("Updating work package with patch ...") marshal, err := json.Marshal(patchDto) if err != nil { @@ -113,10 +137,38 @@ func patch(workPackage *dtos.WorkPackageDto, options map[UpdateOption]string) er return err } + printer.Info(updateString) printer.Done() return nil } +func validateAttachment(workPackage *dtos.WorkPackageDto, path string) error { + if workPackage.Links == nil { + return fmt.Errorf("this work package does not accept attachments (missing permission?)") + } + if workPackage.Links.PrepareAttachment != nil { + return fmt.Errorf("uploads to fog storages are currently not supported") + } + if workPackage.Links.AddAttachment == nil { + return fmt.Errorf("this work package does not accept attachments (missing permission?)") + } + + file, err := os.Open(path) + if err != nil { + return err + } + defer func() { _ = file.Close() }() + + info, err := file.Stat() + if err != nil { + return err + } + if info.IsDir() { + return fmt.Errorf("attachment path %q is a directory", path) + } + return nil +} + func typePatch(patch, workPackage *dtos.WorkPackageDto, input string) (string, error) { types, err := availableTypes(workPackage.Links.Project) if err != nil { @@ -132,9 +184,9 @@ func typePatch(patch, workPackage *dtos.WorkPackageDto, input string) (string, e printer.Red(fmt.Sprintf("#%d", parser.IdFromLink(workPackage.Links.Project.Href))), )) - printer.Types(types.Convert()) + printer.AvailableTypes(types.Convert()) - return "", nil + return "", openerrors.ErrHandled } if patch.Links == nil { @@ -150,8 +202,16 @@ func subjectPatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { return fmt.Sprintf("Subject -> %s", input), nil } +func descriptionPatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { + patch.Description = &dtos.LongTextDto{Format: "markdown", Raw: input} + return "Description updated", nil +} + func assigneePatch(patch, _ *dtos.WorkPackageDto, input string) (string, error) { - userId, _ := strconv.ParseUint(input, 10, 64) + userId, err := strconv.ParseUint(input, 10, 64) + if err != nil { + return "", fmt.Errorf("invalid user id %q: must be a number", input) + } if patch.Links == nil { patch.Links = &dtos.WorkPackageLinksDto{} diff --git a/components/resources/work_packages/update_test.go b/components/resources/work_packages/update_test.go new file mode 100644 index 0000000..329edce --- /dev/null +++ b/components/resources/work_packages/update_test.go @@ -0,0 +1,413 @@ +package work_packages_test + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + + openerrors "github.com/opf/openproject-cli/components/errors" + "github.com/opf/openproject-cli/components/printer" + "github.com/opf/openproject-cli/components/requests" + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestUpdateValidatesTypeBeforeExecutingAction(t *testing.T) { + mutationCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + + switch { + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/work_packages/42": + _, _ = response.Write([]byte(`{ + "id": 42, + "lockVersion": 1, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"} + }, + "_embedded": { + "customActions": [{ + "name": "Claim", + "_links": { + "self": {"href": "/api/v3/custom_actions/1"}, + "executeImmediately": { + "href": "/api/v3/custom_actions/1/execute", + "method": "POST" + } + } + }] + } + }`)) + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1": + _, _ = response.Write([]byte(`{"_links":{"types":{"href":"/api/v3/projects/1/types"}}}`)) + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1/types": + _, _ = response.Write([]byte(`{"_embedded":{"elements":[]}}`)) + case request.Method == http.MethodPost || request.Method == http.MethodPatch: + mutationCount++ + _, _ = response.Write([]byte(`{}`)) + default: + http.Error(response, "unexpected request", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateCustomAction: "Claim", + work_packages.UpdateType: "Missing", + }) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("Update error = %v, want ErrHandled", err) + } + if mutationCount != 0 { + t.Errorf("mutation count = %d, want 0", mutationCount) + } +} + +func TestUpdateValidatesAttachmentBeforeExecutingAction(t *testing.T) { + mutationCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + + if request.Method == http.MethodPost || request.Method == http.MethodPatch { + mutationCount++ + _, _ = response.Write([]byte(`{}`)) + return + } + + _, _ = response.Write([]byte(`{ + "id": 42, + "lockVersion": 1, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"}, + "addAttachment": { + "href": "/api/v3/work_packages/42/attachments", + "method": "POST" + } + }, + "_embedded": { + "customActions": [{ + "name": "Claim", + "_links": { + "self": {"href": "/api/v3/custom_actions/1"}, + "executeImmediately": { + "href": "/api/v3/custom_actions/1/execute", + "method": "POST" + } + } + }] + } + }`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateCustomAction: "Claim", + work_packages.UpdateAttachment: "/file/does/not/exist", + }) + if err == nil { + t.Fatal("Update returned nil, want an attachment validation error") + } + if mutationCount != 0 { + t.Errorf("mutation count = %d, want 0", mutationCount) + } +} + +func TestUpdateValidatesActionBeforePatching(t *testing.T) { + mutationCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + if request.Method == http.MethodPatch || request.Method == http.MethodPost { + mutationCount++ + } + _, _ = response.Write([]byte(`{ + "id": 42, + "lockVersion": 1, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"} + }, + "_embedded": {"customActions": []} + }`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateCustomAction: "Missing", + work_packages.UpdateSubject: "Changed", + }) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("Update error = %v, want ErrHandled", err) + } + if mutationCount != 0 { + t.Errorf("mutation count = %d, want 0", mutationCount) + } +} + +func TestUpdateUsesRefetchedLockVersionAfterAction(t *testing.T) { + fetchCount := 0 + mutationOrder := []string{} + patchedLockVersion := 0 + patchedSubject := "" + + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + switch { + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/work_packages/42": + fetchCount++ + _, _ = fmt.Fprintf(response, `{ + "id": 42, + "subject": "Before", + "lockVersion": %d, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"} + }, + "_embedded": { + "customActions": [{ + "name": "Claim", + "_links": { + "self": {"href": "/api/v3/custom_actions/1"}, + "executeImmediately": { + "href": "/api/v3/custom_actions/1/execute", + "method": "POST" + } + } + }] + } + }`, fetchCount) + case request.Method == http.MethodPost && request.URL.Path == "/api/v3/custom_actions/1/execute": + mutationOrder = append(mutationOrder, "action") + _, _ = response.Write([]byte(`{}`)) + case request.Method == http.MethodPatch && request.URL.Path == "/api/v3/work_packages/42": + mutationOrder = append(mutationOrder, "patch") + var body struct { + LockVersion int `json:"lockVersion"` + Subject string `json:"subject"` + } + if err := json.NewDecoder(request.Body).Decode(&body); err != nil { + t.Errorf("decode PATCH body: %v", err) + } + patchedLockVersion = body.LockVersion + patchedSubject = body.Subject + _, _ = response.Write([]byte(`{}`)) + default: + http.Error(response, "unexpected request", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateCustomAction: "Claim", + work_packages.UpdateSubject: "Changed", + }) + if err != nil { + t.Fatalf("Update returned an error: %v", err) + } + if got := fmt.Sprint(mutationOrder); got != "[action patch]" { + t.Errorf("mutation order = %s, want [action patch]", got) + } + if patchedLockVersion != 2 { + t.Errorf("PATCH lockVersion = %d, want refetched version 2", patchedLockVersion) + } + if patchedSubject != "Changed" { + t.Errorf("PATCH subject = %q, want Changed", patchedSubject) + } +} + +func TestUpdateRejectsInvalidTypeWithoutAction(t *testing.T) { + mutationCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + + switch { + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/work_packages/42": + _, _ = response.Write([]byte(`{ + "id": 42, + "lockVersion": 1, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"} + }, + "_embedded": {"customActions": []} + }`)) + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1": + _, _ = response.Write([]byte(`{"_links":{"types":{"href":"/api/v3/projects/1/types"}}}`)) + case request.Method == http.MethodGet && request.URL.Path == "/api/v3/projects/1/types": + _, _ = response.Write([]byte(`{"_embedded":{"elements":[]}}`)) + case request.Method == http.MethodPost || request.Method == http.MethodPatch: + mutationCount++ + _, _ = response.Write([]byte(`{}`)) + default: + http.Error(response, "unexpected request", http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateSubject: "Changed", + work_packages.UpdateType: "Missing", + }) + if !errors.Is(err, openerrors.ErrHandled) { + t.Fatalf("Update error = %v, want ErrHandled", err) + } + if mutationCount != 0 { + t.Errorf("mutation count = %d, want 0", mutationCount) + } +} + +func TestUpdateRejectsAttachmentWithoutAddAttachmentLink(t *testing.T) { + mutationCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + + if request.Method == http.MethodPost || request.Method == http.MethodPatch { + mutationCount++ + _, _ = response.Write([]byte(`{}`)) + return + } + + _, _ = response.Write([]byte(`{ + "id": 42, + "lockVersion": 1, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"} + }, + "_embedded": { + "customActions": [{ + "name": "Claim", + "_links": { + "self": {"href": "/api/v3/custom_actions/1"}, + "executeImmediately": { + "href": "/api/v3/custom_actions/1/execute", + "method": "POST" + } + } + }] + } + }`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + attachment := filepath.Join(t.TempDir(), "attachment.txt") + if err := os.WriteFile(attachment, []byte("content"), 0600); err != nil { + t.Fatal(err) + } + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateCustomAction: "Claim", + work_packages.UpdateAttachment: attachment, + }) + if err == nil { + t.Fatal("Update returned nil, want a missing addAttachment link error") + } + if !strings.Contains(err.Error(), "does not accept attachments") { + t.Errorf("error should mention missing attachment capability, got: %v", err) + } + if mutationCount != 0 { + t.Errorf("mutation count = %d, want 0", mutationCount) + } +} + +func TestUpdateRejectsFogStorageAttachments(t *testing.T) { + mutationCount := 0 + server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Content-Type", "application/json") + + if request.Method == http.MethodPost || request.Method == http.MethodPatch { + mutationCount++ + _, _ = response.Write([]byte(`{}`)) + return + } + + _, _ = response.Write([]byte(`{ + "id": 42, + "lockVersion": 1, + "_links": { + "self": {"href": "/api/v3/work_packages/42"}, + "project": {"href": "/api/v3/projects/1"}, + "prepareAttachment": { + "href": "/api/v3/work_packages/42/attachments/prepare", + "method": "POST" + } + }, + "_embedded": {"customActions": []} + }`)) + })) + t.Cleanup(server.Close) + + host, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + requests.Init(host, "", false) + printer.Init(&printer.TestingPrinter{}) + + attachment := filepath.Join(t.TempDir(), "attachment.txt") + if err := os.WriteFile(attachment, []byte("content"), 0600); err != nil { + t.Fatal(err) + } + + _, err = work_packages.Update("42", map[work_packages.UpdateOption]string{ + work_packages.UpdateAttachment: attachment, + }) + if err == nil { + t.Fatal("Update returned nil, want a fog storage error") + } + if !strings.Contains(err.Error(), "fog storages") { + t.Errorf("error should mention fog storages, got: %v", err) + } + if mutationCount != 0 { + t.Errorf("mutation count = %d, want 0", mutationCount) + } +} diff --git a/components/resources/work_packages/upload.go b/components/resources/work_packages/upload.go index 96d25c8..c80d15d 100644 --- a/components/resources/work_packages/upload.go +++ b/components/resources/work_packages/upload.go @@ -13,11 +13,12 @@ import ( "github.com/opf/openproject-cli/dtos" ) +// upload assumes validateAttachment already confirmed the work package +// accepts direct uploads (addAttachment link present, no fog storage). func upload(dto *dtos.WorkPackageDto, path string) error { - if dto.Links.PrepareAttachment != nil { - printer.ErrorText(fmt.Sprintf("Uploads to fog storages are currently not supported. :(")) + if dto.Links == nil || dto.Links.AddAttachment == nil { + return fmt.Errorf("work package no longer accepts attachments") } - printer.Info(fmt.Sprintf("Uploading %s to work package ...", printer.Yellow(filepath.Base(path)))) link := dto.Links.AddAttachment reader, contentType, err := bodyReader(path) diff --git a/components/resources/work_packages/validate.go b/components/resources/work_packages/validate.go new file mode 100644 index 0000000..e3d68e6 --- /dev/null +++ b/components/resources/work_packages/validate.go @@ -0,0 +1,20 @@ +package work_packages + +import ( + "fmt" + "regexp" +) + +// matches plain numeric IDs like 12345 +var numericIdPattern = regexp.MustCompile(`^\d+$`) + +// matches project-based identifiers like PROJ-123: uppercase letter start, up to 10 chars +// (letters/digits/underscores), hyphen, numeric sequence — per OpenProject identifier rules +var semanticIdPattern = regexp.MustCompile(`^[A-Z][A-Z0-9_]{0,9}-\d+$`) + +func ValidateIdentifier(id string) error { + if numericIdPattern.MatchString(id) || semanticIdPattern.MatchString(id) { + return nil + } + return fmt.Errorf("'%s' is an invalid work package identifier: must be a numeric ID (e.g. '12345') or a project-based identifier (e.g. 'PROJ-123')", id) +} diff --git a/components/resources/work_packages/validate_test.go b/components/resources/work_packages/validate_test.go new file mode 100644 index 0000000..93b4fb0 --- /dev/null +++ b/components/resources/work_packages/validate_test.go @@ -0,0 +1,48 @@ +package work_packages_test + +import ( + "testing" + + "github.com/opf/openproject-cli/components/resources/work_packages" +) + +func TestValidateIdentifier(t *testing.T) { + valid := []string{ + "1", + "72427", + "SJF-13", + "A-1", + "MYPROJECT-100", + "MY_PROJ-5", + "ABCDEFGHIJ-1", + } + + for _, id := range valid { + if err := work_packages.ValidateIdentifier(id); err != nil { + t.Errorf("ValidateIdentifier(%q) returned unexpected error: %v", id, err) + } + } + + invalid := []struct { + input string + desc string + }{ + {"", "empty string"}, + {"abc", "lowercase letters only"}, + {"sjf-13", "lowercase project identifier"}, + {"1234-5", "project identifier starts with digit"}, + {"SJF-abc", "non-numeric sequence number"}, + {"SJF-", "missing sequence number"}, + {"-13", "missing project identifier"}, + {"SJF_13", "underscore instead of hyphen separator"}, + {"ABCDEFGHIJK-1", "project identifier exceeds 10 characters"}, + {"SJF 13", "space"}, + {"SJ+F-13", "plus sign in project identifier"}, + } + + for _, tc := range invalid { + if err := work_packages.ValidateIdentifier(tc.input); err == nil { + t.Errorf("ValidateIdentifier(%q) (%s) expected error but got nil", tc.input, tc.desc) + } + } +} diff --git a/components/routes/routes.go b/components/routes/routes.go index a761ecd..c000a61 100644 --- a/components/routes/routes.go +++ b/components/routes/routes.go @@ -3,6 +3,7 @@ package routes import ( "fmt" "net/url" + "strconv" "github.com/opf/openproject-cli/models" ) @@ -15,12 +16,24 @@ func Init(h *url.URL) { func WorkPackageUrl(workPackage *models.WorkPackage) *url.URL { routeUrl := *host - routeUrl.Path = fmt.Sprintf("work_packages/%d", workPackage.Id) + // Current servers always set displayId (`identifier.presence || id`), but + // older versions omit it, so fall back to the numeric id. + displayId := workPackage.DisplayId + if displayId == "" { + displayId = strconv.FormatUint(workPackage.Id, 10) + } + routeUrl.Path = fmt.Sprintf("wp/%s", displayId) return &routeUrl } func ProjectUrl(project *models.Project) *url.URL { routeUrl := *host - routeUrl.Path = fmt.Sprintf("projects/%d", project.Id) + // Current servers always set identifier, but fall back to the numeric id + // (also a valid /projects/ address) when it is missing. + identifier := project.Identifier + if identifier == "" { + identifier = strconv.FormatUint(project.Id, 10) + } + routeUrl.Path = fmt.Sprintf("projects/%s", identifier) return &routeUrl } diff --git a/components/routes/routes_test.go b/components/routes/routes_test.go new file mode 100644 index 0000000..b6bd38a --- /dev/null +++ b/components/routes/routes_test.go @@ -0,0 +1,40 @@ +package routes_test + +import ( + "net/url" + "testing" + + "github.com/opf/openproject-cli/components/routes" + "github.com/opf/openproject-cli/models" +) + +func initHost(t *testing.T) { + t.Helper() + host, err := url.Parse("https://op.example.com") + if err != nil { + t.Fatal(err) + } + routes.Init(host) +} + +func TestProjectUrl_UsesIdentifierWhenPresent(t *testing.T) { + initHost(t) + + got := routes.ProjectUrl(&models.Project{Id: 42, Identifier: "my-project"}).String() + want := "https://op.example.com/projects/my-project" + if got != want { + t.Errorf("ProjectUrl = %q, want %q", got, want) + } +} + +// A project whose identifier is missing (older servers, minimal payloads) must +// still produce a working URL by falling back to the numeric id. +func TestProjectUrl_EmptyIdentifierFallsBackToId(t *testing.T) { + initHost(t) + + got := routes.ProjectUrl(&models.Project{Id: 42, Identifier: ""}).String() + want := "https://op.example.com/projects/42" + if got != want { + t.Errorf("ProjectUrl = %q, want %q", got, want) + } +} diff --git a/dtos/budget.go b/dtos/budget.go new file mode 100644 index 0000000..b445c32 --- /dev/null +++ b/dtos/budget.go @@ -0,0 +1,36 @@ +package dtos + +import "github.com/opf/openproject-cli/models" + +type BudgetDto struct { + Id int64 `json:"id"` + Subject string `json:"subject"` +} + +type budgetElements struct { + Elements []*BudgetDto `json:"elements"` +} + +type BudgetCollectionDto struct { + Embedded *budgetElements `json:"_embedded"` +} + +/////////////// MODEL CONVERSION /////////////// + +func (dto *BudgetCollectionDto) Convert() []*models.Budget { + if dto.Embedded == nil { + return []*models.Budget{} + } + budgets := make([]*models.Budget, len(dto.Embedded.Elements)) + for i, b := range dto.Embedded.Elements { + budgets[i] = b.Convert() + } + return budgets +} + +func (dto *BudgetDto) Convert() *models.Budget { + return &models.Budget{ + Id: uint64(dto.Id), + Subject: dto.Subject, + } +} diff --git a/dtos/custom_action.go b/dtos/custom_action.go index 18e6914..ddd34ae 100644 --- a/dtos/custom_action.go +++ b/dtos/custom_action.go @@ -30,5 +30,5 @@ func (dto *CustomActionDto) Convert() *models.CustomAction { return &models.CustomAction{ Id: parser.IdFromLink(dto.Links.Self.Href), Name: dto.Name, - } -} \ No newline at end of file + } +} diff --git a/dtos/notification.go b/dtos/notification.go index a57ecf4..297b161 100644 --- a/dtos/notification.go +++ b/dtos/notification.go @@ -48,5 +48,5 @@ func (dto *NotificationDto) Convert() *models.Notification { Read: dto.ReadIAN, CreatedAt: dto.CreatedAt, UpdatedAt: dto.UpdatedAt, - } -} \ No newline at end of file + } +} 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/time_entry.go b/dtos/time_entry.go index 851677f..ec9b9d1 100644 --- a/dtos/time_entry.go +++ b/dtos/time_entry.go @@ -15,7 +15,7 @@ type TimeEntryDto struct { Ongoing bool `json:"ongoing,omitempty"` CreatedAt string `json:"createdAt,omitempty"` UpdatedAt string `json:"updatedAt,omitempty"` - Links *timeEntryLinksDto `json:"_links,omitempty"` + Links *TimeEntryLinksDto `json:"_links,omitempty"` } type timeEntryElements struct { @@ -29,10 +29,10 @@ type TimeEntryCollectionDto struct { Count uint64 `json:"count"` } -type timeEntryLinksDto struct { - Project *LinkDto `json:"project"` +type TimeEntryLinksDto struct { + Project *LinkDto `json:"project,omitempty"` WorkPackage *LinkDto `json:"workPackage,omitempty"` - User *LinkDto `json:"user"` + User *LinkDto `json:"user,omitempty"` Activity *LinkDto `json:"activity,omitempty"` } @@ -42,16 +42,37 @@ func (dto *TimeEntryDto) Convert() *models.TimeEntry { hours, _ := duration.Parse(dto.Hours) spentOn, _ := time.Parse(time.DateOnly, dto.SpentOn) + comment := "" + if dto.Comment != nil { + comment = dto.Comment.Raw + } + + project, workPackage, user, activity := "", "", "", "" + if dto.Links != nil { + if dto.Links.Project != nil { + project = dto.Links.Project.Title + } + if dto.Links.WorkPackage != nil { + workPackage = dto.Links.WorkPackage.Title + } + if dto.Links.User != nil { + user = dto.Links.User.Title + } + if dto.Links.Activity != nil { + activity = dto.Links.Activity.Title + } + } + return &models.TimeEntry{ Id: uint64(dto.Id), - Comment: dto.Comment.Raw, - Project: dto.Links.Project.Title, - WorkPackage: dto.Links.WorkPackage.Title, + Comment: comment, + Project: project, + WorkPackage: workPackage, SpentOn: spentOn, Hours: hours.ToTimeDuration(), Ongoing: dto.Ongoing, - User: dto.Links.User.Title, - Activity: dto.Links.Activity.Title, + User: user, + Activity: activity, CreatedAt: dto.CreatedAt, UpdatedAt: dto.UpdatedAt, } diff --git a/dtos/time_entry_activity.go b/dtos/time_entry_activity.go new file mode 100644 index 0000000..a65aa05 --- /dev/null +++ b/dtos/time_entry_activity.go @@ -0,0 +1,48 @@ +package dtos + +import "github.com/opf/openproject-cli/models" + +type TimeEntryActivityDto struct { + Id uint64 `json:"id,omitempty"` + Name string `json:"name,omitempty"` + Default bool `json:"default,omitempty"` + Links *timeEntryActivityLinksDto `json:"_links,omitempty"` +} + +type timeEntryActivityElements struct { + Elements []*TimeEntryActivityDto `json:"elements"` +} + +type TimeEntryActivityCollectionDto struct { + Embedded timeEntryActivityElements `json:"_embedded"` + Type string `json:"_type"` + Total uint64 `json:"total"` + Count uint64 `json:"count"` +} + +type timeEntryActivityLinksDto struct { + Self *LinkDto `json:"self,omitempty"` +} + +/////////////// MODEL CONVERSION /////////////// + +func (dto *TimeEntryActivityDto) Convert() *models.TimeEntryActivity { + self := "" + if dto.Links != nil && dto.Links.Self != nil { + self = dto.Links.Self.Href + } + return &models.TimeEntryActivity{ + Id: dto.Id, + Name: dto.Name, + Default: dto.Default, + Href: self, + } +} + +func (dto *TimeEntryActivityCollectionDto) Convert() []*models.TimeEntryActivity { + activities := make([]*models.TimeEntryActivity, len(dto.Embedded.Elements)) + for i, a := range dto.Embedded.Elements { + activities[i] = a.Convert() + } + return activities +} diff --git a/dtos/work_package.go b/dtos/work_package.go index 8110746..9fa432f 100644 --- a/dtos/work_package.go +++ b/dtos/work_package.go @@ -17,11 +17,12 @@ type WorkPackageLinksDto struct { type WorkPackageDto struct { Id int64 `json:"id,omitempty"` + DisplayId string `json:"displayId,omitempty"` Subject string `json:"subject,omitempty"` Links *WorkPackageLinksDto `json:"_links,omitempty"` Description *LongTextDto `json:"description,omitempty"` Embedded *embeddedDto `json:"_embedded,omitempty"` - LockVersion int `json:"lockVersion,omitempty"` + LockVersion int `json:"lockVersion"` } type embeddedDto struct { @@ -48,13 +49,30 @@ type CreateWorkPackageDto struct { /////////////// MODEL CONVERSION /////////////// func (dto *WorkPackageDto) Convert() *models.WorkPackage { + var wpType, assignee, status, description string + if dto.Links != nil { + if dto.Links.Type != nil { + wpType = dto.Links.Type.Title + } + if dto.Links.Assignee != nil { + assignee = dto.Links.Assignee.Title + } + if dto.Links.Status != nil { + status = dto.Links.Status.Title + } + } + if dto.Description != nil { + description = dto.Description.Raw + } + return &models.WorkPackage{ Id: uint64(dto.Id), + DisplayId: dto.DisplayId, Subject: dto.Subject, - Type: dto.Links.Type.Title, - Assignee: dto.Links.Assignee.Title, - Status: dto.Links.Status.Title, - Description: dto.Description.Raw, + Type: wpType, + Assignee: assignee, + Status: status, + Description: description, LockVersion: dto.LockVersion, } } diff --git a/dtos/work_package_test.go b/dtos/work_package_test.go new file mode 100644 index 0000000..96921be --- /dev/null +++ b/dtos/work_package_test.go @@ -0,0 +1,25 @@ +package dtos_test + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/opf/openproject-cli/dtos" +) + +// A freshly created work package has lockVersion 0. The PATCH body must +// still contain it: omitting lockVersion makes the API respond with +// 409 UpdateConflict on every update of a fresh work package. +func TestWorkPackageDtoMarshalKeepsZeroLockVersion(t *testing.T) { + patch := dtos.WorkPackageDto{LockVersion: 0} + + data, err := json.Marshal(patch) + if err != nil { + t.Fatalf("Marshal returned error: %v", err) + } + + if !strings.Contains(string(data), `"lockVersion":0`) { + t.Errorf("expected marshaled patch to contain \"lockVersion\":0, got %s", string(data)) + } +} diff --git a/main.go b/main.go index 72659d0..0981db1 100644 --- a/main.go +++ b/main.go @@ -1,11 +1,13 @@ package main import ( + "errors" "fmt" "os" "github.com/opf/openproject-cli/cmd" "github.com/opf/openproject-cli/components/configuration" + openerrors "github.com/opf/openproject-cli/components/errors" ) var ( @@ -18,7 +20,9 @@ func main() { cliVersion := configuration.BuildCliVersion(version, commit, date) if err := cmd.Execute(cliVersion); err != nil { - _, _ = fmt.Fprintln(os.Stderr, err) + if !errors.Is(err, openerrors.ErrHandled) { + _, _ = fmt.Fprintln(os.Stderr, err) + } os.Exit(1) } } diff --git a/models/budget.go b/models/budget.go new file mode 100644 index 0000000..fe734ba --- /dev/null +++ b/models/budget.go @@ -0,0 +1,6 @@ +package models + +type Budget struct { + Id uint64 + Subject string +} 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/time_entry_activity.go b/models/time_entry_activity.go new file mode 100644 index 0000000..133e9ad --- /dev/null +++ b/models/time_entry_activity.go @@ -0,0 +1,8 @@ +package models + +type TimeEntryActivity struct { + Id uint64 + Name string + Default bool + Href string +} diff --git a/models/type.go b/models/type.go index 09fb92b..5ac843d 100644 --- a/models/type.go +++ b/models/type.go @@ -1,6 +1,6 @@ package models type Type struct { - Id uint64 + Id uint64 Name string -} \ No newline at end of file +} diff --git a/models/work_package.go b/models/work_package.go index 4799c13..629f919 100644 --- a/models/work_package.go +++ b/models/work_package.go @@ -2,6 +2,7 @@ package models type WorkPackage struct { Id uint64 + DisplayId string Subject string Type string Assignee string