diff --git a/docs/azdo_boards_work-item_relation.md b/docs/azdo_boards_work-item_relation.md index 3632d398..6188fdf2 100644 --- a/docs/azdo_boards_work-item_relation.md +++ b/docs/azdo_boards_work-item_relation.md @@ -6,6 +6,7 @@ Work with Azure Boards work item relations. * [azdo boards work-item relation add](./azdo_boards_work-item_relation_add.md) * [azdo boards work-item relation remove](./azdo_boards_work-item_relation_remove.md) +* [azdo boards work-item relation show](./azdo_boards_work-item_relation_show.md) ### See also diff --git a/docs/azdo_boards_work-item_relation_add.md b/docs/azdo_boards_work-item_relation_add.md index f319699b..1710037b 100644 --- a/docs/azdo_boards_work-item_relation_add.md +++ b/docs/azdo_boards_work-item_relation_add.md @@ -6,7 +6,11 @@ azdo boards work-item relation add [ORG:]PROJECT/ID [flags] Attach one or more relations to an existing work item. The relation type must be one of the friendly names returned by 'list-type'. Targets can -be other work items (by ID) or arbitrary artifact URLs. +be other work items (by ID, optionally prefixed with their project) or +arbitrary artifact URLs. Work items in other projects of the same +organization are resolved via 'PROJECT/ID'. Cross-organization links +are not possible by ID; use --target-url with a remote link type such +as 'Remote Related', 'Consumes From' or 'Produces For'. ### Options @@ -24,11 +28,11 @@ be other work items (by ID) or arbitrary artifact URLs. Relation type (friendly name, e.g. parent, child, related). -* `--target-id` `stringArray` +* `-T`, `--target-id` `stringArray` - Target work item ID (repeatable; comma-separated values accepted). + Target work item ID (repeatable; comma-separated; each entry is [PROJECT/]ID; ID-only targets resolve in the current project). -* `--target-url` `stringArray` +* `-u`, `--target-url` `stringArray` Target artifact URL (repeatable; comma-separated values accepted). @@ -51,6 +55,9 @@ be other work items (by ID) or arbitrary artifact URLs. # Add a parent relation to another work item azdo boards work-item relation add Fabrikam/1234 --relation-type parent --target-id 5678 +# Add a parent relation to a work item in another project of the same organization +azdo boards work-item relation add Fabrikam/1234 --relation-type parent --target-id Contoso/77 + # Add a relation to multiple work items azdo boards work-item relation add Fabrikam/1234 --relation-type related --target-id 5678,5679 diff --git a/docs/azdo_boards_work-item_relation_show.md b/docs/azdo_boards_work-item_relation_show.md new file mode 100644 index 00000000..eec5909b --- /dev/null +++ b/docs/azdo_boards_work-item_relation_show.md @@ -0,0 +1,44 @@ +## Command `azdo boards work-item relation show` + +``` +azdo boards work-item relation show [ORG:]PROJECT/ID [flags] +``` + +List all relations of an existing work item. Relation types are +displayed by their friendly name. + + +### Options + + +* `-q`, `--jq` `expression` + + Filter JSON output using a jq expression + +* `--json` `fields` + + Output JSON with the specified fields. Prefix a field with '-' to exclude it. + +* `-t`, `--template` `string` + + Format JSON output using a Go template; see "azdo help formatting" + + +### ALIASES + +- `s` + +### JSON Fields + +`_links`, `commentVersionRef`, `fields`, `id`, `relations`, `rev`, `url` + +### Examples + +```bash +# List the relations of a work item +azdo boards work-item relation show Fabrikam/1234 +``` + +### See also + +* [azdo boards work-item relation](./azdo_boards_work-item_relation.md) diff --git a/docs/azdo_help_reference.md b/docs/azdo_help_reference.md index 47dc619f..b11f6283 100644 --- a/docs/azdo_help_reference.md +++ b/docs/azdo_help_reference.md @@ -325,8 +325,8 @@ Add a relation(s) to a work item. -q, --jq expression Filter JSON output using a jq expression --json fields[=*] Output JSON with the specified fields. Prefix a field with '-' to exclude it. --relation-type string Relation type (friendly name, e.g. parent, child, related). - --target-id stringArray Target work item ID (repeatable; comma-separated values accepted). - --target-url stringArray Target artifact URL (repeatable; comma-separated values accepted). +-T, --target-id stringArray Target work item ID (repeatable; comma-separated; each entry is [PROJECT/]ID; ID-only targets resolve in the current project). +-u, --target-url stringArray Target artifact URL (repeatable; comma-separated values accepted). -t, --template string Format JSON output using a Go template; see "azdo help formatting" ``` @@ -355,6 +355,22 @@ Aliases r, rm ``` +##### `azdo boards work-item relation show [ORG:]PROJECT/ID [flags]` + +List the relations of a work item. + +``` +-q, --jq expression Filter JSON output using a jq expression + --json fields[=*] Output JSON with the specified fields. Prefix a field with '-' to exclude it. +-t, --template string Format JSON output using a Go template; see "azdo help formatting" +``` + +Aliases + +``` +s +``` + #### `azdo boards work-item show [ORG:]PROJECT/ID [flags]` Show work item details diff --git a/internal/azdo/repo.go b/internal/azdo/repo.go index 1cf667bc..ea1b852b 100644 --- a/internal/azdo/repo.go +++ b/internal/azdo/repo.go @@ -182,7 +182,6 @@ func ProjectFromURL(u *url.URL) (ProjectName, error) { } parts := strings.Split(strings.Trim(u.Path, "/"), "/") - orgInHost := strings.HasSuffix(strings.ToLower(u.Hostname()), ".visualstudio.com") for _, part := range parts { if len(strings.TrimSpace(part)) == 0 { @@ -190,32 +189,24 @@ func ProjectFromURL(u *url.URL) (ProjectName, error) { } } - var organization string - var project string - if orgInHost { - if len(parts) < 1 { - return nil, fmt.Errorf("invalid path %q", u.Path) - } - organization = strings.ToLower(strings.SplitN(u.Hostname(), ".", 2)[0]) - project = parts[0] - } else { - if len(parts) < 2 { - return nil, fmt.Errorf("invalid path %q", u.Path) - } - organization = strings.ToLower(parts[0]) - project = parts[1] + id, err := ParseURL(u, false) + if err != nil { + return nil, err + } + if id.Project == "" { + return nil, fmt.Errorf("invalid path %q", u.Path) } - hostname, err := getHostnameFromOrganization(organization) + hostname, err := getHostnameFromOrganization(id.Organization) if err != nil { return nil, err } if !strings.EqualFold(hostname, u.Hostname()) { - return nil, fmt.Errorf("hostname %q of URL does not match configured hostname %q of organization %q", u.Hostname(), hostname, organization) + return nil, fmt.Errorf("hostname %q of URL does not match configured hostname %q of organization %q", u.Hostname(), hostname, id.Organization) } - return ProjectFromName(organization + ":" + project) + return ProjectFromName(id.Organization + ":" + id.Project) } // OrganizationFromURL extracts the Azure DevOps organization from a validated URL. @@ -234,24 +225,11 @@ func OrganizationFromURL(u *url.URL) (string, error) { return "", fmt.Errorf("url %s is not a valid AzDO remote URL", u.String()) } - lowerHostname := strings.ToLower(u.Hostname()) - if strings.HasSuffix(lowerHostname, ".visualstudio.com") { - return strings.SplitN(lowerHostname, ".", 2)[0], nil - } - - parts := strings.Split(strings.Trim(u.Path, "/"), "/") - if len(parts) == 0 || strings.TrimSpace(parts[0]) == "" { - return "", fmt.Errorf("invalid path %q", u.Path) - } - - if strings.EqualFold(u.Scheme, "ssh") { - if len(parts) < 2 || strings.TrimSpace(parts[1]) == "" { - return "", fmt.Errorf("invalid path %q", u.Path) - } - return strings.ToLower(parts[1]), nil + id, err := ParseURL(u, false) + if err != nil { + return "", err } - - return strings.ToLower(parts[0]), nil + return id.Organization, nil } type RepositoryName interface { @@ -497,7 +475,10 @@ func RepositoryFromURL(u *url.URL) (Repository, error) { zap.L().Debug("validated as AzDO remote URL", zap.String("hostname", u.Hostname()), zap.String("scheme", u.Scheme), zap.String("path", u.Path)) parts := strings.SplitN(strings.Trim(u.Path, "/"), "/", 5) zap.L().Debug("split path into parts", zap.Strings("parts", parts)) - orgInHost := strings.HasSuffix(strings.ToLower(u.Hostname()), ".visualstudio.com") + // The host-style form ({org}.visualstudio.com/{project}/...) leads the + // path with the project, so its repository URLs carry one fewer segment + // than the dev.azure.com/{org}/{project}/... form. + orgInHost := IsVisualStudioHost(u.Hostname()) for _, part := range parts { if len(strings.TrimSpace(part)) == 0 { @@ -544,17 +525,13 @@ func RepositoryFromURL(u *url.URL) (Repository, error) { return nil, fmt.Errorf("unsupported scheme %q", u.Scheme) } - var organization string - var project string - if orgInHost { - organization = strings.ToLower(strings.SplitN(u.Hostname(), ".", 2)[0]) - project = parts[0] - zap.L().Debug("extracted organization/project from host style url", zap.String("organization", organization), zap.String("project", project)) - } else { - organization = strings.ToLower(parts[0]) - project = parts[1] - zap.L().Debug("extracted organization/project from path style url", zap.String("organization", organization), zap.String("project", project)) + id, err := ParseURL(u, false) + if err != nil { + return nil, err } + organization := id.Organization + project := id.Project + zap.L().Debug("extracted organization/project from url", zap.String("organization", organization), zap.String("project", project)) hostname, err := getHostnameFromOrganization(organization) if err != nil { @@ -564,7 +541,7 @@ func RepositoryFromURL(u *url.URL) (Repository, error) { if !strings.EqualFold(hostname, strings.TrimPrefix(u.Hostname(), "ssh.")) { zap.L().Debug("hostname mismatch detected", zap.String("url_hostname", u.Hostname()), zap.String("configured_hostname", hostname), zap.String("organization", organization)) - return nil, fmt.Errorf("hostname %q of URL does not match configured hostname %q of organization %q", u.Hostname(), hostname, parts[0]) + return nil, fmt.Errorf("hostname %q of URL does not match configured hostname %q of organization %q", u.Hostname(), hostname, organization) } zap.L().Debug("creating repository object", zap.String("organization", organization), zap.String("project", project), zap.String("repo", strings.TrimSuffix(parts[projectNameIdx], ".git"))) diff --git a/internal/azdo/url.go b/internal/azdo/url.go new file mode 100644 index 00000000..19fe877f --- /dev/null +++ b/internal/azdo/url.go @@ -0,0 +1,118 @@ +package azdo + +import ( + "errors" + "fmt" + "net/url" + "strings" +) + +// Sentinel errors reported by ParseURL for degenerate inputs. Callers +// classify them with errors.Is instead of comparing error text. +var ( + // ErrNotAzDO reports a URL whose hostname is neither dev.azure.com, + // ssh.dev.azure.com, nor a *.visualstudio.com subdomain. + ErrNotAzDO = errors.New("not an Azure DevOps host") + + // ErrInvalidPath reports an Azure DevOps URL whose path lacks the + // segments required to identify an organization. + ErrInvalidPath = errors.New("invalid Azure DevOps URL path") +) + +// IsVisualStudioHost reports whether hostname is a *.visualstudio.com +// subdomain (the classic DevOps host style, e.g. +// https://{organization}.visualstudio.com). The match is case-insensitive +// and mirrors the suffix check that ParseURL and RepositoryFromURL share. +func IsVisualStudioHost(hostname string) bool { + return strings.HasSuffix(strings.ToLower(hostname), ".visualstudio.com") +} + +// URLIdentity captures the organization and optional project carried by an +// Azure DevOps URL. +type URLIdentity struct { + // Organization is the organization identified by the URL. It is empty + // only for degenerate paths (with lax parsing) or unparsable URLs. + Organization string + // Project is empty when the URL carries no project segment. + Project string +} + +// invalidPathError renders like the legacy "invalid path %q" message so +// existing error-string comparisons keep passing, while Unwrap lets callers +// classify it with errors.Is(err, ErrInvalidPath). +type invalidPathError struct { + path string +} + +func (e *invalidPathError) Error() string { + return fmt.Sprintf("invalid path %q", e.path) +} + +func (e *invalidPathError) Unwrap() error { + return ErrInvalidPath +} + +// ParseURL extracts the organization and project identity from an Azure +// DevOps URL. It understands the three canonical host styles: +// +// https://{organization}.visualstudio.com/{project}/... +// https://dev.azure.com/{organization}/{project}/... +// ssh://ssh.dev.azure.com/v3/{organization}/{project}/... +// +// With lax=false, non-Azure hosts yield an error wrapping ErrNotAzDO and +// paths without an organization segment yield an error wrapping +// ErrInvalidPath. With lax=true the parser is best-effort: any hostname is +// accepted as an organization and missing segments leave the corresponding +// field empty. Strict validation of repository paths, schemes, and configured +// hostnames stays with the callers (RepositoryFromURL, ProjectFromURL, ...). +func ParseURL(u *url.URL, lax bool) (URLIdentity, error) { + if u == nil { + return URLIdentity{}, fmt.Errorf("url must not be nil") + } + hostname := strings.ToLower(u.Hostname()) + if hostname == "" { + return URLIdentity{}, fmt.Errorf("url must have a hostname") + } + parts := strings.Split(strings.Trim(u.Path, "/"), "/") + + var id URLIdentity + switch { + case IsVisualStudioHost(hostname): + // {org}.visualstudio.com/{project}/... carries the organization in + // the subdomain. + id.Organization = strings.SplitN(hostname, ".", 2)[0] + if len(parts) > 0 { + id.Project = parts[0] + } + case hostname == "dev.azure.com": + // dev.azure.com/{org}/{project}/... carries segments in the path. + if len(parts) > 0 { + id.Organization = strings.ToLower(strings.TrimSpace(parts[0])) + } + if len(parts) > 1 { + id.Project = parts[1] + } + case hostname == "ssh.dev.azure.com": + // ssh.dev.azure.com/v3/{org}/{project}/.../ skips the protocol + // version segment. + if len(parts) > 1 { + id.Organization = strings.ToLower(strings.TrimSpace(parts[1])) + } + if len(parts) > 2 { + id.Project = parts[2] + } + default: + if !lax { + return URLIdentity{}, fmt.Errorf("not an Azure DevOps host %q: %w", u.Host, ErrNotAzDO) + } + id.Organization = hostname + if len(parts) > 0 { + id.Project = parts[0] + } + } + + if !lax && id.Organization == "" { + return URLIdentity{}, &invalidPathError{path: u.Path} + } + return id, nil +} diff --git a/internal/azdo/url_test.go b/internal/azdo/url_test.go new file mode 100644 index 00000000..b0e09a27 --- /dev/null +++ b/internal/azdo/url_test.go @@ -0,0 +1,152 @@ +package azdo + +import ( + "errors" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseURL(t *testing.T) { + t.Parallel() + + parse := func(raw string) *url.URL { + u, err := url.Parse(raw) + require.NoError(t, err) + return u + } + + tests := []struct { + name string + raw string + lax bool + nilURL bool + wantOrg string + wantProject string + wantErr error + wantErrText string + }{ + { + name: "dev.azure.com with project", + raw: "https://dev.azure.com/defaultorg/monalisa/_git/octo-cat", + wantOrg: "defaultorg", + wantProject: "monalisa", + }, + { + name: "dev.azure.com org-only", + raw: "https://dev.azure.com/defaultorg", + wantOrg: "defaultorg", + }, + { + name: "dev.azure.com empty path", + raw: "https://dev.azure.com", + wantErr: ErrInvalidPath, + }, + { + name: "dev.azure.com trailing slash", + raw: "https://dev.azure.com/", + wantErr: ErrInvalidPath, + wantErrText: `invalid path "/"`, + }, + { + name: "visualstudio.com with project", + raw: "https://vsorg.visualstudio.com/monalisa/_git/octo-cat", + wantOrg: "vsorg", + wantProject: "monalisa", + }, + { + name: "visualstudio.com org-only", + raw: "https://vsorg.visualstudio.com", + wantOrg: "vsorg", + }, + { + name: "ssh URL", + raw: "ssh://ssh.dev.azure.com/v3/defaultorg/monalisa/octo-cat", + wantOrg: "defaultorg", + wantProject: "monalisa", + }, + { + name: "ssh URL without organization", + raw: "ssh://ssh.dev.azure.com/v3", + wantErr: ErrInvalidPath, + }, + { + name: "non-AzDO host strict", + raw: "https://github.com/owner/repo", + wantErr: ErrNotAzDO, + }, + { + name: "non-AzDO host lax", + raw: "https://github.com/owner/repo", + lax: true, + wantOrg: "github.com", + wantProject: "owner", + }, + { + name: "empty hostname", + raw: "https://", + wantErrText: "url must have a hostname", + }, + { + name: "nil URL", + nilURL: true, + wantErrText: "url must not be nil", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var u *url.URL + if !tt.nilURL { + u = parse(tt.raw) + } + id, err := ParseURL(u, tt.lax) + if tt.wantErr != nil || tt.wantErrText != "" { + require.Error(t, err) + if tt.wantErrText != "" { + // The legacy message text is preserved so existing + // callers relying on error strings keep behaving + // identically. + assert.Equal(t, tt.wantErrText, err.Error()) + } + if tt.wantErr != nil && !errors.Is(err, tt.wantErr) { + t.Fatalf("expected error %q, got %q", tt.wantErr, err) + } + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantOrg, id.Organization) + assert.Equal(t, tt.wantProject, id.Project) + }) + } +} + +func TestIsVisualStudioHost(t *testing.T) { + t.Parallel() + + tests := []struct { + hostname string + want bool + }{ + {hostname: "vsorg.visualstudio.com", want: true}, + {hostname: "VSORG.visualstudio.com", want: true}, + {hostname: "org.sub.visualstudio.com", want: true}, + {hostname: "dev.azure.com", want: false}, + {hostname: "ssh.dev.azure.com", want: false}, + {hostname: "vsorg.visualstudio.com.evil.example", want: false}, + {hostname: "example.com", want: false}, + {hostname: "", want: false}, + } + + for _, tt := range tests { + t.Run(tt.hostname, func(t *testing.T) { + t.Parallel() + + assert.Equal(t, tt.want, IsVisualStudioHost(tt.hostname)) + }) + } +} diff --git a/internal/cmd/auth/gitcredential/gitcredential.go b/internal/cmd/auth/gitcredential/gitcredential.go index 8952509a..96009e0d 100644 --- a/internal/cmd/auth/gitcredential/gitcredential.go +++ b/internal/cmd/auth/gitcredential/gitcredential.go @@ -2,11 +2,13 @@ package gitcredential import ( "bufio" + "errors" "fmt" "net/url" "strings" "github.com/spf13/cobra" + azdo "github.com/tmeckel/azdo-cli/internal/azdo" cmdutil "github.com/tmeckel/azdo-cli/internal/cmd/util" "github.com/tmeckel/azdo-cli/internal/util" @@ -131,22 +133,19 @@ func helperRun(ctx cmdutil.CmdContext, opts *credentialOptions) (err error) { var organizationName string lookupHost := strings.ToLower(wants["host"]) zap.L().Debug("detecting organization", zap.String("host", lookupHost), zap.String("path", wants["path"])) - if strings.Contains(lookupHost, ".visualstudio.com") { //nolint:golint,gocritic - organizationName = strings.Split(lookupHost, ".")[0] - zap.L().Debug("organization from visualstudio.com", zap.String("organization", organizationName)) - } else if lookupHost == "dev.azure.com" { - if path, ok := wants["path"]; !ok { - zap.L().Debug("dev.azure.com host requires path") - return fmt.Errorf("authenticating via dev.azure.com host requires path parameter") - } else { //nolint:golint,revive - organizationName = strings.Split(path, "/")[0] - zap.L().Debug("organization from dev.azure.com", zap.String("organization", organizationName)) - } - } else { + + id, err := azdo.ParseURL(&url.URL{Host: lookupHost, Path: wants["path"]}, false) + switch { + case errors.Is(err, azdo.ErrInvalidPath): + zap.L().Debug("dev.azure.com host requires path") + return fmt.Errorf("authenticating via dev.azure.com host requires path parameter") + case errors.Is(err, azdo.ErrNotAzDO): zap.L().Debug("not an Azure DevOps host", zap.String("host", lookupHost)) return fmt.Errorf("not an Azure DevOps host %s", lookupHost) + case err != nil: + return err } - + organizationName = id.Organization if organizationName == "" { zap.L().Debug("unable to extract organization", zap.String("host", wants["host"]), zap.String("path", wants["path"])) return fmt.Errorf("unable to get token from host %s or path %s", wants["host"], wants["path"]) diff --git a/internal/cmd/auth/gitcredential/gitcredential_test.go b/internal/cmd/auth/gitcredential/gitcredential_test.go new file mode 100644 index 00000000..cf34027c --- /dev/null +++ b/internal/cmd/auth/gitcredential/gitcredential_test.go @@ -0,0 +1,122 @@ +package gitcredential + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/tmeckel/azdo-cli/internal/iostreams" + "github.com/tmeckel/azdo-cli/internal/mocks" +) + +func TestHelperRun_get(t *testing.T) { + t.Parallel() + + const token = "secret-token" + + tests := []struct { + name string + input string + tokenFor string // organization GetToken is expected for; "" = no lookup + tokenValue string // token returned by GetToken + wantErr string // expected error text; "" = success + wantOutput []string // substrings expected in stdout on success + }{ + { + name: "visualstudio.com host extracts organization from subdomain", + input: "protocol=https\nhost=vsorg.visualstudio.com\npath=/monalisa/_git/octo-cat\n\n", + tokenFor: "vsorg", + tokenValue: token, + wantOutput: []string{"protocol=https", "host=vsorg.visualstudio.com", "password=secret-token"}, + }, + { + name: "dev.azure.com host extracts organization from path", + input: "protocol=https\nhost=dev.azure.com\npath=/defaultorg/monalisa/_git/octo-cat\n\n", + tokenFor: "defaultorg", + tokenValue: token, + wantOutput: []string{"protocol=https", "host=dev.azure.com", "password=secret-token"}, + }, + { + name: "dev.azure.com host without path", + input: "protocol=https\nhost=dev.azure.com\n\n", + wantErr: "authenticating via dev.azure.com host requires path parameter", + }, + { + name: "non-Azure DevOps host", + input: "protocol=https\nhost=github.com\npath=/owner/repo\n\n", + wantErr: "not an Azure DevOps host github.com", + }, + { + name: "protocol not https", + input: "protocol=git\nhost=dev.azure.com\npath=/defaultorg\n\n", + wantErr: "protocol git != https", + }, + { + name: "token missing for organization", + input: "protocol=https\nhost=dev.azure.com\npath=/defaultorg\n\n", + tokenFor: "defaultorg", + tokenValue: "", + wantErr: "unable to get token for organization defaultorg", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + ios, in, out, _ := iostreams.Test() + _, err := in.WriteString(tt.input) + require.NoError(t, err) + + cmd := mocks.NewMockCmdContext(ctrl) + cmd.EXPECT().IOStreams().Return(ios, nil).AnyTimes() + + cfg := mocks.NewMockConfig(ctrl) + auth := mocks.NewMockAuthConfig(ctrl) + cmd.EXPECT().Config().Return(cfg, nil).AnyTimes() + cfg.EXPECT().Authentication().Return(auth).AnyTimes() + + if tt.tokenFor != "" { + auth.EXPECT().GetToken(tt.tokenFor).Return(tt.tokenValue, nil).AnyTimes() + } + + err = helperRun(cmd, &credentialOptions{operation: "get"}) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } + require.NoError(t, err) + for _, want := range tt.wantOutput { + assert.Contains(t, out.String(), want) + } + assert.Contains(t, out.String(), "username=") + }) + } +} + +func TestHelperRun_urlLine(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + ios, in, out, _ := iostreams.Test() + _, err := in.WriteString("url=https://vsorg.visualstudio.com/monalisa/_git/octo-cat\n\n") + require.NoError(t, err) + + cmd := mocks.NewMockCmdContext(ctrl) + cmd.EXPECT().IOStreams().Return(ios, nil).AnyTimes() + cfg := mocks.NewMockConfig(ctrl) + auth := mocks.NewMockAuthConfig(ctrl) + cmd.EXPECT().Config().Return(cfg, nil).AnyTimes() + cfg.EXPECT().Authentication().Return(auth).AnyTimes() + auth.EXPECT().GetToken("vsorg").Return("secret-token", nil).AnyTimes() + + require.NoError(t, helperRun(cmd, &credentialOptions{operation: "get"})) + assert.Contains(t, out.String(), "password=secret-token") +} diff --git a/internal/cmd/boards/workitem/relation/add/add.go b/internal/cmd/boards/workitem/relation/add/add.go index 68c8d039..2be502d9 100644 --- a/internal/cmd/boards/workitem/relation/add/add.go +++ b/internal/cmd/boards/workitem/relation/add/add.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/relation/shared" + wishared "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/shared" "github.com/tmeckel/azdo-cli/internal/cmd/util" "github.com/tmeckel/azdo-cli/internal/types" ) @@ -34,12 +35,19 @@ func NewCmd(ctx util.CmdContext) *cobra.Command { Long: heredoc.Doc(` Attach one or more relations to an existing work item. The relation type must be one of the friendly names returned by 'list-type'. Targets can - be other work items (by ID) or arbitrary artifact URLs. + be other work items (by ID, optionally prefixed with their project) or + arbitrary artifact URLs. Work items in other projects of the same + organization are resolved via 'PROJECT/ID'. Cross-organization links + are not possible by ID; use --target-url with a remote link type such + as 'Remote Related', 'Consumes From' or 'Produces For'. `), Example: heredoc.Doc(` # Add a parent relation to another work item azdo boards work-item relation add Fabrikam/1234 --relation-type parent --target-id 5678 + # Add a parent relation to a work item in another project of the same organization + azdo boards work-item relation add Fabrikam/1234 --relation-type parent --target-id Contoso/77 + # Add a relation to multiple work items azdo boards work-item relation add Fabrikam/1234 --relation-type related --target-id 5678,5679 @@ -54,8 +62,8 @@ func NewCmd(ctx util.CmdContext) *cobra.Command { } cmd.Flags().StringVar(&opts.relationType, "relation-type", "", "Relation type (friendly name, e.g. parent, child, related).") - cmd.Flags().StringArrayVar(&opts.targetIDs, "target-id", nil, "Target work item ID (repeatable; comma-separated values accepted).") - cmd.Flags().StringArrayVar(&opts.targetURLs, "target-url", nil, "Target artifact URL (repeatable; comma-separated values accepted).") + cmd.Flags().StringArrayVarP(&opts.targetIDs, "target-id", "T", nil, "Target work item ID (repeatable; comma-separated; each entry is [PROJECT/]ID; ID-only targets resolve in the current project).") + cmd.Flags().StringArrayVarP(&opts.targetURLs, "target-url", "u", nil, "Target artifact URL (repeatable; comma-separated values accepted).") util.AddJSONFlags(cmd, &opts.exporter, []string{"id", "rev", "fields", "url", "_links", "relations", "commentVersionRef"}) @@ -95,11 +103,30 @@ func runAdd(cmdCtx util.CmdContext, opts *addOptions) error { if len(targetIDs) > 0 && len(targetURLs) > 0 { return util.FlagErrorf("--target-id and --target-url are mutually exclusive; supply only one") } + type target struct { + id int + project string + } + parsedTargets := make([]target, 0, len(targetIDs)) for _, tid := range targetIDs { - n, err := strconv.Atoi(tid) + p, err := util.Parse(nil, tid, util.ParseOptions{ + DisallowOrganization: true, + AllowBareTargets: true, + MinTargets: 1, + MaxTargets: 1, + }) + if err != nil { + return util.FlagErrorWrap(err) + } + targetProject := scope.Project + if p.Project != "" { + targetProject = p.Project + } + n, err := strconv.Atoi(p.Targets[0]) if err != nil || n <= 0 { - return util.FlagErrorf("target work item ID must be a positive integer; got %q", tid) + return util.FlagErrorf("target work item ID must be a positive integer; got %q", p.Targets[0]) } + parsedTargets = append(parsedTargets, target{id: n, project: targetProject}) } wit, err := cmdCtx.ClientFactory().WorkItemTracking(cmdCtx.Context(), scope.Organization) @@ -117,19 +144,33 @@ func runAdd(cmdCtx util.CmdContext, opts *addOptions) error { return util.FlagErrorWrap(err) } + source, err := wit.GetWorkItem(cmdCtx.Context(), workitemtracking.GetWorkItemArgs{ + Id: &id, + Project: types.ToPtr(scope.Project), + Fields: types.ToPtr([]string{wishared.TeamProjectField}), + }) + if err != nil { + return fmt.Errorf("failed to fetch work item %d: %w", id, err) + } + if !wishared.BelongsToProject(source, scope.Project) { + return fmt.Errorf("work item %d does not belong to project %q", id, scope.Project) + } + // Resolve target IDs to URLs. targetURLsResolved := []string{} - for _, tid := range targetIDs { - n, _ := strconv.Atoi(tid) + for _, tgt := range parsedTargets { target, err := wit.GetWorkItem(cmdCtx.Context(), workitemtracking.GetWorkItemArgs{ - Project: types.ToPtr(scope.Project), - Id: &n, + Project: types.ToPtr(tgt.project), + Id: &tgt.id, }) if err != nil { - return fmt.Errorf("failed to resolve target work item %d: %w", n, err) + return fmt.Errorf("failed to resolve target work item %d: %w", tgt.id, err) + } + if !wishared.BelongsToProject(target, tgt.project) { + return fmt.Errorf("target work item %d does not belong to project %q", tgt.id, tgt.project) } if target == nil || target.Url == nil || *target.Url == "" { - return fmt.Errorf("target work item %d has no URL; cannot create relation", n) + return fmt.Errorf("target work item %d has no URL; cannot create relation", tgt.id) } targetURLsResolved = append(targetURLsResolved, *target.Url) } diff --git a/internal/cmd/boards/workitem/relation/add/add_test.go b/internal/cmd/boards/workitem/relation/add/add_test.go index 22dfb0a6..07884eb7 100644 --- a/internal/cmd/boards/workitem/relation/add/add_test.go +++ b/internal/cmd/boards/workitem/relation/add/add_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + wishared "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/shared" "github.com/tmeckel/azdo-cli/internal/iostreams" "github.com/tmeckel/azdo-cli/internal/mocks" "github.com/tmeckel/azdo-cli/internal/printer" @@ -73,8 +74,12 @@ func (d *dependencies) stubGetWorkItem(t *testing.T, project string, targetIDs m require.NotNil(t, args.Id) require.NotNil(t, args.Project) assert.Equal(t, project, *args.Project) + fields := map[string]interface{}{wishared.TeamProjectField: project} if url, ok := targetIDs[*args.Id]; ok { - return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(url)}, nil + return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(url), Fields: &fields}, nil + } + if populated.Fields == nil { + populated.Fields = &fields } return populated, nil }, @@ -143,6 +148,8 @@ func TestNewCmd_add(t *testing.T) { for _, name := range []string{"relation-type", "target-id", "target-url", "json"} { assert.NotNil(t, f.Lookup(name), "flag %q must exist", name) } + assert.Equal(t, "T", f.Lookup("target-id").Shorthand, "target-id shorthand collides with --template (-t)") + assert.Equal(t, "u", f.Lookup("target-url").Shorthand) } func Test_runAdd_minimal(t *testing.T) { @@ -360,7 +367,16 @@ func Test_runAdd_targetIDNotFound(t *testing.T) { deps := newDependencies(t, "myorg") deps.setupDefaultOrg("myorg") deps.stubGetRelationTypes(relationTypes) - deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).Return(nil, errors.New("not found")).AnyTimes() + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + if *args.Id == 1234 { + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + } + return nil, errors.New("not found") + }, + ).AnyTimes() err := runAdd(deps.cmd, &addOptions{targetArg: "Fabrikam/1234", relationType: "parent", targetIDs: []string{"2"}}) require.Error(t, err) @@ -409,6 +425,191 @@ func Test_runAdd_scope(t *testing.T) { } } +func Test_runAdd_projectMismatchSource(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + require.NotNil(t, args.Project) + if *args.Id == 2 { + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(targetURL(2)), Fields: &fields}, nil + } + fields := map[string]interface{}{wishared.TeamProjectField: "OtherProject"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + }, + ).AnyTimes() + + err := runAdd(deps.cmd, &addOptions{targetArg: "Fabrikam/1234", relationType: "parent", targetIDs: []string{"2"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), `work item 1234 does not belong to project "Fabrikam"`) +} + +func Test_run_add_projectMismatchTarget(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + require.NotNil(t, args.Project) + if *args.Id == 1234 { + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + } + fields := map[string]interface{}{wishared.TeamProjectField: "OtherProject"} + return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(targetURL(2)), Fields: &fields}, nil + }, + ).AnyTimes() + + err := runAdd(deps.cmd, &addOptions{targetArg: "Fabrikam/1234", relationType: "parent", targetIDs: []string{"2"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), `target work item 2 does not belong to project "Fabrikam"`) +} + +func Test_runAdd_targetIDForms(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + targetIDs []string + wantProject map[int]string // id -> expected fetch project + wantError string + }{ + { + name: "bare ID resolves in scope project", + targetIDs: []string{"42"}, + wantProject: map[int]string{ + 42: "Fabrikam", + }, + }, + { + name: "non-numeric bare ID", + targetIDs: []string{"abc"}, + wantError: `target work item ID must be a positive integer; got "abc"`, + }, + { + name: "non-numeric project prefixed ID", + targetIDs: []string{"Contoso/abc"}, + wantError: `target work item ID must be a positive integer; got "abc"`, + }, + { + name: "negative project prefixed ID", + targetIDs: []string{"Contoso/-3"}, + wantError: "target work item ID must be a positive integer", + }, + { + name: "legacy org slash form", + targetIDs: []string{"myorg/Fabrikam/42"}, + wantError: "organization is not allowed", + }, + { + name: "org prefixed colon form", + targetIDs: []string{"myorg:Fabrikam/42"}, + wantError: "organization is not allowed", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + if tt.wantError == "" { + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + require.NotNil(t, args.Project) + if *args.Id == 1234 { + assert.Equal(t, "Fabrikam", *args.Project) + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + } + assert.Equal(t, tt.wantProject[*args.Id], *args.Project) + fields := map[string]interface{}{wishared.TeamProjectField: tt.wantProject[*args.Id]} + return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(targetURL(*args.Id)), Fields: &fields}, nil + }, + ).AnyTimes() + deps.stubUpdateWorkItem(t, "Fabrikam") + } + + err := runAdd(deps.cmd, &addOptions{targetArg: "Fabrikam/1234", relationType: "parent", targetIDs: tt.targetIDs}) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + return + } + require.NoError(t, err) + }) + } +} + +func Test_runAdd_crossProjectTarget(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + require.NotNil(t, args.Project) + switch *args.Id { + case 1234: + assert.Equal(t, "Fabrikam", *args.Project) + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + case 77: + assert.Equal(t, "Contoso", *args.Project) + fields := map[string]interface{}{wishared.TeamProjectField: "Contoso"} + return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(targetURL(77)), Fields: &fields}, nil + default: + t.Fatalf("unexpected work item ID %d", *args.Id) + return nil, nil + } + }, + ).AnyTimes() + args := deps.stubUpdateWorkItem(t, "Fabrikam") + + err := runAdd(deps.cmd, &addOptions{targetArg: "Fabrikam/1234", relationType: "parent", targetIDs: []string{"Contoso/77"}}) + require.NoError(t, err) + + require.NotNil(t, args.Document) + require.Len(t, *args.Document, 1) + values := docValues(args.Document) + assert.Equal(t, targetURL(77), values[0]["url"]) +} + +func Test_runAdd_crossProjectTargetMismatch(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + if *args.Id == 1234 { + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + } + fields := map[string]interface{}{wishared.TeamProjectField: "YetAnother"} + return &workitemtracking.WorkItem{Id: args.Id, Url: types.ToPtr(targetURL(77)), Fields: &fields}, nil + }, + ).AnyTimes() + + err := runAdd(deps.cmd, &addOptions{targetArg: "Fabrikam/1234", relationType: "parent", targetIDs: []string{"Contoso/77"}}) + require.Error(t, err) + assert.Contains(t, err.Error(), `target work item 77 does not belong to project "Contoso"`) +} + func Test_runAdd_APIError(t *testing.T) { t.Parallel() diff --git a/internal/cmd/boards/workitem/relation/relation.go b/internal/cmd/boards/workitem/relation/relation.go index 805e7174..5dc5c107 100644 --- a/internal/cmd/boards/workitem/relation/relation.go +++ b/internal/cmd/boards/workitem/relation/relation.go @@ -5,6 +5,7 @@ import ( "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/relation/add" "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/relation/remove" + "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/relation/show" "github.com/tmeckel/azdo-cli/internal/cmd/util" ) @@ -17,6 +18,7 @@ func NewCmd(ctx util.CmdContext) *cobra.Command { cmd.AddCommand(add.NewCmd(ctx)) cmd.AddCommand(remove.NewCmd(ctx)) + cmd.AddCommand(show.NewCmd(ctx)) return cmd } diff --git a/internal/cmd/boards/workitem/relation/shared/relation.go b/internal/cmd/boards/workitem/relation/shared/relation.go index c1fcdc99..09832ff7 100644 --- a/internal/cmd/boards/workitem/relation/shared/relation.go +++ b/internal/cmd/boards/workitem/relation/shared/relation.go @@ -1,12 +1,29 @@ package shared import ( + "context" "fmt" + "regexp" + "strconv" "strings" "github.com/microsoft/azure-devops-go-api/azuredevops/v7/workitemtracking" + + wishared "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/shared" + "github.com/tmeckel/azdo-cli/internal/cmd/util" + "github.com/tmeckel/azdo-cli/internal/types" ) +// RelationTarget carries the identity of a related work item for table +// renderers. Non-work-item (artifact) relations have an empty identity and put +// the raw URL into Title so the link stays visible. +type RelationTarget struct { + Organization string + Project string + ID int + Title string +} + // ResolveRelationType resolves a friendly relation-type name to its // referenceName via a case-insensitive match against the relation types, // mirroring get_system_relation_name in the Azure DevOps CLI extension. @@ -46,3 +63,67 @@ func PopulateFriendlyNames(relTypes *[]workitemtracking.WorkItemRelationType, wi } return nil } + +var workItemLinkRe = regexp.MustCompile(`/workItems/(\d+)`) + +// WorkItemIDFromURL extracts the work item ID carried by a relation URL. The +// second result reports whether the URL points at a work item at all. +func WorkItemIDFromURL(raw string) (int, bool) { + m := workItemLinkRe.FindStringSubmatch(strings.TrimSpace(raw)) + if len(m) != 2 { + return 0, false + } + id, err := strconv.Atoi(m[1]) + if err != nil || id <= 0 { + return 0, false + } + return id, true +} + +// ResolveRelationTarget identifies the work item a relation URL points at. +// Same-organization links are fetched to recover the true project and title +// (fetches are cached by ID across relations when cached is non-nil). Remote +// or unresolvable links fall back to a best-effort URL parse; non-work-item +// links surface the URL as the title. +func ResolveRelationTarget(ctx context.Context, wit workitemtracking.Client, scope *util.Path, cached map[int]RelationTarget, raw string) RelationTarget { + id, ok := WorkItemIDFromURL(raw) + if !ok { + return RelationTarget{Title: raw} + } + if cached != nil { + if t, ok := cached[id]; ok { + return t + } + } + t := FetchRelationTarget(ctx, wit, scope, id, raw) + if cached != nil { + cached[id] = t + } + return t +} + +// FetchRelationTarget fetches a related work item to recover its real project +// and title. When the fetch fails, the caller-provided URL is parsed as a +// best-effort fallback so remote organization/project remain visible. +func FetchRelationTarget(ctx context.Context, wit workitemtracking.Client, scope *util.Path, id int, raw string) RelationTarget { + target, err := wit.GetWorkItem(ctx, workitemtracking.GetWorkItemArgs{ + Project: types.ToPtr(scope.Project), + Id: types.ToPtr(id), + Fields: types.ToPtr([]string{wishared.TeamProjectField, "System.Title"}), + }) + if err == nil && target != nil { + fields := types.GetValue(target.Fields, map[string]any{}) + return RelationTarget{ + Organization: scope.Organization, + Project: wishared.FieldString(fields, wishared.TeamProjectField), + ID: id, + Title: wishared.FieldString(fields, "System.Title"), + } + } + org, project := wishared.ParseWorkItemURL(raw) + return RelationTarget{ + Organization: org, + Project: project, + ID: id, + } +} diff --git a/internal/cmd/boards/workitem/relation/shared/relation_test.go b/internal/cmd/boards/workitem/relation/shared/relation_test.go new file mode 100644 index 00000000..f83b3821 --- /dev/null +++ b/internal/cmd/boards/workitem/relation/shared/relation_test.go @@ -0,0 +1,198 @@ +package shared + +import ( + "context" + "errors" + "testing" + + "github.com/microsoft/azure-devops-go-api/azuredevops/v7/workitemtracking" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + wishared "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/shared" + "github.com/tmeckel/azdo-cli/internal/cmd/util" + "github.com/tmeckel/azdo-cli/internal/mocks" + "github.com/tmeckel/azdo-cli/internal/types" +) + +func TestWorkItemIDFromURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + want int + ok bool + }{ + { + name: "dev.azure full URL", + raw: "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77", + want: 77, + ok: true, + }, + { + name: "visualstudio URL", + raw: "https://myorg.visualstudio.com/Contoso/_apis/wit/workItems/42", + want: 42, + ok: true, + }, + { + name: "bare path with ID", + raw: "/workItems/3", + want: 3, + ok: true, + }, + { + name: "artifact URL", + raw: "https://example.com/1", + want: 0, + ok: false, + }, + { + name: "no workItems segment", + raw: "https://dev.azure.com/myorg/_apis/wit/workItems/abc", + want: 0, + ok: false, + }, + { + name: "zero ID", + raw: "https://dev.azure.com/myorg/_apis/wit/workItems/0", + want: 0, + ok: false, + }, + { + name: "malformed", + raw: "not a url", + want: 0, + ok: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got, ok := WorkItemIDFromURL(tt.raw) + assert.Equal(t, tt.ok, ok) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestFetchRelationTarget_success(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + wit := mocks.NewMockWorkItemTrackingClient(ctrl) + + fields := map[string]any{ + wishared.TeamProjectField: "Contoso", + "System.Title": "Deploy the fix", + } + wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.Equal(t, "Fabrikam", *args.Project) + require.Equal(t, 77, *args.Id) + require.Equal(t, []string{wishared.TeamProjectField, "System.Title"}, *args.Fields) + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + }, + ) + + got := FetchRelationTarget(context.Background(), wit, &util.Path{Organization: "myorg", Project: "Fabrikam"}, 77, "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77") + assert.Equal(t, RelationTarget{Organization: "myorg", Project: "Contoso", ID: 77, Title: "Deploy the fix"}, got) +} + +func TestFetchRelationTarget_fallback(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + wit := mocks.NewMockWorkItemTrackingClient(ctrl) + wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).Return(nil, errors.New("not found")) + + got := FetchRelationTarget(context.Background(), wit, &util.Path{Organization: "myorg", Project: "Fabrikam"}, 42, "https://dev.azure.com/otherorg/Proj/_apis/wit/workItems/42") + assert.Equal(t, RelationTarget{Organization: "otherorg", Project: "Proj", ID: 42}, got) +} + +func TestResolveRelationTarget_cacheHitAndMiss(t *testing.T) { + t.Parallel() + + t.Run("cache hit avoids fetch", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + wit := mocks.NewMockWorkItemTrackingClient(ctrl) + cached := map[int]RelationTarget{ + 77: {Organization: "myorg", Project: "Contoso", ID: 77, Title: "Cached"}, + } + + got := ResolveRelationTarget(context.Background(), wit, &util.Path{Organization: "myorg"}, cached, "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77") + assert.Equal(t, cached[77], got) + }) + + t.Run("miss fetches once then caches", func(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + wit := mocks.NewMockWorkItemTrackingClient(ctrl) + fetches := 0 + wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + fetches++ + fields := map[string]any{ + wishared.TeamProjectField: "Contoso", + "System.Title": "Deploy the fix", + } + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields}, nil + }, + ).AnyTimes() + + ctx := context.Background() + scope := &util.Path{Organization: "myorg", Project: "Fabrikam"} + cached := map[int]RelationTarget{} + raw := "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77" + + want := RelationTarget{Organization: "myorg", Project: "Contoso", ID: 77, Title: "Deploy the fix"} + assert.Equal(t, want, ResolveRelationTarget(ctx, wit, scope, cached, raw)) + assert.Equal(t, want, ResolveRelationTarget(ctx, wit, scope, cached, raw)) + assert.Equal(t, 1, fetches) + assert.Equal(t, want, cached[77]) + }) +} + +func TestResolveRelationTarget_nonWorkItemURL(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + wit := mocks.NewMockWorkItemTrackingClient(ctrl) + + got := ResolveRelationTarget(context.Background(), wit, &util.Path{Organization: "myorg"}, map[int]RelationTarget{}, "https://example.com/1") + assert.Equal(t, RelationTarget{Title: "https://example.com/1"}, got) +} + +func TestResolveRelationTarget_nilCache(t *testing.T) { + t.Parallel() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + wit := mocks.NewMockWorkItemTrackingClient(ctrl) + fields := map[string]any{ + wishared.TeamProjectField: "Contoso", + "System.Title": "Deploy the fix", + } + wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).Return( + &workitemtracking.WorkItem{Id: types.ToPtr(77), Fields: &fields}, nil, + ).AnyTimes() + + ctx := context.Background() + scope := &util.Path{Organization: "myorg"} + want := RelationTarget{Organization: "myorg", Project: "Contoso", ID: 77, Title: "Deploy the fix"} + + // A nil cache must not panic; repeated calls keep resolving. + assert.Equal(t, want, ResolveRelationTarget(ctx, wit, scope, nil, "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77")) + assert.Equal(t, want, ResolveRelationTarget(ctx, wit, scope, nil, "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77")) +} diff --git a/internal/cmd/boards/workitem/relation/show/show.go b/internal/cmd/boards/workitem/relation/show/show.go new file mode 100644 index 00000000..7596c792 --- /dev/null +++ b/internal/cmd/boards/workitem/relation/show/show.go @@ -0,0 +1,119 @@ +package show + +import ( + "fmt" + "strconv" + + "github.com/MakeNowJust/heredoc/v2" + "github.com/microsoft/azure-devops-go-api/azuredevops/v7/workitemtracking" + "github.com/spf13/cobra" + + "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/relation/shared" + wishared "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/shared" + "github.com/tmeckel/azdo-cli/internal/cmd/util" + "github.com/tmeckel/azdo-cli/internal/types" +) + +type showOptions struct { + targetArg string + + exporter util.Exporter +} + +func NewCmd(ctx util.CmdContext) *cobra.Command { + opts := &showOptions{} + + cmd := &cobra.Command{ + Use: "show [ORG:]PROJECT/ID", + Aliases: []string{"s"}, + Short: "List the relations of a work item.", + Long: heredoc.Doc(` + List all relations of an existing work item. Relation types are + displayed by their friendly name. + `), + Example: heredoc.Doc(` + # List the relations of a work item + azdo boards work-item relation show Fabrikam/1234 + `), + Args: util.ExactArgs(1, "project/work item target required"), + RunE: func(cmd *cobra.Command, args []string) error { + opts.targetArg = args[0] + return runShow(ctx, opts) + }, + } + + util.AddJSONFlags(cmd, &opts.exporter, []string{"id", "rev", "fields", "url", "_links", "relations", "commentVersionRef"}) + + return cmd +} + +func runShow(cmdCtx util.CmdContext, opts *showOptions) error { + ios, err := cmdCtx.IOStreams() + if err != nil { + return err + } + ios.StartProgressIndicator() + defer ios.StopProgressIndicator() + + scope, err := util.ParseProjectTargetWithDefaultOrganization(cmdCtx, opts.targetArg) + if err != nil { + return util.FlagErrorWrap(err) + } + + id, err := strconv.Atoi(scope.Targets[0]) + if err != nil || id <= 0 { + return util.FlagErrorf("work item ID must be a positive integer; got %q", scope.Targets[0]) + } + + wit, err := cmdCtx.ClientFactory().WorkItemTracking(cmdCtx.Context(), scope.Organization) + if err != nil { + return fmt.Errorf("failed to create work item tracking client: %w", err) + } + + expand := workitemtracking.WorkItemExpandValues.All + wi, err := wit.GetWorkItem(cmdCtx.Context(), workitemtracking.GetWorkItemArgs{ + Project: types.ToPtr(scope.Project), + Id: &id, + Expand: &expand, + }) + if err != nil { + return fmt.Errorf("failed to get work item %d: %w", id, err) + } + if !wishared.BelongsToProject(wi, scope.Project) { + return fmt.Errorf("work item %d does not belong to project %q", id, scope.Project) + } + + relTypes, err := wit.GetRelationTypes(cmdCtx.Context(), workitemtracking.GetRelationTypesArgs{}) + if err != nil { + return fmt.Errorf("failed to get relation types: %w", err) + } + if err := shared.PopulateFriendlyNames(relTypes, wi); err != nil { + return err + } + + if opts.exporter != nil { + return opts.exporter.Write(ios, wi) + } + tp, err := cmdCtx.Printer("list") + if err != nil { + return err + } + tp.AddColumns("TYPE", "ORGANIZATION", "PROJECT", "ID", "TITLE") + if wi.Relations != nil { + resolved := map[int]shared.RelationTarget{} + for _, rel := range *wi.Relations { + tp.AddField(types.GetValue(rel.Rel, "")) + target := shared.ResolveRelationTarget(cmdCtx.Context(), wit, scope, resolved, types.GetValue(rel.Url, "")) + tp.AddField(target.Organization) + tp.AddField(target.Project) + idField := "" + if target.ID > 0 { + idField = strconv.Itoa(target.ID) + } + tp.AddField(idField) + tp.AddField(target.Title) + tp.EndRow() + } + } + return tp.Render() +} diff --git a/internal/cmd/boards/workitem/relation/show/show_test.go b/internal/cmd/boards/workitem/relation/show/show_test.go new file mode 100644 index 00000000..2fa9be9d --- /dev/null +++ b/internal/cmd/boards/workitem/relation/show/show_test.go @@ -0,0 +1,336 @@ +package show + +import ( + "bytes" + "context" + "errors" + "io" + "regexp" + "testing" + + "github.com/microsoft/azure-devops-go-api/azuredevops/v7/workitemtracking" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + wishared "github.com/tmeckel/azdo-cli/internal/cmd/boards/workitem/shared" + "github.com/tmeckel/azdo-cli/internal/iostreams" + "github.com/tmeckel/azdo-cli/internal/mocks" + "github.com/tmeckel/azdo-cli/internal/printer" + "github.com/tmeckel/azdo-cli/internal/types" +) + +type dependencies struct { + cmd *mocks.MockCmdContext + clientFact *mocks.MockClientFactory + wit *mocks.MockWorkItemTrackingClient + config *mocks.MockConfig + auth *mocks.MockAuthConfig + stdout *bytes.Buffer +} + +func newDependencies(t *testing.T, organization string) *dependencies { + t.Helper() + + ctrl := gomock.NewController(t) + t.Cleanup(ctrl.Finish) + + io, _, out, _ := iostreams.Test() + + deps := &dependencies{ + cmd: mocks.NewMockCmdContext(ctrl), + clientFact: mocks.NewMockClientFactory(ctrl), + wit: mocks.NewMockWorkItemTrackingClient(ctrl), + config: mocks.NewMockConfig(ctrl), + auth: mocks.NewMockAuthConfig(ctrl), + stdout: out, + } + + deps.cmd.EXPECT().IOStreams().Return(io, nil).AnyTimes() + deps.cmd.EXPECT().Context().Return(context.Background()).AnyTimes() + deps.cmd.EXPECT().ClientFactory().Return(deps.clientFact).AnyTimes() + deps.cmd.EXPECT().Config().Return(deps.config, nil).AnyTimes() + deps.config.EXPECT().Authentication().Return(deps.auth).AnyTimes() + deps.cmd.EXPECT().Printer("list").Return(mustListPrinter(t, out), nil).AnyTimes() + if organization != "" { + deps.clientFact.EXPECT().WorkItemTracking(gomock.Any(), organization).Return(deps.wit, nil).AnyTimes() + } + + return deps +} + +func (d *dependencies) setupDefaultOrg(org string) { + d.auth.EXPECT().GetDefaultOrganization().Return(org, nil).AnyTimes() +} + +func (d *dependencies) stubGetRelationTypes(types []workitemtracking.WorkItemRelationType) { + d.wit.EXPECT().GetRelationTypes(gomock.Any(), gomock.Any()).Return(&types, nil).AnyTimes() +} + +func (d *dependencies) stubGetWorkItem(t *testing.T, project string, wi *workitemtracking.WorkItem, targets ...map[int]*workitemtracking.WorkItem) { + var targetMap map[int]*workitemtracking.WorkItem + if len(targets) > 0 { + targetMap = targets[0] + } + d.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + require.NotNil(t, args.Project) + assert.Equal(t, project, *args.Project) + if twi, ok := targetMap[*args.Id]; ok { + require.NotNil(t, args.Fields) + return twi, nil + } + require.NotNil(t, args.Expand) + assert.Equal(t, workitemtracking.WorkItemExpandValues.All, *args.Expand) + return wi, nil + }, + ).AnyTimes() +} + +func mustListPrinter(t *testing.T, w io.Writer) printer.Printer { + t.Helper() + tp, err := printer.NewListPrinter(w) + require.NoError(t, err) + return tp +} + +type captureExporter struct { + data any +} + +func (c *captureExporter) Fields() []string { return nil } +func (c *captureExporter) Write(_ *iostreams.IOStreams, data any) error { + c.data = data + return nil +} + +var relationTypes = []workitemtracking.WorkItemRelationType{ + {Name: types.ToPtr("parent"), ReferenceName: types.ToPtr("System.LinkTypes.Hierarchy-Reverse")}, + {Name: types.ToPtr("artifact"), ReferenceName: types.ToPtr("System.ArtifactLink")}, +} + +func workItem(id int, project string, relations *[]workitemtracking.WorkItemRelation) *workitemtracking.WorkItem { + fields := map[string]interface{}{wishared.TeamProjectField: project} + return &workitemtracking.WorkItem{Id: types.ToPtr(id), Fields: &fields, Relations: relations} +} + +func targetWorkItem(id int, project, title string) *workitemtracking.WorkItem { + fields := map[string]interface{}{ + wishared.TeamProjectField: project, + "System.Title": title, + } + return &workitemtracking.WorkItem{Id: types.ToPtr(id), Fields: &fields} +} + +func TestNewCmd_show(t *testing.T) { + t.Parallel() + + cmd := NewCmd(nil) + assert.Equal(t, "show [ORG:]PROJECT/ID", cmd.Use) + assert.Equal(t, []string{"s"}, cmd.Aliases) + assert.NotNil(t, cmd.RunE) + require.NoError(t, cmd.Args(cmd, []string{"Fabrikam/1234"})) + assert.Error(t, cmd.Args(cmd, []string{"Fabrikam/1234", "Extra"})) + assert.Error(t, cmd.Args(cmd, []string{})) + + for _, name := range []string{"json"} { + assert.NotNil(t, cmd.Flags().Lookup(name), "flag %q must exist", name) + } +} + +func Test_runShow_tableOutput(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.stubGetWorkItem(t, "Fabrikam", workItem(1234, "Fabrikam", &[]workitemtracking.WorkItemRelation{ + {Rel: types.ToPtr("System.LinkTypes.Hierarchy-Reverse"), Url: types.ToPtr("https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77")}, + {Rel: types.ToPtr("System.ArtifactLink"), Url: types.ToPtr("https://example.com/1")}, + }), map[int]*workitemtracking.WorkItem{ + 77: targetWorkItem(77, "Contoso", "Deploy the fix"), + }) + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.NoError(t, err) + + out := deps.stdout.String() + for _, hdr := range []string{"TYPE", "ORGANIZATION", "PROJECT", "ID", "TITLE"} { + assert.Contains(t, out, hdr) + } + assert.Contains(t, out, "parent") + assert.Contains(t, out, "artifact") + assert.Contains(t, out, "Contoso") + assert.Contains(t, out, "77") + assert.Contains(t, out, "Deploy the fix") + assert.Contains(t, out, "https://example.com/1") + assert.NotContains(t, out, "URL:") +} + +func Test_runShow_artifactRelationEmptyID(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.stubGetWorkItem(t, "Fabrikam", workItem(1234, "Fabrikam", &[]workitemtracking.WorkItemRelation{ + {Rel: types.ToPtr("System.ArtifactLink"), Url: types.ToPtr("https://example.com/1")}, + })) + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.NoError(t, err) + + out := deps.stdout.String() + // Artifact relations carry no work item ID: the ID cell renders empty, + // never a literal "0". + assert.Regexp(t, regexp.MustCompile(`(?m)^ID:\s*$`), out) + assert.NotRegexp(t, regexp.MustCompile(`(?m)^ID:\s*0\s*$`), out) + assert.Contains(t, out, "https://example.com/1") +} + +func Test_runShow_remoteLinkFallback(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, args workitemtracking.GetWorkItemArgs) (*workitemtracking.WorkItem, error) { + require.NotNil(t, args.Id) + if *args.Id == 1234 { + fields := map[string]interface{}{wishared.TeamProjectField: "Fabrikam"} + return &workitemtracking.WorkItem{Id: args.Id, Fields: &fields, Relations: &[]workitemtracking.WorkItemRelation{ + {Rel: types.ToPtr("System.LinkTypes.Hierarchy-Reverse"), Url: types.ToPtr("https://dev.azure.com/otherorg/Proj/_apis/wit/workItems/42")}, + }}, nil + } + return nil, errors.New("not found") + }, + ).AnyTimes() + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.NoError(t, err) + + out := deps.stdout.String() + assert.Contains(t, out, "otherorg") + assert.Contains(t, out, "Proj") + assert.Contains(t, out, "42") +} + +func Test_runShow_emptyRelations(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + deps.stubGetWorkItem(t, "Fabrikam", workItem(1234, "Fabrikam", nil)) + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.NoError(t, err) + assert.Empty(t, deps.stdout.String()) +} + +func Test_runShow_explicitOrganizationProject(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.stubGetRelationTypes(relationTypes) + deps.stubGetWorkItem(t, "Fabrikam", workItem(1234, "Fabrikam", nil)) + + err := runShow(deps.cmd, &showOptions{targetArg: "myorg:Fabrikam/1234"}) + require.NoError(t, err) +} + +func Test_runShow_success_JSON(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetRelationTypes(relationTypes) + wi := workItem(1234, "Fabrikam", &[]workitemtracking.WorkItemRelation{ + {Rel: types.ToPtr("System.LinkTypes.Hierarchy-Reverse"), Url: types.ToPtr("https://dev.azure.com/2")}, + }) + deps.stubGetWorkItem(t, "Fabrikam", wi) + + exporter := &captureExporter{} + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234", exporter: exporter}) + require.NoError(t, err) + assert.Same(t, wi, exporter.data) +} + +func Test_runShow_invalidIDs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + targetArg string + wantError string + }{ + { + name: "non-numeric ID", + targetArg: "Fabrikam/abc", + wantError: `work item ID must be a positive integer; got "abc"`, + }, + { + name: "zero ID", + targetArg: "Fabrikam/0", + wantError: "work item ID must be a positive integer", + }, + { + name: "negative ID", + targetArg: "Fabrikam/-5", + wantError: "work item ID must be a positive integer", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + + err := runShow(deps.cmd, &showOptions{targetArg: tt.targetArg}) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + }) + } +} + +func Test_runShow_getWorkItemError(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.wit.EXPECT().GetWorkItem(gomock.Any(), gomock.Any()).Return(nil, errors.New("boom")).AnyTimes() + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to get work item 1234") + assert.Contains(t, err.Error(), "boom") +} + +func Test_runShow_getRelationTypesError(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetWorkItem(t, "Fabrikam", workItem(1234, "Fabrikam", nil)) + deps.wit.EXPECT().GetRelationTypes(gomock.Any(), gomock.Any()).Return(nil, errors.New("boom")).AnyTimes() + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to get relation types") + assert.Contains(t, err.Error(), "boom") +} + +func Test_runShow_projectMismatch(t *testing.T) { + t.Parallel() + + deps := newDependencies(t, "myorg") + deps.setupDefaultOrg("myorg") + deps.stubGetWorkItem(t, "Fabrikam", workItem(1234, "OtherProject", nil)) + + err := runShow(deps.cmd, &showOptions{targetArg: "Fabrikam/1234"}) + require.Error(t, err) + assert.Contains(t, err.Error(), `work item 1234 does not belong to project "Fabrikam"`) +} diff --git a/internal/cmd/boards/workitem/shared/url.go b/internal/cmd/boards/workitem/shared/url.go new file mode 100644 index 00000000..c48bfe5f --- /dev/null +++ b/internal/cmd/boards/workitem/shared/url.go @@ -0,0 +1,37 @@ +package shared + +import ( + "net/url" + "strings" + + "github.com/tmeckel/azdo-cli/internal/azdo" +) + +// ParseWorkItemURL extracts the organization and project from a work item URL: +// the subdomain of *.visualstudio.com hosts or the first path segment of +// dev.azure.com URLs, and the path segment directly before "/_apis" as the +// project with a fallback to the first segment. It is best-effort: malformed +// URLs and non-Azure hosts yield empty or host-derived values instead of +// errors, which keeps the relation fallback behavior forgiving. The +// organization extraction is delegated to azdo.ParseURL (lax mode); only the +// work-item-specific "_apis" project rule lives here. +func ParseWorkItemURL(raw string) (organization string, project string) { + u, err := url.Parse(strings.TrimSpace(raw)) + if err != nil || u.Hostname() == "" { + return "", "" + } + id, err := azdo.ParseURL(u, true) + if err != nil { + return "", "" + } + segs := strings.Split(strings.Trim(u.Path, "/"), "/") + for i, s := range segs { + if s == "_apis" && i > 0 { + return id.Organization, segs[i-1] + } + } + if len(segs) > 0 { + return id.Organization, segs[0] + } + return id.Organization, "" +} diff --git a/internal/cmd/boards/workitem/shared/url_test.go b/internal/cmd/boards/workitem/shared/url_test.go new file mode 100644 index 00000000..d26693ef --- /dev/null +++ b/internal/cmd/boards/workitem/shared/url_test.go @@ -0,0 +1,79 @@ +package shared + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestParseWorkItemURL covers the best-effort URL parser used for relation +// fallback. It moved here from the relation shared package when the work-item +// URL helpers were consolidated. +func TestParseWorkItemURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + raw string + wantOrg string + wantProject string + }{ + { + name: "dev.azure with project", + raw: "https://dev.azure.com/myorg/Contoso/_apis/wit/workItems/77", + wantOrg: "myorg", + wantProject: "Contoso", + }, + { + name: "dev.azure without _apis falls back to first segment", + raw: "https://dev.azure.com/myorg/Contoso", + wantOrg: "myorg", + wantProject: "myorg", + }, + { + name: "visualstudio with project", + raw: "https://myorg.visualstudio.com/Contoso/_apis/wit/workItems/77", + wantOrg: "myorg", + wantProject: "Contoso", + }, + { + name: "visualstudio org-only", + raw: "https://myorg.visualstudio.com", + wantOrg: "myorg", + wantProject: "", + }, + { + name: "uppercase host", + raw: "https://Dev.Azure.com/MyOrg/Contoso/_apis/wit/workItems/77", + wantOrg: "myorg", + wantProject: "Contoso", + }, + { + name: "non-azure host", + raw: "https://example.com/1", + wantOrg: "example.com", + wantProject: "1", + }, + { + name: "malformed", + raw: "not a url", + wantOrg: "", + wantProject: "", + }, + { + name: "empty", + raw: "", + wantOrg: "", + wantProject: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + gotOrg, gotProject := ParseWorkItemURL(tt.raw) + assert.Equal(t, tt.wantOrg, gotOrg) + assert.Equal(t, tt.wantProject, gotProject) + }) + } +} diff --git a/internal/cmd/util/scope.go b/internal/cmd/util/scope.go index 6eb3c904..447b7f43 100644 --- a/internal/cmd/util/scope.go +++ b/internal/cmd/util/scope.go @@ -11,18 +11,54 @@ import ( // Path represents a parsed user-input scope of the form // [ORG:][PROJECT/]TARGET[/TARGET...] or [ORG:]/TARGET[/TARGET...]. -// Organization is always populated after a successful Parse. +// Organization is always populated after a successful Parse, except when +// DisallowOrganization is set, in which case it is left empty. type Path struct { Organization string Project string Targets []string } -// ParseOptions configures how a raw user input is split into a Path. +// ParseOptions configures how raw command input is split into a Path. // -// The organization is always taken from an explicit ORG: prefix. A bare leading -// segment is never treated as an organization by structured parsing; only -// ParseOrganizationArg classifies a bare segment as an organization. +// The parser is shape-driven, never heuristic: an explicit "ORG:" prefix +// carries the organization, a leading "/" marks the no-project form, and a +// bare leading segment is always a project unless AllowBareTargets +// reclassifies it as a target. The options select which shapes a mode accepts +// and how many targets it expects; they never change what a shape means. +// +// The options fall into three axes plus the two target-count bounds: +// +// Organization: +// AllowImplicitOrg ORG: may be omitted; when omitted, Parse loads the +// default organization from +// ctx.Config().Authentication().GetDefaultOrganization(). +// DisallowOrganization ORG: is rejected and no organization is resolved; +// Path.Organization is left empty. +// -> Mutually exclusive. +// +// Project: +// RequireProject the project-first form PROJECT/TARGET... is +// required; the "/" no-project form is rejected. +// DisallowProject only the no-project form /TARGET... is accepted; +// any project segment is rejected. +// -> Mutually exclusive. With both unset, both forms are accepted. +// +// Targets: +// MinTargets minimum trailing target count. +// MaxTargets maximum trailing target count; 0 means unbounded. +// DisallowTargets rejects inputs that carry target segments. This is +// the only way to express "no targets", because +// MaxTargets == 0 means unbounded. +// AllowBareTargets reclassifies a lone segment without the "/" marker +// as a bare target instead of a project. +// -> AllowBareTargets cannot be combined with RequireProject, +// DisallowProject, or DisallowTargets. +// +// Contradictory combinations are rejected by validateParseOptions before any +// input is parsed. The wrappers below (ParseScope, ParseTarget, ...) are the +// intended entry points; most callers should pick one instead of composing +// ParseOptions by hand. type ParseOptions struct { // AllowImplicitOrg allows the ORG: prefix to be omitted. When omitted, Parse // loads the default organization from ctx.Config().Authentication().GetDefaultOrganization(). @@ -33,6 +69,16 @@ type ParseOptions struct { // use the leading-slash no-project marker, for example "/POOL/AGENT" or // "ORG:/POOL/AGENT". DisallowProject bool + // DisallowOrganization rejects inputs that carry an explicit ORG: prefix. + // When set, no organization is resolved at all — not even the configured + // default — and Path.Organization is left empty. It cannot be combined + // with AllowImplicitOrg. Combined with DisallowProject it accepts only + // the no-project form, for example "/TARGET". + DisallowOrganization bool + // AllowBareTargets reclassifies a lone segment without the "/" marker as a + // target instead of a project. It cannot be combined with RequireProject, + // DisallowProject, or DisallowTargets. + AllowBareTargets bool // DisallowTargets rejects inputs that carry target segments. It must be set // whenever a wrapper accepts no targets at all; MaxTargets cannot express // this because MaxTargets == 0 means unbounded. @@ -49,33 +95,35 @@ type ParseOptions struct { // organization, project, or target: an explicit "ORG:" prefix carries the // organization and a leading "/" marks the no-project form. // -// The input is trimmed, then an optional ORG: prefix is recognized. The -// remainder is split on "/", each segment is trimmed, and empty segments are -// rejected. After the organization prefix the grammar is: +// # Input grammar +// +// The input is trimmed, then an optional ORG: prefix is recognized, then the +// remainder is split on "/" into trimmed segments (empty segments are +// rejected). After the organization prefix the grammar has three shapes: // // PROJECT/TARGET... project-first form; the first segment is the project // /TARGET... no-project form; every segment is a target +// TARGET bare-target form; a lone segment counts as a target +// instead of a project when AllowBareTargets is set // -// The project rule (required, optional, or disallowed) and the target range -// select the valid shapes for a mode: +// # How the options select valid shapes // -// - AllowImplicitOrg allows the ORG: prefix to be omitted. When omitted, Parse -// loads the default organization from the user configuration. -// - RequireProject requires the project-first form. -// - DisallowProject requires the no-project form. -// - DisallowTargets rejects any target segments. -// - MinTargets defines the required trailing target count. -// - MaxTargets defines the allowed trailing target count. Zero means unbounded; -// DisallowTargets is the only way to express that no targets are allowed. +// AllowImplicitOrg ORG: optional; falls back to the default organization +// DisallowOrganization ORG: forbidden; organization never resolved +// RequireProject project-first form required +// DisallowProject no-project form required +// DisallowTargets target segments forbidden +// MinTargets/MaxTargets trailing target count range; 0 means unbounded +// AllowBareTargets lone segment is a target, not a project // -// Ambiguous inputs such as a legacy "ORG/SUBJECT" are interpreted as canonical -// project-first forms (PROJECT/SUBJECT) and are never auto-detected as an -// organization. Structurally detectable legacy organization forms — for example -// "ORG/PROJECT" where a mode cannot accept a project-plus-extra-segment shape — -// are rejected with ORG: guidance. The no-project form requires the "/" marker; -// an organization-only input in a mode that accepts targets must use "ORG:/". +// Project and organization options are mutually exclusive within their axes; +// see ParseOptions for the full compatibility rules. With no project option +// set, both project-first and no-project forms are accepted. Ambiguous inputs +// such as a legacy "ORG/SUBJECT" are interpreted as canonical project-first +// forms and never auto-detected as an organization; structurally detectable +// legacy organization forms are rejected with ORG: guidance. // -// Examples: +// # Examples // // Parse(ctx, "org:/group", ParseOptions{AllowImplicitOrg: false, MinTargets: 1, MaxTargets: 1}) // // => &Path{Organization: "org", Targets: []string{"group"}} @@ -98,18 +146,29 @@ type ParseOptions struct { // Parse(ctx, "/pool/agent", ParseOptions{AllowImplicitOrg: true, DisallowProject: true, MinTargets: 2, MaxTargets: 2}) // // => &Path{Organization: , Targets: []string{"pool", "agent"}} // -// Error conditions: -// - opts are invalid, for example a negative target count, MaxTargets below -// MinTargets, DisallowTargets combined with a positive target bound, or -// RequireProject combined with DisallowProject +// Parse(nil, "project/5", ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}) +// // => &Path{Project: "project", Targets: []string{"5"}} +// +// Parse(nil, "5678", ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}) +// // => &Path{Targets: []string{"5678"}} +// +// # Error conditions +// +// - opts are invalid: a negative target count, MaxTargets below MinTargets, +// DisallowTargets combined with a positive target bound, RequireProject +// combined with DisallowProject, DisallowOrganization combined with +// AllowImplicitOrg, or AllowBareTargets combined with RequireProject, +// DisallowProject, or DisallowTargets // - the input contains multiple or misplaced colons, an empty ORG: prefix, or // an empty segment after trimming, for example "org:project/" or "org:/ /x" -// - the input shape violates the project rule or the target range +// - the input carries an explicit ORG: prefix while DisallowOrganization is set +// - the input shape violates the project rule or the target range, including +// a bare target when AllowBareTargets is unset // - an explicit organization is required but the ORG: prefix is missing // - a structurally detectable legacy ORGANIZATION/... form is used without ORG: // - "ORG:" is used in a mode that accepts targets instead of the "ORG:/" marker -// - organization is omitted but ctx is nil -// - organization is omitted and the default organization lookup fails or returns empty +// - organization is omitted but ctx is nil, or the default organization lookup +// fails or returns empty func Parse(ctx CmdContext, raw string, opts ParseOptions) (*Path, error) { if err := validateParseOptions(opts); err != nil { return nil, err @@ -120,9 +179,15 @@ func Parse(ctx CmdContext, raw string, opts ParseOptions) (*Path, error) { return nil, err } + // An explicit ORG: prefix is never allowed when organizations are + // disallowed for this mode. + if hasOrg && opts.DisallowOrganization { + return nil, fmt.Errorf("invalid input %q: organization is not allowed (expected %s)", raw, expectedForms(opts)) + } + // An explicit ORG: prefix is mandatory when the mode does not allow the // default organization. - if !hasOrg && !opts.AllowImplicitOrg { + if !hasOrg && !opts.AllowImplicitOrg && !opts.DisallowOrganization { return nil, fmt.Errorf("invalid input %q: explicit organization is required, use ORG: syntax (expected %s)", raw, expectedForms(opts)) } @@ -143,6 +208,12 @@ func Parse(ctx CmdContext, raw string, opts ParseOptions) (*Path, error) { if len(segments) > 1 { targets = segments[1:] } + // A lone segment is a bare target instead of a project when the mode + // opts into AllowBareTargets. + if opts.AllowBareTargets && len(segments) == 1 { + project = "" + targets = segments + } if opts.DisallowProject && len(segments) > 0 { return nil, fmt.Errorf("invalid input %q: project is not allowed, use the / no-project marker (expected %s)", raw, expectedForms(opts)) } @@ -159,7 +230,11 @@ func Parse(ctx CmdContext, raw string, opts ParseOptions) (*Path, error) { ((opts.DisallowTargets && len(segments) > 1) || (opts.MaxTargets > 0 && opts.MinTargets == opts.MaxTargets && len(segments) > opts.MaxTargets+1)) if legacy { - return nil, fmt.Errorf("invalid input %q: legacy ORGANIZATION/... form is not supported, use ORG: syntax (expected %s)", raw, expectedForms(opts)) + guidance := "legacy ORGANIZATION/... form is not supported, use ORG: syntax" + if opts.DisallowOrganization { + guidance = "organization is not allowed" + } + return nil, fmt.Errorf("invalid input %q: %s (expected %s)", raw, guidance, expectedForms(opts)) } // Organization-only input requires the "/" marker whenever targets are // allowed, so the explicit no-project shape is unambiguous. @@ -178,7 +253,7 @@ func Parse(ctx CmdContext, raw string, opts ParseOptions) (*Path, error) { return nil, targetCountError(raw, opts, len(targets)) } - if org == "" { + if org == "" && !opts.DisallowOrganization { var err error org, err = defaultOrganization(ctx) if err != nil { @@ -212,12 +287,37 @@ func validateParseOptions(opts ParseOptions) error { if opts.RequireProject && opts.DisallowProject { return fmt.Errorf("invalid options: project cannot be required and disallowed at the same time") } + if opts.DisallowOrganization && opts.AllowImplicitOrg { + return fmt.Errorf("invalid options: organization cannot be disallowed when the implicit organization is allowed") + } + if opts.AllowBareTargets && opts.RequireProject { + return fmt.Errorf("invalid options: bare targets cannot be combined with a required project") + } + if opts.AllowBareTargets && opts.DisallowProject { + return fmt.Errorf("invalid options: bare targets cannot be combined with a disallowed project") + } + if opts.AllowBareTargets && opts.DisallowTargets { + return fmt.Errorf("invalid options: bare targets cannot be combined with disallowed targets") + } return nil } // expectedForms describes the canonical input shapes of a mode for error -// messages, so every syntax error points users to the ORG: and "/" forms. +// messages, so every syntax error points users to the ORG: and "/" forms. ORG: +// alternatives are omitted when organizations are disallowed for the mode. func expectedForms(opts ParseOptions) string { + if opts.DisallowOrganization { + switch { + case opts.DisallowTargets: + return "PROJECT" + case opts.DisallowProject: + return "/TARGET..." + case opts.RequireProject: + return "PROJECT/TARGET..." + default: + return targetForms(opts) + } + } switch { case opts.DisallowTargets && opts.RequireProject: return "PROJECT or ORG:PROJECT" @@ -230,8 +330,21 @@ func expectedForms(opts ParseOptions) string { case opts.RequireProject: return "PROJECT/TARGET... or ORG:PROJECT/TARGET..." default: - return "/TARGET..., PROJECT/TARGET..., ORG:/TARGET..., or ORG:PROJECT/TARGET..." + return targetForms(opts) + } +} + +// targetForms lists the target-bearing shapes, including the bare-target form +// when AllowBareTargets is set. +func targetForms(opts ParseOptions) string { + bare := "" + if opts.AllowBareTargets { + bare = "TARGET, " + } + if opts.DisallowOrganization { + return bare + "/TARGET... or PROJECT/TARGET..." } + return bare + "/TARGET..., PROJECT/TARGET..., ORG:/TARGET..., or ORG:PROJECT/TARGET..." } // targetCountError builds the target cardinality error for a mode. diff --git a/internal/cmd/util/scope_test.go b/internal/cmd/util/scope_test.go index f70576af..c6977d2c 100644 --- a/internal/cmd/util/scope_test.go +++ b/internal/cmd/util/scope_test.go @@ -935,6 +935,146 @@ func TestParse(t *testing.T) { opts: util.ParseOptions{AllowImplicitOrg: true, MinTargets: 1, MaxTargets: 1}, wantErr: "no organization specified and no default organization configured", }, + { + name: "disallowed organization with project target", + raw: "project/group", + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Project: "project", Targets: []string{"group"}}, + }, + { + name: "disallowed organization with no-project target", + raw: "/group", + opts: util.ParseOptions{DisallowOrganization: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Targets: []string{"group"}}, + }, + { + name: "disallowed organization rejects explicit organization", + raw: "myorg:project/group", + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "organization is not allowed", + }, + { + name: "disallowed organization rejects legacy organization form", + raw: "myorg/project/group", + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "organization is not allowed", + }, + { + name: "disallowed organization skips default organization lookup", + raw: "project/group", + ctx: emptyOrgCtx(t), + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Project: "project", Targets: []string{"group"}}, + }, + { + name: "disallowed organization with bare project input", + raw: "project", + ctx: emptyOrgCtx(t), + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, DisallowTargets: true}, + want: &util.Path{Project: "project"}, + }, + { + name: "bare target without organization", + raw: "5678", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Targets: []string{"5678"}}, + }, + { + name: "bare target with default organization", + raw: "5678", + opts: util.ParseOptions{AllowImplicitOrg: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Organization: "default-org", Targets: []string{"5678"}}, + }, + { + name: "bare target rejects explicit organization", + raw: "myorg:5678", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "organization is not allowed", + }, + { + name: "bare target marker form still accepted", + raw: "/5678", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Targets: []string{"5678"}}, + }, + { + name: "project target still project first with bare targets", + raw: "Contoso/5678", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Project: "Contoso", Targets: []string{"5678"}}, + }, + { + name: "two segments are not bare targets", + raw: "a/b", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Project: "a", Targets: []string{"b"}}, + }, + { + name: "bare target with too many targets", + raw: "a/b/c", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "organization is not allowed", + }, + { + name: "bare target too few targets", + raw: "", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "expected exactly 1 targets, got 0", + }, + { + name: "bare target without organization constraints", + raw: "5678", + opts: util.ParseOptions{DisallowOrganization: true, AllowBareTargets: true}, + want: &util.Path{Targets: []string{"5678"}}, + }, + { + name: "disallowed organization and project with no-project target", + raw: "/group", + opts: util.ParseOptions{DisallowOrganization: true, DisallowProject: true, MinTargets: 1, MaxTargets: 1}, + want: &util.Path{Targets: []string{"group"}}, + }, + { + name: "disallowed organization and project rejects project-first form", + raw: "project/group", + opts: util.ParseOptions{DisallowOrganization: true, DisallowProject: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "project is not allowed, use the / no-project marker", + }, + { + name: "disallowed organization rejects explicit organization with no-project marker", + raw: "myorg:/group", + opts: util.ParseOptions{DisallowOrganization: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "organization is not allowed", + }, + { + name: "disallowed organization rejects organization-only input", + raw: "myorg:", + opts: util.ParseOptions{DisallowOrganization: true, DisallowTargets: true}, + wantErr: "organization is not allowed", + }, + { + name: "disallowed organization requires project with no-project marker", + raw: "/group", + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "project is required", + }, + { + name: "disallowed organization empty input requires project", + raw: "", + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "project is required", + }, + { + name: "disallowed organization empty input with no constraints", + raw: "", + opts: util.ParseOptions{DisallowOrganization: true}, + want: &util.Path{}, + }, + { + name: "disallowed organization too few targets", + raw: "project", + opts: util.ParseOptions{DisallowOrganization: true, RequireProject: true, MinTargets: 1, MaxTargets: 1}, + wantErr: "expected exactly 1 targets, got 0", + }, } for i := range tests { @@ -947,6 +1087,17 @@ func TestParse(t *testing.T) { } } +func TestParse_DisallowOrganizationNilContext(t *testing.T) { + got, err := util.Parse(nil, "project/group", util.ParseOptions{ + DisallowOrganization: true, + RequireProject: true, + MinTargets: 1, + MaxTargets: 1, + }) + require.NoError(t, err) + assert.Equal(t, &util.Path{Project: "project", Targets: []string{"group"}}, got) +} + func TestParseInvalidOptions(t *testing.T) { tests := []struct { name string @@ -991,6 +1142,30 @@ func TestParseInvalidOptions(t *testing.T) { opts: util.ParseOptions{RequireProject: true, DisallowProject: true}, wantErr: "project cannot be required and disallowed at the same time", }, + { + name: "organization disallowed with implicit organization", + raw: "org:/a/b", + opts: util.ParseOptions{AllowImplicitOrg: true, DisallowOrganization: true}, + wantErr: "organization cannot be disallowed when the implicit organization is allowed", + }, + { + name: "bare targets with required project", + raw: "a/b", + opts: util.ParseOptions{AllowBareTargets: true, RequireProject: true}, + wantErr: "bare targets cannot be combined with a required project", + }, + { + name: "bare targets with disallowed project", + raw: "/a/b", + opts: util.ParseOptions{AllowBareTargets: true, DisallowProject: true}, + wantErr: "bare targets cannot be combined with a disallowed project", + }, + { + name: "bare targets with disallowed targets", + raw: "a/b", + opts: util.ParseOptions{AllowBareTargets: true, DisallowTargets: true}, + wantErr: "bare targets cannot be combined with disallowed targets", + }, { name: "disallowed targets with zero bounds is valid", raw: "org:",