From e48679f4392f3f6f6e53ae60d7de3980b08e167c Mon Sep 17 00:00:00 2001 From: Ben McNicholl Date: Fri, 27 Mar 2026 15:57:09 +1100 Subject: [PATCH 1/4] chore: support pagination with teams list --- cmd/team/create.go | 97 ++++++++ cmd/team/delete.go | 74 ++++++ cmd/team/list.go | 145 +++++++++++ cmd/team/team_test.go | 486 +++++++++++++++++++++++++++++++++++++ cmd/team/update.go | 142 +++++++++++ cmd/team/view.go | 82 +++++++ internal/team/view.go | 77 ++++++ internal/team/view_test.go | 133 ++++++++++ main.go | 9 + 9 files changed, 1245 insertions(+) create mode 100644 cmd/team/create.go create mode 100644 cmd/team/delete.go create mode 100644 cmd/team/list.go create mode 100644 cmd/team/team_test.go create mode 100644 cmd/team/update.go create mode 100644 cmd/team/view.go create mode 100644 internal/team/view.go create mode 100644 internal/team/view_test.go diff --git a/cmd/team/create.go b/cmd/team/create.go new file mode 100644 index 00000000..39a5a32c --- /dev/null +++ b/cmd/team/create.go @@ -0,0 +1,97 @@ +package team + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + bkIO "github.com/buildkite/cli/v3/internal/io" + "github.com/buildkite/cli/v3/internal/team" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" + "github.com/buildkite/cli/v3/pkg/output" + buildkite "github.com/buildkite/go-buildkite/v4" +) + +type CreateCmd struct { + Name string `arg:"" help:"Name of the team" name:"name"` + Description string `help:"Description of the team" optional:""` + Privacy string `help:"Privacy setting for the team: visible or secret" optional:"" default:"visible" enum:"visible,secret"` + Default bool `help:"Whether this is the default team for new members" optional:"" name:"default"` + DefaultMemberRole string `help:"Default role for new members: member or maintainer" optional:"" name:"default-member-role" default:"member" enum:"member,maintainer"` + MembersCanCreatePipelines bool `help:"Whether members can create pipelines" optional:"" name:"members-can-create-pipelines"` + output.OutputFlags +} + +func (c *CreateCmd) Help() string { + return ` +Create a new team in the organization. + +Examples: + # Create a team with default settings + $ bk team create my-team + + # Create a private team with a description + $ bk team create my-team --description "My team" --privacy secret + + # Create a default team where members can create pipelines + $ bk team create my-team --default --members-can-create-pipelines +` +} + +func (c *CreateCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New(factory.WithDebug(globals.EnableDebug())) + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + f.NoPager = f.NoPager || globals.DisablePager() + + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err + } + + format := output.ResolveFormat(c.Output, f.Config.OutputFormat()) + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + input := buildkite.CreateTeam{ + Name: c.Name, + Description: c.Description, + Privacy: c.Privacy, + IsDefaultTeam: c.Default, + DefaultMemberRole: c.DefaultMemberRole, + MembersCanCreatePipelines: c.MembersCanCreatePipelines, + } + + var t buildkite.Team + spinErr := bkIO.SpinWhile(f, "Creating team", func() { + t, _, err = f.RestAPIClient.Teams.CreateTeam(ctx, f.Config.OrganizationSlug(), input) + }) + if spinErr != nil { + return spinErr + } + if err != nil { + return fmt.Errorf("error creating team: %v", err) + } + + teamView := output.Viewable[buildkite.Team]{ + Data: t, + Render: team.RenderTeamText, + } + + if format != output.FormatText { + return output.Write(os.Stdout, teamView, format) + } + + fmt.Fprintf(os.Stdout, "Team %s created successfully\n\n", t.Name) + return output.Write(os.Stdout, teamView, format) +} diff --git a/cmd/team/delete.go b/cmd/team/delete.go new file mode 100644 index 00000000..075ad593 --- /dev/null +++ b/cmd/team/delete.go @@ -0,0 +1,74 @@ +package team + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + bkIO "github.com/buildkite/cli/v3/internal/io" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" +) + +type DeleteCmd struct { + TeamUUID string `arg:"" help:"UUID of the team to delete" name:"team-uuid"` +} + +func (c *DeleteCmd) Help() string { + return ` +Delete a team from the organization. + +You will be prompted to confirm deletion unless --yes is set. + +Examples: + # Delete a team (with confirmation prompt) + $ bk team delete my-team-uuid + + # Delete a team without confirmation + $ bk team delete my-team-uuid --yes +` +} + +func (c *DeleteCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New(factory.WithDebug(globals.EnableDebug())) + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err + } + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + confirmed, err := bkIO.Confirm(f, fmt.Sprintf("Are you sure you want to delete team %s?", c.TeamUUID)) + if err != nil { + return err + } + if !confirmed { + fmt.Fprintln(os.Stderr, "Deletion cancelled.") + return nil + } + + spinErr := bkIO.SpinWhile(f, "Deleting team", func() { + _, err = f.RestAPIClient.Teams.DeleteTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) + }) + if spinErr != nil { + return spinErr + } + if err != nil { + return fmt.Errorf("error deleting team: %v", err) + } + + fmt.Fprintln(os.Stderr, "Team deleted successfully.") + return nil +} diff --git a/cmd/team/list.go b/cmd/team/list.go new file mode 100644 index 00000000..3add1221 --- /dev/null +++ b/cmd/team/list.go @@ -0,0 +1,145 @@ +package team + +import ( + "context" + "errors" + "fmt" + "os" + "os/signal" + "syscall" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + bkIO "github.com/buildkite/cli/v3/internal/io" + "github.com/buildkite/cli/v3/internal/team" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" + "github.com/buildkite/cli/v3/pkg/output" + buildkite "github.com/buildkite/go-buildkite/v4" +) + +type ListCmd struct { + PerPage int `help:"Number of teams per page" default:"30"` + Limit int `help:"Maximum number of teams to return" default:"100"` + output.OutputFlags +} + +func (c *ListCmd) Help() string { + return ` +List the teams for an organization. By default, shows up to 100 teams. + +Examples: + # List all teams + $ bk team list + + # List teams in JSON format + $ bk team list -o json + + # List up to 200 teams + $ bk team list --limit 200 +` +} + +func (c *ListCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New(factory.WithDebug(globals.EnableDebug())) + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + f.NoPager = f.NoPager || globals.DisablePager() + + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err + } + + format := output.ResolveFormat(c.Output, f.Config.OutputFormat()) + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + teams, hasMore, err := listTeams(ctx, f, c.PerPage, c.Limit) + if err != nil { + return err + } + + if format != output.FormatText { + return output.Write(os.Stdout, teams, format) + } + + summary := team.TeamViewTable(teams...) + + writer, cleanup := bkIO.Pager(f.NoPager, f.Config.Pager()) + defer func() { _ = cleanup() }() + + totalDisplay := fmt.Sprintf("%d", len(teams)) + if hasMore { + totalDisplay = fmt.Sprintf("%d+", len(teams)) + } + fmt.Fprintf(writer, "Showing %s teams in %s\n\n", totalDisplay, f.Config.OrganizationSlug()) + fmt.Fprintf(writer, "%v\n", summary) + + return nil +} + +func listTeams(ctx context.Context, f *factory.Factory, perPage, limit int) ([]buildkite.Team, bool, error) { + var all []buildkite.Team + var err error + page := 1 + hasMore := false + var previousFirstTeamID string + + for len(all) < limit { + opts := &buildkite.TeamsListOptions{ + ListOptions: buildkite.ListOptions{ + Page: page, + PerPage: perPage, + }, + } + + var pageTeams []buildkite.Team + spinErr := bkIO.SpinWhile(f, "Loading teams", func() { + pageTeams, _, err = f.RestAPIClient.Teams.List(ctx, f.Config.OrganizationSlug(), opts) + }) + if spinErr != nil { + return nil, false, spinErr + } + if err != nil { + return nil, false, fmt.Errorf("error fetching team list: %v", err) + } + + if len(pageTeams) == 0 { + break + } + + if page > 1 && pageTeams[0].ID == previousFirstTeamID { + return nil, false, fmt.Errorf("API returned duplicate page content at page %d, stopping pagination to prevent infinite loop", page) + } + previousFirstTeamID = pageTeams[0].ID + + all = append(all, pageTeams...) + + if len(pageTeams) < perPage { + break + } + + if len(all) >= limit { + hasMore = true + break + } + + page++ + } + + if len(all) > limit { + all = all[:limit] + } + + if len(all) == 0 { + return nil, false, errors.New("no teams found in organization") + } + + return all, hasMore, nil +} diff --git a/cmd/team/team_test.go b/cmd/team/team_test.go new file mode 100644 index 00000000..d5e876c2 --- /dev/null +++ b/cmd/team/team_test.go @@ -0,0 +1,486 @@ +package team + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + buildkite "github.com/buildkite/go-buildkite/v4" +) + +func makeTeams(n, offset int) []buildkite.Team { + teams := make([]buildkite.Team, n) + for i := range teams { + teams[i] = buildkite.Team{ + ID: fmt.Sprintf("team-%d", offset+i), + Name: fmt.Sprintf("Team %d", offset+i), + Slug: fmt.Sprintf("team-%d", offset+i), + } + } + return teams +} + +func TestListTeams(t *testing.T) { + t.Parallel() + + t.Run("fetches teams through API", func(t *testing.T) { + t.Parallel() + + teams := []buildkite.Team{ + {ID: "team-1", Name: "Frontend", Slug: "frontend", Privacy: "visible"}, + {ID: "team-2", Name: "Backend", Slug: "backend", Privacy: "secret", Default: true}, + } + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + if !strings.Contains(r.URL.Path, "/teams") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(teams) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + result, _, err := client.Teams.List(context.Background(), "test-org", nil) + if err != nil { + t.Fatal(err) + } + + if len(result) != 2 { + t.Fatalf("expected 2 teams, got %d", len(result)) + } + if result[0].Name != "Frontend" { + t.Errorf("expected name 'Frontend', got %q", result[0].Name) + } + if result[1].Slug != "backend" { + t.Errorf("expected slug 'backend', got %q", result[1].Slug) + } + }) + + t.Run("empty result returns empty slice", func(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode([]buildkite.Team{}) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + result, _, err := client.Teams.List(context.Background(), "test-org", nil) + if err != nil { + t.Fatal(err) + } + + if len(result) != 0 { + t.Errorf("expected 0 teams, got %d", len(result)) + } + }) + + t.Run("paginates across multiple pages", func(t *testing.T) { + t.Parallel() + + // page 1: 30 teams (full page), page 2: 15 teams (partial) → 45 total + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page := r.URL.Query().Get("page") + w.Header().Set("Content-Type", "application/json") + switch page { + case "", "1": + json.NewEncoder(w).Encode(makeTeams(30, 0)) + case "2": + json.NewEncoder(w).Encode(makeTeams(15, 30)) + default: + json.NewEncoder(w).Encode([]buildkite.Team{}) + } + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + page1, _, err := client.Teams.List(context.Background(), "test-org", &buildkite.TeamsListOptions{ + ListOptions: buildkite.ListOptions{Page: 1, PerPage: 30}, + }) + if err != nil { + t.Fatal(err) + } + page2, _, err := client.Teams.List(context.Background(), "test-org", &buildkite.TeamsListOptions{ + ListOptions: buildkite.ListOptions{Page: 2, PerPage: 30}, + }) + if err != nil { + t.Fatal(err) + } + + total := append(page1, page2...) + if len(total) != 45 { + t.Errorf("expected 45 teams across 2 pages, got %d", len(total)) + } + // Partial second page signals no further pages + if len(page2) >= 30 { + t.Error("expected page 2 to be a partial page indicating end of results") + } + }) + + t.Run("stops at limit when pages are full", func(t *testing.T) { + t.Parallel() + + // Server always returns full pages of 30; limit is 30 so only one page needed + callCount := 0 + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + callCount++ + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(makeTeams(30, (callCount-1)*30)) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + result, _, err := client.Teams.List(context.Background(), "test-org", &buildkite.TeamsListOptions{ + ListOptions: buildkite.ListOptions{Page: 1, PerPage: 30}, + }) + if err != nil { + t.Fatal(err) + } + + if len(result) != 30 { + t.Errorf("expected 30 teams, got %d", len(result)) + } + // A full page means there are potentially more results + if len(result) < 30 { + t.Error("expected a full page indicating more results may exist") + } + if callCount != 1 { + t.Errorf("expected 1 API call, got %d", callCount) + } + }) + + t.Run("duplicate page detection", func(t *testing.T) { + t.Parallel() + + // Server always returns the same page content regardless of page param + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(makeTeams(30, 0)) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + page1, _, err := client.Teams.List(context.Background(), "test-org", &buildkite.TeamsListOptions{ + ListOptions: buildkite.ListOptions{Page: 1, PerPage: 30}, + }) + if err != nil { + t.Fatal(err) + } + page2, _, err := client.Teams.List(context.Background(), "test-org", &buildkite.TeamsListOptions{ + ListOptions: buildkite.ListOptions{Page: 2, PerPage: 30}, + }) + if err != nil { + t.Fatal(err) + } + + // Both pages have the same first ID — the listTeams loop would catch this + if page1[0].ID != page2[0].ID { + t.Error("expected duplicate page content to have matching first IDs") + } + }) +} + +func TestGetTeam(t *testing.T) { + t.Parallel() + + team := buildkite.Team{ + ID: "team-uuid-123", + Name: "Fearless Frontenders", + Slug: "fearless-frontenders", + Description: "The frontend team", + Privacy: "secret", + Default: true, + } + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + if !strings.Contains(r.URL.Path, "/teams/team-uuid-123") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(team) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + result, err := client.Teams.GetTeam(context.Background(), "test-org", "team-uuid-123") + if err != nil { + t.Fatal(err) + } + + if result.Name != "Fearless Frontenders" { + t.Errorf("expected name 'Fearless Frontenders', got %q", result.Name) + } + if result.Description != "The frontend team" { + t.Errorf("expected description 'The frontend team', got %q", result.Description) + } + if !result.Default { + t.Error("expected Default to be true") + } +} + +func TestCreateTeam(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("expected POST, got %s", r.Method) + } + if !strings.Contains(r.URL.Path, "/teams") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + + var input buildkite.CreateTeam + if err := json.NewDecoder(r.Body).Decode(&input); err != nil { + t.Fatal(err) + } + + if input.Name != "New Team" { + t.Errorf("expected name 'New Team', got %q", input.Name) + } + if input.Privacy != "secret" { + t.Errorf("expected privacy 'secret', got %q", input.Privacy) + } + if !input.IsDefaultTeam { + t.Error("expected IsDefaultTeam to be true") + } + if input.DefaultMemberRole != "maintainer" { + t.Errorf("expected default member role 'maintainer', got %q", input.DefaultMemberRole) + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(buildkite.Team{ + ID: "new-team-uuid", + Name: input.Name, + Privacy: input.Privacy, + Default: input.IsDefaultTeam, + }) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + result, _, err := client.Teams.CreateTeam(context.Background(), "test-org", buildkite.CreateTeam{ + Name: "New Team", + Privacy: "secret", + IsDefaultTeam: true, + DefaultMemberRole: "maintainer", + }) + if err != nil { + t.Fatal(err) + } + + if result.ID != "new-team-uuid" { + t.Errorf("expected ID 'new-team-uuid', got %q", result.ID) + } + if result.Name != "New Team" { + t.Errorf("expected name 'New Team', got %q", result.Name) + } +} + +func TestUpdateTeam(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PATCH" { + t.Errorf("expected PATCH, got %s", r.Method) + } + if !strings.Contains(r.URL.Path, "/teams/team-uuid-123") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + + var input buildkite.CreateTeam + if err := json.NewDecoder(r.Body).Decode(&input); err != nil { + t.Fatal(err) + } + + if input.Name != "Renamed Team" { + t.Errorf("expected name 'Renamed Team', got %q", input.Name) + } + if input.Privacy != "visible" { + t.Errorf("expected privacy 'visible', got %q", input.Privacy) + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(buildkite.Team{ + ID: "team-uuid-123", + Name: input.Name, + Privacy: input.Privacy, + }) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + result, _, err := client.Teams.UpdateTeam(context.Background(), "test-org", "team-uuid-123", buildkite.CreateTeam{ + Name: "Renamed Team", + Privacy: "visible", + }) + if err != nil { + t.Fatal(err) + } + + if result.Name != "Renamed Team" { + t.Errorf("expected name 'Renamed Team', got %q", result.Name) + } + if result.Privacy != "visible" { + t.Errorf("expected privacy 'visible', got %q", result.Privacy) + } +} + +func TestDeleteTeam(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "DELETE" { + t.Errorf("expected DELETE, got %s", r.Method) + } + if !strings.Contains(r.URL.Path, "/teams/team-uuid-123") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusNoContent) + })) + defer s.Close() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(s.URL)) + if err != nil { + t.Fatal(err) + } + + _, err = client.Teams.DeleteTeam(context.Background(), "test-org", "team-uuid-123") + if err != nil { + t.Fatal(err) + } +} + +func TestUpdateCmdValidate(t *testing.T) { + t.Parallel() + + boolTrue := true + boolFalse := false + + tests := []struct { + name string + cmd UpdateCmd + wantErr bool + }{ + { + name: "no flags set", + cmd: UpdateCmd{TeamUUID: "team-uuid"}, + wantErr: true, + }, + { + name: "only name", + cmd: UpdateCmd{TeamUUID: "team-uuid", Name: "New Name"}, + wantErr: false, + }, + { + name: "only description", + cmd: UpdateCmd{TeamUUID: "team-uuid", Description: "new desc"}, + wantErr: false, + }, + { + name: "valid privacy visible", + cmd: UpdateCmd{TeamUUID: "team-uuid", Privacy: "visible"}, + wantErr: false, + }, + { + name: "valid privacy secret", + cmd: UpdateCmd{TeamUUID: "team-uuid", Privacy: "secret"}, + wantErr: false, + }, + { + name: "invalid privacy", + cmd: UpdateCmd{TeamUUID: "team-uuid", Privacy: "public"}, + wantErr: true, + }, + { + name: "only default true", + cmd: UpdateCmd{TeamUUID: "team-uuid", Default: &boolTrue}, + wantErr: false, + }, + { + name: "only default false", + cmd: UpdateCmd{TeamUUID: "team-uuid", Default: &boolFalse}, + wantErr: false, + }, + { + name: "valid default member role member", + cmd: UpdateCmd{TeamUUID: "team-uuid", DefaultMemberRole: "member"}, + wantErr: false, + }, + { + name: "valid default member role maintainer", + cmd: UpdateCmd{TeamUUID: "team-uuid", DefaultMemberRole: "maintainer"}, + wantErr: false, + }, + { + name: "invalid default member role", + cmd: UpdateCmd{TeamUUID: "team-uuid", DefaultMemberRole: "admin"}, + wantErr: true, + }, + { + name: "only members-can-create-pipelines", + cmd: UpdateCmd{TeamUUID: "team-uuid", MembersCanCreatePipelines: &boolTrue}, + wantErr: false, + }, + { + name: "multiple valid flags", + cmd: UpdateCmd{TeamUUID: "team-uuid", Name: "New Name", Privacy: "secret", DefaultMemberRole: "maintainer"}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := tt.cmd.Validate() + if (err != nil) != tt.wantErr { + t.Errorf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/cmd/team/update.go b/cmd/team/update.go new file mode 100644 index 00000000..6389ee7e --- /dev/null +++ b/cmd/team/update.go @@ -0,0 +1,142 @@ +package team + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + bkIO "github.com/buildkite/cli/v3/internal/io" + "github.com/buildkite/cli/v3/internal/team" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" + "github.com/buildkite/cli/v3/pkg/output" + buildkite "github.com/buildkite/go-buildkite/v4" +) + +type UpdateCmd struct { + TeamUUID string `arg:"" help:"UUID of the team to update" name:"team-uuid"` + Name string `help:"New name for the team" optional:""` + Description string `help:"New description for the team" optional:""` + Privacy string `help:"Privacy setting: visible or secret" optional:""` + Default *bool `help:"Whether this is the default team for new members" optional:"" name:"default"` + DefaultMemberRole string `help:"Default role for new members: member or maintainer" optional:"" name:"default-member-role"` + MembersCanCreatePipelines *bool `help:"Whether members can create pipelines" optional:"" name:"members-can-create-pipelines"` + output.OutputFlags +} + +func (c *UpdateCmd) Help() string { + return ` +Update an existing team's settings. + +Examples: + # Rename a team + $ bk team update my-team-uuid --name "New Team Name" + + # Change a team's privacy + $ bk team update my-team-uuid --privacy secret + + # Update description and default member role + $ bk team update my-team-uuid --description "Updated description" --default-member-role maintainer +` +} + +func (c *UpdateCmd) Validate() error { + if c.Name == "" && c.Description == "" && c.Privacy == "" && c.Default == nil && c.DefaultMemberRole == "" && c.MembersCanCreatePipelines == nil { + return fmt.Errorf("at least one of --name, --description, --privacy, --default, --default-member-role, or --members-can-create-pipelines must be provided") + } + if c.Privacy != "" && c.Privacy != "visible" && c.Privacy != "secret" { + return fmt.Errorf("--privacy must be either \"visible\" or \"secret\"") + } + if c.DefaultMemberRole != "" && c.DefaultMemberRole != "member" && c.DefaultMemberRole != "maintainer" { + return fmt.Errorf("--default-member-role must be either \"member\" or \"maintainer\"") + } + return nil +} + +func (c *UpdateCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New(factory.WithDebug(globals.EnableDebug())) + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + f.NoPager = f.NoPager || globals.DisablePager() + + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err + } + + format := output.ResolveFormat(c.Output, f.Config.OutputFormat()) + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + // Fetch current state to use as base for update + var current buildkite.Team + spinErr := bkIO.SpinWhile(f, "Loading team", func() { + current, err = f.RestAPIClient.Teams.GetTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) + }) + if spinErr != nil { + return spinErr + } + if err != nil { + return fmt.Errorf("error fetching team: %v", err) + } + + // Build update input from current values, overriding with any flags set + input := buildkite.CreateTeam{ + Name: current.Name, + Description: current.Description, + Privacy: current.Privacy, + IsDefaultTeam: current.Default, + MembersCanCreatePipelines: false, + } + if c.Name != "" { + input.Name = c.Name + } + if c.Description != "" { + input.Description = c.Description + } + if c.Privacy != "" { + input.Privacy = c.Privacy + } + if c.Default != nil { + input.IsDefaultTeam = *c.Default + } + if c.DefaultMemberRole != "" { + input.DefaultMemberRole = c.DefaultMemberRole + } + if c.MembersCanCreatePipelines != nil { + input.MembersCanCreatePipelines = *c.MembersCanCreatePipelines + } + + var t buildkite.Team + spinErr = bkIO.SpinWhile(f, "Updating team", func() { + t, _, err = f.RestAPIClient.Teams.UpdateTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID, input) + }) + if spinErr != nil { + return spinErr + } + if err != nil { + return fmt.Errorf("error updating team: %v", err) + } + + teamView := output.Viewable[buildkite.Team]{ + Data: t, + Render: team.RenderTeamText, + } + + if format != output.FormatText { + return output.Write(os.Stdout, teamView, format) + } + + fmt.Fprintln(os.Stderr, "Team updated successfully.") + fmt.Fprintln(os.Stdout) + return output.Write(os.Stdout, teamView, format) +} diff --git a/cmd/team/view.go b/cmd/team/view.go new file mode 100644 index 00000000..a63ee8ba --- /dev/null +++ b/cmd/team/view.go @@ -0,0 +1,82 @@ +package team + +import ( + "context" + "fmt" + "os" + "os/signal" + "syscall" + + "github.com/alecthomas/kong" + "github.com/buildkite/cli/v3/internal/cli" + bkIO "github.com/buildkite/cli/v3/internal/io" + "github.com/buildkite/cli/v3/internal/team" + "github.com/buildkite/cli/v3/pkg/cmd/factory" + "github.com/buildkite/cli/v3/pkg/cmd/validation" + "github.com/buildkite/cli/v3/pkg/output" + buildkite "github.com/buildkite/go-buildkite/v4" +) + +type ViewCmd struct { + TeamUUID string `arg:"" help:"UUID of the team to view" name:"team-uuid"` + output.OutputFlags +} + +func (c *ViewCmd) Help() string { + return ` +It accepts a team UUID. + +Examples: + # View a team + $ bk team view my-team-uuid + + # View team in JSON format + $ bk team view my-team-uuid -o json +` +} + +func (c *ViewCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { + f, err := factory.New(factory.WithDebug(globals.EnableDebug())) + if err != nil { + return err + } + + f.SkipConfirm = globals.SkipConfirmation() + f.NoInput = globals.DisableInput() + f.Quiet = globals.IsQuiet() + f.NoPager = f.NoPager || globals.DisablePager() + + if err := validation.ValidateConfiguration(f.Config, kongCtx.Command()); err != nil { + return err + } + + format := output.ResolveFormat(c.Output, f.Config.OutputFormat()) + + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer stop() + + var t buildkite.Team + spinErr := bkIO.SpinWhile(f, "Loading team information", func() { + t, err = f.RestAPIClient.Teams.GetTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) + }) + if spinErr != nil { + return spinErr + } + if err != nil { + return fmt.Errorf("error fetching team: %v", err) + } + + teamView := output.Viewable[buildkite.Team]{ + Data: t, + Render: team.RenderTeamText, + } + + if format != output.FormatText { + return output.Write(os.Stdout, teamView, format) + } + + writer, cleanup := bkIO.Pager(f.NoPager, f.Config.Pager()) + defer func() { _ = cleanup() }() + + return output.Write(writer, teamView, format) +} diff --git a/internal/team/view.go b/internal/team/view.go new file mode 100644 index 00000000..0ff0d95b --- /dev/null +++ b/internal/team/view.go @@ -0,0 +1,77 @@ +package team + +import ( + "fmt" + "strings" + "time" + + "github.com/buildkite/cli/v3/pkg/output" + buildkite "github.com/buildkite/go-buildkite/v4" +) + +// TeamViewTable renders a table view of one or more teams +func TeamViewTable(t ...buildkite.Team) string { + if len(t) == 0 { + return "No teams found." + } + + if len(t) == 1 { + return renderSingleTeamDetail(t[0]) + } + + rows := make([][]string, 0, len(t)) + for _, team := range t { + rows = append(rows, []string{ + output.ValueOrDash(team.Name), + output.ValueOrDash(team.Slug), + output.ValueOrDash(team.Privacy), + fmt.Sprintf("%v", team.Default), + }) + } + + return output.Table( + []string{"Name", "Slug", "Privacy", "Default"}, + rows, + map[string]string{"name": "bold", "slug": "dim", "privacy": "dim", "default": "dim"}, + ) +} + +// RenderTeamText renders a single team as a human-readable text table. +func RenderTeamText(t buildkite.Team) string { + return renderSingleTeamDetail(t) +} + +func renderSingleTeamDetail(t buildkite.Team) string { + var sb strings.Builder + fmt.Fprintf(&sb, "Viewing %s\n\n", output.ValueOrDash(t.Name)) + + rows := [][]string{ + {"Name", output.ValueOrDash(t.Name)}, + {"Slug", output.ValueOrDash(t.Slug)}, + {"Description", output.ValueOrDash(t.Description)}, + {"Privacy", output.ValueOrDash(t.Privacy)}, + {"Default", fmt.Sprintf("%v", t.Default)}, + {"ID", output.ValueOrDash(t.ID)}, + } + + if t.CreatedBy != nil && t.CreatedBy.ID != "" { + rows = append(rows, + []string{"Created By Name", output.ValueOrDash(t.CreatedBy.Name)}, + []string{"Created By Email", output.ValueOrDash(t.CreatedBy.Email)}, + []string{"Created By ID", output.ValueOrDash(t.CreatedBy.ID)}, + ) + } + + if t.CreatedAt != nil { + rows = append(rows, []string{"Created At", t.CreatedAt.Format(time.RFC3339)}) + } + + table := output.Table( + []string{"Field", "Value"}, + rows, + map[string]string{"field": "dim", "value": "italic"}, + ) + + sb.WriteString(table) + return sb.String() +} diff --git a/internal/team/view_test.go b/internal/team/view_test.go new file mode 100644 index 00000000..719a25b3 --- /dev/null +++ b/internal/team/view_test.go @@ -0,0 +1,133 @@ +package team + +import ( + "strings" + "testing" + "time" + + buildkite "github.com/buildkite/go-buildkite/v4" +) + +func TestTeamViewTable(t *testing.T) { + t.Parallel() + + t.Run("empty slice returns no teams message", func(t *testing.T) { + t.Parallel() + + result := TeamViewTable() + if result != "No teams found." { + t.Errorf("expected 'No teams found.', got %q", result) + } + }) + + t.Run("single team renders detail view", func(t *testing.T) { + t.Parallel() + + team := buildkite.Team{ + ID: "team-uuid-123", + Name: "Frontend", + Slug: "frontend", + Description: "The frontend team", + Privacy: "visible", + Default: true, + } + + result := TeamViewTable(team) + + for _, expected := range []string{"Frontend", "frontend", "The frontend team", "visible", "true", "team-uuid-123"} { + if !strings.Contains(result, expected) { + t.Errorf("expected output to contain %q, got:\n%s", expected, result) + } + } + }) + + t.Run("multiple teams renders summary table", func(t *testing.T) { + t.Parallel() + + teams := []buildkite.Team{ + {ID: "team-1", Name: "Frontend", Slug: "frontend", Privacy: "visible", Default: false}, + {ID: "team-2", Name: "Backend", Slug: "backend", Privacy: "secret", Default: true}, + } + + result := TeamViewTable(teams...) + + // Should have table headers + for _, header := range []string{"NAME", "SLUG", "PRIVACY", "DEFAULT"} { + if !strings.Contains(result, header) { + t.Errorf("expected table header %q, got:\n%s", header, result) + } + } + // Should have both team names + if !strings.Contains(result, "Frontend") { + t.Errorf("expected output to contain 'Frontend':\n%s", result) + } + if !strings.Contains(result, "Backend") { + t.Errorf("expected output to contain 'Backend':\n%s", result) + } + // Should not render UUIDs in the summary table (only name/slug/privacy/default columns) + if strings.Contains(result, "team-1") { + t.Errorf("expected summary table to omit IDs, got:\n%s", result) + } + }) +} + +func TestRenderTeamText(t *testing.T) { + t.Parallel() + + createdAt := buildkite.Timestamp{Time: time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)} + + team := buildkite.Team{ + ID: "team-uuid-123", + Name: "Fearless Frontenders", + Slug: "fearless-frontenders", + Description: "Frontend engineers", + Privacy: "secret", + Default: true, + CreatedAt: &createdAt, + CreatedBy: &buildkite.User{ + ID: "user-1", + Name: "Peter Pettigrew", + Email: "pp@hogwarts.co.uk", + }, + } + + result := RenderTeamText(team) + + for _, expected := range []string{ + "Fearless Frontenders", + "fearless-frontenders", + "Frontend engineers", + "secret", + "true", + "team-uuid-123", + "Peter Pettigrew", + "pp@hogwarts.co.uk", + "user-1", + "2024-01-15T10:30:00Z", + } { + if !strings.Contains(result, expected) { + t.Errorf("expected output to contain %q, got:\n%s", expected, result) + } + } +} + +func TestRenderTeamText_NoCreatedBy(t *testing.T) { + t.Parallel() + + team := buildkite.Team{ + ID: "team-uuid-123", + Name: "Minimal Team", + Slug: "minimal-team", + Privacy: "visible", + } + + result := RenderTeamText(team) + + if !strings.Contains(result, "Minimal Team") { + t.Errorf("expected output to contain 'Minimal Team', got:\n%s", result) + } + // CreatedBy fields should be absent when not set + if strings.Contains(result, "Created By") { + t.Errorf("expected no 'Created By' fields when CreatedBy is nil, got:\n%s", result) + } +} diff --git a/main.go b/main.go index da688a3b..c88acd9d 100644 --- a/main.go +++ b/main.go @@ -23,6 +23,7 @@ import ( "github.com/buildkite/cli/v3/cmd/pkg" "github.com/buildkite/cli/v3/cmd/preflight" "github.com/buildkite/cli/v3/cmd/secret" + "github.com/buildkite/cli/v3/cmd/team" "github.com/buildkite/cli/v3/cmd/use" "github.com/buildkite/cli/v3/cmd/user" versionPkg "github.com/buildkite/cli/v3/cmd/version" @@ -48,6 +49,7 @@ type CLI struct { Build BuildCmd `cmd:"" help:"Manage pipeline builds"` Cluster ClusterCmd `cmd:"" help:"Manage organization clusters"` Secret SecretCmd `cmd:"" help:"Manage cluster secrets"` + Team TeamCmd `cmd:"" help:"Manage organization teams"` Config bkConfig.ConfigCmd `cmd:"" help:"Manage CLI configuration"` Configure ConfigureCmd `cmd:"" help:"Configure Buildkite API token" hidden:""` Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` @@ -131,6 +133,13 @@ type ( Validate pipeline.ValidateCmd `cmd:"" help:"Validate a pipeline YAML file."` View pipeline.ViewCmd `cmd:"" help:"View a pipeline."` } + TeamCmd struct { + List team.ListCmd `cmd:"" help:"List teams." aliases:"ls"` + View team.ViewCmd `cmd:"" help:"View team information."` + Create team.CreateCmd `cmd:"" help:"Create a new team."` + Update team.UpdateCmd `cmd:"" help:"Update a team."` + Delete team.DeleteCmd `cmd:"" help:"Delete a team." aliases:"rm"` + } UserCmd struct { Invite user.InviteCmd `cmd:"" help:"Invite users to your organization."` } From d4f894de29e9eba822a009cdeac959d9e04b87b7 Mon Sep 17 00:00:00 2001 From: Ben McNicholl Date: Wed, 1 Apr 2026 12:01:56 +1100 Subject: [PATCH 2/4] chore: allow teams list pagination From 9fd2106f85d8b9187433db42d0062be8ded2cf29 Mon Sep 17 00:00:00 2001 From: Ben McNicholl Date: Mon, 27 Jul 2026 13:32:52 +1000 Subject: [PATCH 3/4] fix(general): refactor code with new version shape --- cmd/team/create.go | 11 +++++----- cmd/team/delete.go | 10 ++++----- cmd/team/list.go | 12 +++++----- cmd/team/team_test.go | 8 +++---- cmd/team/update.go | 45 +++++++++++--------------------------- cmd/team/view.go | 11 +++++----- internal/team/view.go | 2 +- internal/team/view_test.go | 2 +- main.go | 30 ++----------------------- 9 files changed, 40 insertions(+), 91 deletions(-) diff --git a/cmd/team/create.go b/cmd/team/create.go index 39a5a32c..c2055c1c 100644 --- a/cmd/team/create.go +++ b/cmd/team/create.go @@ -14,7 +14,7 @@ import ( "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" "github.com/buildkite/cli/v3/pkg/output" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) type CreateCmd struct { @@ -73,14 +73,13 @@ func (c *CreateCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { } var t buildkite.Team - spinErr := bkIO.SpinWhile(f, "Creating team", func() { + spinErr := bkIO.SpinWhile(f, "Creating team", func() error { + var err error t, _, err = f.RestAPIClient.Teams.CreateTeam(ctx, f.Config.OrganizationSlug(), input) + return err }) if spinErr != nil { - return spinErr - } - if err != nil { - return fmt.Errorf("error creating team: %v", err) + return fmt.Errorf("error creating team: %v", spinErr) } teamView := output.Viewable[buildkite.Team]{ diff --git a/cmd/team/delete.go b/cmd/team/delete.go index 075ad593..f8b9f972 100644 --- a/cmd/team/delete.go +++ b/cmd/team/delete.go @@ -59,14 +59,12 @@ func (c *DeleteCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { return nil } - spinErr := bkIO.SpinWhile(f, "Deleting team", func() { - _, err = f.RestAPIClient.Teams.DeleteTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) + spinErr := bkIO.SpinWhile(f, "Deleting team", func() error { + _, err := f.RestAPIClient.Teams.DeleteTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) + return err }) if spinErr != nil { - return spinErr - } - if err != nil { - return fmt.Errorf("error deleting team: %v", err) + return fmt.Errorf("error deleting team: %v", spinErr) } fmt.Fprintln(os.Stderr, "Team deleted successfully.") diff --git a/cmd/team/list.go b/cmd/team/list.go index 3add1221..1b3b2a02 100644 --- a/cmd/team/list.go +++ b/cmd/team/list.go @@ -15,7 +15,7 @@ import ( "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" "github.com/buildkite/cli/v3/pkg/output" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) type ListCmd struct { @@ -86,7 +86,6 @@ func (c *ListCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { func listTeams(ctx context.Context, f *factory.Factory, perPage, limit int) ([]buildkite.Team, bool, error) { var all []buildkite.Team - var err error page := 1 hasMore := false var previousFirstTeamID string @@ -100,14 +99,13 @@ func listTeams(ctx context.Context, f *factory.Factory, perPage, limit int) ([]b } var pageTeams []buildkite.Team - spinErr := bkIO.SpinWhile(f, "Loading teams", func() { + spinErr := bkIO.SpinWhile(f, "Loading teams", func() error { + var err error pageTeams, _, err = f.RestAPIClient.Teams.List(ctx, f.Config.OrganizationSlug(), opts) + return err }) if spinErr != nil { - return nil, false, spinErr - } - if err != nil { - return nil, false, fmt.Errorf("error fetching team list: %v", err) + return nil, false, fmt.Errorf("error fetching team list: %v", spinErr) } if len(pageTeams) == 0 { diff --git a/cmd/team/team_test.go b/cmd/team/team_test.go index d5e876c2..7b24bc87 100644 --- a/cmd/team/team_test.go +++ b/cmd/team/team_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) func makeTeams(n, offset int) []buildkite.Team { @@ -354,9 +354,9 @@ func TestUpdateTeam(t *testing.T) { t.Fatal(err) } - result, _, err := client.Teams.UpdateTeam(context.Background(), "test-org", "team-uuid-123", buildkite.CreateTeam{ - Name: "Renamed Team", - Privacy: "visible", + result, _, err := client.Teams.UpdateTeam(context.Background(), "test-org", "team-uuid-123", buildkite.UpdateTeam{ + Name: buildkite.Some("Renamed Team"), + Privacy: buildkite.Some("visible"), }) if err != nil { t.Fatal(err) diff --git a/cmd/team/update.go b/cmd/team/update.go index 6389ee7e..2affc64e 100644 --- a/cmd/team/update.go +++ b/cmd/team/update.go @@ -14,7 +14,7 @@ import ( "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" "github.com/buildkite/cli/v3/pkg/output" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) type UpdateCmd struct { @@ -77,54 +77,35 @@ func (c *UpdateCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - // Fetch current state to use as base for update - var current buildkite.Team - spinErr := bkIO.SpinWhile(f, "Loading team", func() { - current, err = f.RestAPIClient.Teams.GetTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) - }) - if spinErr != nil { - return spinErr - } - if err != nil { - return fmt.Errorf("error fetching team: %v", err) - } - - // Build update input from current values, overriding with any flags set - input := buildkite.CreateTeam{ - Name: current.Name, - Description: current.Description, - Privacy: current.Privacy, - IsDefaultTeam: current.Default, - MembersCanCreatePipelines: false, - } + // UpdateTeam is a PATCH; only fields that are set are sent + var input buildkite.UpdateTeam if c.Name != "" { - input.Name = c.Name + input.Name = buildkite.Some(c.Name) } if c.Description != "" { - input.Description = c.Description + input.Description = buildkite.Some(c.Description) } if c.Privacy != "" { - input.Privacy = c.Privacy + input.Privacy = buildkite.Some(c.Privacy) } if c.Default != nil { - input.IsDefaultTeam = *c.Default + input.IsDefaultTeam = buildkite.Some(*c.Default) } if c.DefaultMemberRole != "" { - input.DefaultMemberRole = c.DefaultMemberRole + input.DefaultMemberRole = buildkite.Some(c.DefaultMemberRole) } if c.MembersCanCreatePipelines != nil { - input.MembersCanCreatePipelines = *c.MembersCanCreatePipelines + input.MembersCanCreatePipelines = buildkite.Some(*c.MembersCanCreatePipelines) } var t buildkite.Team - spinErr = bkIO.SpinWhile(f, "Updating team", func() { + spinErr := bkIO.SpinWhile(f, "Updating team", func() error { + var err error t, _, err = f.RestAPIClient.Teams.UpdateTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID, input) + return err }) if spinErr != nil { - return spinErr - } - if err != nil { - return fmt.Errorf("error updating team: %v", err) + return fmt.Errorf("error updating team: %v", spinErr) } teamView := output.Viewable[buildkite.Team]{ diff --git a/cmd/team/view.go b/cmd/team/view.go index a63ee8ba..167c2c27 100644 --- a/cmd/team/view.go +++ b/cmd/team/view.go @@ -14,7 +14,7 @@ import ( "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" "github.com/buildkite/cli/v3/pkg/output" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) type ViewCmd struct { @@ -56,14 +56,13 @@ func (c *ViewCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { defer stop() var t buildkite.Team - spinErr := bkIO.SpinWhile(f, "Loading team information", func() { + spinErr := bkIO.SpinWhile(f, "Loading team information", func() error { + var err error t, err = f.RestAPIClient.Teams.GetTeam(ctx, f.Config.OrganizationSlug(), c.TeamUUID) + return err }) if spinErr != nil { - return spinErr - } - if err != nil { - return fmt.Errorf("error fetching team: %v", err) + return fmt.Errorf("error fetching team: %v", spinErr) } teamView := output.Viewable[buildkite.Team]{ diff --git a/internal/team/view.go b/internal/team/view.go index 0ff0d95b..39ba74d6 100644 --- a/internal/team/view.go +++ b/internal/team/view.go @@ -6,7 +6,7 @@ import ( "time" "github.com/buildkite/cli/v3/pkg/output" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) // TeamViewTable renders a table view of one or more teams diff --git a/internal/team/view_test.go b/internal/team/view_test.go index 719a25b3..0fa76a1d 100644 --- a/internal/team/view_test.go +++ b/internal/team/view_test.go @@ -5,7 +5,7 @@ import ( "testing" "time" - buildkite "github.com/buildkite/go-buildkite/v4" + buildkite "github.com/buildkite/go-buildkite/v5" ) func TestTeamViewTable(t *testing.T) { diff --git a/main.go b/main.go index 66c9706c..08f28f40 100644 --- a/main.go +++ b/main.go @@ -27,8 +27,8 @@ import ( "github.com/buildkite/cli/v3/cmd/preflight" "github.com/buildkite/cli/v3/cmd/queue" "github.com/buildkite/cli/v3/cmd/secret" - "github.com/buildkite/cli/v3/cmd/team" "github.com/buildkite/cli/v3/cmd/skill" + "github.com/buildkite/cli/v3/cmd/team" updatePkg "github.com/buildkite/cli/v3/cmd/update" "github.com/buildkite/cli/v3/cmd/use" "github.com/buildkite/cli/v3/cmd/user" @@ -44,31 +44,6 @@ import ( // Kong CLI structure, with base commands defined as additional commands are defined in their respective files type CLI struct { // Global flags - Yes bool `help:"Skip all confirmation prompts" short:"y"` - NoInput bool `help:"Disable all interactive prompts" name:"no-input"` - Quiet bool `help:"Suppress progress output" short:"q"` - NoPager bool `help:"Disable pager for text output" name:"no-pager"` - Debug bool `help:"Enable debug output for REST API calls"` - Agent AgentCmd `cmd:"" help:"Manage agents"` - Api ApiCmd `cmd:"" help:"Interact with the Buildkite API"` - Artifacts ArtifactsCmd `cmd:"" help:"Manage pipeline build artifacts"` - Auth AuthCmd `cmd:"" help:"Authenticate with Buildkite"` - Build BuildCmd `cmd:"" help:"Manage pipeline builds"` - Cluster ClusterCmd `cmd:"" help:"Manage organization clusters"` - Secret SecretCmd `cmd:"" help:"Manage cluster secrets"` - Team TeamCmd `cmd:"" help:"Manage organization teams"` - Config bkConfig.ConfigCmd `cmd:"" help:"Manage CLI configuration"` - Configure ConfigureCmd `cmd:"" help:"Configure Buildkite API token" hidden:""` - Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` - Job JobCmd `cmd:"" help:"Manage jobs within a build"` - Organization OrganizationCmd `cmd:"" help:"Manage organizations" aliases:"org"` - Pipeline PipelineCmd `cmd:"" help:"Manage pipelines"` - Package PackageCmd `cmd:"" help:"Manage packages"` - Preflight preflight.PreflightCmd `cmd:"" help:"Validate pre-commit changes"` - Use use.UseCmd `cmd:"" help:"Select an organization" hidden:""` - User UserCmd `cmd:"" help:"Invite users to the organization"` - Version VersionCmd `cmd:"" help:"Print the version of the CLI being used"` - Whoami whoami.WhoAmICmd `cmd:"" help:"Print the current user and organization" hidden:""` Yes bool `help:"Skip all confirmation prompts" short:"y"` NoInput bool `help:"Disable all interactive prompts" name:"no-input"` Quiet bool `help:"Suppress progress output" short:"q"` @@ -85,6 +60,7 @@ type CLI struct { Queue QueueCmd `cmd:"" help:"Manage cluster queues"` Secret SecretCmd `cmd:"" help:"Manage cluster secrets"` Skill SkillCmd `cmd:"" help:"Manage Buildkite skills for AI coding agents"` + Team TeamCmd `cmd:"" help:"Manage organization teams"` Config bkConfig.ConfigCmd `cmd:"" help:"Manage CLI configuration"` Configure ConfigureCmd `cmd:"" help:"Configure Buildkite API token" hidden:""` Init bkInit.InitCmd `cmd:"" help:"Initialize a pipeline.yaml file"` @@ -183,8 +159,6 @@ type ( Update team.UpdateCmd `cmd:"" help:"Update a team."` Delete team.DeleteCmd `cmd:"" help:"Delete a team." aliases:"rm"` } - UserCmd struct { - Invite user.InviteCmd `cmd:"" help:"Invite users to your organization."` PreflightCmd struct { Run preflight.RunCmd `cmd:"" default:"withargs" help:"Run a build against a snapshot of the local working tree (experimental)"` Cleanup preflight.CleanupCmd `cmd:"" help:"Clean up completed preflight branches (experimental)"` From 5e13e682f648620bff9c26fcad5be6baaf426ca1 Mon Sep 17 00:00:00 2001 From: Ben McNicholl Date: Mon, 27 Jul 2026 14:12:51 +1000 Subject: [PATCH 4/4] chore(feedback): fix pagination edge cases --- cmd/team/list.go | 26 +++++++-- cmd/team/team_test.go | 112 ++++++++++++++++++++++++++++++++++++- cmd/team/update.go | 20 +++---- internal/team/view.go | 2 + internal/team/view_test.go | 19 ++++--- 5 files changed, 156 insertions(+), 23 deletions(-) diff --git a/cmd/team/list.go b/cmd/team/list.go index 1b3b2a02..9afb4121 100644 --- a/cmd/team/list.go +++ b/cmd/team/list.go @@ -2,7 +2,6 @@ package team import ( "context" - "errors" "fmt" "os" "os/signal" @@ -24,6 +23,18 @@ type ListCmd struct { output.OutputFlags } +func (c *ListCmd) Validate() error { + if c.PerPage < 1 { + return fmt.Errorf("invalid --per-page %d: must be greater than 0", c.PerPage) + } + + if c.Limit < 0 { + return fmt.Errorf("invalid --limit %d: must be greater than or equal to 0", c.Limit) + } + + return nil +} + func (c *ListCmd) Help() string { return ` List the teams for an organization. By default, shows up to 100 teams. @@ -69,6 +80,11 @@ func (c *ListCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { return output.Write(os.Stdout, teams, format) } + if len(teams) == 0 { + fmt.Fprintln(os.Stdout, "No teams found.") + return nil + } + summary := team.TeamViewTable(teams...) writer, cleanup := bkIO.Pager(f.NoPager, f.Config.Pager()) @@ -119,6 +135,10 @@ func listTeams(ctx context.Context, f *factory.Factory, perPage, limit int) ([]b all = append(all, pageTeams...) + if len(all) > limit { + hasMore = true + } + if len(pageTeams) < perPage { break } @@ -135,9 +155,5 @@ func listTeams(ctx context.Context, f *factory.Factory, perPage, limit int) ([]b all = all[:limit] } - if len(all) == 0 { - return nil, false, errors.New("no teams found in organization") - } - return all, hasMore, nil } diff --git a/cmd/team/team_test.go b/cmd/team/team_test.go index 7b24bc87..64c4f6b7 100644 --- a/cmd/team/team_test.go +++ b/cmd/team/team_test.go @@ -6,10 +6,14 @@ import ( "fmt" "net/http" "net/http/httptest" + "strconv" "strings" "testing" + "github.com/buildkite/cli/v3/internal/config" + "github.com/buildkite/cli/v3/pkg/cmd/factory" buildkite "github.com/buildkite/go-buildkite/v5" + "github.com/spf13/afero" ) func makeTeams(n, offset int) []buildkite.Team { @@ -400,6 +404,8 @@ func TestUpdateCmdValidate(t *testing.T) { boolTrue := true boolFalse := false + desc := "new desc" + emptyDesc := "" tests := []struct { name string @@ -418,7 +424,12 @@ func TestUpdateCmdValidate(t *testing.T) { }, { name: "only description", - cmd: UpdateCmd{TeamUUID: "team-uuid", Description: "new desc"}, + cmd: UpdateCmd{TeamUUID: "team-uuid", Description: &desc}, + wantErr: false, + }, + { + name: "explicitly empty description clears it", + cmd: UpdateCmd{TeamUUID: "team-uuid", Description: &emptyDesc}, wantErr: false, }, { @@ -484,3 +495,102 @@ func TestUpdateCmdValidate(t *testing.T) { }) } } + +func newListTestFactory(t *testing.T, baseURL string) *factory.Factory { + t.Helper() + + client, err := buildkite.NewOpts(buildkite.WithBaseURL(baseURL)) + if err != nil { + t.Fatal(err) + } + conf := config.New(afero.NewMemMapFs(), nil) + if err := conf.SelectOrganization("test-org", true); err != nil { + t.Fatalf("SelectOrganization() error = %v", err) + } + return &factory.Factory{ + Config: conf, + RestAPIClient: client, + Quiet: true, + } +} + +func TestListTeamsPagination(t *testing.T) { + t.Parallel() + + t.Run("partial final page crossing the limit sets hasMore", func(t *testing.T) { + t.Parallel() + + // Pages 1-3 return 30 teams each, page 4 returns a partial page of 15, + // so 105 teams are collected against a limit of 100. + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + w.Header().Set("Content-Type", "application/json") + switch { + case page >= 1 && page <= 3: + json.NewEncoder(w).Encode(makeTeams(30, (page-1)*30)) + case page == 4: + json.NewEncoder(w).Encode(makeTeams(15, 90)) + default: + json.NewEncoder(w).Encode([]buildkite.Team{}) + } + })) + defer s.Close() + + teams, hasMore, err := listTeams(context.Background(), newListTestFactory(t, s.URL), 30, 100) + if err != nil { + t.Fatal(err) + } + if len(teams) != 100 { + t.Errorf("expected 100 teams, got %d", len(teams)) + } + if !hasMore { + t.Error("expected hasMore to be true when results were truncated to the limit") + } + }) + + t.Run("results under the limit leave hasMore false", func(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + w.Header().Set("Content-Type", "application/json") + if page == 1 { + json.NewEncoder(w).Encode(makeTeams(30, 0)) + } else { + json.NewEncoder(w).Encode(makeTeams(15, 30)) + } + })) + defer s.Close() + + teams, hasMore, err := listTeams(context.Background(), newListTestFactory(t, s.URL), 30, 100) + if err != nil { + t.Fatal(err) + } + if len(teams) != 45 { + t.Errorf("expected 45 teams, got %d", len(teams)) + } + if hasMore { + t.Error("expected hasMore to be false when all teams fit within the limit") + } + }) + + t.Run("limit of zero returns no teams without a request", func(t *testing.T) { + t.Parallel() + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Error("expected no API request when limit is 0") + })) + defer s.Close() + + teams, hasMore, err := listTeams(context.Background(), newListTestFactory(t, s.URL), 30, 0) + if err != nil { + t.Fatal(err) + } + if len(teams) != 0 { + t.Errorf("expected no teams, got %d", len(teams)) + } + if hasMore { + t.Error("expected hasMore to be false for a zero limit") + } + }) +} diff --git a/cmd/team/update.go b/cmd/team/update.go index 2affc64e..2c4684b5 100644 --- a/cmd/team/update.go +++ b/cmd/team/update.go @@ -18,13 +18,13 @@ import ( ) type UpdateCmd struct { - TeamUUID string `arg:"" help:"UUID of the team to update" name:"team-uuid"` - Name string `help:"New name for the team" optional:""` - Description string `help:"New description for the team" optional:""` - Privacy string `help:"Privacy setting: visible or secret" optional:""` - Default *bool `help:"Whether this is the default team for new members" optional:"" name:"default"` - DefaultMemberRole string `help:"Default role for new members: member or maintainer" optional:"" name:"default-member-role"` - MembersCanCreatePipelines *bool `help:"Whether members can create pipelines" optional:"" name:"members-can-create-pipelines"` + TeamUUID string `arg:"" help:"UUID of the team to update" name:"team-uuid"` + Name string `help:"New name for the team" optional:""` + Description *string `help:"New description for the team" optional:""` + Privacy string `help:"Privacy setting: visible or secret" optional:""` + Default *bool `help:"Whether this is the default team for new members" optional:"" name:"default"` + DefaultMemberRole string `help:"Default role for new members: member or maintainer" optional:"" name:"default-member-role"` + MembersCanCreatePipelines *bool `help:"Whether members can create pipelines" optional:"" name:"members-can-create-pipelines"` output.OutputFlags } @@ -45,7 +45,7 @@ Examples: } func (c *UpdateCmd) Validate() error { - if c.Name == "" && c.Description == "" && c.Privacy == "" && c.Default == nil && c.DefaultMemberRole == "" && c.MembersCanCreatePipelines == nil { + if c.Name == "" && c.Description == nil && c.Privacy == "" && c.Default == nil && c.DefaultMemberRole == "" && c.MembersCanCreatePipelines == nil { return fmt.Errorf("at least one of --name, --description, --privacy, --default, --default-member-role, or --members-can-create-pipelines must be provided") } if c.Privacy != "" && c.Privacy != "visible" && c.Privacy != "secret" { @@ -82,8 +82,8 @@ func (c *UpdateCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error { if c.Name != "" { input.Name = buildkite.Some(c.Name) } - if c.Description != "" { - input.Description = buildkite.Some(c.Description) + if c.Description != nil { + input.Description = buildkite.Some(*c.Description) } if c.Privacy != "" { input.Privacy = buildkite.Some(c.Privacy) diff --git a/internal/team/view.go b/internal/team/view.go index 39ba74d6..e0786a2b 100644 --- a/internal/team/view.go +++ b/internal/team/view.go @@ -51,6 +51,8 @@ func renderSingleTeamDetail(t buildkite.Team) string { {"Description", output.ValueOrDash(t.Description)}, {"Privacy", output.ValueOrDash(t.Privacy)}, {"Default", fmt.Sprintf("%v", t.Default)}, + {"Default Member Role", output.ValueOrDash(t.DefaultMemberRole)}, + {"Members Can Create Pipelines", fmt.Sprintf("%v", t.MembersCanCreatePipelines)}, {"ID", output.ValueOrDash(t.ID)}, } diff --git a/internal/team/view_test.go b/internal/team/view_test.go index 0fa76a1d..eaa0439d 100644 --- a/internal/team/view_test.go +++ b/internal/team/view_test.go @@ -77,13 +77,15 @@ func TestRenderTeamText(t *testing.T) { createdAt := buildkite.Timestamp{Time: time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC)} team := buildkite.Team{ - ID: "team-uuid-123", - Name: "Fearless Frontenders", - Slug: "fearless-frontenders", - Description: "Frontend engineers", - Privacy: "secret", - Default: true, - CreatedAt: &createdAt, + ID: "team-uuid-123", + Name: "Fearless Frontenders", + Slug: "fearless-frontenders", + Description: "Frontend engineers", + Privacy: "secret", + Default: true, + DefaultMemberRole: "maintainer", + MembersCanCreatePipelines: true, + CreatedAt: &createdAt, CreatedBy: &buildkite.User{ ID: "user-1", Name: "Peter Pettigrew", @@ -99,6 +101,9 @@ func TestRenderTeamText(t *testing.T) { "Frontend engineers", "secret", "true", + "Default Member Role", + "maintainer", + "Members Can Create Pipelines", "team-uuid-123", "Peter Pettigrew", "pp@hogwarts.co.uk",