diff --git a/plugins/chat/app/jobs/scheduled/chat/auto_join_users.rb b/plugins/chat/app/jobs/scheduled/chat/auto_join_users.rb index 5d2c50938bf..84d569bc0d3 100644 --- a/plugins/chat/app/jobs/scheduled/chat/auto_join_users.rb +++ b/plugins/chat/app/jobs/scheduled/chat/auto_join_users.rb @@ -5,14 +5,62 @@ module Jobs class AutoJoinUsers < ::Jobs::Scheduled every 1.hour + LAST_SEEN_DAYS = 30 + def execute(_args) return if !SiteSetting.chat_enabled - ::Chat::Channel - .where(auto_join_users: true) - .each do |channel| - ::Chat::ChannelMembershipManager.new(channel).enforce_automatic_channel_memberships - end + allowed_group_permissions = [ + CategoryGroup.permission_types[:create_post], + CategoryGroup.permission_types[:full], + ] + + join_mode = ::Chat::UserChatChannelMembership.join_modes[:automatic] + + sql = <<~SQL + WITH users AS ( + SELECT id FROM users u + JOIN user_options uo ON uo.user_id = u.id + WHERE id > 0 AND (u.suspended_till IS NULL OR u.suspended_till <= :now) + AND (u.last_seen_at IS NULL OR u.last_seen_at > :last_seen_at) + AND u.active + AND NOT u.staged + AND uo.chat_enabled + AND NOT EXISTS (SELECT 1 FROM anonymous_users a WHERE a.user_id = u.id) + ORDER BY last_seen_at desc + LIMIT :max_users + ) + + INSERT INTO user_chat_channel_memberships (user_id, chat_channel_id, following, created_at, updated_at, join_mode) + SELECT DISTINCT users.id AS user_id, + chat_channels.id AS chat_channel_id, + TRUE AS following, + :now::timestamp AS created_at, + :now::timestamp AS updated_at, + :join_mode AS join_mode + FROM users + JOIN chat_channels on auto_join_users AND chatable_type = 'Category' + JOIN categories c on c.id = chat_channels.chatable_id + + LEFT OUTER JOIN user_chat_channel_memberships uccm ON uccm.chat_channel_id = chat_channels.id + AND uccm.user_id = users.id + LEFT OUTER JOIN group_users gu ON gu.user_id = users.id + LEFT OUTER JOIN category_groups cg ON cg.group_id = gu.group_id + AND cg.permission_type in (:allowed_group_permissions) + AND c.id = cg.category_id + + WHERE (cg.group_id is NOT null OR NOT c.read_restricted) AND uccm.id IS NULL + ON CONFLICT DO NOTHING + SQL + + DB.exec( + sql, + now: Time.zone.now, + last_seen_at: LAST_SEEN_DAYS.days.ago, + allowed_group_permissions: allowed_group_permissions, + join_mode: join_mode, + max_users: SiteSetting.max_chat_auto_joined_users, + ) end end end diff --git a/plugins/chat/spec/jobs/scheduled/auto_join_users_spec.rb b/plugins/chat/spec/jobs/scheduled/auto_join_users_spec.rb index 066c56302a4..4198a416eca 100644 --- a/plugins/chat/spec/jobs/scheduled/auto_join_users_spec.rb +++ b/plugins/chat/spec/jobs/scheduled/auto_join_users_spec.rb @@ -3,17 +3,70 @@ describe Jobs::Chat::AutoJoinUsers do subject(:job) { described_class.new } - it "works" do - Jobs.run_immediately! - channel = Fabricate(:category_channel, auto_join_users: true) - user = Fabricate(:user, last_seen_at: 1.minute.ago, active: true) + fab!(:channel) { Fabricate(:category_channel, auto_join_users: true) } + fab!(:user) { Fabricate(:user, last_seen_at: 1.minute.ago, active: true) } + fab!(:group) + fab!(:user_without_chat) do + user = Fabricate(:user) + user.user_option.update!(chat_enabled: false) + user + end + fab!(:stage_user) { Fabricate(:user, staged: true) } + fab!(:suspended_user) { Fabricate(:user, suspended_till: 1.day.from_now) } + fab!(:inactive_user) { Fabricate(:user, active: false) } + fab!(:anonymous_user) { Fabricate(:anonymous) } + + before { Jobs.run_immediately! } + + it "is does not auto join users without permissions" do + channel.category.read_restricted = true + channel.category.set_permissions(group => :full) + channel.category.save! + + job.execute({}) + + membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel) + expect(membership).to be_nil + + GroupUser.create!(group: group, user: user) + + job.execute({}) + + membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel) + expect(membership).to_not be_nil + end + + it "works for simple workflows" do + # this is just to avoid test fragility, we should always have negative users + bot_id = (User.minimum(:id) - 1) + bot_id = -1 if bot_id > 0 + _bot_user = Fabricate(:user, id: bot_id) membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel) expect(membership).to be_nil job.execute({}) + # should exclude bot / inactive / staged / suspended users + # note category fabricator creates a user so we are stuck with that user in the channel + expect( + Chat::UserChatChannelMembership.where(chat_channel: channel).pluck(:user_id), + ).to contain_exactly(user.id, channel.category.user.id) + membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel) expect(membership.following).to eq(true) + + membership.update!(following: false) + job.execute({}) + + membership.reload + expect(membership.following).to eq(false) + + channel = Fabricate(:category_channel, auto_join_users: false) + job.execute({}) + + membership = Chat::UserChatChannelMembership.find_by(user: user, chat_channel: channel) + + expect(membership).to be_nil end end