diff --git a/app/post.go b/app/post.go index a8a346c9cc..2554df9b21 100644 --- a/app/post.go +++ b/app/post.go @@ -72,8 +72,11 @@ func (a *App) CreatePostAsUser(post *model.Post, currentSessionId string) (*mode return nil, err } - // Update the LastViewAt only if the post does not have from_webhook prop set (eg. Zapier app) - if _, ok := post.GetProps()["from_webhook"]; !ok { + // Update the LastViewAt only if the post does not have from_webhook prop set (e.g. Zapier app), + // or if it does not have from_bot set (e.g. from discovering the user is a bot within CreatePost). + _, fromWebhook := post.GetProps()["from_webhook"] + _, fromBot := post.GetProps()["from_bot"] + if !fromWebhook && !fromBot { if _, err := a.MarkChannelsAsViewed([]string{post.ChannelId}, post.UserId, currentSessionId); err != nil { mlog.Error( "Encountered error updating last viewed", diff --git a/app/post_test.go b/app/post_test.go index 7b8e606d55..aaf8d54285 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -13,11 +13,13 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/v5/mlog" "github.com/mattermost/mattermost-server/v5/model" "github.com/mattermost/mattermost-server/v5/plugin/plugintest/mock" "github.com/mattermost/mattermost-server/v5/services/searchengine/mocks" "github.com/mattermost/mattermost-server/v5/store/storetest" storemocks "github.com/mattermost/mattermost-server/v5/store/storetest/mocks" + "github.com/mattermost/mattermost-server/v5/testlib" ) func TestCreatePostDeduplicate(t *testing.T) { @@ -821,6 +823,128 @@ func TestPatchPost(t *testing.T) { }) } +func TestCreatePostAsUser(t *testing.T) { + t.Run("marks channel as viewed for regular user", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "test", + UserId: th.BasicUser.Id, + } + + channelMemberBefore, appErr := th.App.Srv().Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + time.Sleep(1 * time.Millisecond) + _, appErr = th.App.CreatePostAsUser(post, "") + require.Nil(t, appErr) + + channelMemberAfter, appErr := th.App.Srv().Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + require.Greater(t, channelMemberAfter.LastViewedAt, channelMemberBefore.LastViewedAt) + }) + + t.Run("does not mark channel as viewed for webhook from user", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "test", + UserId: th.BasicUser.Id, + } + post.AddProp("from_webhook", "true") + + channelMemberBefore, appErr := th.App.Srv().Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + time.Sleep(1 * time.Millisecond) + _, appErr = th.App.CreatePostAsUser(post, "") + require.Nil(t, appErr) + + channelMemberAfter, appErr := th.App.Srv().Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + require.Equal(t, channelMemberAfter.LastViewedAt, channelMemberBefore.LastViewedAt) + }) + + t.Run("does not mark channel as viewed for bot user in channel", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + bot := th.CreateBot() + + botUser, appErr := th.App.GetUser(bot.UserId) + require.Nil(t, appErr) + + th.LinkUserToTeam(botUser, th.BasicTeam) + th.AddUserToChannel(botUser, th.BasicChannel) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "test", + UserId: bot.UserId, + } + + channelMemberBefore, appErr := th.App.Srv().Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + time.Sleep(1 * time.Millisecond) + _, appErr = th.App.CreatePostAsUser(post, "") + require.Nil(t, appErr) + + channelMemberAfter, appErr := th.App.Srv().Store.Channel().GetMember(th.BasicChannel.Id, th.BasicUser.Id) + require.Nil(t, appErr) + + require.Equal(t, channelMemberAfter.LastViewedAt, channelMemberBefore.LastViewedAt) + }) + + t.Run("logs warning for user not in channel", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + user := th.CreateUser() + th.LinkUserToTeam(user, th.BasicTeam) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "test", + UserId: user.Id, + } + + _, appErr := th.App.CreatePostAsUser(post, "") + require.Nil(t, appErr) + + testlib.AssertLog(t, th.LogBuffer, mlog.LevelWarn, "Failed to get membership") + }) + + t.Run("does not log warning for bot user not in channel", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + bot := th.CreateBot() + + botUser, appErr := th.App.GetUser(bot.UserId) + require.Nil(t, appErr) + + th.LinkUserToTeam(botUser, th.BasicTeam) + + post := &model.Post{ + ChannelId: th.BasicChannel.Id, + Message: "test", + UserId: bot.UserId, + } + + _, appErr = th.App.CreatePostAsUser(post, "") + require.Nil(t, appErr) + + testlib.AssertNoLog(t, th.LogBuffer, mlog.LevelWarn, "Failed to get membership") + }) +} + func TestPatchPostInArchivedChannel(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/testlib/assertions.go b/testlib/assertions.go index 4fb56ba45a..cb2ab5ce30 100644 --- a/testlib/assertions.go +++ b/testlib/assertions.go @@ -31,3 +31,24 @@ func AssertLog(t *testing.T, logs *bytes.Buffer, level, message string) { t.Fatalf("failed to find %s log message: %s", level, message) } + +// AssertNoLog asserts that a JSON-encoded buffer of logs does not contains one with the given level and message. +func AssertNoLog(t *testing.T, logs *bytes.Buffer, level, message string) { + dec := json.NewDecoder(logs) + for { + var log struct { + Level string + Msg string + } + if err := dec.Decode(&log); err == io.EOF { + break + } else if err != nil { + continue + } + + if log.Level == level && log.Msg == message { + t.Fatalf("found %s log message: %s", level, message) + return + } + } +}