From 5b51ea3241ac0efbc6b87055375e6389e633db50 Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Mon, 4 Jul 2022 16:58:08 -0500 Subject: [PATCH 1/8] prompt for confirmation before deleting templates (#2830) --- cli/templatedelete.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cli/templatedelete.go b/cli/templatedelete.go index e698335c672a6..485e0d00f0a65 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "strings" "github.com/spf13/cobra" "golang.org/x/xerrors" @@ -70,6 +71,16 @@ func templateDelete() *cobra.Command { templates = append(templates, template) } + // Confirm deletion of the template. + _, err = cliui.Prompt(cmd, cliui.PromptOptions{ + Text: fmt.Sprintf("Delete these templates: %s?", cliui.Styles.Code.Render(strings.Join(templateNames, ", "))), + IsConfirm: true, + Default: "no", + }) + if err != nil { + return err + } + for _, template := range templates { err := client.DeleteTemplate(ctx, template.ID) if err != nil { From 5802c6b43fffe41c374ecc8e43c20b684310f345 Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Mon, 4 Jul 2022 17:01:47 -0500 Subject: [PATCH 2/8] populate templateNames from the interactive picker too --- cli/templatedelete.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/cli/templatedelete.go b/cli/templatedelete.go index 485e0d00f0a65..cc28c187a8ff9 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -33,6 +33,16 @@ func templateDelete() *cobra.Command { if len(args) > 0 { templateNames = args + + for _, templateName := range templateNames { + template, err := client.TemplateByName(ctx, organization.ID, templateName) + if err != nil { + return xerrors.Errorf("get template by name: %w", err) + } + + templates = append(templates, template) + } + } else { allTemplates, err := client.TemplatesByOrganization(ctx, organization.ID) if err != nil { @@ -58,19 +68,11 @@ func templateDelete() *cobra.Command { for _, template := range allTemplates { if template.Name == selection { templates = append(templates, template) + templateNames = append(templateNames, template.Name) } } } - for _, templateName := range templateNames { - template, err := client.TemplateByName(ctx, organization.ID, templateName) - if err != nil { - return xerrors.Errorf("get template by name: %w", err) - } - - templates = append(templates, template) - } - // Confirm deletion of the template. _, err = cliui.Prompt(cmd, cliui.PromptOptions{ Text: fmt.Sprintf("Delete these templates: %s?", cliui.Styles.Code.Render(strings.Join(templateNames, ", "))), From 0886f330ac045a7ae9da848377c72cf3f2e81337 Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Fri, 8 Jul 2022 14:23:32 -0500 Subject: [PATCH 3/8] allow skipping delete confirmation prompt with --yes flag --- cli/templatedelete.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cli/templatedelete.go b/cli/templatedelete.go index cc28c187a8ff9..ceeabe6987e19 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -12,7 +12,7 @@ import ( ) func templateDelete() *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Use: "delete [name...]", Short: "Delete templates", RunE: func(cmd *cobra.Command, args []string) error { @@ -95,4 +95,7 @@ func templateDelete() *cobra.Command { return nil }, } + + cliui.AllowSkipPrompt(cmd) + return cmd } From f10c3b87964ff90604be1e3e4899b07c54ba769b Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Fri, 8 Jul 2022 15:08:30 -0500 Subject: [PATCH 4/8] eliminate unnecessary newline --- cli/templatedelete.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/templatedelete.go b/cli/templatedelete.go index ceeabe6987e19..980a33c72fe1e 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -39,7 +39,6 @@ func templateDelete() *cobra.Command { if err != nil { return xerrors.Errorf("get template by name: %w", err) } - templates = append(templates, template) } From bdeec4bb422ba4049de023610bb56b5dd5a9f3b2 Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Sun, 10 Jul 2022 15:21:15 -0500 Subject: [PATCH 5/8] test both confirmation of delete and `--yes` with no confirmation --- cli/templatedelete_test.go | 46 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/cli/templatedelete_test.go b/cli/templatedelete_test.go index 92fb0b2932fa0..714cff6933ec6 100644 --- a/cli/templatedelete_test.go +++ b/cli/templatedelete_test.go @@ -2,11 +2,14 @@ package cli_test import ( "context" + "fmt" + "strings" "testing" "github.com/stretchr/testify/require" "github.com/coder/coder/cli/clitest" + "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/codersdk" "github.com/coder/coder/pty/ptytest" @@ -32,7 +35,7 @@ func TestTemplateDelete(t *testing.T) { require.Error(t, err, "template should not exist") }) - t.Run("Multiple", func(t *testing.T) { + t.Run("Multiple --yes", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) @@ -49,7 +52,7 @@ func TestTemplateDelete(t *testing.T) { templateNames = append(templateNames, template.Name) } - cmd, root := clitest.New(t, append([]string{"templates", "delete"}, templateNames...)...) + cmd, root := clitest.New(t, append([]string{"templates", "delete", "--yes"}, templateNames...)...) clitest.SetupConfig(t, client, root) require.NoError(t, cmd.Execute()) @@ -59,6 +62,45 @@ func TestTemplateDelete(t *testing.T) { } }) + t.Run("Multiple prompted", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + templates := []codersdk.Template{ + coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID), + coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID), + coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID), + } + templateNames := []string{} + for _, template := range templates { + templateNames = append(templateNames, template.Name) + } + + cmd, root := clitest.New(t, append([]string{"templates", "delete"}, templateNames...)...) + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + execDone := make(chan error) + go func() { + execDone <- cmd.Execute() + }() + + pty.ExpectMatch(fmt.Sprintf("Delete these templates: %s?", cliui.Styles.Code.Render(strings.Join(templateNames, ", ")))) + pty.WriteLine("yes") + + for _, template := range templates { + _, err := client.Template(context.Background(), template.ID) + require.Error(t, err, "template should not exist") + } + + require.NoError(t, <-execDone) + }) + t.Run("Selector", func(t *testing.T) { t.Parallel() From 74e65f2fcf605558eebb241cc06ffc898a93e53c Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Mon, 11 Jul 2022 11:59:07 -0500 Subject: [PATCH 6/8] fix failing test that needed --yes --- cli/templatecreate_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 3f1904e5fd6c9..c034b62eeaff3 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -229,6 +229,7 @@ func TestTemplateCreate(t *testing.T) { "templates", "delete", "my-template", + "--yes", } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) From df6b7cce714c9d568fbf16e472554dce2a64acbc Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Mon, 11 Jul 2022 11:59:21 -0500 Subject: [PATCH 7/8] remove unnecessary empty line the linter disliked --- cli/templatedelete.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/templatedelete.go b/cli/templatedelete.go index 980a33c72fe1e..bb64ba540e6f7 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -41,7 +41,6 @@ func templateDelete() *cobra.Command { } templates = append(templates, template) } - } else { allTemplates, err := client.TemplatesByOrganization(ctx, organization.ID) if err != nil { From 8b1966b289cfed1bdbdf6aa2f601ea3d809022c2 Mon Sep 17 00:00:00 2001 From: ketang <33678+ketang@users.noreply.github.com> Date: Mon, 11 Jul 2022 12:47:38 -0500 Subject: [PATCH 8/8] make the tests correct --- cli/templatedelete_test.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/cli/templatedelete_test.go b/cli/templatedelete_test.go index 714cff6933ec6..14364d673f012 100644 --- a/cli/templatedelete_test.go +++ b/cli/templatedelete_test.go @@ -28,8 +28,21 @@ func TestTemplateDelete(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) cmd, root := clitest.New(t, "templates", "delete", template.Name) + clitest.SetupConfig(t, client, root) - require.NoError(t, cmd.Execute()) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + execDone := make(chan error) + go func() { + execDone <- cmd.Execute() + }() + + pty.ExpectMatch(fmt.Sprintf("Delete these templates: %s?", cliui.Styles.Code.Render(template.Name))) + pty.WriteLine("yes") + + require.NoError(t, <-execDone) _, err := client.Template(context.Background(), template.ID) require.Error(t, err, "template should not exist") @@ -93,12 +106,12 @@ func TestTemplateDelete(t *testing.T) { pty.ExpectMatch(fmt.Sprintf("Delete these templates: %s?", cliui.Styles.Code.Render(strings.Join(templateNames, ", ")))) pty.WriteLine("yes") + require.NoError(t, <-execDone) + for _, template := range templates { _, err := client.Template(context.Background(), template.ID) require.Error(t, err, "template should not exist") } - - require.NoError(t, <-execDone) }) t.Run("Selector", func(t *testing.T) { @@ -122,7 +135,7 @@ func TestTemplateDelete(t *testing.T) { execDone <- cmd.Execute() }() - pty.WriteLine("docker-local") + pty.WriteLine("yes") require.NoError(t, <-execDone) _, err := client.Template(context.Background(), template.ID)