From 3dce3b0a783f5d21ffc472a9352ee132c26fced5 Mon Sep 17 00:00:00 2001 From: Neil Martin Date: Fri, 15 May 2026 15:08:58 +0100 Subject: [PATCH 1/3] fix(platform): add --device-type flag to disambiguate device group name lookup When a COMPUTER and MOBILE group share the same name, `get --name` (and all other --name-based ops) previously silently returned the wrong group. - Override generated get/delete/patch/members/patch-members with handwritten versions that accept --device-type COMPUTER|MOBILE (case-insensitive) - apply auto-derives device type from input JSON's deviceType field - add-members / remove-members gain --device-type - ResolveIDByNameFiltered added to internal/platform for filtered name walks - ResolveIDByName now errors on ambiguous matches (same name twice per page) Fixes #209 Co-Authored-By: Claude Sonnet 4.6 --- .../commands/pro_platform_device_groups.go | 324 +++++++++++++++++- internal/platform/resolve_generic.go | 63 +++- internal/platform/resolve_generic_test.go | 56 +++ 3 files changed, 423 insertions(+), 20 deletions(-) diff --git a/internal/commands/pro_platform_device_groups.go b/internal/commands/pro_platform_device_groups.go index 49abb6cb..b0e3c683 100644 --- a/internal/commands/pro_platform_device_groups.go +++ b/internal/commands/pro_platform_device_groups.go @@ -3,14 +3,21 @@ package commands import ( + "context" + "encoding/json" "fmt" + "net/http" + "net/url" "os" + "strconv" + "strings" "github.com/spf13/cobra" platformgen "github.com/Jamf-Concepts/jamf-cli/internal/commands/platform/generated" "github.com/Jamf-Concepts/jamf-cli/internal/platform" "github.com/Jamf-Concepts/jamf-cli/internal/registry" + "github.com/Jamf-Concepts/jamfplatform-go-sdk/jamfplatform" "github.com/Jamf-Concepts/jamfplatform-go-sdk/jamfplatform/devicegroups" ) @@ -21,12 +28,26 @@ func newPlatformDeviceGroupsCmd(cliCtx *registry.CLIContext) *cobra.Command { Long: "Create and manage unified device groups via the Jamf Platform API. Requires platform gateway auth.", } - // Generated CRUD and member ops (list, create, get, patch, delete, members, patch-members) + // Generated CRUD: list, create. Skip --name-using ops; replaced below with + // handwritten versions that add --device-type for COMPUTER/MOBILE disambiguation. + needsType := map[string]bool{ + "get": true, "delete": true, "patch": true, "members": true, "patch-members": true, + } for _, sub := range platformgen.NewDeviceGroupsCmd(cliCtx).Commands() { + if needsType[sub.Name()] { + continue + } cmd.AddCommand(sub) } - // Business logic: upsert apply and ergonomic member mutations + // Name-based CRUD with --device-type disambiguation + cmd.AddCommand(newPDGGetCmd(cliCtx)) + cmd.AddCommand(newPDGDeleteCmd(cliCtx)) + cmd.AddCommand(newPDGPatchCmd(cliCtx)) + cmd.AddCommand(newPDGMembersCmd(cliCtx)) + cmd.AddCommand(newPDGPatchMembersCmd(cliCtx)) + + // Business logic: upsert and ergonomic member mutations cmd.AddCommand(newPDGApplyCmd(cliCtx)) cmd.AddCommand(newPDGAddMembersCmd(cliCtx)) cmd.AddCommand(newPDGRemoveMembersCmd(cliCtx)) @@ -34,6 +55,276 @@ func newPlatformDeviceGroupsCmd(cliCtx *registry.CLIContext) *cobra.Command { return cmd } +// pdgListPath returns the tenant-prefixed list endpoint for device groups. +func pdgListPath(c *jamfplatform.Client) string { + return "/api/device-groups/v1/tenant/" + url.PathEscape(c.Transport().TenantID()) + "/device-groups" +} + +// pdgResolveID resolves a device group name to its ID, optionally filtering by +// deviceType ("COMPUTER" or "MOBILE"). When deviceType is empty the lookup +// searches all groups; if two groups share a name the call errors with a hint +// to add --device-type. +func pdgResolveID(ctx context.Context, c *jamfplatform.Client, name, deviceType string) (string, error) { + filter := "" + if deviceType != "" { + filter = fmt.Sprintf(`deviceType=="%s"`, deviceType) + } + return platform.ResolveIDByNameFiltered(ctx, c, pdgListPath(c), name, filter) +} + +// normalizeDeviceTypeFlag uppercases the value and validates it is COMPUTER, +// MOBILE, or empty. Returns the normalized value and an error if invalid. +func normalizeDeviceTypeFlag(t string) (string, error) { + upper := strings.ToUpper(t) + if upper != "" && upper != "COMPUTER" && upper != "MOBILE" { + return "", fmt.Errorf("--device-type must be COMPUTER or MOBILE (got %q)", t) + } + return upper, nil +} + +func newPDGGetCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag string + cmd := &cobra.Command{ + Use: "get ", + Short: "Get a device group by ID", + Long: "Retrieve a specific device group by its ID", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } + var resolvedID string + if nameFlag != "" { + id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) + if err != nil { + return err + } + resolvedID = id + } else if len(args) == 1 { + resolvedID = args[0] + } else { + return fmt.Errorf("provide a positional ID or --name") + } + path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + var result any + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, path, nil, http.StatusOK, &result); err != nil { + return fmt.Errorf("get: %w", err) + } + if result == nil { + return nil + } + b, err := json.MarshalIndent(result, "", " ") + if err != nil { + return err + } + return cliCtx.Output.PrintRaw(b) + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + return cmd +} + +func newPDGDeleteCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag string + var yes bool + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete a device group", + Long: "Delete an existing device group", + Annotations: map[string]string{"jamf:destructive": "true"}, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } + var resolvedID string + if nameFlag != "" { + id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) + if err != nil { + return err + } + resolvedID = id + } else if len(args) == 1 { + resolvedID = args[0] + } else { + return fmt.Errorf("provide a positional ID or --name") + } + if err := platform.ConfirmAction("delete", resolvedID, yes); err != nil { + return err + } + path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodDelete, path, nil, http.StatusNoContent, nil); err != nil { + return fmt.Errorf("delete: %w", err) + } + return nil + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + cmd.Flags().BoolVar(&yes, "yes", false, "Skip confirmation prompt") + return cmd +} + +func newPDGPatchCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag, bodyFile string + var setFlags []string + var scaffoldFlag bool + cmd := &cobra.Command{ + Use: "patch ", + Short: "Update a device group", + Long: "Update an existing device group", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if scaffoldFlag { + fmt.Println("{\n \"criteria\": [],\n \"description\": \"\",\n \"name\": \"\"\n}") + return nil + } + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } + var resolvedID string + if nameFlag != "" { + id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) + if err != nil { + return err + } + resolvedID = id + } else if len(args) == 1 { + resolvedID = args[0] + } else { + return fmt.Errorf("provide a positional ID or --name") + } + path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + body, err := platform.ReadBody(bodyFile, setFlags) + if err != nil { + return err + } + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodPatch, path, body, http.StatusNoContent, nil); err != nil { + return fmt.Errorf("patch: %w", err) + } + return nil + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + cmd.Flags().StringVar(&bodyFile, "file", "", "Path to JSON file containing the request body") + cmd.Flags().StringArrayVar(&setFlags, "set", nil, "Override body values (key=value, repeatable, supports nested.keys)") + cmd.Flags().BoolVar(&scaffoldFlag, "scaffold", false, "Print an example request body and exit") + return cmd +} + +func newPDGMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag string + cmd := &cobra.Command{ + Use: "members ", + Short: "Get group members", + Long: "Retrieve all members of a device group", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } + var resolvedID string + if nameFlag != "" { + id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) + if err != nil { + return err + } + resolvedID = id + } else if len(args) == 1 { + resolvedID = args[0] + } else { + return fmt.Errorf("provide a positional ID or --name") + } + path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + "/members" + var result any + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, path, nil, http.StatusOK, &result); err != nil { + return fmt.Errorf("members: %w", err) + } + if result == nil { + return nil + } + b, err := json.MarshalIndent(result, "", " ") + if err != nil { + return err + } + return cliCtx.Output.PrintRaw(b) + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + return cmd +} + +func newPDGPatchMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { + var nameFlag, deviceTypeFlag, bodyFile string + var setFlags []string + var scaffoldFlag bool + cmd := &cobra.Command{ + Use: "patch-members ", + Short: "Update device group members", + Long: "Add devices to or remove devices from a static device group. Cannot be used with smart groups.", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if scaffoldFlag { + fmt.Println("{\n \"added\": [],\n \"removed\": []\n}") + return nil + } + if err := requirePlatformClient(cliCtx); err != nil { + return err + } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } + var resolvedID string + if nameFlag != "" { + id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) + if err != nil { + return err + } + resolvedID = id + } else if len(args) == 1 { + resolvedID = args[0] + } else { + return fmt.Errorf("provide a positional ID or --name") + } + path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + "/members" + body, err := platform.ReadBody(bodyFile, setFlags) + if err != nil { + return err + } + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodPatch, path, body, http.StatusNoContent, nil); err != nil { + return fmt.Errorf("patch-members: %w", err) + } + return nil + }, + } + cmd.Flags().StringVar(&nameFlag, "name", "", "Resolve target by name instead of ID") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow --name lookup by device type: COMPUTER or MOBILE") + cmd.Flags().StringVar(&bodyFile, "file", "", "Path to JSON file containing the request body") + cmd.Flags().StringArrayVar(&setFlags, "set", nil, "Override body values (key=value, repeatable, supports nested.keys)") + cmd.Flags().BoolVar(&scaffoldFlag, "scaffold", false, "Print an example request body and exit") + return cmd +} + func newPDGApplyCmd(cliCtx *registry.CLIContext) *cobra.Command { var ( fromFile string @@ -65,8 +356,9 @@ func newPDGApplyCmd(cliCtx *registry.CLIContext) *cobra.Command { return fmt.Errorf("input must include a 'name' field") } - dg := devicegroups.New(cliCtx.PlatformSDKClient) - id, resolveErr := dg.ResolveDeviceGroupIDByName(ctx, createReq.Name) + // Use deviceType from the input JSON to disambiguate when a COMPUTER + // and MOBILE group share the same name. + id, resolveErr := pdgResolveID(ctx, cliCtx.PlatformSDKClient, createReq.Name, string(createReq.DeviceType)) if resolveErr != nil && !platform.IsNotFound(resolveErr) { return resolveErr } @@ -128,6 +420,7 @@ func deviceGroupScaffold() *devicegroups.DeviceGroupCreateRepresentationV1 { func newPDGAddMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { var ids []string + var deviceTypeFlag string cmd := &cobra.Command{ Use: "add-members ", Short: "Add devices to a static group", @@ -136,12 +429,15 @@ func newPDGAddMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } if len(ids) == 0 { return fmt.Errorf("at least one --id is required") } ctx := cmd.Context() - dg := devicegroups.New(cliCtx.PlatformSDKClient) - groupID, err := dg.ResolveDeviceGroupIDByName(ctx, args[0]) + groupID, err := pdgResolveID(ctx, cliCtx.PlatformSDKClient, args[0], dt) if err != nil { return err } @@ -156,11 +452,13 @@ func newPDGAddMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { }, } cmd.Flags().StringSliceVar(&ids, "id", nil, "Device ID to add (repeatable)") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow name lookup by device type: COMPUTER or MOBILE") return cmd } func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { var ids []string + var deviceTypeFlag string cmd := &cobra.Command{ Use: "remove-members ", Short: "Remove devices from a static group", @@ -169,12 +467,15 @@ func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return err + } if len(ids) == 0 { return fmt.Errorf("at least one --id is required") } ctx := cmd.Context() - dg := devicegroups.New(cliCtx.PlatformSDKClient) - groupID, err := dg.ResolveDeviceGroupIDByName(ctx, args[0]) + groupID, err := pdgResolveID(ctx, cliCtx.PlatformSDKClient, args[0], dt) if err != nil { return err } @@ -189,5 +490,12 @@ func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { }, } cmd.Flags().StringSliceVar(&ids, "id", nil, "Device ID to remove (repeatable)") + cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow name lookup by device type: COMPUTER or MOBILE") return cmd } + +// guards against unused-import errors +var ( + _ = strings.Replace + _ = strconv.Itoa +) diff --git a/internal/platform/resolve_generic.go b/internal/platform/resolve_generic.go index e5d8aaed..32e6517e 100644 --- a/internal/platform/resolve_generic.go +++ b/internal/platform/resolve_generic.go @@ -28,10 +28,31 @@ import ( // "name", "title", and "displayName" properties in that order. The ID is read // from "id" (and falls back to "blueprintId", "groupId", "deviceId" for // resources that use a non-standard ID field). +// +// Returns an error when multiple items share the name (ambiguous match within +// a single page). Use ResolveIDByNameFiltered to narrow the lookup first. func ResolveIDByName(ctx context.Context, client *jamfplatform.Client, listPath string, name string) (string, error) { + return ResolveIDByNameFiltered(ctx, client, listPath, name, "") +} + +// ResolveIDByNameFiltered is like ResolveIDByName but narrows the server-side +// results with an RSQL filter expression appended as ?filter= before the +// name walk begins. Pass an empty string for no additional filtering. +// +// Example: ResolveIDByNameFiltered(ctx, c, path, "My Group", `deviceType=="COMPUTER"`) +func ResolveIDByNameFiltered(ctx context.Context, client *jamfplatform.Client, listPath string, name string, filter string) (string, error) { if name == "" { return "", fmt.Errorf("empty name") } + if filter != "" { + q := url.Values{} + q.Set("filter", filter) + sep := "?" + if u, err := url.Parse(listPath); err == nil && u.RawQuery != "" { + sep = "&" + } + listPath = listPath + sep + q.Encode() + } const pageSize = 100 @@ -44,12 +65,10 @@ func ResolveIDByName(ctx context.Context, client *jamfplatform.Client, listPath return "", fmt.Errorf("listing %s: %w", listPath, err) } items, paged := extractItems(raw) - for _, item := range items { - if matchesName(item, name) { - if id := extractID(item); id != "" { - return id, nil - } - } + if id, err := firstMatch(items, name); err != nil { + return "", err + } else if id != "" { + return id, nil } // Paginate only if the first response signalled more pages. @@ -69,12 +88,10 @@ func ResolveIDByName(ctx context.Context, client *jamfplatform.Client, listPath return "", fmt.Errorf("listing %s: %w", listPath, err) } pageItems, _ := extractItems(pageRaw) - for _, item := range pageItems { - if matchesName(item, name) { - if id := extractID(item); id != "" { - return id, nil - } - } + if id, err := firstMatch(pageItems, name); err != nil { + return "", err + } else if id != "" { + return id, nil } if len(pageItems) < pageSize { break @@ -121,6 +138,28 @@ func extractItems(raw json.RawMessage) ([]map[string]any, bool) { return nil, false } +// firstMatch scans items for entries whose name matches. Returns the ID of the +// first match, or an error when multiple items share the name within this page +// (ambiguous). Returns ("", nil) when no match is found. +func firstMatch(items []map[string]any, name string) (string, error) { + var matched []string + for _, item := range items { + if matchesName(item, name) { + if id := extractID(item); id != "" { + matched = append(matched, id) + } + } + } + switch len(matched) { + case 0: + return "", nil + case 1: + return matched[0], nil + default: + return "", fmt.Errorf("ambiguous match: %d items named %q; use a positional ID or add --type to narrow", len(matched), name) + } +} + func matchesName(item map[string]any, name string) bool { for _, key := range []string{"name", "title", "displayName"} { if v, ok := item[key].(string); ok && v == name { diff --git a/internal/platform/resolve_generic_test.go b/internal/platform/resolve_generic_test.go index 5d97d161..55cd812e 100644 --- a/internal/platform/resolve_generic_test.go +++ b/internal/platform/resolve_generic_test.go @@ -156,6 +156,62 @@ func TestResolveIDByName_NotFound(t *testing.T) { } } +// TestResolveIDByName_Ambiguous verifies that when two items share the same +// name on the same page, an error is returned rather than silently picking one. +func TestResolveIDByName_Ambiguous(t *testing.T) { + mux := http.NewServeMux() + path := "/api/device-groups/v1/tenant/" + resolveTestTenantID + "/device-groups" + mux.HandleFunc(path, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "results": []any{ + map[string]any{"id": "dg-computer", "name": "All Managed", "deviceType": "COMPUTER"}, + map[string]any{"id": "dg-mobile", "name": "All Managed", "deviceType": "MOBILE"}, + }, + "totalCount": 2, + }) + }) + client := newResolveTestClient(t, mux) + + _, err := ResolveIDByName(context.Background(), client, path, "All Managed") + if err == nil { + t.Fatal("expected ambiguous error, got nil") + } + if IsNotFound(err) { + t.Errorf("should not be ErrNotFound, got %v", err) + } +} + +// TestResolveIDByNameFiltered verifies that the filter expression is appended +// as a query param and narrows the result to the matching device type. +func TestResolveIDByNameFiltered(t *testing.T) { + mux := http.NewServeMux() + path := "/api/device-groups/v1/tenant/" + resolveTestTenantID + "/device-groups" + var capturedFilter string + mux.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { + capturedFilter = r.URL.Query().Get("filter") + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "results": []any{ + map[string]any{"id": "dg-computer", "name": "All Managed", "deviceType": "COMPUTER"}, + }, + "totalCount": 1, + }) + }) + client := newResolveTestClient(t, mux) + + id, err := ResolveIDByNameFiltered(context.Background(), client, path, "All Managed", `deviceType=="COMPUTER"`) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if id != "dg-computer" { + t.Errorf("id = %q, want %q", id, "dg-computer") + } + if capturedFilter != `deviceType=="COMPUTER"` { + t.Errorf("filter = %q, want %q", capturedFilter, `deviceType=="COMPUTER"`) + } +} + // TestResolveIDByName_BareArray verifies matching against bare-array responses. func TestResolveIDByName_BareArray(t *testing.T) { mux := http.NewServeMux() From 42d997de4ea030e251f510fd5a480929c035482d Mon Sep 17 00:00:00 2001 From: Neil Martin Date: Fri, 15 May 2026 15:09:01 +0100 Subject: [PATCH 2/3] chore: apply go fix modernizations (range-over-int, SplitSeq, scope tag) Co-Authored-By: Claude Sonnet 4.6 --- internal/output/list_hint_test.go | 2 +- internal/scope/types.go | 2 +- scripts/lint-dead-code/scan.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/output/list_hint_test.go b/internal/output/list_hint_test.go index 30fa4ea7..8031bee4 100644 --- a/internal/output/list_hint_test.go +++ b/internal/output/list_hint_test.go @@ -94,7 +94,7 @@ func TestListHint_PrintRawJSONFastPath_TopLevelArray(t *testing.T) { // Top-level JSON array of 60 items, no projector → fast path triggers. var b bytes.Buffer b.WriteString("[") - for i := 0; i < 60; i++ { + for i := range 60 { if i > 0 { b.WriteString(",") } diff --git a/internal/scope/types.go b/internal/scope/types.go index c62a928e..da957ca4 100644 --- a/internal/scope/types.go +++ b/internal/scope/types.go @@ -163,7 +163,7 @@ type ScopeXML struct { JSSUserGroups ScopeItemSlice `xml:"jss_user_groups" json:"jss_user_groups"` Buildings ScopeItemSlice `xml:"buildings" json:"buildings"` Departments ScopeItemSlice `xml:"departments" json:"departments"` - Classes ScopeItemSlice `xml:"classes" json:"classes,omitempty"` + Classes ScopeItemSlice `xml:"classes" json:"classes"` LimitToUsers *LimitToUsersXML `xml:"limit_to_users,omitempty" json:"limit_to_users,omitempty"` Limitations *LimitationsXML `xml:"limitations,omitempty" json:"limitations,omitempty"` Exclusions *ExclusionsXML `xml:"exclusions,omitempty" json:"exclusions,omitempty"` diff --git a/scripts/lint-dead-code/scan.go b/scripts/lint-dead-code/scan.go index 0eaf31f3..4606f8c3 100644 --- a/scripts/lint-dead-code/scan.go +++ b/scripts/lint-dead-code/scan.go @@ -343,7 +343,7 @@ func extractAllowlist(expr ast.Expr) map[string]bool { continue } set := map[string]bool{} - for _, p := range strings.Split(strings.Trim(vLit.Value, `"`), ",") { + for p := range strings.SplitSeq(strings.Trim(vLit.Value, `"`), ",") { if s := strings.TrimSpace(p); s != "" { set[s] = true } From f0dd80a9bfdfe0cb4cc8d7244e39efb21cefcae7 Mon Sep 17 00:00:00 2001 From: Neil Martin Date: Fri, 15 May 2026 16:32:09 +0100 Subject: [PATCH 3/3] fix(platform): address PR review feedback on pdg device-type flag - Fix ambiguous-match error to not reference --type (shared helper used by resources without that flag); now says "pass the positional ID" - Remove strconv import and dead _ = ... import guards left from refactor - Route patch/patch-members --scaffold through printScaffold so --out-file and -o flags are honoured - Extract resolvePDGTarget + pdgItemPath helpers; collapse repeated ~15-line name/positional/path-build block in all 5 commands Co-Authored-By: Claude Opus 4.7 (1M context) --- .../commands/pro_platform_device_groups.go | 126 ++++++------------ internal/platform/resolve_generic.go | 2 +- 2 files changed, 42 insertions(+), 86 deletions(-) diff --git a/internal/commands/pro_platform_device_groups.go b/internal/commands/pro_platform_device_groups.go index b0e3c683..d8a7cdab 100644 --- a/internal/commands/pro_platform_device_groups.go +++ b/internal/commands/pro_platform_device_groups.go @@ -9,7 +9,6 @@ import ( "net/http" "net/url" "os" - "strconv" "strings" "github.com/spf13/cobra" @@ -82,6 +81,27 @@ func normalizeDeviceTypeFlag(t string) (string, error) { return upper, nil } +// resolvePDGTarget normalizes --device-type, then resolves the group ID from +// either --name or a positional ID argument. +func resolvePDGTarget(ctx context.Context, cliCtx *registry.CLIContext, args []string, nameFlag, deviceTypeFlag string) (string, error) { + dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + if err != nil { + return "", err + } + if nameFlag != "" { + return pdgResolveID(ctx, cliCtx.PlatformSDKClient, nameFlag, dt) + } + if len(args) == 1 { + return args[0], nil + } + return "", fmt.Errorf("provide a positional ID or --name") +} + +// pdgItemPath returns the item-level endpoint for a device group ID. +func pdgItemPath(c *jamfplatform.Client, id string) string { + return pdgListPath(c) + "/" + url.PathEscape(id) +} + func newPDGGetCmd(cliCtx *registry.CLIContext) *cobra.Command { var nameFlag, deviceTypeFlag string cmd := &cobra.Command{ @@ -93,25 +113,12 @@ func newPDGGetCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } - dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) if err != nil { return err } - var resolvedID string - if nameFlag != "" { - id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) - if err != nil { - return err - } - resolvedID = id - } else if len(args) == 1 { - resolvedID = args[0] - } else { - return fmt.Errorf("provide a positional ID or --name") - } - path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) var result any - if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, path, nil, http.StatusOK, &result); err != nil { + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, pdgItemPath(cliCtx.PlatformSDKClient, id), nil, http.StatusOK, &result); err != nil { return fmt.Errorf("get: %w", err) } if result == nil { @@ -142,27 +149,14 @@ func newPDGDeleteCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } - dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) if err != nil { return err } - var resolvedID string - if nameFlag != "" { - id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) - if err != nil { - return err - } - resolvedID = id - } else if len(args) == 1 { - resolvedID = args[0] - } else { - return fmt.Errorf("provide a positional ID or --name") - } - if err := platform.ConfirmAction("delete", resolvedID, yes); err != nil { + if err := platform.ConfirmAction("delete", id, yes); err != nil { return err } - path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) - if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodDelete, path, nil, http.StatusNoContent, nil); err != nil { + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodDelete, pdgItemPath(cliCtx.PlatformSDKClient, id), nil, http.StatusNoContent, nil); err != nil { return fmt.Errorf("delete: %w", err) } return nil @@ -185,34 +179,24 @@ func newPDGPatchCmd(cliCtx *registry.CLIContext) *cobra.Command { Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if scaffoldFlag { - fmt.Println("{\n \"criteria\": [],\n \"description\": \"\",\n \"name\": \"\"\n}") - return nil + return printScaffold(map[string]any{ + "criteria": []any{}, + "description": "", + "name": "", + }) } if err := requirePlatformClient(cliCtx); err != nil { return err } - dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) if err != nil { return err } - var resolvedID string - if nameFlag != "" { - id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) - if err != nil { - return err - } - resolvedID = id - } else if len(args) == 1 { - resolvedID = args[0] - } else { - return fmt.Errorf("provide a positional ID or --name") - } - path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) body, err := platform.ReadBody(bodyFile, setFlags) if err != nil { return err } - if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodPatch, path, body, http.StatusNoContent, nil); err != nil { + if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodPatch, pdgItemPath(cliCtx.PlatformSDKClient, id), body, http.StatusNoContent, nil); err != nil { return fmt.Errorf("patch: %w", err) } return nil @@ -237,23 +221,11 @@ func newPDGMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { if err := requirePlatformClient(cliCtx); err != nil { return err } - dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) if err != nil { return err } - var resolvedID string - if nameFlag != "" { - id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) - if err != nil { - return err - } - resolvedID = id - } else if len(args) == 1 { - resolvedID = args[0] - } else { - return fmt.Errorf("provide a positional ID or --name") - } - path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + "/members" + path := pdgItemPath(cliCtx.PlatformSDKClient, id) + "/members" var result any if err := cliCtx.PlatformSDKClient.Transport().DoExpect(cmd.Context(), http.MethodGet, path, nil, http.StatusOK, &result); err != nil { return fmt.Errorf("members: %w", err) @@ -284,29 +256,19 @@ func newPDGPatchMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if scaffoldFlag { - fmt.Println("{\n \"added\": [],\n \"removed\": []\n}") - return nil + return printScaffold(map[string]any{ + "added": []any{}, + "removed": []any{}, + }) } if err := requirePlatformClient(cliCtx); err != nil { return err } - dt, err := normalizeDeviceTypeFlag(deviceTypeFlag) + id, err := resolvePDGTarget(cmd.Context(), cliCtx, args, nameFlag, deviceTypeFlag) if err != nil { return err } - var resolvedID string - if nameFlag != "" { - id, err := pdgResolveID(cmd.Context(), cliCtx.PlatformSDKClient, nameFlag, dt) - if err != nil { - return err - } - resolvedID = id - } else if len(args) == 1 { - resolvedID = args[0] - } else { - return fmt.Errorf("provide a positional ID or --name") - } - path := "/api/device-groups/v1/tenant/" + url.PathEscape(cliCtx.PlatformSDKClient.Transport().TenantID()) + "/device-groups/" + url.PathEscape(resolvedID) + "/members" + path := pdgItemPath(cliCtx.PlatformSDKClient, id) + "/members" body, err := platform.ReadBody(bodyFile, setFlags) if err != nil { return err @@ -493,9 +455,3 @@ func newPDGRemoveMembersCmd(cliCtx *registry.CLIContext) *cobra.Command { cmd.Flags().StringVar(&deviceTypeFlag, "device-type", "", "Narrow name lookup by device type: COMPUTER or MOBILE") return cmd } - -// guards against unused-import errors -var ( - _ = strings.Replace - _ = strconv.Itoa -) diff --git a/internal/platform/resolve_generic.go b/internal/platform/resolve_generic.go index 32e6517e..285d53d1 100644 --- a/internal/platform/resolve_generic.go +++ b/internal/platform/resolve_generic.go @@ -156,7 +156,7 @@ func firstMatch(items []map[string]any, name string) (string, error) { case 1: return matched[0], nil default: - return "", fmt.Errorf("ambiguous match: %d items named %q; use a positional ID or add --type to narrow", len(matched), name) + return "", fmt.Errorf("ambiguous match: %d items named %q; pass the positional ID to disambiguate", len(matched), name) } }