MM-45009: Delete ThreadMemberships from "left" channels (#22559)

* MM-50550: Filter out threads from "left" channels v2

Currently leaving a channel doesn't affect the thread memberships of
that user/channel combination.
This PR aims to filter out all threads from those channels for the user.

Adds a DeleteAt column in the ThreadMemberships table, and filter out
all thread memberships that are "deleted".
Each time a user leaves a channel all thread memberships are going to be
marked as deleted, and when a user joins a channel again all those
existing thread memberships will be re-instantiated.

Adds a migration to mark all existing thread memberships as deleted
depending on whether there exists a channel membership for that
channel/user.

* Added migration files into list

* Fixes tests

* Fixes case where DeleteAt would be null

* Guard thread API endpoints with appropriate perms

* Deletes ThreadMembership rows upon leaving channel

* Minor style changes

* Use NoTranslation error

* Refactors tests

* Adds API tests to assert permissions on Team

* Adds tests, and fixes migrations

* Fixes test description

* Fix test

* Removes check on DM/GMs

* Change the MySQL query in the migration

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Kyriakos Z
2023-04-19 15:20:34 +03:00
коммит произвёл GitHub
родитель c34a50a6c7
Коммит a24111f9bd
16 изменённых файлов: 426 добавлений и 38 удалений

Просмотреть файл

@@ -3106,6 +3106,10 @@ func getThreadForUser(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
if !c.App.SessionHasPermissionToChannelByPost(*c.AppContext.Session(), c.Params.ThreadId, model.PermissionReadChannel) {
c.SetPermissionError(model.PermissionReadChannel)
return
}
extendedStr := r.URL.Query().Get("extended")
extended, _ := strconv.ParseBool(extendedStr)
@@ -3136,6 +3140,10 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
options := model.GetUserThreadsOpts{
Since: 0,
@@ -3213,6 +3221,10 @@ func updateReadStateThreadByUser(c *Context, w http.ResponseWriter, r *http.Requ
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
if !c.App.SessionHasPermissionToChannelByPost(*c.AppContext.Session(), c.Params.ThreadId, model.PermissionReadChannel) {
c.SetPermissionError(model.PermissionReadChannel)
return
}
thread, err := c.App.UpdateThreadReadForUser(c.AppContext, c.AppContext.Session().Id, c.Params.UserId, c.Params.TeamId, c.Params.ThreadId, c.Params.Timestamp)
if err != nil {
@@ -3279,6 +3291,10 @@ func unfollowThreadByUser(c *Context, w http.ResponseWriter, r *http.Request) {
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
if !c.App.SessionHasPermissionToChannelByPost(*c.AppContext.Session(), c.Params.ThreadId, model.PermissionReadChannel) {
c.SetPermissionError(model.PermissionReadChannel)
return
}
err := c.App.UpdateThreadFollowForUser(c.Params.UserId, c.Params.TeamId, c.Params.ThreadId, false)
if err != nil {
@@ -3338,6 +3354,10 @@ func updateReadStateAllThreadsByUser(c *Context, w http.ResponseWriter, r *http.
c.SetPermissionError(model.PermissionEditOtherUsers)
return
}
if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), c.Params.TeamId, model.PermissionViewTeam) {
c.SetPermissionError(model.PermissionViewTeam)
return
}
err := c.App.UpdateThreadsReadForUser(c.Params.UserId, c.Params.TeamId)
if err != nil {

Просмотреть файл

@@ -6360,6 +6360,15 @@ func TestGetThreadsForUser(t *testing.T) {
require.NoError(t, err)
require.Equal(t, uss.TotalUnreadThreads, int64(2))
})
t.Run("should error when not a team member", func(t *testing.T) {
th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam)
defer th.LinkUserToTeam(th.BasicUser, th.BasicTeam)
_, resp, err := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{})
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
}
func TestThreadSocketEvents(t *testing.T) {
@@ -6855,52 +6864,64 @@ func TestSingleThreadGet(t *testing.T) {
})
client := th.Client
defer th.App.Srv().Store().Post().PermanentDeleteByUser(th.BasicUser.Id)
defer th.App.Srv().Store().Post().PermanentDeleteByUser(th.SystemAdminUser.Id)
// create a post by regular user
rpost, _ := postAndCheck(t, client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"})
// reply with another
postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id})
t.Run("get single thread", func(t *testing.T) {
defer th.App.Srv().Store().Post().PermanentDeleteByUser(th.BasicUser.Id)
defer th.App.Srv().Store().Post().PermanentDeleteByUser(th.SystemAdminUser.Id)
// create another thread to check that we are not returning it by mistake
rpost2, _ := postAndCheck(t, client, &model.Post{
ChannelId: th.BasicChannel2.Id,
Message: "testMsg2",
Metadata: &model.PostMetadata{
Priority: &model.PostPriority{
Priority: model.NewString(model.PostPriorityUrgent),
// create a post by regular user
rpost, _ := postAndCheck(t, client, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"})
// reply with another
postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id})
// create another thread to check that we are not returning it by mistake
rpost2, _ := postAndCheck(t, client, &model.Post{
ChannelId: th.BasicChannel2.Id,
Message: "testMsg2",
Metadata: &model.PostMetadata{
Priority: &model.PostPriority{
Priority: model.NewString(model.PostPriorityUrgent),
},
},
},
})
postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testReply", RootId: rpost2.Id})
})
postAndCheck(t, th.SystemAdminClient, &model.Post{ChannelId: th.BasicChannel2.Id, Message: "testReply", RootId: rpost2.Id})
// regular user should have two threads with 3 replies total
threads, _ := checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 2, 2, nil)
// regular user should have two threads with 3 replies total
threads, _ := checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 2, 2, nil)
tr, _, err := th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, threads.Threads[0].PostId, false)
require.NoError(t, err)
require.NotNil(t, tr)
require.Equal(t, threads.Threads[0].PostId, tr.PostId)
require.Empty(t, tr.Participants[0].Username)
tr, _, err := th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, threads.Threads[0].PostId, false)
require.NoError(t, err)
require.NotNil(t, tr)
require.Equal(t, threads.Threads[0].PostId, tr.PostId)
require.Empty(t, tr.Participants[0].Username)
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.PostPriority = false
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.PostPriority = false
})
tr, _, err = th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, threads.Threads[0].PostId, true)
require.NoError(t, err)
require.NotEmpty(t, tr.Participants[0].Username)
require.Equal(t, false, tr.IsUrgent)
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.PostPriority = true
cfg.FeatureFlags.PostPriority = true
})
tr, _, err = th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, threads.Threads[0].PostId, true)
require.NoError(t, err)
require.Equal(t, true, tr.IsUrgent)
})
tr, _, err = th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, threads.Threads[0].PostId, true)
require.NoError(t, err)
require.NotEmpty(t, tr.Participants[0].Username)
require.Equal(t, false, tr.IsUrgent)
t.Run("should error when not a team member", func(t *testing.T) {
th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam)
defer th.LinkUserToTeam(th.BasicUser, th.BasicTeam)
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.PostPriority = true
cfg.FeatureFlags.PostPriority = true
_, resp, err := th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, model.NewId(), false)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
tr, _, err = th.Client.GetUserThread(th.BasicUser.Id, th.BasicTeam.Id, threads.Threads[0].PostId, true)
require.NoError(t, err)
require.Equal(t, true, tr.IsUrgent)
}
func TestMaintainUnreadMentionsInThread(t *testing.T) {
@@ -7072,6 +7093,23 @@ func TestReadThreads(t *testing.T) {
checkThreadListReplies(t, th, th.Client, th.BasicUser.Id, 1, 1, nil)
})
t.Run("should error when not a team member", func(t *testing.T) {
th.UnlinkUserFromTeam(th.BasicUser, th.BasicTeam)
defer th.LinkUserToTeam(th.BasicUser, th.BasicTeam)
_, resp, err := th.Client.UpdateThreadReadForUser(th.BasicUser.Id, th.BasicTeam.Id, model.NewId(), model.GetMillis())
require.Error(t, err)
CheckForbiddenStatus(t, resp)
_, resp, err = th.Client.SetThreadUnreadByPostId(th.BasicUser.Id, th.BasicTeam.Id, model.NewId(), model.NewId())
require.Error(t, err)
CheckForbiddenStatus(t, resp)
resp, err = th.Client.UpdateThreadsReadForUser(th.BasicUser.Id, th.BasicTeam.Id)
require.Error(t, err)
CheckForbiddenStatus(t, resp)
})
}
func TestMarkThreadUnreadMentionCount(t *testing.T) {