diff --git a/api4/post.go b/api4/post.go index ff2b377bb7..a6b0529902 100644 --- a/api4/post.go +++ b/api4/post.go @@ -437,6 +437,9 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Updating the file_ids of a post is not a supported operation and will be ignored + post.FileIds = nil + if !c.App.SessionHasPermissionToChannelByPost(c.App.Session, c.Params.PostId, model.PERMISSION_EDIT_POST) { c.SetPermissionError(model.PERMISSION_EDIT_POST) return @@ -479,6 +482,9 @@ func patchPost(c *Context, w http.ResponseWriter, r *http.Request) { return } + // Updating the file_ids of a post is not a supported operation and will be ignored + post.FileIds = nil + if !c.App.SessionHasPermissionToChannelByPost(c.App.Session, c.Params.PostId, model.PERMISSION_EDIT_POST) { c.SetPermissionError(model.PERMISSION_EDIT_POST) return diff --git a/api4/post_test.go b/api4/post_test.go index 2e3b14b894..0f9a002b6d 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -580,6 +580,29 @@ func TestUpdatePost(t *testing.T) { _, resp = Client.UpdatePost(rpost2.Id, up2) CheckBadRequestStatus(t, resp) + rpost3, err := th.App.CreatePost(&model.Post{ChannelId: channel.Id, Message: "zz" + model.NewId() + "a", UserId: th.BasicUser.Id}, channel, false) + if err != nil { + t.Fatal(err) + } + + fileIds := model.StringArray{"abcdef", "geh"} + up3 := &model.Post{Id: rpost3.Id, ChannelId: channel.Id, Message: "zz" + model.NewId() + " update post 3", FileIds: fileIds} + rrupost3, resp := Client.UpdatePost(rpost3.Id, up3) + CheckNoError(t, resp) + assert.Empty(t, rrupost3.FileIds) + + up4 := &model.Post{Id: rpost3.Id, ChannelId: channel.Id, Message: "zz" + model.NewId() + " update post 3"} + up4.AddProp("attachments", []model.SlackAttachment{ + { + Text: "Hello World", + }, + }) + rrupost3, resp = Client.UpdatePost(rpost3.Id, up4) + CheckNoError(t, resp) + assert.NotEqual(t, rpost3.EditAt, rrupost3.EditAt) + assert.NotEqual(t, rpost3.Attachments(), rrupost3.Attachments()) + + Client.Logout() _, resp = Client.UpdatePost(rpost.Id, rpost) CheckUnauthorizedStatus(t, resp) @@ -671,16 +694,30 @@ func TestPatchPost(t *testing.T) { if rpost.Hashtags != "#otherhashtag" { t.Fatal("Message did not update properly") } - if len(rpost.FileIds) != 3 { - t.Fatal("FileIds did not update properly") + if len(rpost.FileIds) == 3 { + t.Fatal("FileIds should not update properly") } - if !reflect.DeepEqual(rpost.FileIds, *patch.FileIds) { - t.Fatal("FileIds did not update properly") + if reflect.DeepEqual(rpost.FileIds, *patch.FileIds) { + t.Fatal("FileIds should not update") } if rpost.HasReactions { t.Fatal("HasReactions did not update properly") } + patch2 := &model.PostPatch{} + attachments := []model.SlackAttachment{ + { + Text: "Hello World", + }, + } + patch2.Props = new(model.StringInterface) + *patch2.Props = model.StringInterface{"attachments": attachments} + + rpost2, resp := Client.PatchPost(post.Id, patch2) + CheckNoError(t, resp) + assert.NotEmpty(t, rpost2.Props["attachments"]) + assert.NotEqual(t, rpost.EditAt, rpost2.EditAt) + if r, err := Client.DoApiPut("/posts/"+post.Id+"/patch", "garbage"); err == nil { t.Fatal("should have errored") } else { diff --git a/app/post.go b/app/post.go index 1f68843be7..33a45350fd 100644 --- a/app/post.go +++ b/app/post.go @@ -506,6 +506,11 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model newPost.Props = post.Props } + // Avoid deep-equal checks if EditAt was already modified through message change + if newPost.EditAt == oldPost.EditAt && (!oldPost.FileIds.Equals(newPost.FileIds) || !oldPost.AttachmentsEqual(newPost)) { + newPost.EditAt = model.GetMillis() + } + if err := a.FillInPostProps(post, nil); err != nil { return nil, err } diff --git a/model/integration_action.go b/model/integration_action.go index 241981f2ff..b7762333fb 100644 --- a/model/integration_action.go +++ b/model/integration_action.go @@ -58,6 +58,66 @@ type PostAction struct { Cookie string `json:"cookie,omitempty" db:"-"` } +func (p *PostAction) Equals(input *PostAction) bool { + if p.Id != input.Id { + return false + } + + if p.Type != input.Type { + return false + } + + if p.Name != input.Name { + return false + } + + if p.DataSource != input.DataSource { + return false + } + + if p.Cookie != input.Cookie { + return false + } + + // Compare PostActionOptions + if len(p.Options) != len(input.Options) { + return false + } + + for k := range p.Options { + if p.Options[k].Text != input.Options[k].Text { + return false + } + + if p.Options[k].Value != input.Options[k].Value { + return false + } + } + + // Compare PostActionIntegration + if p.Integration.URL != input.Integration.URL { + return false + } + + if len(p.Integration.Context) != len(input.Integration.Context) { + return false + } + + for key, value := range p.Integration.Context { + inputValue, ok := input.Integration.Context[key] + + if !ok { + return false + } + + if value != inputValue { + return false + } + } + + return true +} + // PostActionCookie is set by the server, serialized and encrypted into // PostAction.Cookie. The clients should hold on to it, and include it with // subsequent DoPostAction requests. This allows the server to access the diff --git a/model/post.go b/model/post.go index 09100563e1..6b405f6859 100644 --- a/model/post.go +++ b/model/post.go @@ -403,6 +403,23 @@ func (o *Post) Attachments() []*SlackAttachment { return ret } +func (o *Post) AttachmentsEqual(input *Post) bool { + attachments := o.Attachments() + inputAttachments := input.Attachments() + + if len(attachments) != len(inputAttachments) { + return false + } + + for i := range attachments { + if !attachments[i].Equals(inputAttachments[i]) { + return false + } + } + + return true +} + var markdownDestinationEscaper = strings.NewReplacer( `\`, `\\`, `<`, `\<`, diff --git a/model/post_test.go b/model/post_test.go index 5f3716fe7e..c2e67e37c2 100644 --- a/model/post_test.go +++ b/model/post_test.go @@ -180,6 +180,233 @@ func TestPostSanitizeProps(t *testing.T) { } } +func TestPost_AttachmentsEqual(t *testing.T) { + post1 := &Post { + } + post2 := &Post { + } + for name, tc := range map[string]struct { + Attachments1 []*SlackAttachment + Attachments2 []*SlackAttachment + Expected bool + }{ + "Empty": { + nil, + nil, + true, + }, + "DifferentLength": { + []*SlackAttachment{ + { + Text: "Hello World", + }, + }, + nil, + false, + }, + "EqualText": { + []*SlackAttachment{ + { + Text: "Hello World", + }, + }, + []*SlackAttachment{ + { + Text: "Hello World", + }, + }, + true, + }, + "DifferentText": { + []*SlackAttachment{ + { + Text: "Hello World", + }, + }, + []*SlackAttachment{ + { + Text: "Hello World 2", + }, + }, + false, + }, + "DifferentColor": { + []*SlackAttachment{ + { + Text: "Hello World", + Color: "#152313", + }, + }, + []*SlackAttachment{ + { + Text: "Hello World 2", + }, + }, + false, + }, + "EqualFields": { + []*SlackAttachment{ + { + Fields: []*SlackAttachmentField { + { + Title: "Hello World", + Value: "FooBar", + }, + { + Title: "Hello World2", + Value: "FooBar2", + }, + }, + }, + }, + []*SlackAttachment{ + { + Fields: []*SlackAttachmentField { + { + Title: "Hello World", + Value: "FooBar", + }, + { + Title: "Hello World2", + Value: "FooBar2", + }, + }, + }, + }, + true, + }, + "DifferentFields": { + []*SlackAttachment{ + { + Fields: []*SlackAttachmentField { + { + Title: "Hello World", + Value: "FooBar", + }, + }, + }, + }, + []*SlackAttachment{ + { + Fields: []*SlackAttachmentField { + { + Title: "Hello World", + Value: "FooBar", + Short: false, + }, + { + Title: "Hello World2", + Value: "FooBar2", + Short: true, + }, + }, + }, + }, + false, + }, + "EqualActions": { + []*SlackAttachment{ + { + Actions: []*PostAction{ + { + Name: "FooBar", + Options: []*PostActionOptions { + { + Text: "abcdef", + Value: "abcdef", + }, + }, + Integration: &PostActionIntegration{ + URL: "http://localhost", + Context: map[string]interface{}{ + "context": "foobar", + "test": 123, + }, + }, + }, + }, + }, + }, + []*SlackAttachment{ + { + Actions: []*PostAction{ + { + Name: "FooBar", + Options: []*PostActionOptions { + { + Text: "abcdef", + Value: "abcdef", + }, + }, + Integration: &PostActionIntegration{ + URL: "http://localhost", + Context: map[string]interface{}{ + "context": "foobar", + "test": 123, + }, + }, + }, + }, + }, + }, + true, + }, + "DifferentActions": { + []*SlackAttachment{ + { + Actions: []*PostAction{ + { + Name: "FooBar", + Options: []*PostActionOptions { + { + Text: "abcdef", + Value: "abcdef", + }, + }, + Integration: &PostActionIntegration{ + URL: "http://localhost", + Context: map[string]interface{}{ + "context": "foobar", + "test": "mattermost", + }, + }, + }, + }, + }, + }, + []*SlackAttachment{ + { + Actions: []*PostAction{ + { + Name: "FooBar", + Options: []*PostActionOptions { + { + Text: "abcdef", + Value: "abcdef", + }, + }, + Integration: &PostActionIntegration{ + URL: "http://localhost", + Context: map[string]interface{}{ + "context": "foobar", + "test": 123, + }, + }, + }, + }, + }, + }, + false, + }, + } { + t.Run(name, func(t *testing.T) { + post1.AddProp("attachments", tc.Attachments1) + post2.AddProp("attachments", tc.Attachments2) + assert.Equal(t, tc.Expected, post1.AttachmentsEqual(post2)) + }) + } +} + var markdownSample, markdownSampleWithRewrittenImageURLs string func init() { diff --git a/model/slack_attachment.go b/model/slack_attachment.go index 405686ed82..0f1b96fb50 100644 --- a/model/slack_attachment.go +++ b/model/slack_attachment.go @@ -30,12 +30,116 @@ type SlackAttachment struct { Actions []*PostAction `json:"actions,omitempty"` } +func (s *SlackAttachment) Equals(input *SlackAttachment) bool { + // Direct comparison of simple types + + if s.Id != input.Id { + return false + } + + if s.Fallback != input.Fallback { + return false + } + + if s.Color != input.Color { + return false + } + + if s.Pretext != input.Pretext { + return false + } + + if s.AuthorName != input.AuthorName { + return false + } + + if s.AuthorLink != input.AuthorLink { + return false + } + + if s.AuthorIcon != input.AuthorIcon { + return false + } + + if s.Title != input.Title { + return false + } + + if s.TitleLink != input.TitleLink { + return false + } + + if s.Text != input.Text { + return false + } + + if s.ImageURL != input.ImageURL { + return false + } + + if s.ThumbURL != input.ThumbURL { + return false + } + + if s.Footer != input.Footer { + return false + } + + if s.FooterIcon != input.FooterIcon { + return false + } + + // Compare length & slice values of fields + if len(s.Fields) != len(input.Fields) { + return false + } + + for j := range s.Fields { + if !s.Fields[j].Equals(input.Fields[j]) { + return false + } + } + + // Compare length & slice values of actions + if len(s.Actions) != len(input.Actions) { + return false + } + + for j := range s.Actions { + if !s.Actions[j].Equals(input.Actions[j]) { + return false + } + } + + if s.Timestamp != input.Timestamp { + return false + } + + return true +} + type SlackAttachmentField struct { Title string `json:"title"` Value interface{} `json:"value"` Short SlackCompatibleBool `json:"short"` } +func (s *SlackAttachmentField) Equals(input *SlackAttachmentField) bool { + if s.Title != input.Title { + return false + } + + if s.Value != input.Value { + return false + } + + if s.Short != input.Short { + return false + } + + return true +} + func StringifySlackFieldValue(a []*SlackAttachment) []*SlackAttachment { var nonNilAttachments []*SlackAttachment for _, attachment := range a { diff --git a/model/utils.go b/model/utils.go index 849e529c17..871c4b5ea8 100644 --- a/model/utils.go +++ b/model/utils.go @@ -36,6 +36,22 @@ type StringInterface map[string]interface{} type StringMap map[string]string type StringArray []string +func (sa StringArray) Equals(input StringArray) bool { + + if len(sa) != len(input) { + return false + } + + for index := range sa { + + if sa[index] != input[index] { + return false + } + } + + return true +} + var translateFunc goi18n.TranslateFunc = nil func AppErrorInit(t goi18n.TranslateFunc) { diff --git a/model/utils_test.go b/model/utils_test.go index ec4bb83d1b..51697f8a12 100644 --- a/model/utils_test.go +++ b/model/utils_test.go @@ -250,6 +250,49 @@ var hashtags = map[string]string{ "foo#bar": "", } +func TestStringArray_Equal(t *testing.T) { + for name, tc := range map[string]struct { + Array1 StringArray + Array2 StringArray + Expected bool + }{ + "Empty": { + nil, + nil, + true, + }, + "EqualLength_EqualValue": { + StringArray{"123"}, + StringArray{"123"}, + true, + }, + "DifferentLength": { + StringArray{"123"}, + StringArray{"123", "abc"}, + false, + }, + "DifferentValues_EqualLength": { + StringArray{"123"}, + StringArray{"abc"}, + false, + }, + "EqualLength_EqualValues": { + StringArray{"123", "abc"}, + StringArray{"123", "abc"}, + true, + }, + "EqualLength_EqualValues_DifferentOrder": { + StringArray{"abc", "123"}, + StringArray{"123", "abc"}, + false, + }, + } { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tc.Expected, tc.Array1.Equals(tc.Array2)) + }) + } +} + func TestParseHashtags(t *testing.T) { for input, output := range hashtags { if o, _ := ParseHashtags(input); o != output {