-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4/4] DXCDT-317: Add confirmation prompt on open editor updates #603
Conversation
@@ -211,11 +212,20 @@ func updateEmailTemplateCmd(cli *cli) *cobra.Command { | |||
&inputs.Body, | |||
oldTemplate.GetBody(), | |||
inputs.Template+".*.liquid", | |||
cli.emailTemplateEditorHint, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove emailTemplateEditorHint
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only one I can remove safely:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in ec270f6
(#603).
@@ -310,11 +310,20 @@ func updateActionCmd(cli *cli) *cobra.Command { | |||
&inputs.Code, | |||
oldAction.GetCode(), | |||
inputs.Name+".*.js", | |||
cli.actionEditorHint, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove actionEditorHint
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still being used in the create cmd.
@@ -351,12 +351,21 @@ func updateRuleCmd(cli *cli) *cobra.Command { | |||
&inputs.Script, | |||
oldRule.GetScript(), | |||
oldRule.GetName()+".*.js", | |||
cli.ruleEditorHint, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove ruleEditorHint
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still being used in the create cmd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ec270f6
to
2001b96
Compare
🔧 Changes
The CLI asks for confirmation before an entity is deleted. However, we should also ask for confirmation before updating a template. The reason is because sometimes developers have a tough time wielding the built-in editor (ex: VIM) and accidents may occur; a confirmation would prevent any unintended updates. This practice is already employed with the universal login template. This can be overridden with a --force flag or if the templates are piped-in.
📚 References
🔬 Testing
📝 Checklist