From 8bf0cb8f13bafff98b6be64db208542ac4d65eb2 Mon Sep 17 00:00:00 2001
From: Charlie Andrews <cwndrws@github.com>
Date: Wed, 16 Mar 2022 08:51:33 -0400
Subject: [PATCH] Refactor the getOrChooseCodespace to always check for pending
 ops

---
 pkg/cmd/codespace/code.go      |  7 -------
 pkg/cmd/codespace/code_test.go |  2 +-
 pkg/cmd/codespace/common.go    |  7 +++++++
 pkg/cmd/codespace/logs.go      |  7 -------
 pkg/cmd/codespace/logs_test.go |  2 +-
 pkg/cmd/codespace/ports.go     | 26 +++-----------------------
 pkg/cmd/codespace/ssh.go       |  7 -------
 pkg/cmd/codespace/ssh_test.go  |  2 +-
 8 files changed, 13 insertions(+), 47 deletions(-)

diff --git a/pkg/cmd/codespace/code.go b/pkg/cmd/codespace/code.go
index 7b52db29f..ba2455385 100644
--- a/pkg/cmd/codespace/code.go
+++ b/pkg/cmd/codespace/code.go
@@ -36,13 +36,6 @@ func (a *App) VSCode(ctx context.Context, codespaceName string, useInsiders bool
 		return fmt.Errorf("get or choose codespace: %w", err)
 	}
 
-	if codespace.PendingOperation {
-		return fmt.Errorf(
-			"codespace is disabled while it has a pending operation: %s",
-			codespace.PendingOperationDisabledReason,
-		)
-	}
-
 	url := vscodeProtocolURL(codespace.Name, useInsiders)
 	if err := a.browser.Browse(url); err != nil {
 		return fmt.Errorf("error opening Visual Studio Code: %w", err)
diff --git a/pkg/cmd/codespace/code_test.go b/pkg/cmd/codespace/code_test.go
index e35060c73..0eb3b7ade 100644
--- a/pkg/cmd/codespace/code_test.go
+++ b/pkg/cmd/codespace/code_test.go
@@ -58,7 +58,7 @@ func TestPendingOperationDisallowsCode(t *testing.T) {
 	app := testingLogsApp()
 
 	if err := app.VSCode(context.Background(), "disabledCodespace", false); err != nil {
-		if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
+		if err.Error() != "get or choose codespace: codespace is disabled while it has a pending operation: Some pending operation" {
 			t.Errorf("expected pending operation error, but got: %v", err)
 		}
 	} else {
diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go
index 6061eac78..aec386f6c 100644
--- a/pkg/cmd/codespace/common.go
+++ b/pkg/cmd/codespace/common.go
@@ -183,6 +183,13 @@ func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceNam
 		}
 	}
 
+	if codespace.PendingOperation {
+		return nil, fmt.Errorf(
+			"codespace is disabled while it has a pending operation: %s",
+			codespace.PendingOperationDisabledReason,
+		)
+	}
+
 	return codespace, nil
 }
 
diff --git a/pkg/cmd/codespace/logs.go b/pkg/cmd/codespace/logs.go
index 221d95215..d0a0c233b 100644
--- a/pkg/cmd/codespace/logs.go
+++ b/pkg/cmd/codespace/logs.go
@@ -41,13 +41,6 @@ func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err
 		return fmt.Errorf("get or choose codespace: %w", err)
 	}
 
-	if codespace.PendingOperation {
-		return fmt.Errorf(
-			"codespace is disabled while it has a pending operation: %s",
-			codespace.PendingOperationDisabledReason,
-		)
-	}
-
 	authkeys := make(chan error, 1)
 	go func() {
 		authkeys <- checkAuthorizedKeys(ctx, a.apiClient)
diff --git a/pkg/cmd/codespace/logs_test.go b/pkg/cmd/codespace/logs_test.go
index 1eaf2e9d9..225e10680 100644
--- a/pkg/cmd/codespace/logs_test.go
+++ b/pkg/cmd/codespace/logs_test.go
@@ -12,7 +12,7 @@ func TestPendingOperationDisallowsLogs(t *testing.T) {
 	app := testingLogsApp()
 
 	if err := app.Logs(context.Background(), "disabledCodespace", false); err != nil {
-		if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
+		if err.Error() != "get or choose codespace: codespace is disabled while it has a pending operation: Some pending operation" {
 			t.Errorf("expected pending operation error, but got: %v", err)
 		}
 	} else {
diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go
index cc4365742..274be8d7b 100644
--- a/pkg/cmd/codespace/ports.go
+++ b/pkg/cmd/codespace/ports.go
@@ -46,7 +46,7 @@ func newPortsCmd(app *App) *cobra.Command {
 
 // ListPorts lists known ports in a codespace.
 func (a *App) ListPorts(ctx context.Context, codespaceName string, exporter cmdutil.Exporter) (err error) {
-	codespace, err := getCodespaceForPorts(ctx, a.apiClient, codespaceName)
+	codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
 	if err != nil {
 		return err
 	}
@@ -231,7 +231,7 @@ func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName string, ar
 		return fmt.Errorf("error parsing port arguments: %w", err)
 	}
 
-	codespace, err := getCodespaceForPorts(ctx, a.apiClient, codespaceName)
+	codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
 	if err != nil {
 		return err
 	}
@@ -304,7 +304,7 @@ func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []st
 		return fmt.Errorf("get port pairs: %w", err)
 	}
 
-	codespace, err := getCodespaceForPorts(ctx, a.apiClient, codespaceName)
+	codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
 	if err != nil {
 		return err
 	}
@@ -370,23 +370,3 @@ func normalizeJSON(j []byte) []byte {
 	// remove trailing commas
 	return bytes.ReplaceAll(j, []byte("},}"), []byte("}}"))
 }
-
-func getCodespaceForPorts(ctx context.Context, apiClient apiClient, codespaceName string) (*api.Codespace, error) {
-	codespace, err := getOrChooseCodespace(ctx, apiClient, codespaceName)
-	if err != nil {
-		// TODO(josebalius): remove special handling of this error here and it other places
-		if err == errNoCodespaces {
-			return nil, err
-		}
-		return nil, fmt.Errorf("error choosing codespace: %w", err)
-	}
-
-	if codespace.PendingOperation {
-		return nil, fmt.Errorf(
-			"codespace is disabled while it has a pending operation: %s",
-			codespace.PendingOperationDisabledReason,
-		)
-	}
-
-	return codespace, nil
-}
diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go
index af6c86614..6ce783f2f 100644
--- a/pkg/cmd/codespace/ssh.go
+++ b/pkg/cmd/codespace/ssh.go
@@ -128,13 +128,6 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
 		return fmt.Errorf("get or choose codespace: %w", err)
 	}
 
-	if codespace.PendingOperation {
-		return fmt.Errorf(
-			"codespace is disabled while it has a pending operation: %s",
-			codespace.PendingOperationDisabledReason,
-		)
-	}
-
 	liveshareLogger := noopLogger()
 	if opts.debug {
 		debugLogger, err := newFileLogger(opts.debugFile)
diff --git a/pkg/cmd/codespace/ssh_test.go b/pkg/cmd/codespace/ssh_test.go
index 3b242ae4b..fac5147bb 100644
--- a/pkg/cmd/codespace/ssh_test.go
+++ b/pkg/cmd/codespace/ssh_test.go
@@ -12,7 +12,7 @@ func TestPendingOperationDisallowsSSH(t *testing.T) {
 	app := testingSSHApp()
 
 	if err := app.SSH(context.Background(), []string{}, sshOptions{codespace: "disabledCodespace"}); err != nil {
-		if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
+		if err.Error() != "get or choose codespace: codespace is disabled while it has a pending operation: Some pending operation" {
 			t.Errorf("expected pending operation error, but got: %v", err)
 		}
 	} else {