APIv4 PUT /commands/{command_id} (mattermost#5999)

* APIv4 PUT /commands/{command_id}

* update client parameter and api4 test

api4/command.go

@@ -19,6 +19,8 @@ func InitCommand() {
 	BaseRoutes.Commands.Handle("", ApiSessionRequired(createCommand)).Methods("POST")
 	BaseRoutes.Commands.Handle("", ApiSessionRequired(listCommands)).Methods("GET")
 
+	BaseRoutes.Command.Handle("", ApiSessionRequired(updateCommand)).Methods("PUT")
+
 	BaseRoutes.Team.Handle("/commands/autocomplete", ApiSessionRequired(listAutocompleteCommands)).Methods("GET")
 }
 
@@ -49,6 +51,54 @@ func createCommand(c *Context, w http.ResponseWriter, r *http.Request) {
 	w.Write([]byte(rcmd.ToJson()))
 }
 
+func updateCommand(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireCommandId()
+	if c.Err != nil {
+		return
+	}
+
+	cmd := model.CommandFromJson(r.Body)
+	if cmd == nil || cmd.Id != c.Params.CommandId {
+		c.SetInvalidParam("command")
+		return
+	}
+
+	c.LogAudit("attempt")
+
+	oldCmd, err := app.GetCommand(c.Params.CommandId)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	if cmd.TeamId != oldCmd.TeamId {
+		c.Err = model.NewAppError("updateCommand", "api.command.team_mismatch.app_error", nil, "user_id="+c.Session.UserId, http.StatusBadRequest)
+		return
+	}
+
+	if !app.SessionHasPermissionToTeam(c.Session, oldCmd.TeamId, model.PERMISSION_MANAGE_SLASH_COMMANDS) {
+		c.LogAudit("fail - inappropriate permissions")
+		c.SetPermissionError(model.PERMISSION_MANAGE_SLASH_COMMANDS)
+		return
+	}
+
+	if c.Session.UserId != oldCmd.CreatorId && !app.SessionHasPermissionToTeam(c.Session, oldCmd.TeamId, model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS) {
+		c.LogAudit("fail - inappropriate permissions")
+		c.SetPermissionError(model.PERMISSION_MANAGE_OTHERS_SLASH_COMMANDS)
+		return
+	}
+
+	rcmd, err := app.UpdateCommand(oldCmd, cmd)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	c.LogAudit("success")
+
+	w.Write([]byte(rcmd.ToJson()))
+}
+
 func listCommands(c *Context, w http.ResponseWriter, r *http.Request) {
 	customOnly, failConv := strconv.ParseBool(r.URL.Query().Get("custom_only"))
 	if failConv != nil {

api4/command_test.go

@@ -5,8 +5,8 @@ package api4
 
 import (
 	"testing"
-	// "time"
 
+	"github.com/mattermost/platform/app"
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/utils"
 )
@@ -60,6 +60,92 @@ func TestCreateCommand(t *testing.T) {
 	CheckErrorMessage(t, resp, "api.command.disabled.app_error")
 }
 
+func TestUpdateCommand(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer TearDown()
+	Client := th.SystemAdminClient
+	user := th.SystemAdminUser
+	team := th.BasicTeam
+
+	enableCommands := *utils.Cfg.ServiceSettings.EnableCommands
+	defer func() {
+		utils.Cfg.ServiceSettings.EnableCommands = &enableCommands
+	}()
+	*utils.Cfg.ServiceSettings.EnableCommands = true
+
+	cmd1 := &model.Command{
+		CreatorId: user.Id,
+		TeamId:    team.Id,
+		URL:       "http://nowhere.com",
+		Method:    model.COMMAND_METHOD_POST,
+		Trigger:   "trigger1",
+	}
+
+	cmd1, _ = app.CreateCommand(cmd1)
+
+	cmd2 := &model.Command{
+		CreatorId: GenerateTestId(),
+		TeamId:    team.Id,
+		URL:       "http://nowhere.com/change",
+		Method:    model.COMMAND_METHOD_GET,
+		Trigger:   "trigger2",
+		Id:        cmd1.Id,
+		Token:     "tokenchange",
+	}
+
+	rcmd, resp := Client.UpdateCommand(cmd2)
+	CheckNoError(t, resp)
+
+	if rcmd.Trigger != cmd2.Trigger {
+		t.Fatal("Trigger should have updated")
+	}
+
+	if rcmd.Method != cmd2.Method {
+		t.Fatal("Method should have updated")
+	}
+
+	if rcmd.URL != cmd2.URL {
+		t.Fatal("URL should have updated")
+	}
+
+	if rcmd.CreatorId != cmd1.CreatorId {
+		t.Fatal("CreatorId should have not updated")
+	}
+
+	if rcmd.Token != cmd1.Token {
+		t.Fatal("Token should have not updated")
+	}
+
+	cmd2.Id = GenerateTestId()
+
+	rcmd, resp = Client.UpdateCommand(cmd2)
+	CheckNotFoundStatus(t, resp)
+
+	if rcmd != nil {
+		t.Fatal("should be empty")
+	}
+
+	cmd2.Id = "junk"
+
+	_, resp = Client.UpdateCommand(cmd2)
+	CheckBadRequestStatus(t, resp)
+
+	cmd2.Id = cmd1.Id
+	cmd2.TeamId = GenerateTestId()
+
+	_, resp = Client.UpdateCommand(cmd2)
+	CheckBadRequestStatus(t, resp)
+
+	cmd2.TeamId = team.Id
+
+	_, resp = th.Client.UpdateCommand(cmd2)
+	CheckForbiddenStatus(t, resp)
+
+	Client.Logout()
+	_, resp = Client.UpdateCommand(cmd2)
+	CheckUnauthorizedStatus(t, resp)
+}
+
 func TestListCommands(t *testing.T) {
 	th := Setup().InitBasic().InitSystemAdmin()
 	defer TearDown()

api4/context.go

@@ -477,3 +477,14 @@ func (c *Context) RequireHookId() *Context {
 
 	return c
 }
+
+func (c *Context) RequireCommandId() *Context {
+	if c.Err != nil {
+		return c
+	}
+
+	if len(c.Params.CommandId) != 26 {
+		c.SetInvalidUrlParam("command_id")
+	}
+	return c
+}

app/command.go

@@ -312,6 +312,7 @@ func GetCommand(commandId string) (*model.Command, *model.AppError) {
 	}
 
 	if result := <-Srv.Store.Command().Get(commandId); result.Err != nil {
+		result.Err.StatusCode = http.StatusNotFound
 		return nil, result.Err
 	} else {
 		return result.Data.(*model.Command), nil

model/client4.go

@@ -226,6 +226,10 @@ func (c *Client4) GetCommandsRoute() string {
 	return fmt.Sprintf("/commands")
 }
 
+func (c *Client4) GetCommandRoute(commandId string) string {
+	return fmt.Sprintf(c.GetCommandsRoute()+"/%v", commandId)
+}
+
 func (c *Client4) GetEmojisRoute() string {
 	return fmt.Sprintf("/emoji")
 }
@@ -2188,6 +2192,16 @@ func (c *Client4) CreateCommand(cmd *Command) (*Command, *Response) {
 	}
 }
 
+// UpdateCommand updates a command based on the provided Command struct
+func (c *Client4) UpdateCommand(cmd *Command) (*Command, *Response) {
+	if r, err := c.DoApiPut(c.GetCommandRoute(cmd.Id), cmd.ToJson()); err != nil {
+		return nil, &Response{StatusCode: r.StatusCode, Error: err}
+	} else {
+		defer closeBody(r)
+		return CommandFromJson(r.Body), BuildResponse(r)
+	}
+}
+
 // ListCommands will retrieve a list of commands available in the team.
 func (c *Client4) ListCommands(teamId string, customOnly bool) ([]*Command, *Response) {
 	query := fmt.Sprintf("?team_id=%v&custom_only=%v", teamId, customOnly)