APIv4 DELETE /users/{user_id}/posts/{post_id}/reactions/name (#6117)

* APIv4 DELETE /users/{user_id}/posts/{post_id}/reactions/name

* updated v3 deleteReaction endpoint

* update parameter of app.DeleteReactionForPost()

* update utils.IsValidAlphaNum, add utils.IsValidAlphaNumHyphenUnderscore, and add related tests

api/reaction.go

@@ -106,28 +106,13 @@ func deleteReaction(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var post *model.Post
-
-	if result := <-app.Srv.Store.Post().Get(reaction.PostId); result.Err != nil {
-		c.Err = result.Err
-		return
-	} else if post = result.Data.(*model.PostList).Posts[postId]; post.ChannelId != channelId {
-		c.Err = model.NewLocAppError("deleteReaction", "api.reaction.delete_reaction.mismatched_channel_id.app_error",
-			nil, "channelId="+channelId+", post.ChannelId="+post.ChannelId+", postId="+postId)
-		c.Err.StatusCode = http.StatusBadRequest
+	err := app.DeleteReactionForPost(reaction)
+	if err != nil {
+		c.Err = err
 		return
 	}
 
-	if result := <-app.Srv.Store.Reaction().Delete(reaction); result.Err != nil {
-		c.Err = result.Err
-		return
-	} else {
-		go sendReactionEvent(model.WEBSOCKET_EVENT_REACTION_REMOVED, channelId, reaction, post)
-
-		app.InvalidateCacheForReactions(reaction.PostId)
-
-		ReturnStatusOK(w)
-	}
+	ReturnStatusOK(w)
 }
 
 func sendReactionEvent(event string, channelId string, reaction *model.Reaction, post *model.Post) {

api4/api.go

@@ -48,6 +48,7 @@ type Routes struct {
 	Post            *mux.Router // 'api/v4/posts/{post_id:[A-Za-z0-9]+}'
 	PostsForChannel *mux.Router // 'api/v4/channels/{channel_id:[A-Za-z0-9]+}/posts'
 	PostsForUser    *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts'
+	PostForUser     *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts/{post_id:[A-Za-z0-9]+}'
 
 	Files *mux.Router // 'api/v4/files'
 	File  *mux.Router // 'api/v4/files/{file_id:[A-Za-z0-9]+}'
@@ -89,6 +90,8 @@ type Routes struct {
 	Emojis *mux.Router // 'api/v4/emoji'
 	Emoji  *mux.Router // 'api/v4/emoji/{emoji_id:[A-Za-z0-9]+}'
 
+	ReactionByNameForPostForUser *mux.Router // 'api/v4/users/{user_id:[A-Za-z0-9]+}/posts/{post_id:[A-Za-z0-9]+}/reactions/{emoji_name:[A-Za-z0-9_-]+}'
+
 	Webrtc *mux.Router // 'api/v4/webrtc'
 }
 
@@ -132,6 +135,7 @@ func InitApi(full bool) {
 	BaseRoutes.Post = BaseRoutes.Posts.PathPrefix("/{post_id:[A-Za-z0-9]+}").Subrouter()
 	BaseRoutes.PostsForChannel = BaseRoutes.Channel.PathPrefix("/posts").Subrouter()
 	BaseRoutes.PostsForUser = BaseRoutes.User.PathPrefix("/posts").Subrouter()
+	BaseRoutes.PostForUser = BaseRoutes.PostsForUser.PathPrefix("/{post_id:[A-Za-z0-9]+}").Subrouter()
 
 	BaseRoutes.Files = BaseRoutes.ApiRoot.PathPrefix("/files").Subrouter()
 	BaseRoutes.File = BaseRoutes.Files.PathPrefix("/{file_id:[A-Za-z0-9]+}").Subrouter()
@@ -166,6 +170,8 @@ func InitApi(full bool) {
 	BaseRoutes.Emojis = BaseRoutes.ApiRoot.PathPrefix("/emoji").Subrouter()
 	BaseRoutes.Emoji = BaseRoutes.Emojis.PathPrefix("/{emoji_id:[A-Za-z0-9]+}").Subrouter()
 
+	BaseRoutes.ReactionByNameForPostForUser = BaseRoutes.PostForUser.PathPrefix("/reactions/{emoji_name:[A-Za-z0-9_-]+}").Subrouter()
+
 	BaseRoutes.Webrtc = BaseRoutes.ApiRoot.PathPrefix("/webrtc").Subrouter()
 
 	InitUser()

api4/context.go

@@ -468,7 +468,7 @@ func (c *Context) RequireCategory() *Context {
 		return c
 	}
 
-	if !model.IsValidAlphaNum(c.Params.Category, true) {
+	if !model.IsValidAlphaNumHyphenUnderscore(c.Params.Category, true) {
 		c.SetInvalidUrlParam("category")
 	}
 
@@ -492,13 +492,25 @@ func (c *Context) RequirePreferenceName() *Context {
 		return c
 	}
 
-	if !model.IsValidAlphaNum(c.Params.PreferenceName, true) {
+	if !model.IsValidAlphaNumHyphenUnderscore(c.Params.PreferenceName, true) {
 		c.SetInvalidUrlParam("preference_name")
 	}
 
 	return c
 }
 
+func (c *Context) RequireEmojiName() *Context {
+	if c.Err != nil {
+		return c
+	}
+
+	if len(c.Params.EmojiName) == 0 || len(c.Params.EmojiName) > 64 || !model.IsValidAlphaNumHyphenUnderscore(c.Params.EmojiName, false) {
+		c.SetInvalidUrlParam("emoji_name")
+	}
+
+	return c
+}
+
 func (c *Context) RequireHookId() *Context {
 	if c.Err != nil {
 		return c

api4/params.go

@@ -32,6 +32,7 @@ type ApiParams struct {
 	TeamName       string
 	ChannelName    string
 	PreferenceName string
+	EmojiName      string
 	Category       string
 	Service        string
 	Page           int
@@ -111,6 +112,10 @@ func ApiParamsFromRequest(r *http.Request) *ApiParams {
 		params.PreferenceName = val
 	}
 
+	if val, ok := props["emoji_name"]; ok {
+		params.EmojiName = val
+	}
+
 	if val, err := strconv.Atoi(r.URL.Query().Get("page")); err != nil || val < 0 {
 		params.Page = PAGE_DEFAULT
 	} else {

api4/reaction.go

@@ -17,6 +17,7 @@ func InitReaction() {
 
 	BaseRoutes.Reactions.Handle("", ApiSessionRequired(saveReaction)).Methods("POST")
 	BaseRoutes.Post.Handle("/reactions", ApiSessionRequired(getReactions)).Methods("GET")
+	BaseRoutes.ReactionByNameForPostForUser.Handle("", ApiSessionRequired(deleteReaction)).Methods("DELETE")
 }
 
 func saveReaction(c *Context, w http.ResponseWriter, r *http.Request) {
@@ -71,3 +72,44 @@ func getReactions(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 }
+
+func deleteReaction(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireUserId()
+	if c.Err != nil {
+		return
+	}
+
+	c.RequirePostId()
+	if c.Err != nil {
+		return
+	}
+
+	c.RequireEmojiName()
+	if c.Err != nil {
+		return
+	}
+
+	if !app.SessionHasPermissionToChannelByPost(c.Session, c.Params.PostId, model.PERMISSION_READ_CHANNEL) {
+		c.SetPermissionError(model.PERMISSION_READ_CHANNEL)
+		return
+	}
+
+	if c.Params.UserId != c.Session.UserId && !app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) {
+		c.SetPermissionError(model.PERMISSION_MANAGE_SYSTEM)
+		return
+	}
+
+	reaction := &model.Reaction{
+		UserId:    c.Params.UserId,
+		PostId:    c.Params.PostId,
+		EmojiName: c.Params.EmojiName,
+	}
+
+	err := app.DeleteReactionForPost(reaction)
+	if err != nil {
+		c.Err = err
+		return
+	}
+
+	ReturnStatusOK(w)
+}

api4/reaction_test.go

@@ -193,3 +193,127 @@ func TestGetReactions(t *testing.T) {
 	_, resp = th.SystemAdminClient.GetReactions(postId)
 	CheckNoError(t, resp)
 }
+
+func TestDeleteReaction(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer TearDown()
+	Client := th.Client
+	userId := th.BasicUser.Id
+	user2Id := th.BasicUser2.Id
+	postId := th.BasicPost.Id
+
+	r1 := &model.Reaction{
+		UserId:    userId,
+		PostId:    postId,
+		EmojiName: "smile",
+	}
+
+	app.SaveReactionForPost(r1)
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 1 {
+		t.Fatal("didn't save reaction correctly")
+	}
+
+	ok, resp := Client.DeleteReaction(r1)
+	CheckNoError(t, resp)
+
+	if !ok {
+		t.Fatal("should have returned true")
+	}
+
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 0 {
+		t.Fatal("should have deleted reaction")
+	}
+
+	// deleting one reaction when a post has multiple reactions
+	r2 := &model.Reaction{
+		UserId:    userId,
+		PostId:    postId,
+		EmojiName: "smile-",
+	}
+
+	app.SaveReactionForPost(r1)
+	app.SaveReactionForPost(r2)
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 {
+		t.Fatal("didn't save reactions correctly")
+	}
+
+	_, resp = Client.DeleteReaction(r2)
+	CheckNoError(t, resp)
+
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 1 || *reactions[0] != *r1 {
+		t.Fatal("should have deleted 1 reaction only")
+	}
+
+	// deleting a reaction made by another user
+	r3 := &model.Reaction{
+		UserId:    user2Id,
+		PostId:    postId,
+		EmojiName: "smile_",
+	}
+
+	th.LoginBasic2()
+	app.SaveReactionForPost(r3)
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 {
+		t.Fatal("didn't save reaction correctly")
+	}
+
+	th.LoginBasic()
+
+	ok, resp = Client.DeleteReaction(r3)
+	CheckForbiddenStatus(t, resp)
+
+	if ok {
+		t.Fatal("should have returned false")
+	}
+
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 2 {
+		t.Fatal("should have not deleted a reaction")
+	}
+
+	r1.PostId = GenerateTestId()
+	_, resp = Client.DeleteReaction(r1)
+	CheckForbiddenStatus(t, resp)
+
+	r1.PostId = "junk"
+
+	_, resp = Client.DeleteReaction(r1)
+	CheckBadRequestStatus(t, resp)
+
+	r1.PostId = postId
+	r1.UserId = GenerateTestId()
+
+	_, resp = Client.DeleteReaction(r1)
+	CheckForbiddenStatus(t, resp)
+
+	r1.UserId = "junk"
+
+	_, resp = Client.DeleteReaction(r1)
+	CheckBadRequestStatus(t, resp)
+
+	r1.UserId = userId
+	r1.EmojiName = ""
+
+	_, resp = Client.DeleteReaction(r1)
+	CheckNotFoundStatus(t, resp)
+
+	r1.EmojiName = strings.Repeat("a", 65)
+
+	_, resp = Client.DeleteReaction(r1)
+	CheckBadRequestStatus(t, resp)
+
+	Client.Logout()
+	r1.EmojiName = "smile"
+
+	_, resp = Client.DeleteReaction(r1)
+	CheckUnauthorizedStatus(t, resp)
+
+	_, resp = th.SystemAdminClient.DeleteReaction(r1)
+	CheckNoError(t, resp)
+
+	_, resp = th.SystemAdminClient.DeleteReaction(r3)
+	CheckNoError(t, resp)
+
+	if reactions, err := app.GetReactionsForPost(postId); err != nil || len(reactions) != 0 {
+		t.Fatal("should have deleted both reactions")
+	}
+}

app/reaction.go

@@ -34,6 +34,23 @@ func GetReactionsForPost(postId string) ([]*model.Reaction, *model.AppError) {
 	}
 }
 
+func DeleteReactionForPost(reaction *model.Reaction) *model.AppError {
+	post, err := GetSinglePost(reaction.PostId)
+	if err != nil {
+		return err
+	}
+
+	if result := <-Srv.Store.Reaction().Delete(reaction); result.Err != nil {
+		return result.Err
+	} else {
+		go sendReactionEvent(model.WEBSOCKET_EVENT_REACTION_REMOVED, reaction, post)
+
+		InvalidateCacheForReactions(reaction.PostId)
+	}
+
+	return nil
+}
+
 func sendReactionEvent(event string, reaction *model.Reaction, post *model.Post) {
 	// send out that a reaction has been added/removed
 	message := model.NewWebSocketEvent(event, "", post.ChannelId, "", nil)

model/client4.go

@@ -2533,3 +2533,13 @@ func (c *Client4) GetReactions(postId string) ([]*Reaction, *Response) {
 		return ReactionsFromJson(r.Body), BuildResponse(r)
 	}
 }
+
+// DeleteReaction deletes reaction of a user in a post.
+func (c *Client4) DeleteReaction(reaction *Reaction) (bool, *Response) {
+	if r, err := c.DoApiDelete(c.GetUserRoute(reaction.UserId) + c.GetPostRoute(reaction.PostId) + fmt.Sprintf("/reactions/%v", reaction.EmojiName)); err != nil {
+		return false, &Response{StatusCode: r.StatusCode, Error: err}
+	} else {
+		defer closeBody(r)
+		return CheckStatusOK(r), BuildResponse(r)
+	}
+}

model/emoji.go

@@ -34,7 +34,7 @@ func (emoji *Emoji) IsValid() *AppError {
 		return NewLocAppError("Emoji.IsValid", "model.emoji.user_id.app_error", nil, "")
 	}
 
-	if len(emoji.Name) == 0 || len(emoji.Name) > 64 {
+	if len(emoji.Name) == 0 || len(emoji.Name) > 64 || !IsValidAlphaNumHyphenUnderscore(emoji.Name, false) {
 		return NewLocAppError("Emoji.IsValid", "model.emoji.name.app_error", nil, "")
 	}
 

model/emoji_test.go

@@ -56,8 +56,28 @@ func TestEmojiIsValid(t *testing.T) {
 		t.Fatal()
 	}
 
+	emoji.Name = ""
+	if err := emoji.IsValid(); err == nil {
+		t.Fatal(err)
+	}
+
 	emoji.Name = strings.Repeat("1", 64)
 	if err := emoji.IsValid(); err != nil {
 		t.Fatal(err)
 	}
+
+	emoji.Name = "name-"
+	if err := emoji.IsValid(); err != nil {
+		t.Fatal(err)
+	}
+
+	emoji.Name = "name_"
+	if err := emoji.IsValid(); err != nil {
+		t.Fatal(err)
+	}
+
+	emoji.Name = "name:"
+	if err := emoji.IsValid(); err == nil {
+		t.Fatal(err)
+	}
 }

model/reaction.go

@@ -60,7 +60,7 @@ func (o *Reaction) IsValid() *AppError {
 		return NewLocAppError("Reaction.IsValid", "model.reaction.is_valid.post_id.app_error", nil, "post_id="+o.PostId)
 	}
 
-	if len(o.EmojiName) == 0 || len(o.EmojiName) > 64 {
+	if len(o.EmojiName) == 0 || len(o.EmojiName) > 64 || !IsValidAlphaNumHyphenUnderscore(o.EmojiName, false) {
 		return NewLocAppError("Reaction.IsValid", "model.reaction.is_valid.emoji_name.app_error", nil, "emoji_name="+o.EmojiName)
 	}
 

model/reaction_test.go

@@ -57,6 +57,21 @@ func TestReactionIsValid(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	reaction.EmojiName = "emoji-"
+	if err := reaction.IsValid(); err != nil {
+		t.Fatal(err)
+	}
+
+	reaction.EmojiName = "emoji_"
+	if err := reaction.IsValid(); err != nil {
+		t.Fatal(err)
+	}
+
+	reaction.EmojiName = "emoji:"
+	if err := reaction.IsValid(); err == nil {
+		t.Fatal(err)
+	}
+
 	reaction.CreateAt = 0
 	if err := reaction.IsValid(); err == nil {
 		t.Fatal("create at should be invalid")

model/team.go

@@ -233,7 +233,7 @@ func IsReservedTeamName(s string) bool {
 
 func IsValidTeamName(s string) bool {
 
-	if !IsValidAlphaNum(s, false) {
+	if !IsValidAlphaNum(s) {
 		return false
 	}
 

model/utils.go

@@ -297,7 +297,7 @@ var reservedName = []string{
 
 func IsValidChannelIdentifier(s string) bool {
 
-	if !IsValidAlphaNum(s, true) {
+	if !IsValidAlphaNumHyphenUnderscore(s, true) {
 		return false
 	}
 
@@ -308,22 +308,20 @@ func IsValidChannelIdentifier(s string) bool {
 	return true
 }
 
-var validAlphaNumUnderscore = regexp.MustCompile(`^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]+$`)
-var validAlphaNum = regexp.MustCompile(`^[a-z0-9]+([a-z\-0-9]+|(__)?)[a-z0-9]+$`)
+func IsValidAlphaNum(s string) bool {
+	validAlphaNum := regexp.MustCompile(`^[a-z0-9]+([a-z\-0-9]+|(__)?)[a-z0-9]+$`)
 
-func IsValidAlphaNum(s string, allowUnderscores bool) bool {
-	var match bool
-	if allowUnderscores {
-		match = validAlphaNumUnderscore.MatchString(s)
-	} else {
-		match = validAlphaNum.MatchString(s)
+	return validAlphaNum.MatchString(s)
+}
+
+func IsValidAlphaNumHyphenUnderscore(s string, withFormat bool) bool {
+	if withFormat {
+		validAlphaNumHyphenUnderscore := regexp.MustCompile(`^[a-z0-9]+([a-z\-\_0-9]+|(__)?)[a-z0-9]+$`)
+		return validAlphaNumHyphenUnderscore.MatchString(s)
 	}
 
-	if !match {
-		return false
-	}
-
-	return true
+	validSimpleAlphaNumHyphenUnderscore := regexp.MustCompile(`^[a-zA-Z0-9\-_]+$`)
+	return validSimpleAlphaNumHyphenUnderscore.MatchString(s)
 }
 
 func Etag(parts ...interface{}) string {

model/utils_test.go

@@ -137,3 +137,191 @@ func TestParseHashtags(t *testing.T) {
 		}
 	}
 }
+
+func TestIsValidAlphaNum(t *testing.T) {
+	cases := []struct {
+		Input  string
+		Result bool
+	}{
+		{
+			Input:  "test",
+			Result: true,
+		},
+		{
+			Input:  "test-name",
+			Result: true,
+		},
+		{
+			Input:  "test--name",
+			Result: true,
+		},
+		{
+			Input:  "test__name",
+			Result: true,
+		},
+		{
+			Input:  "-",
+			Result: false,
+		},
+		{
+			Input:  "__",
+			Result: false,
+		},
+		{
+			Input:  "test-",
+			Result: false,
+		},
+		{
+			Input:  "test--",
+			Result: false,
+		},
+		{
+			Input:  "test__",
+			Result: false,
+		},
+		{
+			Input:  "test:name",
+			Result: false,
+		},
+	}
+
+	for _, tc := range cases {
+		actual := IsValidAlphaNum(tc.Input)
+		if actual != tc.Result {
+			t.Fatalf("case: %v\tshould returned: %#v", tc, tc.Result)
+		}
+	}
+}
+
+func TestIsValidAlphaNumHyphenUnderscore(t *testing.T) {
+	casesWithFormat := []struct {
+		Input  string
+		Result bool
+	}{
+		{
+			Input:  "test",
+			Result: true,
+		},
+		{
+			Input:  "test-name",
+			Result: true,
+		},
+		{
+			Input:  "test--name",
+			Result: true,
+		},
+		{
+			Input:  "test__name",
+			Result: true,
+		},
+		{
+			Input:  "test_name",
+			Result: true,
+		},
+		{
+			Input:  "test_-name",
+			Result: true,
+		},
+		{
+			Input:  "-",
+			Result: false,
+		},
+		{
+			Input:  "__",
+			Result: false,
+		},
+		{
+			Input:  "test-",
+			Result: false,
+		},
+		{
+			Input:  "test--",
+			Result: false,
+		},
+		{
+			Input:  "test__",
+			Result: false,
+		},
+		{
+			Input:  "test:name",
+			Result: false,
+		},
+	}
+
+	for _, tc := range casesWithFormat {
+		actual := IsValidAlphaNumHyphenUnderscore(tc.Input, true)
+		if actual != tc.Result {
+			t.Fatalf("case: %v\tshould returned: %#v", tc, tc.Result)
+		}
+	}
+
+	casesWithoutFormat := []struct {
+		Input  string
+		Result bool
+	}{
+		{
+			Input:  "test",
+			Result: true,
+		},
+		{
+			Input:  "test-name",
+			Result: true,
+		},
+		{
+			Input:  "test--name",
+			Result: true,
+		},
+		{
+			Input:  "test__name",
+			Result: true,
+		},
+		{
+			Input:  "test_name",
+			Result: true,
+		},
+		{
+			Input:  "test_-name",
+			Result: true,
+		},
+		{
+			Input:  "-",
+			Result: true,
+		},
+		{
+			Input:  "_",
+			Result: true,
+		},
+		{
+			Input:  "test-",
+			Result: true,
+		},
+		{
+			Input:  "test--",
+			Result: true,
+		},
+		{
+			Input:  "test__",
+			Result: true,
+		},
+		{
+			Input:  ".",
+			Result: false,
+		},
+
+		{
+			Input:  "test,",
+			Result: false,
+		},
+		{
+			Input:  "test:name",
+			Result: false,
+		},
+	}
+
+	for _, tc := range casesWithoutFormat {
+		actual := IsValidAlphaNumHyphenUnderscore(tc.Input, false)
+		if actual != tc.Result {
+			t.Fatalf("case: '%v'\tshould returned: %#v", tc.Input, tc.Result)
+		}
+	}
+}