From 4b36dc031fcffa9ca99f1f4fe5e93718b2dd8905 Mon Sep 17 00:00:00 2001 From: Greggory Rothmeier Date: Tue, 21 Jun 2022 06:57:40 -0700 Subject: [PATCH] Add flag to list codespaces under an organization (#5807) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- internal/codespaces/api/api.go | 21 +++++- internal/codespaces/api/api_test.go | 4 +- pkg/cmd/codespace/common.go | 4 +- pkg/cmd/codespace/delete.go | 2 +- pkg/cmd/codespace/delete_test.go | 2 +- pkg/cmd/codespace/list.go | 28 ++++++-- pkg/cmd/codespace/list_test.go | 103 ++++++++++++++++++++++++++++ pkg/cmd/codespace/mock_api.go | 36 ++++++---- pkg/cmd/codespace/ssh.go | 2 +- pkg/cmd/codespace/stop.go | 2 +- 10 files changed, 174 insertions(+), 30 deletions(-) create mode 100644 pkg/cmd/codespace/list_test.go diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 8eec820a8..6e1b54697 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -270,13 +270,28 @@ func (c *Codespace) ExportData(fields []string) map[string]interface{} { // ListCodespaces returns a list of codespaces for the user. Pass a negative limit to request all pages from // the API until all codespaces have been fetched. -func (a *API) ListCodespaces(ctx context.Context, limit int) (codespaces []*Codespace, err error) { +func (a *API) ListCodespaces(ctx context.Context, limit int, orgName string, userName string) (codespaces []*Codespace, err error) { perPage := 100 if limit > 0 && limit < 100 { perPage = limit } - listURL := fmt.Sprintf("%s/user/codespaces?per_page=%d", a.githubAPI, perPage) + var listURL string + var spanName string + + if orgName != "" { + if userName != "" { + listURL = fmt.Sprintf("%s/orgs/%s/members/%s/codespaces?per_page=%d", a.githubAPI, orgName, userName, perPage) + spanName = "/orgs/*/members/*/codespaces" + } else { + listURL = fmt.Sprintf("%s/orgs/%s/codespaces?per_page=%d", a.githubAPI, orgName, perPage) + spanName = "/orgs/*/codespaces" + } + } else { + listURL = fmt.Sprintf("%s/user/codespaces?per_page=%d", a.githubAPI, perPage) + spanName = "/user/codespaces" + } + for { req, err := http.NewRequest(http.MethodGet, listURL, nil) if err != nil { @@ -284,7 +299,7 @@ func (a *API) ListCodespaces(ctx context.Context, limit int) (codespaces []*Code } a.setHeaders(req) - resp, err := a.do(ctx, req, "/user/codespaces") + resp, err := a.do(ctx, req, spanName) if err != nil { return nil, fmt.Errorf("error making request: %w", err) } diff --git a/internal/codespaces/api/api_test.go b/internal/codespaces/api/api_test.go index 2d1d05b63..c48073795 100644 --- a/internal/codespaces/api/api_test.go +++ b/internal/codespaces/api/api_test.go @@ -140,7 +140,7 @@ func TestListCodespaces_limited(t *testing.T) { client: &http.Client{}, } ctx := context.TODO() - codespaces, err := api.ListCodespaces(ctx, 200) + codespaces, err := api.ListCodespaces(ctx, 200, "", "") if err != nil { t.Fatal(err) } @@ -165,7 +165,7 @@ func TestListCodespaces_unlimited(t *testing.T) { client: &http.Client{}, } ctx := context.TODO() - codespaces, err := api.ListCodespaces(ctx, -1) + codespaces, err := api.ListCodespaces(ctx, -1, "", "") if err != nil { t.Fatal(err) } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index 16b3f7ce6..42ea18f4e 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -107,7 +107,7 @@ func startLiveShareSession(ctx context.Context, codespace *api.Codespace, a *App type apiClient interface { GetUser(ctx context.Context) (*api.User, error) GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) - ListCodespaces(ctx context.Context, limit int) ([]*api.Codespace, error) + ListCodespaces(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) DeleteCodespace(ctx context.Context, name string) error StartCodespace(ctx context.Context, name string) error StopCodespace(ctx context.Context, name string) error @@ -125,7 +125,7 @@ type apiClient interface { var errNoCodespaces = errors.New("you have no codespaces") func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, error) { - codespaces, err := apiClient.ListCodespaces(ctx, -1) + codespaces, err := apiClient.ListCodespaces(ctx, -1, "", "") if err != nil { return nil, fmt.Errorf("error getting codespaces: %w", err) } diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index 941475f24..7b628973e 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -63,7 +63,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { nameFilter := opts.codespaceName if nameFilter == "" { a.StartProgressIndicatorWithLabel("Fetching codespaces") - codespaces, err = a.apiClient.ListCodespaces(ctx, -1) + codespaces, err = a.apiClient.ListCodespaces(ctx, -1, "", "") a.StopProgressIndicator() if err != nil { return fmt.Errorf("error getting codespaces: %w", err) diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index 69b3efb7a..a0a8718e7 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -167,7 +167,7 @@ func TestDelete(t *testing.T) { }, } if tt.opts.codespaceName == "" { - apiMock.ListCodespacesFunc = func(_ context.Context, num int) ([]*api.Codespace, error) { + apiMock.ListCodespacesFunc = func(_ context.Context, num int, orgName string, userName string) ([]*api.Codespace, error) { return tt.codespaces, nil } } else { diff --git a/pkg/cmd/codespace/list.go b/pkg/cmd/codespace/list.go index 9bb1a2103..08d20e8ca 100644 --- a/pkg/cmd/codespace/list.go +++ b/pkg/cmd/codespace/list.go @@ -11,8 +11,14 @@ import ( "github.com/spf13/cobra" ) +type listOptions struct { + limit int + orgName string + userName string +} + func newListCmd(app *App) *cobra.Command { - var limit int + opts := &listOptions{} var exporter cmdutil.Exporter listCmd := &cobra.Command{ @@ -21,23 +27,25 @@ func newListCmd(app *App) *cobra.Command { Aliases: []string{"ls"}, Args: noArgsConstraint, RunE: func(cmd *cobra.Command, args []string) error { - if limit < 1 { - return cmdutil.FlagErrorf("invalid limit: %v", limit) + if opts.limit < 1 { + return cmdutil.FlagErrorf("invalid limit: %v", opts.limit) } - return app.List(cmd.Context(), limit, exporter) + return app.List(cmd.Context(), opts, exporter) }, } - listCmd.Flags().IntVarP(&limit, "limit", "L", 30, "Maximum number of codespaces to list") + listCmd.Flags().IntVarP(&opts.limit, "limit", "L", 30, "Maximum number of codespaces to list") + listCmd.Flags().StringVarP(&opts.orgName, "org", "o", "", "List codespaces for an organization") + listCmd.Flags().StringVarP(&opts.userName, "user", "u", "", "Used with --org to filter to a specific user") cmdutil.AddJSONFlags(listCmd, &exporter, api.CodespaceFields) return listCmd } -func (a *App) List(ctx context.Context, limit int, exporter cmdutil.Exporter) error { +func (a *App) List(ctx context.Context, opts *listOptions, exporter cmdutil.Exporter) error { a.StartProgressIndicatorWithLabel("Fetching codespaces") - codespaces, err := a.apiClient.ListCodespaces(ctx, limit) + codespaces, err := a.apiClient.ListCodespaces(ctx, opts.limit, opts.orgName, opts.userName) a.StopProgressIndicator() if err != nil { return fmt.Errorf("error getting codespaces: %w", err) @@ -68,6 +76,9 @@ func (a *App) List(ctx context.Context, limit int, exporter cmdutil.Exporter) er if tp.IsTTY() { tp.AddField("NAME", nil, nil) tp.AddField("DISPLAY NAME", nil, nil) + if opts.orgName != "" { + tp.AddField("OWNER", nil, nil) + } tp.AddField("REPOSITORY", nil, nil) tp.AddField("BRANCH", nil, nil) tp.AddField("STATE", nil, nil) @@ -104,6 +115,9 @@ func (a *App) List(ctx context.Context, limit int, exporter cmdutil.Exporter) er tp.AddField(formattedName, nil, nameColor) tp.AddField(c.DisplayName, nil, nil) + if opts.orgName != "" { + tp.AddField(c.Owner.Login, nil, nil) + } tp.AddField(c.Repository.FullName, nil, nil) tp.AddField(c.branchWithGitStatus(), nil, cs.Cyan) if c.PendingOperation { diff --git a/pkg/cmd/codespace/list_test.go b/pkg/cmd/codespace/list_test.go new file mode 100644 index 000000000..001806b43 --- /dev/null +++ b/pkg/cmd/codespace/list_test.go @@ -0,0 +1,103 @@ +package codespace + +import ( + "context" + "fmt" + "testing" + + "github.com/cli/cli/v2/internal/codespaces/api" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" +) + +func TestApp_List(t *testing.T) { + type fields struct { + apiClient apiClient + } + tests := []struct { + name string + fields fields + opts *listOptions + }{ + { + name: "list codespaces, no flags", + fields: fields{ + apiClient: &apiClientMock{ + ListCodespacesFunc: func(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) { + if orgName != "" { + return nil, fmt.Errorf("should not be called with an orgName") + } + return []*api.Codespace{ + { + DisplayName: "CS1", + }, + }, nil + }, + }, + }, + opts: &listOptions{}, + }, + { + name: "list codespaces, --org flag", + fields: fields{ + apiClient: &apiClientMock{ + ListCodespacesFunc: func(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) { + if orgName != "TestOrg" { + return nil, fmt.Errorf("Expected orgName to be TestOrg. Got %s", orgName) + } + if userName != "" { + return nil, fmt.Errorf("Expected userName to be blank. Got %s", userName) + } + return []*api.Codespace{ + { + DisplayName: "CS1", + }, + }, nil + }, + }, + }, + opts: &listOptions{ + orgName: "TestOrg", + }, + }, + { + name: "list codespaces, --org and --user flag", + fields: fields{ + apiClient: &apiClientMock{ + ListCodespacesFunc: func(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) { + if orgName != "TestOrg" { + return nil, fmt.Errorf("Expected orgName to be TestOrg. Got %s", orgName) + } + if userName != "jimmy" { + return nil, fmt.Errorf("Expected userName to be jimmy. Got %s", userName) + } + return []*api.Codespace{ + { + DisplayName: "CS1", + }, + }, nil + }, + }, + }, + opts: &listOptions{ + orgName: "TestOrg", + userName: "jimmy", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, _, _ := iostreams.Test() + a := &App{ + io: ios, + apiClient: tt.fields.apiClient, + } + var exporter cmdutil.Exporter + + err := a.List(context.Background(), tt.opts, exporter) + if err != nil { + t.Error(err) + } + }) + } +} diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 3999692f2..7cf715848 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -49,7 +49,7 @@ import ( // GetUserFunc: func(ctx context.Context) (*api.User, error) { // panic("mock out the GetUser method") // }, -// ListCodespacesFunc: func(ctx context.Context, limit int) ([]*api.Codespace, error) { +// ListCodespacesFunc: func(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) { // panic("mock out the ListCodespaces method") // }, // ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { @@ -102,7 +102,7 @@ type apiClientMock struct { GetUserFunc func(ctx context.Context) (*api.User, error) // ListCodespacesFunc mocks the ListCodespaces method. - ListCodespacesFunc func(ctx context.Context, limit int) ([]*api.Codespace, error) + ListCodespacesFunc func(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) // ListDevContainersFunc mocks the ListDevContainers method. ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) @@ -208,6 +208,10 @@ type apiClientMock struct { Ctx context.Context // Limit is the limit argument value. Limit int + // OrgName is the orgName argument value. + OrgName string + // UserName is the userName argument value. + UserName string } // ListDevContainers holds details about calls to the ListDevContainers method. ListDevContainers []struct { @@ -658,33 +662,41 @@ func (mock *apiClientMock) GetUserCalls() []struct { } // ListCodespaces calls ListCodespacesFunc. -func (mock *apiClientMock) ListCodespaces(ctx context.Context, limit int) ([]*api.Codespace, error) { +func (mock *apiClientMock) ListCodespaces(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) { if mock.ListCodespacesFunc == nil { panic("apiClientMock.ListCodespacesFunc: method is nil but apiClient.ListCodespaces was just called") } callInfo := struct { - Ctx context.Context - Limit int + Ctx context.Context + Limit int + OrgName string + UserName string }{ - Ctx: ctx, - Limit: limit, + Ctx: ctx, + Limit: limit, + OrgName: orgName, + UserName: userName, } mock.lockListCodespaces.Lock() mock.calls.ListCodespaces = append(mock.calls.ListCodespaces, callInfo) mock.lockListCodespaces.Unlock() - return mock.ListCodespacesFunc(ctx, limit) + return mock.ListCodespacesFunc(ctx, limit, orgName, userName) } // ListCodespacesCalls gets all the calls that were made to ListCodespaces. // Check the length with: // len(mockedapiClient.ListCodespacesCalls()) func (mock *apiClientMock) ListCodespacesCalls() []struct { - Ctx context.Context - Limit int + Ctx context.Context + Limit int + OrgName string + UserName string } { var calls []struct { - Ctx context.Context - Limit int + Ctx context.Context + Limit int + OrgName string + UserName string } mock.lockListCodespaces.RLock() calls = mock.calls.ListCodespaces diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 2c3f1c93f..ac1f7ecfe 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -235,7 +235,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro var csList []*api.Codespace if opts.codespace == "" { a.StartProgressIndicatorWithLabel("Fetching codespaces") - csList, err = a.apiClient.ListCodespaces(ctx, -1) + csList, err = a.apiClient.ListCodespaces(ctx, -1, "", "") a.StopProgressIndicator() } else { var codespace *api.Codespace diff --git a/pkg/cmd/codespace/stop.go b/pkg/cmd/codespace/stop.go index 6b2268fad..2f97e082c 100644 --- a/pkg/cmd/codespace/stop.go +++ b/pkg/cmd/codespace/stop.go @@ -28,7 +28,7 @@ func newStopCmd(app *App) *cobra.Command { func (a *App) StopCodespace(ctx context.Context, codespaceName string) error { if codespaceName == "" { a.StartProgressIndicatorWithLabel("Fetching codespaces") - codespaces, err := a.apiClient.ListCodespaces(ctx, -1) + codespaces, err := a.apiClient.ListCodespaces(ctx, -1, "", "") a.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to list codespaces: %w", err)