From 47f5abdb6f118401c0804546b980a0a7a467679f Mon Sep 17 00:00:00 2001 From: Ashwin Jeyaseelan <8gitbrix@github.com> Date: Wed, 22 Jun 2022 09:28:09 -0700 Subject: [PATCH] 8gitbrix/add delete org codespace (#5827) 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 | 17 ++++++-- pkg/cmd/codespace/common.go | 6 +-- pkg/cmd/codespace/delete.go | 27 +++++++++++-- pkg/cmd/codespace/delete_test.go | 68 ++++++++++++++++++++++++++++++-- pkg/cmd/codespace/mock_api.go | 36 +++++++++++------ 5 files changed, 129 insertions(+), 25 deletions(-) diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index f1614fcc0..63644ea1f 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -827,14 +827,25 @@ func (a *API) startCreate(ctx context.Context, params *CreateCodespaceParams) (* } // DeleteCodespace deletes the given codespace. -func (a *API) DeleteCodespace(ctx context.Context, codespaceName string) error { - req, err := http.NewRequest(http.MethodDelete, a.githubAPI+"/user/codespaces/"+codespaceName, nil) +func (a *API) DeleteCodespace(ctx context.Context, codespaceName string, orgName string, userName string) error { + var deleteURL string + var spanName string + + if orgName != "" && userName != "" { + deleteURL = fmt.Sprintf("%s/orgs/%s/members/%s/codespaces/%s", a.githubAPI, orgName, userName, codespaceName) + spanName = "/orgs/*/members/*/codespaces/*" + } else { + deleteURL = a.githubAPI + "/user/codespaces/" + codespaceName + spanName = "/user/codespaces/*" + } + + req, err := http.NewRequest(http.MethodDelete, deleteURL, nil) if err != nil { return fmt.Errorf("error creating request: %w", err) } a.setHeaders(req) - resp, err := a.do(ctx, req, "/user/codespaces/*") + resp, err := a.do(ctx, req, spanName) if err != nil { return fmt.Errorf("error making request: %w", err) } diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index e0664775e..816191238 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -109,7 +109,7 @@ type apiClient interface { GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) GetOrgMemberCodespace(ctx context.Context, orgName string, userName string, codespaceName string) (*api.Codespace, error) ListCodespaces(ctx context.Context, limit int, orgName string, userName string) ([]*api.Codespace, error) - DeleteCodespace(ctx context.Context, name string) error + DeleteCodespace(ctx context.Context, name string, orgName string, userName string) error StartCodespace(ctx context.Context, name string) error StopCodespace(ctx context.Context, name string, orgName string, userName string) error CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) @@ -133,8 +133,8 @@ func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, return chooseCodespaceFromList(ctx, codespaces, false) } -// chooseCodespaceFromList returns the selected codespace from the list, -// or an error if there are no codespaces. +// chooseCodespaceFromList returns the codespace that the user has interactively selected from the list, or +// an error if there are no codespaces. func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, includeOwner bool) (*api.Codespace, error) { if len(codespaces) == 0 { return nil, errNoCodespaces diff --git a/pkg/cmd/codespace/delete.go b/pkg/cmd/codespace/delete.go index ca4e37a7a..6443da9d0 100644 --- a/pkg/cmd/codespace/delete.go +++ b/pkg/cmd/codespace/delete.go @@ -19,6 +19,8 @@ type deleteOptions struct { codespaceName string repoFilter string keepDays uint16 + orgName string + userName string isInteractive bool now func() time.Time @@ -41,6 +43,12 @@ func newDeleteCmd(app *App) *cobra.Command { Use: "delete", Short: "Delete a codespace", Args: noArgsConstraint, + PreRunE: func(c *cobra.Command, args []string) error { + if opts.orgName != "" && opts.codespaceName != "" && opts.userName == "" { + return errors.New("`--org` with `--codespace` requires `--username`") + } + return nil + }, RunE: func(cmd *cobra.Command, args []string) error { if opts.deleteAll && opts.repoFilter != "" { return errors.New("both --all and --repo is not supported") @@ -54,6 +62,8 @@ func newDeleteCmd(app *App) *cobra.Command { deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "r", "", "Delete codespaces for a `repository`") deleteCmd.Flags().BoolVarP(&opts.skipConfirm, "force", "f", false, "Skip confirmation for codespaces that contain unsaved changes") deleteCmd.Flags().Uint16Var(&opts.keepDays, "days", 0, "Delete codespaces older than `N` days") + deleteCmd.Flags().StringVarP(&opts.orgName, "org", "o", "", "Select organization to delete codespace from (admin-only)") + deleteCmd.Flags().StringVarP(&opts.userName, "username", "u", "", "Used with --org to filter to a specific user") return deleteCmd } @@ -63,14 +73,15 @@ 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, opts.orgName, opts.userName) a.StopProgressIndicator() if err != nil { return fmt.Errorf("error getting codespaces: %w", err) } if !opts.deleteAll && opts.repoFilter == "" { - c, err := chooseCodespaceFromList(ctx, codespaces, false) + includeUsername := opts.orgName != "" + c, err := chooseCodespaceFromList(ctx, codespaces, includeUsername) if err != nil { return fmt.Errorf("error choosing codespace: %w", err) } @@ -78,7 +89,15 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { } } else { a.StartProgressIndicatorWithLabel("Fetching codespace") - codespace, err := a.apiClient.GetCodespace(ctx, nameFilter, false) + + var codespace *api.Codespace + var err error + + if opts.orgName == "" || opts.userName == "" { + codespace, err = a.apiClient.GetCodespace(ctx, nameFilter, false) + } else { + codespace, err = a.apiClient.GetOrgMemberCodespace(ctx, opts.orgName, opts.userName, opts.codespaceName) + } a.StopProgressIndicator() if err != nil { return fmt.Errorf("error fetching codespace information: %w", err) @@ -132,7 +151,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) { for _, c := range codespacesToDelete { codespaceName := c.Name g.Go(func() error { - if err := a.apiClient.DeleteCodespace(ctx, codespaceName); err != nil { + if err := a.apiClient.DeleteCodespace(ctx, codespaceName, opts.orgName, opts.userName); err != nil { a.errLogger.Printf("error deleting codespace %q: %v\n", codespaceName, err) return err } diff --git a/pkg/cmd/codespace/delete_test.go b/pkg/cmd/codespace/delete_test.go index a0a8718e7..b2700903b 100644 --- a/pkg/cmd/codespace/delete_test.go +++ b/pkg/cmd/codespace/delete_test.go @@ -152,6 +152,57 @@ func TestDelete(t *testing.T) { wantDeleted: []string{"hubot-robawt-abc", "monalisa-spoonknife-c4f3"}, wantStdout: "", }, + { + name: "deletion for org codespace by admin succeeds", + opts: deleteOptions{ + deleteAll: true, + orgName: "bookish", + userName: "monalisa", + codespaceName: "monalisa-spoonknife-123", + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + Owner: api.User{Login: "monalisa"}, + }, + { + Name: "monalisa-spoonknife-123", + Owner: api.User{Login: "monalisa2"}, + }, + { + Name: "dont-delete-abc", + Owner: api.User{Login: "monalisa"}, + }, + }, + wantDeleted: []string{"monalisa-spoonknife-123"}, + wantStdout: "", + }, + { + name: "deletion for org codespace by admin fails for codespace not found", + opts: deleteOptions{ + deleteAll: true, + orgName: "bookish", + userName: "johnDoe", + codespaceName: "monalisa-spoonknife-123", + }, + codespaces: []*api.Codespace{ + { + Name: "monalisa-spoonknife-123", + Owner: api.User{Login: "monalisa"}, + }, + { + Name: "monalisa-spoonknife-123", + Owner: api.User{Login: "monalisa2"}, + }, + { + Name: "dont-delete-abc", + Owner: api.User{Login: "monalisa"}, + }, + }, + wantDeleted: []string{}, + wantStdout: "", + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -159,7 +210,7 @@ func TestDelete(t *testing.T) { GetUserFunc: func(_ context.Context) (*api.User, error) { return user, nil }, - DeleteCodespaceFunc: func(_ context.Context, name string) error { + DeleteCodespaceFunc: func(_ context.Context, name string, orgName string, userName string) error { if tt.deleteErr != nil { return tt.deleteErr } @@ -171,8 +222,19 @@ func TestDelete(t *testing.T) { return tt.codespaces, nil } } else { - apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*api.Codespace, error) { - return tt.codespaces[0], nil + if tt.opts.orgName != "" { + apiMock.GetOrgMemberCodespaceFunc = func(_ context.Context, orgName string, userName string, name string) (*api.Codespace, error) { + for _, codespace := range tt.codespaces { + if codespace.Name == name && codespace.Owner.Login == userName { + return codespace, nil + } + } + return nil, fmt.Errorf("codespace not found for user %s with name %s", userName, name) + } + } else { + apiMock.GetCodespaceFunc = func(_ context.Context, name string, includeConnection bool) (*api.Codespace, error) { + return tt.codespaces[0], nil + } } } opts := tt.opts diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index 0159e7665..8d5c29d4e 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -22,7 +22,7 @@ import ( // CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { // panic("mock out the CreateCodespace method") // }, -// DeleteCodespaceFunc: func(ctx context.Context, name string) error { +// DeleteCodespaceFunc: func(ctx context.Context, name string, orgName string, userName string) error { // panic("mock out the DeleteCodespace method") // }, // EditCodespaceFunc: func(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) { @@ -78,7 +78,7 @@ type apiClientMock struct { CreateCodespaceFunc func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) // DeleteCodespaceFunc mocks the DeleteCodespace method. - DeleteCodespaceFunc func(ctx context.Context, name string) error + DeleteCodespaceFunc func(ctx context.Context, name string, orgName string, userName string) error // EditCodespaceFunc mocks the EditCodespace method. EditCodespaceFunc func(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) @@ -141,6 +141,10 @@ type apiClientMock struct { Ctx context.Context // Name is the name argument value. Name string + // OrgName is the orgName argument value. + OrgName string + // UserName is the userName argument value. + UserName string } // EditCodespace holds details about calls to the EditCodespace method. EditCodespace []struct { @@ -349,33 +353,41 @@ func (mock *apiClientMock) CreateCodespaceCalls() []struct { } // DeleteCodespace calls DeleteCodespaceFunc. -func (mock *apiClientMock) DeleteCodespace(ctx context.Context, name string) error { +func (mock *apiClientMock) DeleteCodespace(ctx context.Context, name string, orgName string, userName string) error { if mock.DeleteCodespaceFunc == nil { panic("apiClientMock.DeleteCodespaceFunc: method is nil but apiClient.DeleteCodespace was just called") } callInfo := struct { - Ctx context.Context - Name string + Ctx context.Context + Name string + OrgName string + UserName string }{ - Ctx: ctx, - Name: name, + Ctx: ctx, + Name: name, + OrgName: orgName, + UserName: userName, } mock.lockDeleteCodespace.Lock() mock.calls.DeleteCodespace = append(mock.calls.DeleteCodespace, callInfo) mock.lockDeleteCodespace.Unlock() - return mock.DeleteCodespaceFunc(ctx, name) + return mock.DeleteCodespaceFunc(ctx, name, orgName, userName) } // DeleteCodespaceCalls gets all the calls that were made to DeleteCodespace. // Check the length with: // len(mockedapiClient.DeleteCodespaceCalls()) func (mock *apiClientMock) DeleteCodespaceCalls() []struct { - Ctx context.Context - Name string + Ctx context.Context + Name string + OrgName string + UserName string } { var calls []struct { - Ctx context.Context - Name string + Ctx context.Context + Name string + OrgName string + UserName string } mock.lockDeleteCodespace.RLock() calls = mock.calls.DeleteCodespace