diff --git a/store/sqlstore/group_supplier.go b/store/sqlstore/group_supplier.go index 4b8938c725..6f4ba9fcb5 100644 --- a/store/sqlstore/group_supplier.go +++ b/store/sqlstore/group_supplier.go @@ -775,10 +775,12 @@ func (s *SqlSupplier) TeamMembersToRemove(ctx context.Context, hints ...store.La FROM TeamMembers JOIN Teams ON Teams.Id = TeamMembers.TeamId + LEFT JOIN Bots ON Bots.UserId = TeamMembers.UserId WHERE TeamMembers.DeleteAt = 0 AND Teams.DeleteAt = 0 AND Teams.GroupConstrained = TRUE + AND Bots.UserId IS NULL AND (TeamMembers.TeamId, TeamMembers.UserId) NOT IN ( SELECT @@ -854,9 +856,11 @@ func (s *SqlSupplier) ChannelMembersToRemove(ctx context.Context, hints ...store FROM ChannelMembers JOIN Channels ON Channels.Id = ChannelMembers.ChannelId + LEFT JOIN Bots ON Bots.UserId = ChannelMembers.UserId WHERE Channels.DeleteAt = 0 AND Channels.GroupConstrained = TRUE + AND Bots.UserId IS NULL AND (ChannelMembers.ChannelId, ChannelMembers.UserId) NOT IN ( SELECT diff --git a/store/storetest/group_supplier.go b/store/storetest/group_supplier.go index 7ce444ec0d..a23b1a337f 100644 --- a/store/storetest/group_supplier.go +++ b/store/storetest/group_supplier.go @@ -34,8 +34,8 @@ func TestGroupStore(t *testing.T, ss store.Store) { t.Run("TeamMembersToAdd", func(t *testing.T) { testPendingAutoAddTeamMembers(t, ss) }) t.Run("ChannelMembersToAdd", func(t *testing.T) { testPendingAutoAddChannelMembers(t, ss) }) - t.Run("TeamMembersToRemove", func(t *testing.T) { testPendingTeamMemberRemovals(t, ss) }) - t.Run("ChannelMembersToRemove", func(t *testing.T) { testPendingChannelMemberRemovals(t, ss) }) + t.Run("TeamMembersToRemove", func(t *testing.T) { testTeamMemberRemovals(t, ss) }) + t.Run("ChannelMembersToRemove", func(t *testing.T) { testChannelMemberRemovals(t, ss) }) t.Run("GetGroupsByChannel", func(t *testing.T) { testGetGroupsByChannel(t, ss) }) t.Run("GetGroupsByTeam", func(t *testing.T) { testGetGroupsByTeam(t, ss) }) @@ -1231,7 +1231,7 @@ func testPendingAutoAddChannelMembers(t *testing.T, ss store.Store) { require.Len(t, res.Data, 1) } -func testPendingTeamMemberRemovals(t *testing.T, ss store.Store) { +func testTeamMemberRemovals(t *testing.T, ss store.Store) { data := pendingMemberRemovalsDataSetup(t, ss) // one result when both users are in the group (for user C) @@ -1267,6 +1267,34 @@ func testPendingTeamMemberRemovals(t *testing.T, ss store.Store) { require.Nil(t, res.Err) require.Len(t, res.Data, 3) + // Make one of them a bot + res = <-ss.Group().TeamMembersToRemove() + teamMembers = res.Data.([]*model.TeamMember) + teamMember := teamMembers[0] + bot := &model.Bot{ + UserId: teamMember.UserId, + Username: "un_" + model.NewId(), + DisplayName: "dn_" + model.NewId(), + OwnerId: teamMember.UserId, + } + res = <-ss.Bot().Save(bot) + require.Nil(t, res.Err) + bot = res.Data.(*model.Bot) + + // verify that bot is not returned in results + res = <-ss.Group().TeamMembersToRemove() + require.Nil(t, res.Err) + require.Len(t, res.Data, 2) + + // delete the bot + res = <-ss.Bot().PermanentDelete(bot.UserId) + require.Nil(t, res.Err) + + // Should be back to 3 users + res = <-ss.Group().TeamMembersToRemove() + require.Nil(t, res.Err) + require.Len(t, res.Data, 3) + // add users back to groups res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id) require.Nil(t, res.Err) @@ -1282,7 +1310,7 @@ func testPendingTeamMemberRemovals(t *testing.T, ss store.Store) { require.Nil(t, res.Err) } -func testPendingChannelMemberRemovals(t *testing.T, ss store.Store) { +func testChannelMemberRemovals(t *testing.T, ss store.Store) { data := pendingMemberRemovalsDataSetup(t, ss) // one result when both users are in the group (for user C) @@ -1318,6 +1346,34 @@ func testPendingChannelMemberRemovals(t *testing.T, ss store.Store) { require.Nil(t, res.Err) require.Len(t, res.Data, 3) + // Make one of them a bot + res = <-ss.Group().ChannelMembersToRemove() + channelMembers = res.Data.([]*model.ChannelMember) + channelMember := channelMembers[0] + bot := &model.Bot{ + UserId: channelMember.UserId, + Username: "un_" + model.NewId(), + DisplayName: "dn_" + model.NewId(), + OwnerId: channelMember.UserId, + } + res = <-ss.Bot().Save(bot) + require.Nil(t, res.Err) + bot = res.Data.(*model.Bot) + + // verify that bot is not returned in results + res = <-ss.Group().ChannelMembersToRemove() + require.Nil(t, res.Err) + require.Len(t, res.Data, 2) + + // delete the bot + res = <-ss.Bot().PermanentDelete(bot.UserId) + require.Nil(t, res.Err) + + // Should be back to 3 users + res = <-ss.Group().ChannelMembersToRemove() + require.Nil(t, res.Err) + require.Len(t, res.Data, 3) + // add users back to groups res = <-ss.Team().RemoveMember(data.ConstrainedTeam.Id, data.UserA.Id) require.Nil(t, res.Err)