From 8baee7e7ccd73ceac7f4cef617c3c1dc1bb8bcdf Mon Sep 17 00:00:00 2001 From: Marta Date: Mon, 13 Jul 2026 10:54:40 +0200 Subject: [PATCH 1/3] feat: add teams resource (dhq teams list|show|create|update|delete) Adds account-level permission groups: SDK types + CRUD methods and a `dhq teams` command tree, following the established resource pattern (request bodies wrapped under `team`, bare responses, PATCH for update). Membership syncs via --user-ids on create/update; the update request uses *[]int so an explicit empty list clears all members while omitting the flag leaves membership untouched. Scalar update flags use pointers so partial updates don't clobber unset permissions. --- internal/assist/prompt.go | 1 + internal/commands/agent_metadata.go | 23 ++ internal/commands/root.go | 1 + internal/commands/teams.go | 332 ++++++++++++++++++++++++++++ internal/commands/teams_test.go | 101 +++++++++ pkg/sdk/teams.go | 143 ++++++++++++ pkg/sdk/teams_test.go | 245 ++++++++++++++++++++ 7 files changed, 846 insertions(+) create mode 100644 internal/commands/teams.go create mode 100644 internal/commands/teams_test.go create mode 100644 pkg/sdk/teams.go create mode 100644 pkg/sdk/teams_test.go diff --git a/internal/assist/prompt.go b/internal/assist/prompt.go index 3e07be3..2cce2d7 100644 --- a/internal/assist/prompt.go +++ b/internal/assist/prompt.go @@ -126,6 +126,7 @@ dhq templates list|show|public|public-show|create|update|delete Account Resources: dhq agents list|create|update|delete|revoke +dhq teams list|show|create|update|delete dhq global-servers list|show|create|update|delete|copy-to-project dhq zones list diff --git a/internal/commands/agent_metadata.go b/internal/commands/agent_metadata.go index 0002152..9236385 100644 --- a/internal/commands/agent_metadata.go +++ b/internal/commands/agent_metadata.go @@ -276,6 +276,29 @@ var commandMetadataTable = map[string]AgentMetadata{ Idempotent: true, SupportsJSON: true, SafeForAutomation: true, ResourceTypes: []string{"ssh_key"}, }, + + // Teams (account-level permission groups) + "dhq teams list": { + Idempotent: true, SupportsJSON: true, SafeForAutomation: true, + ResourceTypes: []string{"team"}, + }, + "dhq teams show": { + Idempotent: true, SupportsJSON: true, SafeForAutomation: true, + ResourceTypes: []string{"team"}, + }, + "dhq teams create": { + Idempotent: false, SupportsJSON: true, SafeForAutomation: true, + ResourceTypes: []string{"team"}, + }, + "dhq teams update": { + Idempotent: true, SupportsJSON: true, SafeForAutomation: true, + ResourceTypes: []string{"team"}, + }, + "dhq teams delete": { + Destructive: true, RequiresConfirmation: true, + Idempotent: false, SupportsJSON: true, SafeForAutomation: true, + ResourceTypes: []string{"team"}, + }, "dhq templates list": { Idempotent: true, SupportsJSON: true, SafeForAutomation: true, ResourceTypes: []string{"template"}, diff --git a/internal/commands/root.go b/internal/commands/root.go index 7f9b66b..f884b88 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -185,6 +185,7 @@ Support: support@deployhq.com`, newExcludedFilesCmd(), newIntegrationsCmd(), newAgentsCmd(), + newTeamsCmd(), newSSHKeysCmd(), newGlobalServersCmd(), newGlobalEnvVarsCmd(), diff --git a/internal/commands/teams.go b/internal/commands/teams.go new file mode 100644 index 0000000..c5a473a --- /dev/null +++ b/internal/commands/teams.go @@ -0,0 +1,332 @@ +package commands + +import ( + "fmt" + + "github.com/deployhq/deployhq-cli/internal/output" + "github.com/deployhq/deployhq-cli/pkg/sdk" + "github.com/spf13/cobra" +) + +func newTeamsCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "teams", + Short: "Manage teams (account-level permission groups)", + Long: `Account-level permission/role groups. A team bundles a set of permission flags (admin, manage users, manage billing, manage agents, create projects, access all projects) and a list of member users who inherit them. + +Teams are distinct from folders (which organise projects for display). Membership is synced with --user-ids on create/update; omit it to leave members untouched. + +Note: when --admin is set, the server force-enables every other permission flag and grants access to all projects.`, + } + + cmd.AddCommand( + newTeamsListCmd(), + newTeamsShowCmd(), + newTeamsCreateCmd(), + newTeamsUpdateCmd(), + newTeamsDeleteCmd(), + ) + + return cmd +} + +func newTeamsListCmd() *cobra.Command { + var page, perPage int + + cmd := &cobra.Command{ + Use: "list", + Short: "List teams", + RunE: func(cmd *cobra.Command, args []string) error { + client, err := cliCtx.Client() + if err != nil { + return err + } + + teams, err := client.ListTeams(cliCtx.Background(), listOptsFromFlags(page, perPage)) + if err != nil { + return err + } + + env := cliCtx.Envelope + if env.WantsJSON() { + return env.WriteJSON(output.NewResponse(teams, fmt.Sprintf("%d teams", len(teams)), + output.Breadcrumb{Action: "show", Cmd: "dhq teams show ", Resource: "team"}, + output.Breadcrumb{Action: "create", Cmd: "dhq teams create ", Resource: "team"}, + )) + } + + if env.QuietMode { + identifiers := make([]string, len(teams)) + for i, tm := range teams { + identifiers[i] = tm.Identifier + } + env.WriteQuiet(identifiers) + return nil + } + + columns := []string{"Name", "Identifier", "Admin", "Members", "All-Projects"} + rows := make([][]string, len(teams)) + for i, tm := range teams { + rows[i] = []string{ + tm.Name, + tm.Identifier, + enabledLabel(tm.IsAdmin), + fmt.Sprintf("%d", len(tm.Members)), + enabledLabel(tm.AllProjectsAllowed), + } + } + env.WriteTable(columns, rows) + + if len(teams) > 0 { + env.Status("\nTip: dhq teams show %s", teams[0].Identifier) + } + return nil + }, + } + + addPaginationFlags(cmd, &page, &perPage) + return cmd +} + +func newTeamsShowCmd() *cobra.Command { + return &cobra.Command{ + Use: "show ", + Short: "Show team details", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + client, err := cliCtx.Client() + if err != nil { + return err + } + + team, err := client.GetTeam(cliCtx.Background(), args[0]) + if err != nil { + return err + } + + env := cliCtx.Envelope + if env.WantsJSON() { + return env.WriteJSON(output.NewResponse(team, fmt.Sprintf("Team: %s", team.Name), + output.Breadcrumb{Action: "update", Cmd: fmt.Sprintf("dhq teams update %s", team.Identifier), Resource: "team", ID: team.Identifier}, + output.Breadcrumb{Action: "delete", Cmd: fmt.Sprintf("dhq teams delete %s", team.Identifier), Resource: "team", ID: team.Identifier}, + )) + } + + env.WriteTable([]string{"Field", "Value"}, [][]string{ + {"Name", team.Name}, + {"Identifier", team.Identifier}, + {"Admin", enabledLabel(team.IsAdmin)}, + {"Can manage users", enabledLabel(team.CanManageUsers)}, + {"Can manage billing", enabledLabel(team.CanManageBilling)}, + {"Can manage agents", enabledLabel(team.CanManageAgents)}, + {"Can create projects", enabledLabel(team.CanCreateProjects)}, + {"All projects allowed", enabledLabel(team.AllProjectsAllowed)}, + {"Members", fmt.Sprintf("%d", len(team.Members))}, + }) + + if len(team.Members) > 0 { + env.Status("\nMembers:") + memberCols := []string{"Name", "Email", "Identifier"} + memberRows := make([][]string, len(team.Members)) + for i, m := range team.Members { + memberRows[i] = []string{ + fmt.Sprintf("%s %s", m.FirstName, m.LastName), + m.EmailAddress, + m.Identifier, + } + } + env.WriteTable(memberCols, memberRows) + } + + if len(team.ProjectAssignments) > 0 { + env.Status("\nProject assignments:") + paCols := []string{"Name", "Identifier", "Deploy-All", "Update-Config", "Manage-Config-Files"} + paRows := make([][]string, len(team.ProjectAssignments)) + for i, pa := range team.ProjectAssignments { + paRows[i] = []string{ + pa.Name, pa.Identifier, + enabledLabel(pa.CanDeployAll), enabledLabel(pa.CanUpdateConfig), enabledLabel(pa.CanManageConfigFiles), + } + } + env.WriteTable(paCols, paRows) + } + + if len(team.ProjectExclusions) > 0 { + env.Status("\nProject exclusions:") + peCols := []string{"Name", "Identifier"} + peRows := make([][]string, len(team.ProjectExclusions)) + for i, pe := range team.ProjectExclusions { + peRows[i] = []string{pe.Name, pe.Identifier} + } + env.WriteTable(peCols, peRows) + } + + env.Status("\nNext commands:") + env.Status(" dhq teams update %s", team.Identifier) + env.Status(" dhq teams delete %s", team.Identifier) + return nil + }, + } +} + +// teamPermissionFlags holds the shared permission flags for create/update. +type teamPermissionFlags struct { + admin bool + canManageUsers bool + canManageBilling bool + canManageAgents bool + canCreateProjects bool + allProjects bool + userIDs []int +} + +func addTeamPermissionFlags(cmd *cobra.Command, f *teamPermissionFlags) { + cmd.Flags().BoolVar(&f.admin, "admin", false, "Grant admin (server force-enables all other permissions)") + cmd.Flags().BoolVar(&f.canManageUsers, "can-manage-users", false, "Allow managing users") + cmd.Flags().BoolVar(&f.canManageBilling, "can-manage-billing", false, "Allow managing billing") + cmd.Flags().BoolVar(&f.canManageAgents, "can-manage-agents", false, "Allow managing build agents") + cmd.Flags().BoolVar(&f.canCreateProjects, "can-create-projects", false, "Allow creating projects") + cmd.Flags().BoolVar(&f.allProjects, "all-projects", false, "Grant access to all projects") + cmd.Flags().IntSliceVar(&f.userIDs, "user-ids", nil, "Sync team members to these user IDs (comma-separated); omit to leave membership untouched") +} + +func newTeamsCreateCmd() *cobra.Command { + var f teamPermissionFlags + + cmd := &cobra.Command{ + Use: "create ", + Short: "Create a team", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + name := args[0] + if name == "" { + return &output.UserError{Message: "Name is required", Hint: "Usage: dhq teams create "} + } + + client, err := cliCtx.Client() + if err != nil { + return err + } + + req := sdk.TeamCreateRequest{ + Name: name, + IsAdmin: f.admin, + CanManageUsers: f.canManageUsers, + CanManageBilling: f.canManageBilling, + CanManageAgents: f.canManageAgents, + CanCreateProjects: f.canCreateProjects, + AllProjectsAllowed: f.allProjects, + } + if cmd.Flags().Changed("user-ids") { + req.UserIDs = f.userIDs + } + + team, err := client.CreateTeam(cliCtx.Background(), req) + if err != nil { + return err + } + + env := cliCtx.Envelope + if env.WantsJSON() { + return env.WriteJSON(output.NewResponse(team, fmt.Sprintf("Created team: %s", team.Name), + output.Breadcrumb{Action: "show", Cmd: fmt.Sprintf("dhq teams show %s", team.Identifier), Resource: "team", ID: team.Identifier}, + )) + } + env.Status("Created team: %s (%s)", team.Name, team.Identifier) + return nil + }, + } + + addTeamPermissionFlags(cmd, &f) + return cmd +} + +func newTeamsUpdateCmd() *cobra.Command { + var f teamPermissionFlags + var name string + + cmd := &cobra.Command{ + Use: "update ", + Short: "Update a team", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + client, err := cliCtx.Client() + if err != nil { + return err + } + + // Only send fields the user explicitly set, so unset flags don't + // clobber existing values. + var req sdk.TeamUpdateRequest + if cmd.Flags().Changed("name") { + req.Name = &name + } + if cmd.Flags().Changed("admin") { + req.IsAdmin = &f.admin + } + if cmd.Flags().Changed("can-manage-users") { + req.CanManageUsers = &f.canManageUsers + } + if cmd.Flags().Changed("can-manage-billing") { + req.CanManageBilling = &f.canManageBilling + } + if cmd.Flags().Changed("can-manage-agents") { + req.CanManageAgents = &f.canManageAgents + } + if cmd.Flags().Changed("can-create-projects") { + req.CanCreateProjects = &f.canCreateProjects + } + if cmd.Flags().Changed("all-projects") { + req.AllProjectsAllowed = &f.allProjects + } + if cmd.Flags().Changed("user-ids") { + // Pointer so an explicit empty list (--user-ids "") clears all + // members instead of being dropped by omitempty. IntSliceVar + // yields a non-nil empty slice for "", not nil. + ids := f.userIDs + if ids == nil { + ids = []int{} + } + req.UserIDs = &ids + } + + team, err := client.UpdateTeam(cliCtx.Background(), args[0], req) + if err != nil { + return err + } + + env := cliCtx.Envelope + if env.WantsJSON() { + return env.WriteJSON(output.NewResponse(team, fmt.Sprintf("Updated team: %s", team.Name), + output.Breadcrumb{Action: "show", Cmd: fmt.Sprintf("dhq teams show %s", team.Identifier), Resource: "team", ID: team.Identifier}, + )) + } + env.Status("Updated team: %s", team.Name) + return nil + }, + } + + cmd.Flags().StringVar(&name, "name", "", "New team name") + addTeamPermissionFlags(cmd, &f) + return cmd +} + +func newTeamsDeleteCmd() *cobra.Command { + return &cobra.Command{ + Use: "delete ", + Short: "Delete a team", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + client, err := cliCtx.Client() + if err != nil { + return err + } + + if err := client.DeleteTeam(cliCtx.Background(), args[0]); err != nil { + return err + } + cliCtx.Envelope.Status("Deleted team: %s", args[0]) + return nil + }, + } +} diff --git a/internal/commands/teams_test.go b/internal/commands/teams_test.go new file mode 100644 index 0000000..e31c94f --- /dev/null +++ b/internal/commands/teams_test.go @@ -0,0 +1,101 @@ +package commands + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTeamsCommand_Registered(t *testing.T) { + cmd := NewRootCmd("test") + + var found bool + for _, child := range cmd.Commands() { + if child.Name() == "teams" { + found = true + break + } + } + assert.True(t, found, "teams command must be registered on root") +} + +func TestTeamsCommand_Help(t *testing.T) { + cmd := NewRootCmd("test") + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"teams", "--help"}) + + err := cmd.Execute() + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "list") + assert.Contains(t, out, "show") + assert.Contains(t, out, "create") + assert.Contains(t, out, "update") + assert.Contains(t, out, "delete") +} + +func TestTeamsCreate_Help(t *testing.T) { + cmd := NewRootCmd("test") + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"teams", "create", "--help"}) + + err := cmd.Execute() + require.NoError(t, err) + + out := stdout.String() + assert.Contains(t, out, "--admin") + assert.Contains(t, out, "--can-manage-users") + assert.Contains(t, out, "--can-manage-billing") + assert.Contains(t, out, "--can-manage-agents") + assert.Contains(t, out, "--can-create-projects") + assert.Contains(t, out, "--all-projects") + assert.Contains(t, out, "--user-ids") +} + +func TestAgentMetadata_TeamsList(t *testing.T) { + m := lookupAgentMetadata("dhq teams list") + assert.True(t, m.Idempotent) + assert.True(t, m.SupportsJSON) + assert.True(t, m.SafeForAutomation) + assert.False(t, m.Destructive) + assert.Contains(t, m.ResourceTypes, "team") +} + +func TestAgentMetadata_TeamsShow(t *testing.T) { + m := lookupAgentMetadata("dhq teams show") + assert.True(t, m.Idempotent) + assert.True(t, m.SupportsJSON) + assert.True(t, m.SafeForAutomation) + assert.Contains(t, m.ResourceTypes, "team") +} + +func TestAgentMetadata_TeamsCreate(t *testing.T) { + m := lookupAgentMetadata("dhq teams create") + assert.False(t, m.Idempotent, "create is not idempotent") + assert.True(t, m.SupportsJSON) + assert.True(t, m.SafeForAutomation) + assert.False(t, m.Destructive) + assert.Contains(t, m.ResourceTypes, "team") +} + +func TestAgentMetadata_TeamsUpdate(t *testing.T) { + m := lookupAgentMetadata("dhq teams update") + assert.True(t, m.Idempotent, "update is idempotent") + assert.True(t, m.SupportsJSON) + assert.True(t, m.SafeForAutomation) + assert.Contains(t, m.ResourceTypes, "team") +} + +func TestAgentMetadata_TeamsDelete(t *testing.T) { + m := lookupAgentMetadata("dhq teams delete") + assert.True(t, m.Destructive) + assert.True(t, m.RequiresConfirmation) + assert.True(t, m.SupportsJSON) + assert.True(t, m.SafeForAutomation) + assert.Contains(t, m.ResourceTypes, "team") +} diff --git a/pkg/sdk/teams.go b/pkg/sdk/teams.go new file mode 100644 index 0000000..06deba6 --- /dev/null +++ b/pkg/sdk/teams.go @@ -0,0 +1,143 @@ +package sdk + +import ( + "context" + "fmt" +) + +// Team is an account-level permission/role group. Members are users who +// inherit the team's permission flags. Teams are distinct from folders +// (which organise projects for display). +type Team struct { + Identifier string `json:"identifier"` + Name string `json:"name"` + IsAdmin bool `json:"is_admin"` + CanManageUsers bool `json:"can_manage_users"` + CanManageBilling bool `json:"can_manage_billing"` + CanManageAgents bool `json:"can_manage_agents"` + CanCreateProjects bool `json:"can_create_projects"` + AllProjectsAllowed bool `json:"all_projects_allowed"` + + Members []TeamMember `json:"members,omitempty"` + ProjectAssignments []TeamProjectAssignment `json:"project_assignments,omitempty"` + // ProjectExclusions is only present when AllProjectsAllowed is true. + ProjectExclusions []TeamProjectExclusion `json:"project_exclusions,omitempty"` +} + +// TeamMember is a user belonging to a team. Unknown fields are ignored. +type TeamMember struct { + ID int `json:"id"` + Identifier string `json:"identifier"` + FirstName string `json:"first_name"` + LastName string `json:"last_name"` + EmailAddress string `json:"email_address"` + TimeZone string `json:"time_zone"` + AccountAdministrator bool `json:"account_administrator"` + Activated bool `json:"activated"` + IsAdmin bool `json:"is_admin"` + CanManageUsers bool `json:"can_manage_users"` + CanManageBilling bool `json:"can_manage_billing"` + CanManageAgents bool `json:"can_manage_agents"` + CanCreateProjects bool `json:"can_create_projects"` + AllProjectsAllowed bool `json:"all_projects_allowed"` +} + +// TeamProjectAssignment describes a project the team can access and the +// per-project capabilities granted. +type TeamProjectAssignment struct { + Name string `json:"name"` + Identifier string `json:"identifier"` + CanDeployAll bool `json:"can_deploy_all"` + CanUpdateConfig bool `json:"can_update_config"` + CanManageConfigFiles bool `json:"can_manage_config_files"` +} + +// TeamProjectExclusion describes a project explicitly excluded from an +// otherwise all-projects team. +type TeamProjectExclusion struct { + Name string `json:"name"` + Identifier string `json:"identifier"` +} + +// TeamCreateRequest is the payload for creating a team. Name is required. +// +// When IsAdmin is true the server force-sets the other permission flags and +// AllProjectsAllowed to true, regardless of what is sent. +// +// UserIDs syncs team membership. Omitting it leaves membership untouched. +type TeamCreateRequest struct { + Name string `json:"name"` + IsAdmin bool `json:"is_admin"` + CanManageUsers bool `json:"can_manage_users"` + CanManageBilling bool `json:"can_manage_billing"` + CanManageAgents bool `json:"can_manage_agents"` + CanCreateProjects bool `json:"can_create_projects"` + AllProjectsAllowed bool `json:"all_projects_allowed"` + UserIDs []int `json:"user_ids,omitempty"` +} + +// TeamUpdateRequest is the payload for updating a team. All fields are pointers +// with omitempty so partial updates don't clobber unset fields. +// +// UserIDs syncs team membership. It is a pointer so the two intents stay +// distinct: a nil pointer (flag omitted) leaves membership untouched, while a +// non-nil pointer to an empty slice (e.g. --user-ids "") clears all members — +// a plain []int with omitempty would drop the empty case and silently no-op. +type TeamUpdateRequest struct { + Name *string `json:"name,omitempty"` + IsAdmin *bool `json:"is_admin,omitempty"` + CanManageUsers *bool `json:"can_manage_users,omitempty"` + CanManageBilling *bool `json:"can_manage_billing,omitempty"` + CanManageAgents *bool `json:"can_manage_agents,omitempty"` + CanCreateProjects *bool `json:"can_create_projects,omitempty"` + AllProjectsAllowed *bool `json:"all_projects_allowed,omitempty"` + UserIDs *[]int `json:"user_ids,omitempty"` +} + +// ListTeams returns all teams on the account. +func (c *Client) ListTeams(ctx context.Context, opts *ListOptions) ([]Team, error) { + var teams []Team + path := appendListParams("/teams", opts) + if err := c.get(ctx, path, &teams); err != nil { + return nil, err + } + return teams, nil +} + +// GetTeam returns a single team by identifier. +func (c *Client) GetTeam(ctx context.Context, id string) (*Team, error) { + var team Team + if err := c.get(ctx, fmt.Sprintf("/teams/%s", id), &team); err != nil { + return nil, err + } + return &team, nil +} + +// CreateTeam creates a new team. +func (c *Client) CreateTeam(ctx context.Context, req TeamCreateRequest) (*Team, error) { + body := struct { + Team TeamCreateRequest `json:"team"` + }{Team: req} + var team Team + if err := c.post(ctx, "/teams", body, &team); err != nil { + return nil, err + } + return &team, nil +} + +// UpdateTeam updates an existing team. +func (c *Client) UpdateTeam(ctx context.Context, id string, req TeamUpdateRequest) (*Team, error) { + body := struct { + Team TeamUpdateRequest `json:"team"` + }{Team: req} + var team Team + if err := c.patch(ctx, fmt.Sprintf("/teams/%s", id), body, &team); err != nil { + return nil, err + } + return &team, nil +} + +// DeleteTeam deletes a team by identifier. +func (c *Client) DeleteTeam(ctx context.Context, id string) error { + return c.delete(ctx, fmt.Sprintf("/teams/%s", id)) +} diff --git a/pkg/sdk/teams_test.go b/pkg/sdk/teams_test.go new file mode 100644 index 0000000..444628e --- /dev/null +++ b/pkg/sdk/teams_test.go @@ -0,0 +1,245 @@ +package sdk + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestListTeams(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/teams", r.URL.Path) + _ = json.NewEncoder(w).Encode([]Team{ + {Identifier: "t1", Name: "Admins", IsAdmin: true, AllProjectsAllowed: true}, + {Identifier: "t2", Name: "Deployers"}, + }) + })) + defer server.Close() + + c := newTestClient(t, server) + teams, err := c.ListTeams(context.Background(), nil) + require.NoError(t, err) + assert.Len(t, teams, 2) + assert.Equal(t, "Admins", teams[0].Name) + assert.True(t, teams[0].IsAdmin) + assert.Equal(t, "Deployers", teams[1].Name) +} + +func TestGetTeam(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodGet, r.Method) + assert.Equal(t, "/teams/t1", r.URL.Path) + _ = json.NewEncoder(w).Encode(Team{ + Identifier: "t1", + Name: "Admins", + IsAdmin: true, + AllProjectsAllowed: true, + Members: []TeamMember{ + {ID: 5, Identifier: "u5", FirstName: "Ada", LastName: "Lovelace", EmailAddress: "ada@example.com"}, + {ID: 6, Identifier: "u6", FirstName: "Alan", LastName: "Turing", EmailAddress: "alan@example.com"}, + }, + ProjectAssignments: []TeamProjectAssignment{ + {Name: "Web", Identifier: "web", CanDeployAll: true, CanUpdateConfig: true}, + }, + ProjectExclusions: []TeamProjectExclusion{ + {Name: "Secret", Identifier: "secret"}, + }, + }) + })) + defer server.Close() + + c := newTestClient(t, server) + team, err := c.GetTeam(context.Background(), "t1") + require.NoError(t, err) + assert.Equal(t, "Admins", team.Name) + assert.Len(t, team.Members, 2) + assert.Equal(t, "ada@example.com", team.Members[0].EmailAddress) + assert.Equal(t, 5, team.Members[0].ID) + assert.Len(t, team.ProjectAssignments, 1) + assert.True(t, team.ProjectAssignments[0].CanDeployAll) + require.Len(t, team.ProjectExclusions, 1) + assert.Equal(t, "secret", team.ProjectExclusions[0].Identifier) +} + +func TestGetTeam_NoExclusionsWhenNotAllProjects(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // project_exclusions absent entirely when all_projects_allowed is false. + _ = json.NewEncoder(w).Encode(map[string]interface{}{ + "identifier": "t2", + "name": "Deployers", + "all_projects_allowed": false, + "project_assignments": []interface{}{}, + }) + })) + defer server.Close() + + c := newTestClient(t, server) + team, err := c.GetTeam(context.Background(), "t2") + require.NoError(t, err) + assert.False(t, team.AllProjectsAllowed) + assert.Empty(t, team.ProjectExclusions) +} + +func TestCreateTeam(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPost, r.Method) + assert.Equal(t, "/teams", r.URL.Path) + + var body struct { + Team TeamCreateRequest `json:"team"` + } + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, "Deployers", body.Team.Name) + assert.True(t, body.Team.CanCreateProjects) + assert.Equal(t, []int{5, 6}, body.Team.UserIDs) + + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(Team{Identifier: "t-new", Name: "Deployers", CanCreateProjects: true}) + })) + defer server.Close() + + c := newTestClient(t, server) + team, err := c.CreateTeam(context.Background(), TeamCreateRequest{ + Name: "Deployers", + CanCreateProjects: true, + UserIDs: []int{5, 6}, + }) + require.NoError(t, err) + assert.Equal(t, "Deployers", team.Name) + assert.Equal(t, "t-new", team.Identifier) +} + +func TestUpdateTeam(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Codebase convention: users/teams update via PATCH. + assert.Equal(t, http.MethodPatch, r.Method) + assert.Equal(t, "/teams/t1", r.URL.Path) + + var raw map[string]map[string]interface{} + require.NoError(t, json.NewDecoder(r.Body).Decode(&raw)) + teamBody, ok := raw["team"] + require.True(t, ok, "body must be wrapped under 'team'") + assert.Equal(t, "Renamed", teamBody["name"]) + // Only 'name' was set; unset pointer fields must be omitted so a partial + // update doesn't clobber the other flags. + _, hasAdmin := teamBody["is_admin"] + assert.False(t, hasAdmin, "unset is_admin must be omitted") + + _ = json.NewEncoder(w).Encode(Team{Identifier: "t1", Name: "Renamed"}) + })) + defer server.Close() + + c := newTestClient(t, server) + name := "Renamed" + team, err := c.UpdateTeam(context.Background(), "t1", TeamUpdateRequest{Name: &name}) + require.NoError(t, err) + assert.Equal(t, "Renamed", team.Name) +} + +func TestUpdateTeam_ClearMembers(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var raw map[string]map[string]interface{} + require.NoError(t, json.NewDecoder(r.Body).Decode(&raw)) + teamBody, ok := raw["team"] + require.True(t, ok, "body must be wrapped under 'team'") + // An explicit empty list must be sent as [] (clear all members), not + // dropped by omitempty — otherwise "remove everyone" silently no-ops. + ids, ok := teamBody["user_ids"] + require.True(t, ok, "explicit empty user_ids must be present") + assert.Equal(t, []interface{}{}, ids) + + _ = json.NewEncoder(w).Encode(Team{Identifier: "t1", Name: "Admins"}) + })) + defer server.Close() + + c := newTestClient(t, server) + empty := []int{} + _, err := c.UpdateTeam(context.Background(), "t1", TeamUpdateRequest{UserIDs: &empty}) + require.NoError(t, err) +} + +func TestUpdateTeam_OmittedMembersUntouched(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var raw map[string]map[string]interface{} + require.NoError(t, json.NewDecoder(r.Body).Decode(&raw)) + teamBody := raw["team"] + // A nil UserIDs pointer (flag omitted) must NOT send user_ids at all. + _, hasUserIDs := teamBody["user_ids"] + assert.False(t, hasUserIDs, "omitted user_ids must be absent so membership is untouched") + + _ = json.NewEncoder(w).Encode(Team{Identifier: "t1", Name: "Admins"}) + })) + defer server.Close() + + c := newTestClient(t, server) + admin := true + _, err := c.UpdateTeam(context.Background(), "t1", TeamUpdateRequest{IsAdmin: &admin}) + require.NoError(t, err) +} + +func TestDeleteTeam(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodDelete, r.Method) + assert.Equal(t, "/teams/t1", r.URL.Path) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(map[string]string{"status": "deleted"}) + })) + defer server.Close() + + c := newTestClient(t, server) + err := c.DeleteTeam(context.Background(), "t1") + require.NoError(t, err) +} + +func TestCreateTeam_DuplicateName(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _ = json.NewEncoder(w).Encode(map[string][]string{"name": {"can't be blank"}}) + })) + defer server.Close() + + c := newTestClient(t, server) + _, err := c.CreateTeam(context.Background(), TeamCreateRequest{Name: ""}) + require.Error(t, err) + var apiErr *APIError + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, http.StatusUnprocessableEntity, apiErr.StatusCode) +} + +func TestGetTeam_Forbidden(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + _ = json.NewEncoder(w).Encode(map[string]string{ + "error": "You do not have access...", + "error_code": "access_denied", + }) + })) + defer server.Close() + + c := newTestClient(t, server) + _, err := c.GetTeam(context.Background(), "t1") + require.Error(t, err) + var apiErr *APIError + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, http.StatusForbidden, apiErr.StatusCode) +} + +func TestGetTeam_NotFound(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer server.Close() + + c := newTestClient(t, server) + _, err := c.GetTeam(context.Background(), "nope") + require.Error(t, err) + var apiErr *APIError + require.ErrorAs(t, err, &apiErr) + assert.Equal(t, http.StatusNotFound, apiErr.StatusCode) +} From 967ab5b14426a96d761b346b489a7032d69f37e3 Mon Sep 17 00:00:00 2001 From: Marta Date: Mon, 13 Jul 2026 11:19:32 +0200 Subject: [PATCH 2/3] feat: add --clear-members to dhq teams update The SDK models an empty membership list as a non-nil *[]int so it reaches the server as "user_ids":[], but that empty case was unreachable from the CLI: cobra's IntSliceVar rejects --user-ids "" at parse time (strconv.Atoi on the empty string fails). Add a dedicated --clear-members flag (update only) to express "remove all members", mutually exclusive with --user-ids. Found by live smoke-testing the command against a real backend; the SDK unit test passed because it builds the request struct directly, bypassing flag parsing. --- internal/commands/teams.go | 30 +++++++++++++++++++++---- internal/commands/teams_test.go | 40 +++++++++++++++++++++++++++++++++ pkg/sdk/teams.go | 7 +++--- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/internal/commands/teams.go b/internal/commands/teams.go index c5a473a..726509d 100644 --- a/internal/commands/teams.go +++ b/internal/commands/teams.go @@ -190,6 +190,13 @@ func addTeamPermissionFlags(cmd *cobra.Command, f *teamPermissionFlags) { cmd.Flags().IntSliceVar(&f.userIDs, "user-ids", nil, "Sync team members to these user IDs (comma-separated); omit to leave membership untouched") } +// addClearMembersFlag adds the update-only --clear-members flag. It exists +// because IntSliceVar cannot express an empty list from the command line +// (--user-ids "" fails to parse), so "remove all members" needs its own flag. +func addClearMembersFlag(cmd *cobra.Command, clear *bool) { + cmd.Flags().BoolVar(clear, "clear-members", false, "Remove all members from the team (mutually exclusive with --user-ids)") +} + func newTeamsCreateCmd() *cobra.Command { var f teamPermissionFlags @@ -244,12 +251,20 @@ func newTeamsCreateCmd() *cobra.Command { func newTeamsUpdateCmd() *cobra.Command { var f teamPermissionFlags var name string + var clearMembers bool cmd := &cobra.Command{ Use: "update ", Short: "Update a team", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + if cmd.Flags().Changed("user-ids") && clearMembers { + return &output.UserError{ + Message: "--user-ids and --clear-members cannot be used together", + Hint: "Use --user-ids to set members, or --clear-members to remove all members", + } + } + client, err := cliCtx.Client() if err != nil { return err @@ -279,10 +294,16 @@ func newTeamsUpdateCmd() *cobra.Command { if cmd.Flags().Changed("all-projects") { req.AllProjectsAllowed = &f.allProjects } - if cmd.Flags().Changed("user-ids") { - // Pointer so an explicit empty list (--user-ids "") clears all - // members instead of being dropped by omitempty. IntSliceVar - // yields a non-nil empty slice for "", not nil. + // Membership: --user-ids syncs to the given users; --clear-members + // removes all members. Both are sent as a non-nil *[]int so the + // empty case reaches the server as "user_ids":[] rather than being + // dropped by omitempty (which IntSliceVar can't express anyway, + // since --user-ids "" fails to parse). + switch { + case clearMembers: + empty := []int{} + req.UserIDs = &empty + case cmd.Flags().Changed("user-ids"): ids := f.userIDs if ids == nil { ids = []int{} @@ -308,6 +329,7 @@ func newTeamsUpdateCmd() *cobra.Command { cmd.Flags().StringVar(&name, "name", "", "New team name") addTeamPermissionFlags(cmd, &f) + addClearMembersFlag(cmd, &clearMembers) return cmd } diff --git a/internal/commands/teams_test.go b/internal/commands/teams_test.go index e31c94f..a472de0 100644 --- a/internal/commands/teams_test.go +++ b/internal/commands/teams_test.go @@ -2,6 +2,7 @@ package commands import ( "bytes" + "io" "testing" "github.com/stretchr/testify/assert" @@ -57,6 +58,45 @@ func TestTeamsCreate_Help(t *testing.T) { assert.Contains(t, out, "--user-ids") } +func TestTeamsUpdate_Help(t *testing.T) { + cmd := NewRootCmd("test") + var stdout bytes.Buffer + cmd.SetOut(&stdout) + cmd.SetArgs([]string{"teams", "update", "--help"}) + + err := cmd.Execute() + require.NoError(t, err) + + out := stdout.String() + // --clear-members is update-only (create has no membership to clear); it + // exists because --user-ids "" cannot be parsed as an int slice. + assert.Contains(t, out, "--clear-members") + assert.Contains(t, out, "--user-ids") +} + +// --clear-members is not offered on create — "no members" there is just the +// default of omitting --user-ids. +func TestTeamsCreate_NoClearMembersFlag(t *testing.T) { + cmd := NewRootCmd("test") + createCmd, _, err := cmd.Find([]string{"teams", "create"}) + require.NoError(t, err) + assert.Nil(t, createCmd.Flags().Lookup("clear-members"), + "--clear-members must not be registered on create") +} + +func TestTeamsUpdate_UserIDsAndClearMembersConflict(t *testing.T) { + cmd := NewRootCmd("test") + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + // The mutual-exclusion guard runs before any API client is built, so this + // fails fast without network access or credentials. + cmd.SetArgs([]string{"teams", "update", "some-id", "--user-ids", "5", "--clear-members"}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot be used together") +} + func TestAgentMetadata_TeamsList(t *testing.T) { m := lookupAgentMetadata("dhq teams list") assert.True(t, m.Idempotent) diff --git a/pkg/sdk/teams.go b/pkg/sdk/teams.go index 06deba6..4beeac0 100644 --- a/pkg/sdk/teams.go +++ b/pkg/sdk/teams.go @@ -80,9 +80,10 @@ type TeamCreateRequest struct { // with omitempty so partial updates don't clobber unset fields. // // UserIDs syncs team membership. It is a pointer so the two intents stay -// distinct: a nil pointer (flag omitted) leaves membership untouched, while a -// non-nil pointer to an empty slice (e.g. --user-ids "") clears all members — -// a plain []int with omitempty would drop the empty case and silently no-op. +// distinct: a nil pointer leaves membership untouched, while a non-nil pointer +// to an empty slice clears all members — a plain []int with omitempty would +// drop the empty case and silently no-op. (The CLI reaches the empty case via +// --clear-members, since --user-ids "" cannot be parsed as an int slice.) type TeamUpdateRequest struct { Name *string `json:"name,omitempty"` IsAdmin *bool `json:"is_admin,omitempty"` From 4b4783b1b1648cfa0a24e82843d3fa9f4abb041a Mon Sep 17 00:00:00 2001 From: Marta Date: Mon, 13 Jul 2026 11:43:57 +0200 Subject: [PATCH 3/3] fix: reject empty --name on dhq teams update Match `teams create`, which rejects a blank name before the API call. Previously `teams update --name ""` passed the Changed() check and sent "name":"" to the server, relying on a round-trip 422 rather than failing fast. Addresses CodeRabbit feedback on PR #33. --- internal/commands/teams.go | 8 ++++++++ internal/commands/teams_test.go | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/internal/commands/teams.go b/internal/commands/teams.go index 726509d..daefb95 100644 --- a/internal/commands/teams.go +++ b/internal/commands/teams.go @@ -258,6 +258,14 @@ func newTeamsUpdateCmd() *cobra.Command { Short: "Update a team", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + // Fail fast on an explicit empty name, matching `teams create`, + // rather than sending "name":"" and relying on a server 422. + if cmd.Flags().Changed("name") && name == "" { + return &output.UserError{ + Message: "Name cannot be empty", + Hint: "Omit --name to leave the team name unchanged", + } + } if cmd.Flags().Changed("user-ids") && clearMembers { return &output.UserError{ Message: "--user-ids and --clear-members cannot be used together", diff --git a/internal/commands/teams_test.go b/internal/commands/teams_test.go index a472de0..66a3a8e 100644 --- a/internal/commands/teams_test.go +++ b/internal/commands/teams_test.go @@ -84,6 +84,19 @@ func TestTeamsCreate_NoClearMembersFlag(t *testing.T) { "--clear-members must not be registered on create") } +func TestTeamsUpdate_RejectsEmptyName(t *testing.T) { + cmd := NewRootCmd("test") + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + // An explicit empty --name must fail fast (like create), before any API + // client is built — so this needs no network or credentials. + cmd.SetArgs([]string{"teams", "update", "some-id", "--name", ""}) + + err := cmd.Execute() + require.Error(t, err) + assert.Contains(t, err.Error(), "Name cannot be empty") +} + func TestTeamsUpdate_UserIDsAndClearMembersConflict(t *testing.T) { cmd := NewRootCmd("test") cmd.SetOut(io.Discard)