Mm 16292 support command overrides (#14951)

* Supporing plugins that override built-in commands.

* Test.

* Fix test.

* Fix override order for command autocomplete.

* Fix test copy paste errors

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

app/command.go

@@ -77,16 +77,6 @@ func (a *App) CreateCommandPost(post *model.Post, teamId string, response *model
 func (a *App) ListAutocompleteCommands(teamId string, T goi18n.TranslateFunc) ([]*model.Command, *model.AppError) {
 	commands := make([]*model.Command, 0, 32)
 	seen := make(map[string]bool)
-	for _, value := range commandProviders {
-		if cmd := value.GetCommand(a, T); cmd != nil {
-			cpy := *cmd
-			if cpy.AutoComplete && !seen[cpy.Id] {
-				cpy.Sanitize()
-				seen[cpy.Trigger] = true
-				commands = append(commands, &cpy)
-			}
-		}
-	}
 
 	for _, cmd := range a.PluginCommandsForTeam(teamId) {
 		if cmd.AutoComplete && !seen[cmd.Trigger] {
@@ -102,7 +92,7 @@ func (a *App) ListAutocompleteCommands(teamId string, T goi18n.TranslateFunc) ([
 		}
 
 		for _, cmd := range teamCmds {
-			if cmd.AutoComplete && !seen[cmd.Id] {
+			if cmd.AutoComplete && !seen[cmd.Trigger] {
 				cmd.Sanitize()
 				seen[cmd.Trigger] = true
 				commands = append(commands, cmd)
@@ -110,6 +100,17 @@ func (a *App) ListAutocompleteCommands(teamId string, T goi18n.TranslateFunc) ([
 		}
 	}
 
+	for _, value := range commandProviders {
+		if cmd := value.GetCommand(a, T); cmd != nil {
+			cpy := *cmd
+			if cpy.AutoComplete && !seen[cpy.Trigger] {
+				cpy.Sanitize()
+				seen[cpy.Trigger] = true
+				commands = append(commands, &cpy)
+			}
+		}
+	}
+
 	return commands, nil
 }
 
@@ -183,12 +184,8 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, *
 
 	args.TriggerId = triggerId
 
-	cmd, response := a.tryExecuteBuiltInCommand(args, trigger, message)
+	// Plugins can override built in and custom commands
-	if cmd != nil && response != nil {
+	cmd, response, appErr := a.tryExecutePluginCommand(args)
-		return a.HandleCommandResponse(cmd, args, response, true)
-	}
-
-	cmd, response, appErr = a.tryExecutePluginCommand(args)
 	if appErr != nil {
 		return nil, appErr
 	} else if cmd != nil && response != nil {
@@ -196,6 +193,7 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, *
 		return a.HandleCommandResponse(cmd, args, response, true)
 	}
 
+	// Custom commands can override built ins
 	cmd, response, appErr = a.tryExecuteCustomCommand(args, trigger, message)
 	if appErr != nil {
 		return nil, appErr
@@ -204,6 +202,11 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, *
 		return a.HandleCommandResponse(cmd, args, response, false)
 	}
 
+	cmd, response = a.tryExecuteBuiltInCommand(args, trigger, message)
+	if cmd != nil && response != nil {
+		return a.HandleCommandResponse(cmd, args, response, true)
+	}
+
 	return nil, model.NewAppError("command", "api.command.execute_command.not_found.app_error", map[string]interface{}{"Trigger": trigger}, "", http.StatusNotFound)
 }
 

app/command_test.go

@@ -90,10 +90,15 @@ func TestExecuteCommand(t *testing.T) {
 
 		for TestCase, result := range TestCases {
 			args := &model.CommandArgs{
-				Command: TestCase,
+				Command:   TestCase,
-				T:       func(s string, args ...interface{}) string { return s },
+				TeamId:    th.BasicTeam.Id,
+				ChannelId: th.BasicChannel.Id,
+				UserId:    th.BasicUser.Id,
+				T:         func(s string, args ...interface{}) string { return s },
 			}
-			resp, _ := th.App.ExecuteCommand(args)
+			resp, err := th.App.ExecuteCommand(args)
+			require.Nil(t, err)
+			require.NotNil(t, resp)
 
 			assert.Equal(t, resp.Text, result)
 		}

app/plugin_commands_test.go

@@ -53,7 +53,6 @@ func TestPluginCommand(t *testing.T) {
 			}
 
 			func (p *MyPlugin) OnConfigurationChange() error {
-				p.API.LogError("hello")
 				if err := p.API.LoadPluginConfiguration(&p.configuration); err != nil {
 					return err
 				}
@@ -62,7 +61,6 @@ func TestPluginCommand(t *testing.T) {
 			}
 
 			func (p *MyPlugin) OnActivate() error {
-				p.API.LogError("team", "team", p.configuration.TeamId)
 				err := p.API.RegisterCommand(&model.Command{
 					TeamId: p.configuration.TeamId,
 					Trigger: "plugin",
@@ -73,7 +71,6 @@ func TestPluginCommand(t *testing.T) {
 				if err != nil {
 					p.API.LogError("error", "err", err)
 				}
-				p.API.LogDebug("team", "team", p.configuration.TeamId)
 
 				return err
 			}
@@ -217,4 +214,76 @@ func TestPluginCommand(t *testing.T) {
 		_, err := th.App.ExecuteCommand(args)
 		require.NotNil(t, err)
 	})
+
+	t.Run("plugins can override built-in commands", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			cfg.PluginSettings.Plugins["testloadpluginconfig"] = map[string]interface{}{
+				"TeamId": args.TeamId,
+			}
+		})
+
+		tearDown, pluginIds, activationErrors := SetAppEnvironmentWithPlugins(t, []string{`
+			package main
+
+			import (
+				"github.com/mattermost/mattermost-server/v5/plugin"
+				"github.com/mattermost/mattermost-server/v5/model"
+			)
+
+			type configuration struct {
+				TeamId string
+			}
+
+			type MyPlugin struct {
+				plugin.MattermostPlugin
+
+				configuration configuration
+			}
+
+			func (p *MyPlugin) OnConfigurationChange() error {
+				if err := p.API.LoadPluginConfiguration(&p.configuration); err != nil {
+					return err
+				}
+
+				return nil
+			}
+
+			func (p *MyPlugin) OnActivate() error {
+				err := p.API.RegisterCommand(&model.Command{
+					TeamId: p.configuration.TeamId,
+					Trigger: "code",
+					DisplayName: "Plugin Command",
+					AutoComplete: true,
+					AutoCompleteDesc: "autocomplete",
+				})
+				if err != nil {
+					p.API.LogError("error", "err", err)
+				}
+
+				return err
+			}
+
+			func (p *MyPlugin) ExecuteCommand(c *plugin.Context, args *model.CommandArgs) (*model.CommandResponse, *model.AppError) {
+				return &model.CommandResponse{
+					ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL,
+					Text: "text",
+				}, nil
+			}
+
+			func main() {
+				plugin.ClientMain(&MyPlugin{})
+			}
+		`}, th.App, th.App.NewPluginAPI)
+		defer tearDown()
+		require.Len(t, activationErrors, 1)
+		require.Nil(t, nil, activationErrors[0])
+
+		args.Command = "/code"
+		resp, err := th.App.ExecuteCommand(args)
+		require.Nil(t, err)
+		require.Equal(t, model.COMMAND_RESPONSE_TYPE_EPHEMERAL, resp.ResponseType)
+		require.Equal(t, "text", resp.Text)
+
+		th.App.RemovePlugin(pluginIds[0])
+	})
 }