Improved how CLI "command delete" command works (#9929)

cmd/mattermost/commands/command.go

@@ -49,7 +49,7 @@ var CommandDeleteCmd = &cobra.Command{
 	Short:   "Delete a slash command",
 	Long:    `Delete a slash command. Commands can be specified by command ID.`,
 	Example: `  command delete commandID`,
-	Args:    cobra.MinimumNArgs(1),
+	Args:    cobra.ExactArgs(1),
 	RunE:    deleteCommandCmdF,
 }
 
@@ -141,9 +141,9 @@ func createCommandCmdF(command *cobra.Command, args []string) error {
 	}
 
 	if _, err := a.CreateCommand(newCommand); err != nil {
-		return errors.New("unable to create command '" + newCommand.Trigger + "'. " + err.Error())
+		return errors.New("unable to create command '" + newCommand.DisplayName + "'. " + err.Error())
 	}
-	CommandPrettyPrintln("created command '" + newCommand.Trigger + "'")
+	CommandPrettyPrintln("created command '" + newCommand.DisplayName + "'")
 
 	return nil
 }
@@ -165,16 +165,15 @@ func moveCommandCmdF(command *cobra.Command, args []string) error {
 	}
 
 	commands := getCommandsFromCommandArgs(a, args[1:])
-	CommandPrintErrorln(commands)
 	for i, command := range commands {
 		if command == nil {
 			CommandPrintErrorln("Unable to find command '" + args[i+1] + "'")
 			continue
 		}
 		if err := moveCommand(a, team, command); err != nil {
-			CommandPrintErrorln("Unable to move command '" + command.Trigger + "' error: " + err.Error())
+			CommandPrintErrorln("Unable to move command '" + command.DisplayName + "' error: " + err.Error())
 		} else {
-			CommandPrettyPrintln("Moved command '" + command.Trigger + "'")
+			CommandPrettyPrintln("Moved command '" + command.DisplayName + "'")
 		}
 	}
 
@@ -229,13 +228,15 @@ func deleteCommandCmdF(command *cobra.Command, args []string) error {
 	}
 	defer a.Shutdown()
 
-	commandID := args[0]
+	slashCommand := getCommandFromCommandArg(a, args[0])
-
+	if slashCommand == nil {
-	deleteErr := a.DeleteCommand(commandID)
+		command.SilenceUsage = true
-	if deleteErr != nil {
+		return errors.New("Unable to find command '" + args[0] + "'")
-		CommandPrintErrorln("Unable to delete command '" + commandID + "' error: " + deleteErr.Error())
-		return deleteErr
 	}
-	CommandPrettyPrintln("Deleted command '" + commandID + "'")
+	if err := a.DeleteCommand(slashCommand.Id); err != nil {
+		command.SilenceUsage = true
+		return errors.New("Unable to delete command '" + slashCommand.Id + "' error: " + err.Error())
+	}
+	CommandPrettyPrintln("Deleted command '" + slashCommand.Id + "' (" + slashCommand.DisplayName + ")")
 	return nil
 }

cmd/mattermost/commands/command_test.go

@@ -9,6 +9,8 @@ import (
 	"testing"
 
 	"github.com/mattermost/mattermost-server/api4"
+	"github.com/mattermost/mattermost-server/app"
+	"github.com/mattermost/mattermost-server/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -132,34 +134,38 @@ func TestCreateCommand(t *testing.T) {
 	}
 }
 
-/* Race
 func TestDeleteCommand(t *testing.T) {
-	th := api4.Setup().InitBasic()
+	th := app.Setup().InitBasic()
 	defer th.TearDown()
 	url := "http://localhost:8000/test-command"
 	team := th.BasicTeam
 	user := th.BasicUser
 	th.LinkUserToTeam(user, team)
 
-	// Check the appropriate permissions are enforced.
-	defaultRolePermissions := th.SaveDefaultRolePermissions()
-	defer func() {
-		th.RestoreDefaultRolePermissions(defaultRolePermissions)
-	}()
-	id := model.NewId()
 	c := &model.Command{
-		DisplayName: "dn_" + id,
+		DisplayName: "dn_" + model.NewId(),
 		Method:      "G",
 		TeamId:      team.Id,
 		Username:    user.Username,
+		CreatorId:   user.Id,
 		URL:         url,
-		Trigger:     "test",
+		Trigger:     "trigger_" + model.NewId(),
 	}
-	th.AddPermissionToRole(model.PERMISSION_MANAGE_SLASH_COMMANDS.Id, model.TEAM_USER_ROLE_ID)
+
-	command, _ := th.Client.CreateCommand(c)
+	t.Run("existing command", func(t *testing.T) {
-	commands, _ := th.Client.ListCommands(team.Id, true)
+		command, err := th.App.CreateCommand(c)
-	assert.Equal(t, len(commands), 1)
+		require.Nil(t, err)
-	CheckCommand(t, "command", "delete", command.Id)
+		commands, err := th.App.ListTeamCommands(team.Id)
-	commands, _ = th.Client.ListCommands(team.Id, true)
+		require.Nil(t, err)
-	assert.Equal(t, len(commands), 0)
+		assert.Equal(t, len(commands), 1)
-}*/
+
+		CheckCommand(t, "command", "delete", command.Id)
+		commands, err = th.App.ListTeamCommands(team.Id)
+		require.Nil(t, err)
+		assert.Equal(t, len(commands), 0)
+	})
+
+	t.Run("not existing command", func(t *testing.T) {
+		assert.Error(t, RunCommand(t, "command", "delete", "invalid"))
+	})
+}