fix(service-registry rule): confirm prompt and remove spinners (#1448)

redhat-developer · Feb 21, 2022 · c01d5bd · c01d5bd
1 parent 22c2689
commit c01d5bd
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 84 deletions.
diff --git a/pkg/cmd/registry/rule/describe/describe.go b/pkg/cmd/registry/rule/describe/describe.go
@@ -9,7 +9,6 @@ import (
 	"github.com/redhat-developer/app-services-cli/pkg/core/config"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/dump"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/iostreams"
-	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/spinner"
 	"github.com/redhat-developer/app-services-cli/pkg/core/localize"
 	"github.com/redhat-developer/app-services-cli/pkg/core/logging"
 	"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
@@ -123,18 +122,14 @@ func runDescribe(opts *options) error {
 
 	if opts.artifactID == "" {
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.describe.log.info.fetching.globalRule", localize.NewEntry("Type", opts.ruleType))
-		s.Start()
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.describe.log.info.fetching.globalRule", localize.NewEntry("Type", opts.ruleType)))
 
 		req := dataAPI.AdminApi.GetGlobalRuleConfig(opts.Context, *rulecmdutil.GetMappedRuleType(opts.ruleType))
 
 		rule, httpRes, err = req.Execute()
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	} else {
 
 		if opts.group == registrycmdutil.DefaultArtifactGroup {
@@ -151,9 +146,7 @@ func runDescribe(opts *options) error {
 			return registrycmdutil.TransformInstanceError(err)
 		}
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.describe.log.info.fetching.artifactRule", localize.NewEntry("Type", opts.ruleType))
-		s.Start()
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.describe.log.info.fetching.artifactRule", localize.NewEntry("Type", opts.ruleType)))
 
 		ruleTypeParam := string(*rulecmdutil.GetMappedRuleType(opts.ruleType))
 
@@ -163,8 +156,6 @@ func runDescribe(opts *options) error {
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	}
 
 	if err != nil {

diff --git a/pkg/cmd/registry/rule/disable/disable.go b/pkg/cmd/registry/rule/disable/disable.go
@@ -4,19 +4,19 @@ import (
 	"context"
 	"net/http"
 
+	"github.com/AlecAivazis/survey/v2"
 	"github.com/redhat-developer/app-services-cli/pkg/cmd/registry/registrycmdutil"
 	"github.com/redhat-developer/app-services-cli/pkg/cmd/registry/rule/rulecmdutil"
+	"github.com/redhat-developer/app-services-cli/pkg/core/cmdutil/flagutil"
 	"github.com/redhat-developer/app-services-cli/pkg/core/config"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/icon"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/iostreams"
-	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/spinner"
 	"github.com/redhat-developer/app-services-cli/pkg/core/localize"
 	"github.com/redhat-developer/app-services-cli/pkg/core/logging"
 	"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
 	"github.com/redhat-developer/app-services-cli/pkg/shared/factory"
-	"github.com/spf13/cobra"
-
 	registryinstanceclient "github.com/redhat-developer/app-services-sdk-go/registryinstance/apiv1internal/client"
+	"github.com/spf13/cobra"
 )
 
 type options struct {
@@ -32,6 +32,8 @@ type options struct {
 	registryID string
 	artifactID string
 	group      string
+
+	skipConfirm bool
 }
 
 // NewDisableCommand creates a new command for disabling rule
@@ -54,6 +56,10 @@ func NewDisableCommand(f *factory.Factory) *cobra.Command {
 		Args:    cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, _ []string) (err error) {
 
+			if !opts.IO.CanPrompt() && !opts.skipConfirm {
+				return flagutil.RequiredWhenNonInteractiveError("yes")
+			}
+
 			validator := rulecmdutil.Validator{
 				Localizer: opts.localizer,
 			}
@@ -78,6 +84,8 @@ func NewDisableCommand(f *factory.Factory) *cobra.Command {
 		},
 	}
 
+	cmd.Flags().BoolVarP(&opts.skipConfirm, "yes", "y", false, opts.localizer.MustLocalize("registry.rule.disable.flag.yes"))
+
 	flags := rulecmdutil.NewFlagSet(cmd, f)
 
 	flags.AddRegistryInstance(&opts.registryID)
@@ -99,6 +107,15 @@ func runDisable(opts *options) error {
 		return err
 	}
 
+	if !opts.skipConfirm {
+		prompt := &survey.Confirm{
+			Message: opts.localizer.MustLocalize("registry.rule.disable.confirm"),
+		}
+		if promptErr := survey.AskOne(prompt, &opts.skipConfirm); err != nil {
+			return promptErr
+		}
+	}
+
 	dataAPI, _, err := conn.API().ServiceRegistryInstance(opts.registryID)
 	if err != nil {
 		return err
@@ -161,26 +178,18 @@ func disableGlobalRule(opts *options, dataAPI *registryinstanceclient.APIClient)
 
 	if opts.ruleType != "" {
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.disable.log.info.disabling.globalRule", localize.NewEntry("RuleType", opts.ruleType))
-		s.Start()
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.disable.log.info.disabling.globalRule", localize.NewEntry("RuleType", opts.ruleType)))
 
 		req := dataAPI.AdminApi.DeleteGlobalRule(opts.Context, *rulecmdutil.GetMappedRuleType(opts.ruleType))
 
 		httpRes, err = req.Execute()
-
-		s.Stop()
 	} else {
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.disable.log.info.disabling.globalRules", localize.NewEntry("RuleType", opts.ruleType))
-		s.Start()
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.disable.log.info.disabling.globalRules", localize.NewEntry("RuleType", opts.ruleType)))
 
 		req := dataAPI.AdminApi.DeleteAllGlobalRules(opts.Context)
 
 		httpRes, err = req.Execute()
-
-		s.Stop()
 	}
 
 	return httpRes, err
@@ -189,13 +198,12 @@ func disableGlobalRule(opts *options, dataAPI *registryinstanceclient.APIClient)
 func disableArtifactRule(opts *options, dataAPI *registryinstanceclient.APIClient) (httpRes *http.Response, err error) {
 	if opts.ruleType != "" {
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix(
-			"registry.rule.disable.log.info.disabling.artifactRule",
-			localize.NewEntry("RuleType", opts.ruleType),
-			localize.NewEntry("ArtifactID", opts.artifactID),
+		opts.Logger.Info(
+			opts.localizer.MustLocalize("registry.rule.disable.log.info.disabling.artifactRule",
+				localize.NewEntry("RuleType", opts.ruleType),
+				localize.NewEntry("ArtifactID", opts.artifactID),
+			),
 		)
-		s.Start()
 
 		req := dataAPI.ArtifactRulesApi.DeleteArtifactRule(opts.Context, opts.group, opts.artifactID, string(*rulecmdutil.GetMappedRuleType(opts.ruleType)))
 
@@ -204,21 +212,15 @@ func disableArtifactRule(opts *options, dataAPI *registryinstanceclient.APIClien
 			defer httpRes.Body.Close()
 		}
 
-		s.Stop()
 	} else {
-
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.disable.log.info.disabling.artifactRules", localize.NewEntry("ArtifactID", opts.artifactID))
-		s.Start()
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.disable.log.info.disabling.artifactRules", localize.NewEntry("ArtifactID", opts.artifactID)))
 
 		req := dataAPI.ArtifactRulesApi.DeleteArtifactRules(opts.Context, opts.group, opts.artifactID)
 
 		httpRes, err = req.Execute()
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	}
 
 	return httpRes, err

diff --git a/pkg/cmd/registry/rule/enable/enable.go b/pkg/cmd/registry/rule/enable/enable.go
@@ -12,7 +12,6 @@ import (
 	"github.com/redhat-developer/app-services-cli/pkg/core/config"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/icon"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/iostreams"
-	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/spinner"
 	"github.com/redhat-developer/app-services-cli/pkg/core/localize"
 	"github.com/redhat-developer/app-services-cli/pkg/core/logging"
 	"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
@@ -149,13 +148,12 @@ func runEnable(opts *options) error {
 
 	if opts.artifactID == "" {
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix(
-			"registry.rule.enable.log.info.enabling.globalRules",
-			localize.NewEntry("RuleType", opts.ruleType),
-			localize.NewEntry("Configuration", opts.config),
+		opts.Logger.Info(
+			opts.localizer.MustLocalize("registry.rule.enable.log.info.enabling.globalRules",
+				localize.NewEntry("RuleType", opts.ruleType),
+				localize.NewEntry("Configuration", opts.config),
+			),
 		)
-		s.Start()
 
 		req := dataAPI.AdminApi.CreateGlobalRule(opts.Context)
 
@@ -165,22 +163,19 @@ func runEnable(opts *options) error {
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	} else {
 
 		if opts.group == registrycmdutil.DefaultArtifactGroup {
 			opts.Logger.Info(opts.localizer.MustLocalize("registry.artifact.common.message.no.group", localize.NewEntry("DefaultArtifactGroup", registrycmdutil.DefaultArtifactGroup)))
 		}
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix(
-			"registry.rule.enable.log.info.enabling.artifactRules",
-			localize.NewEntry("RuleType", opts.ruleType),
-			localize.NewEntry("Configuration", opts.config),
-			localize.NewEntry("ArtifactID", opts.artifactID),
+		opts.Logger.Info(
+			opts.localizer.MustLocalize("registry.rule.enable.log.info.enabling.artifactRules",
+				localize.NewEntry("RuleType", opts.ruleType),
+				localize.NewEntry("Configuration", opts.config),
+				localize.NewEntry("ArtifactID", opts.artifactID),
+			),
 		)
-		s.Start()
 
 		req := dataAPI.ArtifactRulesApi.CreateArtifactRule(opts.Context, opts.group, opts.artifactID)
 
@@ -190,8 +185,6 @@ func runEnable(opts *options) error {
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	}
 
 	ruleErrHandler := &rulecmdutil.RuleErrHandler{

diff --git a/pkg/cmd/registry/rule/list/list.go b/pkg/cmd/registry/rule/list/list.go
@@ -10,7 +10,6 @@ import (
 	"github.com/redhat-developer/app-services-cli/pkg/core/config"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/dump"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/iostreams"
-	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/spinner"
 	"github.com/redhat-developer/app-services-cli/pkg/core/localize"
 	"github.com/redhat-developer/app-services-cli/pkg/core/logging"
 	"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
@@ -120,35 +119,28 @@ func runList(opts *options) error {
 	}
 
 	if opts.artifactID == "" {
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.list.log.info.fetching.globalRules")
-		s.Start()
+
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.list.log.info.fetching.globalRules"))
 
 		req := dataAPI.AdminApi.ListGlobalRules(opts.Context)
 
 		enabledRules, httpRes, newErr = req.Execute()
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	} else {
 		if opts.group == registrycmdutil.DefaultArtifactGroup {
 			opts.Logger.Info(opts.localizer.MustLocalize("registry.artifact.common.message.no.group", localize.NewEntry("DefaultArtifactGroup", registrycmdutil.DefaultArtifactGroup)))
 		}
 
-		s := spinner.New(opts.IO.ErrOut, opts.localizer)
-		s.SetLocalizedSuffix("registry.rule.list.log.info.fetching.artifactRules")
-		s.Start()
+		opts.Logger.Info(opts.localizer.MustLocalize("registry.rule.list.log.info.fetching.artifactRules"))
 
 		req := dataAPI.ArtifactRulesApi.ListArtifactRules(opts.Context, opts.group, opts.artifactID)
 
 		enabledRules, httpRes, newErr = req.Execute()
 		if httpRes != nil {
 			defer httpRes.Body.Close()
 		}
-
-		s.Stop()
 	}
 
 	if newErr != nil {

diff --git a/pkg/cmd/registry/rule/update/update.go b/pkg/cmd/registry/rule/update/update.go
@@ -10,7 +10,6 @@ import (
 	"github.com/redhat-developer/app-services-cli/pkg/core/config"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/icon"
 	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/iostreams"
-	"github.com/redhat-developer/app-services-cli/pkg/core/ioutil/spinner"
 	"github.com/redhat-developer/app-services-cli/pkg/core/localize"
 	"github.com/redhat-developer/app-services-cli/pkg/core/logging"
 	"github.com/redhat-developer/app-services-cli/pkg/shared/connection"
@@ -169,13 +168,12 @@ func runUpdate(opts *options) error {
 
 func updateGlobalRule(opts *options, dataAPI *registryinstanceclient.APIClient) (httpRes *http.Response, err error) {
 
-	s := spinner.New(opts.IO.ErrOut, opts.localizer)
-	s.SetLocalizedSuffix(
-		"registry.rule.update.log.info.updating.globalRule",
-		localize.NewEntry("RuleType", opts.ruleType),
-		localize.NewEntry("Configuration", opts.config),
+	opts.Logger.Info(
+		opts.localizer.MustLocalize("registry.rule.update.log.info.updating.globalRule",
+			localize.NewEntry("RuleType", opts.ruleType),
+			localize.NewEntry("Configuration", opts.config),
+		),
 	)
-	s.Start()
 
 	req := dataAPI.AdminApi.UpdateGlobalRuleConfig(opts.Context, *rulecmdutil.GetMappedRuleType(opts.ruleType))
 
@@ -188,21 +186,18 @@ func updateGlobalRule(opts *options, dataAPI *registryinstanceclient.APIClient)
 
 	_, httpRes, err = req.Execute()
 
-	s.Stop()
-
 	return httpRes, err
 }
 
 func updateArtifactRule(opts *options, dataAPI *registryinstanceclient.APIClient) (httpRes *http.Response, err error) {
 
-	s := spinner.New(opts.IO.ErrOut, opts.localizer)
-	s.SetLocalizedSuffix(
-		"registry.rule.update.log.info.updating.artifactRule",
-		localize.NewEntry("RuleType", opts.ruleType),
-		localize.NewEntry("Configuration", opts.config),
-		localize.NewEntry("ArtifactID", opts.artifactID),
+	opts.Logger.Info(
+		opts.localizer.MustLocalize("registry.rule.update.log.info.updating.artifactRule",
+			localize.NewEntry("RuleType", opts.ruleType),
+			localize.NewEntry("Configuration", opts.config),
+			localize.NewEntry("ArtifactID", opts.artifactID),
+		),
 	)
-	s.Start()
 
 	req := dataAPI.ArtifactRulesApi.UpdateArtifactRuleConfig(opts.Context, opts.group, opts.artifactID, string(*rulecmdutil.GetMappedRuleType(opts.ruleType)))
 
@@ -218,7 +213,5 @@ func updateArtifactRule(opts *options, dataAPI *registryinstanceclient.APIClient
 		defer httpRes.Body.Close()
 	}
 
-	s.Stop()
-
 	return httpRes, err
 }
diff --git a/pkg/core/localize/locales/en/cmd/rule.en.toml b/pkg/core/localize/locales/en/cmd/rule.en.toml
@@ -181,6 +181,12 @@ one='Disabling "{{.RuleType}}" rule for artifact with ID "{{.ArtifactID}}"'
 [registry.rule.disable.log.info.disabling.artifactRules]
 one='Disabling all rules for artifact with ID "{{.ArtifactID}}"'
 
+[registry.rule.disable.flag.yes]
+one = 'Disable rule(s) without prompt'
+
+[registry.rule.disable.confirm]
+one='Do you want to disable specified rule(s)?'
+
 [registry.rule.disable.log.info.success]
 one='Successfully disabled'