Migrate existing confirmation dialogs to use confirm charm. (#1265)

digitalocean · Oct 7, 2022 · c9b8dec · c9b8dec
1 parent 492fd7c
commit c9b8dec
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 201 deletions.
diff --git a/commands/confirmation.go b/commands/confirmation.go
@@ -1,43 +1,27 @@
 package commands
 
 import (
-	"bufio"
 	"fmt"
-	"io"
-	"os"
-	"strings"
-)
-
-// retrieveUserInput is a function that can retrieve user input in form of string. By default,
-// it will prompt the user. In test, you can replace this with code that returns the appropriate response.
-var retrieveUserInput = func(message string) (string, error) {
-	return readUserInput(os.Stdin, message)
-}
-
-// readUserInput is similar to retrieveUserInput but takes an explicit
-// io.Reader to read user input from. It is meant to allow simplified testing
-// as to-be-read inputs can be injected conveniently.
-func readUserInput(in io.Reader, message string) (string, error) {
-	reader := bufio.NewReader(in)
-	warnConfirm("Are you sure you want to " + message + " (y/N) ? ")
-	answer, err := reader.ReadString('\n')
-	if err != nil {
-		return "", err
-	}
 
-	answer = strings.TrimRight(answer, "\r\n")
-
-	return strings.ToLower(answer), nil
-}
+	"github.com/digitalocean/doctl/commands/charm/confirm"
+	"github.com/digitalocean/doctl/commands/charm/template"
+)
 
 // AskForConfirm parses and verifies user input for confirmation.
 func AskForConfirm(message string) error {
-	answer, err := retrieveUserInput(message)
+	if !Interactive {
+		warn("Requires confirmation. Use the `--force` flag to continue without confirmation.")
+		return ErrExitSilently
+	}
+	choice, err := confirm.New(
+		template.String("Are you sure you want to {{.}}", message),
+		confirm.WithDefaultChoice(confirm.No),
+	).Prompt()
 	if err != nil {
-		return fmt.Errorf("Unable to parse users input: %s", err)
+		return err
 	}
 
-	if answer != "y" && answer != "ye" && answer != "yes" {
+	if choice != confirm.Yes {
 		return fmt.Errorf("Invalid user input")
 	}
 

diff --git a/commands/confirmation_test.go b/commands/confirmation_test.go
diff --git a/commands/namespaces_test.go b/commands/namespaces_test.go
@@ -244,11 +244,6 @@ func TestNamespacesDelete(t *testing.T) {
 						APIHost:   "https://sgp1.example.com",
 					},
 				}}
-				// Confirmation dialog can't be fully mocked but it can be replaced
-				// (as in confirmation_test.go).
-				retrieveUserInput = func(string) (string, error) {
-					return "no", nil
-				}
 
 				ctx := context.TODO()
 				tm.serverless.EXPECT().ListNamespaces(ctx).Return(listForMatching, nil)

diff --git a/integration/domain_records_delete_test.go b/integration/domain_records_delete_test.go
@@ -114,7 +114,7 @@ var _ = suite("compute/domain/records/delete", func(t *testing.T, when spec.G, i
 	})
 
 	when("deleting one domain record without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -128,12 +128,12 @@ var _ = suite("compute/domain/records/delete", func(t *testing.T, when spec.G, i
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(domainRecDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two domain records without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -148,12 +148,7 @@ var _ = suite("compute/domain/records/delete", func(t *testing.T, when spec.G, i
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiDomainRecDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	domainRecDelOutput      = "Warning: Are you sure you want to delete this domain record? (y/N) ? Error: Operation aborted."
-	multiDomainRecDelOutput = "Warning: Are you sure you want to delete 2 domain records? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/droplet_delete_test.go b/integration/droplet_delete_test.go
@@ -136,7 +136,7 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 	})
 
 	when("deleting one Droplet without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -148,30 +148,12 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(dropletDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
-	when("deleting two Droplet without force flag", func() {
-		it("correctly prompts for confirmation", func() {
-			cmd := exec.Command(builtBinaryPath,
-				"-t", "some-magic-token",
-				"-u", server.URL,
-				"compute",
-				"droplet",
-				"delete",
-				"some-droplet-name",
-				"another-droplet-name",
-			)
-
-			output, err := cmd.CombinedOutput()
-			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiDropletDelOutput), strings.TrimSpace(string(output)))
-		})
-	})
-
-	when("deleting one Droplet by tag without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+	when("deleting Droplet by tag without force flag", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -183,12 +165,12 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(tagDropletDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two Droplet by tag without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -200,14 +182,7 @@ var _ = suite("compute/droplet/delete", func(t *testing.T, when spec.G, it spec.
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(tagMultiDropletDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	dropletDelOutput         = "Warning: Are you sure you want to delete this Droplet? (y/N) ? Error: Operation aborted."
-	multiDropletDelOutput    = "Warning: Are you sure you want to delete 2 Droplets? (y/N) ? Error: Operation aborted."
-	tagDropletDelOutput      = `Warning: Are you sure you want to delete 1 Droplet tagged "one"? [affected Droplet: 1337] (y/N) ? Error: Operation aborted.`
-	tagMultiDropletDelOutput = `Warning: Are you sure you want to delete 2 Droplets tagged "two"? [affected Droplets: 1337 7331] (y/N) ? Error: Operation aborted.`
-)
diff --git a/integration/firewall_delete_test.go b/integration/firewall_delete_test.go
@@ -98,7 +98,7 @@ var _ = suite("compute/firewall/delete", func(t *testing.T, when spec.G, it spec
 	})
 
 	when("deleting one firewall without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -110,12 +110,12 @@ var _ = suite("compute/firewall/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(fwDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two firewalls without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -128,12 +128,7 @@ var _ = suite("compute/firewall/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiFwDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	fwDelOutput      = "Warning: Are you sure you want to delete this firewall? (y/N) ? Error: Operation aborted."
-	multiFwDelOutput = "Warning: Are you sure you want to delete 2 firewalls? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/image_delete_test.go b/integration/image_delete_test.go
@@ -98,7 +98,7 @@ var _ = suite("compute/image/delete", func(t *testing.T, when spec.G, it spec.S)
 	})
 
 	when("deleting one image without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -110,12 +110,12 @@ var _ = suite("compute/image/delete", func(t *testing.T, when spec.G, it spec.S)
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(imageDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two images without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -128,12 +128,7 @@ var _ = suite("compute/image/delete", func(t *testing.T, when spec.G, it spec.S)
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiImageDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	imageDelOutput      = "Warning: Are you sure you want to delete this image? (y/N) ? Error: Operation aborted."
-	multiImageDelOutput = "Warning: Are you sure you want to delete 2 images? (y/N) ? Error: Operation aborted."
-)
diff --git a/integration/init_test.go b/integration/init_test.go
@@ -21,6 +21,10 @@ var (
 	builtBinaryPath string
 )
 
+const (
+	confirmNonInteractiveOutput = "Warning: Requires confirmation. Use the `--force` flag to continue without confirmation.\nError: Operation aborted."
+)
+
 func TestRun(t *testing.T) {
 	suite.Run(t)
 }

diff --git a/integration/snapshot_delete_test.go b/integration/snapshot_delete_test.go
@@ -110,7 +110,7 @@ var _ = suite("compute/snapshot/delete", func(t *testing.T, when spec.G, it spec
 	})
 
 	when("deleting one snapshot without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -122,12 +122,12 @@ var _ = suite("compute/snapshot/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(snapshotDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 
 	when("deleting two snapshots without force flag", func() {
-		it("correctly prompts for confirmation", func() {
+		it("errors without confirmation", func() {
 			cmd := exec.Command(builtBinaryPath,
 				"-t", "some-magic-token",
 				"-u", server.URL,
@@ -140,12 +140,7 @@ var _ = suite("compute/snapshot/delete", func(t *testing.T, when spec.G, it spec
 
 			output, err := cmd.CombinedOutput()
 			expect.Error(err)
-			expect.Equal(strings.TrimSpace(multiSnapshotDelOutput), strings.TrimSpace(string(output)))
+			expect.Equal(strings.TrimSpace(confirmNonInteractiveOutput), strings.TrimSpace(string(output)))
 		})
 	})
 })
-
-const (
-	snapshotDelOutput      = "Warning: Are you sure you want to delete this snapshot? (y/N) ? Error: Operation aborted."
-	multiSnapshotDelOutput = "Warning: Are you sure you want to delete 2 snapshots? (y/N) ? Error: Operation aborted."
-)