From e1c0c57d31afbb7fd30fb9eeb31c7f6002c95b3c Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Thu, 29 May 2025 09:35:38 -0400 Subject: [PATCH] [MM-63933] Don't include channels from an archived team unless specified in GetChannelsForUser (#31163) * [MM-63933] Don't include channels from an archived team unless specified in GetChannelsForUser * PR feedback --------- Co-authored-by: Mattermost Build --- .../channels/store/sqlstore/channel_store.go | 30 ++++++++++------ .../channels/store/storetest/channel_store.go | 35 +++++++++++++++---- 2 files changed, 49 insertions(+), 16 deletions(-) diff --git a/server/channels/store/sqlstore/channel_store.go b/server/channels/store/sqlstore/channel_store.go index 98fb61a23f7..63a3a7952ca 100644 --- a/server/channels/store/sqlstore/channel_store.go +++ b/server/channels/store/sqlstore/channel_store.go @@ -1136,17 +1136,18 @@ func (s SqlChannelStore) GetChannels(teamId string, userId string, opts *model.C func (s SqlChannelStore) GetChannelsByUser(userId string, includeDeleted bool, lastDeleteAt, pageSize int, fromChannelID string) (model.ChannelList, error) { query := s.getQueryBuilder(). Select(channelSliceColumns(true, "Channels")...). - From("Channels, ChannelMembers"). + From("Channels, ChannelMembers, Teams"). Where( sq.And{ - sq.Expr("Id = ChannelId"), - sq.Eq{"UserId": userId}, + sq.Expr("Channels.Id = ChannelMembers.ChannelId"), + sq.Expr("Channels.TeamId = Teams.Id"), + sq.Eq{"ChannelMembers.UserId": userId}, }, ). - OrderBy("Id ASC") + OrderBy("Channels.Id ASC") if fromChannelID != "" { - query = query.Where(sq.Gt{"Id": fromChannelID}) + query = query.Where(sq.Gt{"Channels.Id": fromChannelID}) } if pageSize != -1 { @@ -1156,15 +1157,24 @@ func (s SqlChannelStore) GetChannelsByUser(userId string, includeDeleted bool, l if includeDeleted { if lastDeleteAt != 0 { // We filter by non-archived, and archived >= a timestamp. - query = query.Where(sq.Or{ - sq.Eq{"DeleteAt": 0}, - sq.GtOrEq{"DeleteAt": lastDeleteAt}, + query = query.Where(sq.And{ + sq.Or{ + sq.Eq{"Channels.DeleteAt": 0}, + sq.GtOrEq{"Channels.DeleteAt": lastDeleteAt}, + }, + sq.Or{ + sq.Eq{"Teams.DeleteAt": 0}, + sq.GtOrEq{"Teams.DeleteAt": lastDeleteAt}, + }, }) } // If lastDeleteAt is not set, we include everything. That means no filter is needed. } else { - // Don't include archived channels. - query = query.Where(sq.Eq{"DeleteAt": 0}) + // Don't include archived channels or channels from deleted teams + query = query.Where(sq.And{ + sq.Eq{"Channels.DeleteAt": 0}, + sq.Eq{"Teams.DeleteAt": 0}, + }) } sql, args, err := query.ToSql() diff --git a/server/channels/store/storetest/channel_store.go b/server/channels/store/storetest/channel_store.go index 62e65fd3bc7..3d0ff7207e0 100644 --- a/server/channels/store/storetest/channel_store.go +++ b/server/channels/store/storetest/channel_store.go @@ -3820,10 +3820,24 @@ func testChannelStoreGetChannels(t *testing.T, rctx request.CTX, ss store.Store) } func testChannelStoreGetChannelsByUser(t *testing.T, rctx request.CTX, ss store.Store) { - team := model.NewId() - team2 := model.NewId() + team := &model.Team{ + DisplayName: "Team1", + Name: NewTestID(), + Type: model.TeamOpen, + } + team, err := ss.Team().Save(team) + require.NoError(t, err) + + team2 := &model.Team{ + DisplayName: "Team2", + Name: NewTestID(), + Type: model.TeamOpen, + } + team2, err = ss.Team().Save(team2) + require.NoError(t, err) + o1 := model.Channel{} - o1.TeamId = team + o1.TeamId = team.Id o1.DisplayName = "Channel1" o1.Name = NewTestID() o1.Type = model.ChannelTypeOpen @@ -3831,7 +3845,7 @@ func testChannelStoreGetChannelsByUser(t *testing.T, rctx request.CTX, ss store. require.NoError(t, nErr) o2 := model.Channel{} - o2.TeamId = team + o2.TeamId = team.Id o2.DisplayName = "Channel2" o2.Name = NewTestID() o2.Type = model.ChannelTypeOpen @@ -3839,7 +3853,7 @@ func testChannelStoreGetChannelsByUser(t *testing.T, rctx request.CTX, ss store. require.NoError(t, nErr) o3 := model.Channel{} - o3.TeamId = team2 + o3.TeamId = team2.Id o3.DisplayName = "Channel3" o3.Name = NewTestID() o3.Type = model.ChannelTypeOpen @@ -3850,7 +3864,7 @@ func testChannelStoreGetChannelsByUser(t *testing.T, rctx request.CTX, ss store. m1.ChannelId = o1.Id m1.UserId = model.NewId() m1.NotifyProps = model.GetDefaultChannelNotifyProps() - _, err := ss.Channel().SaveMember(rctx, &m1) + _, err = ss.Channel().SaveMember(rctx, &m1) require.NoError(t, err) m2 := model.ChannelMember{} @@ -3908,6 +3922,15 @@ func testChannelStoreGetChannelsByUser(t *testing.T, rctx request.CTX, ss store. require.NoError(t, nErr) require.Len(t, list, 2) require.ElementsMatch(t, []string{o1.Id, o3.Id}, []string{list[0].Id, list[1].Id}, "channels did not match") + + // Archive team and verify channels don't show up + team.DeleteAt = model.GetMillis() + _, nErr = ss.Team().Update(team) + require.NoError(t, nErr) + + // Should return an error since team is archived and there are no results + _, nErr = ss.Channel().GetChannelsByUser(m1.UserId, false, 0, -1, "") + require.Error(t, nErr) } func testChannelStoreGetAllChannels(t *testing.T, rctx request.CTX, ss store.Store, s SqlStore) {